[PATCH] drm/xe: Create a helper function to init job's user fence
Lucas De Marchi
lucas.demarchi at intel.com
Thu Mar 21 14:43:55 UTC 2024
On Wed, Mar 20, 2024 at 01:45:56PM +0100, Nirmoy Das wrote:
>Refactor xe_sync_entry_signal so it doesn't have to
>modify xe_sched_job struct instead create a new helper function
>to set user fence values for a job.
>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Matthew Auld <matthew.auld at intel.com>
>Cc: Matthew Brost <matthew.brost at intel.com>
>Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>Signed-off-by: Nirmoy Das <nirmoy.das at intel.com>
>---
> drivers/gpu/drm/xe/xe_exec.c | 10 ++++++----
> drivers/gpu/drm/xe/xe_sched_job.c | 14 ++++++++++++++
> drivers/gpu/drm/xe/xe_sched_job.h | 3 +++
> drivers/gpu/drm/xe/xe_sync.c | 7 +------
> drivers/gpu/drm/xe/xe_sync.h | 1 -
> drivers/gpu/drm/xe/xe_vm.c | 10 +++++-----
> 6 files changed, 29 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c
>index 7692ebfe7d47..1fb4dd4d0557 100644
>--- a/drivers/gpu/drm/xe/xe_exec.c
>+++ b/drivers/gpu/drm/xe/xe_exec.c
>@@ -249,7 +249,7 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> goto err_unlock_list;
> }
> for (i = 0; i < num_syncs; i++)
>- xe_sync_entry_signal(&syncs[i], NULL, fence);
>+ xe_sync_entry_signal(&syncs[i], fence);
> xe_exec_queue_last_fence_set(q, vm, fence);
> dma_fence_put(fence);
> }
>@@ -359,9 +359,11 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> drm_gpuvm_resv_add_fence(&vm->gpuvm, exec, &job->drm.s_fence->finished,
> DMA_RESV_USAGE_BOOKKEEP, DMA_RESV_USAGE_WRITE);
>
>- for (i = 0; i < num_syncs; i++)
>- xe_sync_entry_signal(&syncs[i], job,
>- &job->drm.s_fence->finished);
>+ for (i = 0; i < num_syncs; i++) {
>+ xe_sync_entry_signal(&syncs[i], &job->drm.s_fence->finished);
>+ if (syncs[i].type == DRM_XE_SYNC_TYPE_USER_FENCE)
>+ xe_sched_job_init_user_fence(job, &syncs[i]);
>+ }
>
> if (xe_exec_queue_is_lr(q))
> q->ring_ops->emit_job(job);
>diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
>index 8151ddafb940..734900b9c7d2 100644
>--- a/drivers/gpu/drm/xe/xe_sched_job.c
>+++ b/drivers/gpu/drm/xe/xe_sched_job.c
>@@ -17,6 +17,7 @@
> #include "xe_macros.h"
> #include "xe_trace.h"
> #include "xe_vm.h"
>+#include "xe_sync_types.h"
>
> static struct kmem_cache *xe_sched_job_slab;
> static struct kmem_cache *xe_sched_job_parallel_slab;
>@@ -278,6 +279,19 @@ int xe_sched_job_last_fence_add_dep(struct xe_sched_job *job, struct xe_vm *vm)
> return drm_sched_job_add_dependency(&job->drm, fence);
> }
>
>+/**
>+ * xe_sched_job_init_user_fence - Initialize user_fence for the job
>+ * @job: job whose user_fence needs an init:w
>+ * @sync: sync to be use to init user_fence
>+ */
>+void xe_sched_job_init_user_fence(struct xe_sched_job *job,
>+ struct xe_sync_entry *sync)
>+{
here we need either an assert that sync->type == DRM_XE_SYNC_TYPE_USER_FENCE,
or we remove the check in the caller and just silently return
if type doesn't match.
otherwise LGTM.
Lucas De Marchi
>+ job->user_fence.used = true;
>+ job->user_fence.addr = sync->addr;
>+ job->user_fence.value = sync->timeline_value;
>+}
>+
> struct xe_sched_job_snapshot *
> xe_sched_job_snapshot_capture(struct xe_sched_job *job)
> {
>diff --git a/drivers/gpu/drm/xe/xe_sched_job.h b/drivers/gpu/drm/xe/xe_sched_job.h
>index f1a660648cf0..c75018f4660d 100644
>--- a/drivers/gpu/drm/xe/xe_sched_job.h
>+++ b/drivers/gpu/drm/xe/xe_sched_job.h
>@@ -10,6 +10,7 @@
>
> struct drm_printer;
> struct xe_vm;
>+struct xe_sync_entry;
>
> #define XE_SCHED_HANG_LIMIT 1
> #define XE_SCHED_JOB_TIMEOUT LONG_MAX
>@@ -58,6 +59,8 @@ void xe_sched_job_arm(struct xe_sched_job *job);
> void xe_sched_job_push(struct xe_sched_job *job);
>
> int xe_sched_job_last_fence_add_dep(struct xe_sched_job *job, struct xe_vm *vm);
>+void xe_sched_job_init_user_fence(struct xe_sched_job *job,
>+ struct xe_sync_entry *sync);
>
> static inline struct xe_sched_job *
> to_xe_sched_job(struct drm_sched_job *drm)
>diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
>index 02c9577fe418..65f1f1628235 100644
>--- a/drivers/gpu/drm/xe/xe_sync.c
>+++ b/drivers/gpu/drm/xe/xe_sync.c
>@@ -224,8 +224,7 @@ int xe_sync_entry_add_deps(struct xe_sync_entry *sync, struct xe_sched_job *job)
> return 0;
> }
>
>-void xe_sync_entry_signal(struct xe_sync_entry *sync, struct xe_sched_job *job,
>- struct dma_fence *fence)
>+void xe_sync_entry_signal(struct xe_sync_entry *sync, struct dma_fence *fence)
> {
> if (!(sync->flags & DRM_XE_SYNC_FLAG_SIGNAL))
> return;
>@@ -254,10 +253,6 @@ void xe_sync_entry_signal(struct xe_sync_entry *sync, struct xe_sched_job *job,
> user_fence_put(sync->ufence);
> dma_fence_put(fence);
> }
>- } else if (sync->type == DRM_XE_SYNC_TYPE_USER_FENCE) {
>- job->user_fence.used = true;
>- job->user_fence.addr = sync->addr;
>- job->user_fence.value = sync->timeline_value;
> }
> }
>
>diff --git a/drivers/gpu/drm/xe/xe_sync.h b/drivers/gpu/drm/xe/xe_sync.h
>index 0fd0d51208e6..3e03396af2c6 100644
>--- a/drivers/gpu/drm/xe/xe_sync.h
>+++ b/drivers/gpu/drm/xe/xe_sync.h
>@@ -26,7 +26,6 @@ int xe_sync_entry_wait(struct xe_sync_entry *sync);
> int xe_sync_entry_add_deps(struct xe_sync_entry *sync,
> struct xe_sched_job *job);
> void xe_sync_entry_signal(struct xe_sync_entry *sync,
>- struct xe_sched_job *job,
> struct dma_fence *fence);
> void xe_sync_entry_cleanup(struct xe_sync_entry *sync);
> struct dma_fence *
>diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>index 80d43d75b1da..497e49b867fc 100644
>--- a/drivers/gpu/drm/xe/xe_vm.c
>+++ b/drivers/gpu/drm/xe/xe_vm.c
>@@ -1696,7 +1696,7 @@ xe_vm_unbind_vma(struct xe_vma *vma, struct xe_exec_queue *q,
> xe_exec_queue_last_fence_get(wait_exec_queue, vm) : fence;
> if (last_op) {
> for (i = 0; i < num_syncs; i++)
>- xe_sync_entry_signal(&syncs[i], NULL, fence);
>+ xe_sync_entry_signal(&syncs[i], fence);
> }
>
> return fence;
>@@ -1770,7 +1770,7 @@ xe_vm_bind_vma(struct xe_vma *vma, struct xe_exec_queue *q,
>
> if (last_op) {
> for (i = 0; i < num_syncs; i++)
>- xe_sync_entry_signal(&syncs[i], NULL,
>+ xe_sync_entry_signal(&syncs[i],
> cf ? &cf->base : fence);
> }
>
>@@ -1831,7 +1831,7 @@ static int __xe_vm_bind(struct xe_vm *vm, struct xe_vma *vma,
> fence = xe_exec_queue_last_fence_get(wait_exec_queue, vm);
> if (last_op) {
> for (i = 0; i < num_syncs; i++)
>- xe_sync_entry_signal(&syncs[i], NULL, fence);
>+ xe_sync_entry_signal(&syncs[i], fence);
> }
> }
>
>@@ -2052,7 +2052,7 @@ static int xe_vm_prefetch(struct xe_vm *vm, struct xe_vma *vma,
> struct dma_fence *fence =
> xe_exec_queue_last_fence_get(wait_exec_queue, vm);
>
>- xe_sync_entry_signal(&syncs[i], NULL, fence);
>+ xe_sync_entry_signal(&syncs[i], fence);
> dma_fence_put(fence);
> }
> }
>@@ -2930,7 +2930,7 @@ static int vm_bind_ioctl_signal_fences(struct xe_vm *vm,
> return PTR_ERR(fence);
>
> for (i = 0; i < num_syncs; i++)
>- xe_sync_entry_signal(&syncs[i], NULL, fence);
>+ xe_sync_entry_signal(&syncs[i], fence);
>
> xe_exec_queue_last_fence_set(to_wait_exec_queue(vm, q), vm,
> fence);
>--
>2.42.0
>
More information about the Intel-xe
mailing list