Re: [PATCH] drm/amdgpu: Move SR-IOV check into amdgpu_gfx_sysfs_compute_init

["Christian König" <christian.koenig@amd.com>]

Am 06.06.24 um 14:05 schrieb Srinivasan Shanmugam:
> Previously, this check was performed in the gfx_v9_4_3_sw_init function,
> and the amdgpu_gfx_sysfs_compute_init function was only called if the
> GPU was not a VF in SR-IOV mode. This is because the sysfs entries
> created by amdgpu_gfx_sysfs_compute_init are specific to compute
> partitions, which are only applicable on GFX9 and not on a VF in SR-IOV
> mode.
>
> By moving the check into amdgpu_gfx_sysfs_compute_init, we make this
> function responsible for deciding whether or not to create the compute
> partition sysfs entries.
>
> This change improves the code organization and maintainability. If in
> the future the  conditions for creating the compute partition sysfs
> entries change, we  would only need to update the
> amdgpu_gfx_sysfs_compute_init function.
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 24 +++++++++++++++---------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  4 ++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c | 11 +++++------
>   3 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 19b1817b55d7..72477a5aedca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1376,21 +1376,27 @@ static DEVICE_ATTR(current_compute_partition, 0644,
>   static DEVICE_ATTR(available_compute_partition, 0444,
>   		   amdgpu_gfx_get_available_compute_partition, NULL);
>   
> -int amdgpu_gfx_sysfs_init(struct amdgpu_device *adev)
> +int amdgpu_gfx_sysfs_compute_init(struct amdgpu_device *adev)
>   {
>   	int r;
>   
> -	r = device_create_file(adev->dev, &dev_attr_current_compute_partition);
> -	if (r)
> -		return r;
> +	if (!amdgpu_sriov_vf(adev)) {
> +		r = device_create_file(adev->dev, &dev_attr_current_compute_partition);
> +		if (r)
> +			return r;
>   
> -	r = device_create_file(adev->dev, &dev_attr_available_compute_partition);
> +		r = device_create_file(adev->dev, &dev_attr_available_compute_partition);
> +		if (r)
> +			return r;
> +	}
>   
> -	return r;
> +	return 0;
>   }
>   
> -void amdgpu_gfx_sysfs_fini(struct amdgpu_device *adev)
> +void amdgpu_gfx_sysfs_compute_fini(struct amdgpu_device *adev)
>   {
> -	device_remove_file(adev->dev, &dev_attr_current_compute_partition);
> -	device_remove_file(adev->dev, &dev_attr_available_compute_partition);
> +	if (!amdgpu_sriov_vf(adev)) {
> +		device_remove_file(adev->dev, &dev_attr_current_compute_partition);
> +		device_remove_file(adev->dev, &dev_attr_available_compute_partition);
> +	}
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 6b0416777c5b..b65c459b3aa9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -533,8 +533,8 @@ int amdgpu_gfx_poison_consumption_handler(struct amdgpu_device *adev,
>   						struct amdgpu_iv_entry *entry);
>   
>   bool amdgpu_gfx_is_master_xcc(struct amdgpu_device *adev, int xcc_id);
> -int amdgpu_gfx_sysfs_init(struct amdgpu_device *adev);
> -void amdgpu_gfx_sysfs_fini(struct amdgpu_device *adev);
> +int amdgpu_gfx_sysfs_compute_init(struct amdgpu_device *adev);
> +void amdgpu_gfx_sysfs_compute_fini(struct amdgpu_device *adev);
>   void amdgpu_gfx_ras_error_func(struct amdgpu_device *adev,
>   		void *ras_error_status,
>   		void (*func)(struct amdgpu_device *adev, void *ras_error_status,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> index aecc2bcea145..07ce614ef282 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> @@ -939,11 +939,11 @@ static int gfx_v9_4_3_sw_init(void *handle)
>   	if (r)
>   		return r;
>   
> +	r = amdgpu_gfx_sysfs_compute_init(adev);
> +	if (r)
> +		return r;
>   
> -	if (!amdgpu_sriov_vf(adev))
> -		r = amdgpu_gfx_sysfs_init(adev);
> -
> -	return r;
> +	return 0;
>   }
>   
>   static int gfx_v9_4_3_sw_fini(void *handle)
> @@ -964,8 +964,7 @@ static int gfx_v9_4_3_sw_fini(void *handle)
>   	gfx_v9_4_3_mec_fini(adev);
>   	amdgpu_bo_unref(&adev->gfx.rlc.clear_state_obj);
>   	gfx_v9_4_3_free_microcode(adev);
> -	if (!amdgpu_sriov_vf(adev))
> -		amdgpu_gfx_sysfs_fini(adev);
> +	amdgpu_gfx_sysfs_compute_fini(adev);
>   
>   	return 0;
>   }