xf86-video-intel: 7 commits - src/sna/kgem.c src/sna/kgem.h src/sna/sna_accel.c src/sna/sna_damage.c src/sna/sna_io.c

Chris Wilson ickle at kemper.freedesktop.org
Sat Jun 29 08:08:50 PDT 2013


 src/sna/kgem.c       |   50 +++++++++++++++
 src/sna/kgem.h       |   16 ++++
 src/sna/sna_accel.c  |  169 +++++++++++++++++++++++++++++++--------------------
 src/sna/sna_damage.c |   49 +++++++++++++-
 src/sna/sna_io.c     |   11 +--
 5 files changed, 218 insertions(+), 77 deletions(-)

New commits:
commit 6ab2a3acf71b5204c399c7649e5601c93a99f25f
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Sat Jun 29 15:04:09 2013 +0100

    sna: Improve checks for coherent access through CPU mappings
    
    Refactor the CPU mapping tests to a single function, and remember to
    test for a pending GPU write (i.e. bo->exec).
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/kgem.h b/src/sna/kgem.h
index d1a391a..c7c7fce 100644
--- a/src/sna/kgem.h
+++ b/src/sna/kgem.h
@@ -564,6 +564,22 @@ static inline bool kgem_bo_can_map(struct kgem *kgem, struct kgem_bo *bo)
 	return kgem_bo_size(bo) <= kgem->aperture_mappable / 4;
 }
 
