Re: [PATCH i-g-t] tests/amdgpu: add check for deadlock test

[vitaly prosyak <vprosyak@amd.com>]

Hi Kamil,

Thanks for your suggestion, we reflected it in our final commit.


On 2024-04-17 05:08, Kamil Konieczny wrote:
> Hi Jesse,
> On 2024-04-17 at 09:17:01 +0800, Jesse Zhang wrote:
>
> imho you could improve subject, was:
>
> [PATCH i-g-t] tests/amdgpu: add check for deadlock test
>
> and it would be better with:
>
> [PATCH i-g-t] tests/amdgpu/amd_deadlock: run only on GPUs supporting reset
>
> No need for resend, it could be changed at merge.
>
>> Only enable deadlock test  for ASICs supporting GPU reset.
>> The issue report by:GMP, Vikram <vikram.gmp@amd.com>
>>
>> Cc: Vitaly Prosyak <vitaly.prosyak@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
>> Reviewed by:  Vitaly Prosyak <vitaly.prosyak@amd.com>
>> ---
>>  tests/amdgpu/amd_deadlock.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/tests/amdgpu/amd_deadlock.c b/tests/amdgpu/amd_deadlock.c
>> index dc7ec4366..7a27fae51 100644
>> --- a/tests/amdgpu/amd_deadlock.c
>> +++ b/tests/amdgpu/amd_deadlock.c
>> @@ -9,6 +9,28 @@
>>  #include "lib/amdgpu/amd_command_submission.h"
>>  #include "lib/amdgpu/amd_deadlock_helpers.h"
>>  
>> +#define AMDGPU_FAMILY_SI                        110 /* Hainan, Oland, Verde, Pitcairn, Tahiti */
>> +#define AMDGPU_FAMILY_CI                        120 /* Bonaire, Hawaii */
>> +#define AMDGPU_FAMILY_CZ                        135 /* Carrizo, Stoney */
>> +#define AMDGPU_FAMILY_RV                        142 /* Raven */
>> +
>> +static bool
>> +is_deadlock_tests_enable(const struct amdgpu_gpu_info *gpu_info)
>> +{
>> +	bool enable = true;
>> +	/*
>> +	 * skip for the ASICs that don't support GPU reset.
>> +	 */
>> +	if (gpu_info->family_id == AMDGPU_FAMILY_SI ||
>> +	    gpu_info->family_id == AMDGPU_FAMILY_KV ||
>> +	    gpu_info->family_id == AMDGPU_FAMILY_CZ ||
>> +	    gpu_info->family_id == AMDGPU_FAMILY_RV) {
>> +		igt_info("\n\nGPU reset is not enabled for the ASIC, deadlock test skip\n");
> This print suggest other name for this function but as it is local
> to this test it can stay as is. As it looks, you could run this test
> on these GPUs but then you need to power cycle GPU (or machine?).
The deadlock tests are skipped on GPU which does not support hardware reset, but when it is supported on GPU, the series of miscellaneous deadlock tests could be run without a power cycle.
>
> Regards,
> Kamil
>
>> +		enable = false;
>> +	}
>> +	return enable;
>> +}
>> +
>>  igt_main
>>  {
>>  	amdgpu_device_handle device;
>> @@ -34,6 +56,7 @@ igt_main
>>  		r = setup_amdgpu_ip_blocks(major, minor, &gpu_info, device);
>>  		igt_assert_eq(r, 0);
>>  		asic_rings_readness(device, 1, arr_cap);
>> +		igt_skip_on(!is_deadlock_tests_enable(&gpu_info));
>>  
>>  	}
>>  	igt_describe("Test-GPU-reset-by-flooding-sdma-ring-with-jobs");
>> -- 
>> 2.25.1
>>
Thanks, Vitaly