On Wed, Sep 4, 2024 at 9:27 AM Srinivasan Shanmugam
<srinivasan.shanmugam@xxxxxxx> wrote:
>
> This commit refactors the cleaner shader initialization process. The
> changes remove unnecessary checks for adev->gfx.enable_cleaner_shader in
> the amdgpu_gfx_cleaner_shader_sw_init,
> amdgpu_gfx_cleaner_shader_sw_fini, and amdgpu_gfx_cleaner_shader_init
> functions. These checks are now performed before these functions are
> called, which simplifies the functions and makes the control flow of the
> program clearer.
>
> Additionally, the cleaner_shader_size and cleaner_shader_ptr parameters
> have been removed from the amdgpu_gfx_cleaner_shader_sw_init and
> amdgpu_gfx_cleaner_shader_init functions. These values are now obtained
> directly from the adev->gfx structure inside the functions.
>
> Fixes: 63063b6c5a8d ("drm/amdgpu: Add infrastructure for Cleaner Shader feature")
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> Suggested-by: Christian König <christian.koenig@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 21 ++++++---------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 7 ++-----
> 2 files changed, 8 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 83e54697f0ee..9891114ae6d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1655,13 +1655,10 @@ void amdgpu_gfx_sysfs_isolation_shader_fini(struct amdgpu_device *adev)
> device_remove_file(adev->dev, &dev_attr_run_cleaner_shader);
> }
>
> -int amdgpu_gfx_cleaner_shader_sw_init(struct amdgpu_device *adev,
> - unsigned int cleaner_shader_size)
> +int amdgpu_gfx_cleaner_shader_sw_init(struct amdgpu_device *adev)
> {
> - if (!adev->gfx.enable_cleaner_shader)
> - return -EOPNOTSUPP;
>
> - return amdgpu_bo_create_kernel(adev, cleaner_shader_size, PAGE_SIZE,
> + return amdgpu_bo_create_kernel(adev, adev->gfx.cleaner_shader_size, PAGE_SIZE,
> AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT,
> &adev->gfx.cleaner_shader_obj,
> &adev->gfx.cleaner_shader_gpu_addr,
> @@ -1670,24 +1667,18 @@ int amdgpu_gfx_cleaner_shader_sw_init(struct amdgpu_device *adev,
>
> 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)
> +void amdgpu_gfx_cleaner_shader_init(struct amdgpu_device *adev)
> {
> - 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);
> + if (adev->gfx.cleaner_shader_cpu_ptr && adev->gfx.cleaner_shader_ptr)
This is confusing. Two cpu pointers with pretty much the same name.
Alex
> + memcpy_toio(adev->gfx.cleaner_shader_cpu_ptr, adev->gfx.cleaner_shader_ptr,
> + adev->gfx.cleaner_shader_size);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 5644e10a86a9..07bd27c066c3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -573,12 +573,9 @@ 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);
> +int amdgpu_gfx_cleaner_shader_sw_init(struct amdgpu_device *adev);
> 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);
> +void amdgpu_gfx_cleaner_shader_init(struct amdgpu_device *adev);
> int amdgpu_gfx_sysfs_isolation_shader_init(struct amdgpu_device *adev);
> void amdgpu_gfx_sysfs_isolation_shader_fini(struct amdgpu_device *adev);
> void amdgpu_gfx_enforce_isolation_handler(struct work_struct *work);
> --
> 2.34.1
>
