This patch only applies to 3.15, right?
On 19.06.2014 02:11, Christian K?nig wrote:
> From: Christian K?nig <christian.koenig at amd.com>
>
> 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 at amd.com>
> Cc: stable at vger.kernel.org
> ---
> 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.
--
Earthling Michel D?nzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
