From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: Matthew Brost <matthew.brost@intel.com>, intel-xe@lists.freedesktop.org
Subject: Re: [Intel-xe] [PATCH v2 03/27] drm/xe: Take in-syncs into account when num_execs or num_binds == 0
Date: Mon, 13 Nov 2023 13:50:41 +0100 [thread overview]
Message-ID: <e345d385-b692-3ff1-82be-57f6392ca719@linux.intel.com> (raw)
In-Reply-To: <20231107052603.124361-4-matthew.brost@intel.com>
Hi,
On 11/7/23 06:25, Matthew Brost wrote:
> Wait on in-syncs before signaling out-syncs if num_execs or num_binds ==
> 0 in execbuf IOCTL or VM bind IOCTL respectfully.
>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/xe/xe_exec.c | 10 ++++-
> drivers/gpu/drm/xe/xe_sync.c | 75 ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_sync.h | 5 +++
> drivers/gpu/drm/xe/xe_vm.c | 24 ++++++++++--
> 4 files changed, 108 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_exec.c b/drivers/gpu/drm/xe/xe_exec.c
> index 4666f5b145f7..80ee6d8fcf68 100644
> --- a/drivers/gpu/drm/xe/xe_exec.c
> +++ b/drivers/gpu/drm/xe/xe_exec.c
> @@ -238,11 +238,17 @@ int xe_exec_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>
> if (!args->num_batch_buffer) {
> if (!xe_vm_no_dma_fences(vm)) {
> - struct dma_fence *fence =
> - xe_exec_queue_last_fence_get(q, vm);
> + struct dma_fence *fence;
>
> + fence = xe_sync_in_fence_get(syncs, num_syncs, q, vm);
> + if (IS_ERR(fence)) {
> + err = PTR_ERR(fence);
> + goto err_exec;
> + }
> for (i = 0; i < num_syncs; i++)
> xe_sync_entry_signal(&syncs[i], NULL, fence);
> + xe_exec_queue_last_fence_set(q, vm, fence);
> + dma_fence_put(fence);
> }
>
> goto err_exec;
> diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c
> index 2461e7d4814c..6b38c74a1de1 100644
> --- a/drivers/gpu/drm/xe/xe_sync.c
> +++ b/drivers/gpu/drm/xe/xe_sync.c
> @@ -5,6 +5,7 @@
>
> #include "xe_sync.h"
>
> +#include <linux/dma-fence-array.h>
> #include <linux/kthread.h>
> #include <linux/sched/mm.h>
> #include <linux/uaccess.h>
> @@ -14,6 +15,7 @@
> #include <drm/xe_drm.h>
>
> #include "xe_device_types.h"
> +#include "xe_exec_queue.h"
> #include "xe_macros.h"
> #include "xe_sched_job_types.h"
>
> @@ -274,3 +276,76 @@ void xe_sync_entry_cleanup(struct xe_sync_entry *sync)
> if (sync->ufence)
> user_fence_put(sync->ufence);
> }
> +
> +/**
> + * xe_sync_in_fence_get() - Get a fence from syncs, exec queue, and VM
> + * @sync: input syncs
> + * @num_sync: number of syncs
> + * @q: exec queue
> + * @vm: VM
> + *
> + * Get a fence from syncs, exec queue, and VM. If syncs contain more than 1
> + * in-fence create and return a composite fence of all in-fences, if syncs
> + * contain 1 in-fence return in-fence, if no in-fences return last fence on
> + * input exec queue. Caller must drop reference to returned fence.
> + *
> + * Return: fence on success, ERR_PTR(-ENOMEM) on failure
> + */
> +struct dma_fence *
> +xe_sync_in_fence_get(struct xe_sync_entry *sync, int num_sync,
> + struct xe_exec_queue *q, struct xe_vm *vm)
> +{
> + struct dma_fence **fences = NULL;
> + struct dma_fence_array *cf = NULL;
> + struct dma_fence *fence;
> + int i, num_in_fence = 0, current_fence = 0;
> +
> + lockdep_assert_held(&vm->lock);
> +
> + /* Count in-fences */
> + for (i = 0; i < num_sync; ++i) {
> + if (sync[i].fence) {
> + ++num_in_fence;
> + fence = sync[i].fence;
> + }
> + }
> +
> + /* Easy cases... */
> + if (!num_in_fence) {
> + fence = xe_exec_queue_last_fence_get(q, vm);
> + dma_fence_get(fence);
> + return fence;
> + } else if (num_in_fence == 1) {
Don't we need to also wait on the exec_queue last fence in this case,
and the multiple-in-fences below?
Otherwise we only wait for the in-fences but not on currently executing
jobs?
Did you investigate just to forward a non-existing batchbuffer in the
exec case and create a NOP binding job in the bind case?
/Thomas
> + dma_fence_get(fence);
> + return fence;
> + }
> +
> + /* Create composite fence */
> + fences = kmalloc_array(num_in_fence, sizeof(*fences), GFP_KERNEL);
> + if (!fences)
> + return ERR_PTR(-ENOMEM);
> + for (i = 0; i < num_sync; ++i) {
> + if (sync[i].fence) {
> + dma_fence_get(sync[i].fence);
> + fences[current_fence++] = sync[i].fence;
> + }
> + }
> + cf = dma_fence_array_create(num_in_fence, fences,
> + vm->composite_fence_ctx,
> + vm->composite_fence_seqno++,
> + false);
> + if (!cf) {
> + --vm->composite_fence_seqno;
> + goto err_out;
> + }
> +
> + return &cf->base;
> +
> +err_out:
> + while (current_fence)
> + dma_fence_put(fences[--current_fence]);
> + kfree(fences);
> + kfree(cf);
> +
> + return ERR_PTR(-ENOMEM);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sync.h b/drivers/gpu/drm/xe/xe_sync.h
> index 98f02bb34637..c0c8ddac805d 100644
> --- a/drivers/gpu/drm/xe/xe_sync.h
> +++ b/drivers/gpu/drm/xe/xe_sync.h
> @@ -9,8 +9,10 @@
> #include "xe_sync_types.h"
>
> struct xe_device;
> +struct xe_exec_queue;
> struct xe_file;
> struct xe_sched_job;
> +struct xe_vm;
>
> int xe_sync_entry_parse(struct xe_device *xe, struct xe_file *xef,
> struct xe_sync_entry *sync,
> @@ -23,5 +25,8 @@ 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 *
> +xe_sync_in_fence_get(struct xe_sync_entry *sync, int num_sync,
> + struct xe_exec_queue *q, struct xe_vm *vm);
>
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 2f212939d2b5..2a7fa8e2058e 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -3155,12 +3155,28 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> unwind_ops:
> vm_bind_ioctl_ops_unwind(vm, ops, args->num_binds);
> free_syncs:
> - for (i = 0; err == -ENODATA && i < num_syncs; i++) {
> - struct dma_fence *fence =
> - xe_exec_queue_last_fence_get(to_wait_exec_queue(vm, q), vm);
> + if (err == -ENODATA) {
> + struct dma_fence *fence;
>
> - xe_sync_entry_signal(&syncs[i], NULL, fence);
> + fence = xe_sync_in_fence_get(syncs, num_syncs,
> + to_wait_exec_queue(vm, q), vm);
> + if (IS_ERR(fence)) {
> + err = PTR_ERR(fence);
> + goto cleanup_syncs;
> + }
> + for (i = 0; i < num_syncs; i++)
> + xe_sync_entry_signal(&syncs[i], NULL, fence);
> + if (!async) {
> + long timeout = dma_fence_wait(fence, true);
> +
> + if (timeout < 0)
> + err = -EINTR;
> + }
> + xe_exec_queue_last_fence_set(to_wait_exec_queue(vm, q), vm,
> + fence);
> + dma_fence_put(fence);
> }
> +cleanup_syncs:
> while (num_syncs--)
> xe_sync_entry_cleanup(&syncs[num_syncs]);
>
next prev parent reply other threads:[~2023-11-13 12:50 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-07 5:25 [Intel-xe] [PATCH v2 00/27] Refactor VM bind code Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 01/27] drm/xe: Allow num_binds == 0 in VM bind IOCTL Matthew Brost
2023-11-08 10:53 ` Dafna Hirschfeld
2023-11-08 10:55 ` Dafna Hirschfeld
2023-11-09 14:06 ` Matthew Brost
2023-11-08 11:30 ` Lionel Landwerlin
2023-11-09 13:59 ` Matthew Brost
2023-11-10 10:35 ` Thomas Hellström
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 02/27] drm/xe: Allow num_batch_buffer == 0 in exec IOCTL Matthew Brost
2023-11-10 11:11 ` Thomas Hellström
2023-11-10 9:03 ` Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 03/27] drm/xe: Take in-syncs into account when num_execs or num_binds == 0 Matthew Brost
2023-11-13 12:50 ` Thomas Hellström [this message]
2023-11-13 8:28 ` Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 04/27] drm/xe: Lock all gpuva ops during VM bind IOCTL Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 05/27] drm/xe: Add ops_execute function which returns a fence Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 06/27] drm/xe: Move migrate to prefetch to op_lock funtion Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 07/27] drm/xe: Add struct xe_vma_ops abstraction Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 08/27] drm/xe: Update xe_vm_rebind to use dummy VMA operations Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 09/27] drm/xe: Move drm exec loop to vm_bind_ioctl_ops_execute Matthew Brost
2023-11-12 14:26 ` Dafna Hirschfeld
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 10/27] drm/xe: Fixup error handling / ref counting in VM bind IOCTL Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 11/27] drm/xe: Convert pagefault rebind to use ops interface Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 12/27] drm/xe: Add some members to xe_vma_ops Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 13/27] drm/xe: Add vm_bind_ioctl_ops_install_fences helper Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 14/27] drm/xe: Drop rebind argument from xe_pt_prepare_bind Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 15/27] drm/xe: Add xe_vm_pgtable_update_op to xe_vma_ops Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 16/27] drm/xe: Move setting last fence and sync wait to vm_bind_ioctl_ops_install_fences Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 17/27] drm/xe: Fix vma_is_valid to use tile argument Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 18/27] drm/xe: Adjust tile mask in operations create Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 19/27] drm/xe: Add xe_gt_tlb_invalidation_range and convert PT layer to use this Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 20/27] drm/xe: s/xe_tile_migrate_engine/xe_tile_migrate_exec_queue Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 21/27] drm/xe: Convert multiple bind ops into single job Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 22/27] drm/xe: Update clear / populate arguments Matthew Brost
2023-11-07 5:25 ` [Intel-xe] [PATCH v2 23/27] drm/xe: Add __xe_migrate_update_pgtables_cpu helper Matthew Brost
2023-11-07 5:26 ` [Intel-xe] [PATCH v2 24/27] drm/xe: Add xe_hw_fence_signal helper Matthew Brost
2023-11-07 5:26 ` [Intel-xe] [PATCH v2 25/27] drm/xe: CPU binds for jobs Matthew Brost
2023-11-07 5:26 ` [Intel-xe] [PATCH v2 26/27] drm/xe: Don't use migrate exec queue for page fault binds Matthew Brost
2023-11-07 5:26 ` [Intel-xe] [PATCH v2 27/27] drm/xe/uapi: Make sync vs async VM bind operations per IOCTL rather than queue Matthew Brost
2023-11-07 5:29 ` [Intel-xe] ✓ CI.Patch_applied: success for Refactor VM bind code (rev2) Patchwork
2023-11-07 5:30 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-11-07 5:30 ` [Intel-xe] ✗ CI.KUnit: failure " Patchwork
2023-11-08 13:55 ` [Intel-xe] [PATCH v2 00/27] Refactor VM bind code Thomas Hellström
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e345d385-b692-3ff1-82be-57f6392ca719@linux.intel.com \
--to=thomas.hellstrom@linux.intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.brost@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.