xf86-video-intel: 2 commits - src/sna/gen3_render.c src/sna/gen4_render.c src/sna/gen5_render.c src/sna/gen6_render.c src/sna/gen7_render.c src/sna/kgem.c src/sna/kgem.h src/sna/sna_accel.c src/sna/sna_display.c src/sna/sna_render.h

Chris Wilson ickle at kemper.freedesktop.org
Thu Jan 31 15:14:47 PST 2013


 src/sna/gen3_render.c |   21 ++---
 src/sna/gen4_render.c |   23 +++---
 src/sna/gen5_render.c |   23 +++---
 src/sna/gen6_render.c |   23 +++---
 src/sna/gen7_render.c |   23 +++---
 src/sna/kgem.c        |  180 +++++++++++++++++---------------------------------
 src/sna/kgem.h        |   35 ++++++++-
 src/sna/sna_accel.c   |    5 -
 src/sna/sna_display.c |    2 
 src/sna/sna_render.h  |    4 -
 10 files changed, 156 insertions(+), 183 deletions(-)

New commits:
commit 268285d9a64fc47fe81fe5bfbfbd1890dad53e1e
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Jan 31 21:57:41 2013 +0000

    sna/gen3+: Flush vertex threads before touching global state
    
    We need to be careful not just when finishing the current vbo to
    synchronize with the sharing threads, but also before we emit the batch
    state that no other thread will try and do the same.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/gen3_render.c b/src/sna/gen3_render.c
