Re: [Intel-xe] [19/21] drm/xe/uapi: Drop OA_IOCTL_VERSION

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Mon, 23 Oct 2023 21:08:52 -0700, Dixit, Ashutosh wrote:
>
> On Mon, 23 Oct 2023 16:02:53 -0700, Umesh Nerlige Ramappa wrote:
> >
>
> Hi Umesh/Lionel,
>
> > On Fri, Oct 20, 2023 at 10:36:37AM +0300, Lionel Landwerlin wrote:
> > > On 19/09/2023 19:10, Ashutosh Dixit wrote:
> > >> OA version was previously used to track which OA properties were introduced
> > >> at which version. However OA version is an outlier in that a similar
> > >> version is not used anywhere else in the kernel.
> > >>
> > >> For XE, we will track addition of new properties by means of
> > >> xe_user_extension. Userland can either maintain a mapping of OA properties
> > >> against the kernel version, or rely on return codes (e.g. ENOTSUPP) to
> > >> "discover" OA properties.
> > >
> > >
> > > As a userspace driver this terrible.
> > >
> > > You're making us write more code because we have to discover every single
> > > piece of implementation tweaks ever made to the kernel driver.
> > >
> > > Yes OA version was an outlier in that it was better than the rest of
> > > kernel API.
> > >
> > > By reading a single value we could make decisions about how to
> > > communicate with the kernel driver.
> > >
> > >
> > > Having xe_user_extension here is not helping, we still have to fuzz our
> > > way around the kernel to figure out what feature level is available.
> >
> > Hi Lionel, Ashutosh,
> >
> > I don't want to change interfaces to make it more difficult for the
> > UMDs. Do you think capabilities could help achieve what you want?  We could
> > have a bitmask of feature availability and the bitmask could be part of the
> > xe oa query.
> >
> > If that's seen as an unnecessary uApi impact, then I am ok with version
> > being returned in the oa info query.
> >
> > My primary concerns with the version in i915 were
> >
> > (1) version bumps would need documentation even when version bump did not
> > have explicit uApi change. I couldn't find a good place in i915_drm.h to
> > put such documentation. I think capabilities would be self documenting and
> > it would be explicitly added to the uApi header.
> > (2) sometimes before upstreaming a feature, we maintained an internal
> > version for the feature that was using a different range of versions.  This
> > would cause some grief when rebasing since the versions need to be
> > increasing in the order of commits. If we had a bitmask, we can manage it
> > better since there is no need to have an increasing value.
> >
> > Suggestions welcome.
>
> My 2 cents on this is that we should have uniform interfaces across Xe,
> rather than have special treatment for OA. Or is OA special? Why? Because
> we haven't absracted HW well? Why do OA interfaces have to change
> frequently that we are talking about an OA version or OA capabilities when
> other interfaces/subsystems don't seem to need these?
>
> To me it seems capabilities should not be in the uapi since capabilities
> can be constructed entirely in userspace by "probing" for available user
> extensions (so capabilities seem redundant/duplicated). KMD can make it
> easier for UMD's by returning unique return codes when particular
> extensions are not present.

That said, we could probably go with capabilities if needed. An extension N
would be represented by a capability BIT(N). What we don't want to do is
add the same extension twice in xe_drm.h, once as an extension and a second
time as a capability.

>
> I am copying other people who are defining the uapi in Xe for comments,
> hopefully we'll hear something. As I said the best way would be to do
> things uniformly across various subsystems so UMD's can also build and use
> common user xspace API's.
>
> Francois/Matt/Rodrigo: could you please chime in here about how UMD's are
> supposed to handle extensions when uapi changes happen.
>
> Thanks.
> --
> Ashutosh