[Intel-xe] [PATCH 00/17] uAPI Alignment - take 1

Zhang, Carl carl.zhang at intel.com
Sun Oct 8 08:06:55 UTC 2023


Hi Rodrigo, 
I checked v5, 
About 
[v5,14/20] drm/xe: Remove async worker and rework sync binds

-#define DRM_XE_ENGINE_CLASS_VM_BIND		5
+#define DRM_XE_ENGINE_CLASS_VM_BIND_ASYNC	5
+#define DRM_XE_ENGINE_CLASS_VM_BIND_SYNC	6


My question is: bind engine is separated to SYNC and ASYNC
Media is using default 0 as binding engine id. It is sync or async?

Thanks
Carl

> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> Sent: Saturday, September 23, 2023 2:56 AM
> To: Zhang, Carl <carl.zhang at intel.com>
> Cc: intel-xe at lists.freedesktop.org; Hazubski, Filip <filip.hazubski at intel.com>;
> Kopryk, Kamil <kamil.kopryk at intel.com>; Chaberek, Jakub
> <jakub.chaberek at intel.com>; Dunajski, Bartosz <bartosz.dunajski at intel.com>;
> Souza, Jose <jose.souza at intel.com>; Yu, Effie <effie.yu at intel.com>
> Subject: Re: [PATCH 00/17] uAPI Alignment - take 1
> 
> On Thu, Sep 21, 2023 at 11:06:24AM -0400, Zhang, Carl wrote:
> > Hi Rodrigo,
> > Have a quick review,  several questions:
> > 1. we need add pad in drm_xe_engine_class_instance, in the future, media
> may need to distinguish slim vdbox and full vdbox , or other special
> configurations.
> > So, reserve one word  should be suitable for future use. and then the
> structure length become 8 bytes.
> 
> it makes sense. let's do that.
> 
> >
> > 2.  no clear description about DRM_XE_DEVICE_QUERY_HWCONFIG, even we
> > know it is in intel_hwconfig_types.h
> 
> Yes, it is that.
> We also need to make that clear.
> 
> >
> > 3.  about query GT, if the there are multiple tile , and it is symmetrical, which
> one will be MAIN, which is REMOTE?
> 
> right now, in an hypothetical scenario where we have 2 tiles with 2 render GTs,
> we would have 2 main GTs and 2 remote GTs. Is this a problem?
> 
> >
> > 4. no method to query HuC readiness .
> 
> We are already discussing this and the trend is to extend the uc_fw_version
> that Jose implemented for guc to also work for huc.
> 
> >
> > 5. XE_QUERY_CONFIG_NUM_PARAM is only used to represent the number of
> query type? And it is actually useless, no one should use it.
> 
> I agree. Let's remove it.
> 
> >
> > 6.
> > +/** struct drm_xe_ext_set_property - XE set property extension */
> > +struct drm_xe_ext_set_property {
> > It should be VM set property extension
> ( XE_VM_EXTENSION_SET_PROPERTY)?
> 
> Please take a look to patch 11
> https://lore.kernel.org/all/20230920192940.135004-12-rodrigo.vivi@intel.com
> It looks like a generic struct for all the possible set_property makes more sense.
> 
> >
> > ps. I still not clear why we need extension for single bind operation.
> > :)
> 
> Every ioctl in xe is extensible by design as documented in the xe_drm.h.
> That apparently matches the Vulkan extensibility and the desire is to maintain
> that everywhere so we minimize the changes of creating new versions of the
> ioctls later.
> 
> > may raise more question in following days.
> 
> Thank you so much for all the review and comments so far.
> 
> >
> > thanks
> > Carl
> >
> > > -----Original Message-----
> > > From: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> > > Sent: Tuesday, September 19, 2023 10:25 PM
> > > To: intel-xe at lists.freedesktop.org
> > > Cc: Hazubski, Filip <filip.hazubski at intel.com>; Kopryk, Kamil
> > > <kamil.kopryk at intel.com>; Chaberek, Jakub
> > > <jakub.chaberek at intel.com>; Dunajski, Bartosz
> > > <bartosz.dunajski at intel.com>; Souza, Jose <jose.souza at intel.com>;
> > > Yu, Effie <effie.yu at intel.com>; Zhang, Carl <carl.zhang at intel.com>;
> > > Vivi, Rodrigo <rodrigo.vivi at intel.com>
> > > Subject: [PATCH 00/17] uAPI Alignment - take 1
> > >
> > > 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
> > > .c
> > > amel at l\
> > > inux.intel.com/
> > > [2] - https://lore.kernel.org/all/20230919142000.91363-1-
> > > rodrigo.vivi at intel.com
> > >
> > > Ashutosh Dixit (1):
> > >   drm/xe/uapi: Use common drm_xe_ext_set_property extension
> > >
> > > Francois Dugast (4):
> > >   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
> > >
> > > 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
> > >
> > > Rodrigo Vivi (5):
> > >   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
> > >
> > > 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            | 187 +++++++++++++--
> > >  drivers/gpu/drm/xe/xe_vm.c               | 221 +++++-------------
> > >  drivers/gpu/drm/xe/xe_vm.h               |   1 +
> > >  include/uapi/drm/xe_drm.h                | 280 ++++++++++++-----------
> > >  10 files changed, 402 insertions(+), 501 deletions(-)
> > >
> > > --
> > > 2.41.0


More information about the Intel-xe mailing list