Re: [PATCH] drm/i915: fixup the plane->pipe fixup code

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Sep 11, 2012 at 08:17:33AM +0200, Daniel Vetter wrote:
> We need to check whether the _other plane is on our pipe, not whether
> our plane is on the other pipe. Otherwise if not both pipes/planes are
> active, we won't properly clean up the mess and set up our desired
> plane->pipe mapping.
> 
> v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.
> 
> v3: I've checked Bspec, and the flexible plane->pipe mapping is a
> gen2/3 feature, so test for that instead of PCH_SPLIT
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49838
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

For a reason that seems to totally ellude me this breaks resume on my
i855gm. Reliably, and that despite that the plane fixup code isn't run
(according to the debug printk) both with and without this patch ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fd9c275..7454df6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7994,11 +7994,27 @@ static void intel_enable_pipe_a(struct drm_device *dev)
>  
>  }
>  
> +static bool
> +intel_check_plane_mapping(struct intel_crtc *crtc)
> +{
> +	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +	u32 reg, val;
> +
> +	reg = DSPCNTR(!crtc->plane);
> +	val = I915_READ(reg);
> +
> +	if ((val & DISPLAY_PLANE_ENABLE) &&
> +	    (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
> +		return false;
> +
> +	return true;
> +}
> +
>  static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 reg, val;
> +	u32 reg;
>  
>  	/* Clear any frame start delays used for debugging left by the BIOS */
>  	reg = PIPECONF(crtc->pipe);
> @@ -8006,17 +8022,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  
>  	/* We need to sanitize the plane -> pipe mapping first because this will
>  	 * disable the crtc (and hence change the state) if it is wrong. */
> -	if (!HAS_PCH_SPLIT(dev)) {
> +	if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) {
>  		struct intel_connector *connector;
>  		bool plane;
>  
> -		reg = DSPCNTR(crtc->plane);
> -		val = I915_READ(reg);
> -
> -		if ((val & DISPLAY_PLANE_ENABLE) == 0 &&
> -		    (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe))
> -			goto ok;
> -
>  		DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
>  			      crtc->base.base.id);
>  
> @@ -8040,7 +8049,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  		WARN_ON(crtc->active);
>  		crtc->base.enabled = false;
>  	}
> -ok:
>  
>  	if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
>  	    crtc->pipe == PIPE_A && !crtc->active) {
> -- 
> 1.7.11.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch