-----Original Message----- From: Intel-xe <intel-xe-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Matthew Brost Sent: Wednesday, September 4, 2024 10:05 AM To: intel-xe@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx Cc: simona.vetter@xxxxxxxx; boris.brezillon@xxxxxxxxxxxxx; Landwerlin, Lionel G <lionel.g.landwerlin@xxxxxxxxx>; Graunke, Kenneth W <kenneth.w.graunke@xxxxxxxxx> Subject: [PATCH 2/2] drm/xe: Wire up DRM_XE_VM_CREATE_FLAG_EXTOBJ_BOOKKEEP > > Fix external BO's dma-resv usage in exec IOCTL with an opt into bookkeep > slot. This leaves syncing to user space rather than the KMD blindly > enforcing write semantics on every external BO. > > Cc: Kenneth Graunke <kenneth.w.graunke@xxxxxxxxx> > Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx> > Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> > Reported-by: Simona Vetter <simona.vetter@xxxxxxxx> > Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2673 > Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx> Nit/open question below, but nothing blocking: Reviewed-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx> > --- > drivers/gpu/drm/xe/xe_exec.c | 11 +++++++++-- > drivers/gpu/drm/xe/xe_vm.c | 5 ++++- > drivers/gpu/drm/xe/xe_vm_types.h | 5 +++-- > 3 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c > index 7b38485817dc..ea0aba77db84 100644 > --- a/drivers/gpu/drm/xe/xe_exec.c > +++ b/drivers/gpu/drm/xe/xe_exec.c > @@ -302,9 +302,16 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file) > * the job and let the DRM scheduler / backend clean up the job. > */ > xe_sched_job_arm(job); > - if (!xe_vm_in_lr_mode(vm)) > + if (!xe_vm_in_lr_mode(vm)) { > + enum dma_resv_usage extobj_resv_usage = DMA_RESV_USAGE_WRITE; > + > + /* Override original incorrect behavior */ > + if (vm->flags & XE_VM_FLAG_EXTOBJ_BOOKKEEP) > + extobj_resv_usage = DMA_RESV_USAGE_BOOKKEEP; > + > drm_gpuvm_resv_add_fence(&vm->gpuvm, exec, &job->drm.s_fence->finished, > - DMA_RESV_USAGE_BOOKKEEP, DMA_RESV_USAGE_WRITE); > + DMA_RESV_USAGE_BOOKKEEP, extobj_resv_usage); > + } > > for (i = 0; i < num_syncs; i++) { > xe_sync_entry_signal(&syncs[i], &job->drm.s_fence->finished); > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c > index 7acd5fc9d032..a1f98f233c37 100644 > --- a/drivers/gpu/drm/xe/xe_vm.c > +++ b/drivers/gpu/drm/xe/xe_vm.c > @@ -1713,7 +1713,8 @@ find_ufence_get(struct xe_sync_entry *syncs, u32 num_syncs) > > #define ALL_DRM_XE_VM_CREATE_FLAGS (DRM_XE_VM_CREATE_FLAG_SCRATCH_PAGE | \ > DRM_XE_VM_CREATE_FLAG_LR_MODE | \ > - DRM_XE_VM_CREATE_FLAG_FAULT_MODE) > + DRM_XE_VM_CREATE_FLAG_FAULT_MODE | \ > + DRM_XE_VM_CREATE_FLAG_EXTOBJ_BOOKKEEP) > > int xe_vm_create_ioctl(struct drm_device *dev, void *data, > struct drm_file *file) > @@ -1760,6 +1761,8 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data, > flags |= XE_VM_FLAG_LR_MODE; > if (args->flags & DRM_XE_VM_CREATE_FLAG_FAULT_MODE) > flags |= XE_VM_FLAG_FAULT_MODE; > + if (args->flags & DRM_XE_VM_CREATE_FLAG_EXTOBJ_BOOKKEEP) > + flags |= XE_VM_FLAG_EXTOBJ_BOOKKEEP; > > vm = xe_vm_create(xe, flags); > if (IS_ERR(vm)) > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h > index 7f9a303e51d8..b7056d63d8dc 100644 > --- a/drivers/gpu/drm/xe/xe_vm_types.h > +++ b/drivers/gpu/drm/xe/xe_vm_types.h > @@ -162,8 +162,9 @@ struct xe_vm { > #define XE_VM_FLAG_SCRATCH_PAGE BIT(3) > #define XE_VM_FLAG_FAULT_MODE BIT(4) > #define XE_VM_FLAG_BANNED BIT(5) > -#define XE_VM_FLAG_TILE_ID(flags) FIELD_GET(GENMASK(7, 6), flags) > -#define XE_VM_FLAG_SET_TILE_ID(tile) FIELD_PREP(GENMASK(7, 6), (tile)->id) > +#define XE_VM_FLAG_EXTOBJ_BOOKKEEP BIT(6) > +#define XE_VM_FLAG_TILE_ID(flags) FIELD_GET(GENMASK(8, 7), flags) > +#define XE_VM_FLAG_SET_TILE_ID(tile) FIELD_PREP(GENMASK(8, 7), (tile)->id) I don't know about any formatting restrictions, but if you use BIT(8) you could append the new flag to the end of this list instead. -Jonathan Cavitt > unsigned long flags; > > /** @composite_fence_ctx: context composite fence */ > -- > 2.34.1 > >
