Re: [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb
Date: Mon, 16 Jan 2012 23:48:06 +0100	[thread overview]
Message-ID: <20120116224806.GT3627@phenom.ffwll.local> (raw)
In-Reply-To: <1322158938-11261-1-git-send-email-chris@chris-wilson.co.uk>

On Thu, Nov 24, 2011 at 06:22:18PM +0000, Chris Wilson wrote:
> Staring at an error state such as:
> 
> PGTBL_ER: 0x00000400
>     Display B: Invalid tiling
> fence[0] = 05001001
>     valid, x-tiled, pitch: 512, start: 0x05000000, size: 1048576
> Pinned [2]:
>   00000000   131072 0001 0001 00000000 P uncached
>   00020000  4096000 0041 0000 00000000 P uncached (name: 1)
> Plane [1]:
>   CNTR: c0000000 # enabled | gamma
>   STRIDE: 00001400
>   SIZE: 03ff04ff
>   POS: 00000000
>   ADDR: 05000000
> 
> Suggests that we did not clear the DSPBCNTR prior to unpinning the
> framebuffer and reusing the GTT space. Impossible! Unless our DPMS
> bookkeeping ran afoul again...
> 
> In the meantime add an assertion that the plane is decoupled from the
> framebuffer prior to release.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Jesse acked this on irc, but it doesn't apply anymore. Care to rebase?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |   17 ++++++++++++-----
>  1 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5d4cbff..800b36e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -932,19 +932,24 @@ static void assert_pipe(struct drm_i915_private *dev_priv,
>  #define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
>  #define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
>  
> -static void assert_plane_enabled(struct drm_i915_private *dev_priv,
> -				 enum plane plane)
> +static void assert_plane(struct drm_i915_private *dev_priv,
> +			 enum plane plane, bool state)
>  {
>  	int reg;
>  	u32 val;
> +	bool cur_state;
>  
>  	reg = DSPCNTR(plane);
>  	val = I915_READ(reg);
> -	WARN(!(val & DISPLAY_PLANE_ENABLE),
> -	     "plane %c assertion failure, should be active but is disabled\n",
> -	     plane_name(plane));
> +	cur_state = !!(val & DISPLAY_PLANE_ENABLE);
> +	WARN(cur_state != state,
> +	     "plane %c assertion failure (expected %s, current %s)\n",
> +	     plane_name(plane), state_string(state), state_string(cur_state));
>  }
>  
> +#define assert_plane_enabled(d, p) assert_plane(d, p, true)
> +#define assert_plane_disabled(d, p) assert_plane(d, p, false)
> +
>  static void assert_planes_disabled(struct drm_i915_private *dev_priv,
>  				   enum pipe pipe)
>  {
> @@ -3320,6 +3325,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
>  	struct drm_device *dev = crtc->dev;
>  
>  	crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> +	assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane);
> +	assert_pipe_disabled(dev->dev_private, to_intel_crtc(crtc)->pipe);
>  
>  	if (crtc->fb) {
>  		mutex_lock(&dev->struct_mutex);
> -- 
> 1.7.7.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48