sna/dri2: Prevent the sw cursor from copyig to a buffer as we discard it

During swapbuffers, the sw cursor tries to write to the old buffer.
Ordinary this is not an issue as we are discarding it, but under
TearFree that write causes us to instantiate the shadow buffer with a
possible recursion into set_bo and mayhem.

v2:

commit 226a58bc592d4ed305b7ad0e460f1ee2548e0ddf
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sat Apr 4 20:58:24 2015 +0100

    sna/dri2: Prevent the sw cursor from copyig to a buffer as we discard it

Tried to fix it by disabling SourceValidate. However, it a direct hook
into the Damage code by miSprite that triggers the copy. Since there
appears to be no way to intervene, we just mark that copy as internal
and ignore it.

Reported-by: Chris Bainbridge <chris.bainbridge@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=89903
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

3 files changed, 28 insertions, 18 deletions

diff --git a/src/sna/sna.h b/src/sna/sna.h
index a4984849..51533995 100644
--- a/src/sna/sna.h
+++ b/src/sna/sna.h
@@ -265,6 +265,8 @@ struct sna {
 #define AVX 0x80
 #define AVX2 0x100
 
+	bool ignore_copy_area : 1;
+
 	unsigned watch_flush;
 
 	struct timeval timer_tv;
diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 2626988f..f76447bd 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -7207,6 +7207,9 @@ sna_copy_area(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 	if (gc->planemask == 0)
 		return NULL;
 
+	if (sna->ignore_copy_area)
+		return NULL;
+
 	DBG(("%s: src=(%d, %d)x(%d, %d)+(%d, %d) -> dst=(%d, %d)+(%d, %d); alu=%d, pm=%lx, depth=%d\n",
 	     __FUNCTION__,
 	     src_x, src_y, width, height, src->x, src->y,
diff --git a/src/sna/sna_dri2.c b/src/sna/sna_dri2.c
index 2b7187d8..43d47015 100644
--- a/src/sna/sna_dri2.c
+++ b/src/sna/sna_dri2.c
@@ -807,8 +807,8 @@ static void set_bo(PixmapPtr pixmap, struct kgem_bo *bo)
 	struct sna *sna = to_sna_from_pixmap(pixmap);
 	struct sna_pixmap *priv = sna_pixmap(pixmap);
 
-	DBG(("%s: pixmap=%ld, handle=%d\n",
-	     __FUNCTION__, pixmap->drawable.serialNumber, bo->handle));
+	DBG(("%s: pixmap=%ld, handle=%d (old handle=%d)\n",
+	     __FUNCTION__, pixmap->drawable.serialNumber, bo->handle, priv->gpu_bo->handle));
 
 	assert(pixmap->drawable.width * pixmap->drawable.bitsPerPixel <= 8*bo->pitch);
 	assert(pixmap->drawable.height * bo->pitch <= kgem_bo_size(bo));
@@ -818,8 +818,6 @@ static void set_bo(PixmapPtr pixmap, struct kgem_bo *bo)
 	assert(priv->flush);
 
 	if (APPLY_DAMAGE) {
-		ScreenPtr screen = pixmap->drawable.pScreen;
-		SourceValidateProcPtr SourceValidate;
 		RegionRec region;
 
 		/* Post damage on the new front buffer so that listeners, such
@@ -832,17 +830,12 @@ static void set_bo(PixmapPtr pixmap, struct kgem_bo *bo)
 		region.data = NULL;
 
 		/*
-		 * SourceValidate is used by the software cursor code
-		 * to copy the original contents back before the drawing
-		 * operation causing us to instantiate the shadow buffer
-		 * just as we are in the process of swapping it away.
+		 * Eeek, beware the sw cursor copying to the old bo
+		 * causing recursion and mayhem.
 		 */
-		SourceValidate = screen->SourceValidate;
-		screen->SourceValidate = NULL;
-
+		DBG(("%s: marking whole pixmap as damaged\n", __FUNCTION__));
+		sna->ignore_copy_area = true;
 		DamageRegionAppend(&pixmap->drawable, &region);
-
-		screen->SourceValidate = SourceValidate;
 	}
 
 	damage(pixmap, priv, NULL);
@@ -871,8 +864,10 @@ static void set_bo(PixmapPtr pixmap, struct kgem_bo *bo)
 		bo->domain = DOMAIN_NONE;
 	assert(bo->flush);
 
-	if (APPLY_DAMAGE)
+	if (APPLY_DAMAGE) {
 		DamageRegionProcessPending(&pixmap->drawable);
+		sna->ignore_copy_area = false;
+	}
 }
 
 static void sna_dri2_select_mode(struct sna *sna, struct kgem_bo *dst, struct kgem_bo *src, bool sync)
@@ -1163,8 +1158,11 @@ __sna_dri2_copy_region(struct sna *sna, DrawablePtr draw, RegionPtr region,
 		boxes = &clip.extents;
 		n = 1;
 	}
-	if (APPLY_DAMAGE || flags & DRI2_DAMAGE)
+	if (APPLY_DAMAGE || flags & DRI2_DAMAGE) {
+		DBG(("%s: marking region as damaged\n", __FUNCTION__));
+		sna->ignore_copy_area = true;
 		DamageRegionAppend(&pixmap->drawable, region);
+	}
 
 	DBG(("%s: copying [(%d, %d), (%d, %d)]x%d src=(%d, %d), dst=(%d, %d)\n",
 	     __FUNCTION__,
@@ -1194,8 +1192,10 @@ __sna_dri2_copy_region(struct sna *sna, DrawablePtr draw, RegionPtr region,
 		}
 	}
 
-	if (APPLY_DAMAGE || flags & DRI2_DAMAGE)
+	if (APPLY_DAMAGE || flags & DRI2_DAMAGE) {
 		DamageRegionProcessPending(&pixmap->drawable);
+		sna->ignore_copy_area = false;
+	}
 
 	if (clip.data)
 		pixman_region_fini(&clip);
@@ -2045,11 +2045,16 @@ static void sna_dri2_xchg_crtc(struct sna *sna, DrawablePtr draw, xf86CrtcPtr cr
 	     get_window_pixmap(win)->drawable.width,
 	     get_window_pixmap(win)->drawable.height));
 
-	if (APPLY_DAMAGE)
+	if (APPLY_DAMAGE) {
+		DBG(("%s: marking drawable as damaged\n", __FUNCTION__));
+		sna->ignore_copy_area = true;
 		DamageRegionAppend(&win->drawable, &win->clipList);
+	}
 	sna_shadow_set_crtc(sna, crtc, get_private(back)->bo);
-	if (APPLY_DAMAGE)
+	if (APPLY_DAMAGE) {
 		DamageRegionProcessPending(&win->drawable);
+		sna->ignore_copy_area = false;
+	}
 
 	assert(dri2_window(win)->front == NULL);