sna: Auto-retire upload proxies

This was disabled in

commit 9f4f855ba37966fb91d31e9081d03cf72affb154
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon May 26 07:06:18 2014 +0100

    sna: Implicit release of upload buffers considered bad

as retiring the buffers during the command setup could free one of the
earlier bo used in the command. But discarding the snooped bo could
still be advantageous. So restore the automatic discard of upload
proxies, but make sure we only do between operations.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matti Hämäläinen <ccr@tnsp.org>

4 files changed, 58 insertions, 21 deletions

diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index a45dedda..69e9d4ca 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -2313,10 +2313,27 @@ static void kgem_bo_unref(struct kgem *kgem, struct kgem_bo *bo)
 		__kgem_bo_destroy(kgem, bo);
 }
 
-static bool kgem_retire__buffers(struct kgem *kgem)
+static void kgem_buffer_release(struct kgem *kgem, struct kgem_buffer *bo)
 {
-	bool retired = false;
+	assert(bo->base.io);
+	while (!list_is_empty(&bo->base.vma)) {
+		struct kgem_bo *cached;
+
+		cached = list_first_entry(&bo->base.vma, struct kgem_bo, vma);
+		assert(cached->proxy == &bo->base);
+		assert(cached != &bo->base);
+		list_del(&cached->vma);
 
+		assert(*(struct kgem_bo **)cached->map__gtt == cached);
+		*(struct kgem_bo **)cached->map__gtt = NULL;
+		cached->map__gtt = NULL;
+
+		kgem_bo_destroy(kgem, cached);
+	}
+}
+
+void kgem_retire__buffers(struct kgem *kgem)
+{
 	while (!list_is_empty(&kgem->active_buffers)) {
 		struct kgem_buffer *bo =
 			list_last_entry(&kgem->active_buffers,
@@ -2333,11 +2350,9 @@ static bool kgem_retire__buffers(struct kgem *kgem)
 		DBG(("%s: releasing upload cache for handle=%d? %d\n",
 		     __FUNCTION__, bo->base.handle, !list_is_empty(&bo->base.vma)));
 		list_del(&bo->base.list);
+		kgem_buffer_release(kgem, bo);
 		kgem_bo_unref(kgem, &bo->base);
-		retired = true;
 	}
-
-	return retired;
 }
 
 static bool kgem_retire__flushing(struct kgem *kgem)
@@ -2495,7 +2510,6 @@ bool kgem_retire(struct kgem *kgem)
 
 	retired |= kgem_retire__flushing(kgem);
 	retired |= kgem_retire__requests(kgem);
-	retired |= kgem_retire__buffers(kgem);
 
 	DBG(("%s -- retired=%d, need_retire=%d\n",
 	     __FUNCTION__, retired, kgem->need_retire));
@@ -2525,7 +2539,6 @@ bool __kgem_ring_is_idle(struct kgem *kgem, int ring)
 	     __FUNCTION__, ring, rq->bo->handle));
 
 	kgem_retire__requests_ring(kgem, ring);
-	kgem_retire__buffers(kgem);
 
 	assert(list_is_empty(&kgem->requests[ring]));
 	return true;
@@ -6760,6 +6773,17 @@ struct kgem_bo *kgem_upload_source_image(struct kgem *kgem,
 	return bo;
 }
 
+void kgem_proxy_bo_attach(struct kgem_bo *bo,
+			  struct kgem_bo **ptr)
+{
+	DBG(("%s: handle=%d\n", __FUNCTION__, bo->handle));
+	assert(bo->map__gtt == NULL);
+	assert(bo->proxy);
+	list_add(&bo->vma, &bo->proxy->vma);
+	bo->map__gtt = ptr;
+	*ptr = kgem_bo_reference(bo);
+}
+
 void kgem_buffer_read_sync(struct kgem *kgem, struct kgem_bo *_bo)
 {
 	struct kgem_buffer *bo;
diff --git a/src/sna/kgem.h b/src/sna/kgem.h
index 370894be..a52287d1 100644
--- a/src/sna/kgem.h
+++ b/src/sna/kgem.h
@@ -260,6 +260,7 @@ struct kgem_bo *kgem_upload_source_image(struct kgem *kgem,
 					 const void *data,
 					 const BoxRec *box,
 					 int stride, int bpp);
+void kgem_proxy_bo_attach(struct kgem_bo *bo, struct kgem_bo **ptr);
 
 int kgem_choose_tiling(struct kgem *kgem,
 		       int tiling, int width, int height, int bpp);
@@ -316,6 +317,21 @@ uint32_t kgem_bo_get_binding(struct kgem_bo *bo, uint32_t format);
 void kgem_bo_set_binding(struct kgem_bo *bo, uint32_t format, uint16_t offset);
 
 bool kgem_retire(struct kgem *kgem);
+void kgem_retire__buffers(struct kgem *kgem);
+
+static inline bool kgem_bo_discard_cache(struct kgem_bo *bo, bool force)
+{
+	if (bo == NULL || bo->proxy == NULL)
+		return false;
+
+	if (force)
+		return true;
+
+	if (bo->proxy->rq)
+		return false;
+
+	return bo->snoop;
+}
 
 bool __kgem_ring_is_idle(struct kgem *kgem, int ring);
 static inline bool kgem_ring_is_idle(struct kgem *kgem, int ring)
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 47a346c1..e9c1ef23 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -2081,8 +2081,7 @@ _sna_pixmap_move_to_cpu(PixmapPtr pixmap, unsigned int flags)
 			sna_pixmap_discard_shadow_damage(priv, NULL);
 	}
 
