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

[Jordan Justen]

On 2023-04-25 06:41:54, Joonas Lahtinen wrote:
> (+ 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>

As I've mentioned a couple times, I'm asking for query item that
returns it all the extensions that conceivably could work.

In theory it could be made a single query item which somehow then
enumerates if something is a context extension or bo extension. But,
it seems simpler for all if we just have a couple query items
returning a simple u64 array for the few classes of extensions.

> VS
> 
> create_[context,buffer,whatever]_with_extension();
> <check errno>
> destroy_[context,buffer,whatever]();
> 
> For contexts and buffers there's no overhead,

There's no-overhead to creating and destroying things? I think the 8s
timeout when trying create a protected content context shows it's not
always quite that simple.

Over time userspace will have to continue growing this set of
create/destroy tests as new extensions are added. Simply so we can
discover what extensions are supported.

This doesn't seem like a very robust way to advertise extensions for
an api.

Another point ... say you need to debug why some simple app is failing
to start the driver. One tool might be strace. But now you have a
bunch of noise of calls from the driver creating and destroying things
just to discover what extensions the kernel supports. It would be nice
if all this was handled with a query item vs a bunch of
create/destroys.

> and we're keeping the uAPI surface smaller (less bugs, less
> validation effort).

I understand wanting to keep the kernel uapi and implementation
simple.

Is it too complicated to return a u64 array for a query item
indicating the extensions supported for the various classes of
extensions? I think in most cases it'll just be essentially shuffling
across an array of u64 items. (Since most known extensions will always
be supported by the kernel.)

> Additionally we support checking combinations of extensions A, B and
> C without extra effort.

Regarding combinations of extensions, is that really something
userspace needs to probe? I would think if userspace knows about the
possible extensions, then it's on userspace to use combinations
appropriately.

But, in detecting extensions, it is possible that, say extension B
relies on extension A. Now userspace may have to probe to see if
extension A is supported, and then probe for extension B while using
extension A. Essentially, probing for an extension could become a bit
complicated. Vs the kernel just giving us a u64 array of known
extensions...

-Jordan