<div dir="ltr"><div>Just to be clear, is this just adding a CLFLUSH or is it actually changing the default caching state of buffers from CACHED to NONE? If it's actually changing the default state, that's going to break userspace badly.</div><div> </div><div>--Jason </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 19, 2019 at 5:21 AM Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Lionel Landwerlin (2019-07-19 11:18:42) > On 18/07/2019 17:54, Chris Wilson wrote: > > Since userspace has the ability to bypass the CPU cache from within its > > unprivileged command stream, we have to flush the CPU cache to memory > > in order to overwrite the previous contents on creation. > > > > Signed-off-by: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> > > Cc: Joonas Lahtinen <<a href="mailto:joonas.lahtinen@linux.intel.com" target="_blank">joonas.lahtinen@linux.intel.com</a>> > > Cc: <a href="http://stablevger.kernel.org" rel="noreferrer" target="_blank">stablevger.kernel.org</a> > > --- > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 26 ++++++----------------- > > 1 file changed, 7 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > index d2a1158868e7..f752b326d399 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > @@ -459,7 +459,6 @@ i915_gem_object_create_shmem(struct drm_i915_private *i915, u64 size) > > { > > struct drm_i915_gem_object *obj; > > struct address_space *mapping; > > - unsigned int cache_level; > > gfp_t mask; > > int ret; > > > > @@ -498,24 +497,13 @@ i915_gem_object_create_shmem(struct drm_i915_private *i915, u64 size) > > obj->write_domain = I915_GEM_DOMAIN_CPU; > > obj->read_domains = I915_GEM_DOMAIN_CPU; > > > > - if (HAS_LLC(i915)) > > - /* On some devices, we can have the GPU use the LLC (the CPU > > - * cache) for about a 10% performance improvement > > - * compared to uncached. Graphics requests other than > > - * display scanout are coherent with the CPU in > > - * accessing this cache. This means in this mode we > > - * don't need to clflush on the CPU side, and on the > > - * GPU side we only need to flush internal caches to > > - * get data visible to the CPU. > > - * > > - * However, we maintain the display planes as UC, and so > > - * need to rebind when first used as such. > > - */ > > - cache_level = I915_CACHE_LLC; > > - else > > - cache_level = I915_CACHE_NONE; > > - > > - i915_gem_object_set_cache_coherency(obj, cache_level); > > + /* > > + * Note that userspace has control over cache-bypass > > + * via its command stream, so even on LLC architectures > > + * we have to flush out the CPU cache to memory to > > + * clear previous contents. > > + */ > > + i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE); > > > > trace_i915_gem_object_create(obj); > > > > Does i915_drm.h needs updating? : > > > /** > * I915_CACHING_CACHED > * > * GPU access is coherent with cpu caches and furthermore the data is > cached in > * last-level caches shared between cpu cores and the gpu GT. Default on > * machines with HAS_LLC. > */ > #define I915_CACHING_CACHED 1 Sneaky. Thanks, -Chris _______________________________________________ Intel-gfx mailing list <a href="mailto:Intel-gfx@lists.freedesktop.org" target="_blank">Intel-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/intel-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/intel-gfx</a></blockquote></div>