[Intel-gfx] [PATCH 7/7] drm/i915: Allow user to set cache at BO creation

[Yang, Fei]

> Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915: Allow user to set cache at BO creation
>
> On 2023-04-05 13:26:43, Jordan Justen wrote:
>> On 2023-04-05 00:45:24, Lionel Landwerlin wrote:
>>> On 04/04/2023 19:04, Yang, Fei wrote:
>>>>> Subject: Re: [Intel-gfx] [PATCH 7/7] drm/i915: Allow user to set 
>>>>> cache at BO creation
>>>>>
>>>>> Just like the protected content uAPI, there is no way for 
>>>>> userspace to tell this feature is available other than trying using it.
>>>>>
>>>>> Given the issues with protected content, is it not thing we could want to add?
>>>> Sorry I'm not aware of the issues with protected content, could you elaborate?
>>>> There was a long discussion on teams uAPI channel, could you 
>>>> comment there if any concerns?
>>>>
>>> 
>>> We wanted to have a getparam to detect protected support and were 
>>> told to detect it by trying to create a context with it.
>>> 
>> 
>> An extensions system where the detection mechanism is "just try it", 
>> and assume it's not supported if it fails. ??
>> 
>
> I guess no one wants to discuss the issues with this so-called detection
> mechanism for i915 extensions. (Just try it and if it fails, it must not
 be supported.)
>
> I wonder how many ioctls we will be making a couple years down the road
> just to see what the kernel supports.
>
> Maybe we'll get more fun 8 second timeouts to deal with. Maybe these probing
> ioctls failing or succeeding will alter the kmd's state in some unexpected way.

For this SET_PAT extension, I can assure you there is no 8 second wait :)
This is definitely a non-blocking call.

> It'll also be fun to debug cases where the driver is not starting up with the
> noise of a bunch of probing ioctls flying by.
>
> I thought about suggesting at least something like I915_PARAM_CMD_PARSER_VERSION,
> but I don't know if that could have prevented this 8 second timeout for creating
> a protected content context. Maybe it's better than nothing though.
>
> Of course, there was also the vague idea I threw out below for getting a list of
> supported extentions.

The detection mechanism itself is an uAPI change, I don't think it's a good idea to
combine that with this SET_PAT extension patch.
I suggest we start a discussion in the "i915 uAPI changes" teams channel just like
how we sorted out a solution for this setting cache policy issue. Would that work?

https://teams.microsoft.com/l/channel/19%3af1767bda6734476ba0a9c7d147b928d1%40thread.skype/i915%2520uAPI%2520changes?groupId=379f3ae1-d138-4205-bb65-d4c7d38cb481&tenantId=46c98d88-e344-4ed4-8496-4ed7712e255d

-Fei

> -Jordan
>
>> 
>> This seem likely to get more and more problematic as a detection 
>> mechanism as more extensions are added.
>> 
>> > 
>> > Now it appears trying to create a protected context can block for 
>> > several seconds.
>> > 
>> > Since we have to report capabilities to the user even before it 
>> > creates protected contexts, any app is at risk of blocking.
>> > 
>> 
>> This failure path is not causing any re-thinking about using this as 
>> the extension detection mechanism?
>> 
>> Doesn't the ioctl# + input-struct-size + u64-extension# identify the 
>> extension such that the kernel could indicate if it is supported or 
>> not. (Or, perhaps return an array of the supported extensions so the 
>> umd doesn't have to potentially make many ioctls for each extension of
>> interest.)
>> 
>> -Jordan