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

[Eero Tamminen]

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