[Intel-gfx] [PATCH] Add modesetting pageflip ioctl and corresponding drm event

Chris Wilson chris at chris-wilson.co.uk
Tue Jun 30 20:37:53 CEST 2009


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 <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







More information about the Intel-gfx mailing list