[PATCH i-g-t] tests/amdgpu: move bad mem compute test to amd deadlock

[Kamil Konieczny]

Hi Jesse,
On 2024-08-05 at 14:52:18 +0800, Jesse Zhang wrote:
> 1. remove duplicate test case.
> 2. Since the bad memory test only has page faults
> and does not trigger job timeouts on mi300.
> So temporarily moved this test to amd deadlock.
> 
> Signed-off-by: Jesse Zhang <jesse.zhang at amd.com>
> ---
>  tests/amdgpu/amd_deadlock.c    |  9 +++++++++
>  tests/amdgpu/amd_queue_reset.c | 11 +----------

amd_queue_reset was reverted, please prepare new version with
your changes, also see note about subtests naming convetion
below.

>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/amdgpu/amd_deadlock.c b/tests/amdgpu/amd_deadlock.c
> index 01185bfd3..6996855cb 100644
> --- a/tests/amdgpu/amd_deadlock.c
> +++ b/tests/amdgpu/amd_deadlock.c
> @@ -94,6 +94,15 @@ igt_main
>  		}
>  	}
>  
> +	igt_describe("Test-GPU-reset-by-access-compute-illegal-mem-addr");
> +	igt_subtest_with_dynamic("amdgpu-compute-illegal-mem-access") {

Why dynamic here? Dynamic tests are used for not-known number/names
for its dynamic sub-subtests, not whe you do have static number of
them.

> +		if (arr_cap[AMD_IP_COMPUTE]) {
> +			igt_dynamic_f("amdgpu-illegal-mem-access")

Do not do such if(...) dynamic-name,
imho better skip:
	igt_subtest("amdgpu-compute-illegal-mem-access") {
		igt_skip_on_f(!arr_cap[AMD_IP_COMPUTE], "message why skipping\n");

		bad_access_helper(device, CMD_STREAM_TRANS_BAD_MEM_ADDRESS,
					AMDGPU_HW_IP_COMPUTE);
	}

> +			bad_access_helper(device, CMD_STREAM_TRANS_BAD_MEM_ADDRESS,
> +					AMDGPU_HW_IP_COMPUTE);
> +		}
> +	}
> +
>  	igt_describe("Test-GPU-reset-by-flooding-compute-ring-with-jobs");
>  	igt_subtest_with_dynamic("amdgpu-deadlock-compute") {
>  		if (arr_cap[AMD_IP_COMPUTE]) {
> diff --git a/tests/amdgpu/amd_queue_reset.c b/tests/amdgpu/amd_queue_reset.c
> index fb05aee35..df96c9f76 100644
> --- a/tests/amdgpu/amd_queue_reset.c
> +++ b/tests/amdgpu/amd_queue_reset.c
> @@ -893,7 +893,6 @@ igt_main
>  	unsigned int arr_err[] = {
>  			CMD_STREAM_EXEC_INVALID_PACKET_LENGTH,
>  			CMD_STREAM_EXEC_INVALID_OPCODE,
> -			CMD_STREAM_TRANS_BAD_MEM_ADDRESS,
>  			//CMD_STREAM_TRANS_BAD_MEM_ADDRESS_BY_SYNC,TODO  not job timeout, debug why for n31
>  			//CMD_STREAM_TRANS_BAD_REG_ADDRESS, TODO  amdgpu: device lost from bus! for n31
>  			BACKEND_SE_GC_SHADER_INVALID_PROGRAM_ADDR,
> @@ -975,14 +974,6 @@ igt_main
>  		}
>  	}
>  
> -	igt_describe("Stressful-and-multiple-cs-of-bad and good mem-operations-using-multiple-processes");
> -	igt_subtest_with_dynamic("amdgpu-compute-CMD_STREAM_TRANS_BAD_MEM_ADDRESS") {
> -		if (arr_cap[ip_test] && get_next_rings(ring_id_good, info.available_rings, &ring_id_good, &ring_id_bad)) {
> -			igt_dynamic_f("amdgpu-compute-CMD_STREAM_TRANS_BAD_MEM_ADDRESS")

You cannot just repeat a name in a dynamic subtest, each name
should be uniqe, I would suggest to rename it to something like:
			char name[32];

			fill_in_name(name, sizeof(name) - 1, ring_id_good, ring_id_bad);
			igt_dynamic_f(name);

So dynamic subtests should have dynamic names, not a static ones.

Regards,
Kamil

> -				set_next_test_to_run(sh_mem, CMD_STREAM_TRANS_BAD_MEM_ADDRESS,
> -						ip_background, ip_test, ring_id_good, ring_id_bad);
> -		}
> -	}
>  	/* TODO  not job timeout, debug why for nv32
>  	 *igt_describe("Stressful-and-multiple-cs-of-bad and good mem-sync-operations-using-multiple-processes");
>  	 *igt_subtest_with_dynamic("amdgpu-compute-CMD_STREAM_TRANS_BAD_MEM_ADDRESS_BY_SYNC") {
> @@ -997,7 +988,7 @@ igt_main
>  	 *igt_describe("Stressful-and-multiple-cs-of-bad and good reg-operations-using-multiple-processes");
>  	 *igt_subtest_with_dynamic("amdgpu-compute-CMD_STREAM_TRANS_BAD_REG_ADDRESS") {
>  	 *	if (arr_cap[ip_test] && get_next_rings(ring_id_good, info.available_rings, &ring_id_good, &ring_id_bad)) {
> -	 *		igt_dynamic_f("amdgpu-compute-CMD_STREAM_TRANS_BAD_MEM_ADDRESS_BY_SYNC")
> +	 *		igt_dynamic_f("amdgpu-compute-CMD_STREAM_TRANS_BAD_MEM_ADDRESS")
>  	 *			set_next_test_to_run(sh_mem, CMD_STREAM_TRANS_BAD_REG_ADDRESS,
>  	 *					ip_background, ip_test, ring_id_good, ring_id_bad);
>  	 *	}
> -- 
> 2.25.1
>