Re: [PATCH] drm/i915: don't set unpin_work if vblank_get fails

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 29 Aug 2011 09:44:52 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Mon, 22 Aug 2011 11:05:31 -0700
> Jesse Barnes <jbarnes@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@virtuousgeek.org>
> 
> Updated patch.  Though now I've lost track of the outstanding
> issues...  IIRC we still have another race here, but it's unrelated to
> the one in the bug report.

With patch...

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2319f62..9cd523b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6896,11 +6896,16 @@ 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) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		kfree(work);
+		drm_vblank_put(dev, intel_crtc->pipe);
 
 		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
 		return -EBUSY;
@@ -6919,10 +6924,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	crtc->fb = fb;
 
-	ret = drm_vblank_get(dev, intel_crtc->pipe);
-	if (ret)
-		goto cleanup_objs;
-
 	work->pending_flip_obj = obj;
 
 	work->enable_stall_check = true;
@@ -6945,7 +6946,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 cleanup_pending:
 	atomic_sub(1 << intel_crtc->plane, &work->old_fb_obj->pending_flip);
-cleanup_objs:
 	drm_gem_object_unreference(&work->old_fb_obj->base);
 	drm_gem_object_unreference(&obj->base);
 	mutex_unlock(&dev->struct_mutex);
@@ -6954,6 +6954,8 @@ cleanup_objs:
 	intel_crtc->unpin_work = NULL;
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
+	drm_vblank_put(dev, intel_crtc->pipe);
+free_work:
 	kfree(work);
 
 	return ret;