Re: [Intel-gfx] [RFC 2/2] drm/i915: Remove PAT hack from i915_gem_object_can_bypass_llc

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 14/07/2023 06:43, Yang, Fei wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> According to the comment in i915_gem_object_can_bypass_llc the
>> purpose of the function is to return false if the platform/object
>> has a caching mode where GPU can bypass the LLC.
>>
>> So far the only platforms which allegedly can do this are Jasperlake
>> and Elkhartlake, and that via MOCS (not PAT).
>>
>> Instead of blindly assuming that objects where userspace has set the
>> PAT index can (bypass the LLC), question is is there a such PAT index
>> on a platform. Probably starting with Meteorlake since that one is the
>> only one where set PAT extension can be currently used. Or if there is
>> a MOCS entry which can achieve the same thing on Meteorlake.
>>
>> If there is such PAT, now that i915 can be made to understand them
>> better, we can make the check more fine grained. Or if there is a MOCS
>> entry then we probably should apply the blanket IS_METEORLAKE condition.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Fixes: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
>> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
>> Cc: Fei Yang <fei.yang@intel.com>
>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_object.c | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> index 33a1e97d18b3..1e34171c4162 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> @@ -229,12 +229,6 @@ bool i915_gem_object_can_bypass_llc(struct drm_i915_gem_object *obj)
>>        if (!(obj->flags & I915_BO_ALLOC_USER))
>>                return false;
>>
>> -     /*
>> -      * Always flush cache for UMD objects at creation time.
>> -      */
>> -     if (obj->pat_set_by_user)
> 
> I'm afraid this is going to break MESA. Can we run MESA tests with this patch?

I can't, but question is why it would break Mesa which would need a nice 
comment here?

For instance should the check be IS_METEORLAKE?

Or should it be "is wb" && "not has 1-way coherent"?

Or both?

Or, given how Meteorlake does not have LLC, how can anything bypass it 
there? Or is it about snooping on Meteorlake and how?

Regards,

Tvrtko

> 
>>        /*
>>         * EHL and JSL add the 'Bypass LLC' MOCS entry, which should make it
>>         * possible for userspace to bypass the GTT caching bits set by the
>> --
>> 2.39.2