[Intel-gfx] [PATCH] Add modesetting pageflip ioctl and corresponding drm event
Kristian Høgsberg
krh at bitplanet.net
Wed Jul 1 02:29:13 CEST 2009
On Tue, Jun 30, 2009 at 2:37 PM, Chris Wilson<chris at chris-wilson.co.uk> wrote:
> 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:
Yikes, that's embarassing. I saw the same kind of glitch that when
running this with wayland too but never debugged it. Your patch looks
good, but I haven't tested it yet.
thanks,
Kristian
> >From aa017e6056cf2faf6be7eeaa71d2fded4a9f6295 Mon Sep 17 00:00:00 2001
> From: Chris Wilson <chris at 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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
>
More information about the Intel-gfx
mailing list