Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Cc: francois.dugast@intel.com, intel-xe@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [Intel-xe] [19/21] drm/xe/uapi: Drop OA_IOCTL_VERSION
Date: Mon, 23 Oct 2023 21:08:52 -0700	[thread overview]
Message-ID: <87r0lkach7.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <ZTb7nVm/8nLOYiaB@unerlige-ril>

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.

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

  reply	other threads:[~2023-10-24  4:09 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-19 16:10 [Intel-xe] [PATCH 00/21] Add OA functionality to Xe Ashutosh Dixit
2023-09-19 16:10 ` [Intel-xe] [PATCH 01/21] drm/xe/uapi: Introduce OA (observability architecture) uapi Ashutosh Dixit
2023-10-04  0:26   ` Umesh Nerlige Ramappa
2023-10-04  0:36     ` Dixit, Ashutosh
2023-11-04  1:23   ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 02/21] drm/xe/oa: Add OA types Ashutosh Dixit
2023-10-13 17:05   ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 03/21] drm/xe/oa: Add registers and GPU commands used by OA Ashutosh Dixit
2023-10-13 17:06   ` Umesh Nerlige Ramappa
2023-11-17 22:52     ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 04/21] drm/xe/oa: Module init/exit and probe/remove Ashutosh Dixit
2023-10-13 17:50   ` Umesh Nerlige Ramappa
2023-10-20  7:08   ` [Intel-xe] [04/21] " Lionel Landwerlin
2023-10-27 20:28     ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 05/21] drm/xe/oa: Add/remove config ioctl's Ashutosh Dixit
2023-10-13 17:59   ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 06/21] drm/xe/oa: Start implementing OA stream open ioctl Ashutosh Dixit
2023-10-13 18:09   ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 07/21] drm/xe/oa: OA stream initialization Ashutosh Dixit
2023-10-04 15:22   ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 08/21] drm/xe/oa: Expose OA stream fd Ashutosh Dixit
2023-10-13 18:17   ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 09/21] drm/xe/oa: Read file_operation Ashutosh Dixit
2023-10-14  0:56   ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 10/21] drm/xe/oa: Implement queries Ashutosh Dixit
2023-10-14  0:58   ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 11/21] drm/xe/oa: Override GuC RC with OA on PVC Ashutosh Dixit
2023-10-16 17:43   ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 12/21] drm/xe/uapi: "Perf" layer to support multiple perf counter stream types Ashutosh Dixit
2023-10-04  2:13   ` Umesh Nerlige Ramappa
2023-10-05  4:33     ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 13/21] drm/xe/uapi: Multiplex PERF ops through a single PERF ioctl Ashutosh Dixit
2023-10-04  2:23   ` Umesh Nerlige Ramappa
2023-10-05  5:27     ` Dixit, Ashutosh
2023-10-05 15:22       ` Dixit, Ashutosh
2023-10-05 18:27         ` Umesh Nerlige Ramappa
2023-10-05 23:18           ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 14/21] drm/xe/uapi: Simplify OA configs in uapi Ashutosh Dixit
2023-10-04  2:26   ` Umesh Nerlige Ramappa
2023-10-04 15:44     ` Dixit, Ashutosh
2023-10-04 16:13       ` Rodrigo Vivi
2023-09-19 16:10 ` [Intel-xe] [PATCH 15/21] drm/xe/uapi: Remove OA format names from OA uapi Ashutosh Dixit
2023-10-04  2:33   ` Umesh Nerlige Ramappa
2023-10-05  6:13     ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 16/21] drm/xe/oa: Make xe_oa_timestamp_frequency per gt Ashutosh Dixit
2023-09-21 20:45   ` Rodrigo Vivi
2023-09-21 21:58     ` Dixit, Ashutosh
2023-09-22 19:10       ` Rodrigo Vivi
2023-09-19 16:10 ` [Intel-xe] [PATCH 17/21] drm/xe/oa: Remove filtering reports on context id Ashutosh Dixit
2023-10-14  1:01   ` Umesh Nerlige Ramappa
2023-10-20  7:30   ` [Intel-xe] [17/21] " Lionel Landwerlin
2023-10-20 17:00     ` Umesh Nerlige Ramappa
2023-09-19 16:10 ` [Intel-xe] [PATCH 18/21] drm/xe/uapi: More OA uapi fixes/additions Ashutosh Dixit
2023-10-04  0:23   ` Dixit, Ashutosh
2023-10-05 22:33   ` Dixit, Ashutosh
2023-10-12  3:14     ` Umesh Nerlige Ramappa
2023-10-20  7:28   ` [Intel-xe] [18/21] " Lionel Landwerlin
2023-10-27 20:28     ` Dixit, Ashutosh
2023-10-30 10:06       ` Lionel Landwerlin
2023-10-31  2:08         ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 19/21] drm/xe/uapi: Drop OA_IOCTL_VERSION Ashutosh Dixit
2023-09-19 17:02   ` Dixit, Ashutosh
2023-10-04  2:37     ` Umesh Nerlige Ramappa
2023-10-05  3:28       ` Dixit, Ashutosh
2023-10-05 19:35         ` Umesh Nerlige Ramappa
2023-10-20  7:36   ` [Intel-xe] [19/21] " Lionel Landwerlin
2023-10-23 23:02     ` Umesh Nerlige Ramappa
2023-10-24  4:08       ` Dixit, Ashutosh [this message]
2023-10-24 15:54         ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 20/21] drm/xe/uapi: Use OA unit id to identify OA unit Ashutosh Dixit
2023-10-04 22:37   ` Umesh Nerlige Ramappa
2023-10-05  3:04     ` Dixit, Ashutosh
2023-10-05  3:09       ` Dixit, Ashutosh
2023-09-19 16:10 ` [Intel-xe] [PATCH 21/21] drm/xe/uapi: Convert OA property key/value pairs to a struct Ashutosh Dixit
2023-09-21 23:53   ` Dixit, Ashutosh
2023-10-05  5:37     ` Dixit, Ashutosh
2023-10-05 19:26       ` Umesh Nerlige Ramappa
2023-09-19 16:19 ` [Intel-xe] ✓ CI.Patch_applied: success for Add OA functionality to Xe (rev6) Patchwork
2023-09-19 16:19 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-09-19 16:21 ` [Intel-xe] ✓ CI.KUnit: success " Patchwork
2023-09-19 16:28 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-09-19 16:28 ` [Intel-xe] ✗ CI.Hooks: failure " Patchwork
2023-09-19 16:29 ` [Intel-xe] ✓ CI.checksparse: success " Patchwork
2023-09-19 17:04 ` [Intel-xe] ✗ CI.BAT: failure " Patchwork
2023-10-14  1:05 ` [Intel-xe] [PATCH 00/21] Add OA functionality to Xe Umesh Nerlige Ramappa
2023-10-20  7:44 ` Lionel Landwerlin
2023-10-20  7:52   ` Lionel Landwerlin
2023-10-31  6:51     ` Dixit, Ashutosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r0lkach7.wl-ashutosh.dixit@intel.com \
    --to=ashutosh.dixit@intel.com \
    --cc=francois.dugast@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=umesh.nerlige.ramappa@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox