[igt-dev] [PATCH V3 i-g-t 2/2] test/amdgpu: add apu check for pciplug test

[Kamil Konieczny]

Hi Jesse,

On 2023-11-08 at 10:57:38 +0800, Jesse Zhang wrote:
> It is integrated with cpu,
> for APU(Accelerated Processing Unit CPU + iGPU).
> So hotplug test should be unnecessary for it.
> 
> V3:
>  - place the APU check in particular subtests.(Kamil)
>  - Add igt_info about skipping tests (Vitaly)
> 
> Cc: Vitaly Prosyak <vitaly.prosyak at amd.com>
> Cc: Luben Tuikov <luben.tuikov at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Christian Koenig <christian.koenig at amd.com>
> Cc: Tim Huang <tim.huang at amd.com>
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang at amd.com>
> Reviewed-by: Vitaly Prosyak <vitaly.prosyak at amd.com>
> ---
>  lib/amdgpu/amd_pci_unplug.c | 34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/amdgpu/amd_pci_unplug.c b/lib/amdgpu/amd_pci_unplug.c
> index 9811e0cc2..1e795757d 100644
> --- a/lib/amdgpu/amd_pci_unplug.c
> +++ b/lib/amdgpu/amd_pci_unplug.c
> @@ -92,6 +92,7 @@ amdgpu_hotunplug_setup_test(bool render_mode, const struct amd_pci_unplug_setup
>  	bool ret = false;
>  	int r;
>  	uint32_t  major_version, minor_version;
> +	struct amdgpu_gpu_info gpu_info = {0};
>  
>  	unplug->num_devices = amdgpu_open_devices(render_mode, MAX_CARDS_SUPPORTED,
>  											  unplug->drm_amdgpu_fds);
> @@ -101,7 +102,7 @@ amdgpu_hotunplug_setup_test(bool render_mode, const struct amd_pci_unplug_setup
>  	if (setup->open_device && setup->open_device2 && unplug->num_devices < 2) {
>  		/*Not enough board for the test*/
>  		printf("SKIP ... more than 1 GPU is required for this test\n");
------- ^^^^^^
printf in igt lib? Should be igt_info or igt_debug
If you will correct this please do it in separate patch(es).

> -		goto release;
> +		igt_skip_on(!ret);
>  	}
>  
>  	tmp_str = amdgpu_get_device_from_fd(unplug->drm_amdgpu_fds[0]);
> @@ -115,8 +116,19 @@ amdgpu_hotunplug_setup_test(bool render_mode, const struct amd_pci_unplug_setup
>  	if (r != 0)
>  		goto release;
>  
> -	if (minor_version < setup->minor_version_req)
> -		goto release;
> +	if (minor_version < setup->minor_version_req) {
> +		igt_info("SKIP ... minor_version:%d mismatch, required: %d for this test\n",
> +			minor_version, setup->minor_version_req);
> +		igt_skip_on(!ret);
------------------- ^^^^^ ?

> +	}

You do not need to print message before skip_on, look at other
skip_on like:

igt_skip_on_f(ret, "KTAP parser failed\n");
or
igt_skip_on_f(fd<0, "No known gpu found for chipset flags 0x%u (%s)\n",

So here just:
	    igt_skip_on_f(minor_version < setup->minor_version_req, "minor version mismatch\n");
or
	    igt_skip_on_f(minor_version < setup->minor_version_req,
                     "It has minor_version: %d but it is required: %d for this test\n",
                     minor_version, setup->minor_version_req);

Regards,
Kamil

> +
> +	/* skip hotplug test on APU (Accelerated Processing Unit CPU + iGPU) */
> +	r = amdgpu_query_gpu_info(unplug->device_handle, &gpu_info);
> +	igt_assert_eq(r, 0);
> +	if (gpu_info.ids_flags & AMDGPU_IDS_FLAGS_FUSION) {
> +		igt_info("SKIP ... APU does not require this test\n");
> +		igt_skip_on(!ret);
> +	}
>  
>  	if (!setup->open_device) {
>  			/* device handle is not always required for test */
> @@ -130,10 +142,22 @@ amdgpu_hotunplug_setup_test(bool render_mode, const struct amd_pci_unplug_setup
>  						   &minor_version, &unplug->device_handle2);
>  		if (r != 0)
>  			goto release;
> -		if (minor_version < setup->minor_version_req)
> -			goto release;
> +		if (minor_version < setup->minor_version_req) {
> +			igt_info("SKIP ... minor_version:%d mismatch, required: %d for this test\n",
> +				minor_version, setup->minor_version_req);
> +			igt_skip_on(!ret);
> +		}
> +
> +		/* skip hotplug test on APU (Accelerated Processing Unit CPU + iGPU) */
> +		r = amdgpu_query_gpu_info(unplug->device_handle2, &gpu_info);
> +		igt_assert_eq(r, 0);
> +		if (gpu_info.ids_flags & AMDGPU_IDS_FLAGS_FUSION) {
> +			igt_info("SKIP ... APU does not require this test\n");
> +			igt_skip_on(!ret);
> +		}
>  	}
>  	ret = true;
> +
>  release:
>  	return ret;
>  }
> -- 
> 2.25.1
>