diff mbox series

[2/2] drm/xe: Wire up DRM_XE_VM_CREATE_FLAG_EXTOBJ_BOOKKEEP

Message ID 20240904170500.3303081-3-matthew.brost@intel.com (mailing list archive)
State New, archived
Headers show
Series Fix extobj dma-resv slot usage in XE's exec IOCTL | expand

Commit Message

Matthew Brost Sept. 4, 2024, 5:05 p.m. UTC 
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@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Reported-by: Simona Vetter <simona.vetter@ffwll.ch>
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2673
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
---
 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(-)

Comments

Cavitt, Jonathan Sept. 4, 2024, 5:56 p.m. UTC | #1 
-----Original Message-----
From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Matthew Brost
Sent: Wednesday, September 4, 2024 10:05 AM
To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: simona.vetter@ffwll.ch; boris.brezillon@collabora.com; Landwerlin, Lionel G <lionel.g.landwerlin@intel.com>; Graunke, Kenneth W <kenneth.w.graunke@intel.com>
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@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Reported-by: Simona Vetter <simona.vetter@ffwll.ch>
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2673
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>

Nit/open question below, but nothing blocking:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

> ---
>  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
> 
>
diff mbox series

Patch

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)
 	unsigned long flags;
 
 	/** @composite_fence_ctx: context composite fence */