[Intel-gfx] [PATCH 4/4] drm/i915: Flush stale cachelines on set-cache-level

[Chris Wilson]

Quoting Daniel Vetter (2019-11-12 09:09:06)
> On Thu, Jul 18, 2019 at 4:54 PM Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Ensure that we flush any cache dirt out to main memory before the user
> > changes the cache-level as they may elect to bypass the cache (even after
> > declaring their access cache-coherent) via use of unprivileged MOCS.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: stable at vger.kernel.org
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > index 2e3ce2a69653..5d41e769a428 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> > @@ -277,6 +277,11 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
> >
> >         list_for_each_entry(vma, &obj->vma.list, obj_link)
> >                 vma->node.color = cache_level;
> > +
> > +       /* Flush any previous cache dirt in case of cache bypass */
> > +       if (obj->cache_dirty & ~obj->cache_coherent)
> > +               i915_gem_clflush_object(obj, I915_CLFLUSH_SYNC);
> 
> I think writing out the bit logic instead of implicitly relying on the
> #defines would be much better, i.e. && !(cache_coherent &
> COHERENT_FOR_READ). Plus I think we only need to set cache_dirty =
> true if we don't flush here already, to avoid double flushing?

No. The mask is being updated, so you need to flush before you lose
track. The cache is then cleared of the dirty bit so won't be flushed
again until dirty and no longer coherent. We need to flag that the page
is no longer coherent at the end of its lifetime (passing back to the
system) to force the flush then.
-Chris