[Intel-xe] [PATCH v2 00/20] uAPI Alignment - take 1 v2
Matt Roper
matthew.d.roper at intel.com
Wed Sep 20 23:07:22 UTC 2023
On Wed, Sep 20, 2023 at 09:54:28PM +0000, Matthew Brost wrote:
> On Wed, Sep 20, 2023 at 02:14:55PM -0700, Matt Roper wrote:
> > As a follow-up to this series I just noticed one other uapi thing that
> > doesn't make sense:
> >
> > struct drm_xe_vm_madvise {
> > ...
> > /*
> > * Setting the preferred location will trigger a migrate of the VMA
> > * backing store to new location if the backing store is already
> > * allocated.
> > *
> > * For DRM_XE_VM_MADVISE_PREFERRED_MEM_CLASS usage, see enum
> > * drm_xe_memory_class.
> > */
> > #define DRM_XE_VM_MADVISE_PREFERRED_MEM_CLASS 0
> > #define DRM_XE_VM_MADVISE_PREFERRED_GT 1
> > /*
> > * In this case lower 32 bits are mem class, upper 32 are GT.
> > * Combination provides a single IOCTL plus migrate VMA to preferred
> > * location.
> > */
> > #define DRM_XE_VM_MADVISE_PREFERRED_MEM_CLASS_GT 2
> >
> > memory-specific stuff like madvise should be operating on tiles, not
> > GTs. From a quick glance at the code, it seems to be comparing the
>
> IMO we expose either GTs or tiless to the uAPI not both. This includes
> the query API, bind, madvise, etc...
>
> If I had to pick one, I'd say we stick with GTs. Internal we then map GT
> to a tile if needed.
I don't understand why we'd want to do this; GTs are unrelated to memory
so it makes it confusing if userspace needs to pass in a
seemingly-unrelated identifier. If we're not going to use the tile
(which at least roughly describes which pool of VRAM we want), then it
seems like a hardware engine would be a little bit less confusing as an
identifier than a GT; the hwe at least reflects how they're likely to
access the memory and is probably more aligned with the way userspace
code is thinking about things.
Matt
>
> Matt
>
> > value it gets against tile_count (which is good), but we still don't
> > want someone passing in a GT ID instead of a tile ID for something like
> > this.
> >
> >
> > Matt
> >
> > On Wed, Sep 20, 2023 at 03:29:20PM -0400, Rodrigo Vivi wrote:
> > > v2:
> > > This v2 has 3 extra patches that was missing on yesterday's submission.
> > > But that aligns with the IGT series sent yesterday.
> > >
> > > Also I'm glad that Mesa reacted super fast and we already have the MR
> > > ready that aligns with v1 sent yesterday:
> > >
> > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25300
> > >
> > > Thank you Jose!
> > >
> > > v1:
> > > As a result of the uAPI review efforts started by Thomas[1],
> > > we have identified many updates on our uAPI that would lead to
> > > breakage in the compatibility. What it is not acceptable after
> > > we are merged upstream. So, let's break it before it is too late,
> > > and start upstreaming a good, reliable and clean uapi.
> > >
> > > Most of this work on putting these patches together for a single
> > > shot was led by Francois.
> > >
> > > The IGT counter part of this series is available as well[2].
> > >
> > > [1] - https://lore.kernel.org/all/863bebd0c624d6fc2b38c0a06b63e468b4185128.camel@l\
> > > inux.intel.com/
> > > [2] - https://lore.kernel.org/all/20230919142000.91363-1-rodrigo.vivi@intel.com
> > >
> > > Ashutosh Dixit (1):
> > > drm/xe/uapi: Use common drm_xe_ext_set_property extension
> > >
> > > Francois Dugast (5):
> > > drm/xe/uapi: Separate VM_BIND's operation and flag
> > > drm/xe/vm: Remove VM_BIND_OP macro
> > > drm/xe/uapi: Remove MMIO ioctl
> > > drm/xe/uapi: Fix naming of XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY
> > > drm/xe/uapi: Add documentation for query
> > >
> > > Matthew Brost (4):
> > > drm/xe: Fix xe_exec_queue_is_idle for parallel exec queues
> > > drm/xe: Deprecate XE_EXEC_QUEUE_SET_PROPERTY_COMPUTE_MODE
> > > implementation
> > > drm/xe: Rename exec_queue_kill_compute to
> > > xe_vm_remove_compute_exec_queue
> > > drm/xe: Remove XE_EXEC_QUEUE_SET_PROPERTY_COMPUTE_MODE from uAPI
> > >
> > > Mika Kuoppala (1):
> > > drm/xe: Extend drm_xe_vm_bind_op
> > >
> > > Rodrigo Vivi (6):
> > > drm/xe: Kill XE_VM_PROPERTY_BIND_OP_ERROR_CAPTURE_ADDRESS extension
> > > drm/xe/uapi: Document drm_xe_query_gt
> > > drm/xe/uapi: Replace useless 'instance' per unique gt_id
> > > drm/xe/uapi: Remove unused field of drm_xe_query_gt
> > > drm/xe/uapi: Rename gts to gt_list
> > > drm/xe/uapi: Crystal Reference Clock updates
> > >
> > > Umesh Nerlige Ramappa (3):
> > > drm/xe: Fix array bounds check for queries
> > > drm/xe: Set the correct type for xe_to_user_engine_class
> > > drm/xe: Correlate engine and cpu timestamps with better accuracy
> > >
> > > drivers/gpu/drm/xe/xe_device.c | 1 -
> > > drivers/gpu/drm/xe/xe_exec_queue.c | 99 ++-----
> > > drivers/gpu/drm/xe/xe_exec_queue_types.h | 6 +-
> > > drivers/gpu/drm/xe/xe_gt_types.h | 2 +-
> > > drivers/gpu/drm/xe/xe_mmio.c | 102 -------
> > > drivers/gpu/drm/xe/xe_pci.c | 4 -
> > > drivers/gpu/drm/xe/xe_query.c | 186 +++++++++++--
> > > drivers/gpu/drm/xe/xe_vm.c | 221 +++++-----------
> > > drivers/gpu/drm/xe/xe_vm.h | 1 +
> > > include/uapi/drm/xe_drm.h | 322 +++++++++++++----------
> > > 10 files changed, 440 insertions(+), 504 deletions(-)
> > >
> > > --
> > > 2.41.0
> > >
> >
> > --
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list