[PATCH] drm/radeon: fix race condition in radeon_crtc_page_flip v2

Christian König deathsimple at vodafone.de
Thu Jun 19 02:23:40 PDT 2014


From: Christian König <christian.koenig at amd.com>

radeon_crtc_handle_flip can be called concurrently, and if
we set the unpin_work too early, it may try to flip an unpinned BO or
worse.

v2: fix compiler warning, update commit message,
    set crtc->primary->fb only when everything went well

Signed-off-by: Christian König <christian.koenig at amd.com>
Cc: stable at vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_display.c | 39 ++++++++++++++-------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 356b733..8aaa7ac 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -393,17 +393,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
 
 	INIT_WORK(&work->work, radeon_unpin_work_func);
 
-	/* We borrow the event spin lock for protecting unpin_work */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	if (radeon_crtc->unpin_work) {
-		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
-		r = -EBUSY;
-		goto unlock_free;
-	}
-	radeon_crtc->unpin_work = work;
-	radeon_crtc->deferred_flip_completion = 0;
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-
 	/* pin the new buffer */
 	DRM_DEBUG_DRIVER("flip-ioctl() cur_fbo = %p, cur_bbo = %p\n",
 			 work->old_rbo, rbo);
@@ -461,13 +450,6 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
 		base &= ~7;
 	}
 
-	spin_lock_irqsave(&dev->event_lock, flags);
-	work->new_crtc_base = base;
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-
-	/* update crtc fb */
-	crtc->primary->fb = fb;
-
 	r = drm_vblank_get(dev, radeon_crtc->crtc_id);
 	if (r) {
 		DRM_ERROR("failed to get vblank before flip\n");
@@ -477,6 +459,23 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc,
 	/* set the proper interrupt */
 	radeon_pre_page_flip(rdev, radeon_crtc->crtc_id);
 
+	/* We borrow the event spin lock for protecting unpin_work */
+	spin_lock_irqsave(&dev->event_lock, flags);
+	if (radeon_crtc->unpin_work) {
+		spin_unlock_irqrestore(&dev->event_lock, flags);
+		radeon_post_page_flip(rdev, radeon_crtc->crtc_id);
+		drm_vblank_put(dev, radeon_crtc->crtc_id);
+
+		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
+		r = -EBUSY;
+		goto pflip_cleanup1;
+	}
+	radeon_crtc->unpin_work = work;
+	radeon_crtc->deferred_flip_completion = 0;
+	work->new_crtc_base = base;
+	crtc->primary->fb = fb;
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
 	return 0;
 
 pflip_cleanup1:
@@ -490,10 +489,6 @@ pflip_cleanup1:
 	radeon_bo_unreserve(rbo);
 
 pflip_cleanup:
-	spin_lock_irqsave(&dev->event_lock, flags);
-	radeon_crtc->unpin_work = NULL;
-unlock_free:
-	spin_unlock_irqrestore(&dev->event_lock, flags);
 	drm_gem_object_unreference_unlocked(old_radeon_fb->obj);
 	radeon_fence_unref(&work->fence);
 	kfree(work);
-- 
1.9.1



More information about the dri-devel mailing list