Re: [Intel-xe] [PATCH v2 00/20] uAPI Alignment - take 1 v2

[Matthew Brost <matthew.brost@intel.com>]

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.

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