Re: [PATCH 02/17] drm/amdgpu: Add infrastructure for Cleaner Shader feature

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4037 bytes --]



Am 15.08.24 um 02:04 schrieb Alex Deucher:
> From: Srinivasan Shanmugam<srinivasan.shanmugam@amd.com>
>
> The cleaner shader is used by the CP firmware to clean LDS and GPRs
> between processes on the CUs.
>
> This adds an internal API for GFX IP code to allocate and initialize the
> cleaner shader.
>
> Cc: Christian König<christian.koenig@amd.com>
> Cc: Alex Deucher<alexander.deucher@amd.com>
> Signed-off-by: Alex Deucher<alexander.deucher@amd.com>
> Signed-off-by: Srinivasan Shanmugam<srinivasan.shanmugam@amd.com>
> Suggested-by: Christian König<christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 35 +++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 14 ++++++++++
>   2 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 9be8cafdcecc..4ed69fcfe9c1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1416,3 +1416,38 @@ void amdgpu_gfx_sysfs_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);
>   }
> +
> +int amdgpu_gfx_cleaner_shader_sw_init(struct amdgpu_device *adev,
> +				      unsigned int cleaner_shader_size)
> +{
> +	if (!adev->gfx.enable_cleaner_shader)
> +		return -EOPNOTSUPP;

It's probably better to not call the function in the first place instead 
of returning an error.

> +
> +	return amdgpu_bo_create_kernel(adev, cleaner_shader_size, PAGE_SIZE,
> +				       AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT,
> +				       &adev->gfx.cleaner_shader_obj,
> +				       &adev->gfx.cleaner_shader_gpu_addr,
> +				       (void **)&adev->gfx.cleaner_shader_cpu_ptr);
> +}
> +
> +void amdgpu_gfx_cleaner_shader_sw_fini(struct amdgpu_device *adev)
> +{
> +	if (!adev->gfx.enable_cleaner_shader)
> +		return;
> +
> +	amdgpu_bo_free_kernel(&adev->gfx.cleaner_shader_obj,
> +			      &adev->gfx.cleaner_shader_gpu_addr,
> +			      (void **)&adev->gfx.cleaner_shader_cpu_ptr);
> +}
> +
> +void amdgpu_gfx_cleaner_shader_init(struct amdgpu_device *adev,
> +				    unsigned int cleaner_shader_size,
> +				    const void *cleaner_shader_ptr)
> +{
> +	if (!adev->gfx.enable_cleaner_shader)
> +		return;
> +
> +	if (adev->gfx.cleaner_shader_cpu_ptr && cleaner_shader_ptr)
> +		memcpy_toio(adev->gfx.cleaner_shader_cpu_ptr, cleaner_shader_ptr,
> +			    cleaner_shader_size);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 6fe77e483bb7..5ff3ab7d429a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -444,6 +444,14 @@ struct amdgpu_gfx {
>   	uint32_t			*ip_dump_core;
>   	uint32_t			*ip_dump_compute_queues;
>   	uint32_t			*ip_dump_gfx_queues;
> +
> +	/* cleaner shader */
> +	struct amdgpu_bo		*cleaner_shader_obj;
> +	unsigned int                    cleaner_shader_size;
This is a parameter now and the member unused.

> +	u64				cleaner_shader_gpu_addr;
> +	void				*cleaner_shader_cpu_ptr;
> +	const void			*cleaner_shader_ptr;

> +	bool				enable_cleaner_shader;
Probably better to test if cleaner_shader_obj is allocated or not 
instead of having a separate bool.

Regards,
Christian.

>   };
>   
>   struct amdgpu_gfx_ras_reg_entry {
> @@ -545,6 +553,12 @@ void amdgpu_gfx_ras_error_func(struct amdgpu_device *adev,
>   		void *ras_error_status,
>   		void (*func)(struct amdgpu_device *adev, void *ras_error_status,
>   				int xcc_id));
> +int amdgpu_gfx_cleaner_shader_sw_init(struct amdgpu_device *adev,
> +				      unsigned int cleaner_shader_size);
> +void amdgpu_gfx_cleaner_shader_sw_fini(struct amdgpu_device *adev);
> +void amdgpu_gfx_cleaner_shader_init(struct amdgpu_device *adev,
> +				    unsigned int cleaner_shader_size,
> +				    const void *cleaner_shader_ptr);
>   
>   static inline const char *amdgpu_gfx_compute_mode_desc(int mode)
>   {

[-- Attachment #2: Type: text/html, Size: 5764 bytes --]