[PATCH 1/3] drm/amdgpu: Refactor cleaner shader initialization in amdgpu

[Alex Deucher]

On Wed, Sep 4, 2024 at 9:27 AM Srinivasan Shanmugam
<srinivasan.shanmugam at amd.com> 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 at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> Suggested-by: Christian König <christian.koenig at amd.com>
> ---
>  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
>