[PATCH] drm/i915: Pass along pipe_config to intel_psr_ready()

[PATCH] drm/i915: Pass along pipe_config to intel_psr_ready()

[Chris Wilson]

Currently intel_psr_ready() blindly dereferences the crtc associated
with the DP connector/encoder, however this is called during atomic
preparation in which case the association is not on the encoder but in
the passed along pipe_config.

Fixes regression and OOPS from
commit d6e6929df37dc4758e93f87a3a9a2dc0eabd9743
Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
Date:   Thu Mar 26 12:20:26 2015 -0700

    drm/i915: Add psr_ready on pipe_config

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89835
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
CC: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c  |  2 +-
 drivers/gpu/drm/i915/intel_drv.h |  3 ++-
 drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++++-------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9b741b581768..cc20b33a76c7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1389,7 +1389,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		min_lane_count = max_lane_count;
 		min_clock = max_clock;
 
-		pipe_config->psr_ready = intel_psr_ready(intel_dp);
+		pipe_config->psr_ready = intel_psr_ready(intel_dp, pipe_config);
 	}
 
 	for (; bpp >= 6*3; bpp -= 2*3) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0794b610fc2e..0e6534397ed9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1210,7 +1210,8 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
-bool intel_psr_ready(struct intel_dp *intel_dp);
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 74f6556c1386..229b849180d6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -255,6 +255,7 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 /**
  * intel_psr_ready - PSR ready
  * @intel_dp: Intel DP
+ * @pipe_config: ditto
  *
  * This function Checks if PSR is supported by Hardware/Source and
  * Panel/Sink and if all conditions to be enabled are fulfilled.
@@ -264,21 +265,23 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
  * Returns:
  * True when PSR is ready to be enabled, false otherwise.
  */
-bool intel_psr_ready(struct intel_dp *intel_dp)
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = dig_port->base.base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
 
 	if (!HAS_PSR(dev)) {
 		DRM_DEBUG_KMS("PSR not supported on this platform\n");
 		return false;
 	}
 
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	if (WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)))
+		return false;
+	if (WARN_ON(crtc == NULL || !drm_modeset_is_locked(&crtc->base.mutex)))
+		return false;
 
 	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
 		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
@@ -291,14 +294,14 @@ bool intel_psr_ready(struct intel_dp *intel_dp)
 	}
 
 	if (IS_HASWELL(dev) &&
-	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config->cpu_transcoder)) &
+	    I915_READ(HSW_STEREO_3D_CTL(crtc->config->cpu_transcoder)) &
 		      S3D_ENABLE) {
 		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
 		return false;
 	}
 
 	if (IS_HASWELL(dev) &&
-	    intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+	    crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
 		return false;
 	}
-- 
2.1.4

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

Re: [PATCH] drm/i915: Pass along pipe_config to intel_psr_ready()

[Daniel Vetter]

On Tue, Mar 31, 2015 at 10:52:31AM +0100, Chris Wilson wrote:
> Currently intel_psr_ready() blindly dereferences the crtc associated
> with the DP connector/encoder, however this is called during atomic
> preparation in which case the association is not on the encoder but in
> the passed along pipe_config.
> 
> Fixes regression and OOPS from
> commit d6e6929df37dc4758e93f87a3a9a2dc0eabd9743
> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Date:   Thu Mar 26 12:20:26 2015 -0700
> 
>     drm/i915: Add psr_ready on pipe_config
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89835
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> CC: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h |  3 ++-
>  drivers/gpu/drm/i915/intel_psr.c | 17 ++++++++++-------
>  3 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9b741b581768..cc20b33a76c7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1389,7 +1389,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		min_lane_count = max_lane_count;
>  		min_clock = max_clock;
>  
> -		pipe_config->psr_ready = intel_psr_ready(intel_dp);
> +		pipe_config->psr_ready = intel_psr_ready(intel_dp, pipe_config);
>  	}
>  
>  	for (; bpp >= 6*3; bpp -= 2*3) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0794b610fc2e..0e6534397ed9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1210,7 +1210,8 @@ void intel_backlight_unregister(struct drm_device *dev);
>  
>  
>  /* intel_psr.c */
> -bool intel_psr_ready(struct intel_dp *intel_dp);
> +bool intel_psr_ready(struct intel_dp *intel_dp,
> +		     struct intel_crtc_state *pipe_config);
>  void intel_psr_enable(struct intel_dp *intel_dp);
>  void intel_psr_disable(struct intel_dp *intel_dp);
>  void intel_psr_invalidate(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 74f6556c1386..229b849180d6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -255,6 +255,7 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>  /**
>   * intel_psr_ready - PSR ready
>   * @intel_dp: Intel DP
> + * @pipe_config: ditto
>   *
>   * This function Checks if PSR is supported by Hardware/Source and
>   * Panel/Sink and if all conditions to be enabled are fulfilled.
> @@ -264,21 +265,23 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>   * Returns:
>   * True when PSR is ready to be enabled, false otherwise.
>   */
> -bool intel_psr_ready(struct intel_dp *intel_dp)
> +bool intel_psr_ready(struct intel_dp *intel_dp,
> +		     struct intel_crtc_state *pipe_config)
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_crtc *crtc = dig_port->base.base.crtc;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
>  
>  	if (!HAS_PSR(dev)) {
>  		DRM_DEBUG_KMS("PSR not supported on this platform\n");
>  		return false;
>  	}
>  
> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> -	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
> +	if (WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)))
> +		return false;
> +	if (WARN_ON(crtc == NULL || !drm_modeset_is_locked(&crtc->base.mutex)))
> +		return false;
>  
>  	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
>  		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
> @@ -291,14 +294,14 @@ bool intel_psr_ready(struct intel_dp *intel_dp)
>  	}
>  
>  	if (IS_HASWELL(dev) &&
> -	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config->cpu_transcoder)) &
> +	    I915_READ(HSW_STEREO_3D_CTL(crtc->config->cpu_transcoder)) &

Shouldn't you check pipe_config here to make this patch complete? Dropped
the offending two patches meanwhile to avoid trouble.
-Daniel

>  		      S3D_ENABLE) {
>  		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
>  		return false;
>  	}
>  
>  	if (IS_HASWELL(dev) &&
> -	    intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
> +	    crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
>  		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
>  		return false;
>  	}
> -- 
> 2.1.4
> 

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

Re: [PATCH] drm/i915: Pass along pipe_config to intel_psr_ready()

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6102
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -3              272/272              269/272
ILK                 -1              302/302              301/302
SNB                                  303/303              303/303
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  361/361              361/361
BDW                                  308/308              308/308
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(2)PASS(3)      CRASH(2)
 PNV  igt@gem_tiled_pread_pwrite      FAIL(3)PASS(2)      FAIL(2)
 PNV  igt@gen3_render_tiledx_blits      FAIL(3)PASS(1)      FAIL(2)
*ILK  igt@kms_flip@wf_vblank-ts-check      PASS(2)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:drm_edid_block_valid[drm]]*ERROR*EDID_checksum_is_invalid,remainder_is@EDID checksum is .* remainder is
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx