[Intel-gfx] [PATCH] drm/i915: Flush chipset caches after GGTT writes

[Chris Wilson]

Quoting Rodrigo Vivi (2018-07-17 16:13:41)
> On Tue, Jul 17, 2018 at 03:37:13PM +0100, Chris Wilson wrote:
> > Quoting Rodrigo Vivi (2018-07-17 15:32:08)
> > > On Tue, Jul 17, 2018 at 10:26:55AM +0100, Chris Wilson wrote:
> > > > Our I915g (early gen3, the oldest machine we have in the farm) is still
> > > > reporting occasional incoherency performing the following operations:
> > > > 
> > > >   1) write through GGTT (indirect write into memory)
> > > >   2) write through either CPU or WC (direct write into memory)
> > > >   3) read from GGTT (indirect read)
> > > > 
> > > > Instead of reporting the value from (2), the read from GGTT reports the
> > > > earlier value written via the GGTT. We have made sure that the writes are
> > > > flushed from the CPU (commit 3a32497f0dbe ("drm/i915/selftests: Provide
> > > > full mb() around clflush") and commit add00e6d896f ("drm/i915: Flush the
> > > > WCB following a WC write")), but still see the error, just less
> > > > frequently. The only remaining cache that might be affected here is a
> > > > chipset cache, so flush that as well.
> > > > 
> > > > Testcase: igt/drv_selftest/live_coherency #gdg
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_gem.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > > index 42d24410a98c..08266791801e 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -802,7 +802,7 @@ void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv)
> > > >        * that was!).
> > > >        */
> > > >  
> > > > -     wmb();
> > > > +     i915_gem_chipset_flush(dev_priv);
> > > 
> > > this seems a void change for me... because I couldn't find the implementation
> > > of intel_gtt_chipset_flush() so it seems that we are just replacing wmb per wmb
> > 
> > For gen3, it triggers a write into the igfx flush page via
> > agp/intel-gtt.c, see i9xx_chipset_flush().
> 
> thanks for the pointer... now it makes sense
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

Sadly the only way to find out if this is the final piece of the puzzle
is to soak test it in CI, so in goes.

Thanks,
-Chris