[Intel-gfx] [PATCH] drm/i915: don't set unpin_work if vblank_get fails

[Keith Packard]

On Mon, 22 Aug 2011 11:05:31 -0700, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> This fixes a race where we may try to finish a page flip and decrement
> the refcount even if our vblank_get failed and we ended up with a
> spurious flip pending interrupt.
> 
> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=34211.
> 
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2319f62..0910537 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6896,6 +6896,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	work->old_fb_obj = intel_fb->obj;
>  	INIT_WORK(&work->work, intel_unpin_work_fn);
>  
> +	ret = drm_vblank_get(dev, intel_crtc->pipe);
> +	if (ret)
> +		goto free_work;
> +
>  	/* We borrow the event spin lock for protecting unpin_work */
>  	spin_lock_irqsave(&dev->event_lock, flags);
>  	if (intel_crtc->unpin_work) {
> @@ -6906,6 +6910,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  		return -EBUSY;

You'll need a drm_vblank_put above this return.

>  	}
>  	intel_crtc->unpin_work = work;
> +	/*
> +	 * Past this point, if we fail we'll let the flip completion code
> +	 * clean up the vblank refcount and pin work.  It'll be a spurious
> +	 * completion, but we handle that case.
> +	 */

I don't see how this is going to happen reliably; the hardware will have
to generate a suitable interrupt, which on IRL and later will have to be
an actual page flip interrupt.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20110822/53afec60/attachment.sig>