Am 19.06.2014 05:49, schrieb Michel Dänzer:
This patch only applies to 3.15, right?
Yes correct. For 3.16 we have the reworked flip which I think is still a
good idea to keep for now.
I've addresses your comments and send out a v2 to the list CCing you,
please review.
Thanks,
Christian.
On 19.06.2014 02:11, Christian König wrote:
From: Christian König <christian.koenig@xxxxxxx>
radeon_crtc_handle_flip can be called concurrently and if
we set the unpin_work to early try to flip an unpinned BO or
worse.
Spelling: 'too early'
Maybe something like:
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.
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
drivers/gpu/drm/radeon/radeon_display.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 356b733..cf22741 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,10 +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;
@@ -477,6 +462,22 @@ 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;
+ spin_unlock_irqrestore(&dev->event_lock, flags);
+
This introduces a path where crtc->primary->fb is updated, but then we
return -EBUSY.
It also introduces a warning:
drivers/gpu/drm/radeon/radeon_display.c: In function ‘radeon_crtc_page_flip’:
drivers/gpu/drm/radeon/radeon_display.c:496:1: warning: label ‘unlock_free’ defined but not used [-Wunused-label]
unlock_free:
^
Apart from that, looks good.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
