diff mbox

[1/2] drm/i915/BXT: Fixed COS blanking issue

Message ID 1454435657-29027-1-git-send-email-ramalingam.c@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ramalingam C Feb. 2, 2016, 5:54 p.m. UTC
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(-)

Comments

kernel test robot Feb. 2, 2016, 6:23 p.m. UTC | #1
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
kernel test robot Feb. 2, 2016, 6:38 p.m. UTC | #2
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
Sivakumar Thulasimani Feb. 3, 2016, 1:49 a.m. UTC | #3
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);
Jani Nikula Feb. 3, 2016, 9:44 a.m. UTC | #4
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);
Ramalingam C Feb. 3, 2016, 12:18 p.m. UTC | #5
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);
Ramalingam C Feb. 3, 2016, 12:20 p.m. UTC | #6
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);
>
Daniel Vetter Feb. 15, 2016, 4:24 p.m. UTC | #7
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 Feb. 15, 2016, 4:28 p.m. UTC | #8
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
diff mbox

Patch

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);