-	if ((priv->gpu_bo && priv->gpu_bo->proxy) &&
-	    (flags & MOVE_WRITE || priv->gpu_bo->proxy->rq == NULL)) {
+	if (kgem_bo_discard_cache(priv->gpu_bo, flags & MOVE_WRITE)) {
 		DBG(("%s: discarding cached upload buffer\n", __FUNCTION__));
 		assert(DAMAGE_IS_ALL(priv->cpu_damage));
 		assert(!priv->pinned);
@@ -2492,8 +2491,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 
 	assert(priv->gpu_damage == NULL || priv->gpu_bo);
 
-	if ((priv->gpu_bo && priv->gpu_bo->proxy) &&
-	    (flags & MOVE_WRITE || priv->gpu_bo->proxy->rq == NULL)) {
+	if (kgem_bo_discard_cache(priv->gpu_bo, flags & MOVE_WRITE)) {
 		DBG(("%s: discarding cached upload buffer\n", __FUNCTION__));
 		assert(DAMAGE_IS_ALL(priv->cpu_damage));
 		assert(priv->gpu_damage == NULL);
@@ -3206,8 +3204,7 @@ sna_pixmap_move_area_to_gpu(PixmapPtr pixmap, const BoxRec *box, unsigned int fl
 		goto done;
 	}
 
-	if ((priv->gpu_bo && priv->gpu_bo->proxy) &&
-	    (flags & (MOVE_WRITE | __MOVE_FORCE) || priv->gpu_bo->proxy->rq == NULL)) {
+	if (kgem_bo_discard_cache(priv->gpu_bo, flags & (MOVE_WRITE | __MOVE_FORCE))) {
 		DBG(("%s: discarding cached upload buffer\n", __FUNCTION__));
 		assert(priv->gpu_damage == NULL);
 		assert(!priv->pinned);
@@ -3447,7 +3444,7 @@ sna_drawable_use_bo(DrawablePtr drawable, unsigned flags, const BoxRec *box,
 			sna_damage_destroy(&priv->gpu_damage);
 	}
 
-	if (priv->gpu_bo && priv->gpu_bo->proxy) {
+	if (kgem_bo_discard_cache(priv->gpu_bo, true)) {
 		DBG(("%s: cached upload proxy, discard and revert to GPU\n",
 		     __FUNCTION__));
 		assert(priv->gpu_damage == NULL);
@@ -3943,8 +3940,7 @@ sna_pixmap_move_to_gpu(PixmapPtr pixmap, unsigned flags)
 		goto active;
 	}
 
-	if ((priv->gpu_bo && priv->gpu_bo->proxy) &&
-	    (flags & (MOVE_WRITE | __MOVE_FORCE) || priv->gpu_bo->proxy->rq == NULL)) {
+	if (kgem_bo_discard_cache(priv->gpu_bo, flags & (MOVE_WRITE | __MOVE_FORCE))) {
 		DBG(("%s: discarding cached upload buffer\n", __FUNCTION__));
 		assert(priv->gpu_damage == NULL);
 		assert(!priv->pinned);
@@ -4526,7 +4522,7 @@ try_upload_tiled_x(PixmapPtr pixmap, RegionRec *region,
 	     priv->gpu_bo ? kgem_bo_can_map__cpu(&sna->kgem, priv->gpu_bo, true) : 0,
 	     replaces));
 
-	if (priv->gpu_bo && priv->gpu_bo->proxy) {
+	if (kgem_bo_discard_cache(priv->gpu_bo, true)) {
 		kgem_bo_destroy(&sna->kgem, priv->gpu_bo);
 		priv->gpu_bo = NULL;
 	}
@@ -5876,7 +5872,7 @@ upload_inplace:
 static void discard_cpu_damage(struct sna *sna, struct sna_pixmap *priv)
 {
 	DBG(("%s: discarding existing CPU damage\n", __FUNCTION__));
-	if (priv->gpu_bo && priv->gpu_bo->proxy) {
+	if (kgem_bo_discard_cache(priv->gpu_bo, true)) {
 		assert(priv->gpu_damage == NULL);
 		assert(!priv->pinned);
 		assert(!priv->mapped);
@@ -6283,7 +6279,7 @@ sna_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 				     __FUNCTION__));
 				assert(src_priv->gpu_damage == NULL);
 				assert(src_priv->gpu_bo == NULL);
-				src_priv->gpu_bo = kgem_bo_reference(src_bo);
+				kgem_proxy_bo_attach(src_bo, &src_priv->gpu_bo);
 			}
 
 			if (!sna->render.copy_boxes(sna, alu,
@@ -17334,6 +17330,7 @@ void sna_accel_block_handler(struct sna *sna, struct timeval **tv)
 
 	if (sna->kgem.need_retire)
 		kgem_retire(&sna->kgem);
+	kgem_retire__buffers(&sna->kgem);
 
 	if (sna->timer_active)
 		UpdateCurrentTimeIf();
diff --git a/src/sna/sna_render.c b/src/sna/sna_render.c
index b03134d6..e58b6e1d 100644
--- a/src/sna/sna_render.c
+++ b/src/sna/sna_render.c
@@ -565,7 +565,7 @@ static struct kgem_bo *upload(struct sna *sna,
 			assert(priv->gpu_damage == NULL);
 			assert(priv->gpu_bo == NULL);
 			assert(bo->proxy != NULL);
-			priv->gpu_bo = kgem_bo_reference(bo);
+			kgem_proxy_bo_attach(bo, &priv->gpu_bo);
 		}
 	}
 
@@ -1271,7 +1271,7 @@ sna_render_picture_extract(struct sna *sna,
 			assert(priv->gpu_damage == NULL);
 			assert(priv->gpu_bo == NULL);
 			assert(bo->proxy != NULL);
-			priv->gpu_bo = kgem_bo_reference(bo);
+			kgem_proxy_bo_attach(bo, &priv->gpu_bo);
 		}
 	}