All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
@ 2016-02-02 17:54 Ramalingam C
  2016-02-02 17:54 ` [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder Ramalingam C
                   ` (7 more replies)
  0 siblings, 8 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-02 17:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: Uma Shankar <uma.shankar@intel.com>

During Charging OS mode, mipi display was blanking.This is
because during driver load, though encoder, connector were
active but crtc returned inactive. This caused sanitize
function to disable the DSI panel. In AOS, this is fine
since HWC will do a modeset and crtc, connector, encoder
mapping will be restored. But in COS, no modeset is called,
it just calls DPMS ON/OFF.

This is fine on BYT/CHT since transcoder is common b/w
all encoders. But for BXT, there is a separate mipi
transcoder. Hence this needs special handling for BXT.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  107 ++++++++++++++++++++++++++++++++--
 1 file changed, 101 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a66220a..f8685f5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7814,6 +7814,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 		   (intel_crtc->config->pipe_src_h - 1));
 }
 
+static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
+				   struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+	struct intel_encoder *encoder;
+	uint32_t tmp;
+
+	tmp = I915_READ(HTOTAL(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_htotal =
+						((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HBLANK(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_hblank_end =
+						((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HSYNC(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_hsync_end =
+						((tmp >> 16) & 0xffff) + 1;
+
+	tmp = I915_READ(VBLANK(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_vblank_end =
+						((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(VSYNC(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_vsync_end =
+						((tmp >> 16) & 0xffff) + 1;
+
+	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
+		pipe_config->base.adjusted_mode.flags |=
+						DRM_MODE_FLAG_INTERLACE;
+		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
+		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
+	}
+
+
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+		struct intel_dsi *intel_dsi =
+			enc_to_intel_dsi(&encoder->base);
+		enum port port;
+
+		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
+		for_each_dsi_port(port, intel_dsi->ports) {
+			pipe_config->base.adjusted_mode.crtc_hdisplay =
+				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
+			pipe_config->base.adjusted_mode.crtc_vdisplay =
+				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
+			pipe_config->base.adjusted_mode.crtc_vtotal =
+				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
+		}
+	}
+
+	tmp = I915_READ(PIPESRC(crtc->pipe));
+	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
+	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
+
+	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
+	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
+}
+
 static void intel_get_pipe_timings(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config)
 {
@@ -9969,6 +10032,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum intel_display_power_domain pfit_domain;
 	uint32_t tmp;
+	bool is_dsi = false;
 
 	if (!intel_display_power_is_enabled(dev_priv,
 					 POWER_DOMAIN_PIPE(crtc->pipe)))
@@ -9999,17 +10063,48 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 			pipe_config->cpu_transcoder = TRANSCODER_EDP;
 	}
 
+	if (dev_priv->vbt.has_mipi) {
+		enum port port_num = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
+					? PORT_A : PORT_C;
+		uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port_num));
+
+		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port_num));
+		if (tmp & DPI_ENABLE) {
+			enum pipe trans_dsi_pipe;
+
+			switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
+			default:
+				WARN(1, "unknown pipe linked to dsi transcoder\n");
+				return false;
+			case BXT_PIPE_SELECT_A:
+				trans_dsi_pipe = PIPE_A;
+				break;
+			case BXT_PIPE_SELECT_B:
+				trans_dsi_pipe = PIPE_B;
+				break;
+			case BXT_PIPE_SELECT_C:
+				trans_dsi_pipe = PIPE_C;
+				break;
+			}
+
+			if (trans_dsi_pipe == crtc->pipe)
+				is_dsi = true;
+		}
+	}
+
 	if (!intel_display_power_is_enabled(dev_priv,
 			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
 		return false;
 
-	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
-	if (!(tmp & PIPECONF_ENABLE))
-		return false;
-
-	haswell_get_ddi_port_state(crtc, pipe_config);
+	if (!is_dsi) {
+		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
+		if (!(tmp & PIPECONF_ENABLE))
+			return false;
 
-	intel_get_pipe_timings(crtc, pipe_config);
+		haswell_get_ddi_port_state(crtc, pipe_config);
+		intel_get_pipe_timings(crtc, pipe_config);
+	} else
+		intel_get_dsi_pipe_timings(crtc, pipe_config);
 
 	if (INTEL_INFO(dev)->gen >= 9) {
 		skl_init_scalers(dev, crtc, pipe_config);
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
@ 2016-02-02 17:54 ` Ramalingam C
  2016-02-03  1:52   ` Thulasimani, Sivakumar
  2016-02-03  8:57   ` Jani Nikula
  2016-02-02 18:23 ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue kbuild test robot
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-02 17:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>

We are re-using Mipi encoder enabled by GOP driver, but not incrementing
reference count for Audio Power domain, so audio was not working. This
patch increments the reference count during DSI init and Adds get/put in
DSI enable/disable functions as well so audio power domain will be on
when mipi is in use.

Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 91cef35..8b43ef6 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -461,6 +461,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 		intel_dsi_port_enable(encoder);
 	}
 
+	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+	intel_audio_codec_enable(encoder);
+
 	intel_panel_enable_backlight(intel_dsi->attached_connector);
 }
 
@@ -531,11 +534,16 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
 
 static void intel_dsi_pre_disable(struct intel_encoder *encoder)
 {
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 	enum port port;
 
 	DRM_DEBUG_KMS("\n");
 
+	intel_audio_codec_disable(encoder);
+	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
+
 	intel_panel_disable_backlight(intel_dsi->attached_connector);
 
 	if (is_vid_mode(intel_dsi)) {
@@ -1236,6 +1244,11 @@ void intel_dsi_init(struct drm_device *dev)
 	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
+	/* Enable Audio Power to fix use-count state machine */
+	port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) ? PORT_A : PORT_C;
+	if (I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)
+		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+
 	return;
 
 err:
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
  2016-02-02 17:54 ` [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder Ramalingam C
@ 2016-02-02 18:23 ` kbuild test robot
  2016-02-02 18:38 ` kbuild test robot
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 33+ messages in thread
From: kbuild test robot @ 2016-02-02 18:23 UTC (permalink / raw)
  To: Ramalingam C; +Cc: jani.nikula, intel-gfx, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]

Hi Uma,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.5-rc2 next-20160201]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Ramalingam-C/drm-i915-BXT-Fixed-COS-blanking-issue/20160203-020606
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x019-201605 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_display.c: In function 'intel_get_dsi_pipe_timings':
>> drivers/gpu/drm/i915/intel_display.c:7858:4: error: implicit declaration of function 'enc_to_intel_dsi' [-Werror=implicit-function-declaration]
       enc_to_intel_dsi(&encoder->base);
       ^
>> drivers/gpu/drm/i915/intel_display.c:7858:4: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>> drivers/gpu/drm/i915/intel_display.c:7861:36: error: dereferencing pointer to incomplete type 'struct intel_dsi'
      pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
                                       ^
>> drivers/gpu/drm/i915/intel_display.c:7862:3: error: implicit declaration of function 'for_each_dsi_port' [-Werror=implicit-function-declaration]
      for_each_dsi_port(port, intel_dsi->ports) {
      ^
>> drivers/gpu/drm/i915/intel_display.c:7862:45: error: expected ';' before '{' token
      for_each_dsi_port(port, intel_dsi->ports) {
                                                ^
   drivers/gpu/drm/i915/intel_display.c: In function 'haswell_get_pipe_config':
>> drivers/gpu/drm/i915/intel_display.c:10079:9: error: 'BXT_PIPE_SELECT_A' undeclared (first use in this function)
       case BXT_PIPE_SELECT_A:
            ^
   drivers/gpu/drm/i915/intel_display.c:10079:9: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/i915/intel_display.c:10082:9: error: 'BXT_PIPE_SELECT_B' undeclared (first use in this function)
       case BXT_PIPE_SELECT_B:
            ^
>> drivers/gpu/drm/i915/intel_display.c:10085:9: error: 'BXT_PIPE_SELECT_C' undeclared (first use in this function)
       case BXT_PIPE_SELECT_C:
            ^
   cc1: some warnings being treated as errors

vim +/enc_to_intel_dsi +7858 drivers/gpu/drm/i915/intel_display.c

  7852			pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
  7853		}
  7854	
  7855	
  7856		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
  7857			struct intel_dsi *intel_dsi =
> 7858				enc_to_intel_dsi(&encoder->base);
  7859			enum port port;
  7860	
> 7861			pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
> 7862			for_each_dsi_port(port, intel_dsi->ports) {
  7863				pipe_config->base.adjusted_mode.crtc_hdisplay =
  7864					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
  7865				pipe_config->base.adjusted_mode.crtc_vdisplay =

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 30488 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
  2016-02-02 17:54 ` [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder Ramalingam C
  2016-02-02 18:23 ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue kbuild test robot
@ 2016-02-02 18:38 ` kbuild test robot
  2016-02-03  1:49 ` Thulasimani, Sivakumar
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 33+ messages in thread
From: kbuild test robot @ 2016-02-02 18:38 UTC (permalink / raw)
  To: Ramalingam C; +Cc: jani.nikula, intel-gfx, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3167 bytes --]

Hi Uma,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.5-rc2 next-20160201]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Ramalingam-C/drm-i915-BXT-Fixed-COS-blanking-issue/20160203-020606
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-rhel (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_display.c: In function 'intel_get_dsi_pipe_timings':
   drivers/gpu/drm/i915/intel_display.c:7858:4: error: implicit declaration of function 'enc_to_intel_dsi' [-Werror=implicit-function-declaration]
       enc_to_intel_dsi(&encoder->base);
       ^
   drivers/gpu/drm/i915/intel_display.c:7858:4: warning: initialization makes pointer from integer without a cast
>> drivers/gpu/drm/i915/intel_display.c:7861:36: error: dereferencing pointer to incomplete type
      pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
                                       ^
   drivers/gpu/drm/i915/intel_display.c:7862:3: error: implicit declaration of function 'for_each_dsi_port' [-Werror=implicit-function-declaration]
      for_each_dsi_port(port, intel_dsi->ports) {
      ^
   drivers/gpu/drm/i915/intel_display.c:7862:36: error: dereferencing pointer to incomplete type
      for_each_dsi_port(port, intel_dsi->ports) {
                                       ^
   drivers/gpu/drm/i915/intel_display.c:7862:45: error: expected ';' before '{' token
      for_each_dsi_port(port, intel_dsi->ports) {
                                                ^
   drivers/gpu/drm/i915/intel_display.c: In function 'haswell_get_pipe_config':
   drivers/gpu/drm/i915/intel_display.c:10079:9: error: 'BXT_PIPE_SELECT_A' undeclared (first use in this function)
       case BXT_PIPE_SELECT_A:
            ^
   drivers/gpu/drm/i915/intel_display.c:10079:9: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/i915/intel_display.c:10082:9: error: 'BXT_PIPE_SELECT_B' undeclared (first use in this function)
       case BXT_PIPE_SELECT_B:
            ^
   drivers/gpu/drm/i915/intel_display.c:10085:9: error: 'BXT_PIPE_SELECT_C' undeclared (first use in this function)
       case BXT_PIPE_SELECT_C:
            ^
   cc1: some warnings being treated as errors

vim +7861 drivers/gpu/drm/i915/intel_display.c

  7852			pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
  7853		}
  7854	
  7855	
  7856		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
  7857			struct intel_dsi *intel_dsi =
> 7858				enc_to_intel_dsi(&encoder->base);
  7859			enum port port;
  7860	
> 7861			pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
  7862			for_each_dsi_port(port, intel_dsi->ports) {
  7863				pipe_config->base.adjusted_mode.crtc_hdisplay =
  7864					I915_READ(BXT_MIPI_TRANS_HACTIVE(port));

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 35609 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
                   ` (2 preceding siblings ...)
  2016-02-02 18:38 ` kbuild test robot
@ 2016-02-03  1:49 ` Thulasimani, Sivakumar
  2016-02-03 12:20   ` Ramalingam C
  2016-02-03  8:02 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 33+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-03  1:49 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx; +Cc: jani.nikula



On 2/2/2016 11:24 PM, Ramalingam C wrote:
> From: Uma Shankar <uma.shankar@intel.com>
>
> During Charging OS mode, mipi display was blanking.This is
> because during driver load, though encoder, connector were
> active but crtc returned inactive. This caused sanitize
> function to disable the DSI panel. In AOS, this is fine
> since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in COS, no modeset is called,
> it just calls DPMS ON/OFF.
>
> This is fine on BYT/CHT since transcoder is common b/w
> all encoders. But for BXT, there is a separate mipi
> transcoder. Hence this needs special handling for BXT.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c |  107 ++++++++++++++++++++++++++++++++--
>   1 file changed, 101 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a66220a..f8685f5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7814,6 +7814,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>   		   (intel_crtc->config->pipe_src_h - 1));
>   }
>   
> +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
> +				   struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	struct intel_encoder *encoder;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(HTOTAL(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_htotal =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	tmp = I915_READ(VBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(VSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
> +		pipe_config->base.adjusted_mode.flags |=
> +						DRM_MODE_FLAG_INTERLACE;
> +		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
> +		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
> +	}
> +
> +
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		struct intel_dsi *intel_dsi =
> +			enc_to_intel_dsi(&encoder->base);
> +		enum port port;
> +
> +		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			pipe_config->base.adjusted_mode.crtc_hdisplay =
> +				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vdisplay =
> +				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vtotal =
> +				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +		}
> +	}
> +
> +	tmp = I915_READ(PIPESRC(crtc->pipe));
> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
> +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
> +}
> +
>   static void intel_get_pipe_timings(struct intel_crtc *crtc,
>   				   struct intel_crtc_state *pipe_config)
>   {
> @@ -9969,6 +10032,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	enum intel_display_power_domain pfit_domain;
>   	uint32_t tmp;
> +	bool is_dsi = false;
>   
>   	if (!intel_display_power_is_enabled(dev_priv,
>   					 POWER_DOMAIN_PIPE(crtc->pipe)))
> @@ -9999,17 +10063,48 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>   			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>   	}
>   
> +	if (dev_priv->vbt.has_mipi) {
shouldn't the above check for bxt too ? since the section inside is only 
for bxt ?
regards,
Sivakumar
> +		enum port port_num = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
> +					? PORT_A : PORT_C;
> +		uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port_num));
> +
> +		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port_num));
> +		if (tmp & DPI_ENABLE) {
> +			enum pipe trans_dsi_pipe;
> +
> +			switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
> +			default:
> +				WARN(1, "unknown pipe linked to dsi transcoder\n");
> +				return false;
> +			case BXT_PIPE_SELECT_A:
> +				trans_dsi_pipe = PIPE_A;
> +				break;
> +			case BXT_PIPE_SELECT_B:
> +				trans_dsi_pipe = PIPE_B;
> +				break;
> +			case BXT_PIPE_SELECT_C:
> +				trans_dsi_pipe = PIPE_C;
> +				break;
> +			}
> +
> +			if (trans_dsi_pipe == crtc->pipe)
> +				is_dsi = true;
> +		}
> +	}
> +
>   	if (!intel_display_power_is_enabled(dev_priv,
>   			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>   		return false;
>   
> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> -	if (!(tmp & PIPECONF_ENABLE))
> -		return false;
> -
> -	haswell_get_ddi_port_state(crtc, pipe_config);
> +	if (!is_dsi) {
> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> +		if (!(tmp & PIPECONF_ENABLE))
> +			return false;
>   
> -	intel_get_pipe_timings(crtc, pipe_config);
> +		haswell_get_ddi_port_state(crtc, pipe_config);
> +		intel_get_pipe_timings(crtc, pipe_config);
> +	} else
> +		intel_get_dsi_pipe_timings(crtc, pipe_config);
>   
>   	if (INTEL_INFO(dev)->gen >= 9) {
>   		skl_init_scalers(dev, crtc, pipe_config);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder
  2016-02-02 17:54 ` [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder Ramalingam C
@ 2016-02-03  1:52   ` Thulasimani, Sivakumar
  2016-02-03  8:57   ` Jani Nikula
  1 sibling, 0 replies; 33+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-03  1:52 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx; +Cc: jani.nikula

Again like previous patch can you add checks for IS_BROXTON for all of 
these changes so it does not affect CHV/VLV ?

regards,
Sivakumar

On 2/2/2016 11:24 PM, Ramalingam C wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>
> We are re-using Mipi encoder enabled by GOP driver, but not incrementing
> reference count for Audio Power domain, so audio was not working. This
> patch increments the reference count during DSI init and Adds get/put in
> DSI enable/disable functions as well so audio power domain will be on
> when mipi is in use.
>
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 91cef35..8b43ef6 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -461,6 +461,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>   		intel_dsi_port_enable(encoder);
>   	}
>   
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> +	intel_audio_codec_enable(encoder);
> +
>   	intel_panel_enable_backlight(intel_dsi->attached_connector);
>   }
>   
> @@ -531,11 +534,16 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>   
>   static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>   {
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>   	enum port port;
>   
>   	DRM_DEBUG_KMS("\n");
>   
> +	intel_audio_codec_disable(encoder);
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> +
>   	intel_panel_disable_backlight(intel_dsi->attached_connector);
>   
>   	if (is_vid_mode(intel_dsi)) {
> @@ -1236,6 +1244,11 @@ void intel_dsi_init(struct drm_device *dev)
>   	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>   	intel_panel_setup_backlight(connector, INVALID_PIPE);
>   
> +	/* Enable Audio Power to fix use-count state machine */
> +	port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) ? PORT_A : PORT_C;
> +	if (I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)
> +		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> +
>   	return;
>   
>   err:

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
                   ` (3 preceding siblings ...)
  2016-02-03  1:49 ` Thulasimani, Sivakumar
@ 2016-02-03  8:02 ` Patchwork
  2016-02-03  9:44 ` [PATCH 1/2] " Jani Nikula
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2016-02-03  8:02 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx

== Summary ==

   ^
drivers/gpu/drm/i915/intel_display.c:7862:36: error: dereferencing pointer to incomplete type
   for_each_dsi_port(port, intel_dsi->ports) {
                                    ^
drivers/gpu/drm/i915/intel_display.c:7862:45: error: expected ';' before '{' token
   for_each_dsi_port(port, intel_dsi->ports) {
                                             ^
drivers/gpu/drm/i915/intel_display.c: In function 'haswell_get_pipe_config':
drivers/gpu/drm/i915/intel_display.c:10079:9: error: 'BXT_PIPE_SELECT_A' undeclared (first use in this function)
    case BXT_PIPE_SELECT_A:
         ^
drivers/gpu/drm/i915/intel_display.c:10079:9: note: each undeclared identifier is reported only once for each function it appears in
drivers/gpu/drm/i915/intel_display.c:10082:9: error: 'BXT_PIPE_SELECT_B' undeclared (first use in this function)
    case BXT_PIPE_SELECT_B:
         ^
drivers/gpu/drm/i915/intel_display.c:10085:9: error: 'BXT_PIPE_SELECT_C' undeclared (first use in this function)
    case BXT_PIPE_SELECT_C:
         ^
cc1: some warnings being treated as errors
scripts/Makefile.build:258: recipe for target 'drivers/gpu/drm/i915/intel_display.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_display.o] Error 1
make[4]: *** Waiting for unfinished jobs....
scripts/Makefile.build:407: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:407: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:407: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:950: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder
  2016-02-02 17:54 ` [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder Ramalingam C
  2016-02-03  1:52   ` Thulasimani, Sivakumar
@ 2016-02-03  8:57   ` Jani Nikula
  2016-02-03  9:24     ` Ramalingam C
  1 sibling, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2016-02-03  8:57 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx; +Cc: Jakobsson, Patrik

On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>
> We are re-using Mipi encoder enabled by GOP driver, but not incrementing
> reference count for Audio Power domain, so audio was not working. This
> patch increments the reference count during DSI init and Adds get/put in
> DSI enable/disable functions as well so audio power domain will be on
> when mipi is in use.

Confused. DSI does not have HD audio. The codec enable calls are no-ops
because DSI does not have EDID and therefore no ELD either. Hopefully
the codec disable calls don't mess up things by accident. We don't need
or want those calls in DSI.

IMO the DSI encoder should not get/put the audio power domain either,
because DSI simply does not use that.

The question remains, *what* audio was failing? DP/HDMI audio?

We may have problems in the power domain setup code, failing to take DSI
into account or something. But I don't think this is the place to fix
it.

Cc Imre and Patrik.


BR,
Jani.




>
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 91cef35..8b43ef6 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -461,6 +461,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>  		intel_dsi_port_enable(encoder);
>  	}
>  
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> +	intel_audio_codec_enable(encoder);
> +
>  	intel_panel_enable_backlight(intel_dsi->attached_connector);
>  }
>  
> @@ -531,11 +534,16 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>  
>  static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>  {
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	enum port port;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	intel_audio_codec_disable(encoder);
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
> +
>  	intel_panel_disable_backlight(intel_dsi->attached_connector);
>  
>  	if (is_vid_mode(intel_dsi)) {
> @@ -1236,6 +1244,11 @@ void intel_dsi_init(struct drm_device *dev)
>  	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>  	intel_panel_setup_backlight(connector, INVALID_PIPE);
>  
> +	/* Enable Audio Power to fix use-count state machine */
> +	port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) ? PORT_A : PORT_C;
> +	if (I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)
> +		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> +
>  	return;
>  
>  err:

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder
  2016-02-03  8:57   ` Jani Nikula
@ 2016-02-03  9:24     ` Ramalingam C
  2016-02-03 10:01       ` Jani Nikula
  0 siblings, 1 reply; 33+ messages in thread
From: Ramalingam C @ 2016-02-03  9:24 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Jakobsson, Patrik


On Wednesday 03 February 2016 02:27 PM, Jani Nikula wrote:
> On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>
>> We are re-using Mipi encoder enabled by GOP driver, but not incrementing
>> reference count for Audio Power domain, so audio was not working. This
>> patch increments the reference count during DSI init and Adds get/put in
>> DSI enable/disable functions as well so audio power domain will be on
>> when mipi is in use.
> Confused. DSI does not have HD audio. The codec enable calls are no-ops
> because DSI does not have EDID and therefore no ELD either. Hopefully
> the codec disable calls don't mess up things by accident. We don't need
> or want those calls in DSI.
>
> IMO the DSI encoder should not get/put the audio power domain either,
> because DSI simply does not use that.
>
> The question remains, *what* audio was failing? DP/HDMI audio?
Since HDMI and DP has the audio we are handling the audio power well.
For MIPI since we dont have the audio with encoder, we need not get the 
power well on.
In such scenario, power well use count is not incrementing (Means no one 
is enabling
the powerwell for board audio) and hence Board audio is not working.
>
> We may have problems in the power domain setup code, failing to take DSI
> into account or something. But I don't think this is the place to fix
> it.
Possibly this is not the place. But Either DSI code or audio code has to 
handle it,
to make the board audio to work on BXT. Share your view on that.

>
> Cc Imre and Patrik.
>
>
> BR,
> Jani.
>
>
>
>
>> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 91cef35..8b43ef6 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -461,6 +461,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>>   		intel_dsi_port_enable(encoder);
>>   	}
>>   
>> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>> +	intel_audio_codec_enable(encoder);
>> +
>>   	intel_panel_enable_backlight(intel_dsi->attached_connector);
>>   }
>>   
>> @@ -531,11 +534,16 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>>   
>>   static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>>   {
>> +	struct drm_device *dev = encoder->base.dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>   	enum port port;
>>   
>>   	DRM_DEBUG_KMS("\n");
>>   
>> +	intel_audio_codec_disable(encoder);
>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
>> +
>>   	intel_panel_disable_backlight(intel_dsi->attached_connector);
>>   
>>   	if (is_vid_mode(intel_dsi)) {
>> @@ -1236,6 +1244,11 @@ void intel_dsi_init(struct drm_device *dev)
>>   	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>>   	intel_panel_setup_backlight(connector, INVALID_PIPE);
>>   
>> +	/* Enable Audio Power to fix use-count state machine */
>> +	port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) ? PORT_A : PORT_C;
>> +	if (I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)
>> +		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>> +
>>   	return;
>>   
>>   err:

-- 
Thanks,
--Ram

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
                   ` (4 preceding siblings ...)
  2016-02-03  8:02 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
@ 2016-02-03  9:44 ` Jani Nikula
  2016-02-03 12:18   ` Ramalingam C
                     ` (2 more replies)
  2016-02-03 13:12 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Using the bpp value wrt the pixel format (rev2) Patchwork
  2016-02-15 16:24 ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Daniel Vetter
  7 siblings, 3 replies; 33+ messages in thread
From: Jani Nikula @ 2016-02-03  9:44 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx

On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> From: Uma Shankar <uma.shankar@intel.com>
>
> During Charging OS mode, mipi display was blanking.This is
> because during driver load, though encoder, connector were
> active but crtc returned inactive. This caused sanitize
> function to disable the DSI panel. In AOS, this is fine
> since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in COS, no modeset is called,
> it just calls DPMS ON/OFF.
>
> This is fine on BYT/CHT since transcoder is common b/w
> all encoders. But for BXT, there is a separate mipi
> transcoder. Hence this needs special handling for BXT.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  107 ++++++++++++++++++++++++++++++++--
>  1 file changed, 101 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a66220a..f8685f5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7814,6 +7814,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  		   (intel_crtc->config->pipe_src_h - 1));
>  }
>  
> +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
> +				   struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	struct intel_encoder *encoder;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(HTOTAL(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_htotal =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	tmp = I915_READ(VBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(VSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
> +		pipe_config->base.adjusted_mode.flags |=
> +						DRM_MODE_FLAG_INTERLACE;
> +		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
> +		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
> +	}
> +
> +
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		struct intel_dsi *intel_dsi =
> +			enc_to_intel_dsi(&encoder->base);
> +		enum port port;
> +
> +		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			pipe_config->base.adjusted_mode.crtc_hdisplay =
> +				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vdisplay =
> +				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vtotal =
> +				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +		}
> +	}
> +
> +	tmp = I915_READ(PIPESRC(crtc->pipe));
> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
> +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
> +}
> +
>  static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config)
>  {
> @@ -9969,6 +10032,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum intel_display_power_domain pfit_domain;
>  	uint32_t tmp;
> +	bool is_dsi = false;
>  
>  	if (!intel_display_power_is_enabled(dev_priv,
>  					 POWER_DOMAIN_PIPE(crtc->pipe)))
> @@ -9999,17 +10063,48 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>  	}
>  
> +	if (dev_priv->vbt.has_mipi) {
> +		enum port port_num = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
> +					? PORT_A : PORT_C;

Just "enum port port" please.

> +		uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port_num));
> +
> +		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port_num));
> +		if (tmp & DPI_ENABLE) {
> +			enum pipe trans_dsi_pipe;
> +
> +			switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
> +			default:
> +				WARN(1, "unknown pipe linked to dsi transcoder\n");
> +				return false;
> +			case BXT_PIPE_SELECT_A:
> +				trans_dsi_pipe = PIPE_A;
> +				break;
> +			case BXT_PIPE_SELECT_B:
> +				trans_dsi_pipe = PIPE_B;
> +				break;
> +			case BXT_PIPE_SELECT_C:
> +				trans_dsi_pipe = PIPE_C;
> +				break;
> +			}
> +
> +			if (trans_dsi_pipe == crtc->pipe)
> +				is_dsi = true;
> +		}
> +	}
> +

You need to set pipe_config->has_dsi_encoder.

This is possibly cleanest, if you abstract the above into a separate
function, say

bxt_get_pipe_config_dsi(struct intel_crtc *crtc, struct intel_crtc_state
*pipe_config)

and set pipe_config->has_dsi_encoder if trans_dsi_pipe == crtc->pipe.

This probably needs to take into account the different power domains as
well?

BR,
Jani.


PS. I guess we're missing the has_dsi_encoder readout for BYT/CHV as
well. :(



>  	if (!intel_display_power_is_enabled(dev_priv,
>  			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>  		return false;
>  
> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> -	if (!(tmp & PIPECONF_ENABLE))
> -		return false;
> -
> -	haswell_get_ddi_port_state(crtc, pipe_config);
> +	if (!is_dsi) {
> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> +		if (!(tmp & PIPECONF_ENABLE))
> +			return false;
>  
> -	intel_get_pipe_timings(crtc, pipe_config);
> +		haswell_get_ddi_port_state(crtc, pipe_config);
> +		intel_get_pipe_timings(crtc, pipe_config);
> +	} else
> +		intel_get_dsi_pipe_timings(crtc, pipe_config);
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
>  		skl_init_scalers(dev, crtc, pipe_config);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder
  2016-02-03  9:24     ` Ramalingam C
@ 2016-02-03 10:01       ` Jani Nikula
  2016-02-19  9:23         ` Jani Nikula
  0 siblings, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2016-02-03 10:01 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx; +Cc: Jakobsson, Patrik

On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> On Wednesday 03 February 2016 02:27 PM, Jani Nikula wrote:
>> On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>>
>>> We are re-using Mipi encoder enabled by GOP driver, but not incrementing
>>> reference count for Audio Power domain, so audio was not working. This
>>> patch increments the reference count during DSI init and Adds get/put in
>>> DSI enable/disable functions as well so audio power domain will be on
>>> when mipi is in use.
>> Confused. DSI does not have HD audio. The codec enable calls are no-ops
>> because DSI does not have EDID and therefore no ELD either. Hopefully
>> the codec disable calls don't mess up things by accident. We don't need
>> or want those calls in DSI.
>>
>> IMO the DSI encoder should not get/put the audio power domain either,
>> because DSI simply does not use that.
>>
>> The question remains, *what* audio was failing? DP/HDMI audio?
> Since HDMI and DP has the audio we are handling the audio power well.
> For MIPI since we dont have the audio with encoder, we need not get the 
> power well on.
> In such scenario, power well use count is not incrementing (Means no one 
> is enabling
> the powerwell for board audio) and hence Board audio is not working.
>>
>> We may have problems in the power domain setup code, failing to take DSI
>> into account or something. But I don't think this is the place to fix
>> it.
> Possibly this is not the place. But Either DSI code or audio code has to 
> handle it,
> to make the board audio to work on BXT. Share your view on that.

I'm not aware of what board audio is, but based on your description I
understand it has nothing to do with HDMI, DP, DSI - or our driver in
general. Correct me if I'm wrong.

Which driver handles board audio?

Sounds like the right approach is to have that driver use the i915
component API to get/put the power well.

See sound/hda/hdac_i915.c for how HDA binds to the component API, and
how it calls the ->get_power() and ->put_power() hooks as
needed. Without that, HDA would lose all the registers in audio power
well during mode set. I expect the same for board audio.

The modeset for HDMI/DP need to get/put the power wells just because
those encoders do the codec enable/disable sequence, actually touching
registers in the power well.

BR,
Jani.



>
>>
>> Cc Imre and Patrik.
>>
>>
>> BR,
>> Jani.
>>
>>
>>
>>
>>> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>> index 91cef35..8b43ef6 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>> @@ -461,6 +461,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>>>   		intel_dsi_port_enable(encoder);
>>>   	}
>>>   
>>> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>>> +	intel_audio_codec_enable(encoder);
>>> +
>>>   	intel_panel_enable_backlight(intel_dsi->attached_connector);
>>>   }
>>>   
>>> @@ -531,11 +534,16 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>>>   
>>>   static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>>>   {
>>> +	struct drm_device *dev = encoder->base.dev;
>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>>   	enum port port;
>>>   
>>>   	DRM_DEBUG_KMS("\n");
>>>   
>>> +	intel_audio_codec_disable(encoder);
>>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
>>> +
>>>   	intel_panel_disable_backlight(intel_dsi->attached_connector);
>>>   
>>>   	if (is_vid_mode(intel_dsi)) {
>>> @@ -1236,6 +1244,11 @@ void intel_dsi_init(struct drm_device *dev)
>>>   	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>>>   	intel_panel_setup_backlight(connector, INVALID_PIPE);
>>>   
>>> +	/* Enable Audio Power to fix use-count state machine */
>>> +	port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) ? PORT_A : PORT_C;
>>> +	if (I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)
>>> +		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>>> +
>>>   	return;
>>>   
>>>   err:

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-03  9:44 ` [PATCH 1/2] " Jani Nikula
@ 2016-02-03 12:18   ` Ramalingam C
  2016-02-03 12:27   ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Ramalingam C
  2016-02-15 16:28   ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Daniel Vetter
  2 siblings, 0 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-03 12:18 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx

Hi,

On Wednesday 03 February 2016 03:14 PM, Jani Nikula wrote:
> On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>> From: Uma Shankar <uma.shankar@intel.com>
>>
>> During Charging OS mode, mipi display was blanking.This is
>> because during driver load, though encoder, connector were
>> active but crtc returned inactive. This caused sanitize
>> function to disable the DSI panel. In AOS, this is fine
>> since HWC will do a modeset and crtc, connector, encoder
>> mapping will be restored. But in COS, no modeset is called,
>> it just calls DPMS ON/OFF.
>>
>> This is fine on BYT/CHT since transcoder is common b/w
>> all encoders. But for BXT, there is a separate mipi
>> transcoder. Hence this needs special handling for BXT.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  107 ++++++++++++++++++++++++++++++++--
>>   1 file changed, 101 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index a66220a..f8685f5 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7814,6 +7814,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>>   		   (intel_crtc->config->pipe_src_h - 1));
>>   }
>>   
>> +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
>> +				   struct intel_crtc_state *pipe_config)
>> +{
>> +	struct drm_device *dev = crtc->base.dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>> +	struct intel_encoder *encoder;
>> +	uint32_t tmp;
>> +
>> +	tmp = I915_READ(HTOTAL(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_htotal =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +	tmp = I915_READ(HBLANK(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_hblank_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +	tmp = I915_READ(HSYNC(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_hsync_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +
>> +	tmp = I915_READ(VBLANK(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_vblank_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +	tmp = I915_READ(VSYNC(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_vsync_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +
>> +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
>> +		pipe_config->base.adjusted_mode.flags |=
>> +						DRM_MODE_FLAG_INTERLACE;
>> +		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
>> +		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>> +	}
>> +
>> +
>> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> +		struct intel_dsi *intel_dsi =
>> +			enc_to_intel_dsi(&encoder->base);
>> +		enum port port;
>> +
>> +		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
>> +		for_each_dsi_port(port, intel_dsi->ports) {
>> +			pipe_config->base.adjusted_mode.crtc_hdisplay =
>> +				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
>> +			pipe_config->base.adjusted_mode.crtc_vdisplay =
>> +				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
>> +			pipe_config->base.adjusted_mode.crtc_vtotal =
>> +				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
>> +		}
>> +	}
>> +
>> +	tmp = I915_READ(PIPESRC(crtc->pipe));
>> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
>> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
>> +
>> +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
>> +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
>> +}
>> +
>>   static void intel_get_pipe_timings(struct intel_crtc *crtc,
>>   				   struct intel_crtc_state *pipe_config)
>>   {
>> @@ -9969,6 +10032,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	enum intel_display_power_domain pfit_domain;
>>   	uint32_t tmp;
>> +	bool is_dsi = false;
>>   
>>   	if (!intel_display_power_is_enabled(dev_priv,
>>   					 POWER_DOMAIN_PIPE(crtc->pipe)))
>> @@ -9999,17 +10063,48 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>   			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>>   	}
>>   
>> +	if (dev_priv->vbt.has_mipi) {
>> +		enum port port_num = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
>> +					? PORT_A : PORT_C;
> Just "enum port port" please.
I will take care of this suggestion in the next patch ver.
>
>> +		uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port_num));
>> +
>> +		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port_num));
>> +		if (tmp & DPI_ENABLE) {
>> +			enum pipe trans_dsi_pipe;
>> +
>> +			switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
>> +			default:
>> +				WARN(1, "unknown pipe linked to dsi transcoder\n");
>> +				return false;
>> +			case BXT_PIPE_SELECT_A:
>> +				trans_dsi_pipe = PIPE_A;
>> +				break;
>> +			case BXT_PIPE_SELECT_B:
>> +				trans_dsi_pipe = PIPE_B;
>> +				break;
>> +			case BXT_PIPE_SELECT_C:
>> +				trans_dsi_pipe = PIPE_C;
>> +				break;
>> +			}
>> +
>> +			if (trans_dsi_pipe == crtc->pipe)
>> +				is_dsi = true;
>> +		}
>> +	}
>> +
> You need to set pipe_config->has_dsi_encoder.
>
> This is possibly cleanest, if you abstract the above into a separate
> function, say
>
> bxt_get_pipe_config_dsi(struct intel_crtc *crtc, struct intel_crtc_state
> *pipe_config)
>
> and set pipe_config->has_dsi_encoder if trans_dsi_pipe == crtc->pipe.
Ok. I am resubmitting with these changes.
>
> This probably needs to take into account the different power domains as
> well?
>
> BR,
> Jani.
>
>
> PS. I guess we're missing the has_dsi_encoder readout for BYT/CHV as
> well. :(
>
>
>
>>   	if (!intel_display_power_is_enabled(dev_priv,
>>   			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>>   		return false;
>>   
>> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> -	if (!(tmp & PIPECONF_ENABLE))
>> -		return false;
>> -
>> -	haswell_get_ddi_port_state(crtc, pipe_config);
>> +	if (!is_dsi) {
>> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> +		if (!(tmp & PIPECONF_ENABLE))
>> +			return false;
>>   
>> -	intel_get_pipe_timings(crtc, pipe_config);
>> +		haswell_get_ddi_port_state(crtc, pipe_config);
>> +		intel_get_pipe_timings(crtc, pipe_config);
>> +	} else
>> +		intel_get_dsi_pipe_timings(crtc, pipe_config);
>>   
>>   	if (INTEL_INFO(dev)->gen >= 9) {
>>   		skl_init_scalers(dev, crtc, pipe_config);

-- 
Thanks,
--Ram

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-03  1:49 ` Thulasimani, Sivakumar
@ 2016-02-03 12:20   ` Ramalingam C
  0 siblings, 0 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-03 12:20 UTC (permalink / raw)
  To: Thulasimani, Sivakumar, intel-gfx; +Cc: jani.nikula


On Wednesday 03 February 2016 07:19 AM, Thulasimani, Sivakumar wrote:
>
>
> On 2/2/2016 11:24 PM, Ramalingam C wrote:
>> From: Uma Shankar <uma.shankar@intel.com>
>>
>> During Charging OS mode, mipi display was blanking.This is
>> because during driver load, though encoder, connector were
>> active but crtc returned inactive. This caused sanitize
>> function to disable the DSI panel. In AOS, this is fine
>> since HWC will do a modeset and crtc, connector, encoder
>> mapping will be restored. But in COS, no modeset is called,
>> it just calls DPMS ON/OFF.
>>
>> This is fine on BYT/CHT since transcoder is common b/w
>> all encoders. But for BXT, there is a separate mipi
>> transcoder. Hence this needs special handling for BXT.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  107 
>> ++++++++++++++++++++++++++++++++--
>>   1 file changed, 101 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index a66220a..f8685f5 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7814,6 +7814,69 @@ static void intel_set_pipe_timings(struct 
>> intel_crtc *intel_crtc)
>>              (intel_crtc->config->pipe_src_h - 1));
>>   }
>>   +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
>> +                   struct intel_crtc_state *pipe_config)
>> +{
>> +    struct drm_device *dev = crtc->base.dev;
>> +    struct drm_i915_private *dev_priv = dev->dev_private;
>> +    enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>> +    struct intel_encoder *encoder;
>> +    uint32_t tmp;
>> +
>> +    tmp = I915_READ(HTOTAL(cpu_transcoder));
>> +    pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
>> +    pipe_config->base.adjusted_mode.crtc_htotal =
>> +                        ((tmp >> 16) & 0xffff) + 1;
>> +    tmp = I915_READ(HBLANK(cpu_transcoder));
>> +    pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 
>> 0xffff) + 1;
>> +    pipe_config->base.adjusted_mode.crtc_hblank_end =
>> +                        ((tmp >> 16) & 0xffff) + 1;
>> +    tmp = I915_READ(HSYNC(cpu_transcoder));
>> +    pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 
>> 0xffff) + 1;
>> +    pipe_config->base.adjusted_mode.crtc_hsync_end =
>> +                        ((tmp >> 16) & 0xffff) + 1;
>> +
>> +    tmp = I915_READ(VBLANK(cpu_transcoder));
>> +    pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 
>> 0xffff) + 1;
>> +    pipe_config->base.adjusted_mode.crtc_vblank_end =
>> +                        ((tmp >> 16) & 0xffff) + 1;
>> +    tmp = I915_READ(VSYNC(cpu_transcoder));
>> +    pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 
>> 0xffff) + 1;
>> +    pipe_config->base.adjusted_mode.crtc_vsync_end =
>> +                        ((tmp >> 16) & 0xffff) + 1;
>> +
>> +    if (I915_READ(PIPECONF(cpu_transcoder)) & 
>> PIPECONF_INTERLACE_MASK) {
>> +        pipe_config->base.adjusted_mode.flags |=
>> +                        DRM_MODE_FLAG_INTERLACE;
>> +        pipe_config->base.adjusted_mode.crtc_vtotal += 1;
>> +        pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>> +    }
>> +
>> +
>> +    for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> +        struct intel_dsi *intel_dsi =
>> +            enc_to_intel_dsi(&encoder->base);
>> +        enum port port;
>> +
>> +        pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
>> +        for_each_dsi_port(port, intel_dsi->ports) {
>> +            pipe_config->base.adjusted_mode.crtc_hdisplay =
>> +                I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
>> +            pipe_config->base.adjusted_mode.crtc_vdisplay =
>> +                I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
>> +            pipe_config->base.adjusted_mode.crtc_vtotal =
>> +                I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
>> +        }
>> +    }
>> +
>> +    tmp = I915_READ(PIPESRC(crtc->pipe));
>> +    pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
>> +    pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
>> +
>> +    pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
>> +    pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
>> +}
>> +
>>   static void intel_get_pipe_timings(struct intel_crtc *crtc,
>>                      struct intel_crtc_state *pipe_config)
>>   {
>> @@ -9969,6 +10032,7 @@ static bool haswell_get_pipe_config(struct 
>> intel_crtc *crtc,
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       enum intel_display_power_domain pfit_domain;
>>       uint32_t tmp;
>> +    bool is_dsi = false;
>>         if (!intel_display_power_is_enabled(dev_priv,
>>                        POWER_DOMAIN_PIPE(crtc->pipe)))
>> @@ -9999,17 +10063,48 @@ static bool haswell_get_pipe_config(struct 
>> intel_crtc *crtc,
>>               pipe_config->cpu_transcoder = TRANSCODER_EDP;
>>       }
>>   +    if (dev_priv->vbt.has_mipi) {
> shouldn't the above check for bxt too ? since the section inside is 
> only for bxt ?
Thanks siva. Will take care of this in the next version of the patch.
> regards,
> Sivakumar
>> +        enum port port_num = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
>> +                    ? PORT_A : PORT_C;
>> +        uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port_num));
>> +
>> +        tmp = I915_READ(BXT_MIPI_PORT_CTRL(port_num));
>> +        if (tmp & DPI_ENABLE) {
>> +            enum pipe trans_dsi_pipe;
>> +
>> +            switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
>> +            default:
>> +                WARN(1, "unknown pipe linked to dsi transcoder\n");
>> +                return false;
>> +            case BXT_PIPE_SELECT_A:
>> +                trans_dsi_pipe = PIPE_A;
>> +                break;
>> +            case BXT_PIPE_SELECT_B:
>> +                trans_dsi_pipe = PIPE_B;
>> +                break;
>> +            case BXT_PIPE_SELECT_C:
>> +                trans_dsi_pipe = PIPE_C;
>> +                break;
>> +            }
>> +
>> +            if (trans_dsi_pipe == crtc->pipe)
>> +                is_dsi = true;
>> +        }
>> +    }
>> +
>>       if (!intel_display_power_is_enabled(dev_priv,
>> POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>>           return false;
>>   -    tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> -    if (!(tmp & PIPECONF_ENABLE))
>> -        return false;
>> -
>> -    haswell_get_ddi_port_state(crtc, pipe_config);
>> +    if (!is_dsi) {
>> +        tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> +        if (!(tmp & PIPECONF_ENABLE))
>> +            return false;
>>   -    intel_get_pipe_timings(crtc, pipe_config);
>> +        haswell_get_ddi_port_state(crtc, pipe_config);
>> +        intel_get_pipe_timings(crtc, pipe_config);
>> +    } else
>> +        intel_get_dsi_pipe_timings(crtc, pipe_config);
>>         if (INTEL_INFO(dev)->gen >= 9) {
>>           skl_init_scalers(dev, crtc, pipe_config);
>

-- 
Thanks,
--Ram

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format
  2016-02-03  9:44 ` [PATCH 1/2] " Jani Nikula
  2016-02-03 12:18   ` Ramalingam C
@ 2016-02-03 12:27   ` Ramalingam C
  2016-02-03 12:27     ` [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
  2016-02-04 13:13     ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
  2016-02-15 16:28   ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Daniel Vetter
  2 siblings, 2 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-03 12:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, Deepak M

From: Deepak M <m.deepak@intel.com>

The bpp value which is used while calulating the txbyteclkhs values
should be wrt the pixel format value. Currently bpp is coming
from pipe config to calculate txbyteclkhs. Fix it in this patch.

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
 drivers/gpu/drm/i915/intel_dsi.h           |    1 +
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 91cef35..aa11293 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
-	unsigned int bpp = intel_crtc->config->pipe_bpp;
+	unsigned int bpp = intel_dsi->dsi_bpp;
 	unsigned int lane_count = intel_dsi->lane_count;
 
 	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
@@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
 	enum port port;
-	unsigned int bpp = intel_crtc->config->pipe_bpp;
+	unsigned int bpp = intel_dsi->dsi_bpp;
 	u32 val, tmp;
 	u16 mode_hdisplay;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index de7be7f..9bf6fa1d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -64,6 +64,7 @@ struct intel_dsi {
 
 	/* video mode pixel format for MIPI_DSI_FUNC_PRG register */
 	u32 pixel_format;
+	u32 dsi_bpp;
 
 	/* video mode format for MIPI_VIDEO_MODE_FORMAT register */
 	u32 video_mode_format;
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 1d43e6f..bf266cb 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -435,6 +435,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
 	intel_dsi->video_frmt_cfg_bits =
 		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
+	intel_dsi->dsi_bpp = bits_per_pixel;
 
 	pclk = mode->clock;
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-03 12:27   ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Ramalingam C
@ 2016-02-03 12:27     ` Ramalingam C
  2016-02-04 13:54       ` Jani Nikula
  2016-02-04 13:13     ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
  1 sibling, 1 reply; 33+ messages in thread
From: Ramalingam C @ 2016-02-03 12:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: Uma Shankar <uma.shankar@intel.com>

During Charging OS mode, mipi display was blanking.This is
because during driver load, though encoder, connector were
active but crtc returned inactive. This caused sanitize
function to disable the DSI panel. In AOS, this is fine
since HWC will do a modeset and crtc, connector, encoder
mapping will be restored. But in COS, no modeset is called,
it just calls DPMS ON/OFF.

This is fine on BYT/CHT since transcoder is common b/w
all encoders. But for BXT, there is a separate mipi
transcoder. Hence this needs special handling for BXT.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  115 ++++++++++++++++++++++++++++++++--
 1 file changed, 109 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a66220a..58d2cd9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -34,6 +34,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drmP.h>
 #include "intel_drv.h"
+#include "intel_dsi.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 #include "i915_trace.h"
@@ -7814,6 +7815,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 		   (intel_crtc->config->pipe_src_h - 1));
 }
 
+static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
+				   struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
+	struct intel_encoder *encoder;
+	uint32_t tmp;
+
+	tmp = I915_READ(HTOTAL(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_htotal =
+						((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HBLANK(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_hblank_end =
+						((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(HSYNC(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_hsync_end =
+						((tmp >> 16) & 0xffff) + 1;
+
+	tmp = I915_READ(VBLANK(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_vblank_end =
+						((tmp >> 16) & 0xffff) + 1;
+	tmp = I915_READ(VSYNC(cpu_transcoder));
+	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
+	pipe_config->base.adjusted_mode.crtc_vsync_end =
+						((tmp >> 16) & 0xffff) + 1;
+
+	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
+		pipe_config->base.adjusted_mode.flags |=
+						DRM_MODE_FLAG_INTERLACE;
+		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
+		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
+	}
+
+
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+		struct intel_dsi *intel_dsi =
+			enc_to_intel_dsi(&encoder->base);
+		enum port port;
+
+		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
+		for_each_dsi_port(port, intel_dsi->ports) {
+			pipe_config->base.adjusted_mode.crtc_hdisplay =
+				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
+			pipe_config->base.adjusted_mode.crtc_vdisplay =
+				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
+			pipe_config->base.adjusted_mode.crtc_vtotal =
+				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
+		}
+	}
+
+	tmp = I915_READ(PIPESRC(crtc->pipe));
+	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
+	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
+
+	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
+	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
+}
+
 static void intel_get_pipe_timings(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config)
 {
@@ -9962,6 +10026,40 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 	}
 }
 
+static void bxt_get_pipe_config_dsi(struct intel_crtc *crtc,
+					struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum port port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
+				? PORT_A : PORT_C;
+	uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port));
+	uint32_t tmp;
+
+	tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
+	if (tmp & DPI_ENABLE) {
+		enum pipe trans_dsi_pipe;
+
+		switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
+		default:
+			WARN(1, "unknown pipe linked to dsi transcoder\n");
+			return;
+		case BXT_PIPE_SELECT(PIPE_A):
+			trans_dsi_pipe = PIPE_A;
+			break;
+		case BXT_PIPE_SELECT(PIPE_B):
+			trans_dsi_pipe = PIPE_B;
+			break;
+		case BXT_PIPE_SELECT(PIPE_C):
+			trans_dsi_pipe = PIPE_C;
+			break;
+		}
+
+		if (trans_dsi_pipe == crtc->pipe)
+			pipe_config->has_dsi_encoder = true;
+	}
+}
+
 static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 				    struct intel_crtc_state *pipe_config)
 {
@@ -9999,17 +10097,22 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 			pipe_config->cpu_transcoder = TRANSCODER_EDP;
 	}
 
+	if (dev_priv->vbt.has_mipi && IS_BROXTON(dev))
+		bxt_get_pipe_config_dsi(crtc, pipe_config);
+
 	if (!intel_display_power_is_enabled(dev_priv,
 			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
 		return false;
 
-	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
-	if (!(tmp & PIPECONF_ENABLE))
-		return false;
-
-	haswell_get_ddi_port_state(crtc, pipe_config);
+	if (!pipe_config->has_dsi_encoder) {
+		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
+		if (!(tmp & PIPECONF_ENABLE))
+			return false;
 
-	intel_get_pipe_timings(crtc, pipe_config);
+		haswell_get_ddi_port_state(crtc, pipe_config);
+		intel_get_pipe_timings(crtc, pipe_config);
+	} else
+		intel_get_dsi_pipe_timings(crtc, pipe_config);
 
 	if (INTEL_INFO(dev)->gen >= 9) {
 		skl_init_scalers(dev, crtc, pipe_config);
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Using the bpp value wrt the pixel format (rev2)
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
                   ` (5 preceding siblings ...)
  2016-02-03  9:44 ` [PATCH 1/2] " Jani Nikula
@ 2016-02-03 13:12 ` Patchwork
  2016-02-15 16:24 ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Daniel Vetter
  7 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2016-02-03 13:12 UTC (permalink / raw)
  To: Ramalingam C; +Cc: intel-gfx

== Summary ==

Series 3017v2 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/3017/revisions/2/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (hsw-gt2)

bdw-nuci7        total:156  pass:147  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:159  pass:147  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:159  pass:131  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc          total:159  pass:136  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox      total:159  pass:146  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:159  pass:149  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p      total:159  pass:111  dwarn:0   dfail:0   fail:0   skip:48 
ivb-t430s        total:159  pass:145  dwarn:0   dfail:0   fail:0   skip:14 
snb-dellxps      total:159  pass:137  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:159  pass:137  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1352/

02932377a975a59ccd83095816d5b23183107d79 drm-intel-nightly: 2016y-02m-03d-01h-54m-27s UTC integration manifest
5aa340e83bfa4a73b3b40ec49bf972a4324b3c62 drm/i915/dsi: Add audio reference in dsi encoder
c11b5a301945a99ac2376fae823d1e3aa30effdf drm/i915: Using the bpp value wrt the pixel format

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format
  2016-02-03 12:27   ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Ramalingam C
  2016-02-03 12:27     ` [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
@ 2016-02-04 13:13     ` Jani Nikula
  2016-02-11 15:00       ` Ramalingam C
  1 sibling, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2016-02-04 13:13 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx; +Cc: Deepak M

On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> From: Deepak M <m.deepak@intel.com>
>
> The bpp value which is used while calulating the txbyteclkhs values
> should be wrt the pixel format value. Currently bpp is coming
> from pipe config to calculate txbyteclkhs. Fix it in this patch.
>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
>  drivers/gpu/drm/i915/intel_dsi.h           |    1 +
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 91cef35..aa11293 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>  {
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
> -	unsigned int bpp = intel_crtc->config->pipe_bpp;
> +	unsigned int bpp = intel_dsi->dsi_bpp;
>  	unsigned int lane_count = intel_dsi->lane_count;
>  
>  	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
> @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>  	enum port port;
> -	unsigned int bpp = intel_crtc->config->pipe_bpp;
> +	unsigned int bpp = intel_dsi->dsi_bpp;
>  	u32 val, tmp;
>  	u16 mode_hdisplay;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index de7be7f..9bf6fa1d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -64,6 +64,7 @@ struct intel_dsi {
>  
>  	/* video mode pixel format for MIPI_DSI_FUNC_PRG register */
>  	u32 pixel_format;
> +	u32 dsi_bpp;

Please never add extra state for things that can trivially be derived
from existing information.

Given the dsi_pixel_format_bpp() in intel_dsi_pll.c, this should always
hold, right:

	intel_dsi->dsi_bpp == dsi_pixel_format_bpp(intel_dsi->pixel_format)

Please just make dsi_pixel_format_bpp() available and use it. As a nice
bonus, this becomes self-documenting code on why pipe config is not used
where the bpp based on pixel format is used.

BR,
Jani.


>  
>  	/* video mode format for MIPI_VIDEO_MODE_FORMAT register */
>  	u32 video_mode_format;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 1d43e6f..bf266cb 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -435,6 +435,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
>  	intel_dsi->video_frmt_cfg_bits =
>  		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
> +	intel_dsi->dsi_bpp = bits_per_pixel;
>  
>  	pclk = mode->clock;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-03 12:27     ` [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
@ 2016-02-04 13:54       ` Jani Nikula
  2016-02-11 14:49         ` Ramalingam C
  0 siblings, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2016-02-04 13:54 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx

On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> From: Uma Shankar <uma.shankar@intel.com>
>
> During Charging OS mode, mipi display was blanking.This is
> because during driver load, though encoder, connector were
> active but crtc returned inactive. This caused sanitize
> function to disable the DSI panel. In AOS, this is fine
> since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in COS, no modeset is called,
> it just calls DPMS ON/OFF.
>
> This is fine on BYT/CHT since transcoder is common b/w
> all encoders. But for BXT, there is a separate mipi
> transcoder. Hence this needs special handling for BXT.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  115 ++++++++++++++++++++++++++++++++--
>  1 file changed, 109 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a66220a..58d2cd9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -34,6 +34,7 @@
>  #include <drm/drm_edid.h>
>  #include <drm/drmP.h>
>  #include "intel_drv.h"
> +#include "intel_dsi.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  #include "i915_trace.h"
> @@ -7814,6 +7815,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  		   (intel_crtc->config->pipe_src_h - 1));
>  }
>  
> +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
> +				   struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;

Hum, how does this work for DSI transcoders.

> +	struct intel_encoder *encoder;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(HTOTAL(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_htotal =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	tmp = I915_READ(VBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(VSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
> +		pipe_config->base.adjusted_mode.flags |=
> +						DRM_MODE_FLAG_INTERLACE;
> +		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
> +		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
> +	}
> +
> +
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		struct intel_dsi *intel_dsi =
> +			enc_to_intel_dsi(&encoder->base);
> +		enum port port;
> +
> +		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			pipe_config->base.adjusted_mode.crtc_hdisplay =
> +				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vdisplay =
> +				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vtotal =
> +				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +		}
> +	}

Since you already figure out the port/pipe in bxt_get_pipe_config_dsi, I
feel it would be better to use that instead of doing the for loops here
and semi-blindly casting the encoder to intel_dsi.

> +
> +	tmp = I915_READ(PIPESRC(crtc->pipe));
> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
> +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
> +}

What's the point of duplicating most of intel_get_pipe_timings() when
you could just call that first, and then do your bxt dsi specific stuff?
I can't spot any differences.

Do we even need to read the generic HTOTAL etc. registers for DSI? Do
they even make sense?

BR,
Jani.

> +
>  static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config)
>  {
> @@ -9962,6 +10026,40 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  	}
>  }
>  
> +static void bxt_get_pipe_config_dsi(struct intel_crtc *crtc,
> +					struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum port port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
> +				? PORT_A : PORT_C;
> +	uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port));
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
> +	if (tmp & DPI_ENABLE) {
> +		enum pipe trans_dsi_pipe;
> +
> +		switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
> +		default:
> +			WARN(1, "unknown pipe linked to dsi transcoder\n");
> +			return;
> +		case BXT_PIPE_SELECT(PIPE_A):
> +			trans_dsi_pipe = PIPE_A;
> +			break;
> +		case BXT_PIPE_SELECT(PIPE_B):
> +			trans_dsi_pipe = PIPE_B;
> +			break;
> +		case BXT_PIPE_SELECT(PIPE_C):
> +			trans_dsi_pipe = PIPE_C;
> +			break;
> +		}
> +
> +		if (trans_dsi_pipe == crtc->pipe)
> +			pipe_config->has_dsi_encoder = true;
> +	}
> +}
> +
>  static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  				    struct intel_crtc_state *pipe_config)
>  {
> @@ -9999,17 +10097,22 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>  	}
>  
> +	if (dev_priv->vbt.has_mipi && IS_BROXTON(dev))
> +		bxt_get_pipe_config_dsi(crtc, pipe_config);
> +
>  	if (!intel_display_power_is_enabled(dev_priv,
>  			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>  		return false;
>  
> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> -	if (!(tmp & PIPECONF_ENABLE))
> -		return false;
> -
> -	haswell_get_ddi_port_state(crtc, pipe_config);
> +	if (!pipe_config->has_dsi_encoder) {
> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> +		if (!(tmp & PIPECONF_ENABLE))
> +			return false;
>  
> -	intel_get_pipe_timings(crtc, pipe_config);
> +		haswell_get_ddi_port_state(crtc, pipe_config);
> +		intel_get_pipe_timings(crtc, pipe_config);
> +	} else
> +		intel_get_dsi_pipe_timings(crtc, pipe_config);
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
>  		skl_init_scalers(dev, crtc, pipe_config);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-04 13:54       ` Jani Nikula
@ 2016-02-11 14:49         ` Ramalingam C
  2016-02-11 14:59           ` [PATCH 2/3 V3] " Ramalingam C
  0 siblings, 1 reply; 33+ messages in thread
From: Ramalingam C @ 2016-02-11 14:49 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx

Hi,

Thanks for the review comments. Addressing them in next version.

On Thursday 04 February 2016 07:24 PM, Jani Nikula wrote:
> On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>> From: Uma Shankar <uma.shankar@intel.com>
>>
>> During Charging OS mode, mipi display was blanking.This is
>> because during driver load, though encoder, connector were
>> active but crtc returned inactive. This caused sanitize
>> function to disable the DSI panel. In AOS, this is fine
>> since HWC will do a modeset and crtc, connector, encoder
>> mapping will be restored. But in COS, no modeset is called,
>> it just calls DPMS ON/OFF.
>>
>> This is fine on BYT/CHT since transcoder is common b/w
>> all encoders. But for BXT, there is a separate mipi
>> transcoder. Hence this needs special handling for BXT.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c |  115 ++++++++++++++++++++++++++++++++--
>>   1 file changed, 109 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index a66220a..58d2cd9 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -34,6 +34,7 @@
>>   #include <drm/drm_edid.h>
>>   #include <drm/drmP.h>
>>   #include "intel_drv.h"
>> +#include "intel_dsi.h"
>>   #include <drm/i915_drm.h>
>>   #include "i915_drv.h"
>>   #include "i915_trace.h"
>> @@ -7814,6 +7815,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>>   		   (intel_crtc->config->pipe_src_h - 1));
>>   }
>>   
>> +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
>> +				   struct intel_crtc_state *pipe_config)
>> +{
>> +	struct drm_device *dev = crtc->base.dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> Hum, how does this work for DSI transcoders.
>
>> +	struct intel_encoder *encoder;
>> +	uint32_t tmp;
>> +
>> +	tmp = I915_READ(HTOTAL(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_htotal =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +	tmp = I915_READ(HBLANK(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_hblank_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +	tmp = I915_READ(HSYNC(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_hsync_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +
>> +	tmp = I915_READ(VBLANK(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_vblank_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +	tmp = I915_READ(VSYNC(cpu_transcoder));
>> +	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
>> +	pipe_config->base.adjusted_mode.crtc_vsync_end =
>> +						((tmp >> 16) & 0xffff) + 1;
>> +
>> +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
>> +		pipe_config->base.adjusted_mode.flags |=
>> +						DRM_MODE_FLAG_INTERLACE;
>> +		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
>> +		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>> +	}
>> +
>> +
>> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>> +		struct intel_dsi *intel_dsi =
>> +			enc_to_intel_dsi(&encoder->base);
>> +		enum port port;
>> +
>> +		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
>> +		for_each_dsi_port(port, intel_dsi->ports) {
>> +			pipe_config->base.adjusted_mode.crtc_hdisplay =
>> +				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
>> +			pipe_config->base.adjusted_mode.crtc_vdisplay =
>> +				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
>> +			pipe_config->base.adjusted_mode.crtc_vtotal =
>> +				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
>> +		}
>> +	}
> Since you already figure out the port/pipe in bxt_get_pipe_config_dsi, I
> feel it would be better to use that instead of doing the for loops here
> and semi-blindly casting the encoder to intel_dsi.
>
>> +
>> +	tmp = I915_READ(PIPESRC(crtc->pipe));
>> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
>> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
>> +
>> +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
>> +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
>> +}
> What's the point of duplicating most of intel_get_pipe_timings() when
> you could just call that first, and then do your bxt dsi specific stuff?
> I can't spot any differences.
>
> Do we even need to read the generic HTOTAL etc. registers for DSI? Do
> they even make sense?
>
> BR,
> Jani.
>
>> +
>>   static void intel_get_pipe_timings(struct intel_crtc *crtc,
>>   				   struct intel_crtc_state *pipe_config)
>>   {
>> @@ -9962,6 +10026,40 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>>   	}
>>   }
>>   
>> +static void bxt_get_pipe_config_dsi(struct intel_crtc *crtc,
>> +					struct intel_crtc_state *pipe_config)
>> +{
>> +	struct drm_device *dev = crtc->base.dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	enum port port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
>> +				? PORT_A : PORT_C;
>> +	uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port));
>> +	uint32_t tmp;
>> +
>> +	tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
>> +	if (tmp & DPI_ENABLE) {
>> +		enum pipe trans_dsi_pipe;
>> +
>> +		switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
>> +		default:
>> +			WARN(1, "unknown pipe linked to dsi transcoder\n");
>> +			return;
>> +		case BXT_PIPE_SELECT(PIPE_A):
>> +			trans_dsi_pipe = PIPE_A;
>> +			break;
>> +		case BXT_PIPE_SELECT(PIPE_B):
>> +			trans_dsi_pipe = PIPE_B;
>> +			break;
>> +		case BXT_PIPE_SELECT(PIPE_C):
>> +			trans_dsi_pipe = PIPE_C;
>> +			break;
>> +		}
>> +
>> +		if (trans_dsi_pipe == crtc->pipe)
>> +			pipe_config->has_dsi_encoder = true;
>> +	}
>> +}
>> +
>>   static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>   				    struct intel_crtc_state *pipe_config)
>>   {
>> @@ -9999,17 +10097,22 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>   			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>>   	}
>>   
>> +	if (dev_priv->vbt.has_mipi && IS_BROXTON(dev))
>> +		bxt_get_pipe_config_dsi(crtc, pipe_config);
>> +
>>   	if (!intel_display_power_is_enabled(dev_priv,
>>   			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>>   		return false;
>>   
>> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> -	if (!(tmp & PIPECONF_ENABLE))
>> -		return false;
>> -
>> -	haswell_get_ddi_port_state(crtc, pipe_config);
>> +	if (!pipe_config->has_dsi_encoder) {
>> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> +		if (!(tmp & PIPECONF_ENABLE))
>> +			return false;
>>   
>> -	intel_get_pipe_timings(crtc, pipe_config);
>> +		haswell_get_ddi_port_state(crtc, pipe_config);
>> +		intel_get_pipe_timings(crtc, pipe_config);
>> +	} else
>> +		intel_get_dsi_pipe_timings(crtc, pipe_config);
>>   
>>   	if (INTEL_INFO(dev)->gen >= 9) {
>>   		skl_init_scalers(dev, crtc, pipe_config);

-- 
Thanks,
--Ram

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 2/3 V3] drm/i915/BXT: Fixed COS blanking issue
  2016-02-11 14:49         ` Ramalingam C
@ 2016-02-11 14:59           ` Ramalingam C
  2016-02-19  9:16             ` Jani Nikula
  0 siblings, 1 reply; 33+ messages in thread
From: Ramalingam C @ 2016-02-11 14:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

From: Uma Shankar <uma.shankar@intel.com>

During Charging OS mode, mipi display was blanking.This is
because during driver load, though encoder, connector were
active but crtc returned inactive. This caused sanitize
function to disable the DSI panel. In AOS, this is fine
since HWC will do a modeset and crtc, connector, encoder
mapping will be restored. But in COS, no modeset is called,
it just calls DPMS ON/OFF.

This is fine on BYT/CHT since transcoder is common b/w
all encoders. But for BXT, there is a separate mipi
transcoder. Hence this needs special handling for BXT.

V2: pipe_config->has_dsi_encoder is set for BXT [By Jani]

V3: Timing parameters are retrieved from VBT. [By Jani]

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  106 ++++++++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a66220a..e47a75c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -34,6 +34,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drmP.h>
 #include "intel_drv.h"
+#include "intel_dsi.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 #include "i915_trace.h"
@@ -7814,6 +7815,53 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 		   (intel_crtc->config->pipe_src_h - 1));
 }
 
+static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
+				   struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_display_mode *vbt_dsi_mode =
+						dev_priv->vbt.lfp_lvds_vbt_mode;
+	struct intel_encoder *encoder;
+	uint32_t tmp;
+
+	pipe_config->base.adjusted_mode.crtc_hdisplay = vbt_dsi_mode->hdisplay;
+	pipe_config->base.adjusted_mode.crtc_htotal = vbt_dsi_mode->htotal;
+	pipe_config->base.adjusted_mode.crtc_hblank_start =
+						vbt_dsi_mode->hdisplay;
+	pipe_config->base.adjusted_mode.crtc_hblank_end = vbt_dsi_mode->htotal;
+	pipe_config->base.adjusted_mode.crtc_hsync_start =
+						vbt_dsi_mode->hsync_start;
+	pipe_config->base.adjusted_mode.crtc_hsync_end =
+						vbt_dsi_mode->hsync_end;
+
+	pipe_config->base.adjusted_mode.crtc_vdisplay = vbt_dsi_mode->vdisplay;
+	pipe_config->base.adjusted_mode.crtc_vtotal = vbt_dsi_mode->vtotal;
+	pipe_config->base.adjusted_mode.crtc_vblank_start =
+							vbt_dsi_mode->vdisplay;
+	pipe_config->base.adjusted_mode.crtc_vblank_end = vbt_dsi_mode->vtotal;
+	pipe_config->base.adjusted_mode.crtc_vsync_start =
+						vbt_dsi_mode->vsync_start;
+	pipe_config->base.adjusted_mode.crtc_vsync_end =
+						vbt_dsi_mode->vsync_end;
+
+	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+		if (encoder->type == INTEL_OUTPUT_DSI) {
+			struct intel_dsi *intel_dsi =
+				enc_to_intel_dsi(&encoder->base);
+			pipe_config->pipe_bpp =
+				dsi_pixel_format_bpp(intel_dsi->pixel_format);
+		}
+	}
+
+	tmp = I915_READ(PIPESRC(crtc->pipe));
+	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
+	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
+
+	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
+	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
+}
+
 static void intel_get_pipe_timings(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config)
 {
@@ -9962,6 +10010,40 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 	}
 }
 
+static void bxt_get_pipe_config_dsi(struct intel_crtc *crtc,
+					struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum port port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
+				? PORT_A : PORT_C;
+	uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port));
+	uint32_t tmp;
+
+	tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
+	if (tmp & DPI_ENABLE) {
+		enum pipe trans_dsi_pipe;
+
+		switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
+		default:
+			WARN(1, "unknown pipe linked to dsi transcoder\n");
+			return;
+		case BXT_PIPE_SELECT(PIPE_A):
+			trans_dsi_pipe = PIPE_A;
+			break;
+		case BXT_PIPE_SELECT(PIPE_B):
+			trans_dsi_pipe = PIPE_B;
+			break;
+		case BXT_PIPE_SELECT(PIPE_C):
+			trans_dsi_pipe = PIPE_C;
+			break;
+		}
+
+		if (trans_dsi_pipe == crtc->pipe)
+			pipe_config->has_dsi_encoder = true;
+	}
+}
+
 static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 				    struct intel_crtc_state *pipe_config)
 {
@@ -9999,17 +10081,29 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 			pipe_config->cpu_transcoder = TRANSCODER_EDP;
 	}
 
+	if (dev_priv->vbt.has_mipi && IS_BROXTON(dev))
+		bxt_get_pipe_config_dsi(crtc, pipe_config);
+
 	if (!intel_display_power_is_enabled(dev_priv,
 			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
 		return false;
 
-	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
-	if (!(tmp & PIPECONF_ENABLE))
-		return false;
-
-	haswell_get_ddi_port_state(crtc, pipe_config);
+	if (pipe_config->has_dsi_encoder && IS_BROXTON(dev))
+		/*
+		 * BXT doesn't have the PIPE timing registers.
+		 * Hence retrieved the pipe detail from PORT register.
+		 * And timing parameters are obtained from VBT
+		 */
+		intel_get_dsi_pipe_timings(crtc, pipe_config);
+	else {
+		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
+		if (!(tmp & PIPECONF_ENABLE))
+			return false;
+		if (!IS_VALLEYVIEW(dev))
+			haswell_get_ddi_port_state(crtc, pipe_config);
 
-	intel_get_pipe_timings(crtc, pipe_config);
+		intel_get_pipe_timings(crtc, pipe_config);
+	}
 
 	if (INTEL_INFO(dev)->gen >= 9) {
 		skl_init_scalers(dev, crtc, pipe_config);
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format
  2016-02-04 13:13     ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
@ 2016-02-11 15:00       ` Ramalingam C
  2016-02-11 15:03         ` [PATCH 1/3 V2] " Ramalingam C
  0 siblings, 1 reply; 33+ messages in thread
From: Ramalingam C @ 2016-02-11 15:00 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Deepak M


On Thursday 04 February 2016 06:43 PM, Jani Nikula wrote:
> On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>> From: Deepak M <m.deepak@intel.com>
>>
>> The bpp value which is used while calulating the txbyteclkhs values
>> should be wrt the pixel format value. Currently bpp is coming
>> from pipe config to calculate txbyteclkhs. Fix it in this patch.
>>
>> Signed-off-by: Deepak M <m.deepak@intel.com>
>> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
>>   drivers/gpu/drm/i915/intel_dsi.h           |    1 +
>>   drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    1 +
>>   3 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 91cef35..aa11293 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>>   {
>>   	struct drm_device *dev = encoder->dev;
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>   	enum port port;
>> -	unsigned int bpp = intel_crtc->config->pipe_bpp;
>> +	unsigned int bpp = intel_dsi->dsi_bpp;
>>   	unsigned int lane_count = intel_dsi->lane_count;
>>   
>>   	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
>> @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>   	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>>   	enum port port;
>> -	unsigned int bpp = intel_crtc->config->pipe_bpp;
>> +	unsigned int bpp = intel_dsi->dsi_bpp;
>>   	u32 val, tmp;
>>   	u16 mode_hdisplay;
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> index de7be7f..9bf6fa1d 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -64,6 +64,7 @@ struct intel_dsi {
>>   
>>   	/* video mode pixel format for MIPI_DSI_FUNC_PRG register */
>>   	u32 pixel_format;
>> +	u32 dsi_bpp;
> Please never add extra state for things that can trivially be derived
> from existing information.
>
> Given the dsi_pixel_format_bpp() in intel_dsi_pll.c, this should always
> hold, right:
>
> 	intel_dsi->dsi_bpp == dsi_pixel_format_bpp(intel_dsi->pixel_format)
>
> Please just make dsi_pixel_format_bpp() available and use it. As a nice
> bonus, this becomes self-documenting code on why pipe config is not used
> where the bpp based on pixel format is used.
Agreed. Implemented in the next version. Thanks
>
> BR,
> Jani.
>
>
>>   
>>   	/* video mode format for MIPI_VIDEO_MODE_FORMAT register */
>>   	u32 video_mode_format;
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> index 1d43e6f..bf266cb 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> @@ -435,6 +435,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>>   	intel_dsi->bw_timer = mipi_config->dbi_bw_timer;
>>   	intel_dsi->video_frmt_cfg_bits =
>>   		mipi_config->bta_enabled ? DISABLE_VIDEO_BTA : 0;
>> +	intel_dsi->dsi_bpp = bits_per_pixel;
>>   
>>   	pclk = mode->clock;

-- 
Thanks,
--Ram

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format
  2016-02-11 15:00       ` Ramalingam C
@ 2016-02-11 15:03         ` Ramalingam C
  2016-02-11 15:05           ` [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI Ramalingam C
  2016-02-19  8:50           ` [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
  0 siblings, 2 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-11 15:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, Deepak M

From: Deepak M <m.deepak@intel.com>

The bpp value which is used while calulating the txbyteclkhs values
should be wrt the pixel format value. Currently bpp is coming
from pipe config to calculate txbyteclkhs. Fix it in this patch.

V2: dsi_pixel_format_bpp is used to retrieve the bpp from pixel_format
	[Review: Jani]

Signed-off-by: Deepak M <m.deepak@intel.com>
Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
 drivers/gpu/drm/i915/intel_dsi.h           |    2 ++
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    5 +----
 drivers/gpu/drm/i915/intel_dsi_pll.c       |    2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 91cef35..ce94342 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
-	unsigned int bpp = intel_crtc->config->pipe_bpp;
+	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
 	unsigned int lane_count = intel_dsi->lane_count;
 
 	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
@@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
 	enum port port;
-	unsigned int bpp = intel_crtc->config->pipe_bpp;
+	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
 	u32 val, tmp;
 	u16 mode_hdisplay;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index de7be7f..92f3922 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -34,6 +34,8 @@
 #define DSI_DUAL_LINK_FRONT_BACK	1
 #define DSI_DUAL_LINK_PIXEL_ALT		2
 
+int dsi_pixel_format_bpp(int pixel_format);
+
 struct intel_dsi_host;
 
 struct intel_dsi {
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 1d43e6f..23c0f67 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -420,10 +420,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
 	intel_dsi->dual_link = mipi_config->dual_link;
 	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
 
-	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
-		bits_per_pixel = 18;
-	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
-		bits_per_pixel = 16;
+	bits_per_pixel = dsi_pixel_format_bpp(intel_dsi->pixel_format);
 
 	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
 	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
index bb5e95a..70883c5 100644
--- a/drivers/gpu/drm/i915/intel_dsi_pll.c
+++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
@@ -30,7 +30,7 @@
 #include "i915_drv.h"
 #include "intel_dsi.h"
 
-static int dsi_pixel_format_bpp(int pixel_format)
+int dsi_pixel_format_bpp(int pixel_format)
 {
 	int bpp;
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI
  2016-02-11 15:03         ` [PATCH 1/3 V2] " Ramalingam C
@ 2016-02-11 15:05           ` Ramalingam C
  2016-02-19  9:07             ` Jani Nikula
  2016-02-19  8:50           ` [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
  1 sibling, 1 reply; 33+ messages in thread
From: Ramalingam C @ 2016-02-11 15:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

In case of BXT DSI we are updating the CPU_TRANSCODER
with appropriate value.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    2 ++
 drivers/gpu/drm/i915/intel_display.c |    5 +++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 65a2cd0..ef4b376 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -122,6 +122,8 @@ enum transcoder {
 	TRANSCODER_B,
 	TRANSCODER_C,
 	TRANSCODER_EDP,
+	TRANSCODER_MIPI_A,
+	TRANSCODER_MIPI_C,
 	I915_MAX_TRANSCODERS
 };
 #define transcoder_name(t) ((t) + 'A')
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e47a75c..9715056 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7851,6 +7851,11 @@ static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
 				enc_to_intel_dsi(&encoder->base);
 			pipe_config->pipe_bpp =
 				dsi_pixel_format_bpp(intel_dsi->pixel_format);
+
+			if (intel_dsi->ports & (1 << PORT_A))
+				pipe_config->cpu_transcoder = TRANSCODER_MIPI_A;
+			else
+				pipe_config->cpu_transcoder = TRANSCODER_MIPI_C;
 		}
 	}
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
                   ` (6 preceding siblings ...)
  2016-02-03 13:12 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Using the bpp value wrt the pixel format (rev2) Patchwork
@ 2016-02-15 16:24 ` Daniel Vetter
  7 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2016-02-15 16:24 UTC (permalink / raw)
  To: Ramalingam C; +Cc: jani.nikula, intel-gfx

On Tue, Feb 02, 2016 at 11:24:16PM +0530, Ramalingam C wrote:
> From: Uma Shankar <uma.shankar@intel.com>
> 
> During Charging OS mode, mipi display was blanking.This is
> because during driver load, though encoder, connector were
> active but crtc returned inactive. This caused sanitize
> function to disable the DSI panel. In AOS, this is fine
> since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in COS, no modeset is called,
> it just calls DPMS ON/OFF.
> 
> This is fine on BYT/CHT since transcoder is common b/w
> all encoders. But for BXT, there is a separate mipi
> transcoder. Hence this needs special handling for BXT.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  107 ++++++++++++++++++++++++++++++++--
>  1 file changed, 101 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a66220a..f8685f5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7814,6 +7814,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  		   (intel_crtc->config->pipe_src_h - 1));
>  }
>  
> +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
> +				   struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> +	struct intel_encoder *encoder;
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(HTOTAL(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_htotal =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(HSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_hsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	tmp = I915_READ(VBLANK(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vblank_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +	tmp = I915_READ(VSYNC(cpu_transcoder));
> +	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
> +	pipe_config->base.adjusted_mode.crtc_vsync_end =
> +						((tmp >> 16) & 0xffff) + 1;
> +
> +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
> +		pipe_config->base.adjusted_mode.flags |=
> +						DRM_MODE_FLAG_INTERLACE;
> +		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
> +		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
> +	}
> +
> +
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		struct intel_dsi *intel_dsi =
> +			enc_to_intel_dsi(&encoder->base);
> +		enum port port;
> +
> +		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
> +		for_each_dsi_port(port, intel_dsi->ports) {
> +			pipe_config->base.adjusted_mode.crtc_hdisplay =
> +				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vdisplay =
> +				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> +			pipe_config->base.adjusted_mode.crtc_vtotal =
> +				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> +		}
> +	}

If some state is encoder specific (and a for_each_encoder_on_crtc highly
suggests that), then please read out such state from the
encoder->get_config hook.
-Daniel

> +
> +	tmp = I915_READ(PIPESRC(crtc->pipe));
> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
> +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
> +}
> +
>  static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config)
>  {
> @@ -9969,6 +10032,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum intel_display_power_domain pfit_domain;
>  	uint32_t tmp;
> +	bool is_dsi = false;
>  
>  	if (!intel_display_power_is_enabled(dev_priv,
>  					 POWER_DOMAIN_PIPE(crtc->pipe)))
> @@ -9999,17 +10063,48 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>  	}
>  
> +	if (dev_priv->vbt.has_mipi) {
> +		enum port port_num = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
> +					? PORT_A : PORT_C;
> +		uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port_num));
> +
> +		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port_num));
> +		if (tmp & DPI_ENABLE) {
> +			enum pipe trans_dsi_pipe;
> +
> +			switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
> +			default:
> +				WARN(1, "unknown pipe linked to dsi transcoder\n");
> +				return false;
> +			case BXT_PIPE_SELECT_A:
> +				trans_dsi_pipe = PIPE_A;
> +				break;
> +			case BXT_PIPE_SELECT_B:
> +				trans_dsi_pipe = PIPE_B;
> +				break;
> +			case BXT_PIPE_SELECT_C:
> +				trans_dsi_pipe = PIPE_C;
> +				break;
> +			}
> +
> +			if (trans_dsi_pipe == crtc->pipe)
> +				is_dsi = true;
> +		}
> +	}
> +
>  	if (!intel_display_power_is_enabled(dev_priv,
>  			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>  		return false;
>  
> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> -	if (!(tmp & PIPECONF_ENABLE))
> -		return false;
> -
> -	haswell_get_ddi_port_state(crtc, pipe_config);
> +	if (!is_dsi) {
> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> +		if (!(tmp & PIPECONF_ENABLE))
> +			return false;
>  
> -	intel_get_pipe_timings(crtc, pipe_config);
> +		haswell_get_ddi_port_state(crtc, pipe_config);
> +		intel_get_pipe_timings(crtc, pipe_config);
> +	} else
> +		intel_get_dsi_pipe_timings(crtc, pipe_config);
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
>  		skl_init_scalers(dev, crtc, pipe_config);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue
  2016-02-03  9:44 ` [PATCH 1/2] " Jani Nikula
  2016-02-03 12:18   ` Ramalingam C
  2016-02-03 12:27   ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Ramalingam C
@ 2016-02-15 16:28   ` Daniel Vetter
  2 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2016-02-15 16:28 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, Feb 03, 2016 at 11:44:03AM +0200, Jani Nikula wrote:
> On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> > From: Uma Shankar <uma.shankar@intel.com>
> >
> > During Charging OS mode, mipi display was blanking.This is
> > because during driver load, though encoder, connector were
> > active but crtc returned inactive. This caused sanitize
> > function to disable the DSI panel. In AOS, this is fine
> > since HWC will do a modeset and crtc, connector, encoder
> > mapping will be restored. But in COS, no modeset is called,
> > it just calls DPMS ON/OFF.
> >
> > This is fine on BYT/CHT since transcoder is common b/w
> > all encoders. But for BXT, there is a separate mipi
> > transcoder. Hence this needs special handling for BXT.
> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |  107 ++++++++++++++++++++++++++++++++--
> >  1 file changed, 101 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a66220a..f8685f5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7814,6 +7814,69 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> >  		   (intel_crtc->config->pipe_src_h - 1));
> >  }
> >  
> > +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
> > +				   struct intel_crtc_state *pipe_config)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> > +	struct intel_encoder *encoder;
> > +	uint32_t tmp;
> > +
> > +	tmp = I915_READ(HTOTAL(cpu_transcoder));
> > +	pipe_config->base.adjusted_mode.crtc_hdisplay = (tmp & 0xffff) + 1;
> > +	pipe_config->base.adjusted_mode.crtc_htotal =
> > +						((tmp >> 16) & 0xffff) + 1;
> > +	tmp = I915_READ(HBLANK(cpu_transcoder));
> > +	pipe_config->base.adjusted_mode.crtc_hblank_start = (tmp & 0xffff) + 1;
> > +	pipe_config->base.adjusted_mode.crtc_hblank_end =
> > +						((tmp >> 16) & 0xffff) + 1;
> > +	tmp = I915_READ(HSYNC(cpu_transcoder));
> > +	pipe_config->base.adjusted_mode.crtc_hsync_start = (tmp & 0xffff) + 1;
> > +	pipe_config->base.adjusted_mode.crtc_hsync_end =
> > +						((tmp >> 16) & 0xffff) + 1;
> > +
> > +	tmp = I915_READ(VBLANK(cpu_transcoder));
> > +	pipe_config->base.adjusted_mode.crtc_vblank_start = (tmp & 0xffff) + 1;
> > +	pipe_config->base.adjusted_mode.crtc_vblank_end =
> > +						((tmp >> 16) & 0xffff) + 1;
> > +	tmp = I915_READ(VSYNC(cpu_transcoder));
> > +	pipe_config->base.adjusted_mode.crtc_vsync_start = (tmp & 0xffff) + 1;
> > +	pipe_config->base.adjusted_mode.crtc_vsync_end =
> > +						((tmp >> 16) & 0xffff) + 1;
> > +
> > +	if (I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_INTERLACE_MASK) {
> > +		pipe_config->base.adjusted_mode.flags |=
> > +						DRM_MODE_FLAG_INTERLACE;
> > +		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
> > +		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
> > +	}
> > +
> > +
> > +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> > +		struct intel_dsi *intel_dsi =
> > +			enc_to_intel_dsi(&encoder->base);
> > +		enum port port;
> > +
> > +		pipe_config->pipe_bpp = intel_dsi->dsi_bpp;
> > +		for_each_dsi_port(port, intel_dsi->ports) {
> > +			pipe_config->base.adjusted_mode.crtc_hdisplay =
> > +				I915_READ(BXT_MIPI_TRANS_HACTIVE(port));
> > +			pipe_config->base.adjusted_mode.crtc_vdisplay =
> > +				I915_READ(BXT_MIPI_TRANS_VACTIVE(port));
> > +			pipe_config->base.adjusted_mode.crtc_vtotal =
> > +				I915_READ(BXT_MIPI_TRANS_VTOTAL(port));
> > +		}
> > +	}
> > +
> > +	tmp = I915_READ(PIPESRC(crtc->pipe));
> > +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> > +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> > +
> > +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
> > +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
> > +}
> > +
> >  static void intel_get_pipe_timings(struct intel_crtc *crtc,
> >  				   struct intel_crtc_state *pipe_config)
> >  {
> > @@ -9969,6 +10032,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	enum intel_display_power_domain pfit_domain;
> >  	uint32_t tmp;
> > +	bool is_dsi = false;
> >  
> >  	if (!intel_display_power_is_enabled(dev_priv,
> >  					 POWER_DOMAIN_PIPE(crtc->pipe)))
> > @@ -9999,17 +10063,48 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  			pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >  	}
> >  
> > +	if (dev_priv->vbt.has_mipi) {
> > +		enum port port_num = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
> > +					? PORT_A : PORT_C;
> 
> Just "enum port port" please.
> 
> > +		uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port_num));
> > +
> > +		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port_num));
> > +		if (tmp & DPI_ENABLE) {
> > +			enum pipe trans_dsi_pipe;
> > +
> > +			switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
> > +			default:
> > +				WARN(1, "unknown pipe linked to dsi transcoder\n");
> > +				return false;
> > +			case BXT_PIPE_SELECT_A:
> > +				trans_dsi_pipe = PIPE_A;
> > +				break;
> > +			case BXT_PIPE_SELECT_B:
> > +				trans_dsi_pipe = PIPE_B;
> > +				break;
> > +			case BXT_PIPE_SELECT_C:
> > +				trans_dsi_pipe = PIPE_C;
> > +				break;
> > +			}
> > +
> > +			if (trans_dsi_pipe == crtc->pipe)
> > +				is_dsi = true;
> > +		}
> > +	}
> > +
> 
> You need to set pipe_config->has_dsi_encoder.
> 
> This is possibly cleanest, if you abstract the above into a separate
> function, say
> 
> bxt_get_pipe_config_dsi(struct intel_crtc *crtc, struct intel_crtc_state
> *pipe_config)
> 
> and set pipe_config->has_dsi_encoder if trans_dsi_pipe == crtc->pipe.
> 
> This probably needs to take into account the different power domains as
> well?
> 
> BR,
> Jani.
> 
> 
> PS. I guess we're missing the has_dsi_encoder readout for BYT/CHV as
> well. :(

That means we really, really should have a pipe config check for
has_dsi_encoder, which we're lacking too. Plus we don't even have a
machine with DSI in CI, which is why no one noticed that either.

Please add all the missing bits here. Iron rule is that if you read out
some hw state, then you _must_ add corresponding state checks ot
intel_pipe_config_compare.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format
  2016-02-11 15:03         ` [PATCH 1/3 V2] " Ramalingam C
  2016-02-11 15:05           ` [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI Ramalingam C
@ 2016-02-19  8:50           ` Jani Nikula
  2016-02-19 12:50             ` Mika Kahola
  1 sibling, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2016-02-19  8:50 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx; +Cc: Deepak M

On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> From: Deepak M <m.deepak@intel.com>
>
> The bpp value which is used while calulating the txbyteclkhs values
> should be wrt the pixel format value. Currently bpp is coming
> from pipe config to calculate txbyteclkhs. Fix it in this patch.
>
> V2: dsi_pixel_format_bpp is used to retrieve the bpp from pixel_format
> 	[Review: Jani]
>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

This needs a Tested-by on at least BYT to ensure everything still works.

> ---
>  drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
>  drivers/gpu/drm/i915/intel_dsi.h           |    2 ++
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    5 +----
>  drivers/gpu/drm/i915/intel_dsi_pll.c       |    2 +-
>  4 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 91cef35..ce94342 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>  {
>  	struct drm_device *dev = encoder->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
> -	unsigned int bpp = intel_crtc->config->pipe_bpp;
> +	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
>  	unsigned int lane_count = intel_dsi->lane_count;
>  
>  	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
> @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>  	enum port port;
> -	unsigned int bpp = intel_crtc->config->pipe_bpp;
> +	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
>  	u32 val, tmp;
>  	u16 mode_hdisplay;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index de7be7f..92f3922 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -34,6 +34,8 @@
>  #define DSI_DUAL_LINK_FRONT_BACK	1
>  #define DSI_DUAL_LINK_PIXEL_ALT		2
>  
> +int dsi_pixel_format_bpp(int pixel_format);
> +
>  struct intel_dsi_host;
>  
>  struct intel_dsi {
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 1d43e6f..23c0f67 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -420,10 +420,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>  	intel_dsi->dual_link = mipi_config->dual_link;
>  	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
>  
> -	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
> -		bits_per_pixel = 18;
> -	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
> -		bits_per_pixel = 16;
> +	bits_per_pixel = dsi_pixel_format_bpp(intel_dsi->pixel_format);
>  
>  	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
>  	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index bb5e95a..70883c5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -30,7 +30,7 @@
>  #include "i915_drv.h"
>  #include "intel_dsi.h"
>  
> -static int dsi_pixel_format_bpp(int pixel_format)
> +int dsi_pixel_format_bpp(int pixel_format)
>  {
>  	int bpp;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI
  2016-02-11 15:05           ` [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI Ramalingam C
@ 2016-02-19  9:07             ` Jani Nikula
  2016-02-23 14:31               ` Ramalingam C
  0 siblings, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2016-02-19  9:07 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx

On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> In case of BXT DSI we are updating the CPU_TRANSCODER
> with appropriate value.
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    2 ++
>  drivers/gpu/drm/i915/intel_display.c |    5 +++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 65a2cd0..ef4b376 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -122,6 +122,8 @@ enum transcoder {
>  	TRANSCODER_B,
>  	TRANSCODER_C,
>  	TRANSCODER_EDP,
> +	TRANSCODER_MIPI_A,
> +	TRANSCODER_MIPI_C,
>  	I915_MAX_TRANSCODERS
>  };
>  #define transcoder_name(t) ((t) + 'A')
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e47a75c..9715056 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7851,6 +7851,11 @@ static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
>  				enc_to_intel_dsi(&encoder->base);
>  			pipe_config->pipe_bpp =
>  				dsi_pixel_format_bpp(intel_dsi->pixel_format);
> +
> +			if (intel_dsi->ports & (1 << PORT_A))
> +				pipe_config->cpu_transcoder = TRANSCODER_MIPI_A;
> +			else
> +				pipe_config->cpu_transcoder = TRANSCODER_MIPI_C;
>  		}
>  	}

As I've said in my review of the previous version of the patch [1], you
can't just add transcoder identifiers and expect everything to magically
work.

The current assumption is there are at most transcoders A, B, C, and
eDP. We use them to index registers. Not all registers have a
corresponding DSI transcoder variant, and if they do, they are not
uniformly spread in the register offset space. See haswell_crtc_disable
for an example where things go wrong. There are others.

I do not know what the best approach here would be; it is obvious to me
this can't work.

BR,
Jani.


[1] http://mid.gmane.org/87powceh4r.fsf@intel.com



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/3 V3] drm/i915/BXT: Fixed COS blanking issue
  2016-02-11 14:59           ` [PATCH 2/3 V3] " Ramalingam C
@ 2016-02-19  9:16             ` Jani Nikula
  0 siblings, 0 replies; 33+ messages in thread
From: Jani Nikula @ 2016-02-19  9:16 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx

On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> From: Uma Shankar <uma.shankar@intel.com>
>
> During Charging OS mode, mipi display was blanking.This is
> because during driver load, though encoder, connector were
> active but crtc returned inactive. This caused sanitize
> function to disable the DSI panel. In AOS, this is fine
> since HWC will do a modeset and crtc, connector, encoder
> mapping will be restored. But in COS, no modeset is called,
> it just calls DPMS ON/OFF.
>
> This is fine on BYT/CHT since transcoder is common b/w
> all encoders. But for BXT, there is a separate mipi
> transcoder. Hence this needs special handling for BXT.
>
> V2: pipe_config->has_dsi_encoder is set for BXT [By Jani]
>
> V3: Timing parameters are retrieved from VBT. [By Jani]
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  106 ++++++++++++++++++++++++++++++++--
>  1 file changed, 100 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a66220a..e47a75c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -34,6 +34,7 @@
>  #include <drm/drm_edid.h>
>  #include <drm/drmP.h>
>  #include "intel_drv.h"
> +#include "intel_dsi.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  #include "i915_trace.h"
> @@ -7814,6 +7815,53 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  		   (intel_crtc->config->pipe_src_h - 1));
>  }
>  
> +static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
> +				   struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_display_mode *vbt_dsi_mode =
> +						dev_priv->vbt.lfp_lvds_vbt_mode;

The whole point of the function (and the call chain here) is to get the
information from the hardware, so that we can a) set up the internal
software state according to the hardware state at driver load, and b)
later cross check the hardware state against the software state. Reading
the information from the VBT provides no value.

> +	struct intel_encoder *encoder;
> +	uint32_t tmp;
> +
> +	pipe_config->base.adjusted_mode.crtc_hdisplay = vbt_dsi_mode->hdisplay;
> +	pipe_config->base.adjusted_mode.crtc_htotal = vbt_dsi_mode->htotal;
> +	pipe_config->base.adjusted_mode.crtc_hblank_start =
> +						vbt_dsi_mode->hdisplay;
> +	pipe_config->base.adjusted_mode.crtc_hblank_end = vbt_dsi_mode->htotal;
> +	pipe_config->base.adjusted_mode.crtc_hsync_start =
> +						vbt_dsi_mode->hsync_start;
> +	pipe_config->base.adjusted_mode.crtc_hsync_end =
> +						vbt_dsi_mode->hsync_end;
> +
> +	pipe_config->base.adjusted_mode.crtc_vdisplay = vbt_dsi_mode->vdisplay;
> +	pipe_config->base.adjusted_mode.crtc_vtotal = vbt_dsi_mode->vtotal;
> +	pipe_config->base.adjusted_mode.crtc_vblank_start =
> +							vbt_dsi_mode->vdisplay;
> +	pipe_config->base.adjusted_mode.crtc_vblank_end = vbt_dsi_mode->vtotal;
> +	pipe_config->base.adjusted_mode.crtc_vsync_start =
> +						vbt_dsi_mode->vsync_start;
> +	pipe_config->base.adjusted_mode.crtc_vsync_end =
> +						vbt_dsi_mode->vsync_end;
> +
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
> +		if (encoder->type == INTEL_OUTPUT_DSI) {
> +			struct intel_dsi *intel_dsi =
> +				enc_to_intel_dsi(&encoder->base);
> +			pipe_config->pipe_bpp =
> +				dsi_pixel_format_bpp(intel_dsi->pixel_format);
> +		}
> +	}

Things like this should be done in the encoder->get_config hook. Maybe
*all* of this DSI timing business should be done there.

Maybe there needs to be a bxt specific get_config hook for DSI.

> +
> +	tmp = I915_READ(PIPESRC(crtc->pipe));
> +	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> +	pipe_config->pipe_src_w = ((tmp >> 16) & 0xffff) + 1;
> +
> +	pipe_config->base.mode.vdisplay = pipe_config->pipe_src_h;
> +	pipe_config->base.mode.hdisplay = pipe_config->pipe_src_w;
> +}
> +
>  static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *pipe_config)
>  {
> @@ -9962,6 +10010,40 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  	}
>  }
>  
> +static void bxt_get_pipe_config_dsi(struct intel_crtc *crtc,
> +					struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum port port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA)
> +				? PORT_A : PORT_C;

Again, should read the hardware, not VBT.

> +	uint32_t dsi_ctrl = I915_READ(MIPI_CTRL(port));
> +	uint32_t tmp;
> +
> +	tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
> +	if (tmp & DPI_ENABLE) {
> +		enum pipe trans_dsi_pipe;
> +
> +		switch (dsi_ctrl & BXT_PIPE_SELECT_MASK) {
> +		default:
> +			WARN(1, "unknown pipe linked to dsi transcoder\n");
> +			return;
> +		case BXT_PIPE_SELECT(PIPE_A):
> +			trans_dsi_pipe = PIPE_A;
> +			break;
> +		case BXT_PIPE_SELECT(PIPE_B):
> +			trans_dsi_pipe = PIPE_B;
> +			break;
> +		case BXT_PIPE_SELECT(PIPE_C):
> +			trans_dsi_pipe = PIPE_C;
> +			break;
> +		}
> +
> +		if (trans_dsi_pipe == crtc->pipe)
> +			pipe_config->has_dsi_encoder = true;
> +	}
> +}
> +
>  static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  				    struct intel_crtc_state *pipe_config)
>  {
> @@ -9999,17 +10081,29 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>  	}
>  
> +	if (dev_priv->vbt.has_mipi && IS_BROXTON(dev))
> +		bxt_get_pipe_config_dsi(crtc, pipe_config);

We should read the hardware, not VBT. The point is, what if we have DSI
enabled even though VBT says no? Then we have a problem somewhere, and
this code is used for cross checking such problems.

But maybe this should be moved into the encoder hook no matter what.

> +
>  	if (!intel_display_power_is_enabled(dev_priv,
>  			POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder)))
>  		return false;
>  
> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> -	if (!(tmp & PIPECONF_ENABLE))
> -		return false;
> -
> -	haswell_get_ddi_port_state(crtc, pipe_config);
> +	if (pipe_config->has_dsi_encoder && IS_BROXTON(dev))

No need to check for IS_BROXTON.

> +		/*
> +		 * BXT doesn't have the PIPE timing registers.
> +		 * Hence retrieved the pipe detail from PORT register.
> +		 * And timing parameters are obtained from VBT
> +		 */
> +		intel_get_dsi_pipe_timings(crtc, pipe_config);
> +	else {
> +		tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> +		if (!(tmp & PIPECONF_ENABLE))
> +			return false;
> +		if (!IS_VALLEYVIEW(dev))
> +			haswell_get_ddi_port_state(crtc, pipe_config);
>  
> -	intel_get_pipe_timings(crtc, pipe_config);
> +		intel_get_pipe_timings(crtc, pipe_config);
> +	}
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
>  		skl_init_scalers(dev, crtc, pipe_config);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder
  2016-02-03 10:01       ` Jani Nikula
@ 2016-02-19  9:23         ` Jani Nikula
  2016-02-19  9:31           ` Ramalingam C
  0 siblings, 1 reply; 33+ messages in thread
From: Jani Nikula @ 2016-02-19  9:23 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx; +Cc: Jakobsson, Patrik

On Wed, 03 Feb 2016, Jani Nikula <jani.nikula@intel.com> wrote:
> On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>> On Wednesday 03 February 2016 02:27 PM, Jani Nikula wrote:
>>> On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>>>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>>>
>>>> We are re-using Mipi encoder enabled by GOP driver, but not incrementing
>>>> reference count for Audio Power domain, so audio was not working. This
>>>> patch increments the reference count during DSI init and Adds get/put in
>>>> DSI enable/disable functions as well so audio power domain will be on
>>>> when mipi is in use.
>>> Confused. DSI does not have HD audio. The codec enable calls are no-ops
>>> because DSI does not have EDID and therefore no ELD either. Hopefully
>>> the codec disable calls don't mess up things by accident. We don't need
>>> or want those calls in DSI.
>>>
>>> IMO the DSI encoder should not get/put the audio power domain either,
>>> because DSI simply does not use that.
>>>
>>> The question remains, *what* audio was failing? DP/HDMI audio?
>> Since HDMI and DP has the audio we are handling the audio power well.
>> For MIPI since we dont have the audio with encoder, we need not get the 
>> power well on.
>> In such scenario, power well use count is not incrementing (Means no one 
>> is enabling
>> the powerwell for board audio) and hence Board audio is not working.
>>>
>>> We may have problems in the power domain setup code, failing to take DSI
>>> into account or something. But I don't think this is the place to fix
>>> it.
>> Possibly this is not the place. But Either DSI code or audio code has to 
>> handle it,
>> to make the board audio to work on BXT. Share your view on that.
>
> I'm not aware of what board audio is, but based on your description I
> understand it has nothing to do with HDMI, DP, DSI - or our driver in
> general. Correct me if I'm wrong.
>
> Which driver handles board audio?
>
> Sounds like the right approach is to have that driver use the i915
> component API to get/put the power well.

I did not get a reply to my question here. Is there an upstream driver
for the audio? Does using the component API sound like the right thing
to do?

If it isn't clear, this patch is not acceptable.

BR,
Jani.

>
> See sound/hda/hdac_i915.c for how HDA binds to the component API, and
> how it calls the ->get_power() and ->put_power() hooks as
> needed. Without that, HDA would lose all the registers in audio power
> well during mode set. I expect the same for board audio.
>
> The modeset for HDMI/DP need to get/put the power wells just because
> those encoders do the codec enable/disable sequence, actually touching
> registers in the power well.
>
> BR,
> Jani.
>
>
>
>>
>>>
>>> Cc Imre and Patrik.
>>>
>>>
>>> BR,
>>> Jani.
>>>
>>>
>>>
>>>
>>>> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
>>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++++
>>>>   1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>>> index 91cef35..8b43ef6 100644
>>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>>> @@ -461,6 +461,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>>>>   		intel_dsi_port_enable(encoder);
>>>>   	}
>>>>   
>>>> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>>>> +	intel_audio_codec_enable(encoder);
>>>> +
>>>>   	intel_panel_enable_backlight(intel_dsi->attached_connector);
>>>>   }
>>>>   
>>>> @@ -531,11 +534,16 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>>>>   
>>>>   static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>>>>   {
>>>> +	struct drm_device *dev = encoder->base.dev;
>>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>>>   	enum port port;
>>>>   
>>>>   	DRM_DEBUG_KMS("\n");
>>>>   
>>>> +	intel_audio_codec_disable(encoder);
>>>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
>>>> +
>>>>   	intel_panel_disable_backlight(intel_dsi->attached_connector);
>>>>   
>>>>   	if (is_vid_mode(intel_dsi)) {
>>>> @@ -1236,6 +1244,11 @@ void intel_dsi_init(struct drm_device *dev)
>>>>   	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>>>>   	intel_panel_setup_backlight(connector, INVALID_PIPE);
>>>>   
>>>> +	/* Enable Audio Power to fix use-count state machine */
>>>> +	port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) ? PORT_A : PORT_C;
>>>> +	if (I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)
>>>> +		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>>>> +
>>>>   	return;
>>>>   
>>>>   err:

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder
  2016-02-19  9:23         ` Jani Nikula
@ 2016-02-19  9:31           ` Ramalingam C
  0 siblings, 0 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-19  9:31 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: Jakobsson, Patrik


On Friday 19 February 2016 02:53 PM, Jani Nikula wrote:
> On Wed, 03 Feb 2016, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Wed, 03 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>>> On Wednesday 03 February 2016 02:27 PM, Jani Nikula wrote:
>>>> On Tue, 02 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>>>>> From: "Kumar, Mahesh" <mahesh1.kumar@intel.com>
>>>>>
>>>>> We are re-using Mipi encoder enabled by GOP driver, but not incrementing
>>>>> reference count for Audio Power domain, so audio was not working. This
>>>>> patch increments the reference count during DSI init and Adds get/put in
>>>>> DSI enable/disable functions as well so audio power domain will be on
>>>>> when mipi is in use.
>>>> Confused. DSI does not have HD audio. The codec enable calls are no-ops
>>>> because DSI does not have EDID and therefore no ELD either. Hopefully
>>>> the codec disable calls don't mess up things by accident. We don't need
>>>> or want those calls in DSI.
>>>>
>>>> IMO the DSI encoder should not get/put the audio power domain either,
>>>> because DSI simply does not use that.
>>>>
>>>> The question remains, *what* audio was failing? DP/HDMI audio?
>>> Since HDMI and DP has the audio we are handling the audio power well.
>>> For MIPI since we dont have the audio with encoder, we need not get the
>>> power well on.
>>> In such scenario, power well use count is not incrementing (Means no one
>>> is enabling
>>> the powerwell for board audio) and hence Board audio is not working.
>>>> We may have problems in the power domain setup code, failing to take DSI
>>>> into account or something. But I don't think this is the place to fix
>>>> it.
>>> Possibly this is not the place. But Either DSI code or audio code has to
>>> handle it,
>>> to make the board audio to work on BXT. Share your view on that.
>> I'm not aware of what board audio is, but based on your description I
>> understand it has nothing to do with HDMI, DP, DSI - or our driver in
>> general. Correct me if I'm wrong.
>>
>> Which driver handles board audio?
>>
>> Sounds like the right approach is to have that driver use the i915
>> component API to get/put the power well.
> I did not get a reply to my question here. Is there an upstream driver
> for the audio? Does using the component API sound like the right thing
> to do?
>
> If it isn't clear, this patch is not acceptable.
Jani,

sorry somehow missed this thread.
I was referring to on board audio which is not attached to any display 
encoder. This patch
is trying to fix a audio bug, which is disclosed when COS related bug 
was fixed.

This has to be debugged from the audio perspective why the power well is 
not requested
for at this modified code sequence.

We can drop this patch as this wont be the correct way and place to fix.

-- 
Thanks,
--Ram


>
> BR,
> Jani.
>
>> See sound/hda/hdac_i915.c for how HDA binds to the component API, and
>> how it calls the ->get_power() and ->put_power() hooks as
>> needed. Without that, HDA would lose all the registers in audio power
>> well during mode set. I expect the same for board audio.
>>
>> The modeset for HDMI/DP need to get/put the power wells just because
>> those encoders do the codec enable/disable sequence, actually touching
>> registers in the power well.
>>
>> BR,
>> Jani.
>>
>>
>>
>>>> Cc Imre and Patrik.
>>>>
>>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>>
>>>>
>>>>> Signed-off-by: Kumar, Mahesh <mahesh1.kumar@intel.com>
>>>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/intel_dsi.c |   13 +++++++++++++
>>>>>    1 file changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>>>> index 91cef35..8b43ef6 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>>>> @@ -461,6 +461,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>>>>>    		intel_dsi_port_enable(encoder);
>>>>>    	}
>>>>>    
>>>>> +	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>>>>> +	intel_audio_codec_enable(encoder);
>>>>> +
>>>>>    	intel_panel_enable_backlight(intel_dsi->attached_connector);
>>>>>    }
>>>>>    
>>>>> @@ -531,11 +534,16 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>>>>>    
>>>>>    static void intel_dsi_pre_disable(struct intel_encoder *encoder)
>>>>>    {
>>>>> +	struct drm_device *dev = encoder->base.dev;
>>>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>>>>    	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>>>>>    	enum port port;
>>>>>    
>>>>>    	DRM_DEBUG_KMS("\n");
>>>>>    
>>>>> +	intel_audio_codec_disable(encoder);
>>>>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
>>>>> +
>>>>>    	intel_panel_disable_backlight(intel_dsi->attached_connector);
>>>>>    
>>>>>    	if (is_vid_mode(intel_dsi)) {
>>>>> @@ -1236,6 +1244,11 @@ void intel_dsi_init(struct drm_device *dev)
>>>>>    	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
>>>>>    	intel_panel_setup_backlight(connector, INVALID_PIPE);
>>>>>    
>>>>> +	/* Enable Audio Power to fix use-count state machine */
>>>>> +	port = (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) ? PORT_A : PORT_C;
>>>>> +	if (I915_READ(BXT_MIPI_PORT_CTRL(port)) & DPI_ENABLE)
>>>>> +		intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>>>>> +
>>>>>    	return;
>>>>>    
>>>>>    err:

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format
  2016-02-19  8:50           ` [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
@ 2016-02-19 12:50             ` Mika Kahola
  2016-02-19 13:08               ` Jani Nikula
  0 siblings, 1 reply; 33+ messages in thread
From: Mika Kahola @ 2016-02-19 12:50 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Deepak M, intel-gfx

On Fri, 2016-02-19 at 10:50 +0200, Jani Nikula wrote:
> On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
> > From: Deepak M <m.deepak@intel.com>
> >
> > The bpp value which is used while calulating the txbyteclkhs values
> > should be wrt the pixel format value. Currently bpp is coming
> > from pipe config to calculate txbyteclkhs. Fix it in this patch.
> >
> > V2: dsi_pixel_format_bpp is used to retrieve the bpp from pixel_format
> > 	[Review: Jani]
> >
> > Signed-off-by: Deepak M <m.deepak@intel.com>
> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
Tested-by: Mika Kahola <mika.kahola@intel.com>

Tested on BYT, Asus T100

> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> This needs a Tested-by on at least BYT to ensure everything still works.
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
> >  drivers/gpu/drm/i915/intel_dsi.h           |    2 ++
> >  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    5 +----
> >  drivers/gpu/drm/i915/intel_dsi_pll.c       |    2 +-
> >  4 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 91cef35..ce94342 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
> >  {
> >  	struct drm_device *dev = encoder->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> >  	enum port port;
> > -	unsigned int bpp = intel_crtc->config->pipe_bpp;
> > +	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
> >  	unsigned int lane_count = intel_dsi->lane_count;
> >  
> >  	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
> > @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> >  	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
> >  	enum port port;
> > -	unsigned int bpp = intel_crtc->config->pipe_bpp;
> > +	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
> >  	u32 val, tmp;
> >  	u16 mode_hdisplay;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> > index de7be7f..92f3922 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/intel_dsi.h
> > @@ -34,6 +34,8 @@
> >  #define DSI_DUAL_LINK_FRONT_BACK	1
> >  #define DSI_DUAL_LINK_PIXEL_ALT		2
> >  
> > +int dsi_pixel_format_bpp(int pixel_format);
> > +
> >  struct intel_dsi_host;
> >  
> >  struct intel_dsi {
> > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> > index 1d43e6f..23c0f67 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> > @@ -420,10 +420,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
> >  	intel_dsi->dual_link = mipi_config->dual_link;
> >  	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
> >  
> > -	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
> > -		bits_per_pixel = 18;
> > -	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
> > -		bits_per_pixel = 16;
> > +	bits_per_pixel = dsi_pixel_format_bpp(intel_dsi->pixel_format);
> >  
> >  	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
> >  	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
> > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > index bb5e95a..70883c5 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> > @@ -30,7 +30,7 @@
> >  #include "i915_drv.h"
> >  #include "intel_dsi.h"
> >  
> > -static int dsi_pixel_format_bpp(int pixel_format)
> > +int dsi_pixel_format_bpp(int pixel_format)
> >  {
> >  	int bpp;
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format
  2016-02-19 12:50             ` Mika Kahola
@ 2016-02-19 13:08               ` Jani Nikula
  0 siblings, 0 replies; 33+ messages in thread
From: Jani Nikula @ 2016-02-19 13:08 UTC (permalink / raw)
  To: mika.kahola; +Cc: Deepak M, intel-gfx

On Fri, 19 Feb 2016, Mika Kahola <mika.kahola@intel.com> wrote:
> On Fri, 2016-02-19 at 10:50 +0200, Jani Nikula wrote:
>> On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>> > From: Deepak M <m.deepak@intel.com>
>> >
>> > The bpp value which is used while calulating the txbyteclkhs values
>> > should be wrt the pixel format value. Currently bpp is coming
>> > from pipe config to calculate txbyteclkhs. Fix it in this patch.
>> >
>> > V2: dsi_pixel_format_bpp is used to retrieve the bpp from pixel_format
>> > 	[Review: Jani]
>> >
>> > Signed-off-by: Deepak M <m.deepak@intel.com>
>> > Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohan.marimuthu@intel.com>
>> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> Tested-by: Mika Kahola <mika.kahola@intel.com>
>
> Tested on BYT, Asus T100

Thanks, pushed to drm-intel-next-queued.

BR,
Jani.

>
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> This needs a Tested-by on at least BYT to ensure everything still works.
>> 
>> > ---
>> >  drivers/gpu/drm/i915/intel_dsi.c           |    5 ++---
>> >  drivers/gpu/drm/i915/intel_dsi.h           |    2 ++
>> >  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c |    5 +----
>> >  drivers/gpu/drm/i915/intel_dsi_pll.c       |    2 +-
>> >  4 files changed, 6 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> > index 91cef35..ce94342 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> > @@ -775,10 +775,9 @@ static void set_dsi_timings(struct drm_encoder *encoder,
>> >  {
>> >  	struct drm_device *dev = encoder->dev;
>> >  	struct drm_i915_private *dev_priv = dev->dev_private;
>> > -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>> >  	enum port port;
>> > -	unsigned int bpp = intel_crtc->config->pipe_bpp;
>> > +	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
>> >  	unsigned int lane_count = intel_dsi->lane_count;
>> >  
>> >  	u16 hactive, hfp, hsync, hbp, vfp, vsync, vbp;
>> > @@ -849,7 +848,7 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>> >  	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>> >  	enum port port;
>> > -	unsigned int bpp = intel_crtc->config->pipe_bpp;
>> > +	unsigned int bpp = dsi_pixel_format_bpp(intel_dsi->pixel_format);
>> >  	u32 val, tmp;
>> >  	u16 mode_hdisplay;
>> >  
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> > index de7be7f..92f3922 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi.h
>> > +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> > @@ -34,6 +34,8 @@
>> >  #define DSI_DUAL_LINK_FRONT_BACK	1
>> >  #define DSI_DUAL_LINK_PIXEL_ALT		2
>> >  
>> > +int dsi_pixel_format_bpp(int pixel_format);
>> > +
>> >  struct intel_dsi_host;
>> >  
>> >  struct intel_dsi {
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> > index 1d43e6f..23c0f67 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> > +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> > @@ -420,10 +420,7 @@ struct drm_panel *vbt_panel_init(struct intel_dsi *intel_dsi, u16 panel_id)
>> >  	intel_dsi->dual_link = mipi_config->dual_link;
>> >  	intel_dsi->pixel_overlap = mipi_config->pixel_overlap;
>> >  
>> > -	if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB666)
>> > -		bits_per_pixel = 18;
>> > -	else if (intel_dsi->pixel_format == VID_MODE_FORMAT_RGB565)
>> > -		bits_per_pixel = 16;
>> > +	bits_per_pixel = dsi_pixel_format_bpp(intel_dsi->pixel_format);
>> >  
>> >  	intel_dsi->operation_mode = mipi_config->is_cmd_mode;
>> >  	intel_dsi->video_mode_format = mipi_config->video_transfer_mode;
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> > index bb5e95a..70883c5 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
>> > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
>> > @@ -30,7 +30,7 @@
>> >  #include "i915_drv.h"
>> >  #include "intel_dsi.h"
>> >  
>> > -static int dsi_pixel_format_bpp(int pixel_format)
>> > +int dsi_pixel_format_bpp(int pixel_format)
>> >  {
>> >  	int bpp;
>> 
>
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI
  2016-02-19  9:07             ` Jani Nikula
@ 2016-02-23 14:31               ` Ramalingam C
  0 siblings, 0 replies; 33+ messages in thread
From: Ramalingam C @ 2016-02-23 14:31 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx; +Cc: ville.syrjala, mika.kahola


On Friday 19 February 2016 02:37 PM, Jani Nikula wrote:
> On Thu, 11 Feb 2016, Ramalingam C <ramalingam.c@intel.com> wrote:
>> In case of BXT DSI we are updating the CPU_TRANSCODER
>> with appropriate value.
>>
>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h      |    2 ++
>>   drivers/gpu/drm/i915/intel_display.c |    5 +++++
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 65a2cd0..ef4b376 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -122,6 +122,8 @@ enum transcoder {
>>   	TRANSCODER_B,
>>   	TRANSCODER_C,
>>   	TRANSCODER_EDP,
>> +	TRANSCODER_MIPI_A,
>> +	TRANSCODER_MIPI_C,
>>   	I915_MAX_TRANSCODERS
>>   };
>>   #define transcoder_name(t) ((t) + 'A')
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index e47a75c..9715056 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -7851,6 +7851,11 @@ static void intel_get_dsi_pipe_timings(struct intel_crtc *crtc,
>>   				enc_to_intel_dsi(&encoder->base);
>>   			pipe_config->pipe_bpp =
>>   				dsi_pixel_format_bpp(intel_dsi->pixel_format);
>> +
>> +			if (intel_dsi->ports & (1 << PORT_A))
>> +				pipe_config->cpu_transcoder = TRANSCODER_MIPI_A;
>> +			else
>> +				pipe_config->cpu_transcoder = TRANSCODER_MIPI_C;
>>   		}
>>   	}
> As I've said in my review of the previous version of the patch [1], you
> can't just add transcoder identifiers and expect everything to magically
> work.
>
> The current assumption is there are at most transcoders A, B, C, and
> eDP. We use them to index registers. Not all registers have a
> corresponding DSI transcoder variant, and if they do, they are not
> uniformly spread in the register offset space. See haswell_crtc_disable
> for an example where things go wrong. There are others.
>
> I do not know what the best approach here would be; it is obvious to me
> this can't work.
As correcting the CPU transcoder value alone wont solve our problem.
In the next patch set I am adding corresponding POWER_DOMAIN bits
and adding them to the respective POWER wells. This will enable generic 
power well
management calls to map the cpu transcoders to the corresponding powerwell
and help them to maintain them as per the need. Hope that will address 
the concern raised here.
>
> BR,
> Jani.
>
>
> [1] http://mid.gmane.org/87powceh4r.fsf@intel.com
>
>
>

-- 
Thanks,
--Ram

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2016-02-23 14:39 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 17:54 [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
2016-02-02 17:54 ` [PATCH 2/2] drm/i915/dsi: Add audio reference in dsi encoder Ramalingam C
2016-02-03  1:52   ` Thulasimani, Sivakumar
2016-02-03  8:57   ` Jani Nikula
2016-02-03  9:24     ` Ramalingam C
2016-02-03 10:01       ` Jani Nikula
2016-02-19  9:23         ` Jani Nikula
2016-02-19  9:31           ` Ramalingam C
2016-02-02 18:23 ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue kbuild test robot
2016-02-02 18:38 ` kbuild test robot
2016-02-03  1:49 ` Thulasimani, Sivakumar
2016-02-03 12:20   ` Ramalingam C
2016-02-03  8:02 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] " Patchwork
2016-02-03  9:44 ` [PATCH 1/2] " Jani Nikula
2016-02-03 12:18   ` Ramalingam C
2016-02-03 12:27   ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Ramalingam C
2016-02-03 12:27     ` [PATCH 2/2] drm/i915/BXT: Fixed COS blanking issue Ramalingam C
2016-02-04 13:54       ` Jani Nikula
2016-02-11 14:49         ` Ramalingam C
2016-02-11 14:59           ` [PATCH 2/3 V3] " Ramalingam C
2016-02-19  9:16             ` Jani Nikula
2016-02-04 13:13     ` [PATCH 1/2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
2016-02-11 15:00       ` Ramalingam C
2016-02-11 15:03         ` [PATCH 1/3 V2] " Ramalingam C
2016-02-11 15:05           ` [PATCH 3/3] drm/i915: Updating the CPU_TRANSCODER for BXT DSI Ramalingam C
2016-02-19  9:07             ` Jani Nikula
2016-02-23 14:31               ` Ramalingam C
2016-02-19  8:50           ` [PATCH 1/3 V2] drm/i915: Using the bpp value wrt the pixel format Jani Nikula
2016-02-19 12:50             ` Mika Kahola
2016-02-19 13:08               ` Jani Nikula
2016-02-15 16:28   ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Daniel Vetter
2016-02-03 13:12 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Using the bpp value wrt the pixel format (rev2) Patchwork
2016-02-15 16:24 ` [PATCH 1/2] drm/i915/BXT: Fixed COS blanking issue Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.