[Intel-gfx] [PATCH v3] drm/i915: Move to CPU domain in pwrite/pread

[Ville Syrjälä]

On Fri, Nov 14, 2014 at 05:00:59PM +0000, Chris Wilson wrote:
> On Wed, Nov 12, 2014 at 11:47:14PM +0200, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Currently it's possible to get visible cache dirt on scanout on LLC
> > machines when using pwrite on the future scanout bo if its cache_level
> > is already NONE.
> > 
> > pwrite's "does this need clflush?" checks would decide that no clflush
> > is necessary since the bo isn't currently pinned to the display and LLC
> > makes everything else coherent. The subsequent set_cache_level(NONE)
> > would also do nothing since cache_level is already correct. And hence
> > no clflush will be performed and we flip to a bo which can still have
> > dirty data in the caches.
> > 
> > To correctly track the cache dirtyness move the object to CPU write
> > domain in pwrite. This cures the cache dirt since we explicitly flush
> > the CPU write domain in the pin_to_display path.
> > 
> > Give pread the same treatment simply in the name of symmetry.
> > 
> > v2: Use trace_i915_gem_object_change_domain() and provide some kind
> >     of commit message
> > v3: Don't mark things as clean if we're not sure everything got
> >     flushed (Chris)
> 
> I think we just want to be more conservative during clflushes after
> pwrite:
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 557746b2b72b..e9f98531b9d2 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -75,7 +75,7 @@ static bool cpu_cache_is_coherent(struct drm_device *dev,
>  
>  static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
>  {
> -       if (!cpu_cache_is_coherent(obj->base.dev, obj->cache_level))
> +       if (level != I915_CACHE_NONE)

You mean == ?

And I guess you'd then have to consider WT as well.

It would mean we'd end up clflushing even when not strictly needed. But
maybe that's acceptable.

>                 return true;
>  
>         return obj->pin_display;
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC