diff mbox

[CI-resend] drm: Restore double clflush on the last partial cacheline

Message ID 1462203651-26457-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 2, 2016, 3:40 p.m. UTC
This effectively reverts

commit afcd950cafea6e27b739fe7772cbbeed37d05b8b
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Jun 10 15:58:01 2015 +0100

    drm: Avoid the double clflush on the last cache line in drm_clflush_virt_range()

as we have observed issues with serialisation of the clflush operations
on Baytrail+ Atoms with partial updates. Applying the double flush on the
last cacheline forces that clflush to be ordered with respect to the
previous clflush, and the mfence then protects against prefetches crossing
the clflush boundary.

The same issue can be demonstrated in userspace with igt/gem_exec_flush.

Fixes: afcd950cafea6 (drm: Avoid the double clflush on the last cache...)
Testcase: igt/gem_concurrent_blit
Testcase: igt/gem_partial_pread_pwrite
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92845
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: dri-devel@lists.freedesktop.org
Cc: Akash Goel <akash.goel@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jason Ekstrand <jason.ekstrand@intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---

Just a quick resend to test this patch against BAT with the new
gem_exec_flush test cases.
-Chris

---
 drivers/gpu/drm/drm_cache.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Chris Wilson May 2, 2016, 4:32 p.m. UTC | #1
On Mon, May 02, 2016 at 04:24:30PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm: Restore double clflush on the last partial cacheline (rev2)
> URL   : https://patchwork.freedesktop.org/series/6573/
> State : failure
> 
> == Summary ==
> 
> Series 6573v2 drm: Restore double clflush on the last partial cacheline
> http://patchwork.freedesktop.org/api/1.0/series/6573/revisions/2/mbox/
> 
> Test gem_exec_flush:
>         Subgroup basic-uc-prw-default:
>                 fail       -> PASS       (bsw-nuc-2)
>         Subgroup basic-uc-rw-default:
>                 fail       -> PASS       (bsw-nuc-2)

Hmm, this should have impacted pro/prw/set as well as -interuptible
variants. I guess some of those are too new to have baselines yet.

Plain old rw should not be affected by the kernel, hopefully it is just
the baseline is older than the userspace w/a. Hopefully with the
baseline reset overnight, we should see a more complete set of results. 
-Chris
Chris Wilson May 2, 2016, 4:45 p.m. UTC | #2
On Mon, May 02, 2016 at 05:32:53PM +0100, Chris Wilson wrote:
> On Mon, May 02, 2016 at 04:24:30PM -0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm: Restore double clflush on the last partial cacheline (rev2)
> > URL   : https://patchwork.freedesktop.org/series/6573/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 6573v2 drm: Restore double clflush on the last partial cacheline
> > http://patchwork.freedesktop.org/api/1.0/series/6573/revisions/2/mbox/
> > 
> > Test gem_exec_flush:
> >         Subgroup basic-uc-prw-default:
> >                 fail       -> PASS       (bsw-nuc-2)
> >         Subgroup basic-uc-rw-default:
> >                 fail       -> PASS       (bsw-nuc-2)
> 
> Hmm, this should have impacted pro/prw/set as well as -interuptible
> variants. I guess some of those are too new to have baselines yet.

It should also be effective for byt...
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 6743ff7dccfa..7f4a6c550319 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -136,6 +136,7 @@  drm_clflush_virt_range(void *addr, unsigned long length)
 		mb();
 		for (; addr < end; addr += size)
 			clflushopt(addr);
+		clflushopt(end - 1); /* force serialisation */
 		mb();
 		return;
 	}