Re: [PATCH i-g-t V2] tests/amd_queue_reset: add sdma test in queue reset

[vitaly prosyak <vprosyak@amd.com>]

Hi Jesse,

Overall, the changes look good to me.

Please address the following points:

1.    Add bool support_sdma to struct dynamic_test: Please include a bool support_sdma field in the struct dynamic_test { ... }.

2.    Improve Context Calculation: When calculating const_num_of_tests, add a function that iterates through the arr_err array to determine the number of tests needed to create the required contexts. Ideally, I’d like to drop this and create contexts on demand, but this would require exporting from one process to another. Currently, this is handled automatically using fork, so we need to explore the best approach for this—there's a TODO here.

3.    Remove !strstr(it->name, "CMD"): Since we can now directly check if a given test is supported by SDMA, please drop the !strstr(it->name, "CMD") condition.

Please let me know if this approach works for you.

With these changes made, the patch is:

Reviewed-by: Vitaly Prosyak vitaly.prosyak@amd.com


Thanks Vitaly


On 2024-08-29 04:40, Jesse.zhang@amd.com wrote:
> To enhance queue reset, add sdma ip test.
>
> Cc: Vitaly Prosyak <vitaly.prosyak@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
>
> Signed-off-by: Jesse Zhang <jesse.zhang@amd.com>
> ---
>  tests/amdgpu/amd_queue_reset.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/tests/amdgpu/amd_queue_reset.c b/tests/amdgpu/amd_queue_reset.c
> index 537f653f9..8482a371d 100644
> --- a/tests/amdgpu/amd_queue_reset.c
> +++ b/tests/amdgpu/amd_queue_reset.c
> @@ -1035,7 +1035,6 @@ igt_main
>  	posix_spawn_file_actions_t action;
>  	amdgpu_device_handle device;
>  	struct amdgpu_gpu_info gpu_info = {0};
> -	struct drm_amdgpu_info_hw_ip info[2] = {0};
>  	int fd = -1;
>  	int fd_shm = -1;
>  	struct shmbuf *sh_mem = NULL;
> @@ -1047,8 +1046,9 @@ igt_main
>  	unsigned int ring_id_job_good;
>  	unsigned int ring_id_job_bad;
>  
> -	enum amd_ip_block_type ip_tests[2] = {AMD_IP_COMPUTE/*keep first*/, AMD_IP_GFX};
> +	enum amd_ip_block_type ip_tests[] = {AMD_IP_COMPUTE/*keep first*/, AMD_IP_GFX, AMD_IP_DMA};
>  	enum amd_ip_block_type ip_background = AMD_IP_COMPUTE;
> +	struct drm_amdgpu_info_hw_ip info[ARRAY_SIZE(ip_tests)] = {0};
>  
>  	amdgpu_context_handle *arr_context_handle = NULL;
>  
> @@ -1098,7 +1098,8 @@ igt_main
>  		if (is_run_subtest_parameter_found(argc, argv))
>  			const_num_of_tests = 1;
>  		else
> -			const_num_of_tests = (sizeof(arr_err)/sizeof(struct dynamic_test) - 1) * ARRAY_SIZE(ip_tests);
> +			/* -3 because sdma don't support shader test*/
> +			const_num_of_tests = (sizeof(arr_err)/sizeof(struct dynamic_test) - 1) * ARRAY_SIZE(ip_tests) - 3 ;
>  
>  		fd = drm_open_driver(DRIVER_AMDGPU);
>  
> @@ -1142,13 +1143,17 @@ igt_main
>  	for (int i = 0; i < ARRAY_SIZE(ip_tests); i++) {
>  		reset_rings_numbers(&ring_id_good, &ring_id_bad, &ring_id_job_good, &ring_id_job_bad);
>  		for (struct dynamic_test *it = &arr_err[0]; it->name; it++) {
> +			if(ip_tests[i] == AMD_IP_DMA && !strstr(it->name,"CMD"))
> +					continue;
>  			igt_describe("Stressful-and-multiple-cs-of-bad-and-good-length-operations-using-multiple-processes");
> -			igt_subtest_with_dynamic_f("amdgpu-%s-%s", ip_tests[i] == AMD_IP_COMPUTE ? "COMPUTE":"GFX", it->name) {
> +			igt_subtest_with_dynamic_f("amdgpu-%s-%s", ip_tests[i] == AMD_IP_COMPUTE ? "COMPUTE":
> +						ip_tests[i] == AMD_IP_GFX ? "GFX":"SDMA", it->name) {
>  				if (arr_cap[ip_tests[i]] && is_sub_test_queue_reset_enable(&gpu_info, it->exclude_filter, it) &&
>  						get_next_rings(&ring_id_good, &ring_id_bad, info[0].available_rings,
>  						info[i].available_rings, ip_background != ip_tests[i], &ring_id_job_good, &ring_id_job_bad)) {
>  					igt_dynamic_f("amdgpu-%s-ring-good-%d-bad-%d-%s", it->name, ring_id_job_good, ring_id_job_bad,
> -							ip_tests[i] == AMD_IP_COMPUTE ? "COMPUTE":"GFX")
> +							ip_tests[i] == AMD_IP_COMPUTE ? "COMPUTE":
> +							ip_tests[i] == AMD_IP_GFX? "GFX":"SDMA")
>  					set_next_test_to_run(sh_mem, it->test, ip_background, ip_tests[i], ring_id_job_good, ring_id_job_bad);
>  				} else {
>  					set_next_test_to_skip(sh_mem);