| Message ID | 20171018141944.29358-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 18, 2017 at 03:19:44PM +0100, Chris Wilson wrote: > We never declared that we were about to read from the mmap after copying > into using the BLT (a missed call to prime_sync_start); leaving its > coherency ill-defined. For completeness, add the missing > prime_sync_end() as well. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103168 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > tests/prime_mmap_coherency.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/tests/prime_mmap_coherency.c b/tests/prime_mmap_coherency.c > index a213ac0f..064c61a7 100644 > --- a/tests/prime_mmap_coherency.c > +++ b/tests/prime_mmap_coherency.c > @@ -98,6 +98,8 @@ static int test_read_flush(void) > if (ptr_cpu[i] != 0xc5c5c5c5) > stale++; Shouldn't we add a prime_sync_start/end for the first ptr_cpu access too? Just for over-the-top correctness? I think otherwise you caught them all, so with that one also fixed: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > + prime_sync_end(dma_buf_fd, false); > + > drm_intel_bo_unreference(bo_1); > munmap(ptr_cpu, width * height); > > @@ -141,8 +143,8 @@ static int test_write_flush(void) > /* This is the main point of this test: !llc hw requires a cache write > * flush right here (explained in step #4). */ > prime_sync_start(dma_buf_fd, true); > - > memset(ptr_cpu, 0x11, width * height); > + prime_sync_end(dma_buf_fd, true); > > /* STEP #3: Copy BO 1 into BO 2, using blitter. */ > bo_2 = drm_intel_bo_alloc(bufmgr, "BO 2", width * height * 4, 4096); > @@ -159,10 +161,14 @@ static int test_write_flush(void) > MAP_SHARED, dma_buf2_fd, 0); > igt_assert(ptr2_cpu != MAP_FAILED); > > + prime_sync_start(dma_buf2_fd, false); > + > for (i = 0; i < (width * height) / 4; i++) > if (ptr2_cpu[i] != 0x11111111) > stale++; > > + prime_sync_end(dma_buf2_fd, false); > + > drm_intel_bo_unreference(bo_1); > drm_intel_bo_unreference(bo_2); > munmap(ptr_cpu, width * height); > -- > 2.15.0.rc1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Daniel Vetter (2017-10-18 15:47:27) > On Wed, Oct 18, 2017 at 03:19:44PM +0100, Chris Wilson wrote: > > We never declared that we were about to read from the mmap after copying > > into using the BLT (a missed call to prime_sync_start); leaving its > > coherency ill-defined. For completeness, add the missing > > prime_sync_end() as well. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103168 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > tests/prime_mmap_coherency.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/tests/prime_mmap_coherency.c b/tests/prime_mmap_coherency.c > > index a213ac0f..064c61a7 100644 > > --- a/tests/prime_mmap_coherency.c > > +++ b/tests/prime_mmap_coherency.c > > @@ -98,6 +98,8 @@ static int test_read_flush(void) > > if (ptr_cpu[i] != 0xc5c5c5c5) > > stale++; > > Shouldn't we add a prime_sync_start/end for the first ptr_cpu access too? > Just for over-the-top correctness? The loop where we check the new buffer is zero? Yup, missed that. That works as an interesting check with the mmap held across the blt, with sync's before/after. I suppose as we repeat the test several times, in addition to different bit values, we should also try different patterns of blts. Another day. -Chris
Quoting Patchwork (2017-10-19 01:18:19) > == Series Details == > > Series: igt/prime_mmap_coherency: Call prime_sync_start before read after write > URL : https://patchwork.freedesktop.org/series/32219/ > State : success > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_378/shards.html Oh well, at least it didn't regress on the machines that were run. -Chris
diff --git a/tests/prime_mmap_coherency.c b/tests/prime_mmap_coherency.c index a213ac0f..064c61a7 100644 --- a/tests/prime_mmap_coherency.c +++ b/tests/prime_mmap_coherency.c @@ -98,6 +98,8 @@ static int test_read_flush(void) if (ptr_cpu[i] != 0xc5c5c5c5) stale++; + prime_sync_end(dma_buf_fd, false); + drm_intel_bo_unreference(bo_1); munmap(ptr_cpu, width * height); @@ -141,8 +143,8 @@ static int test_write_flush(void) /* This is the main point of this test: !llc hw requires a cache write * flush right here (explained in step #4). */ prime_sync_start(dma_buf_fd, true); - memset(ptr_cpu, 0x11, width * height); + prime_sync_end(dma_buf_fd, true); /* STEP #3: Copy BO 1 into BO 2, using blitter. */ bo_2 = drm_intel_bo_alloc(bufmgr, "BO 2", width * height * 4, 4096); @@ -159,10 +161,14 @@ static int test_write_flush(void) MAP_SHARED, dma_buf2_fd, 0); igt_assert(ptr2_cpu != MAP_FAILED); + prime_sync_start(dma_buf2_fd, false); + for (i = 0; i < (width * height) / 4; i++) if (ptr2_cpu[i] != 0x11111111) stale++; + prime_sync_end(dma_buf2_fd, false); + drm_intel_bo_unreference(bo_1); drm_intel_bo_unreference(bo_2); munmap(ptr_cpu, width * height);
We never declared that we were about to read from the mmap after copying into using the BLT (a missed call to prime_sync_start); leaving its coherency ill-defined. For completeness, add the missing prime_sync_end() as well. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103168 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- tests/prime_mmap_coherency.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)