sna: Support binding of a bo for multiple formats

Applications may use the same pixmap with multiple formats within the
same operation. For instance, you can premultiply and composite a normal
pixmap in this manner.  However, as we reused the sampler binding
locations of the source (without an alpha channel) for the mask, we
failed to read and multiply by the alpha channel causing it to remain
black instead of transparent.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40926
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

7 files changed, 117 insertions, 174 deletions

diff --git a/src/sna/gen4_render.c b/src/sna/gen4_render.c
index 8f59dc12..3c269945 100644
--- a/src/sna/gen4_render.c
+++ b/src/sna/gen4_render.c
@@ -496,22 +496,6 @@ static Bool gen4_check_dst_format(PictFormat format)
 	return FALSE;
 }
 
-static bool gen4_format_is_dst(uint32_t format)
-{
-	switch (format) {
-	case GEN4_SURFACEFORMAT_B8G8R8A8_UNORM:
-	case GEN4_SURFACEFORMAT_R8G8B8A8_UNORM:
-	case GEN4_SURFACEFORMAT_B10G10R10A2_UNORM:
-	case GEN4_SURFACEFORMAT_B5G6R5_UNORM:
-	case GEN4_SURFACEFORMAT_B5G5R5A1_UNORM:
-	case GEN4_SURFACEFORMAT_A8_UNORM:
-	case GEN4_SURFACEFORMAT_B4G4R4A4_UNORM:
-		return true;
-	default:
-		return false;
-	}
-}
-
 typedef struct gen4_surface_state_padded {
 	struct gen4_surface_state state;
 	char pad[32 - sizeof(struct gen4_surface_state)];
@@ -657,26 +641,16 @@ gen4_bind_bo(struct sna *sna,
 	if (is_dst) {
 		domains = I915_GEM_DOMAIN_RENDER << 16 | I915_GEM_DOMAIN_RENDER;
 		kgem_bo_mark_dirty(bo);
-	} else {
+	} else
 		domains = I915_GEM_DOMAIN_SAMPLER << 16;
-		is_dst = gen4_format_is_dst(format);
-	}
+
+	offset = kgem_bo_get_binding(bo, format);
+	if (offset)
+		return offset;
 
 	offset = sna->kgem.surface - sizeof(struct gen4_surface_state_padded) / sizeof(uint32_t);
 	offset *= sizeof(uint32_t);
 
-	if (is_dst) {
-		if (bo->dst_bound)
-			return bo->dst_bound;
-
-		bo->dst_bound = offset;
-	} else {
-		if (bo->src_bound)
-			return bo->src_bound;
-
-		bo->src_bound = offset;
-	}
-
 	sna->kgem.surface -=
 		sizeof(struct gen4_surface_state_padded) / sizeof(uint32_t);
 	ss = memset(sna->kgem.batch + sna->kgem.surface, 0, sizeof(*ss));
@@ -697,6 +671,8 @@ gen4_bind_bo(struct sna *sna,
 	ss->ss3.tiled_surface = bo->tiling != I915_TILING_NONE;
 	ss->ss3.tile_walk     = bo->tiling == I915_TILING_Y;
 
+	kgem_bo_set_binding(bo, format, offset);
+
 	DBG(("[%x] bind bo(handle=%d, addr=%d), format=%d, width=%d, height=%d, pitch=%d, tiling=%d -> %s\n",
 	     offset, bo->handle, ss->ss1.base_addr,
 	     ss->ss0.surface_format, width, height, bo->pitch, bo->tiling,
diff --git a/src/sna/gen5_render.c b/src/sna/gen5_render.c
index c48e828e..7839d8e1 100644
--- a/src/sna/gen5_render.c
+++ b/src/sna/gen5_render.c
@@ -505,22 +505,6 @@ static uint32_t gen5_get_card_format_for_depth(int depth)
 	}
 }
 
-static bool gen5_format_is_dst(uint32_t format)
-{
-	switch (format) {
-	case GEN5_SURFACEFORMAT_B8G8R8A8_UNORM:
-	case GEN5_SURFACEFORMAT_R8G8B8A8_UNORM:
-	case GEN5_SURFACEFORMAT_B10G10R10A2_UNORM:
-	case GEN5_SURFACEFORMAT_B5G6R5_UNORM:
-	case GEN5_SURFACEFORMAT_B5G5R5A1_UNORM:
-	case GEN5_SURFACEFORMAT_A8_UNORM:
-	case GEN5_SURFACEFORMAT_B4G4R4A4_UNORM:
-		return true;
-	default:
-		return false;
-	}
-}
-
 typedef struct gen5_surface_state_padded {
 	struct gen5_surface_state state;
 	char pad[32 - sizeof(struct gen5_surface_state)];
@@ -677,26 +661,18 @@ gen5_bind_bo(struct sna *sna,
 	if (is_dst) {
 		domains = I915_GEM_DOMAIN_RENDER << 16 | I915_GEM_DOMAIN_RENDER;
 		kgem_bo_mark_dirty(bo);
-	} else {
+	} else
 		domains = I915_GEM_DOMAIN_SAMPLER << 16;
-		is_dst = gen5_format_is_dst(format);
+
+	if (!DBG_NO_SURFACE_CACHE) {
+		offset = kgem_bo_get_binding(bo, format);
+		if (offset)
+			return offset;
 	}
 
 	offset = sna->kgem.surface - sizeof(struct gen5_surface_state_padded) / sizeof(uint32_t);
 	offset *= sizeof(uint32_t);
 
-	if (is_dst) {
-		if (!DBG_NO_SURFACE_CACHE && bo->dst_bound)
-			return bo->dst_bound;
-
-		bo->dst_bound = offset;
-	} else {
-		if (!DBG_NO_SURFACE_CACHE && bo->src_bound)
-			return bo->src_bound;
-
-		bo->src_bound = offset;
-	}
-
 	sna->kgem.surface -=
 		sizeof(struct gen5_surface_state_padded) / sizeof(uint32_t);
 	ss = sna->kgem.batch + sna->kgem.surface;
@@ -716,6 +692,8 @@ gen5_bind_bo(struct sna *sna,
 	ss[4] = 0;
 	ss[5] = 0;
 
+	kgem_bo_set_binding(bo, format, offset);
+
 	DBG(("[%x] bind bo(handle=%d, addr=%d), format=%d, width=%d, height=%d, pitch=%d, tiling=%d -> %s\n",
 	     offset, bo->handle, ss[1],
 	     format, width, height, bo->pitch, bo->tiling,
diff --git a/src/sna/gen6_render.c b/src/sna/gen6_render.c
index 51d26c8d..02b051fe 100644
--- a/src/sna/gen6_render.c
+++ b/src/sna/gen6_render.c
@@ -330,22 +330,6 @@ static uint32_t gen6_get_card_format_for_depth(int depth)
 	}
 }
 
-static bool gen6_format_is_dst(uint32_t format)
-{
-	switch (format) {
-	case GEN6_SURFACEFORMAT_B8G8R8A8_UNORM:
-	case GEN6_SURFACEFORMAT_R8G8B8A8_UNORM:
-	case GEN6_SURFACEFORMAT_B10G10R10A2_UNORM:
-	case GEN6_SURFACEFORMAT_B5G6R5_UNORM:
-	case GEN6_SURFACEFORMAT_B5G5R5A1_UNORM:
-	case GEN6_SURFACEFORMAT_A8_UNORM:
-	case GEN6_SURFACEFORMAT_B4G4R4A4_UNORM:
-		return true;
-	default:
-		return false;
-	}
-}
-
 static uint32_t gen6_filter(uint32_t filter)
 {
 	switch (filter) {
@@ -1073,26 +1057,20 @@ gen6_bind_bo(struct sna *sna,
 	if (is_dst) {
 		domains = I915_GEM_DOMAIN_RENDER << 16 |I915_GEM_DOMAIN_RENDER;
 		kgem_bo_mark_dirty(bo);
-	} else {
+	} else
 		domains = I915_GEM_DOMAIN_SAMPLER << 16;
-		is_dst = gen6_format_is_dst(format);
+
+	offset = kgem_bo_get_binding(bo, format);
+	if (offset) {
+		DBG(("[%x]  bo(handle=%d), format=%d, reuse %s binding\n",
+		     offset, bo->handle, format,
+		     domains & 0xffff ? "render" : "sampler"));
+		return offset;
 	}
 
 	offset = sna->kgem.surface - sizeof(struct gen6_surface_state_padded) / sizeof(uint32_t);
 	offset *= sizeof(uint32_t);
 
-	if (is_dst) {
-		if (bo->dst_bound)
-			return bo->dst_bound;
-
-		bo->dst_bound = offset;
-	} else {
-		if (bo->src_bound)
-			return bo->src_bound;
-
-		bo->src_bound = offset;
-	}
-
 	sna->kgem.surface -=
 		sizeof(struct gen6_surface_state_padded) / sizeof(uint32_t);
 	ss = sna->kgem.batch + sna->kgem.surface;
@@ -1109,6 +1087,8 @@ gen6_bind_bo(struct sna *sna,
 	ss[4] = 0;
 	ss[5] = 0;
 
+	kgem_bo_set_binding(bo, format, offset);
+
 	DBG(("[%x] bind bo(handle=%d, addr=%d), format=%d, width=%d, height=%d, pitch=%d, tiling=%d -> %s\n",
 	     offset, bo->handle, ss[1],
 	     format, width, height, bo->pitch, bo->tiling,
diff --git a/src/sna/gen7_render.c b/src/sna/gen7_render.c
index 3be87b71..ba6e9d54 100644
--- a/src/sna/gen7_render.c
+++ b/src/sna/gen7_render.c
@@ -342,22 +342,6 @@ static uint32_t gen7_get_card_format_for_depth(int depth)
 	}
 }
 
-static bool gen7_format_is_dst(uint32_t format)
-{
-	switch (format) {
-	case GEN7_SURFACEFORMAT_B8G8R8A8_UNORM:
-	case GEN7_SURFACEFORMAT_R8G8B8A8_UNORM:
-	case GEN7_SURFACEFORMAT_B10G10R10A2_UNORM:
-	case GEN7_SURFACEFORMAT_B5G6R5_UNORM:
-	case GEN7_SURFACEFORMAT_B5G5R5A1_UNORM:
-	case GEN7_SURFACEFORMAT_A8_UNORM:
-	case GEN7_SURFACEFORMAT_B4G4R4A4_UNORM:
-		return true;
-	default:
-		return false;
-	}
-}
-
 static uint32_t gen7_filter(uint32_t filter)
 {
 	switch (filter) {
@@ -1208,26 +1192,16 @@ gen7_bind_bo(struct sna *sna,
 	if (is_dst) {
 		domains = I915_GEM_DOMAIN_RENDER << 16 |I915_GEM_DOMAIN_RENDER;
 		kgem_bo_mark_dirty(bo);
-	} else {
+	} else
 		domains = I915_GEM_DOMAIN_SAMPLER << 16;
-		is_dst = gen7_format_is_dst(format);
-	}
+
+	offset = kgem_bo_get_binding(bo, format);
+	if (offset)
+		return offset;
 
 	offset = sna->kgem.surface - sizeof(struct gen7_surface_state_padded) / sizeof(uint32_t);
 	offset *= sizeof(uint32_t);
 
-	if (is_dst) {
-		if (bo->dst_bound)
-			return bo->dst_bound;
-
-		bo->dst_bound = offset;
-	} else {
-		if (bo->src_bound)
-			return bo->src_bound;
-
-		bo->src_bound = offset;
-	}
-
 	sna->kgem.surface -=
 		sizeof(struct gen7_surface_state_padded) / sizeof(uint32_t);
 	ss = sna->kgem.batch + sna->kgem.surface;
@@ -1245,6 +1219,8 @@ gen7_bind_bo(struct sna *sna,
 	ss[6] = 0;
 	ss[7] = 0;
 
+	kgem_bo_set_binding(bo, format, offset);
+
 	DBG(("[%x] bind bo(handle=%d, addr=%d), format=%d, width=%d, height=%d, pitch=%d, tiling=%d -> %s\n",
 	     offset, bo->handle, ss[1],
 	     format, width, height, bo->pitch, bo->tiling,
diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 37e60357..046df150 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -537,12 +537,29 @@ static void kgem_fixup_self_relocs(struct kgem *kgem, struct kgem_bo *bo)
 	}
 }
 
+static void kgem_bo_free(struct kgem *kgem, struct kgem_bo *bo)
+{
+	struct kgem_bo_binding *b;
+
+	b = bo->binding.next;
+	while (b) {
+		struct kgem_bo_binding *next = b->next;
+		free (b);
+		b = next;
+	}
+
+	list_del(&bo->list);
+	list_del(&bo->request);
+	gem_close(kgem->fd, bo->handle);
+	free(bo);
+}
+
 static void __kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 {
 	assert(list_is_empty(&bo->list));
 	assert(bo->refcnt == 0);
 
-	bo->src_bound = bo->dst_bound = 0;
+	bo->binding.offset = 0;
 
 	if (NO_CACHE)
 		goto destroy;
@@ -573,11 +590,8 @@ static void __kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 	return;
 
 destroy:
-	if (!bo->exec) {
-		list_del(&bo->request);
-		gem_close(kgem->fd, bo->handle);
-		free(bo);
-	}
+	if (!bo->exec)
+		kgem_bo_free(kgem, bo);
 }
 
 static void kgem_bo_unref(struct kgem *kgem, struct kgem_bo *bo)
@@ -647,8 +661,7 @@ bool kgem_retire(struct kgem *kgem)
 				} else {
 					DBG(("%s: closing %d\n",
 					     __FUNCTION__, bo->handle));
-					gem_close(kgem->fd, bo->handle);
-					free(bo);
+					kgem_bo_free(kgem, bo);
 				}
 			}
 		}
@@ -684,8 +697,8 @@ static void kgem_commit(struct kgem *kgem)
 	struct kgem_bo *bo, *next;
 
 	list_for_each_entry_safe(bo, next, &rq->buffers, request) {
-		bo->src_bound = bo->dst_bound = 0;
 		bo->presumed_offset = bo->exec->offset;
+		bo->binding.offset = 0;
 		bo->exec = NULL;
 		bo->dirty = false;
 		bo->cpu_read = false;
@@ -693,11 +706,7 @@ static void kgem_commit(struct kgem *kgem)
 
 		if (!bo->refcnt) {
 			if (!bo->reusable) {
-destroy:
-				list_del(&bo->list);
-				list_del(&bo->request);
-				gem_close(kgem->fd, bo->handle);
-				free(bo);
+destroy:			kgem_bo_free(kgem, bo);
 				continue;
 			}
 			if (!bo->deleted) {
@@ -718,15 +727,8 @@ destroy:
 
 static void kgem_close_list(struct kgem *kgem, struct list *head)
 {
-	while (!list_is_empty(head)) {
-		struct kgem_bo *bo;
-
-		bo = list_first_entry(head, struct kgem_bo, list);
-		gem_close(kgem->fd, bo->handle);
-		list_del(&bo->list);
-		list_del(&bo->request);
-		free(bo);
-	}
+	while (!list_is_empty(head))
+		kgem_bo_free(kgem, list_first_entry(head, struct kgem_bo, list));
 }
 
 static void kgem_close_inactive(struct kgem *kgem)
@@ -747,9 +749,7 @@ static void kgem_finish_partials(struct kgem *kgem)
 				DBG(("%s: discarding unused partial array: %d/%d\n",
 				     __FUNCTION__, bo->used, bo->alloc));
 
-				list_del(&bo->base.list);
-				gem_close(kgem->fd, bo->base.handle);
-				free(bo);
+				kgem_bo_free(kgem, &bo->base);
 			}
 
 			continue;
@@ -808,11 +808,8 @@ static void kgem_cleanup(struct kgem *kgem)
 			list_del(&bo->request);
 			bo->rq = NULL;
 			bo->gpu = false;
-			if (bo->refcnt == 0) {
-				list_del(&bo->list);
-				gem_close(kgem->fd, bo->handle);
-				free(bo);
-			}
+			if (bo->refcnt == 0)
+				kgem_bo_free(kgem, bo);
 		}
 
 		list_del(&rq->list);
@@ -861,7 +858,7 @@ void kgem_reset(struct kgem *kgem)
 	while (!list_is_empty(&rq->buffers)) {
 		bo = list_first_entry(&rq->buffers, struct kgem_bo, request);
 
-		bo->src_bound = bo->dst_bound = 0;
+		bo->binding.offset = 0;
 		bo->exec = NULL;
 		bo->dirty = false;
 		bo->cpu_read = false;
@@ -1137,9 +1134,7 @@ bool kgem_expire_cache(struct kgem *kgem)
 			count++;
 			size += bo->size;
 
-			gem_close(kgem->fd, bo->handle);
-			list_del(&bo->list);
-			free(bo);
+			kgem_bo_free(kgem, bo);
 		}
 	}
 
@@ -1155,7 +1150,6 @@ bool kgem_expire_cache(struct kgem *kgem)
 
 void kgem_cleanup_cache(struct kgem *kgem)
 {
-	struct kgem_bo *bo;
 	unsigned int i;
 
 	/* sync to the most recent request */
@@ -1178,14 +1172,10 @@ void kgem_cleanup_cache(struct kgem *kgem)
 	kgem_expire_partial(kgem);
 
 	for (i = 0; i < ARRAY_SIZE(kgem->inactive); i++) {
-		while (!list_is_empty(&kgem->inactive[i])) {
-			bo = list_last_entry(&kgem->inactive[i],
-					     struct kgem_bo, list);
-
-			gem_close(kgem->fd, bo->handle);
-			list_del(&bo->list);
-			free(bo);
-		}
+		while (!list_is_empty(&kgem->inactive[i]))
+			kgem_bo_free(kgem,
+				     list_last_entry(&kgem->inactive[i],
+						     struct kgem_bo, list));
 	}
 
 	kgem->need_purge = false;
@@ -1482,9 +1472,7 @@ search_active: /* Best active match first */
 				if (!gem_madvise(kgem->fd, bo->handle,
 						 I915_MADV_WILLNEED)) {
 					kgem->need_purge |= bo->gpu;
-					gem_close(kgem->fd, bo->handle);
-					list_del(&bo->request);
-					free(bo);
+					kgem_bo_free(kgem, bo);
 					bo = NULL;
 					goto search_active;
 				}
@@ -1549,9 +1537,7 @@ skip_active_search:
 		return kgem_bo_reference(bo);
 
 next_bo:
-		gem_close(kgem->fd, bo->handle);
-		list_del(&bo->request);
-		free(bo);
+		kgem_bo_free(kgem, bo);
 		continue;
 	}
 
@@ -1588,6 +1574,8 @@ void _kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 {
 	if (bo->proxy) {
 		kgem_bo_unref(kgem, bo->proxy);
+
+		assert(bo->binding.next == NULL);
 		list_del(&bo->request);
 		free(bo);
 		return;
@@ -2185,3 +2173,40 @@ void kgem_buffer_read_sync(struct kgem *kgem, struct kgem_bo *_bo)
 	} else
 		kgem_bo_sync(kgem, &bo->base, false);
 }
+
+uint32_t kgem_bo_get_binding(struct kgem_bo *bo, uint32_t format)
+{
+	struct kgem_bo_binding *b;
+
+	for (b = &bo->binding; b && b->offset; b = b->next)
+		if (format == b->format)
+			return b->offset;
+
+	return 0;
+}
+
+void kgem_bo_set_binding(struct kgem_bo *bo, uint32_t format, uint16_t offset)
+{
+	struct kgem_bo_binding *b;
+
+	for (b = &bo->binding; b; b = b->next) {
+		if (b->offset)
+			continue;
+
+		b->offset = offset;
+		b->format = format;
+
+		if (b->next)
+			b->next->offset = 0;
+
+		return;
+	}
+
+	b = malloc(sizeof(*b));
+	if (b) {
+		b->next = bo->binding.next;
+		b->format = format;
+		b->offset = offset;
+		bo->binding.next = b;
+	}
+}
diff --git a/src/sna/kgem.h b/src/sna/kgem.h
index 4b260e21..a2258e18 100644
--- a/src/sna/kgem.h
+++ b/src/sna/kgem.h
@@ -51,7 +51,12 @@ struct kgem_bo {
 	struct kgem_request *rq;
 	struct drm_i915_gem_exec_object2 *exec;
 
-	uint16_t src_bound, dst_bound;
+	struct kgem_bo_binding {
+		struct kgem_bo_binding *next;
+		uint32_t format;
+		uint16_t offset;
+	} binding;
+
 	uint32_t unique_id;
 	uint32_t refcnt;
 	uint32_t handle;
@@ -172,6 +177,9 @@ struct kgem_bo *kgem_create_2d(struct kgem *kgem,
 			       int tiling,
 			       uint32_t flags);
 
+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_submit(struct kgem *kgem);
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 555cbc67..e0bf47be 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -6820,7 +6820,7 @@ sna_glyph_blt(DrawablePtr drawable, GCPtr gc,
 	uint8_t rop = transparent ? copy_ROP[gc->alu] : ROP_S;
 
 	DBG(("%s (%d, %d) x %d, transparent? %d, alu=%d\n",
-	     __FUNCTION__, _x, _y, _n, transparent, alu));
+	     __FUNCTION__, _x, _y, _n, transparent, rop));
 
 	if (!sna_drawable_use_gpu_bo(drawable, &clip->extents, &damage))
 		return false;