+static inline bool kgem_bo_can_map__cpu(struct kgem *kgem,
+					struct kgem_bo *bo,
+					bool write)
+{
+	if (bo->scanout)
+		return false;
+
+	if (kgem->has_llc)
+		return true;
+
+	if (bo->domain != DOMAIN_CPU)
+		return false;
+
+	return !write || bo->exec == NULL;
+}
+
 static inline bool kgem_bo_is_snoop(struct kgem_bo *bo)
 {
 	assert(bo->refcnt);
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 18836c8..2666798 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -1403,7 +1403,8 @@ void sna_pixmap_destroy(PixmapPtr pixmap)
 }
 
 static inline bool has_coherent_map(struct sna *sna,
-				    struct kgem_bo *bo)
+				    struct kgem_bo *bo,
+				    unsigned flags)
 {
 	assert(bo->map);
 
@@ -1413,7 +1414,7 @@ static inline bool has_coherent_map(struct sna *sna,
 	if (bo->tiling != I915_TILING_NONE)
 		return false;
 
-	return bo->domain == DOMAIN_CPU || sna->kgem.has_llc;
+	return kgem_bo_can_map__cpu(&sna->kgem, bo, flags & MOVE_WRITE);
 }
 
 static inline bool has_coherent_ptr(struct sna_pixmap *priv)
@@ -1437,7 +1438,7 @@ static inline bool has_coherent_ptr(struct sna_pixmap *priv)
 static inline bool pixmap_inplace(struct sna *sna,
 				  PixmapPtr pixmap,
 				  struct sna_pixmap *priv,
-				  bool write_only)
+				  unsigned flags)
 {
 	if (FORCE_INPLACE)
 		return FORCE_INPLACE > 0;
@@ -1446,9 +1447,9 @@ static inline bool pixmap_inplace(struct sna *sna,
 		return false;
 
 	if (priv->mapped)
-		return has_coherent_map(sna, priv->gpu_bo);
+		return has_coherent_map(sna, priv->gpu_bo, flags);
 
-	if (!write_only && priv->cpu_damage)
+	if (flags & MOVE_READ && priv->cpu_damage)
 		return false;
 
 	return (pixmap->devKind * pixmap->drawable.height >> 12) >
@@ -1858,7 +1859,7 @@ _sna_pixmap_move_to_cpu(PixmapPtr pixmap, unsigned int flags)
 			if (!priv->mapped)
 				goto skip_inplace_map;
 
-			assert(has_coherent_map(sna, priv->gpu_bo));
+			assert(has_coherent_map(sna, priv->gpu_bo, flags));
 			pixmap->devKind = priv->gpu_bo->pitch;
 
 			assert(priv->gpu_bo->proxy == NULL);
@@ -1906,7 +1907,7 @@ skip_inplace_map:
 	assert(priv->gpu_bo == NULL || priv->gpu_bo->proxy == NULL);
 
 	if (operate_inplace(priv, flags) &&
-	    pixmap_inplace(sna, pixmap, priv, (flags & MOVE_READ) == 0) &&
+	    pixmap_inplace(sna, pixmap, priv, flags) &&
 	     sna_pixmap_create_mappable_gpu(pixmap, (flags & MOVE_READ) == 0)) {
 		DBG(("%s: try to operate inplace (GTT)\n", __FUNCTION__));
 		assert(priv->cow == NULL || (flags & MOVE_WRITE) == 0);
@@ -1918,7 +1919,7 @@ skip_inplace_map:
 		pixmap->devPrivate.ptr = kgem_bo_map(&sna->kgem, priv->gpu_bo);
 		priv->mapped = pixmap->devPrivate.ptr != NULL;
 		if (priv->mapped) {
-			assert(has_coherent_map(sna, priv->gpu_bo));
+			assert(has_coherent_map(sna, priv->gpu_bo, flags));
 			pixmap->devKind = priv->gpu_bo->pitch;
 			if (flags & MOVE_WRITE) {
 				assert(priv->gpu_bo->proxy == NULL);
@@ -1946,7 +1947,7 @@ skip_inplace_map:
 	}
 
 	if (priv->gpu_damage && priv->cpu_damage == NULL && !priv->cow &&
-	    (flags & MOVE_READ || priv->gpu_bo->domain == DOMAIN_CPU || sna->kgem.has_llc) &&
+	    (flags & MOVE_READ || kgem_bo_can_map__cpu(&sna->kgem, priv->gpu_bo, flags & MOVE_WRITE)) &&
 	    priv->gpu_bo->tiling == I915_TILING_NONE &&
 	    ((flags & (MOVE_WRITE | MOVE_ASYNC_HINT)) == 0 ||
 	     !__kgem_bo_is_busy(&sna->kgem, priv->gpu_bo))) {
@@ -2161,7 +2162,7 @@ static inline bool region_inplace(struct sna *sna,
 				  PixmapPtr pixmap,
 				  RegionPtr region,
 				  struct sna_pixmap *priv,
-				  bool write_only)
+				  unsigned flags)
 {
 	assert_pixmap_damage(pixmap);
 
@@ -2171,7 +2172,7 @@ static inline bool region_inplace(struct sna *sna,
 	if (wedged(sna) && !priv->pinned)
 		return false;
 
-	if ((priv->cpu || !write_only) &&
+	if ((priv->cpu || flags & MOVE_READ) &&
 	    region_overlaps_damage(region, priv->cpu_damage, 0, 0)) {
 		DBG(("%s: no, uncovered CPU damage pending\n", __FUNCTION__));
 		return false;
@@ -2184,7 +2185,7 @@ static inline bool region_inplace(struct sna *sna,
 
 	if (priv->mapped) {
 		DBG(("%s: yes, already mapped, continuiung\n", __FUNCTION__));
-		return has_coherent_map(sna, priv->gpu_bo);
+		return has_coherent_map(sna, priv->gpu_bo, flags);
 	}
 
 	if (priv->flush) {
@@ -2301,7 +2302,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 	}
 
 	if (operate_inplace(priv, flags) &&
-	    region_inplace(sna, pixmap, region, priv, (flags & MOVE_READ) == 0) &&
+	    region_inplace(sna, pixmap, region, priv, flags) &&
 	     sna_pixmap_create_mappable_gpu(pixmap, false)) {
 		DBG(("%s: try to operate inplace\n", __FUNCTION__));
 		assert(priv->cow == NULL || (flags & MOVE_WRITE) == 0);
@@ -2312,7 +2313,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 		pixmap->devPrivate.ptr = kgem_bo_map(&sna->kgem, priv->gpu_bo);
 		priv->mapped = pixmap->devPrivate.ptr != NULL;
 		if (priv->mapped) {
-			assert(has_coherent_map(sna, priv->gpu_bo));
+			assert(has_coherent_map(sna, priv->gpu_bo, flags));
 			pixmap->devKind = priv->gpu_bo->pitch;
 			if (flags & MOVE_WRITE) {
 				if (!DAMAGE_IS_ALL(priv->gpu_damage)) {
@@ -2359,7 +2360,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 	     sna_damage_contains_box__no_reduce(priv->gpu_damage,
 						&region->extents)) &&
 	    priv->gpu_bo->tiling == I915_TILING_NONE &&
-	    (priv->gpu_bo->domain == DOMAIN_CPU || sna->kgem.has_llc) &&
+	    kgem_bo_can_map__cpu(&sna->kgem, priv->gpu_bo, flags & MOVE_WRITE) &&
 	    ((flags & (MOVE_WRITE | MOVE_ASYNC_HINT)) == 0 ||
 	     !__kgem_bo_is_busy(&sna->kgem, priv->gpu_bo))) {
 		DBG(("%s: try to operate inplace (CPU), read? %d, write? %d\n",
@@ -2370,7 +2371,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 		pixmap->devPrivate.ptr =
 			kgem_bo_map__cpu(&sna->kgem, priv->gpu_bo);
 		if (pixmap->devPrivate.ptr != NULL) {
-			assert(has_coherent_map(sna, priv->gpu_bo));
+			assert(has_coherent_map(sna, priv->gpu_bo, flags));
 			assert(IS_CPU_MAP(priv->gpu_bo->map));
 			pixmap->devKind = priv->gpu_bo->pitch;
 			priv->cpu = true;
@@ -3966,13 +3967,7 @@ static bool can_upload_tiled_x(struct kgem *kgem, struct sna_pixmap *priv)
 		return false;
 	}
 
-	if (bo->scanout) {
-		DBG(("%s: no, is scanout\n", __FUNCTION__, bo->scanout));
-		return false;
-	}
-
-	DBG(("%s? domain=%d, has_llc=%d\n", __FUNCTION__, bo->domain, kgem->has_llc));
-	return bo->domain == DOMAIN_CPU || kgem->has_llc;
+	return kgem_bo_can_map__cpu(kgem, bo, true);
 }
 
 static bool
@@ -4972,7 +4967,7 @@ sna_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 
 	hint = source_prefer_gpu(sna, src_priv, region, src_dx, src_dy) ?:
 		region_inplace(sna, dst_pixmap, region,
-			       dst_priv, alu_overwrites(alu));
+			       dst_priv, alu_overwrites(alu) ? MOVE_WRITE : MOVE_READ | MOVE_WRITE);
 	if (dst_priv->cpu_damage && alu_overwrites(alu)) {
 		DBG(("%s: overwritting CPU damage\n", __FUNCTION__));
 		if (region_subsumes_damage(region, dst_priv->cpu_damage)) {
@@ -14409,10 +14404,7 @@ sna_get_image_inplace(DrawablePtr drawable,
 		break;
 	}
 
-	if (priv->gpu_bo->scanout)
-		return false;
-
-	if (!sna->kgem.has_llc && priv->gpu_bo->domain != DOMAIN_CPU)
+	if (!kgem_bo_can_map__cpu(&sna->kgem, priv->gpu_bo, false))
 		return false;
 
 	if (priv->gpu_damage == NULL ||
diff --git a/src/sna/sna_io.c b/src/sna/sna_io.c
index 6ab907f..a4932b8 100644
--- a/src/sna/sna_io.c
+++ b/src/sna/sna_io.c
@@ -53,15 +53,12 @@ static inline bool must_tile(struct sna *sna, int width, int height)
 		upload_too_large(sna, width, height));
 }
 
-static bool bo_inplace_tiled(struct kgem *kgem, struct kgem_bo *bo)
+static bool bo_inplace_tiled(struct kgem *kgem, struct kgem_bo *bo, bool write)
 {
 	if (bo->tiling != I915_TILING_X)
 		return false;
 
-	if (bo->scanout)
-		return false;
-
-	return bo->domain == DOMAIN_CPU || kgem->has_llc;
+	return kgem_bo_can_map__cpu(kgem, bo, write);
 }
 
 static bool download_inplace__tiled(struct kgem *kgem, struct kgem_bo *bo)
@@ -69,7 +66,7 @@ static bool download_inplace__tiled(struct kgem *kgem, struct kgem_bo *bo)
 	if (!kgem->memcpy_from_tiled_x)
 		return false;
 
-	return bo_inplace_tiled(kgem, bo);
+	return bo_inplace_tiled(kgem, bo, false);
 }
 
 static bool
@@ -537,7 +534,7 @@ static bool upload_inplace__tiled(struct kgem *kgem, struct kgem_bo *bo)
 	if (!kgem->memcpy_to_tiled_x)
 		return false;
 
-	return bo_inplace_tiled(kgem, bo);
+	return bo_inplace_tiled(kgem, bo, true);
 }
 
 static bool
commit 9026bb954646c0425360c2236e26c79d097142cd
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jun 28 15:59:17 2013 +0100

    sna: Inspect the dirty boxes when querying whether damage contains a rectangle
    
    This helps in the cases where we have subtracted a small number of
    rectangles from an all-damage pixmap (such as a number of successive
    GetImage, PutImage operations). The danger is that we end up searching a
    long list of dirty boxes - maybe just search the first chunk if that
    becomes noticeable?
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_damage.c b/src/sna/sna_damage.c
index d1ab903..377b5ac 100644
--- a/src/sna/sna_damage.c
+++ b/src/sna/sna_damage.c
@@ -1332,18 +1332,59 @@ int _sna_damage_contains_box(struct sna_damage *damage,
 }
 #endif
 
+static bool box_overlaps(const BoxRec *a, const BoxRec *b)
+{
+	return (a->x1 < b->x2 && a->x2 > b->x1 &&
+		a->y1 < b->y2 && a->y2 > b->y1);
+}
+
 bool _sna_damage_contains_box__no_reduce(const struct sna_damage *damage,
 					 const BoxRec *box)
 {
+	struct sna_damage_box *iter;
+	int ret;
+
 	assert(damage && damage->mode != DAMAGE_ALL);
-	if (damage->mode == DAMAGE_SUBTRACT)
+	if (!sna_damage_overlaps_box(damage, box))
 		return false;
 
-	if (!sna_damage_overlaps_box(damage, box))
+	ret = pixman_region_contains_rectangle(&damage->region, (BoxPtr)box);
+	if (!damage->dirty)
+		return ret == PIXMAN_REGION_IN;
+
+	if (damage->mode == DAMAGE_ADD) {
+		if (ret == PIXMAN_REGION_IN)
+			return true;
+
+		list_for_each_entry(iter, &damage->embedded_box.list, list) {
+			BoxPtr b;
+			int n;
+
+			b = (BoxPtr)(iter + 1);
+			for (n = 0; n < iter->size; n++) {
+				if (box_contains(&b[n], box))
+					return true;
+			}
+		}
+
 		return false;
+	} else {
+		if (ret != PIXMAN_REGION_IN)
+			return false;
+
+		list_for_each_entry(iter, &damage->embedded_box.list, list) {
+			BoxPtr b;
+			int n;
+
+			b = (BoxPtr)(iter + 1);
+			for (n = 0; n < iter->size; n++) {
+				if (box_overlaps(&b[n], box))
+					return false;
+			}
+		}
 
-	return pixman_region_contains_rectangle((RegionPtr)&damage->region,
-						(BoxPtr)box) == PIXMAN_REGION_IN;
+		return true;
+	}
 }
 
 static bool __sna_damage_intersect(struct sna_damage *damage,
commit d635e05c9dd26a397ccf958be091b56d1075e923
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jun 28 19:24:06 2013 +0100

    sna: Promote assert(!priv->mapped) along migration paths
    
    With the advent of the more permissive mapping schemes and finer damage
    tracking, we are more liable to have pixmaps mapped and so can reach the
    upload path with the pixmap still mapped.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index eb588c7..18836c8 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -1578,7 +1578,6 @@ sna_pixmap_undo_cow(struct sna *sna, struct sna_pixmap *priv, unsigned flags)
 	     flags));
 
 	assert(priv->gpu_bo == cow->bo);
-	assert(!priv->mapped);
 	assert(cow->refcnt);
 
 	list_del(&priv->cow_list);
@@ -1670,6 +1669,10 @@ sna_pixmap_undo_cow(struct sna *sna, struct sna_pixmap *priv, unsigned flags)
 		assert(priv->gpu_bo);
 		kgem_bo_destroy(&sna->kgem, priv->gpu_bo);
 		priv->gpu_bo = bo;
+		if (priv->gpu_bo == NULL && priv->mapped) {
+			priv->pixmap->devPrivate.ptr = NULL;
+			priv->mapped = false;
+		}
 	}
 
 	priv->cow = NULL;
@@ -2953,11 +2956,11 @@ sna_pixmap_move_area_to_gpu(PixmapPtr pixmap, const BoxRec *box, unsigned int fl
 							    box, n, 0);
 			}
 			if (!ok) {
-				assert(!priv->mapped);
-				if (pixmap->devPrivate.ptr == NULL) {
+				if (priv->mapped || pixmap->devPrivate.ptr == NULL) {
 					assert(priv->ptr && priv->stride);
 					pixmap->devPrivate.ptr = PTR(priv->ptr);
 					pixmap->devKind = priv->stride;
+					priv->mapped = false;
 				}
 				if (n == 1 && !priv->pinned &&
 				    box->x1 <= 0 && box->y1 <= 0 &&
@@ -2992,11 +2995,11 @@ sna_pixmap_move_area_to_gpu(PixmapPtr pixmap, const BoxRec *box, unsigned int fl
 						    box, 1, 0);
 		}
 		if (!ok) {
-			assert(!priv->mapped);
-			if (pixmap->devPrivate.ptr == NULL) {
+			if (priv->mapped || pixmap->devPrivate.ptr == NULL) {
 				assert(priv->ptr && priv->stride);
 				pixmap->devPrivate.ptr = PTR(priv->ptr);
 				pixmap->devKind = priv->stride;
+				priv->mapped = false;
 			}
 			ok = sna_write_boxes(sna, pixmap,
 					     priv->gpu_bo, 0, 0,
@@ -3023,11 +3026,11 @@ sna_pixmap_move_area_to_gpu(PixmapPtr pixmap, const BoxRec *box, unsigned int fl
 						    box, n, 0);
 		}
 		if (!ok) {
-			assert(!priv->mapped);
-			if (pixmap->devPrivate.ptr == NULL) {
+			if (priv->mapped || pixmap->devPrivate.ptr == NULL) {
 				assert(priv->ptr && priv->stride);
 				pixmap->devPrivate.ptr = PTR(priv->ptr);
 				pixmap->devKind = priv->stride;
+				priv->mapped = false;
 			}
 			ok = sna_write_boxes(sna, pixmap,
 					     priv->gpu_bo, 0, 0,
commit 2d40500851a7c6d857b17258e4989ddf7401cfbc
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jun 28 15:12:13 2013 +0100

    sna: Add asserts around applying clears
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index aeaf8cc..eb588c7 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -2006,7 +2006,7 @@ skip_inplace_map:
 
 		if (priv->clear_color == 0 ||
 		    pixmap->drawable.bitsPerPixel == 8 ||
-		    priv->clear_color == (1 << pixmap->drawable.bitsPerPixel) - 1) {
+		    priv->clear_color == (1 << pixmap->drawable.depth) - 1) {
 			memset(pixmap->devPrivate.ptr, priv->clear_color,
 			       pixmap->devKind * pixmap->drawable.height);
 		} else {
@@ -2254,6 +2254,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 			      pixmap->drawable.height)) {
 		DBG(("%s: pixmap=%ld all damaged on CPU\n",
 		     __FUNCTION__, pixmap->drawable.serialNumber));
+		assert(!priv->clear);
 
 		sna_damage_destroy(&priv->gpu_damage);
 
@@ -2312,6 +2313,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 			pixmap->devKind = priv->gpu_bo->pitch;
 			if (flags & MOVE_WRITE) {
 				if (!DAMAGE_IS_ALL(priv->gpu_damage)) {
+					assert(!priv->clear);
 					sna_damage_add(&priv->gpu_damage, region);
 					if (sna_damage_is_all(&priv->gpu_damage,
 							      pixmap->drawable.width,
@@ -2372,6 +2374,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 			priv->mapped = true;
 			if (flags & MOVE_WRITE) {
 				if (!DAMAGE_IS_ALL(priv->gpu_damage)) {
+					assert(!priv->clear);
 					sna_damage_add(&priv->gpu_damage, region);
 					if (sna_damage_is_all(&priv->gpu_damage,
 							      pixmap->drawable.width,
@@ -2432,6 +2435,8 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 		int n = RegionNumRects(region);
 		BoxPtr box = RegionRects(region);
 
+		assert(DAMAGE_IS_ALL(priv->gpu_damage));
+
 		DBG(("%s: pending clear, doing partial fill\n", __FUNCTION__));
 		if (priv->cpu_bo) {
 			DBG(("%s: syncing CPU bo\n", __FUNCTION__));
@@ -2704,6 +2709,7 @@ out:
 		assert(priv->gpu_bo == NULL || priv->gpu_bo->proxy == NULL);
 		assert(priv->gpu_bo || priv->gpu_damage == NULL);
 		assert(!priv->flush || !list_is_empty(&priv->flush_list));
+		assert(!priv->clear);
 	}
 	if ((flags & MOVE_ASYNC_HINT) == 0 && priv->cpu_bo) {
 		DBG(("%s: syncing cpu bo\n", __FUNCTION__));
@@ -4067,6 +4073,7 @@ try_upload_tiled_x(PixmapPtr pixmap, RegionRec *region,
 	} while (--n);
 
 	if (!DAMAGE_IS_ALL(priv->gpu_damage)) {
+		assert(!priv->clear);
 		if (replaces) {
 			sna_damage_all(&priv->gpu_damage,
 					pixmap->drawable.width,
@@ -4756,6 +4763,7 @@ sna_self_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 		}
 
 		if (!DAMAGE_IS_ALL(priv->gpu_damage)) {
+			assert(!priv->clear);
 			RegionTranslate(region, tx, ty);
 			sna_damage_add(&priv->gpu_damage, region);
 		}
@@ -4993,7 +5001,7 @@ sna_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 					 &region->extents, &damage);
 	if (bo) {
 		if (src_priv && src_priv->clear) {
-			DBG(("%s: applying src clear[%08x] to dst\n",
+			DBG(("%s: applying src clear [%08x] to dst\n",
 			     __FUNCTION__, src_priv->clear_color));
 			if (n == 1) {
 				if (replaces)
@@ -5011,6 +5019,10 @@ sna_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 				}
 
 				if (replaces && bo == dst_priv->gpu_bo) {
+					DBG(("%s: marking dst handle=%d as all clear [%08x]\n",
+					     __FUNCTION__,
+					     dst_priv->gpu_bo->handle,
+					     src_priv->clear_color));
 					dst_priv->clear = true;
 					dst_priv->clear_color = src_priv->clear_color;
 					sna_damage_all(&dst_priv->gpu_damage,
@@ -5308,8 +5320,10 @@ sna_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 			assert(dst_priv->clear == false);
 			dst_priv->cpu = false;
 			if (damage) {
+				assert(!dst_priv->clear);
 				assert(dst_priv->gpu_bo);
 				assert(dst_priv->gpu_bo->proxy == NULL);
+				assert(*damage == dst_priv->gpu_damage);
 				if (replaces) {
 					sna_damage_destroy(&dst_priv->cpu_damage);
 					sna_damage_all(&dst_priv->gpu_damage,
@@ -14294,10 +14308,17 @@ sna_get_image_blt(DrawablePtr drawable,
 		int w = region->extents.x2 - region->extents.x1;
 		int h = region->extents.y2 - region->extents.y1;
 
+		DBG(("%s: applying clear [%08x]\n",
+		     __FUNCTION__, priv->clear_color));
+		assert(DAMAGE_IS_ALL(priv->gpu_damage));
+		assert(priv->cpu_damage == NULL);
+
 		pitch = PixmapBytePad(w, pixmap->drawable.depth);
 		if (priv->clear_color == 0 ||
 		    pixmap->drawable.bitsPerPixel == 8 ||
-		    priv->clear_color == (1U << pixmap->drawable.bitsPerPixel) - 1) {
+		    priv->clear_color == (1U << pixmap->drawable.depth) - 1) {
+			DBG(("%s: memset clear [%02x]\n",
+			     __FUNCTION__, priv->clear_color & 0xff));
 			memset(dst, priv->clear_color, pitch * h);
 		} else {
 			pixman_fill((uint32_t *)dst,
commit e3ad737ef9d33e924b206741949d59224bfef566
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jun 28 15:33:54 2013 +0100

    sna: Use inplace CPU mapping readback for GetImage on linear buffers
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 6e71f7b..aeaf8cc 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -14362,27 +14362,28 @@ sna_get_image_blt(DrawablePtr drawable,
 }
 
 static bool
-sna_get_image_tiled(DrawablePtr drawable,
-		    RegionPtr region,
-		    char *dst,
-		    unsigned flags)
+sna_get_image_inplace(DrawablePtr drawable,
+		      RegionPtr region,
+		      char *dst,
+		      unsigned flags)
 {
 	PixmapPtr pixmap = get_drawable_pixmap(drawable);
 	struct sna_pixmap *priv = sna_pixmap(pixmap);
 	struct sna *sna = to_sna_from_pixmap(pixmap);
 	char *src;
 
-	if (!sna->kgem.memcpy_from_tiled_x)
-		return false;
-
-	if (flags & MOVE_INPLACE_HINT)
-		return false;
-
 	if (priv == NULL || priv->gpu_bo == NULL)
 		return false;
 
-	if (priv->gpu_bo->tiling != I915_TILING_X)
+	switch (priv->gpu_bo->tiling) {
+	case I915_TILING_Y:
 		return false;
+	case I915_TILING_X:
+		if (!sna->kgem.memcpy_from_tiled_x)
+			return false;
+	default:
+		break;
+	}
 
 	if (priv->gpu_bo->scanout)
 		return false;
@@ -14400,19 +14401,32 @@ sna_get_image_tiled(DrawablePtr drawable,
 	if (src == NULL)
 		return false;
 
-	DBG(("%s: download through a tiled CPU map\n", __FUNCTION__));
-
 	kgem_bo_sync__cpu_full(&sna->kgem, priv->gpu_bo, FORCE_FULL_SYNC);
 
-	memcpy_from_tiled_x(&sna->kgem, src, dst,
-			    pixmap->drawable.bitsPerPixel,
-			    priv->gpu_bo->pitch,
-			    PixmapBytePad(region->extents.x2 - region->extents.x1,
-					  drawable->depth),
-			    region->extents.x1, region->extents.y1,
-			    0, 0,
-			    region->extents.x2 - region->extents.x1,
-			    region->extents.y2 - region->extents.y1);
+
+	if (priv->gpu_bo->tiling) {
+		DBG(("%s: download through a tiled CPU map\n", __FUNCTION__));
+		memcpy_from_tiled_x(&sna->kgem, src, dst,
+				    pixmap->drawable.bitsPerPixel,
+				    priv->gpu_bo->pitch,
+				    PixmapBytePad(region->extents.x2 - region->extents.x1,
+						  drawable->depth),
+				    region->extents.x1, region->extents.y1,
+				    0, 0,
+				    region->extents.x2 - region->extents.x1,
+				    region->extents.y2 - region->extents.y1);
+	} else {
+		DBG(("%s: download through a linear CPU map\n", __FUNCTION__));
+		memcpy_blt(src, dst,
+			   pixmap->drawable.bitsPerPixel,
+			   priv->gpu_bo->pitch,
+			   PixmapBytePad(region->extents.x2 - region->extents.x1,
+					 drawable->depth),
+			   region->extents.x1, region->extents.y1,
+			   0, 0,
+			   region->extents.x2 - region->extents.x1,
+			   region->extents.y2 - region->extents.y1);
+	}
 
 	return true;
 }
@@ -14456,7 +14470,7 @@ sna_get_image(DrawablePtr drawable,
 	if (can_blt && sna_get_image_blt(drawable, &region, dst, flags))
 		return;
 
-	if (can_blt && sna_get_image_tiled(drawable, &region, dst, flags))
+	if (can_blt && sna_get_image_inplace(drawable, &region, dst, flags))
 		return;
 
 	if (!sna_drawable_move_region_to_cpu(drawable, &region, flags))
commit 626b5e541663f838475eaef2c1bc3ae4d4848165
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jun 28 14:24:10 2013 +0100

    sna: Add debug control for disabling accelerated GetImage
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 53c8c8f..6e71f7b 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -77,6 +77,7 @@
 #define ACCEL_FILL_SPANS 1
 #define ACCEL_SET_SPANS 1
 #define ACCEL_PUT_IMAGE 1
+#define ACCEL_GET_IMAGE 1
 #define ACCEL_COPY_AREA 1
 #define ACCEL_COPY_PLANE 1
 #define ACCEL_COPY_WINDOW 1
@@ -14429,7 +14430,10 @@ sna_get_image(DrawablePtr drawable,
 	if (!fbDrawableEnabled(drawable))
 		return;
 
-	DBG(("%s (%d, %d)x(%d, %d)\n", __FUNCTION__, x, y, w, h));
+	DBG(("%s: pixmap=%ld (%d, %d)x(%d, %d), format=%d, mask=%lx, depth=%d\n",
+	     __FUNCTION__,
+	     (long)get_drawable_pixmap(drawable)->drawable.serialNumber,
+	     x, y, w, h, format, mask, drawable->depth));
 
 	region.extents.x1 = x + drawable->x;
 	region.extents.y1 = y + drawable->y;
@@ -14437,9 +14441,11 @@ sna_get_image(DrawablePtr drawable,
 	region.extents.y2 = region.extents.y1 + h;
 	region.data = NULL;
 
-	can_blt = format == ZPixmap &&
-		drawable->bitsPerPixel >= 8 &&
-		PM_IS_SOLID(drawable, mask);
+	can_blt = (ACCEL_GET_IMAGE &&
+		   !FORCE_FALLBACK &&
+		   format == ZPixmap &&
+		   drawable->bitsPerPixel >= 8 &&
+		   PM_IS_SOLID(drawable, mask));
 
 	flags = MOVE_READ;
 	if ((w | h) == 1)
commit 2356579cdff36adf58fb69894f646a6e63053a15
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jun 28 13:18:00 2013 +0100

    sna: Assert that the kernel tiling mode matches our bo
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 3859e2d..5c029ad 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -182,6 +182,23 @@ static void debug_alloc__bo(struct kgem *kgem, struct kgem_bo *bo)
 #define debug_alloc__bo(k, b)
 #endif
 
+#ifndef NDEBUG
+static void assert_tiling(struct kgem *kgem, struct kgem_bo *bo)
+{
+	struct drm_i915_gem_get_tiling tiling;
+
+	assert(bo);
+
+	VG_CLEAR(tiling);
+	tiling.handle = bo->handle;
+	tiling.tiling_mode = -1;
+	(void)drmIoctl(kgem->fd, DRM_IOCTL_I915_GEM_GET_TILING, &tiling);
+	assert(tiling.tiling_mode == bo->tiling);
+}
+#else
+#define assert_tiling(kgem, bo)
+#endif
+
 static void kgem_sna_reset(struct kgem *kgem)
 {
 	struct sna *sna = container_of(kgem, struct sna, kgem);
@@ -1573,6 +1590,7 @@ inline static void kgem_bo_move_to_inactive(struct kgem *kgem,
 	assert(!bo->flush);
 	assert(!bo->needs_flush);
 	assert(list_is_empty(&bo->vma));
+	assert_tiling(kgem, bo);
 	ASSERT_IDLE(kgem, bo->handle);
 
 	kgem->need_expire = true;
@@ -1835,6 +1853,7 @@ static void __kgem_bo_destroy(struct kgem *kgem, struct kgem_bo *bo)
 	assert(bo->refcnt == 0);
 	assert(!bo->purged);
 	assert(bo->proxy == NULL);
+	assert_tiling(kgem, bo);
 
 	bo->binding.offset = 0;
 
@@ -3121,6 +3140,7 @@ retry_large:
 				list_del(&bo->request);
 
 			bo->delta = 0;
+			assert_tiling(kgem, bo);
 			return bo;
 
 discard:
@@ -3203,6 +3223,7 @@ discard:
 			     __FUNCTION__, bo->handle, num_pages(bo)));
 			assert(use_active || bo->domain != DOMAIN_GPU);
 			assert(!bo->needs_flush);
+			assert_tiling(kgem, bo);
 			ASSERT_MAYBE_IDLE(kgem, bo->handle, !use_active);
 			return bo;
 		}
@@ -3291,6 +3312,7 @@ discard:
 		assert(list_is_empty(&bo->list));
 		assert(use_active || bo->domain != DOMAIN_GPU);
 		assert(!bo->needs_flush || use_active);
+		assert_tiling(kgem, bo);
 		ASSERT_MAYBE_IDLE(kgem, bo->handle, !use_active);
 		return bo;
 	}
@@ -3621,6 +3643,7 @@ inline int kgem_bo_fenced_size(struct kgem *kgem, struct kgem_bo *bo)
 	unsigned int size;
 
 	assert(bo->tiling);
+	assert_tiling(kgem, bo);
 	assert(kgem->gen < 040);
 
 	if (kgem->gen < 030)
@@ -3671,6 +3694,7 @@ struct kgem_bo *kgem_create_2d(struct kgem *kgem,
 			assert(bo->delta);
 			assert(!bo->purged);
 			assert(!bo->flush);
+			assert_tiling(kgem, bo);
 
 			if (size > num_pages(bo) || num_pages(bo) > 2*size)
 				continue;
@@ -3694,6 +3718,7 @@ struct kgem_bo *kgem_create_2d(struct kgem *kgem,
 			DBG(("  1:from scanout: pitch=%d, tiling=%d, handle=%d, id=%d\n",
 			     bo->pitch, bo->tiling, bo->handle, bo->unique_id));
 			assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+			assert_tiling(kgem, bo);
 			bo->refcnt = 1;
 			return bo;
 		}
@@ -3713,6 +3738,7 @@ struct kgem_bo *kgem_create_2d(struct kgem *kgem,
 			assert(!bo->scanout);
 			assert(bo->refcnt == 0);
 			assert(bo->reusable);
+			assert_tiling(kgem, bo);
 
 			if (kgem->gen < 040) {
 				if (bo->pitch < pitch) {
@@ -3745,6 +3771,7 @@ struct kgem_bo *kgem_create_2d(struct kgem *kgem,
 			DBG(("  1:from active: pitch=%d, tiling=%d, handle=%d, id=%d\n",
 			     bo->pitch, bo->tiling, bo->handle, bo->unique_id));
 			assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+			assert_tiling(kgem, bo);
 			bo->refcnt = 1;
 			bo->flush = true;
 			return bo;
@@ -3756,6 +3783,7 @@ large_inactive:
 			assert(bo->refcnt == 0);
 			assert(bo->reusable);
 			assert(!bo->scanout);
+			assert_tiling(kgem, bo);
 
 			if (size > num_pages(bo))
 				continue;
@@ -3784,6 +3812,7 @@ large_inactive:
 			DBG(("  1:from large inactive: pitch=%d, tiling=%d, handle=%d, id=%d\n",
 			     bo->pitch, bo->tiling, bo->handle, bo->unique_id));
 			assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+			assert_tiling(kgem, bo);
 			bo->refcnt = 1;
 			return bo;
 		}
@@ -3809,6 +3838,7 @@ large_inactive:
 				assert(bo->rq == NULL);
 				assert(list_is_empty(&bo->request));
 				assert(bo->flush == false);
+				assert_tiling(kgem, bo);
 
 				if (size > num_pages(bo)) {
 					DBG(("inactive too small: %d < %d\n",
@@ -3842,6 +3872,7 @@ large_inactive:
 				assert(bo->domain != DOMAIN_GPU);
 				ASSERT_IDLE(kgem, bo->handle);
 				assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+				assert_tiling(kgem, bo);
 				bo->refcnt = 1;
 				return bo;
 			}
@@ -3877,6 +3908,7 @@ search_again:
 			assert(bo->tiling == tiling);
 			assert(bo->flush == false);
 			assert(!bo->scanout);
+			assert_tiling(kgem, bo);
 
 			if (kgem->gen < 040) {
 				if (bo->pitch < pitch) {
@@ -3909,6 +3941,7 @@ search_again:
 			DBG(("  1:from active: pitch=%d, tiling=%d, handle=%d, id=%d\n",
 			     bo->pitch, bo->tiling, bo->handle, bo->unique_id));
 			assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+			assert_tiling(kgem, bo);
 			bo->refcnt = 1;
 			return bo;
 		}
@@ -3921,6 +3954,7 @@ search_again:
 			assert(!bo->scanout);
 			assert(bo->tiling == tiling);
 			assert(bo->flush == false);
+			assert_tiling(kgem, bo);
 
 			if (num_pages(bo) < size)
 				continue;
@@ -3933,6 +3967,7 @@ search_again:
 			DBG(("  1:from active: pitch=%d, tiling=%d, handle=%d, id=%d\n",
 			     bo->pitch, bo->tiling, bo->handle, bo->unique_id));
 			assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+			assert_tiling(kgem, bo);
 			bo->refcnt = 1;
 			return bo;
 		}
@@ -3951,6 +3986,7 @@ search_again:
 					assert(bo->reusable);
 					assert(!bo->scanout);
 					assert(bo->flush == false);
+					assert_tiling(kgem, bo);
 
 					if (num_pages(bo) < size)
 						continue;
@@ -3969,6 +4005,7 @@ search_again:
 					DBG(("  1:from active: pitch=%d, tiling=%d, handle=%d, id=%d\n",
 					     bo->pitch, bo->tiling, bo->handle, bo->unique_id));
 					assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+					assert_tiling(kgem, bo);
 					bo->refcnt = 1;
 					return bo;
 				}
@@ -3992,6 +4029,7 @@ search_again:
 				assert(bo->reusable);
 				assert(!bo->scanout);
 				assert(bo->flush == false);
+				assert_tiling(kgem, bo);
 
 				if (bo->tiling) {
 					if (bo->pitch < pitch) {
@@ -4013,6 +4051,7 @@ search_again:
 				DBG(("  1:from active: pitch=%d, tiling=%d, handle=%d, id=%d\n",
 				     bo->pitch, bo->tiling, bo->handle, bo->unique_id));
 				assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+				assert_tiling(kgem, bo);
 				bo->refcnt = 1;
 				return bo;
 			}
@@ -4033,6 +4072,7 @@ search_inactive:
 		assert(bo->reusable);
 		assert(!bo->scanout);
 		assert(bo->flush == false);
+		assert_tiling(kgem, bo);
 
 		if (size > num_pages(bo)) {
 			DBG(("inactive too small: %d < %d\n",
@@ -4070,6 +4110,7 @@ search_inactive:
 		assert((flags & CREATE_INACTIVE) == 0 || bo->domain != DOMAIN_GPU);
 		ASSERT_MAYBE_IDLE(kgem, bo->handle, flags & CREATE_INACTIVE);
 		assert(bo->pitch*kgem_aligned_height(kgem, height, bo->tiling) <= kgem_bo_size(bo));
+		assert_tiling(kgem, bo);
 		bo->refcnt = 1;
 		return bo;
 	}
@@ -4124,6 +4165,7 @@ create:
 	}
 
 	assert(bytes(bo) >= bo->pitch * kgem_aligned_height(kgem, height, bo->tiling));
+	assert_tiling(kgem, bo);
 
 	debug_alloc__bo(kgem, bo);
 
@@ -4154,6 +4196,7 @@ struct kgem_bo *kgem_create_cpu_2d(struct kgem *kgem,
 			return bo;
 
 		assert(bo->tiling == I915_TILING_NONE);
+		assert_tiling(kgem, bo);
 
 		if (kgem_bo_map__cpu(kgem, bo) == NULL) {
 			kgem_bo_destroy(kgem, bo);
@@ -4175,6 +4218,7 @@ struct kgem_bo *kgem_create_cpu_2d(struct kgem *kgem,
 	bo = search_snoop_cache(kgem, NUM_PAGES(size), 0);
 	if (bo) {
 		assert(bo->tiling == I915_TILING_NONE);
+		assert_tiling(kgem, bo);
 		assert(bo->snoop);
 		bo->refcnt = 1;
 		bo->pitch = stride;
@@ -4188,6 +4232,7 @@ struct kgem_bo *kgem_create_cpu_2d(struct kgem *kgem,
 			return NULL;
 
 		assert(bo->tiling == I915_TILING_NONE);
+		assert_tiling(kgem, bo);
 
 		if (!gem_set_cacheing(kgem->fd, bo->handle, SNOOPED)) {
 			kgem_bo_destroy(kgem, bo);
@@ -4385,6 +4430,7 @@ bool kgem_check_bo_fenced(struct kgem *kgem, struct kgem_bo *bo)
 	if (kgem->aperture + num_pages(bo) > kgem->aperture_high)
 		return false;
 
+	assert_tiling(kgem, bo);
 	if (kgem->gen < 040 && bo->tiling != I915_TILING_NONE) {
 		if (kgem->nfence >= kgem->fence_max)
 			return false;
@@ -4431,6 +4477,7 @@ bool kgem_check_many_bo_fenced(struct kgem *kgem, ...)
 		if (needs_semaphore(kgem, bo))
 			return false;
 
+		assert_tiling(kgem, bo);
 		num_pages += num_pages(bo);
 		num_exec++;
 		if (kgem->gen < 040 && bo->tiling) {
@@ -4618,6 +4665,7 @@ void *kgem_bo_map__async(struct kgem *kgem, struct kgem_bo *bo)
 	assert(bo->proxy == NULL);
 	assert(list_is_empty(&bo->list));
 	assert(!IS_USER_MAP(bo->map));
+	assert_tiling(kgem, bo);
 
 	if (bo->tiling == I915_TILING_NONE && !bo->scanout && kgem->has_llc) {
 		DBG(("%s: converting request for GTT map into CPU map\n",
@@ -4662,6 +4710,7 @@ void *kgem_bo_map(struct kgem *kgem, struct kgem_bo *bo)
 	assert(list_is_empty(&bo->list));
 	assert(!IS_USER_MAP(bo->map));
 	assert(bo->exec == NULL);
+	assert_tiling(kgem, bo);
 
 	if (bo->tiling == I915_TILING_NONE && !bo->scanout &&
 	    (kgem->has_llc || bo->domain == DOMAIN_CPU)) {
@@ -4729,6 +4778,7 @@ void *kgem_bo_map__gtt(struct kgem *kgem, struct kgem_bo *bo)
 	assert(bo->exec == NULL);
 	assert(list_is_empty(&bo->list));
 	assert(!IS_USER_MAP(bo->map));
+	assert_tiling(kgem, bo);
 
 	if (IS_CPU_MAP(bo->map))
 		kgem_bo_release_map(kgem, bo);
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index af68a14..53c8c8f 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -2356,7 +2356,8 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 	    (priv->gpu_bo->domain == DOMAIN_CPU || sna->kgem.has_llc) &&
 	    ((flags & (MOVE_WRITE | MOVE_ASYNC_HINT)) == 0 ||
 	     !__kgem_bo_is_busy(&sna->kgem, priv->gpu_bo))) {
-		DBG(("%s: try to operate inplace (CPU)\n", __FUNCTION__));
+		DBG(("%s: try to operate inplace (CPU), read? %d, write? %d\n",
+		     __FUNCTION__, !!(flags & MOVE_READ), !!(flags & MOVE_WRITE)));
 		assert(priv->cow == NULL || (flags & MOVE_WRITE) == 0);
 
 		assert(!priv->mapped);


More information about the xorg-commit mailing list