xf86-video-intel: 5 commits - man/intel.man src/intel_display.c src/intel_dri.c src/intel_driver.c src/intel.h src/sna/sna_dri.c

Chris Wilson ickle at kemper.freedesktop.org
Mon Jul 11 14:21:03 PDT 2011


 man/intel.man       |   11 +
 src/intel.h         |   14 +
 src/intel_display.c |    7 
 src/intel_dri.c     |  403 +++++++++++++++++++++++++++++++++-------------------
 src/intel_driver.c  |   16 ++
 src/sna/sna_dri.c   |  159 ++++++++++++++------
 6 files changed, 421 insertions(+), 189 deletions(-)

New commits:
commit 32f42358140ee812984149ae52b0df3dfd1778c3
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Mon Jul 11 20:29:53 2011 +0100

    sna/dri: Add some simple debugging
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_dri.c b/src/sna/sna_dri.c
index 663e247..30233ac 100644
--- a/src/sna/sna_dri.c
+++ b/src/sna/sna_dri.c
@@ -557,6 +557,8 @@ get_resource(XID id, RESTYPE type)
 		return NULL;
 
 	if (!AddResource(id, type, resource)) {
+		DBG(("%s: failed to add resource (%ld, %ld)\n",
+		     __FUNCTION__, (long)id, (long)type));
 		free(resource);
 		return NULL;
 	}
