[PATCH i-g-t 1/2] tests/amdgpu: userq skip waiting for signal fence
Khatri, Sunil
sukhatri at amd.com
Fri May 16 10:17:01 UTC 2025
On 5/16/2025 3:33 PM, Mohan Marimuthu, Yogesh wrote:
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>
> Hi Sunil,
>
> I did not test with true as logically the code flow does not change if
> skip_signal is true.
> The indentation is off because of the editor I had used. I will fix in
> the next patch update.
If you are planning to use this in a new test case then we should
handle/declare the test as success on failure as this is a negative test
case. So this needs to be handled while writing a new test case.
Regards
Sunil Khatri
>
> Thank you,
> Yogesh
>
> ------------------------------------------------------------------------
> *From:* Khatri, Sunil <Sunil.Khatri at amd.com>
> *Sent:* Friday, May 16, 2025 11:12 AM
> *To:* Mohan Marimuthu, Yogesh <Yogesh.Mohanmarimuthu at amd.com>;
> igt-dev at lists.freedesktop.org <igt-dev at lists.freedesktop.org>
> *Subject:* Re: [PATCH i-g-t 1/2] tests/amdgpu: userq skip waiting for
> signal fence
>
> @yogesh
>
> Functionally code looks good to me but i have a question,
>
> Have you validated the any test case with the skip flag set to true.
> Test should not fail with that as its a negative test.
> How the igt test work here is that when you return from the
> amdgpu_user_queue_submit function the next function checks the value
> written in the memory and test might fail if the values dont match.
>
>
> Also, Indentation seems little off to me.
>
>
> On 5/15/2025 12:13 PM, Mohan Marimuthu, Yogesh wrote:
>
> [Public]
>
>
> [Public]
>
>
> For negative test cases where the job will not complete need to
> skip signal fence. Pass a flag to amdgpu_user_queue_submit() to
> skip signal fence wait.
>
> Signed-off-by: Yogesh Mohan Marimuthu
> <yogesh.mohanmarimuthu at amd.com> <mailto:yogesh.mohanmarimuthu at amd.com>
> ---
> lib/amdgpu/amd_command_submission.c | 2 +-
> lib/amdgpu/amd_compute.c | 2 +-
> lib/amdgpu/amd_userq.c | 36
> +++++++++++++++--------------
> lib/amdgpu/amd_userq.h | 2 +-
> tests/amdgpu/amd_basic.c | 4 ++--
> tests/amdgpu/amd_cs_nop.c | 2 +-
> 6 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/lib/amdgpu/amd_command_submission.c
> b/lib/amdgpu/amd_command_submission.c
> index 80d03a498..74091da5a 100644
> --- a/lib/amdgpu/amd_command_submission.c
> +++ b/lib/amdgpu/amd_command_submission.c
> @@ -68,7 +68,7 @@ int
> amdgpu_test_exec_cs_helper(amdgpu_device_handle device, unsigned
> int ip_type
> memcpy(ring_ptr, ring_context->pm4, ring_context->pm4_dw *
> sizeof(*ring_context->pm4));
> if (user_queue)
> - amdgpu_user_queue_submit(device, ring_context,
> ip_type, ib_result_mc_address);
> + amdgpu_user_queue_submit(device, ring_context,
> ip_type, ib_result_mc_address, false);
>
> Indentation here
>
> else {
> ring_context->ib_info.ib_mc_address =
> ib_result_mc_address;
> ring_context->ib_info.size = ring_context->pm4_dw;
> diff --git a/lib/amdgpu/amd_compute.c b/lib/amdgpu/amd_compute.c
> index 95bfa53aa..008186049 100644
> --- a/lib/amdgpu/amd_compute.c
> +++ b/lib/amdgpu/amd_compute.c
> @@ -91,7 +91,7 @@ void
> amdgpu_command_submission_compute_nop(amdgpu_device_handle device,
> bool use
> if (user_queue) {
> amdgpu_user_queue_submit(device, ring_context,
> AMD_IP_COMPUTE,
> - ib_result_mc_address);
> + ib_result_mc_address, false);
> } else {
> memset(&ib_info, 0, sizeof(struct
> amdgpu_cs_ib_info));
> ib_info.ib_mc_address = ib_result_mc_address;
> diff --git a/lib/amdgpu/amd_userq.c b/lib/amdgpu/amd_userq.c
> index 50d058609..727df8222 100644
> --- a/lib/amdgpu/amd_userq.c
> +++ b/lib/amdgpu/amd_userq.c
> @@ -127,7 +127,7 @@ int
> amdgpu_timeline_syncobj_wait(amdgpu_device_handle device_handle,
> }
> void amdgpu_user_queue_submit(amdgpu_device_handle device, struct
> amdgpu_ring_context *ring_context,
> - unsigned int ip_type, uint64_t mc_address)
> + unsigned int ip_type, uint64_t mc_address,
> bool skip_signal)
>
> Indentation here too.
>
> {
> int r;
> uint32_t control = ring_context->pm4_dw;
> @@ -166,22 +166,24 @@ void
> amdgpu_user_queue_submit(amdgpu_device_handle device, struct
> amdgpu_ring_co
> /* Update the door bell */
> ring_context->doorbell_cpu[DOORBELL_INDEX] =
> *ring_context->wptr_cpu;
> - /* Add a fence packet for signal */
> - syncarray[0] = ring_context->timeline_syncobj_handle;
> - signal_data.queue_id = ring_context->queue_id;
> - signal_data.syncobj_handles = (uintptr_t)syncarray;
> - signal_data.num_syncobj_handles = 1;
> - signal_data.bo_read_handles = 0;
> - signal_data.bo_write_handles = 0;
> - signal_data.num_bo_read_handles = 0;
> - signal_data.num_bo_write_handles = 0;
> -
> - r = amdgpu_userq_signal(device, &signal_data);
> - igt_assert_eq(r, 0);
> + if (!skip_signal) {
> + /* Add a fence packet for signal */
> + syncarray[0] = ring_context->timeline_syncobj_handle;
> + signal_data.queue_id = ring_context->queue_id;
> + signal_data.syncobj_handles = (uintptr_t)syncarray;
> + signal_data.num_syncobj_handles = 1;
> + signal_data.bo_read_handles = 0;
> + signal_data.bo_write_handles = 0;
> + signal_data.num_bo_read_handles = 0;
> + signal_data.num_bo_write_handles = 0;
> +
> + r = amdgpu_userq_signal(device, &signal_data);
> + igt_assert_eq(r, 0);
> - r = amdgpu_cs_syncobj_wait(device,
> &ring_context->timeline_syncobj_handle, 1, INT64_MAX,
> - DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL, NULL);
> - igt_assert_eq(r, 0);
> + r = amdgpu_cs_syncobj_wait(device,
> &ring_context->timeline_syncobj_handle, 1, INT64_MAX,
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL,
> NULL);
>
> Indentation.
>
> + igt_assert_eq(r, 0);
> + }
> }
> void amdgpu_user_queue_destroy(amdgpu_device_handle
> device_handle, struct amdgpu_ring_context *ctxt,
> @@ -456,7 +458,7 @@ int
> amdgpu_timeline_syncobj_wait(amdgpu_device_handle device_handle,
> }
> void amdgpu_user_queue_submit(amdgpu_device_handle device, struct
> amdgpu_ring_context *ring_context,
> - unsigned int ip_type, uint64_t mc_address)
> + unsigned int ip_type, uint64_t mc_address, bool skip_signal)
> {
> }
> diff --git a/lib/amdgpu/amd_userq.h b/lib/amdgpu/amd_userq.h
> index b29e97ccf..dc39c1ca4 100644
> --- a/lib/amdgpu/amd_userq.h
> +++ b/lib/amdgpu/amd_userq.h
> @@ -50,6 +50,6 @@ void
> amdgpu_user_queue_destroy(amdgpu_device_handle device_handle,
> struct amdgpu
> unsigned int ip_type);
> void amdgpu_user_queue_submit(amdgpu_device_handle device, struct
> amdgpu_ring_context *ring_context,
> - unsigned int ip_type, uint64_t mc_address);
> + unsigned int ip_type, uint64_t mc_address,
> bool skip_signal);
> #endif
> diff --git a/tests/amdgpu/amd_basic.c b/tests/amdgpu/amd_basic.c
> index 97a08a9a3..914d27909 100644
> --- a/tests/amdgpu/amd_basic.c
> +++ b/tests/amdgpu/amd_basic.c
> @@ -607,7 +607,7 @@
> amdgpu_sync_dependency_test(amdgpu_device_handle device_handle,
> bool user_queue)
> if (user_queue) {
> ring_context->pm4_dw = ib_info.size;
> amdgpu_user_queue_submit(device_handle, ring_context,
> ip_block->type,
> - ib_result_mc_address);
> + ib_result_mc_address, false);
> } else {
> r = amdgpu_cs_submit(context_handle[1], 0,
> &ibs_request, 1);
> }
> @@ -647,7 +647,7 @@
> amdgpu_sync_dependency_test(amdgpu_device_handle device_handle,
> bool user_queue)
> if (user_queue) {
> ring_context->pm4_dw = ib_info.size;
> amdgpu_user_queue_submit(device_handle, ring_context,
> ip_block->type,
> - ib_info.ib_mc_address);
> + ib_info.ib_mc_address, false);
> } else {
> r = amdgpu_cs_submit(context_handle[0], 0,
> &ibs_request, 1);
> igt_assert_eq(r, 0);
> diff --git a/tests/amdgpu/amd_cs_nop.c b/tests/amdgpu/amd_cs_nop.c
> index 268bc9201..658c8d050 100644
> --- a/tests/amdgpu/amd_cs_nop.c
> +++ b/tests/amdgpu/amd_cs_nop.c
> @@ -108,7 +108,7 @@ static void nop_cs(amdgpu_device_handle device,
> if (user_queue) {
> ring_context->pm4_dw = ib_info.size;
> amdgpu_user_queue_submit(device,
> ring_context, ip_type,
> - ib_info.ib_mc_address);
> + ib_info.ib_mc_address,
> false);
>
> Not sure if its the mail editor or what i see its shifted by one. But
> make sure indentation is correct and you run checkpatch.pl.
>
> igt_assert_eq(r, 0);
> } else {
> r = amdgpu_cs_submit(context, 0,
> &ibs_request, 1);
> --
> 2.43.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20250516/7765ac08/attachment-0001.htm>
More information about the igt-dev
mailing list