index 06c4e78..6b9a2f8 100644
--- a/src/sna/gen3_render.c
+++ b/src/sna/gen3_render.c
@@ -1900,6 +1900,9 @@ static bool gen3_rectangle_begin(struct sna *sna,
 	struct gen3_render_state *state = &sna->render_state.gen3;
 	int ndwords, i1_cmd = 0, i1_len = 0;
 
+	if (sna_vertex_wait__locked(&sna->render) && sna->render.vertex_offset)
+		return true;
+
 	ndwords = 2;
 	if (op->need_magic_ca_pass)
 		ndwords += 100;
@@ -1939,6 +1942,13 @@ static bool gen3_rectangle_begin(struct sna *sna,
 static int gen3_get_rectangles__flush(struct sna *sna,
 				      const struct sna_composite_op *op)
 {
+	/* Preventing discarding new vbo after lock contention */
+	if (sna_vertex_wait__locked(&sna->render)) {
+		int rem = vertex_space(sna);
+		if (rem > op->floats_per_rect)
+			return rem;
+	}
+
 	if (!kgem_check_batch(&sna->kgem, op->need_magic_ca_pass ? 105: 5))
 		return 0;
 	if (!kgem_check_reloc_and_exec(&sna->kgem, 1))
@@ -1955,17 +1965,6 @@ static int gen3_get_rectangles__flush(struct sna *sna,
 		}
 	}
 
-	/* Preventing discarding new vbo after lock contention */
-	if (sna->render.active) {
-		int rem;
-
-		sna_vertex_wait__locked(&sna->render);
-
-		rem = vertex_space(sna);
-		if (rem > op->floats_per_rect)
-			return rem;
-	}
-
 	return gen3_vertex_finish(sna);
 }
 
diff --git a/src/sna/gen4_render.c b/src/sna/gen4_render.c
index 829f4e9..7fc8520 100644
--- a/src/sna/gen4_render.c
+++ b/src/sna/gen4_render.c
@@ -587,6 +587,9 @@ static bool gen4_rectangle_begin(struct sna *sna,
 	int id = op->u.gen4.ve_id;
 	int ndwords;
 
+	if (sna_vertex_wait__locked(&sna->render) && sna->render.vertex_offset)
+		return true;
+
 	/* 7xpipelined pointers + 6xprimitive + 1xflush */
 	ndwords = op->need_magic_ca_pass? 20 : 6;
 	if ((sna->render.vb_id & (1 << id)) == 0)
@@ -606,6 +609,13 @@ static bool gen4_rectangle_begin(struct sna *sna,
 static int gen4_get_rectangles__flush(struct sna *sna,
 				      const struct sna_composite_op *op)
 {
+	/* Preventing discarding new vbo after lock contention */
+	if (sna_vertex_wait__locked(&sna->render)) {
+		int rem = vertex_space(sna);
+		if (rem > op->floats_per_rect)
+			return rem;
+	}
+
 	if (!kgem_check_batch(&sna->kgem, op->need_magic_ca_pass ? 25 : 6))
 		return 0;
 	if (!kgem_check_reloc_and_exec(&sna->kgem, 2))
@@ -621,17 +631,6 @@ static int gen4_get_rectangles__flush(struct sna *sna,
 						     op->u.gen4.wm_kernel);
 	}
 
-	/* Preventing discarding new vbo after lock contention */
-	if (sna->render.active) {
-		int rem;
-
-		sna_vertex_wait__locked(&sna->render);
-
-		rem = vertex_space(sna);
-		if (rem > op->floats_per_rect)
-			return rem;
-	}
-
 	return gen4_vertex_finish(sna);
 }
 
@@ -644,7 +643,7 @@ inline static int gen4_get_rectangles(struct sna *sna,
 
 start:
 	rem = vertex_space(sna);
-	if (rem < op->floats_per_rect) {
+	if (unlikely(rem < op->floats_per_rect)) {
 		DBG(("flushing vbo for %s: %d < %d\n",
 		     __FUNCTION__, rem, op->floats_per_rect));
 		rem = gen4_get_rectangles__flush(sna, op);
diff --git a/src/sna/gen5_render.c b/src/sna/gen5_render.c
index 07e9571..35f70f8 100644
--- a/src/sna/gen5_render.c
+++ b/src/sna/gen5_render.c
@@ -577,6 +577,9 @@ static bool gen5_rectangle_begin(struct sna *sna,
 	int id = op->u.gen5.ve_id;
 	int ndwords;
 
+	if (sna_vertex_wait__locked(&sna->render) && sna->render.vertex_offset)
+		return true;
+
 	ndwords = op->need_magic_ca_pass ? 20 : 6;
 	if ((sna->render.vb_id & (1 << id)) == 0)
 		ndwords += 5;
@@ -595,6 +598,13 @@ static bool gen5_rectangle_begin(struct sna *sna,
 static int gen5_get_rectangles__flush(struct sna *sna,
 				      const struct sna_composite_op *op)
 {
+	/* Preventing discarding new vbo after lock contention */
+	if (sna_vertex_wait__locked(&sna->render)) {
+		int rem = vertex_space(sna);
+		if (rem > op->floats_per_rect)
+			return rem;
+	}
+
 	if (!kgem_check_batch(&sna->kgem, op->need_magic_ca_pass ? 20 : 6))
 		return 0;
 	if (!kgem_check_reloc_and_exec(&sna->kgem, 2))
@@ -607,17 +617,6 @@ static int gen5_get_rectangles__flush(struct sna *sna,
 						     op->u.gen5.wm_kernel);
 	}
 
-	/* Preventing discarding new vbo after lock contention */
-	if (sna->render.active) {
-		int rem;
-
-		sna_vertex_wait__locked(&sna->render);
-
-		rem = vertex_space(sna);
-		if (rem > op->floats_per_rect)
-			return rem;
-	}
-
 	return gen4_vertex_finish(sna);
 }
 
@@ -631,7 +630,7 @@ inline static int gen5_get_rectangles(struct sna *sna,
 
 start:
 	rem = vertex_space(sna);
-	if (rem < op->floats_per_rect) {
+	if (unlikely(rem < op->floats_per_rect)) {
 		DBG(("flushing vbo for %s: %d < %d\n",
 		     __FUNCTION__, rem, op->floats_per_rect));
 		rem = gen5_get_rectangles__flush(sna, op);
diff --git a/src/sna/gen6_render.c b/src/sna/gen6_render.c
index a9d8111..ec64250 100644
--- a/src/sna/gen6_render.c
+++ b/src/sna/gen6_render.c
@@ -1125,6 +1125,9 @@ static bool gen6_rectangle_begin(struct sna *sna,
 	int id = 1 << GEN6_VERTEX(op->u.gen6.flags);
 	int ndwords;
 
+	if (sna_vertex_wait__locked(&sna->render) && sna->render.vertex_offset)
+		return true;
+
 	ndwords = op->need_magic_ca_pass ? 60 : 6;
 	if ((sna->render.vb_id & id) == 0)
 		ndwords += 5;
@@ -1141,6 +1144,13 @@ static bool gen6_rectangle_begin(struct sna *sna,
 static int gen6_get_rectangles__flush(struct sna *sna,
 				      const struct sna_composite_op *op)
 {
+	/* Preventing discarding new vbo after lock contention */
+	if (sna_vertex_wait__locked(&sna->render)) {
+		int rem = vertex_space(sna);
+		if (rem > op->floats_per_rect)
+			return rem;
+	}
+
 	if (!kgem_check_batch(&sna->kgem, op->need_magic_ca_pass ? 65 : 5))
 		return 0;
 	if (!kgem_check_reloc_and_exec(&sna->kgem, 2))
@@ -1157,17 +1167,6 @@ static int gen6_get_rectangles__flush(struct sna *sna,
 		}
 	}
 
-	/* Preventing discarding new vbo after lock contention */
-	if (sna->render.active) {
-		int rem;
-
-		sna_vertex_wait__locked(&sna->render);
-
-		rem = vertex_space(sna);
-		if (rem > op->floats_per_rect)
-			return rem;
-	}
-
 	return gen4_vertex_finish(sna);
 }
 
@@ -1180,7 +1179,7 @@ inline static int gen6_get_rectangles(struct sna *sna,
 
 start:
 	rem = vertex_space(sna);
-	if (rem < op->floats_per_rect) {
+	if (unlikely(rem < op->floats_per_rect)) {
 		DBG(("flushing vbo for %s: %d < %d\n",
 		     __FUNCTION__, rem, op->floats_per_rect));
 		rem = gen6_get_rectangles__flush(sna, op);
diff --git a/src/sna/gen7_render.c b/src/sna/gen7_render.c
index dc704dd..59b38f6 100644
--- a/src/sna/gen7_render.c
+++ b/src/sna/gen7_render.c
@@ -1270,6 +1270,9 @@ static bool gen7_rectangle_begin(struct sna *sna,
 	int id = 1 << GEN7_VERTEX(op->u.gen7.flags);
 	int ndwords;
 
+	if (sna_vertex_wait__locked(&sna->render) && sna->render.vertex_offset)
+		return true;
+
 	ndwords = op->need_magic_ca_pass ? 60 : 6;
 	if ((sna->render.vb_id & id) == 0)
 		ndwords += 5;
@@ -1286,6 +1289,13 @@ static bool gen7_rectangle_begin(struct sna *sna,
 static int gen7_get_rectangles__flush(struct sna *sna,
 				      const struct sna_composite_op *op)
 {
+	/* Preventing discarding new vbo after lock contention */
+	if (sna_vertex_wait__locked(&sna->render)) {
+		int rem = vertex_space(sna);
+		if (rem > op->floats_per_rect)
+			return rem;
+	}
+
 	if (!kgem_check_batch(&sna->kgem, op->need_magic_ca_pass ? 65 : 6))
 		return 0;
 	if (!kgem_check_reloc_and_exec(&sna->kgem, 2))
@@ -1300,17 +1310,6 @@ static int gen7_get_rectangles__flush(struct sna *sna,
 		}
 	}
 
-	/* Preventing discarding new vbo after lock contention */
-	if (sna->render.active) {
-		int rem;
-
-		sna_vertex_wait__locked(&sna->render);
-
-		rem = vertex_space(sna);
-		if (rem > op->floats_per_rect)
-			return rem;
-	}
-
 	return gen4_vertex_finish(sna);
 }
 
@@ -1323,7 +1322,7 @@ inline static int gen7_get_rectangles(struct sna *sna,
 
 start:
 	rem = vertex_space(sna);
-	if (rem < op->floats_per_rect) {
+	if (unlikely(rem < op->floats_per_rect)) {
 		DBG(("flushing vbo for %s: %d < %d\n",
 		     __FUNCTION__, rem, op->floats_per_rect));
 		rem = gen7_get_rectangles__flush(sna, op);
diff --git a/src/sna/sna_render.h b/src/sna/sna_render.h
index c953e50..6cf90eb 100644
--- a/src/sna/sna_render.h
+++ b/src/sna/sna_render.h
@@ -763,10 +763,12 @@ static inline void sna_vertex_release__locked(struct sna_render *r)
 		pthread_cond_signal(&r->wait);
 }
 
-static inline void sna_vertex_wait__locked(struct sna_render *r)
+static inline bool sna_vertex_wait__locked(struct sna_render *r)
 {
+	bool was_active = r->active;
 	while (r->active)
 		pthread_cond_wait(&r->wait, &r->lock);
+	return was_active;
 }
 
 #endif /* SNA_RENDER_H */
commit 1239e012ae6d4f00ce73f32d7244905a601170ea
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Jan 31 19:18:17 2013 +0000

    sna: Make sure the needs_flush is always accompanied by a tracking request
    
    References: https://bugs.freedesktop.org/show_bug.cgi?id=47597
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 4f9306a..c050a52 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -82,8 +82,8 @@ search_snoop_cache(struct kgem *kgem, unsigned int num_pages, unsigned flags);
 #define SHOW_BATCH 0
 
 #if 0
-#define ASSERT_IDLE(kgem__, handle__) assert(!kgem_busy(kgem__, handle__))
-#define ASSERT_MAYBE_IDLE(kgem__, handle__, expect__) assert(!(expect__) || !kgem_busy(kgem__, handle__))
+#define ASSERT_IDLE(kgem__, handle__) assert(!__kgem_busy(kgem__, handle__))
+#define ASSERT_MAYBE_IDLE(kgem__, handle__, expect__) assert(!(expect__) || !__kgem_busy(kgem__, handle__))
 #else
 #define ASSERT_IDLE(kgem__, handle__)
 #define ASSERT_MAYBE_IDLE(kgem__, handle__, expect__)
@@ -375,8 +375,7 @@ static int gem_read(int fd, uint32_t handle, const void *dst,
 	return 0;
 }
 
-static bool
-kgem_busy(struct kgem *kgem, int handle)
+bool __kgem_busy(struct kgem *kgem, int handle)
 {
 	struct drm_i915_gem_busy busy;
 
@@ -394,19 +393,14 @@ static void kgem_bo_retire(struct kgem *kgem, struct kgem_bo *bo)
 {
 	DBG(("%s: retiring bo handle=%d (needed flush? %d), rq? %d [busy?=%d]\n",
 	     __FUNCTION__, bo->handle, bo->needs_flush, bo->rq != NULL,
-	     kgem_busy(kgem, bo->handle)));
+	     __kgem_busy(kgem, bo->handle)));
+	assert(bo->refcnt);
 	assert(bo->exec == NULL);
 	assert(list_is_empty(&bo->vma));
 
 	if (bo->rq) {
-		if (bo->needs_flush)
-			bo->needs_flush = kgem_busy(kgem, bo->handle);
-		if (bo->needs_flush) {
-			list_move(&bo->request, &kgem->flushing);
-			bo->rq = (void *)kgem;
-		} else {
-			bo->rq = NULL;
-			list_del(&bo->request);
+		if (!__kgem_busy(kgem, bo->handle)) {
+			__kgem_bo_clear_busy(bo);
 			kgem_retire(kgem);
 		}
 	} else {
@@ -1575,7 +1569,6 @@ static void kgem_bo_clear_scanout(struct kgem *kgem, struct kgem_bo *bo)
 	}
 
 	bo->scanout = false;
-	bo->needs_flush = true;
 	bo->flush = false;
 	bo->reusable = true;
 
@@ -1702,16 +1695,12 @@ static void __kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 		DBG(("%s: handle=%d is snooped\n", __FUNCTION__, bo->handle));
 		assert(!bo->flush);
 		assert(list_is_empty(&bo->list));
+		if (bo->exec == NULL && bo->rq && !__kgem_busy(kgem, bo->handle))
+			__kgem_bo_clear_busy(bo);
 		if (bo->rq == NULL) {
-			if (bo->needs_flush && kgem_busy(kgem, bo->handle)) {
-				DBG(("%s: handle=%d is snooped, tracking until free\n",
-				     __FUNCTION__, bo->handle));
-				list_add(&bo->request, &kgem->flushing);
-				bo->rq = (void *)kgem;
-			}
-		}
-		if (bo->rq == NULL)
+			assert(!bo->needs_flush);
 			kgem_bo_move_to_snoop(kgem, bo);
+		}
 		return;
 	}
 
@@ -1766,6 +1755,9 @@ static void __kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 		bo->refcnt = 0;
 	}
 
+	if (bo->rq && bo->exec == NULL && !__kgem_busy(kgem, bo->handle))
+		__kgem_bo_clear_busy(bo);
+
 	if (bo->rq) {
 		struct list *cache;
 
@@ -1781,27 +1773,6 @@ static void __kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 	assert(bo->exec == NULL);
 	assert(list_is_empty(&bo->request));
 
-	if (bo->needs_flush) {
-		if ((bo->needs_flush = kgem_busy(kgem, bo->handle))) {
-			struct list *cache;
-
-			DBG(("%s: handle=%d -> flushing\n",
-			     __FUNCTION__, bo->handle));
-
-			assert(bo->reusable);
-			list_add(&bo->request, &kgem->flushing);
-			if (bucket(bo) < NUM_CACHE_BUCKETS)
-				cache = &kgem->active[bucket(bo)][bo->tiling];
-			else
-				cache = &kgem->large;
-			list_add(&bo->list, cache);
-			bo->rq = (void *)kgem;
-			return;
-		}
-
-		bo->domain = DOMAIN_NONE;
-	}
-
 	if (!IS_CPU_MAP(bo->map)) {
 		if (!kgem_bo_set_purgeable(kgem, bo))
 			goto destroy;
@@ -1878,26 +1849,23 @@ static bool kgem_retire__flushing(struct kgem *kgem)
 		assert(bo->rq == (void *)kgem);
 		assert(bo->exec == NULL);
 
-		if (kgem_busy(kgem, bo->handle))
+		if (__kgem_busy(kgem, bo->handle))
 			break;
 
-		bo->needs_flush = false;
-		bo->domain = DOMAIN_NONE;
-		bo->rq = NULL;
-		list_del(&bo->request);
+		__kgem_bo_clear_busy(bo);
 
-		if (!bo->refcnt) {
-			if (bo->snoop) {
-				kgem_bo_move_to_snoop(kgem, bo);
-			} else if (bo->scanout) {
-				kgem_bo_move_to_scanout(kgem, bo);
-			} else if (bo->reusable &&
-				   kgem_bo_set_purgeable(kgem, bo)) {
-				kgem_bo_move_to_inactive(kgem, bo);
-				retired = true;
-			} else
-				kgem_bo_free(kgem, bo);
-		}
+		if (bo->refcnt)
+			continue;
+
+		if (bo->snoop) {
+			kgem_bo_move_to_snoop(kgem, bo);
+		} else if (bo->scanout) {
+			kgem_bo_move_to_scanout(kgem, bo);
+		} else if (bo->reusable && kgem_bo_set_purgeable(kgem, bo)) {
+			kgem_bo_move_to_inactive(kgem, bo);
+			retired = true;
+		} else
+			kgem_bo_free(kgem, bo);
 	}
 #if HAS_DEBUG_FULL
 	{
@@ -1935,44 +1903,31 @@ static bool __kgem_retire_rq(struct kgem *kgem, struct kgem_request *rq)
 		list_del(&bo->request);
 
 		if (bo->needs_flush)
-			bo->needs_flush = kgem_busy(kgem, bo->handle);
+			bo->needs_flush = __kgem_busy(kgem, bo->handle);
 		if (bo->needs_flush) {
 			DBG(("%s: moving %d to flushing\n",
 			     __FUNCTION__, bo->handle));
 			list_add(&bo->request, &kgem->flushing);
 			bo->rq = (void *)kgem;
-		} else {
-			bo->domain = DOMAIN_NONE;
-			bo->rq = NULL;
+			continue;
 		}
 
+		bo->domain = DOMAIN_NONE;
+		bo->rq = NULL;
 		if (bo->refcnt)
 			continue;
 
 		if (bo->snoop) {
-			if (!bo->needs_flush)
-				kgem_bo_move_to_snoop(kgem, bo);
-			continue;
-		}
-
-		if (!bo->reusable) {
+			kgem_bo_move_to_snoop(kgem, bo);
+		} else if (bo->scanout) {
+			kgem_bo_move_to_scanout(kgem, bo);
+		} else if (bo->reusable && kgem_bo_set_purgeable(kgem, bo)) {
+			kgem_bo_move_to_inactive(kgem, bo);
+			retired = true;
+		} else {
 			DBG(("%s: closing %d\n",
 			     __FUNCTION__, bo->handle));
 			kgem_bo_free(kgem, bo);
-			continue;
-		}
-
-		if (!bo->needs_flush) {
-			if (bo->scanout) {
-				kgem_bo_move_to_scanout(kgem, bo);
-			} else if (kgem_bo_set_purgeable(kgem, bo)) {
-				kgem_bo_move_to_inactive(kgem, bo);
-				retired = true;
-			} else {
-				DBG(("%s: closing %d\n",
-				     __FUNCTION__, bo->handle));
-				kgem_bo_free(kgem, bo);
-			}
 		}
 	}
 
@@ -2004,7 +1959,7 @@ static bool kgem_retire__requests_ring(struct kgem *kgem, int ring)
 		rq = list_first_entry(&kgem->requests[ring],
 				      struct kgem_request,
 				      list);
-		if (kgem_busy(kgem, rq->bo->handle))
+		if (__kgem_busy(kgem, rq->bo->handle))
 			break;
 
 		retired |= __kgem_retire_rq(kgem, rq);
@@ -2073,7 +2028,7 @@ bool __kgem_ring_is_idle(struct kgem *kgem, int ring)
 
 	rq = list_last_entry(&kgem->requests[ring],
 			     struct kgem_request, list);
-	if (kgem_busy(kgem, rq->bo->handle)) {
+	if (__kgem_busy(kgem, rq->bo->handle)) {
 		DBG(("%s: last requests handle=%d still busy\n",
 		     __FUNCTION__, rq->bo->handle));
 		return false;
@@ -2121,10 +2076,8 @@ static void kgem_commit(struct kgem *kgem)
 
 		if (bo->proxy) {
 			/* proxies are not used for domain tracking */
-			list_del(&bo->request);
-			bo->rq = NULL;
 			bo->exec = NULL;
-			bo->needs_flush = false;
+			__kgem_bo_clear_busy(bo);
 		}
 
 		kgem->scanout_busy |= bo->scanout;
@@ -2272,6 +2225,7 @@ static void kgem_finish_buffers(struct kgem *kgem)
 					list_init(&bo->base.request);
 					shrink->needs_flush = bo->base.dirty;
 
+					assert(bo->base.domain == DOMAIN_NONE);
 					bo->base.exec = NULL;
 					bo->base.rq = NULL;
 					bo->base.dirty = false;
@@ -2317,6 +2271,7 @@ static void kgem_finish_buffers(struct kgem *kgem)
 					list_init(&bo->base.request);
 					shrink->needs_flush = bo->base.dirty;
 
+					assert(bo->base.domain == DOMAIN_NONE);
 					bo->base.exec = NULL;
 					bo->base.rq = NULL;
 					bo->base.dirty = false;
@@ -2364,12 +2319,9 @@ static void kgem_cleanup(struct kgem *kgem)
 						      struct kgem_bo,
 						      request);
 
-				list_del(&bo->request);
-				bo->rq = NULL;
 				bo->exec = NULL;
-				bo->domain = DOMAIN_NONE;
 				bo->dirty = false;
-				bo->needs_flush = false;
+				__kgem_bo_clear_busy(bo);
 				if (bo->refcnt == 0)
 					kgem_bo_free(kgem, bo);
 			}
@@ -2434,15 +2386,12 @@ void kgem_reset(struct kgem *kgem)
 			bo->exec = NULL;
 			bo->target_handle = -1;
 			bo->dirty = false;
-			bo->domain = DOMAIN_NONE;
 
-			if (bo->needs_flush && kgem_busy(kgem, bo->handle)) {
+			if (bo->needs_flush && __kgem_busy(kgem, bo->handle)) {
 				list_add(&bo->request, &kgem->flushing);
 				bo->rq = (void *)kgem;
-			} else {
-				bo->rq = NULL;
-				bo->needs_flush = false;
-			}
+			} else
+				__kgem_bo_clear_busy(bo);
 
 			if (!bo->refcnt && !bo->reusable) {
 				assert(!bo->snoop);
@@ -2521,7 +2470,7 @@ out_4096:
 			return kgem_bo_reference(bo);
 		}
 
-		if (!kgem_busy(kgem, bo->handle)) {
+		if (!__kgem_busy(kgem, bo->handle)) {
 			assert(RQ(bo->rq)->bo == bo);
 			__kgem_retire_rq(kgem, RQ(bo->rq));
 			goto out_4096;
@@ -2538,7 +2487,7 @@ out_16384:
 			return kgem_bo_reference(bo);
 		}
 
-		if (!kgem_busy(kgem, bo->handle)) {
+		if (!__kgem_busy(kgem, bo->handle)) {
 			assert(RQ(bo->rq)->bo == bo);
 			__kgem_retire_rq(kgem, RQ(bo->rq));
 			goto out_16384;
@@ -4078,16 +4027,18 @@ void _kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 	__kgem_bo_destroy(kgem, bo);
 }
 
-bool __kgem_flush(struct kgem *kgem, struct kgem_bo *bo)
+void __kgem_flush(struct kgem *kgem, struct kgem_bo *bo)
 {
+	assert(bo->rq);
+	assert(bo->exec == NULL);
+	assert(bo->needs_flush);
+
 	/* The kernel will emit a flush *and* update its own flushing lists. */
-	if (!bo->needs_flush)
-		return false;
+	if (!__kgem_busy(kgem, bo->handle))
+		__kgem_bo_clear_busy(bo);
 
-	bo->needs_flush = kgem_busy(kgem, bo->handle);
 	DBG(("%s: handle=%d, busy?=%d\n",
-	     __FUNCTION__, bo->handle, bo->needs_flush));
-	return bo->needs_flush;
+	     __FUNCTION__, bo->handle, bo->rq != NULL));
 }
 
 inline static bool needs_semaphore(struct kgem *kgem, struct kgem_bo *bo)
@@ -4505,7 +4456,7 @@ void *kgem_bo_map(struct kgem *kgem, struct kgem_bo *bo)
 		struct drm_i915_gem_set_domain set_domain;
 
 		DBG(("%s: sync: needs_flush? %d, domain? %d, busy? %d\n", __FUNCTION__,
-		     bo->needs_flush, bo->domain, kgem_busy(kgem, bo->handle)));
+		     bo->needs_flush, bo->domain, __kgem_busy(kgem, bo->handle)));
 
 		/* XXX use PROT_READ to avoid the write flush? */
 
@@ -4680,10 +4631,7 @@ uint32_t kgem_bo_flink(struct kgem *kgem, struct kgem_bo *bo)
 	 */
 	bo->reusable = false;
 
-	/* The bo is outside of our control, so presume it is written to */
-	bo->needs_flush = true;
-	if (bo->domain != DOMAIN_GPU)
-		bo->domain = DOMAIN_NONE;
+	kgem_bo_unclean(kgem, bo);
 
 	/* Henceforth, we need to broadcast all updates to clients and
 	 * flush our rendering before doing so.
@@ -4732,7 +4680,7 @@ void kgem_bo_sync__cpu(struct kgem *kgem, struct kgem_bo *bo)
 		struct drm_i915_gem_set_domain set_domain;
 
 		DBG(("%s: SYNC: needs_flush? %d, domain? %d, busy? %d\n", __FUNCTION__,
-		     bo->needs_flush, bo->domain, kgem_busy(kgem, bo->handle)));
+		     bo->needs_flush, bo->domain, __kgem_busy(kgem, bo->handle)));
 
 		VG_CLEAR(set_domain);
 		set_domain.handle = bo->handle;
@@ -4755,7 +4703,7 @@ void kgem_bo_sync__cpu_full(struct kgem *kgem, struct kgem_bo *bo, bool write)
 		struct drm_i915_gem_set_domain set_domain;
 
 		DBG(("%s: SYNC: needs_flush? %d, domain? %d, busy? %d\n", __FUNCTION__,
-		     bo->needs_flush, bo->domain, kgem_busy(kgem, bo->handle)));
+		     bo->needs_flush, bo->domain, __kgem_busy(kgem, bo->handle)));
 
 		VG_CLEAR(set_domain);
 		set_domain.handle = bo->handle;
@@ -4779,7 +4727,7 @@ void kgem_bo_sync__gtt(struct kgem *kgem, struct kgem_bo *bo)
 		struct drm_i915_gem_set_domain set_domain;
 
 		DBG(("%s: SYNC: needs_flush? %d, domain? %d, busy? %d\n", __FUNCTION__,
-		     bo->needs_flush, bo->domain, kgem_busy(kgem, bo->handle)));
+		     bo->needs_flush, bo->domain, __kgem_busy(kgem, bo->handle)));
 
 		VG_CLEAR(set_domain);
 		set_domain.handle = bo->handle;
@@ -5511,7 +5459,7 @@ void kgem_buffer_read_sync(struct kgem *kgem, struct kgem_bo *_bo)
 		     __FUNCTION__,
 		     bo->base.needs_flush,
 		     bo->base.domain,
-		     kgem_busy(kgem, bo->base.handle)));
+		     __kgem_busy(kgem, bo->base.handle)));
 
 		assert(!IS_CPU_MAP(bo->base.map) || bo->base.snoop || kgem->has_llc);
 
diff --git a/src/sna/kgem.h b/src/sna/kgem.h
index b345b49..d05326a 100644
--- a/src/sna/kgem.h
+++ b/src/sna/kgem.h
@@ -326,7 +326,7 @@ static inline void kgem_bo_submit(struct kgem *kgem, struct kgem_bo *bo)
 		_kgem_submit(kgem);
 }
 
-bool __kgem_flush(struct kgem *kgem, struct kgem_bo *bo);
+void __kgem_flush(struct kgem *kgem, struct kgem_bo *bo);
 static inline void kgem_bo_flush(struct kgem *kgem, struct kgem_bo *bo)
 {
 	kgem_bo_submit(kgem, bo);
@@ -338,7 +338,7 @@ static inline void kgem_bo_flush(struct kgem *kgem, struct kgem_bo *bo)
 	 * we assume direct access. And as the useual failure is EIO, we do
 	 * not actualy care.
 	 */
-	(void)__kgem_flush(kgem, bo);
+	__kgem_flush(kgem, bo);
 }
 
 static inline struct kgem_bo *kgem_bo_reference(struct kgem_bo *bo)
@@ -567,11 +567,21 @@ static inline bool kgem_bo_is_snoop(struct kgem_bo *bo)
 	return bo->snoop;
 }
 
+bool __kgem_busy(struct kgem *kgem, int handle);
+
 static inline void kgem_bo_mark_busy(struct kgem_bo *bo, int ring)
 {
 	bo->rq = (struct kgem_request *)((uintptr_t)bo->rq | ring);
 }
 
+inline static void __kgem_bo_clear_busy(struct kgem_bo *bo)
+{
+	bo->needs_flush = false;
+	list_del(&bo->request);
+	bo->rq = NULL;
+	bo->domain = DOMAIN_NONE;
+}
+
 static inline bool kgem_bo_is_busy(struct kgem_bo *bo)
 {
 	DBG(("%s: handle=%d, domain: %d exec? %d, rq? %d\n", __FUNCTION__,
@@ -585,10 +595,16 @@ static inline bool __kgem_bo_is_busy(struct kgem *kgem, struct kgem_bo *bo)
 	DBG(("%s: handle=%d, domain: %d exec? %d, rq? %d\n", __FUNCTION__,
 	     bo->handle, bo->domain, bo->exec != NULL, bo->rq != NULL));
 	assert(bo->refcnt);
+
+	if (bo->exec)
+		return true;
+
 	if (kgem_flush(kgem, bo->flush))
 		kgem_submit(kgem);
-	if (bo->rq && !bo->exec)
-		kgem_retire(kgem);
+
+	if (bo->rq && !__kgem_busy(kgem, bo->handle))
+		__kgem_bo_clear_busy(bo);
+
 	return kgem_bo_is_busy(bo);
 }
 
@@ -601,6 +617,17 @@ static inline bool kgem_bo_is_dirty(struct kgem_bo *bo)
 	return bo->dirty;
 }
 
+static inline void kgem_bo_unclean(struct kgem *kgem, struct kgem_bo *bo)
+{
+	/* The bo is outside of our control, so presume it is written to */
+	bo->needs_flush = true;
+	if (bo->rq == NULL)
+		bo->rq = (void *)kgem;
+
+	if (bo->domain != DOMAIN_GPU)
+		bo->domain = DOMAIN_NONE;
+}
+
 static inline void __kgem_bo_mark_dirty(struct kgem_bo *bo)
 {
 	DBG(("%s: handle=%d (proxy? %d)\n", __FUNCTION__,
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index e406506..9b1a385 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -13612,8 +13612,9 @@ sna_accel_flush_callback(CallbackListPtr *list,
 		} else {
 			DBG(("%s: flushing DRI pixmap=%ld\n", __FUNCTION__,
 			     priv->pixmap->drawable.serialNumber));
-			ret = sna_pixmap_move_to_gpu(priv->pixmap,
-						     MOVE_READ | __MOVE_FORCE);
+			if (sna_pixmap_move_to_gpu(priv->pixmap,
+						   MOVE_READ | __MOVE_FORCE))
+				kgem_bo_unclean(&sna->kgem, priv->gpu_bo);
 		}
 		(void)ret;
 	}
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index b368648..432b341 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -3305,7 +3305,7 @@ void sna_mode_redisplay(struct sna *sna)
 		RegionIntersect(&damage, &damage, region);
 		if (RegionNotEmpty(&damage)) {
 			sna_crtc_redisplay(crtc, &damage);
-			__kgem_flush(&sna->kgem, sna_crtc->bo);
+			kgem_bo_flush(&sna->kgem, sna_crtc->bo);
 		}
 		RegionUninit(&damage);
 	}


More information about the xorg-commit mailing list