[PATCH i-g-t v3] tests/amdgpu: Fix compilation warnings for USERQ

[Kamil Konieczny]

Hi Khatri,,
On 2025-06-12 at 05:08:14 +0000, Khatri, Sunil wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> This looks good.
> Reviewed-by: Sunil Khatri <sunil.khatri at amd.com>
> 
> 
> -----Original Message-----
> From: Prosyak, Vitaly <Vitaly.Prosyak at amd.com>
> Sent: Thursday, June 12, 2025 4:32 AM
> To: Kamil Konieczny <kamil.konieczny at linux.intel.com>; igt-dev at lists.freedesktop.org
> Cc: Khatri, Sunil <Sunil.Khatri at amd.com>; Prosyak, Vitaly <Vitaly.Prosyak at amd.com>
> Subject: Re: [PATCH i-g-t v3] tests/amdgpu: Fix compilation warnings for USERQ
> 
> Hi Kamil,
> 
> 
> Sorry I missed those warnings and accidentally merged them — my mistake.
> Your change looks great to me. Thanks for taking care of it!
> 
> I hope Sunil will also agree with your proposal. I’m planning to merge it tomorrow — unless you’d prefer to do it yourself?

Thank you all, I merged it.

> 
> Reviewed-by Vitaly Prosyak ,vitaly.prosyak at amd.com>

Here missed ':' and s/,/</, I corrected it at merge.

Few more nits below.

> 
> On 2025-06-11 17:25, Kamil Konieczny wrote:
> > Recent change brings compilation warnings when AMDGPU_USERQ_ENABLED
> > was not defined, for example:
> >
> > ../tests/amdgpu/amd_security.c: In function '__igt_unique____real_main311':
> > ../tests/amdgpu/amd_security.c:319:14: warning: variable 'enable_test' set but not used [-Wunused-but-set-variable]
> >   319 |         bool enable_test = false;
> >
> > Fix this.
> >
> > v2: removed additional vars and prints (Sunil)
> > v3: correcting description, moved var setting back (Kamil)
> >
> > Cc: Sunil Khatri <sunil.khatri at amd.com>
> > Cc: Vitaly Prosyak <vitaly.prosyak at amd.com>
> > Fixes: dad4b2bb9e5a ("tests/amdgpu: add environment variable to enable
> > tests")
> > Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > ---
> >  tests/amdgpu/amd_cs_nop.c   | 2 ++
> >  tests/amdgpu/amd_deadlock.c | 2 ++
> >  tests/amdgpu/amd_security.c | 2 ++
> >  3 files changed, 6 insertions(+)
> >
> > diff --git a/tests/amdgpu/amd_cs_nop.c b/tests/amdgpu/amd_cs_nop.c
> > index 644016d5d..c406f2fc5 100644
> > --- a/tests/amdgpu/amd_cs_nop.c
> > +++ b/tests/amdgpu/amd_cs_nop.c
> > @@ -171,10 +171,12 @@ igt_main
> >       int fd = -1;
> >       bool arr_cap[AMD_IP_MAX] = {0};
> >       bool userq_arr_cap[AMD_IP_MAX] = {0};
> > +#ifdef AMDGPU_USERQ_ENABLED
> >       bool enable_test;
> >       const char *env = getenv("AMDGPU_DISABLE_USERQTEST");
> >
> >       enable_test = env && atoi(env);
> > +#endif

One more way to have it would be:

	bool enable_test = false;

#ifdef AMDGPU_USERQ_ENABLED
       const char *env = getenv("AMDGPU_DISABLE_USERQTEST");

       enable_test = env && atoi(env);
#endif

and later on use it at least one. Also consider compiling always
USERQ code, for example:

igt_describe("Test-GPU-reset-by-access-umq-gfx-illegal-reg");
igt_subtest("amdgpu-umq-gfx-illegal-reg-access") {
	if (userq_arr_cap[AMD_IP_GFX] &&
	    is_reset_enable(AMD_IP_GFX, AMDGPU_RESET_TYPE_PER_QUEUE, &pci)) {
       		if (enable_test)
			bad_access_ring_helper(device, CMD_STREAM_TRANS_BAD_REG_ADDRESS, AMDGPU_HW_IP_GFX, &pci, true);
		else
			igt_skip_f(1, "Test still in development\n");
	} else {
		igt_skip_on_f(!userq_arr_cap[AMD_IP_GFX], "No userq CAP\n");
		igt_skip_on_f(!is_reset_enable(AMD_IP_GFX, AMDGPU_RESET_TYPE_PER_QUEUE, &pci), "No reset per queue\n");
		igt_assert_f(0, "Error - should not reach here\n");
	}
}

Dynamic subtests are used when you will dynamically generete
sub-subtests and you do not know beforehand their names (scope),
for example you could have one, two or more queues depending on
type of GPU tested and then you will have:

for (i = 0; i < max_q; ++i)
	igt_dynamic_f("amdgpu-umq-illegal-reg-access-with-queue-number-%d", num_of_q) {
		test_illegal_reg_queue(amdgpu_fd, i);
	}

For static names there is no point in using dynamics.

Regards,
Kamil

> >
> >       igt_fixture {
> >               uint32_t major, minor;
> > diff --git a/tests/amdgpu/amd_deadlock.c b/tests/amdgpu/amd_deadlock.c
> > index 45a864feb..2330b2955 100644
> > --- a/tests/amdgpu/amd_deadlock.c
> > +++ b/tests/amdgpu/amd_deadlock.c
> > @@ -43,10 +43,12 @@ igt_main
> >       bool arr_cap[AMD_IP_MAX] = {0};
> >       bool userq_arr_cap[AMD_IP_MAX] = {0};
> >       struct pci_addr pci;
> > +#ifdef AMDGPU_USERQ_ENABLED
> >       bool enable_test = false;
> >       const char *env = getenv("AMDGPU_DISABLE_USERQTEST");
> >
> >       enable_test = env && atoi(env);
> > +#endif
> >
> >       igt_fixture {
> >               uint32_t major, minor;
> > diff --git a/tests/amdgpu/amd_security.c b/tests/amdgpu/amd_security.c
> > index 45bd7e771..4f38ee04e 100644
> > --- a/tests/amdgpu/amd_security.c
> > +++ b/tests/amdgpu/amd_security.c
> > @@ -316,10 +316,12 @@ igt_main
> >       int r, fd = -1;
> >       bool is_secure = true;
> >       bool userq_arr_cap[AMD_IP_MAX] = {0};
> > +#ifdef AMDGPU_USERQ_ENABLED
> >       bool enable_test = false;
> >       const char *env = getenv("AMDGPU_DISABLE_USERQTEST");
> >
> >       enable_test = env && atoi(env);
> > +#endif
> >
> >       igt_fixture {
> >               uint32_t major, minor;