[PATCH] drm: Restore double clflush on the last partial cacheline

[PATCH] drm: Restore double clflush on the last partial cacheline

[Chris Wilson]

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
---
 drivers/gpu/drm/drm_cache.c | 1 +
 1 file changed, 1 insertion(+)

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;
 	}
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.BAT: success for drm: Restore double clflush on the last partial cacheline

[Patchwork]

== Series Details ==

Series: drm: Restore double clflush on the last partial cacheline
URL   : https://patchwork.freedesktop.org/series/6573/
State : success

== Summary ==

Series 6573v1 drm: Restore double clflush on the last partial cacheline
http://patchwork.freedesktop.org/api/1.0/series/6573/revisions/1/mbox/


bdw-nuci7-2      total:203  pass:191  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:202  pass:161  dwarn:0   dfail:0   fail:0   skip:41 
byt-nuc          total:202  pass:161  dwarn:0   dfail:0   fail:0   skip:41 
ivb-t430s        total:203  pass:172  dwarn:0   dfail:0   fail:0   skip:31 
skl-i7k-2        total:203  pass:176  dwarn:0   dfail:0   fail:0   skip:27 
snb-dellxps      total:203  pass:161  dwarn:0   dfail:0   fail:0   skip:42 
snb-x220t        total:203  pass:161  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/Patchwork_2121/

4acfa93fec0ce38d4efdb0acf0081c39338833c8 drm-intel-nightly: 2016y-04m-29d-16h-48m-26s UTC integration manifest
fd0ccb4 drm: Restore double clflush on the last partial cacheline

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Restore double clflush on the last partial cacheline

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> [ text/plain ]
> 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>

> ---
>  drivers/gpu/drm/drm_cache.c | 1 +
>  1 file changed, 1 insertion(+)
>
> 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;
>  	}
> -- 
> 2.8.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Restore double clflush on the last partial cacheline

[Chris Wilson]

On Sun, May 01, 2016 at 09:15:03AM +0100, Chris Wilson wrote:
> 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.

Fwiw, longer test cycles still show an issue along the pread/pwrite
paths. Not yet convinced if this is the only issue or if it is just
paper (though seemingly very pretty paper).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre