IOCTL feature detection (Was: Re: [Intel-gfx] [PATCH 8/8] drm/i915: Allow user to set cache at BO creation)

[Joonas Lahtinen]

(+ Faith and Daniel as they have been involved in previous discussions)

Quoting Jordan Justen (2023-04-24 20:13:00)
> On 2023-04-24 02:08:43, Tvrtko Ursulin wrote:
> > 
> > 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.
> 
> This still leaves the issue of no reasonable detection mechanism for
> the extension.

I remember this exact discussion being repeated at least a few times in
the past 5 years. Previously the conclusion was always that detection by
trying to use the extension leads to least amount of uAPI surface. There
is also no concern of having mismatch in backporting of the query and the
functionality patches.

Why exactly do you think it is more reasonable to do the following?

check_if_extension_query_is_supported();
<check retval>
check_if_extension_xyz_is_supported();
<check retval>

VS

create_[context,buffer,whatever]_with_extension();
<check errno>
destroy_[context,buffer,whatever]();

For contexts and buffers there's no overhead, and we're keeping the uAPI
surface smaller (less bugs, less validation effort). Additionally we
support checking combinations of extensions A, B and C without extra
effort.

If we're not returning enough clear errnos, then that is something to
make sure we do.

Regards, Joonas

> If the discussion gets too complicated, then can we add
> a GET_PARAM for the SET_PAT extension? I'm hoping we could either come
> up with something better reasonably quickly, or i915/Xe can add a new
> param for each new extensions until a better approach is available.
> 
> > 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.
> 
> This seems little better than the "try it, and if it works then it's
> supported".
> 
> I'm not suggesting that userspace should be able to check that
> scenario x+y+z will work, but more a list of extensions that
> conceivably could work. Normally this should just a matter of the
> kernel unconditionally adding the newly implemented extension to the
> list returned in the query call.
> 
> If a GET_PARAM can be made for the PXP case, then it seems like a
> query item returning CONTEXT_CREATE extensions could conditionally
> omit that extension just as easily as implementing the proposed new
> GET_PARAM.
> 
> -Jordan