From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH v4 1/2] drm-uapi/xe: add exec_queue_id member to drm_xe_wait_user_fence structure
Date: Wed, 6 Dec 2023 12:04:24 -0500 [thread overview]
Message-ID: <ZXCpmPTieKBeE387@intel.com> (raw)
In-Reply-To: <20231206144452.19745-2-krishnaiah.bommu@intel.com>
On Wed, Dec 06, 2023 at 08:14:50PM +0530, Bommu Krishnaiah wrote:
> remove the num_engines/instances members from drm_xe_wait_user_fence structure
> and add a exec_queue_id member
>
> Right now this is only checking if the engine list is sane and nothing
> else. In the end every operation with this IOCTL is a soft check.
> So, let's formalize that and only use this IOCTL to wait on the fence.
>
> exec_queue_id member will help to user space to get proper error code
> from kernel while in exec_queue reset
>
> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Francois Dugast <francois.dugast@intel.com>
> ---
> include/drm-uapi/xe_drm.h | 16 ++----
Since you are touching the header file it would be good if you could
add a mention to the commit message of which kernel patch you were
when you aligned this header.
Something like xe_drm.h aligns with kernel commit ("drm/xe/uapi: Return
correct error code for xe_wait_user_fence_ioctl")
(no need to add the commit hash since it is still volatile in drm-xe-next)
Also ensure that you do not copy the header directly, but always make it
from the kernel and then copy the 'built' one.
make headers_install INSTALL_HDR_PATH=/tmp/blah
cp /tmp/blah/include/drm/xe_drm.h ./include/drm-uapi/xe_drm.h
> lib/xe/xe_ioctl.c | 42 +-------------
> lib/xe/xe_ioctl.h | 6 +-
> tests/intel/xe_evict.c | 4 +-
> tests/intel/xe_exec_balancer.c | 12 ++--
> tests/intel/xe_exec_compute_mode.c | 12 ++--
> tests/intel/xe_exec_fault_mode.c | 14 ++---
> tests/intel/xe_exec_reset.c | 6 +-
> tests/intel/xe_exec_threads.c | 12 ++--
> tests/intel/xe_waitfence.c | 88 ++++++++++++++----------------
> 10 files changed, 82 insertions(+), 130 deletions(-)
>
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> index 590f7b7af..ceaca3991 100644
> --- a/include/drm-uapi/xe_drm.h
> +++ b/include/drm-uapi/xe_drm.h
> @@ -1024,8 +1024,7 @@ struct drm_xe_wait_user_fence {
> /** @op: wait operation (type of comparison) */
> __u16 op;
>
> -#define DRM_XE_UFENCE_WAIT_FLAG_SOFT_OP (1 << 0) /* e.g. Wait on VM bind */
> -#define DRM_XE_UFENCE_WAIT_FLAG_ABSTIME (1 << 1)
> +#define DRM_XE_UFENCE_WAIT_FLAG_ABSTIME (1 << 0)
> /** @flags: wait flags */
> __u16 flags;
>
> @@ -1059,16 +1058,13 @@ struct drm_xe_wait_user_fence {
> __s64 timeout;
>
> /**
> - * @num_engines: number of engine instances to wait on, must be zero
> - * when DRM_XE_UFENCE_WAIT_FLAG_SOFT_OP set
> + * @exec_queue_id: exec_queue_id returned from xe_exec_queue_create_ioctl
> + * exec_queue_id is help to find exec_queue reset status
> */
> - __u64 num_engines;
> + __u32 exec_queue_id;
>
> - /**
> - * @instances: user pointer to array of drm_xe_engine_class_instance to
> - * wait on, must be NULL when DRM_XE_UFENCE_WAIT_FLAG_SOFT_OP set
> - */
> - __u64 instances;
> + /** @reserved: Reserved */
please fix the alignments and pad doc as Francois already pointed out
and then when that is finalized, import the result here as mentioned above.
> + __u32 pad2;
>
> /** @reserved: Reserved */
> __u64 reserved[2];
> diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c
> index c91bf25c4..87e2db5b4 100644
> --- a/lib/xe/xe_ioctl.c
> +++ b/lib/xe/xe_ioctl.c
> @@ -458,18 +458,16 @@ void xe_exec_wait(int fd, uint32_t exec_queue, uint64_t addr)
> }
>
> int64_t xe_wait_ufence(int fd, uint64_t *addr, uint64_t value,
> - struct drm_xe_engine_class_instance *eci,
> - int64_t timeout)
> + uint32_t exec_queue, int64_t timeout)
> {
> struct drm_xe_wait_user_fence wait = {
> .addr = to_user_pointer(addr),
> .op = DRM_XE_UFENCE_WAIT_OP_EQ,
> - .flags = !eci ? DRM_XE_UFENCE_WAIT_FLAG_SOFT_OP : 0,
> + .flags = 0,
> .value = value,
> .mask = DRM_XE_UFENCE_WAIT_MASK_U64,
> .timeout = timeout,
> - .num_engines = eci ? 1 :0,
> - .instances = eci ? to_user_pointer(eci) : 0,
> + .exec_queue_id = exec_queue,
> };
>
> igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait), 0);
> @@ -477,40 +475,6 @@ int64_t xe_wait_ufence(int fd, uint64_t *addr, uint64_t value,
> return wait.timeout;
> }
>
> -/**
> - * xe_wait_ufence_abstime:
> - * @fd: xe device fd
> - * @addr: address of value to compare
> - * @value: expected value (equal) in @address
> - * @eci: engine class instance
> - * @timeout: absolute time when wait expire
> - *
> - * Function compares @value with memory pointed by @addr until they are equal.
> - *
> - * Returns elapsed time in nanoseconds if user fence was signalled.
> - */
> -int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
> - struct drm_xe_engine_class_instance *eci,
> - int64_t timeout)
> -{
> - struct drm_xe_wait_user_fence wait = {
> - .addr = to_user_pointer(addr),
> - .op = DRM_XE_UFENCE_WAIT_OP_EQ,
> - .flags = !eci ? DRM_XE_UFENCE_WAIT_FLAG_SOFT_OP | DRM_XE_UFENCE_WAIT_FLAG_ABSTIME : 0,
> - .value = value,
> - .mask = DRM_XE_UFENCE_WAIT_MASK_U64,
> - .timeout = timeout,
> - .num_engines = eci ? 1 : 0,
> - .instances = eci ? to_user_pointer(eci) : 0,
> - };
> - struct timespec ts;
> -
> - igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait), 0);
> - igt_assert_eq(clock_gettime(CLOCK_MONOTONIC, &ts), 0);
> -
> - return ts.tv_sec * 1e9 + ts.tv_nsec;
> -}
Well, I believe it would be better this movement in a separated patch.
> -
> void xe_force_gt_reset(int fd, int gt)
> {
> char reset_string[128];
> diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h
> index 32b34b15a..186544a34 100644
> --- a/lib/xe/xe_ioctl.h
> +++ b/lib/xe/xe_ioctl.h
> @@ -89,11 +89,9 @@ void xe_exec_sync(int fd, uint32_t exec_queue, uint64_t addr,
> struct drm_xe_sync *sync, uint32_t num_syncs);
> void xe_exec_wait(int fd, uint32_t exec_queue, uint64_t addr);
> int64_t xe_wait_ufence(int fd, uint64_t *addr, uint64_t value,
> - struct drm_xe_engine_class_instance *eci,
> - int64_t timeout);
> + uint32_t exec_queue,int64_t timeout);
> int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
> - struct drm_xe_engine_class_instance *eci,
> - int64_t timeout);
> + uint32_t exec_queue, int64_t timeout);
> void xe_force_gt_reset(int fd, int gt);
>
> #endif /* XE_IOCTL_H */
> diff --git a/tests/intel/xe_evict.c b/tests/intel/xe_evict.c
> index 89dc46fae..12fbaf586 100644
> --- a/tests/intel/xe_evict.c
> +++ b/tests/intel/xe_evict.c
> @@ -317,7 +317,7 @@ test_evict_cm(int fd, struct drm_xe_engine_class_instance *eci,
> }
> #define TWENTY_SEC MS_TO_NS(20000)
> xe_wait_ufence(fd, &data[i].vm_sync, USER_FENCE_VALUE,
> - NULL, TWENTY_SEC);
> + 0, TWENTY_SEC);
> }
> sync[0].addr = addr + (char *)&data[i].exec_sync -
> (char *)data;
> @@ -352,7 +352,7 @@ test_evict_cm(int fd, struct drm_xe_engine_class_instance *eci,
> data = xe_bo_map(fd, __bo,
> ALIGN(sizeof(*data) * n_execs, 0x1000));
> xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
> - NULL, TWENTY_SEC);
> + exec_queues[i], TWENTY_SEC);
> igt_assert_eq(data[i].data, 0xc0ffee);
> }
> munmap(data, ALIGN(sizeof(*data) * n_execs, 0x1000));
> diff --git a/tests/intel/xe_exec_balancer.c b/tests/intel/xe_exec_balancer.c
> index 79ff65e89..0a02c6767 100644
> --- a/tests/intel/xe_exec_balancer.c
> +++ b/tests/intel/xe_exec_balancer.c
> @@ -483,7 +483,7 @@ test_cm(int fd, int gt, int class, int n_exec_queues, int n_execs,
> bo_size, sync, 1);
>
> #define ONE_SEC MS_TO_NS(1000)
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, ONE_SEC);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, ONE_SEC);
> data[0].vm_sync = 0;
>
> for (i = 0; i < n_execs; i++) {
> @@ -514,7 +514,7 @@ test_cm(int fd, int gt, int class, int n_exec_queues, int n_execs,
>
> if (flags & REBIND && i + 1 != n_execs) {
> xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
> - NULL, ONE_SEC);
> + exec_queues[e], ONE_SEC);
> xe_vm_unbind_async(fd, vm, 0, 0, addr, bo_size, NULL,
> 0);
>
> @@ -529,7 +529,7 @@ test_cm(int fd, int gt, int class, int n_exec_queues, int n_execs,
> addr, bo_size, sync,
> 1);
> xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE,
> - NULL, ONE_SEC);
> + 0, ONE_SEC);
> data[0].vm_sync = 0;
> }
>
> @@ -542,7 +542,7 @@ test_cm(int fd, int gt, int class, int n_exec_queues, int n_execs,
> * an invalidate.
> */
> xe_wait_ufence(fd, &data[i].exec_sync,
> - USER_FENCE_VALUE, NULL, ONE_SEC);
> + USER_FENCE_VALUE, exec_queues[e], ONE_SEC);
> igt_assert_eq(data[i].data, 0xc0ffee);
> } else if (i * 2 != n_execs) {
> /*
> @@ -571,7 +571,7 @@ test_cm(int fd, int gt, int class, int n_exec_queues, int n_execs,
>
> j = flags & INVALIDATE && n_execs ? n_execs - 1 : 0;
> for (i = j; i < n_execs; i++)
> - xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE, NULL,
> + xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE, exec_queues[i],
> ONE_SEC);
>
> /* Wait for all execs to complete */
> @@ -580,7 +580,7 @@ test_cm(int fd, int gt, int class, int n_exec_queues, int n_execs,
>
> sync[0].addr = to_user_pointer(&data[0].vm_sync);
> xe_vm_unbind_async(fd, vm, 0, 0, addr, bo_size, sync, 1);
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, ONE_SEC);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, ONE_SEC);
>
> for (i = (flags & INVALIDATE && n_execs) ? n_execs - 1 : 0;
> i < n_execs; i++)
> diff --git a/tests/intel/xe_exec_compute_mode.c b/tests/intel/xe_exec_compute_mode.c
> index 7d3004d65..e8497a520 100644
> --- a/tests/intel/xe_exec_compute_mode.c
> +++ b/tests/intel/xe_exec_compute_mode.c
> @@ -171,7 +171,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>
> fence_timeout = igt_run_in_simulation() ? HUNDRED_SEC : ONE_SEC;
>
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL,
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0,
> fence_timeout);
> data[0].vm_sync = 0;
>
> @@ -198,7 +198,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>
> if (flags & REBIND && i + 1 != n_execs) {
> xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
> - NULL, fence_timeout);
> + exec_queues[e], fence_timeout);
> xe_vm_unbind_async(fd, vm, bind_exec_queues[e], 0,
> addr, bo_size, NULL, 0);
>
> @@ -214,7 +214,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
> addr, bo_size, sync,
> 1);
> xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE,
> - NULL, fence_timeout);
> + 0, fence_timeout);
> data[0].vm_sync = 0;
> }
>
> @@ -227,7 +227,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
> * an invalidate.
> */
> xe_wait_ufence(fd, &data[i].exec_sync,
> - USER_FENCE_VALUE, NULL,
> + USER_FENCE_VALUE, exec_queues[e],
> fence_timeout);
> igt_assert_eq(data[i].data, 0xc0ffee);
> } else if (i * 2 != n_execs) {
> @@ -257,7 +257,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>
> j = flags & INVALIDATE ? n_execs - 1 : 0;
> for (i = j; i < n_execs; i++)
> - xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE, NULL,
> + xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE, exec_queues[i],
> fence_timeout);
>
> /* Wait for all execs to complete */
> @@ -267,7 +267,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
> sync[0].addr = to_user_pointer(&data[0].vm_sync);
> xe_vm_unbind_async(fd, vm, bind_exec_queues[0], 0, addr, bo_size,
> sync, 1);
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL,
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0,
> fence_timeout);
>
> for (i = j; i < n_execs; i++)
> diff --git a/tests/intel/xe_exec_fault_mode.c b/tests/intel/xe_exec_fault_mode.c
> index ee7cbb604..27921f47a 100644
> --- a/tests/intel/xe_exec_fault_mode.c
> +++ b/tests/intel/xe_exec_fault_mode.c
> @@ -195,14 +195,14 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
> }
>
> #define ONE_SEC MS_TO_NS(1000)
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, ONE_SEC);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, ONE_SEC);
> data[0].vm_sync = 0;
>
> if (flags & PREFETCH) {
> /* Should move to system memory */
> xe_vm_prefetch_async(fd, vm, bind_exec_queues[0], 0, addr,
> bo_size, sync, 1, 0);
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL,
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0,
> ONE_SEC);
> data[0].vm_sync = 0;
> }
> @@ -230,7 +230,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
>
> if (flags & REBIND && i + 1 != n_execs) {
> xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
> - NULL, ONE_SEC);
> + exec_queues[e], ONE_SEC);
> xe_vm_unbind_async(fd, vm, bind_exec_queues[e], 0,
> addr, bo_size, NULL, 0);
>
> @@ -246,7 +246,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
> addr, bo_size, sync,
> 1);
> xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE,
> - NULL, ONE_SEC);
> + 0, ONE_SEC);
> data[0].vm_sync = 0;
> }
>
> @@ -259,7 +259,7 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
> * an invalidate.
> */
> xe_wait_ufence(fd, &data[i].exec_sync,
> - USER_FENCE_VALUE, NULL, ONE_SEC);
> + USER_FENCE_VALUE, exec_queues[e], ONE_SEC);
> igt_assert_eq(data[i].data, 0xc0ffee);
> } else if (i * 2 != n_execs) {
> /*
> @@ -290,13 +290,13 @@ test_exec(int fd, struct drm_xe_engine_class_instance *eci,
> j = flags & INVALIDATE ? n_execs - 1 : 0;
> for (i = j; i < n_execs; i++)
> xe_wait_ufence(fd, &data[i].exec_sync,
> - USER_FENCE_VALUE, NULL, ONE_SEC);
> + USER_FENCE_VALUE, exec_queues[i], ONE_SEC);
> }
>
> sync[0].addr = to_user_pointer(&data[0].vm_sync);
> xe_vm_unbind_async(fd, vm, bind_exec_queues[0], 0, addr, bo_size,
> sync, 1);
just to confirm... we don't need to get the bind_exec_queues there right?!
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, ONE_SEC);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, ONE_SEC);
>
> if (!(flags & INVALID_FAULT)) {
> for (i = j; i < n_execs; i++)
> diff --git a/tests/intel/xe_exec_reset.c b/tests/intel/xe_exec_reset.c
> index edfd27fe0..e133706ee 100644
> --- a/tests/intel/xe_exec_reset.c
> +++ b/tests/intel/xe_exec_reset.c
> @@ -564,7 +564,7 @@ test_compute_mode(int fd, struct drm_xe_engine_class_instance *eci,
> xe_vm_bind_async(fd, vm, 0, bo, 0, addr, bo_size, sync, 1);
>
> #define THREE_SEC MS_TO_NS(3000)
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, THREE_SEC);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, THREE_SEC);
> data[0].vm_sync = 0;
>
> for (i = 0; i < n_execs; i++) {
> @@ -618,11 +618,11 @@ test_compute_mode(int fd, struct drm_xe_engine_class_instance *eci,
>
> for (i = 1; i < n_execs; i++)
> xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE,
> - NULL, THREE_SEC);
> + exec_queues[i], THREE_SEC);
>
> sync[0].addr = to_user_pointer(&data[0].vm_sync);
> xe_vm_unbind_async(fd, vm, 0, 0, addr, bo_size, sync, 1);
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, THREE_SEC);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, THREE_SEC);
why do you pick '0' here and 'exec_queue' in other places in this same code?
>
> for (i = 1; i < n_execs; i++)
> igt_assert_eq(data[i].data, 0xc0ffee);
> diff --git a/tests/intel/xe_exec_threads.c b/tests/intel/xe_exec_threads.c
> index fcb926698..f8450a844 100644
> --- a/tests/intel/xe_exec_threads.c
> +++ b/tests/intel/xe_exec_threads.c
> @@ -331,7 +331,7 @@ test_compute_mode(int fd, uint32_t vm, uint64_t addr, uint64_t userptr,
>
> fence_timeout = igt_run_in_simulation() ? THIRTY_SEC : THREE_SEC;
>
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, fence_timeout);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, fence_timeout);
> data[0].vm_sync = 0;
>
> for (i = 0; i < n_execs; i++) {
> @@ -359,7 +359,7 @@ test_compute_mode(int fd, uint32_t vm, uint64_t addr, uint64_t userptr,
> for (j = i - 0x20; j <= i; ++j)
> xe_wait_ufence(fd, &data[j].exec_sync,
> USER_FENCE_VALUE,
> - NULL, fence_timeout);
> + exec_queues[e], fence_timeout);
> xe_vm_unbind_async(fd, vm, 0, 0, addr, bo_size,
> NULL, 0);
>
> @@ -374,7 +374,7 @@ test_compute_mode(int fd, uint32_t vm, uint64_t addr, uint64_t userptr,
> addr, bo_size, sync,
> 1);
> xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE,
> - NULL, fence_timeout);
> + 0, fence_timeout);
> data[0].vm_sync = 0;
> }
>
> @@ -389,7 +389,7 @@ test_compute_mode(int fd, uint32_t vm, uint64_t addr, uint64_t userptr,
> for (j = i == 0x20 ? 0 : i - 0x1f; j <= i; ++j)
> xe_wait_ufence(fd, &data[j].exec_sync,
> USER_FENCE_VALUE,
> - NULL, fence_timeout);
> + exec_queues[e], fence_timeout);
> igt_assert_eq(data[i].data, 0xc0ffee);
> } else if (i * 2 != n_execs) {
> /*
> @@ -421,7 +421,7 @@ test_compute_mode(int fd, uint32_t vm, uint64_t addr, uint64_t userptr,
> j = flags & INVALIDATE ?
> (flags & RACE ? n_execs / 2 + 1 : n_execs - 1) : 0;
> for (i = j; i < n_execs; i++)
> - xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE, NULL,
> + xe_wait_ufence(fd, &data[i].exec_sync, USER_FENCE_VALUE, exec_queues[i],
> fence_timeout);
>
> /* Wait for all execs to complete */
> @@ -430,7 +430,7 @@ test_compute_mode(int fd, uint32_t vm, uint64_t addr, uint64_t userptr,
>
> sync[0].addr = to_user_pointer(&data[0].vm_sync);
> xe_vm_unbind_async(fd, vm, 0, 0, addr, bo_size, sync, 1);
> - xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, NULL, fence_timeout);
> + xe_wait_ufence(fd, &data[0].vm_sync, USER_FENCE_VALUE, 0, fence_timeout);
>
> for (i = j; i < n_execs; i++)
> igt_assert_eq(data[i].data, 0xc0ffee);
> diff --git a/tests/intel/xe_waitfence.c b/tests/intel/xe_waitfence.c
> index 3be987954..0bd7e5dce 100644
> --- a/tests/intel/xe_waitfence.c
> +++ b/tests/intel/xe_waitfence.c
> @@ -37,22 +37,51 @@ static void do_bind(int fd, uint32_t vm, uint32_t bo, uint64_t offset,
> }
>
> static int64_t wait_with_eci_abstime(int fd, uint64_t *addr, uint64_t value,
> - struct drm_xe_engine_class_instance *eci,
> - int64_t timeout)
> + uint32_t exec_queue, int64_t timeout)
> {
> struct drm_xe_wait_user_fence wait = {
> .addr = to_user_pointer(addr),
> .op = DRM_XE_UFENCE_WAIT_OP_EQ,
> - .flags = !eci ? 0 : DRM_XE_UFENCE_WAIT_FLAG_ABSTIME,
> + .flags = !exec_queue ? 0 : DRM_XE_UFENCE_WAIT_FLAG_ABSTIME,
> .value = value,
> .mask = DRM_XE_UFENCE_WAIT_MASK_U64,
> .timeout = timeout,
> - .num_engines = eci ? 1 : 0,
> - .instances = eci ? to_user_pointer(eci) : 0,
> + .exec_queue_id = exec_queue,
> + };
> + struct timespec ts;
> +
> + igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait), 0);
> + igt_assert_eq(clock_gettime(CLOCK_MONOTONIC, &ts), 0);
> +
> + return ts.tv_sec * 1e9 + ts.tv_nsec;
> +}
> +
> +/**
> + * xe_wait_ufence_abstime:
> + * @fd: xe device fd
> + * @addr: address of value to compare
> + * @value: expected value (equal) in @address
> + * @exec_queue: exec_queue id
> + * @timeout: absolute time when wait expire
> + *
> + * Function compares @value with memory pointed by @addr until they are equal.
> + *
> + * Returns elapsed time in nanoseconds if user fence was signalled.
> + */
> +int64_t xe_wait_ufence_abstime(int fd, uint64_t *addr, uint64_t value,
> + uint32_t exec_queue, int64_t timeout)
> +{
> + struct drm_xe_wait_user_fence wait = {
> + .addr = to_user_pointer(addr),
> + .op = DRM_XE_UFENCE_WAIT_OP_EQ,
> + .flags = !exec_queue ? DRM_XE_UFENCE_WAIT_FLAG_ABSTIME : 0,
> + .value = value,
> + .mask = DRM_XE_UFENCE_WAIT_MASK_U64,
> + .timeout = timeout,
> + .exec_queue_id = exec_queue,
> };
> struct timespec ts;
>
> - igt_assert(eci);
> igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait), 0);
> igt_assert_eq(clock_gettime(CLOCK_MONOTONIC, &ts), 0);
>
> @@ -82,7 +111,6 @@ enum waittype {
> static void
> waitfence(int fd, enum waittype wt)
> {
> - struct drm_xe_engine *engine = NULL;
> struct timespec ts;
> int64_t current, signalled;
> uint32_t bo_1;
> @@ -111,15 +139,15 @@ waitfence(int fd, enum waittype wt)
> do_bind(fd, vm, bo_7, 0, 0xeffff0000, 0x10000, 7);
>
> if (wt == RELTIME) {
> - timeout = xe_wait_ufence(fd, &wait_fence, 7, NULL, MS_TO_NS(10));
> + timeout = xe_wait_ufence(fd, &wait_fence, 7, 0, MS_TO_NS(10));
> igt_debug("wait type: RELTIME - timeout: %ld, timeout left: %ld\n",
> MS_TO_NS(10), timeout);
> } else if (wt == ENGINE) {
> - engine = xe_engine(fd, 1);
> + uint32_t exec_queue = xe_exec_queue_create_class(fd, vm, DRM_XE_ENGINE_CLASS_COPY);
> clock_gettime(CLOCK_MONOTONIC, &ts);
> current = ts.tv_sec * 1e9 + ts.tv_nsec;
> timeout = current + MS_TO_NS(10);
> - signalled = wait_with_eci_abstime(fd, &wait_fence, 7, &engine->instance, timeout);
> + signalled = wait_with_eci_abstime(fd, &wait_fence, 7, exec_queue, timeout);
> igt_debug("wait type: ENGINE ABSTIME - timeout: %" PRId64
> ", signalled: %" PRId64
> ", elapsed: %" PRId64 "\n",
> @@ -128,7 +156,7 @@ waitfence(int fd, enum waittype wt)
> clock_gettime(CLOCK_MONOTONIC, &ts);
> current = ts.tv_sec * 1e9 + ts.tv_nsec;
> timeout = current + MS_TO_NS(10);
> - signalled = xe_wait_ufence_abstime(fd, &wait_fence, 7, NULL, timeout);
> + signalled = xe_wait_ufence_abstime(fd, &wait_fence, 7, 0, timeout);
> igt_debug("wait type: ABSTIME - timeout: %" PRId64
> ", signalled: %" PRId64
> ", elapsed: %" PRId64 "\n",
> @@ -149,9 +177,6 @@ waitfence(int fd, enum waittype wt)
> *
> * SUBTEST: invalid-ops
> * Description: Check query with invalid ops returns expected error code
> - *
> - * SUBTEST: invalid-engine
> - * Description: Check query with invalid engine info returns expected error code
> */
>
> static void
> @@ -166,8 +191,7 @@ invalid_flag(int fd)
> .value = 1,
> .mask = DRM_XE_UFENCE_WAIT_MASK_U64,
> .timeout = -1,
> - .num_engines = 0,
> - .instances = 0,
> + .exec_queue_id = 0,
> };
>
> uint32_t vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT, 0);
> @@ -191,8 +215,7 @@ invalid_ops(int fd)
> .value = 1,
> .mask = DRM_XE_UFENCE_WAIT_MASK_U64,
> .timeout = 1,
> - .num_engines = 0,
> - .instances = 0,
> + .exec_queue_id = 0,
> };
>
> uint32_t vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT, 0);
> @@ -204,32 +227,6 @@ invalid_ops(int fd)
> do_ioctl_err(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait, EINVAL);
> }
>
> -static void
> -invalid_engine(int fd)
> -{
> - uint32_t bo;
> -
> - struct drm_xe_wait_user_fence wait = {
> - .addr = to_user_pointer(&wait_fence),
> - .op = DRM_XE_UFENCE_WAIT_OP_EQ,
> - .flags = 0,
> - .value = 1,
> - .mask = DRM_XE_UFENCE_WAIT_MASK_U64,
> - .timeout = -1,
> - .num_engines = 1,
> - .instances = 0,
> - };
> -
> - uint32_t vm = xe_vm_create(fd, DRM_XE_VM_CREATE_FLAG_ASYNC_DEFAULT, 0);
> -
> - bo = xe_bo_create(fd, vm, 0x40000, vram_if_possible(fd, 0), 0);
> -
> - do_bind(fd, vm, bo, 0, 0x200000, 0x40000, 1);
> -
> - do_ioctl_err(fd, DRM_IOCTL_XE_WAIT_USER_FENCE, &wait, EFAULT);
> -}
> -
why are you removing this?
should be in a separate patch as well
> -
> igt_main
> {
> int fd;
> @@ -252,9 +249,6 @@ igt_main
> igt_subtest("invalid-ops")
> invalid_ops(fd);
>
> - igt_subtest("invalid-engine")
> - invalid_engine(fd);
> -
> igt_fixture
> drm_close_driver(fd);
> }
> --
> 2.25.1
>
next prev parent reply other threads:[~2023-12-06 17:04 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-06 14:44 [igt-dev] [PATCH v4 0/2] RFC: drm-uapi/xe: add exec_queue_id member to drm_xe_wait_user_fence structure Bommu Krishnaiah
2023-12-06 14:44 ` [igt-dev] [PATCH v4 1/2] " Bommu Krishnaiah
2023-12-06 17:04 ` Rodrigo Vivi [this message]
2023-12-06 17:23 ` Bommu, Krishnaiah
2023-12-06 17:36 ` Rodrigo Vivi
2023-12-06 14:44 ` [igt-dev] [PATCH v4 1/1] drm-uapi/xe: kill xe_wait_user_fence_ioctl when exec_queue reset happen Bommu Krishnaiah
2023-12-06 17:06 ` Rodrigo Vivi
2023-12-06 14:44 ` [igt-dev] [PATCH v4 2/2] " Bommu Krishnaiah
2023-12-06 17:08 ` Rodrigo Vivi
2023-12-06 17:25 ` [igt-dev] ✓ Fi.CI.BAT: success for RFC: drm-uapi/xe: add exec_queue_id member to drm_xe_wait_user_fence structure (rev4) Patchwork
2023-12-06 19:09 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2023-12-06 19:20 ` [igt-dev] ✗ CI.xeBAT: " Patchwork
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=ZXCpmPTieKBeE387@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=krishnaiah.bommu@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.