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

From: Eero Tamminen <eero.t.tamminen@intel.com>
To: "Chris Wilson" <chris@chris-wilson.co.uk>,
	"Imre Deak" <imre.deak@intel.com>,
	intel-gfx@lists.freedesktop.org,
	"Valtteri Rantala" <valtteri.rantala@intel.com>,
	"Michael T Frederick" <michael.t.frederick@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.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 17:26:43 +0300	[thread overview]
Message-ID: <571F7AA3.8080509@intel.com> (raw)
In-Reply-To: <20160426132321.GL27856@nuc-i3427.alporthouse.com>

Hi,

On 26.04.2016 16:23, 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.
>
>> 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.
>
>> 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?

What this kernel ABI (index entry #2) has been agreed & documented to 
provide?

I thought this entry is supposed to replace the writeback LLC/eLLC cache 
MOCS setting Mesa is using on (e.g. BDW) to speed up accesses to a 
memory area which it knows always to be accessed so that it can be cached.

If app runs on HW where LLC/eLLC is missing, giving the app extra 
slowdown instead of potential speedup sounds like failed HW abstraction. :-)

	- Eero

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