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

From: Imre Deak <imre.deak@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org,
	Valtteri Rantala <valtteri.rantala@intel.com>,
	Eero Tamminen <eero.t.tamminen@intel.com>,
	Michael T Frederick <michael.t.frederick@intel.com>
Subject: Re: [PATCH v2 2/2] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config
Date: Tue, 26 Apr 2016 16:17:55 +0300	[thread overview]
Message-ID: <1461676675.18080.18.camel@intel.com> (raw)
In-Reply-To: <20160426125726.GJ27856@nuc-i3427.alporthouse.com>

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. 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.

The bit is also bogus, if we wanted snooping via MOCS we'd use the
dedicated HW flag for that.

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.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx