diff mbox

[xf86-video-intel] sna: Replace the blt SYNC_COPY assert with a check

Message ID 20180618173943.4562-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä June 18, 2018, 5:39 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My IVB hits the SYNC_COPY assert in prefer_blt_copy() when I force the
use of the software cursor and I move the cursor on top of a dri2
window. Looks like any platform with sna_wait_for_scanline() implemented
should be capable of tripping this assert.

Let's just replace the assert with a check, which is what gen6 already
does. IVB/HSW/BDW have sna_wait_for_scanline() so we'll have to change
the gen7 and gen8 code at least. gen9+ doesn't have sna_wait_for_scanline()
so in theory we could leave that one be, but I like consistency so let's
change that one too.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 src/sna/gen7_render.c | 5 +++--
 src/sna/gen8_render.c | 5 +++--
 src/sna/gen9_render.c | 5 +++--
 3 files changed, 9 insertions(+), 6 deletions(-)

Comments

Chris Wilson June 19, 2018, 9:25 a.m. UTC | #1 
Quoting Ville Syrjala (2018-06-18 18:39:43)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> My IVB hits the SYNC_COPY assert in prefer_blt_copy() when I force the
> use of the software cursor and I move the cursor on top of a dri2
> window. Looks like any platform with sna_wait_for_scanline() implemented
> should be capable of tripping this assert.
> 
> Let's just replace the assert with a check, which is what gen6 already
> does. IVB/HSW/BDW have sna_wait_for_scanline() so we'll have to change
> the gen7 and gen8 code at least. gen9+ doesn't have sna_wait_for_scanline()
> so in theory we could leave that one be, but I like consistency so let's
> change that one too.

Hmm, my memory says that the assertion was concerned with making sure
the wait-for-event + copy operation was in the same batch. At any rate
it seems like the swcursor is breaking a few of my assumptions, so
before acting I'd like to tidy up the validation logic to check if it is
not working as intended.
-Chris
diff mbox

Patch

diff --git a/src/sna/gen7_render.c b/src/sna/gen7_render.c
index 0a3bda768100..68002409a73d 100644
--- a/src/sna/gen7_render.c
+++ b/src/sna/gen7_render.c
@@ -2957,11 +2957,12 @@  prefer_blt_copy(struct sna *sna,
 		struct kgem_bo *dst_bo,
 		unsigned flags)
 {
+	if (flags & COPY_SYNC)
+		return false;
+
 	if (sna->kgem.mode == KGEM_BLT)
 		return true;
 
-	assert((flags & COPY_SYNC) == 0);
-
 	if (untiled_tlb_miss(src_bo) ||
 	    untiled_tlb_miss(dst_bo))
 		return true;
diff --git a/src/sna/gen8_render.c b/src/sna/gen8_render.c
index 69617da561b6..69c29bc6d901 100644
--- a/src/sna/gen8_render.c
+++ b/src/sna/gen8_render.c
@@ -2796,11 +2796,12 @@  prefer_blt_copy(struct sna *sna,
 		struct kgem_bo *dst_bo,
 		unsigned flags)
 {
+	if (flags & COPY_SYNC)
+		return false;
+
 	if (sna->kgem.mode == KGEM_BLT)
 		return true;
 
-	assert((flags & COPY_SYNC) == 0);
-
 	if (untiled_tlb_miss(src_bo) ||
 	    untiled_tlb_miss(dst_bo))
 		return true;
diff --git a/src/sna/gen9_render.c b/src/sna/gen9_render.c
index 505b98afa38c..b9622437897a 100644
--- a/src/sna/gen9_render.c
+++ b/src/sna/gen9_render.c
@@ -2859,11 +2859,12 @@  prefer_blt_copy(struct sna *sna,
 		struct kgem_bo *dst_bo,
 		unsigned flags)
 {
+	if (flags & COPY_SYNC)
+		return false;
+
 	if (sna->kgem.mode == KGEM_BLT)
 		return true;
 
-	assert((flags & COPY_SYNC) == 0);
-
 	if (untiled_tlb_miss(src_bo) ||
 	    untiled_tlb_miss(dst_bo))
 		return true;