[PATCH 3/3] drm/amdgpu/gfx9: Refactor cleaner shader initialization for GFX9.4.3

[Alex Deucher]

On Wed, Sep 4, 2024 at 9:38 AM Srinivasan Shanmugam
<srinivasan.shanmugam at amd.com> wrote:
>
> This commit modifies the initialization only if the cleaner shader
> object has been allocated. This is done by adding checks for
> adev->gfx.cleaner_shader_obj before calling
> amdgpu_gfx_cleaner_shader_init
>
> The changes are made in the gfx_v9_4_3_sw_init, gfx_v9_4_3_sw_fini, and
> gfx_v9_4_3_hw_init functions. These functions are responsible for
> initializing software components of the GFX v9.4.3 engines.
>
> This change prevents unnecessary function calls and makes the control
> flow of the program clearer. It also ensures that the cleaner shader is
> only initialized when it has been properly allocated.
>
> Fixes: 1b66421d29b7 ("drm/amdgpu/gfx9: Implement cleaner shader support for GFX9.4.3 hardware")
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Suggested-by: Christian König <christian.koenig at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> 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 408e5600bb61..abf934863421 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> @@ -1061,10 +1061,12 @@ static int gfx_v9_4_3_sw_init(void *handle)
>                 adev->gfx.cleaner_shader_size = sizeof(gfx_9_4_3_cleaner_shader_hex);
>                 if (adev->gfx.mec_fw_version >= 153) {
>                         adev->gfx.enable_cleaner_shader = true;
> -                       r = amdgpu_gfx_cleaner_shader_sw_init(adev, adev->gfx.cleaner_shader_size);
> -                       if (r) {
> -                               adev->gfx.enable_cleaner_shader = false;
> -                               dev_err(adev->dev, "Failed to initialize cleaner shader\n");
> +                       if (adev->gfx.cleaner_shader_obj) {

This check doesn't make any sense. This function is where we allocate
the cleaner shader object.

> +                               r = amdgpu_gfx_cleaner_shader_sw_init(adev);
> +                               if (r) {
> +                                       adev->gfx.enable_cleaner_shader = false;
> +                                       dev_err(adev->dev, "Failed to initialize cleaner shader\n");
> +                               }
>                         }
>                 }
>                 break;
> @@ -1196,7 +1198,8 @@ static int gfx_v9_4_3_sw_fini(void *handle)
>                 amdgpu_gfx_kiq_fini(adev, i);
>         }
>
> -       amdgpu_gfx_cleaner_shader_sw_fini(adev);
> +       if (adev->gfx.cleaner_shader_obj)
> +               amdgpu_gfx_cleaner_shader_sw_fini(adev);

Is this check actually needed?  I think amdgpu_bo_free_kernel() can
deal with a NULL pointer.


>
>         gfx_v9_4_3_mec_fini(adev);
>         amdgpu_bo_unref(&adev->gfx.rlc.clear_state_obj);
> @@ -2344,8 +2347,8 @@ static int gfx_v9_4_3_hw_init(void *handle)
>         int r;
>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> -       amdgpu_gfx_cleaner_shader_init(adev, adev->gfx.cleaner_shader_size,
> -                                      adev->gfx.cleaner_shader_ptr);
> +       if (adev->gfx.cleaner_shader_obj)
> +               amdgpu_gfx_cleaner_shader_init(adev);

Same comment as patch 2.


>
>         if (!amdgpu_sriov_vf(adev))
>                 gfx_v9_4_3_init_golden_registers(adev);
> --
> 2.34.1
>