@@ -572,6 +574,8 @@ sna_dri_frame_event_client_gone(void *data, XID id)
 {
 	struct sna_dri_resource *resource = data;
 
+	DBG(("%s(%ld)\n", __FUNCTION__, (long)id));
+
 	while (!list_is_empty(&resource->list)) {
 		struct sna_dri_frame_event *info =
 			list_first_entry(&resource->list,
@@ -591,6 +595,8 @@ sna_dri_frame_event_drawable_gone(void *data, XID id)
 {
 	struct sna_dri_resource *resource = data;
 
+	DBG(("%s(%ld)\n", __FUNCTION__, (long)id));
+
 	while (!list_is_empty(&resource->list)) {
 		struct sna_dri_frame_event *info =
 			list_first_entry(&resource->list,
@@ -644,13 +650,16 @@ sna_dri_add_frame_event(struct sna_dri_frame_event *info)
 
 	resource = get_resource(get_client_id(info->client),
 				frame_event_client_type);
-	if (resource == NULL)
+	if (resource == NULL) {
+		DBG(("%s: failed to get client resource\n", __FUNCTION__));
 		return FALSE;
+	}
 
 	list_add(&info->client_resource, &resource->list);
 
 	resource = get_resource(info->drawable_id, frame_event_drawable_type);
 	if (resource == NULL) {
+		DBG(("%s: failed to get drawable resource\n", __FUNCTION__));
 		list_del(&info->client_resource);
 		return FALSE;
 	}
@@ -1169,6 +1178,8 @@ sna_dri_schedule_swap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 		struct sna_dri_private *back_priv = back->driverPrivate;
 		PixmapPtr pixmap;
 
+		DBG(("%s: off-screen, immediate update\n", __FUNCTION__));
+
 		if (!flip)
 			goto blit_fallback;
 
@@ -1201,6 +1212,7 @@ sna_dri_schedule_swap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 	info->pipe = pipe;
 
 	if (!sna_dri_add_frame_event(info)) {
+		DBG(("%s: failed to hook up frame event\n", __FUNCTION__));
 		free(info);
 		info = NULL;
 		goto blit_fallback;
commit a46598220ebf5d4e629e1e0a7baf47ce144ed2c8
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Mon Jul 11 14:40:30 2011 +0100

    sna/dri: Refactor common code for assigning a pixmap to the DRI2 buffer
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_dri.c b/src/sna/sna_dri.c
index f5d81b3..663e247 100644
--- a/src/sna/sna_dri.c
+++ b/src/sna/sna_dri.c
@@ -896,10 +896,21 @@ done:
 	sna_dri_frame_event_info_free(info);
 }
 
+static void set_pixmap(struct sna *sna, DRI2BufferPtr buffer, PixmapPtr pixmap)
+{
+	struct sna_dri_private *priv = buffer->driverPrivate;
+
+	assert(priv->pixmap->refcnt > 1);
+	priv->pixmap->refcnt--;
+	priv->pixmap = pixmap;
+	priv->bo = sna_pixmap_set_dri(sna, pixmap);
+	buffer->name = kgem_bo_flink(&sna->kgem, priv->bo);
+	buffer->pitch = priv->bo->pitch;
+}
+
 static int
 sna_dri_flip(struct sna *sna, DrawablePtr draw, struct sna_dri_frame_event *info)
 {
-	struct sna_dri_private *front_priv = info->front->driverPrivate;
 	struct sna_dri_frame_event *pending;
 	ScreenPtr screen = draw->pScreen;
 	PixmapPtr pixmap;
@@ -939,11 +950,7 @@ sna_dri_flip(struct sna *sna, DrawablePtr draw, struct sna_dri_frame_event *info
 					   draw->depth,
 					   SNA_CREATE_FB))) {
 		DBG(("%s: new back buffer\n", __FUNCTION__));
-		front_priv->pixmap->refcnt--;
-		front_priv->pixmap = pixmap;
-		front_priv->bo = sna_pixmap_set_dri(sna, pixmap);
-		info->front->name = kgem_bo_flink(&sna->kgem, front_priv->bo);
-		info->front->pitch = front_priv->bo->pitch;
+		set_pixmap(sna, info->front, pixmap);
 	}
 
 	sna_dri_exchange_buffers(draw, info->front, info->back);
@@ -1402,12 +1409,7 @@ blit:
 						   draw->depth,
 						   SNA_CREATE_FB))) {
 			DBG(("%s: new back buffer\n", __FUNCTION__));
-			assert(front_priv->pixmap->refcnt > 1);
-			front_priv->pixmap->refcnt--;
-			front_priv->pixmap = pixmap;
-			front_priv->bo = sna_pixmap_set_dri(sna, pixmap);
-			front->name = kgem_bo_flink(&sna->kgem, front_priv->bo);
-			front->pitch = front_priv->bo->pitch;
+			set_pixmap(sna, front, pixamp);
 		}
 	} else if (info->type != DRI2_ASYNC_FLIP) {
 		/* A normal vsync'ed client is finishing, wait for it
@@ -1423,12 +1425,7 @@ blit:
 					   draw->depth,
 					   SNA_CREATE_FB))) {
 		DBG(("%s: new back buffer\n", __FUNCTION__));
-		assert(front_priv->pixmap->refcnt > 1);
-		front_priv->pixmap->refcnt--;
-		front_priv->pixmap = pixmap;
-		front_priv->bo = sna_pixmap_set_dri(sna, pixmap);
-		front->name = kgem_bo_flink(&sna->kgem, front_priv->bo);
-		front->pitch = front_priv->bo->pitch;
+		set_pixmap(sna, front, pixamp);
 	}
 
 exchange:
commit 7538be3315b8683b05e8f6b22023baadcc0bc4da
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Mon Jul 11 10:50:36 2011 +0100

    dri: Enable triple-bufferred pageflips
    
    By popular demand.
    
    Triple-buffering trade-offs output latency versus jitter. By having a
    pre-rendered frame ready to swap in following a pageflip, we avoid the
    scenario where the latency between receiving the flip complete signal
    from the kernel, waking up the vsynced application, it render the new
    frame and then for the server to process the swap request is greater
    than the frame interval, causing us to miss the vblank. The result is
    that application can become frame-locked to 30fps. Instead, we report to
    the application that the first frame swap is immediately completed,
    supply a new back buffer (or else the rendering would be blocked on
    waiting for the front-buffer to be swapped away from the scanout) and
    let them proceed to render the second frame. The second frame is added
    to the swap queue, and the client throttled to vrefresh. (If the client
    missed the vblank, the swap queue is empty and the client is immediately
    woken again, whilst the pageflip is pending.)
    
    Note, for practical reasons this only applies to page-flipping, for
    example, calls to glXSwapBuffer() on fullscreen applications.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/man/intel.man b/man/intel.man
index e5e0572..282b9f3 100644
--- a/man/intel.man
+++ b/man/intel.man
@@ -180,6 +180,17 @@ the framerate of applications that render frames at less than refresh rate.
 .IP
 Default: enabled.
 .TP
+.BI "Option \*qTripleBuffer\*q \*q" boolean \*q
+This option enables the use of a third buffer for page-flipping. The third
+buffer allows applications to run at vrefresh rates even if they occasionally
+fail to swapbuffers on time. The effect of such missed swaps is the output
+jitters between 60fps and 30fps, and in the worst case appears frame-locked
+to 30fps. The disadvantage of triple buffering is that there is an extra
+frame of latency, due to the pre-rendered frame sitting in the swap queue,
+between input and any display update.
+.IP
+Default: enabled.
+.TP
 .BI "Option \*qTiling\*q \*q" boolean \*q
 This option controls whether memory buffers for Pixmaps are allocated in tiled mode.  In
 most cases (especially for complex rendering), tiling dramatically improves
diff --git a/src/intel.h b/src/intel.h
index 9d64d30..6135349 100644
--- a/src/intel.h
+++ b/src/intel.h
@@ -260,7 +260,7 @@ typedef struct intel_screen_private {
 	unsigned int current_batch;
 
 	void *modes;
-	drm_intel_bo *front_buffer;
+	drm_intel_bo *front_buffer, *back_buffer;
 	long front_pitch, front_tiling;
 	void *shadow_buffer;
 	int shadow_stride;
@@ -423,12 +423,15 @@ typedef struct intel_screen_private {
 	char *deviceName;
 
 	Bool use_pageflipping;
+	Bool use_triple_buffer;
 	Bool force_fallback;
 	Bool can_blt;
 	Bool has_kernel_flush;
 	Bool needs_flush;
 	Bool use_shadow;
 
+	struct _DRI2FrameEvent *pending_flip[2];
+
 	/* Broken-out options. */
 	OptionInfoPtr Options;
 
@@ -465,6 +468,7 @@ extern int intel_output_dpms_status(xf86OutputPtr output);
 
 enum DRI2FrameEventType {
 	DRI2_SWAP,
+	DRI2_SWAP_CHAIN,
 	DRI2_FLIP,
 	DRI2_WAITMSC,
 };
@@ -475,10 +479,13 @@ typedef void (*DRI2SwapEventPtr)(ClientPtr client, void *data, int type,
 #endif
 
 typedef struct _DRI2FrameEvent {
+	struct intel_screen_private *intel;
+
 	XID drawable_id;
 	ClientPtr client;
 	enum DRI2FrameEventType type;
 	int frame;
+	int pipe;
 
 	struct list drawable_resource, client_resource;
 
@@ -487,6 +494,8 @@ typedef struct _DRI2FrameEvent {
 	void *event_data;
 	DRI2BufferPtr front;
 	DRI2BufferPtr back;
+
+	struct _DRI2FrameEvent *chain;
 } DRI2FrameEventRec, *DRI2FrameEventPtr;
 
 extern Bool intel_do_pageflip(intel_screen_private *intel,
diff --git a/src/intel_display.c b/src/intel_display.c
index b55b110..e75819d 100644
--- a/src/intel_display.c
+++ b/src/intel_display.c
@@ -1371,6 +1371,11 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
 	old_fb_id = mode->fb_id;
 	old_front = intel->front_buffer;
 
+	if (intel->back_buffer) {
+		drm_intel_bo_unreference(intel->back_buffer);
+		intel->back_buffer = NULL;
+	}
+
 	intel->front_buffer = intel_allocate_framebuffer(scrn,
 							 width, height,
 							 intel->cpp,
@@ -1447,6 +1452,8 @@ intel_do_pageflip(intel_screen_private *intel,
 			 new_front->handle, &mode->fb_id))
 		goto error_out;
 
+	intel_batch_submit(scrn);
+
 	/*
 	 * Queue flips on all enabled CRTCs
 	 * Note that if/when we get per-CRTC buffers, we'll have to update this.
diff --git a/src/intel_dri.c b/src/intel_dri.c
index 38d7a6b..0b28452 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -732,15 +732,21 @@ i830_dri2_add_frame_event(DRI2FrameEventPtr info)
 }
 
 static void
-i830_dri2_del_frame_event(DRI2FrameEventPtr info)
+i830_dri2_del_frame_event(DrawablePtr drawable, DRI2FrameEventPtr info)
 {
 	list_del(&info->client_resource);
 	list_del(&info->drawable_resource);
+
+	if (info->front)
+		I830DRI2DestroyBuffer(drawable, info->front);
+	if (info->back)
+		I830DRI2DestroyBuffer(drawable, info->back);
+
+	free(info);
 }
 
 static void
-I830DRI2ExchangeBuffers(DrawablePtr draw, DRI2BufferPtr front,
-			DRI2BufferPtr back)
+I830DRI2ExchangeBuffers(DrawablePtr draw, DRI2BufferPtr front, DRI2BufferPtr back)
 {
 	I830DRI2BufferPrivatePtr front_priv, back_priv;
 	struct intel_pixmap *front_intel, *back_intel;
@@ -760,20 +766,18 @@ I830DRI2ExchangeBuffers(DrawablePtr draw, DRI2BufferPtr front,
 	front_intel = intel_get_pixmap_private(front_priv->pixmap);
 	back_intel = intel_get_pixmap_private(back_priv->pixmap);
 	intel_set_pixmap_private(front_priv->pixmap, back_intel);
-	intel_set_pixmap_private(back_priv->pixmap, front_intel); /* should be screen */
+	intel_set_pixmap_private(back_priv->pixmap, front_intel);
 
-	/* Do we need to update the Screen? */
 	screen = draw->pScreen;
 	intel = intel_get_screen_private(xf86Screens[screen->myNum]);
-	if (front_intel->bo == intel->front_buffer) {
-	    dri_bo_unreference (intel->front_buffer);
-	    intel->front_buffer = back_intel->bo;
-	    dri_bo_reference (intel->front_buffer);
-	    intel_set_pixmap_private(screen->GetScreenPixmap(screen),
-				     back_intel);
-	    back_intel->busy = 1;
-	    front_intel->busy = -1;
-	}
+
+	dri_bo_unreference (intel->front_buffer);
+	intel->front_buffer = back_intel->bo;
+	dri_bo_reference (intel->front_buffer);
+
+	intel_set_pixmap_private(screen->GetScreenPixmap(screen), back_intel);
+	back_intel->busy = 1;
+	front_intel->busy = -1;
 }
 
 /*
@@ -782,47 +786,79 @@ I830DRI2ExchangeBuffers(DrawablePtr draw, DRI2BufferPtr front,
  */
 static Bool
 I830DRI2ScheduleFlip(struct intel_screen_private *intel,
-		     ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
-		     DRI2BufferPtr back, DRI2SwapEventPtr func, void *data,
-		     unsigned int target_msc)
+		     DrawablePtr draw,
+		     DRI2FrameEventPtr info)
 {
-	I830DRI2BufferPrivatePtr back_priv;
-	DRI2FrameEventPtr flip_info;
-
-	/* Main crtc for this drawable shall finally deliver pageflip event. */
-	int ref_crtc_hw_id = I830DRI2DrawablePipe(draw);
-
-	flip_info = calloc(1, sizeof(DRI2FrameEventRec));
-	if (!flip_info)
-	    return FALSE;
-
-	flip_info->drawable_id = draw->id;
-	flip_info->client = client;
-	flip_info->type = DRI2_SWAP;
-	flip_info->event_complete = func;
-	flip_info->event_data = data;
-	flip_info->frame = target_msc;
-
-	if (!i830_dri2_add_frame_event(flip_info)) {
-	    free(flip_info);
-	    return FALSE;
+	I830DRI2BufferPrivatePtr priv = info->back->driverPrivate;
+	drm_intel_bo *new_back, *old_back;
+
+	if (!intel->use_triple_buffer) {
+		if (!intel_do_pageflip(intel,
+				       intel_get_pixmap_bo(priv->pixmap),
+				       info, info->pipe))
+			return FALSE;
+
+		info->type = DRI2_SWAP;
+		I830DRI2ExchangeBuffers(draw, info->front, info->back);
+		return TRUE;
 	}
 
-	/* Page flip the full screen buffer */
-	back_priv = back->driverPrivate;
-	if (intel_do_pageflip(intel,
-			      intel_get_pixmap_bo(back_priv->pixmap),
-			      flip_info, ref_crtc_hw_id))
+	if (intel->pending_flip[info->pipe]) {
+		assert(intel->pending_flip[info->pipe]->chain == NULL);
+		intel->pending_flip[info->pipe]->chain = info;
 		return TRUE;
+	}
 
-	i830_dri2_del_frame_event(flip_info);
-	free(flip_info);
-	return FALSE;
+	if (intel->back_buffer == NULL) {
+		new_back = drm_intel_bo_alloc(intel->bufmgr, "front buffer",
+					      intel->front_buffer->size, 0);
+		if (new_back == NULL)
+			return FALSE;
+
+		if (intel->front_tiling != I915_TILING_NONE) {
+			uint32_t tiling = intel->front_tiling;
+			drm_intel_bo_set_tiling(new_back, &tiling, intel->front_pitch);
+			if (tiling != intel->front_tiling) {
+				drm_intel_bo_unreference(new_back);
+				return FALSE;
+			}
+		}
+
+		drm_intel_bo_disable_reuse(new_back);
+	} else {
+		new_back = intel->back_buffer;
+		intel->back_buffer = NULL;
+	}
+
+	old_back = intel_get_pixmap_bo(priv->pixmap);
+	if (!intel_do_pageflip(intel, old_back, info, info->pipe)) {
+		intel->back_buffer = new_back;
+		return FALSE;
+	}
+	info->type = DRI2_SWAP_CHAIN;
+	intel->pending_flip[info->pipe] = info;
+
+	/* Exchange the current front-buffer with the fresh bo */
+	intel->back_buffer = intel->front_buffer;
+	drm_intel_bo_reference(intel->back_buffer);
+
+	priv = info->front->driverPrivate;
+	intel_set_pixmap_bo(priv->pixmap, new_back);
+	dri_bo_flink(new_back, &info->front->name);
+
+	/* Then flip DRI2 pointers and update the screen pixmap */
+	I830DRI2ExchangeBuffers(draw, info->front, info->back);
+	DRI2SwapComplete(info->client, draw, 0, 0, 0,
+			 DRI2_EXCHANGE_COMPLETE,
+			 info->event_complete,
+			 info->event_data);
+	return TRUE;
 }
 
 static Bool
-can_exchange(DRI2BufferPtr front, DRI2BufferPtr back)
+can_exchange(DrawablePtr drawable, DRI2BufferPtr front, DRI2BufferPtr back)
 {
+	struct intel_screen_private *intel = intel_get_screen_private(xf86Screens[drawable->pScreen->myNum]);
 	I830DRI2BufferPrivatePtr front_priv = front->driverPrivate;
 	I830DRI2BufferPrivatePtr back_priv = back->driverPrivate;
 	PixmapPtr front_pixmap = front_priv->pixmap;
@@ -830,6 +866,18 @@ can_exchange(DRI2BufferPtr front, DRI2BufferPtr back)
 	struct intel_pixmap *front_intel = intel_get_pixmap_private(front_pixmap);
 	struct intel_pixmap *back_intel = intel_get_pixmap_private(back_pixmap);
 
+	if (drawable == NULL)
+		return FALSE;
+
+	if (!DRI2CanFlip(drawable))
+		return FALSE;
+
+	if (intel->shadow_present)
+		return FALSE;
+
+	if (!intel->use_pageflipping)
+		return FALSE;
+
 	if (front_pixmap->drawable.width != back_pixmap->drawable.width)
 		return FALSE;
 
@@ -855,10 +903,8 @@ can_exchange(DRI2BufferPtr front, DRI2BufferPtr back)
 void I830DRI2FrameEventHandler(unsigned int frame, unsigned int tv_sec,
 			       unsigned int tv_usec, DRI2FrameEventPtr swap_info)
 {
+	intel_screen_private *intel = swap_info->intel;
 	DrawablePtr drawable;
-	ScreenPtr screen;
-	ScrnInfoPtr scrn;
-	intel_screen_private *intel;
 	int status;
 
 	if (!swap_info->drawable_id)
@@ -867,56 +913,33 @@ void I830DRI2FrameEventHandler(unsigned int frame, unsigned int tv_sec,
 		status = dixLookupDrawable(&drawable, swap_info->drawable_id, serverClient,
 					   M_ANY, DixWriteAccess);
 	if (status != Success) {
-		i830_dri2_del_frame_event(swap_info);
-		I830DRI2DestroyBuffer(NULL, swap_info->front);
-		I830DRI2DestroyBuffer(NULL, swap_info->back);
-		free(swap_info);
+		i830_dri2_del_frame_event(NULL, swap_info);
 		return;
 	}
 
-	screen = drawable->pScreen;
-	scrn = xf86Screens[screen->myNum];
-	intel = intel_get_screen_private(scrn);
 
 	switch (swap_info->type) {
 	case DRI2_FLIP:
 		/* If we can still flip... */
-		if (DRI2CanFlip(drawable) && !intel->shadow_present &&
-		    intel->use_pageflipping &&
-		    can_exchange(swap_info->front, swap_info->back) &&
-		    I830DRI2ScheduleFlip(intel,
-					 swap_info->client, drawable, swap_info->front,
-					 swap_info->back, swap_info->event_complete,
-					 swap_info->event_data, swap_info->frame)) {
-			I830DRI2ExchangeBuffers(drawable,
-						swap_info->front, swap_info->back);
-			break;
-		}
+		if (can_exchange(drawable, swap_info->front, swap_info->back) &&
+		    I830DRI2ScheduleFlip(intel, drawable, swap_info))
+			return;
+
 		/* else fall through to exchange/blit */
 	case DRI2_SWAP: {
-		int swap_type;
-
-		if (DRI2CanExchange(drawable) && can_exchange(swap_info->front,
-							      swap_info->back)) {
-			I830DRI2ExchangeBuffers(drawable,
-						swap_info->front, swap_info->back);
-			swap_type = DRI2_EXCHANGE_COMPLETE;
-		} else {
-			BoxRec	    box;
-			RegionRec	    region;
-
-			box.x1 = 0;
-			box.y1 = 0;
-			box.x2 = drawable->width;
-			box.y2 = drawable->height;
-			REGION_INIT(pScreen, &region, &box, 0);
-
-			I830DRI2CopyRegion(drawable,
-					   &region, swap_info->front, swap_info->back);
-			swap_type = DRI2_BLIT_COMPLETE;
-		}
+		BoxRec box;
+		RegionRec region;
+
+		box.x1 = 0;
+		box.y1 = 0;
+		box.x2 = drawable->width;
+		box.y2 = drawable->height;
+		REGION_INIT(pScreen, &region, &box, 0);
+
+		I830DRI2CopyRegion(drawable,
+				   &region, swap_info->front, swap_info->back);
 		DRI2SwapComplete(swap_info->client, drawable, frame, tv_sec, tv_usec,
-				 swap_type,
+				 DRI2_BLIT_COMPLETE,
 				 swap_info->client ? swap_info->event_complete : NULL,
 				 swap_info->event_data);
 		break;
@@ -927,43 +950,34 @@ void I830DRI2FrameEventHandler(unsigned int frame, unsigned int tv_sec,
 					    frame, tv_sec, tv_usec);
 		break;
 	default:
-		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+		xf86DrvMsg(intel->scrn->scrnIndex, X_WARNING,
 			   "%s: unknown vblank event received\n", __func__);
 		/* Unknown type */
 		break;
 	}
 
-	i830_dri2_del_frame_event(swap_info);
-	I830DRI2DestroyBuffer(drawable, swap_info->front);
-	I830DRI2DestroyBuffer(drawable, swap_info->back);
-	free(swap_info);
+	i830_dri2_del_frame_event(drawable, swap_info);
 }
 
 void I830DRI2FlipEventHandler(unsigned int frame, unsigned int tv_sec,
 			      unsigned int tv_usec, DRI2FrameEventPtr flip_info)
 {
+	struct intel_screen_private *intel = flip_info->intel;
 	DrawablePtr drawable;
-	ScreenPtr screen;
-	ScrnInfoPtr scrn;
-	int status;
+	DRI2FrameEventPtr chain;
 
-	if (!flip_info->drawable_id)
-		status = BadDrawable;
-	else
-		status = dixLookupDrawable(&drawable, flip_info->drawable_id, serverClient,
-					   M_ANY, DixWriteAccess);
-	if (status != Success) {
-		i830_dri2_del_frame_event(flip_info);
-		free(flip_info);
-		return;
-	}
+	drawable = NULL;
+	if (flip_info->drawable_id)
+		dixLookupDrawable(&drawable, flip_info->drawable_id, serverClient,
+				  M_ANY, DixWriteAccess);
 
-	screen = drawable->pScreen;
-	scrn = xf86Screens[screen->myNum];
 
 	/* We assume our flips arrive in order, so we don't check the frame */
 	switch (flip_info->type) {
 	case DRI2_SWAP:
+		if (!drawable)
+			break;
+
 		/* Check for too small vblank count of pageflip completion, taking wraparound
 		 * into account. This usually means some defective kms pageflip completion,
 		 * causing wrong (msc, ust) return values and possible visual corruption.
@@ -975,7 +989,7 @@ void I830DRI2FlipEventHandler(unsigned int frame, unsigned int tv_sec,
 			 * kernels, so make it quieter.
 			 */
 			if (limit) {
-				xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+				xf86DrvMsg(intel->scrn->scrnIndex, X_WARNING,
 					   "%s: Pageflip completion has impossible msc %d < target_msc %d\n",
 					   __func__, frame, flip_info->frame);
 				limit--;
@@ -988,16 +1002,52 @@ void I830DRI2FlipEventHandler(unsigned int frame, unsigned int tv_sec,
 		DRI2SwapComplete(flip_info->client, drawable, frame, tv_sec, tv_usec,
 				 DRI2_FLIP_COMPLETE, flip_info->client ? flip_info->event_complete : NULL,
 				 flip_info->event_data);
-	break;
+		break;
+
+	case DRI2_SWAP_CHAIN:
+		assert(intel->pending_flip[flip_info->pipe] == flip_info);
+		intel->pending_flip[flip_info->pipe] = NULL;
+
+		chain = flip_info->chain;
+		if (chain) {
+			DrawablePtr chain_drawable = NULL;
+			if (chain->drawable_id)
+				 dixLookupDrawable(&chain_drawable,
+						   chain->drawable_id,
+						   serverClient,
+						   M_ANY, DixWriteAccess);
+			if (chain_drawable == NULL) {
+				i830_dri2_del_frame_event(chain_drawable, chain);
+			} else if (!can_exchange(chain_drawable, chain->front, chain->back) ||
+				   !I830DRI2ScheduleFlip(intel, chain_drawable, chain)) {
+				BoxRec box;
+				RegionRec region;
+
+				box.x1 = 0;
+				box.y1 = 0;
+				box.x2 = chain_drawable->width;
+				box.y2 = chain_drawable->height;
+				REGION_INIT(pScreen, &region, &box, 0);
+
+				I830DRI2CopyRegion(chain_drawable, &region,
+						   chain->front, chain->back);
+				DRI2SwapComplete(chain->client, chain_drawable, frame, tv_sec, tv_usec,
+						 DRI2_BLIT_COMPLETE,
+						 chain->client ? chain->event_complete : NULL,
+						 chain->event_data);
+				i830_dri2_del_frame_event(chain_drawable, chain);
+			}
+		}
+		break;
+
 	default:
-		xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+		xf86DrvMsg(intel->scrn->scrnIndex, X_WARNING,
 			   "%s: unknown vblank event received\n", __func__);
 		/* Unknown type */
 		break;
 	}
 
-	i830_dri2_del_frame_event(flip_info);
-	free(flip_info);
+	i830_dri2_del_frame_event(drawable, flip_info);
 }
 
 /*
@@ -1050,12 +1100,14 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 	if (!swap_info)
 	    goto blit_fallback;
 
+	swap_info->intel = intel;
 	swap_info->drawable_id = draw->id;
 	swap_info->client = client;
 	swap_info->event_complete = func;
 	swap_info->event_data = data;
 	swap_info->front = front;
 	swap_info->back = back;
+	swap_info->pipe = I830DRI2DrawablePipe(draw);
 
 	if (!i830_dri2_add_frame_event(swap_info)) {
 	    free(swap_info);
@@ -1082,10 +1134,7 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 	current_msc = vbl.reply.sequence;
 
 	/* Flips need to be submitted one frame before */
-	if (intel->use_pageflipping &&
-	    !intel->shadow_present &&
-	    DRI2CanFlip(draw) &&
-	    can_exchange(front, back)) {
+	if (can_exchange(draw, front, back)) {
 	    swap_type = DRI2_FLIP;
 	    flip = 1;
 	}
@@ -1105,6 +1154,9 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 	 * the swap.
 	 */
 	if (divisor == 0 || current_msc < *target_msc) {
+		if (flip && I830DRI2ScheduleFlip(intel, draw, swap_info))
+			return TRUE;
+
 		vbl.request.type =  DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
 		if (pipe > 0)
 			vbl.request.type |= DRM_VBLANK_SECONDARY;
@@ -1197,12 +1249,8 @@ blit_fallback:
 	I830DRI2CopyRegion(draw, &region, front, back);
 
 	DRI2SwapComplete(client, draw, 0, 0, 0, DRI2_BLIT_COMPLETE, func, data);
-	if (swap_info) {
-	    i830_dri2_del_frame_event(swap_info);
-	    I830DRI2DestroyBuffer(draw, swap_info->front);
-	    I830DRI2DestroyBuffer(draw, swap_info->back);
-	    free(swap_info);
-	}
+	if (swap_info)
+	    i830_dri2_del_frame_event(draw, swap_info);
 	*target_msc = 0; /* offscreen, so zero out target vblank count */
 	return TRUE;
 }
@@ -1283,6 +1331,7 @@ I830DRI2ScheduleWaitMSC(ClientPtr client, DrawablePtr draw, CARD64 target_msc,
 	if (!wait_info)
 		goto out_complete;
 
+	wait_info->intel = intel;
 	wait_info->drawable_id = draw->id;
 	wait_info->client = client;
 	wait_info->type = DRI2_WAITMSC;
diff --git a/src/intel_driver.c b/src/intel_driver.c
index 3efc7f4..7fc1c1a 100644
--- a/src/intel_driver.c
+++ b/src/intel_driver.c
@@ -91,6 +91,7 @@ typedef enum {
    OPTION_TILING_2D,
    OPTION_SHADOW,
    OPTION_SWAPBUFFERS_WAIT,
+   OPTION_TRIPLE_BUFFER,
 #ifdef INTEL_XVMC
    OPTION_XVMC,
 #endif
@@ -111,6 +112,7 @@ static OptionInfoRec I830Options[] = {
    {OPTION_TILING_FB,	"LinearFramebuffer",	OPTV_BOOLEAN,	{0},	FALSE},
    {OPTION_SHADOW,	"Shadow",	OPTV_BOOLEAN,	{0},	FALSE},
    {OPTION_SWAPBUFFERS_WAIT, "SwapbuffersWait", OPTV_BOOLEAN,	{0},	TRUE},
+   {OPTION_TRIPLE_BUFFER, "TripleBuffer", OPTV_BOOLEAN,	{0},	TRUE},
 #ifdef INTEL_XVMC
    {OPTION_XVMC,	"XvMC",		OPTV_BOOLEAN,	{0},	TRUE},
 #endif
@@ -656,6 +658,15 @@ static Bool I830PreInit(ScrnInfoPtr scrn, int flags)
 	intel->swapbuffers_wait = xf86ReturnOptValBool(intel->Options,
 						       OPTION_SWAPBUFFERS_WAIT,
 						       TRUE);
+	xf86DrvMsg(scrn->scrnIndex, X_CONFIG, "Wait on SwapBuffers? %s\n",
+		   intel->swapbuffers_wait ? "enabled" : "disabled");
+
+	intel->use_triple_buffer =
+		xf86ReturnOptValBool(intel->Options,
+				     OPTION_TRIPLE_BUFFER,
+				     TRUE);
+	xf86DrvMsg(scrn->scrnIndex, X_CONFIG, "Triple buffering? %s\n",
+		   intel->use_triple_buffer ? "enabled" : "disabled");
 
 	xf86DrvMsg(scrn->scrnIndex, X_CONFIG, "Framebuffer %s\n",
 		   intel->tiling & INTEL_TILING_FB ? "tiled" : "linear");
@@ -1177,6 +1188,11 @@ static Bool I830CloseScreen(int scrnIndex, ScreenPtr screen)
 		intel->uxa_driver = NULL;
 	}
 
+	if (intel->back_buffer) {
+		drm_intel_bo_unreference(intel->back_buffer);
+		intel->back_buffer = NULL;
+	}
+
 	if (intel->front_buffer) {
 		if (!intel->use_shadow)
 			intel_set_pixmap_bo(screen->GetScreenPixmap(screen),
commit 2608a367acba7247e50754c3daeed09ba2e97d05
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Mon Jul 11 16:28:15 2011 +0100

    dri: Prevent abuse of the Resource database
    
    The Resource database is only designed to store a single value for a
    particular type associated with an XID. Due to the asynchronous nature
    of the vblank/flip requests, we would often associate multiple frame
    events with a particular drawable/client. Upon freeing the resource, we
    would not necessarily decouple the right value, leaving a stale pointer
    behind. Later when the client disappeared, we would write through that
    stale pointer upsetting valgrind and causing memory corruption. MDK.
    
    Instead, we need to implement an extra layer for tracking multiple
    frames within a single Resource.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37700
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/intel.h b/src/intel.h
index 2b114c3..9d64d30 100644
--- a/src/intel.h
+++ b/src/intel.h
@@ -476,11 +476,12 @@ typedef void (*DRI2SwapEventPtr)(ClientPtr client, void *data, int type,
 
 typedef struct _DRI2FrameEvent {
 	XID drawable_id;
-	XID client_id;	/* fake client ID to track client destruction */
 	ClientPtr client;
 	enum DRI2FrameEventType type;
 	int frame;
 
+	struct list drawable_resource, client_resource;
+
 	/* for swaps & flips only */
 	DRI2SwapEventPtr event_complete;
 	void *event_data;
diff --git a/src/intel_dri.c b/src/intel_dri.c
index a6be07f..38d7a6b 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -71,6 +71,8 @@ typedef struct {
 	PixmapPtr pixmap;
 } I830DRI2BufferPrivateRec, *I830DRI2BufferPrivatePtr;
 
+static DevPrivateKeyRec i830_client_key;
+
 static uint32_t pixmap_flink(PixmapPtr pixmap)
 {
 	struct intel_pixmap *priv = intel_get_pixmap_private(pixmap);
@@ -608,22 +610,73 @@ I830DRI2DrawablePipe(DrawablePtr pDraw)
 
 static RESTYPE	frame_event_client_type, frame_event_drawable_type;
 
+struct i830_dri2_resource {
+	XID id;
+	RESTYPE type;
+	struct list list;
+};
+
+static struct i830_dri2_resource *
+get_resource(XID id, RESTYPE type)
+{
+	struct i830_dri2_resource *resource;
+	void *ptr;
+
+	ptr = NULL;
+	dixLookupResourceByType(&ptr, id, type, NULL, DixWriteAccess);
+	if (ptr)
+		return ptr;
+
+	resource = malloc(sizeof(*resource));
+	if (resource == NULL)
+		return NULL;
+
+	if (!AddResource(id, type, resource)) {
+		free(resource);
+		return NULL;
+	}
+
+	resource->id = id;
+	resource->type = type;
+	list_init(&resource->list);
+	return resource;
+}
+
 static int
 i830_dri2_frame_event_client_gone(void *data, XID id)
 {
-	DRI2FrameEventPtr	frame_event = data;
+	struct i830_dri2_resource *resource = data;
+
+	while (!list_is_empty(&resource->list)) {
+		DRI2FrameEventPtr info =
+			list_first_entry(&resource->list,
+					 DRI2FrameEventRec,
+					 client_resource);
+
+		list_del(&info->client_resource);
+		info->client = NULL;
+	}
+	free(resource);
 
-	frame_event->client = NULL;
-	frame_event->client_id = None;
 	return Success;
 }
 
 static int
 i830_dri2_frame_event_drawable_gone(void *data, XID id)
 {
-	DRI2FrameEventPtr	frame_event = data;
+	struct i830_dri2_resource *resource = data;
+
+	while (!list_is_empty(&resource->list)) {
+		DRI2FrameEventPtr info =
+			list_first_entry(&resource->list,
+					 DRI2FrameEventRec,
+					 drawable_resource);
+
+		list_del(&info->drawable_resource);
+		info->drawable_id = None;
+	}
+	free(resource);
 
-	frame_event->drawable_id = None;
 	return Success;
 }
 
@@ -641,34 +694,48 @@ i830_dri2_register_frame_event_resource_types(void)
 	return TRUE;
 }
 
+static XID
+get_client_id(ClientPtr client)
+{
+	XID *ptr = dixGetPrivateAddr(&client->devPrivates, &i830_client_key);
+	if (*ptr == 0)
+		*ptr = FakeClientID(client->index);
+	return *ptr;
+}
+
 /*
  * Hook this frame event into the server resource
  * database so we can clean it up if the drawable or
  * client exits while the swap is pending
  */
 static Bool
-i830_dri2_add_frame_event(DRI2FrameEventPtr frame_event)
+i830_dri2_add_frame_event(DRI2FrameEventPtr info)
 {
-	frame_event->client_id = FakeClientID(frame_event->client->index);
+	struct i830_dri2_resource *resource;
 
-	if (!AddResource(frame_event->client_id, frame_event_client_type, frame_event))
+	resource = get_resource(get_client_id(info->client),
+				frame_event_client_type);
+	if (resource == NULL)
 		return FALSE;
 
-	if (!AddResource(frame_event->drawable_id, frame_event_drawable_type, frame_event)) {
-		FreeResourceByType(frame_event->client_id, frame_event_client_type, TRUE);
+	list_add(&info->client_resource, &resource->list);
+
+	resource = get_resource(info->drawable_id, frame_event_drawable_type);
+	if (resource == NULL) {
+		list_del(&info->client_resource);
 		return FALSE;
 	}
 
+	list_add(&info->drawable_resource, &resource->list);
+
 	return TRUE;
 }
 
 static void
-i830_dri2_del_frame_event(DRI2FrameEventPtr frame_event)
+i830_dri2_del_frame_event(DRI2FrameEventPtr info)
 {
-	if (frame_event->client_id)
-		FreeResourceByType(frame_event->client_id, frame_event_client_type, TRUE);
-	if (frame_event->drawable_id)
-		FreeResourceByType(frame_event->drawable_id, frame_event_drawable_type, TRUE);
+	list_del(&info->client_resource);
+	list_del(&info->drawable_resource);
 }
 
 static void
@@ -1350,6 +1417,10 @@ Bool I830DRI2ScreenInit(ScreenPtr screen)
 		return FALSE;
 	}
 
+	if (!dixRegisterPrivateKey(&i830_client_key, PRIVATE_CLIENT, sizeof(XID)))
+		return FALSE;
+
+
 #if DRI2INFOREC_VERSION >= 4
 	if (serverGeneration != dri2_server_generation) {
 	    dri2_server_generation = serverGeneration;
diff --git a/src/sna/sna_dri.c b/src/sna/sna_dri.c
index a6cc9ce..f5d81b3 100644
--- a/src/sna/sna_dri.c
+++ b/src/sna/sna_dri.c
@@ -93,16 +93,24 @@ struct sna_dri_private {
 	struct kgem_bo *bo;
 };
 
+struct sna_dri_resource {
+	XID id;
+	RESTYPE type;
+	struct list list;
+};
+
 struct sna_dri_frame_event {
 	struct sna *sna;
 	XID drawable_id;
-	XID client_id;	/* fake client ID to track client destruction */
 	ClientPtr client;
 	enum frame_event_type type;
 	int frame;
 	int pipe;
 	int count;
 
+	struct list drawable_resource;
+	struct list client_resource;
+
 	/* for swaps & flips only */
 	DRI2SwapEventPtr event_complete;
 	void *event_data;
@@ -120,6 +128,8 @@ struct sna_dri_frame_event {
 	struct sna_dri_frame_event *chain;
 };
 
+static DevPrivateKeyRec sna_client_key;
+
 static inline struct sna_dri_frame_event *
 to_frame_event(void *data)
 {
@@ -531,22 +541,67 @@ sna_dri_get_pipe(DrawablePtr pDraw)
 
 static RESTYPE frame_event_client_type, frame_event_drawable_type;
 
+static struct sna_dri_resource *
+get_resource(XID id, RESTYPE type)
+{
+	struct sna_dri_resource *resource;
+	void *ptr;
+
+	ptr = NULL;
+	dixLookupResourceByType(&ptr, id, type, NULL, DixWriteAccess);
+	if (ptr)
+		return ptr;
+
+	resource = malloc(sizeof(*resource));
+	if (resource == NULL)
+		return NULL;
+
+	if (!AddResource(id, type, resource)) {
+		free(resource);
+		return NULL;
+	}
+
+	resource->id = id;
+	resource->type = type;
+	list_init(&resource->list);
+	return resource;
+}
+
 static int
 sna_dri_frame_event_client_gone(void *data, XID id)
 {
-	struct sna_dri_frame_event *frame_event = data;
+	struct sna_dri_resource *resource = data;
+
+	while (!list_is_empty(&resource->list)) {
+		struct sna_dri_frame_event *info =
+			list_first_entry(&resource->list,
+					 struct sna_dri_frame_event,
+					 client_resource);
+
+		list_del(&info->client_resource);
+		info->client = NULL;
+	}
+	free(resource);
 
-	frame_event->client = NULL;
-	frame_event->client_id = None;
 	return Success;
 }
 
 static int
 sna_dri_frame_event_drawable_gone(void *data, XID id)
 {
-	struct sna_dri_frame_event *frame_event = data;
+	struct sna_dri_resource *resource = data;
+
+	while (!list_is_empty(&resource->list)) {
+		struct sna_dri_frame_event *info =
+			list_first_entry(&resource->list,
+					 struct sna_dri_frame_event,
+					 drawable_resource);
+
+		list_del(&info->drawable_resource);
+		info->drawable_id = None;
+	}
+	free(resource);
 
-	frame_event->drawable_id = None;
 	return Success;
 }
 
@@ -568,45 +623,48 @@ sna_dri_register_frame_event_resource_types(void)
 	return TRUE;
 }
 
+static XID
+get_client_id(ClientPtr client)
+{
+	XID *ptr = dixGetPrivateAddr(&client->devPrivates, &sna_client_key);
+	if (*ptr == 0)
+		*ptr = FakeClientID(client->index);
+	return *ptr;
+}
+
 /*
  * Hook this frame event into the server resource
  * database so we can clean it up if the drawable or
  * client exits while the swap is pending
  */
 static Bool
-sna_dri_add_frame_event(struct sna_dri_frame_event *frame_event)
+sna_dri_add_frame_event(struct sna_dri_frame_event *info)
 {
-	frame_event->client_id = FakeClientID(frame_event->client->index);
+	struct sna_dri_resource *resource;
 
-	if (!AddResource(frame_event->client_id,
-			 frame_event_client_type,
-			 frame_event))
+	resource = get_resource(get_client_id(info->client),
+				frame_event_client_type);
+	if (resource == NULL)
 		return FALSE;
 
-	if (!AddResource(frame_event->drawable_id,
-			 frame_event_drawable_type,
-			 frame_event)) {
-		FreeResourceByType(frame_event->client_id,
-				   frame_event_client_type,
-				   TRUE);
+	list_add(&info->client_resource, &resource->list);
+
+	resource = get_resource(info->drawable_id, frame_event_drawable_type);
+	if (resource == NULL) {
+		list_del(&info->client_resource);
 		return FALSE;
 	}
 
+	list_add(&info->drawable_resource, &resource->list);
+
 	return TRUE;
 }
 
 static void
 sna_dri_frame_event_info_free(struct sna_dri_frame_event *info)
 {
-	if (info->client_id)
-		FreeResourceByType(info->client_id,
-				   frame_event_client_type,
-				   TRUE);
-
-	if (info->drawable_id)
-		FreeResourceByType(info->drawable_id,
-				   frame_event_drawable_type,
-				   TRUE);
+	list_del(&info->client_resource);
+	list_del(&info->drawable_resource);
 
 	if (info->front)
 		_sna_dri_destroy_buffer(info->sna, info->front);
@@ -1606,6 +1664,10 @@ Bool sna_dri_open(struct sna *sna, ScreenPtr screen)
 		return FALSE;
 	    }
 	}
+
+	if (!dixRegisterPrivateKey(&sna_client_key, PRIVATE_CLIENT, sizeof(XID)))
+		return FALSE;
+
 	sna->deviceName = drmGetDeviceNameFromFd(sna->kgem.fd);
 	memset(&info, '\0', sizeof(info));
 	info.fd = sna->kgem.fd;
commit ab1000821ae881a301fb0e1f2210493ec383e681
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Sat Jul 9 19:44:26 2011 +0100

    dri: Remove the shadow copy of attachment
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/intel_dri.c b/src/intel_dri.c
index 4bcf928..a6be07f 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -69,7 +69,6 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
 typedef struct {
 	int refcnt;
 	PixmapPtr pixmap;
-	unsigned int attachment;
 } I830DRI2BufferPrivateRec, *I830DRI2BufferPrivatePtr;
 
 static uint32_t pixmap_flink(PixmapPtr pixmap)
@@ -264,7 +263,6 @@ I830DRI2CreateBuffers(DrawablePtr drawable, unsigned int *attachments,
 		buffers[i].flags = 0;	/* not tiled */
 		privates[i].refcnt = 1;
 		privates[i].pixmap = pixmap;
-		privates[i].attachment = attachments[i];
 
 		if ((buffers[i].name = pixmap_flink(pixmap)) == 0) {
 			/* failed to name buffer */
@@ -401,7 +399,6 @@ I830DRI2CreateBuffer(DrawablePtr drawable, unsigned int attachment,
 	buffer->flags = 0;	/* not tiled */
 	privates->refcnt = 1;
 	privates->pixmap = pixmap;
-	privates->attachment = attachment;
 
 	if ((buffer->name = pixmap_flink(pixmap)) == 0) {
 		/* failed to name buffer */
@@ -440,9 +437,9 @@ I830DRI2CopyRegion(DrawablePtr drawable, RegionPtr pRegion,
 	ScreenPtr screen = drawable->pScreen;
 	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
 	intel_screen_private *intel = intel_get_screen_private(scrn);
-	DrawablePtr src = (srcPrivate->attachment == DRI2BufferFrontLeft)
+	DrawablePtr src = (sourceBuffer->attachment == DRI2BufferFrontLeft)
 		? drawable : &srcPrivate->pixmap->drawable;
-	DrawablePtr dst = (dstPrivate->attachment == DRI2BufferFrontLeft)
+	DrawablePtr dst = (destBuffer->attachment == DRI2BufferFrontLeft)
 		? drawable : &dstPrivate->pixmap->drawable;
 	RegionPtr pCopyClip;
 	GCPtr gc;


More information about the xorg-commit mailing list