I've just got around to playing with the modesetting page-flip ioctl and found a nasty rendering glitch where the flip occurred before the rendering was flushed. This appears to be because the finish of the pending_flip is queued at the same time as the async set_base().
Applying the following patch prevents the glitch for me: >From aa017e6056cf2faf6be7eeaa71d2fded4a9f6295 Mon Sep 17 00:00:00 2001 From: Chris Wilson <ch...@chris-wilson.co.uk> Date: Tue, 30 Jun 2009 18:21:54 +0100 Subject: [PATCH 1/3] drm: delay unpinning the current fb til after the flip is complete --- drivers/gpu/drm/drm_crtc.c | 45 +++++++++++++++++++++++++++++++++++-------- drivers/gpu/drm/drm_irq.c | 7 +++-- include/drm/drm_crtc.h | 4 ++- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 6a5a779..32212e6 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -344,20 +344,30 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb) EXPORT_SYMBOL(drm_framebuffer_cleanup); /** - * drm_crtc_async_work - do a set_base call from a work queue + * drm_crtc_async_flip - do a set_base call from a work queue * @work: work struct * * Called when a set_base call is queued by the page flip code. This * allows the flip ioctl itself to return immediately and allow userspace * to continue working. */ -static void drm_crtc_async_work(struct work_struct *work) +static void drm_crtc_async_flip(struct work_struct *work) { - struct drm_crtc *crtc = container_of(work, struct drm_crtc, async_work); + struct drm_crtc *crtc = container_of(work, struct drm_crtc, async_flip); struct drm_device *dev = crtc->dev; + struct drm_pending_flip *pending; + + BUG_ON(crtc->pending_flip == NULL); mutex_lock(&dev->struct_mutex); crtc->funcs->set_base(crtc, 0, 0, NULL); + + pending = crtc->pending_flip; + crtc->pending_flip = NULL; + + pending->frame = drm_vblank_count(dev, crtc->pipe); + list_add_tail(&pending->link, &dev->flip_list); + mutex_unlock(&dev->struct_mutex); } @@ -384,7 +394,7 @@ void drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, int pipe, list_add_tail(&crtc->head, &dev->mode_config.crtc_list); dev->mode_config.num_crtc++; - INIT_WORK(&crtc->async_work, drm_crtc_async_work); + INIT_WORK(&crtc->async_flip, drm_crtc_async_flip); mutex_unlock(&dev->mode_config.mutex); } EXPORT_SYMBOL(drm_crtc_init); @@ -404,7 +414,7 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; mutex_lock(&dev->mode_config.mutex); - flush_work(&crtc->async_work); + flush_work(&crtc->async_flip); if (crtc->gamma_store) { kfree(crtc->gamma_store); @@ -2569,9 +2579,6 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, void *data, goto out_unlock; } - pending->frame = drm_vblank_count(dev, crtc->pipe); - list_add_tail(&pending->link, &dev->flip_list); - /* * The set_base call will change the domain on the new fb, * which will force the rendering to finish and block the @@ -2580,7 +2587,27 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, void *data, */ crtc->fb = obj_to_fb(fb_obj); - schedule_work(&crtc->async_work); + if (crtc->pending_flip != NULL) { + struct drm_pending_flip *old_flip; + + /* We have an outstanding flip request for this crtc/pipe. + * In order to satisfy the user we can either queue the requests + * and apply them on sequential vblanks, or we can drop old + * requests. + * + * Here we choose to discard the previous request for + * simplicity. Note that since we have not yet applied the + * previous flip, we need to preserve the original (i.e. still + * current) fb. + */ + + old_flip = crtc->pending_flip; + pending->old_fb = old_flip->old_fb; + old_flip->old_fb = NULL; + drm_finish_pending_flip (dev, old_flip, 0); + } else + schedule_work(&crtc->async_flip); + crtc->pending_flip = pending; mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 8b4d0c8..c7a17f6 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -72,7 +72,7 @@ int drm_irq_by_busid(struct drm_device *dev, void *data, return 0; } -#define vblank_after(a,b) ((long)(b) - (long)(a) < 0) +#define vblank_passed(a,b) ((long)(a - b) > 0) void drm_finish_pending_flip(struct drm_device *dev, struct drm_pending_flip *f, u32 frame) @@ -87,7 +87,8 @@ void drm_finish_pending_flip(struct drm_device *dev, list_del_init(&f->link); list_add_tail(&f->pending_event.link, &f->pending_event.file_priv->event_list); - f->old_fb->funcs->unpin(f->old_fb); + if (f->old_fb) + f->old_fb->funcs->unpin(f->old_fb); wake_up_interruptible(&f->pending_event.file_priv->event_wait); } @@ -102,7 +103,7 @@ static void drm_flip_work_func(struct work_struct *work) list_for_each_entry_safe(f, t, &dev->flip_list, link) { frame = drm_vblank_count(dev, f->pipe); - if (vblank_after(frame, f->frame)) + if (vblank_passed(frame, f->frame)) drm_finish_pending_flip(dev, f, frame); } diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 99fae10..0b5dc47 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -294,6 +294,7 @@ struct drm_property { struct drm_crtc; struct drm_connector; struct drm_encoder; +struct drm_pending_flip; /** * drm_crtc_funcs - control CRTCs for a given device @@ -388,7 +389,8 @@ struct drm_crtc { uint16_t *gamma_store; /* Allow async set_pipe_base calls for flipping */ - struct work_struct async_work; + struct work_struct async_flip; + struct drm_pending_flip *pending_flip; /* if you are using the helper */ void *helper_private; -- 1.6.3.3 ------------------------------------------------------------------------------ -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel