sna/dri: Accurately track front and pending front for async flips

By not tracking the front buffer correctly, i.e. performing the exchange
on every swap, GL_FRONT was no longer pointing at the updated buffer and
neither was the root pixmap. So both X and GL would read the wrong
buffer was the flip was pending.

The other issue was that we would feed the old front buffer back to the
application as a future back buffer (due to buffer caching) and so the
kernel would duly insert a WAIT_EVENT for the pending flip to complete
before allowing rendering to affect it.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

5 files changed, 109 insertions, 73 deletions

diff --git a/src/sna/gen4_render.c b/src/sna/gen4_render.c
index c7bc751f..da2bb764 100644
--- a/src/sna/gen4_render.c
+++ b/src/sna/gen4_render.c
@@ -1271,7 +1271,7 @@ gen4_emit_pipelined_pointers(struct sna *sna,
 	     __FUNCTION__, op->mask.bo != NULL,
 	     op->src.filter, op->src.repeat,
 	     op->mask.filter, op->mask.repeat,
-	     kernel, blend, op->has_component_alpha, op->dst.format));
+	     kernel, blend, op->has_component_alpha, (int)op->dst.format));
 
 	OUT_BATCH(GEN4_3DSTATE_PIPELINED_POINTERS | 5);
 	OUT_BATCH(sna->render_state.gen4.vs);
diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 8a8bca2a..1d5ed067 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -973,8 +973,11 @@ bool kgem_expire_cache(struct kgem *kgem)
 		}
 	}
 	if (!kgem->need_purge) {
-		if (idle)
+		if (idle) {
+			DBG(("%s: idle\n", __FUNCTION__));
+			kgem->need_expire = false;
 			return false;
+		}
 		if (expire == 0)
 			return true;
 	}
@@ -985,12 +988,11 @@ bool kgem_expire_cache(struct kgem *kgem)
 			bo = list_last_entry(&kgem->inactive[i],
 					     struct kgem_bo, list);
 
-			if (!gem_madvise(kgem->fd, bo->handle,
-					 I915_MADV_DONTNEED)) {
-				if (bo->delta > expire) {
-					idle = false;
-					break;
-				}
+			if (gem_madvise(kgem->fd, bo->handle,
+					I915_MADV_DONTNEED) &&
+			    bo->delta > expire) {
+				idle = false;
+				break;
 			}
 
 			count++;
@@ -1002,11 +1004,12 @@ bool kgem_expire_cache(struct kgem *kgem)
 		}
 	}
 
-	DBG(("%s: purge? %d -- expired %d objects, %d bytes\n", __FUNCTION__, kgem->need_purge,  count, size));
+	DBG(("%s: purge? %d -- expired %d objects, %d bytes, idle? %d\n",
+	     __FUNCTION__, kgem->need_purge,  count, size, idle));
 
-	kgem->need_expire = !idle;
 	kgem->need_purge = false;
-	return idle;
+	kgem->need_expire = !idle;
+	return !idle;
 	(void)count;
 	(void)size;
 }
diff --git a/src/sna/sna.h b/src/sna/sna.h
index ae8e5548..3a380271 100644
--- a/src/sna/sna.h
+++ b/src/sna/sna.h
@@ -213,7 +213,7 @@ struct sna {
 	} mode;
 
 	struct sna_dri {
-		int flip_pending[2];
+		void *flip_pending[2];
 	} dri;
 
 	unsigned int tiling;
@@ -271,7 +271,6 @@ extern int sna_do_pageflip(struct sna *sna,
 			    PixmapPtr pixmap,
 			    void *data,
 			    int ref_crtc_hw_id,
-			    PixmapPtr *old_front,
 			    uint32_t *old_fb);
 
 extern PixmapPtr sna_set_screen_pixmap(struct sna *sna, PixmapPtr pixmap);
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index c2347ef3..0e1a1225 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -1528,10 +1528,12 @@ PixmapPtr sna_set_screen_pixmap(struct sna *sna, PixmapPtr pixmap)
 	PixmapPtr old = sna->front;
 	ScrnInfoPtr scrn = sna->scrn;
 
+	assert(sna->front != pixmap);
+
 	sna->front = pixmap;
 	pixmap->refcnt++;
 
-	sna_redirect_screen_pixmap(scrn, old, sna->front);
+	sna_redirect_screen_pixmap(scrn, old, pixmap);
 	scrn->displayWidth = sna_pixmap_get_bo(pixmap)->pitch / sna->mode.cpp;
 
 	return old;
@@ -1542,7 +1544,6 @@ sna_do_pageflip(struct sna *sna,
 		PixmapPtr pixmap,
 		void *data,
 		int ref_crtc_hw_id,
-		PixmapPtr *old_front,
 		uint32_t *old_fb)
 {
 	ScrnInfoPtr scrn = sna->scrn;
@@ -1550,14 +1551,11 @@ sna_do_pageflip(struct sna *sna,
 	struct kgem_bo *bo;
 	int count;
 
-	assert(pixmap != sna->front);
-
 	bo = sna_pixmap_pin(pixmap);
 	if (!bo)
 		return 0;
 
 	*old_fb = mode->fb_id;
-	*old_front = sna->front;
 
 	/*
 	 * Create a new handle for the back buffer
@@ -1577,8 +1575,6 @@ sna_do_pageflip(struct sna *sna,
 	DBG(("%s: handle %d attached to fb %d\n",
 	     __FUNCTION__, bo->handle, mode->fb_id));
 
-	if (kgem_bo_is_dirty(bo))
-		kgem_emit_flush(&sna->kgem);
 	kgem_submit(&sna->kgem);
 
 	/*
@@ -1592,12 +1588,10 @@ sna_do_pageflip(struct sna *sna,
 	 */
 	count = do_page_flip(sna, data, ref_crtc_hw_id);
 	DBG(("%s: page flipped %d crtcs\n", __FUNCTION__, count));
-	if (count > 0) {
-		sna->front = pixmap;
-		pixmap->refcnt++;
-
-		sna_redirect_screen_pixmap(scrn, *old_front, sna->front);
-		scrn->displayWidth = bo->pitch / sna->mode.cpp;
+	if (count) {
+		bo->cpu_read = bo->cpu_write = false;
+		bo->gpu = true;
+		bo->needs_flush = true;
 	} else {
 		drmModeRmFB(sna->kgem.fd, mode->fb_id);
 		mode->fb_id = *old_fb;
diff --git a/src/sna/sna_dri.c b/src/sna/sna_dri.c
index 502d05b3..74a7145c 100644
--- a/src/sna/sna_dri.c
+++ b/src/sna/sna_dri.c
@@ -112,15 +112,23 @@ struct sna_dri_frame_event {
 	unsigned int fe_tv_usec;
 
 	PixmapPtr old_front;
+	PixmapPtr next_front;
 	uint32_t old_fb;
 };
 
-static struct sna_dri_frame_event *
+static inline struct sna_dri_frame_event *
 to_frame_event(void *data)
 {
 	 return (struct sna_dri_frame_event *)((uintptr_t)data & ~1);
 }
 
+static inline PixmapPtr
+get_pixmap(DRI2Buffer2Ptr buffer)
+{
+	struct sna_dri_private *priv = buffer->driverPrivate;
+	return priv->pixmap;
+}
+
 static struct kgem_bo *sna_pixmap_set_dri(struct sna *sna,
 					  PixmapPtr pixmap)
 {
@@ -176,9 +184,10 @@ sna_dri_create_buffer(DrawablePtr drawable,
 		pixmap->refcnt++;
 		bo = sna_pixmap_set_dri(sna, pixmap);
 		bpp = pixmap->drawable.bitsPerPixel;
-		DBG(("%s: attaching to front buffer %dx%d\n",
+		DBG(("%s: attaching to front buffer %dx%d [%p:%d]\n",
 		     __FUNCTION__,
-		     pixmap->drawable.width, pixmap->drawable.height));
+		     pixmap->drawable.width, pixmap->drawable.height,
+		     pixmap, pixmap->refcnt));
 		break;
 
 	case DRI2BufferBackLeft:
@@ -626,8 +635,6 @@ sna_dri_exchange_buffers(DrawablePtr draw,
 	DBG(("%s(%d <--> %d)\n",
 	     __FUNCTION__, front->attachment, back->attachment));
 
-	assert(front->format == back->format);
-
 	tmp = front->attachment;
 	front->attachment = back->attachment;
 	back->attachment = tmp;
@@ -651,7 +658,6 @@ sna_dri_schedule_flip(struct sna *sna,
 	info->count = sna_do_pageflip(sna,
 				      back_priv->pixmap,
 				      info, info->pipe,
-				      &info->old_front,
 				      &info->old_fb);
 	return info->count != 0;
 }
@@ -663,9 +669,10 @@ can_flip(struct sna * sna,
 	 DRI2BufferPtr back)
 {
 	struct sna_dri_private *back_priv = back->driverPrivate;
+	struct sna_dri_private *front_priv = front->driverPrivate;
 	struct sna_pixmap *front_sna, *back_sna;
 	WindowPtr win = (WindowPtr)draw;
-	PixmapPtr front_pixmap;
+	PixmapPtr front_pixmap = front_priv->pixmap;
 	PixmapPtr back_pixmap = back_priv->pixmap;
 
 	ScreenPtr screen = draw->pScreen;
@@ -692,13 +699,17 @@ can_flip(struct sna * sna,
 		return FALSE;
 	}
 
-	front_pixmap = screen->GetWindowPixmap(win);
 	if (front_pixmap != sna->front) {
 		DBG(("%s: no, window is not on the front buffer\n",
 		     __FUNCTION__));
 		return FALSE;
 	}
 
+	DBG(("%s: window size: %dx%d, clip=(%d, %d), (%d, %d)\n",
+	     __FUNCTION__,
+	     win->drawable.width, win->drawable.height,
+	     win->clipList.extents.x1, win->clipList.extents.y1,
+	     win->clipList.extents.x2, win->clipList.extents.y2));
 	if (!RegionEqual(&win->clipList, &screen->root->winSize)) {
 		DBG(("%s: no, window is clipped: clip region=(%d, %d), (%d, %d), root size=(%d, %d), (%d, %d)\n",
 		     __FUNCTION__,
@@ -889,12 +900,40 @@ static void sna_dri_flip_event(struct sna *sna,
 				 DRI2_FLIP_COMPLETE,
 				 flip->client ? flip->event_complete : NULL,
 				 flip->event_data);
+
+		sna_mode_delete_fb(flip->sna, flip->old_front, flip->old_fb);
+		sna_dri_frame_event_info_free(flip);
 		break;
 
 	case DRI2_ASYNC_FLIP:
-		DBG(("%s: async swap flip completed on pipe %d, pending %d\n",
-		     __FUNCTION__, flip->pipe, sna->dri.flip_pending[flip->pipe]));
-		sna->dri.flip_pending[flip->pipe]--;
+		DBG(("%s: async swap flip completed on pipe %d, pending? %d, new? %d\n",
+		     __FUNCTION__, flip->pipe,
+		     sna->dri.flip_pending[flip->pipe] != NULL,
+		     flip->next_front != sna->front));
+		assert(sna->dri.flip_pending[flip->pipe] == flip);
+
+		sna_mode_delete_fb(flip->sna, flip->old_front, flip->old_fb);
+
+		if (sna->front != flip->next_front) {
+			PixmapPtr next = sna->front;
+
+			DBG(("%s: async flip continuing\n", __FUNCTION__));
+			flip->count = sna_do_pageflip(sna, next,
+						      flip, flip->pipe,
+						      &flip->old_fb);
+			if (flip->count) {
+				flip->old_front = flip->next_front;
+				flip->next_front = next;
+				flip->next_front->refcnt++;
+			} else
+				goto finish_async_flip;
+		} else {
+finish_async_flip:
+			DBG(("%s: async flip completed\n", __FUNCTION__));
+			flip->next_front->drawable.pScreen->DestroyPixmap(flip->next_front);
+			sna->dri.flip_pending[flip->pipe] = NULL;
+			sna_dri_frame_event_info_free(flip);
+		}
 		break;
 
 	default:
@@ -925,9 +964,6 @@ sna_dri_page_flip_handler(int fd, unsigned int frame, unsigned int tv_sec,
 		return;
 
 	sna_dri_flip_event(info->sna, info);
-
-	sna_mode_delete_fb(info->sna, info->old_front, info->old_fb);
-	sna_dri_frame_event_info_free(info);
 }
 
 /*
@@ -987,8 +1023,8 @@ sna_dri_schedule_swap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 			goto blit_fallback;
 
 		pixmap = sna_set_screen_pixmap(sna, back_priv->pixmap);
+		assert(pixmap->refcnt > 1);
 		pixmap->refcnt--;
-		assert(pixmap->refcnt > 0);
 		sna_dri_exchange_buffers(draw, front, back);
 		DRI2SwapComplete(client, draw, 0, 0, 0,
 				 DRI2_EXCHANGE_COMPLETE, func, data);
@@ -1063,6 +1099,8 @@ sna_dri_schedule_swap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 immediate:
 		info->frame = *target_msc;
 		if (flip && sna_dri_schedule_flip(sna, draw, info)) {
+			info->old_front =
+				sna_set_screen_pixmap(sna, get_pixmap(back));
 			sna_dri_exchange_buffers(draw, front, back);
 			return TRUE;
 		}
@@ -1160,11 +1198,11 @@ sna_dri_async_swap(ClientPtr client, DrawablePtr draw,
 		   DRI2SwapEventPtr func, void *data)
 {
 	ScreenPtr screen = draw->pScreen;
-	ScrnInfoPtr scrn = xf86Screens[screen->myNum];
-	struct sna *sna = to_sna(scrn);
+	struct sna *sna = to_sna_from_screen(screen);
 	int type = DRI2_EXCHANGE_COMPLETE;
 	struct sna_dri_private *back_priv = back->driverPrivate;
 	struct sna_dri_private *front_priv = front->driverPrivate;
+	struct sna_dri_frame_event *info;
 	PixmapPtr pixmap;
 	int pipe;
 
@@ -1207,43 +1245,30 @@ sna_dri_async_swap(ClientPtr client, DrawablePtr draw,
 		return;
 	}
 
+	assert(front_priv->pixmap == sna->front);
+
 	pipe = sna_dri_get_pipe(draw);
 	if (pipe == -1) {
 		/* Drawable not displayed... just complete the swap */
-		pixmap = sna_set_screen_pixmap(sna, back_priv->pixmap);
-		pixmap->refcnt--;
-		assert(pixmap->refcnt > 0);
 		goto exchange;
 	}
 
-	DBG(("%s: flip pending on pipe %d? %d\n",
-	     __FUNCTION__, pipe, sna->dri.flip_pending[pipe]));
-	if (!sna->dri.flip_pending[pipe]) {
-		struct sna_dri_frame_event *info;
-		DRI2BufferPtr t;
-
+	info = sna->dri.flip_pending[pipe];
+	if (info == NULL) {
 		DBG(("%s: no pending flip on pipe %d, so updating scanout\n",
 		     __FUNCTION__, pipe));
 
 		info = calloc(1, sizeof(struct sna_dri_frame_event));
-		if (!info) {
+		if (!info)
 			goto exchange;
-		}
 
 		info->sna = sna;
-		info->drawable_id = draw->id;
-		info->client = client;
 		info->type = DRI2_ASYNC_FLIP;
 		info->pipe = pipe;
 
-		if (!sna_dri_add_frame_event(info)) {
-			free(info);
-			goto exchange;
-		}
-
 		info->count = sna_do_pageflip(sna, back_priv->pixmap,
 					      info, pipe,
-					      &info->old_front, &info->old_fb);
+					      &info->old_fb);
 
 		if (info->count == 0) {
 			DBG(("%s: pageflip failed\n", __FUNCTION__));
@@ -1251,26 +1276,39 @@ sna_dri_async_swap(ClientPtr client, DrawablePtr draw,
 			goto exchange;
 		}
 
-		type = DRI2_FLIP_COMPLETE;
-		sna->dri.flip_pending[pipe]++;
+		info->old_front = sna->front;
+		info->old_front->refcnt++;
 
-		/* and flip the pointers */
-		t = front;
-		front = back;
-		back = t;
+		info->next_front = back_priv->pixmap;
+		info->next_front->refcnt++;
 
-		front_priv = front->driverPrivate;
-		back_priv = back->driverPrivate;
+		type = DRI2_FLIP_COMPLETE;
+		sna->dri.flip_pending[pipe] = info;
+
+		if ((pixmap = screen->CreatePixmap(screen,
+						   draw->width,
+						   draw->height,
+						   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;
+		}
 	}
 
-	if (front_priv->pixmap == sna->front &&
+	if (front_priv->pixmap == info->next_front &&
 	    (pixmap = screen->CreatePixmap(screen,
 					   draw->width,
 					   draw->height,
 					   draw->depth,
 					   SNA_CREATE_FB))) {
 		DBG(("%s: new back buffer\n", __FUNCTION__));
-		screen->DestroyPixmap(front_priv->pixmap);
+		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);
@@ -1278,8 +1316,10 @@ sna_dri_async_swap(ClientPtr client, DrawablePtr draw,
 	}
 
 exchange:
+	pixmap = sna_set_screen_pixmap(sna, back_priv->pixmap);
+	screen->DestroyPixmap(pixmap);
+
 	sna_dri_exchange_buffers(draw, front, back);
-	assert(((struct sna_dri_private *)front->driverPrivate)->pixmap != sna->front);
 	DRI2SwapComplete(client, draw, 0, 0, 0, type, func, data);
 }
 #endif