[Intel-gfx] [PATCH v2 2/2] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config

[Imre Deak]

On ti, 2016-04-26 at 14:23 +0100, Chris Wilson wrote:
> On Tue, Apr 26, 2016 at 04:17:55PM +0300, Imre Deak wrote:
> > On ti, 2016-04-26 at 13:57 +0100, Chris Wilson wrote:
> > > On Tue, Apr 26, 2016 at 03:44:22PM +0300, Imre Deak wrote:
> > > > Setting a write-back cache policy in the MOCS entry definition
> > > > also
> > > > implies snooping, which has a considerable overhead. This is
> > > > unexpected for a few reasons:
> > > 
> > > If it is snooping, then I don't see why it is undesirable to have
> > > it
> > > available in a mocs setting. If it is bogus and the bit is
> > > undefined,
> > > then by all means remove it.
> > 
> > None of these entries are used alone for coherent surfaces. For
> > that
> > the application would have to use entry index#1 or #2 _and_ call
> > the
> > set caching IOCTL to set the corresponding buffer to be cached.
> 
> No, the application doesn't. There are sufficent interfaces exposed
> that
> userspace can bypass the kernel if it so desired.
> 
> > The
> > problem is that without setting the buffer to be cacheable the
> > expectation is that we won't be snooping and incur the
> > corresponding
> > overhead. This is what this patch addresses.
> 
> Not true.

This is what we get running basic tests with current Mesa and UFO
drivers. They use entry #2 and do not expect snooping and the incurred
overhead. If they wanted a coherent surface they would have to call the
set caching IOCTL at least on VLV/CHV since on those platforms you
don't have a snooping flag in the MOCS, you have to set up the PTE
accordingly.

> > The bit is also bogus, if we wanted snooping via MOCS we'd use the
> > dedicated HW flag for that.
> 
> But you keep saying this bit *enables* snooping. So either it does or
> it doesn't.

It enables snooping but that's just a side effect. WB certainly doesn't
make sense for BXT since this WB flag controls the cacheability in
(e)LLC which doesn't exist on BXT. We'd use the 'snooping' HW MOCS flag
for this purpose.

> > If we wanted to have a snooping MOCS entry we should add that
> > separately (as a forth entry), but we'd need this change as a fix
> > for
> > current users.
> 
> The current users who are getting what they request but don't know
> what they were requesting?

They were requesting a non-coherent surface via MOCS entry #2.

--Imre