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

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

[fixed mailing lists addresses]

On 24/04/2023 09:36, Jordan Justen wrote:
> On 2023-04-23 00:05:06, Yang, Fei wrote:
>>> On 2023-04-20 09:11:18, Yang, Fei wrote:
>>>>> On 20/04/2023 12:39, Andi Shyti wrote:
>>>>>> Hi Fei,
>>>>>>
>>>>>> because this is an API change, we need some more information here.
>>>>>>
>>>>>> First of all you need to CC the userspace guys that have been
>>>>>> working on top of your series and get their ack's.
>>>>>
>>>>> Yes, and a link to a Mesa merge request which uses the uapi should
>>>>> be included.
>>>>
>>>> Working with Mesa team on this, stay tuned.
>>>>
>>>
>>> I would like to see the extension detection issue is handled
>>> before ack'ing this.
>>>
>>> How about a new DRM_I915_QUERY_GEM_CREATE_EXTENSIONS item, that returns
>>> a u64 array of usable extension names for DRM_IOCTL_I915_GEM_CREATE_EXT?
>>
>> I agree a query mechanism is necessary, but that should be generic for all
>> uAPI's, not just for GEM_CREATE.
>>
>>> A similar DRM_I915_QUERY_GEM_CONTEXT_CREATE_EXTENSIONS could also provide
>>> an alternative to Alan's "drm/i915/uapi/pxp: Add a GET_PARAM for PXP",
>>> and more easily allow advertising future new extensions for context/buffer
>>> creation.
>>
>> I think we should have a discussion and come up with a sustainable design for
>> the query uAPI, rather than just put in a quick hack for this.
> 
> I think you are being a bit too quick to dismiss my idea as a quick
> hack... Nevetheless, I would love to hear an alternate suggestion.
> Just as long as it's not, "let's figure this out later, because I need
> to add this feature now".
> 
> I don't think "just try to use it and if it fails, I guess it isn't
> supported" is reasonable. So, if we can't do better, at least add a
> GET_PARAM. Yeah, it's a quick hack, but it's better than nothing.

Being able to "list" supported extensions sounds like a reasonable principle, albeit a departure from the design direction to date. Which means there are probably no quick solutions. Also, AFAIU, only PXP context create is the problematic one, right? Everything else is pretty much instant or delayed allocation so super cheap to probe by attempting to use.

If I got that right and given this series is about drm_i915_gem_create_ext I don't think this side discussion should be blocking it.

Furthermore the PXP context create story is even more complicated, in a way that it is not just about querying whether the extension is supported, but the expensive check is something more complicated.

Going back to implementation details for this proposed new feature, one alternative to query could be something like:

   drm_i915_gem_create_ext.flags |= I915_GEM_CREATE_EXT_FLAG_PROBE_EXTENSIONS;

That would be somewhat more light weight to implement that the i915_query route. And it appears it would work for all ioctls which support extensions apart for i915_context_param_engines.

Regards,

Tvrtko