[PATCH] drm/amd/pm: re-enable the gfx imu when smu resume

[Huang, Tim]

[Public]

-----Original Message-----
From: Chen, Guchun <Guchun.Chen at amd.com>
Sent: Friday, March 24, 2023 3:26 PM
To: Huang, Tim <Tim.Huang at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Zhang, Yifan <Yifan1.Zhang at amd.com>; Ma, Li <Li.Ma at amd.com>; Du, Xiaojian <Xiaojian.Du at amd.com>; Huang, Tim <Tim.Huang at amd.com>
Subject: RE: [PATCH] drm/amd/pm: re-enable the gfx imu when smu resume



> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Tim
> Huang
> Sent: Friday, March 24, 2023 3:08 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Zhang, Yifan
> <Yifan1.Zhang at amd.com>; Ma, Li <Li.Ma at amd.com>; Du, Xiaojian
> <Xiaojian.Du at amd.com>; Huang, Tim <Tim.Huang at amd.com>
> Subject: [PATCH] drm/amd/pm: re-enable the gfx imu when smu resume
>
> If the gfx imu is poweroff when suspend, then it need to be re-enabled
> when resume.
>
> Signed-off-by: Tim Huang <tim.huang at amd.com>
> ---
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 40 ++++++++++++++++-
> ------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index b5d64749990e..94fe8593444a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -162,10 +162,15 @@ int smu_get_dpm_freq_range(struct smu_context
> *smu,
>
>  int smu_set_gfx_power_up_by_imu(struct smu_context *smu)  {
> -     if (!smu->ppt_funcs || !smu->ppt_funcs->set_gfx_power_up_by_imu)
> -             return -EOPNOTSUPP;
> +     int ret = 0;
> +     struct amdgpu_device *adev = smu->adev;
>
> -     return smu->ppt_funcs->set_gfx_power_up_by_imu(smu);
> +     if (smu->ppt_funcs->set_gfx_power_up_by_imu) {
> +             ret = smu->ppt_funcs->set_gfx_power_up_by_imu(smu);
> +             if (ret)
> +                     dev_err(adev->dev, "Failed to enable gfx imu!\n");
> +     }
> +     return ret;
>  }
>
>  static u32 smu_get_mclk(void *handle, bool low) @@ -196,6 +201,19 @@
> static u32 smu_get_sclk(void *handle, bool low)
>       return clk_freq * 100;
>  }
>
> +static int smu_set_gfx_imu_enable(struct smu_context *smu) {
> +     struct amdgpu_device *adev = smu->adev;
> +
> +     if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
> +             return 0;
> +
> +     if (amdgpu_in_reset(smu->adev) || adev->in_s0ix)
> +             return 0;
> +
> +     return smu_set_gfx_power_up_by_imu(smu); }
> +
>  static int smu_dpm_set_vcn_enable(struct smu_context *smu,
>                                 bool enable)
>  {
> @@ -1396,15 +1414,9 @@ static int smu_hw_init(void *handle)
>       }
>
>       if (smu->is_apu) {
> -             if ((smu->ppt_funcs->set_gfx_power_up_by_imu) &&
> -                             likely(adev->firmware.load_type ==
> AMDGPU_FW_LOAD_PSP)) {
> -                     ret = smu->ppt_funcs-
> >set_gfx_power_up_by_imu(smu);
> -                     if (ret) {
> -                             dev_err(adev->dev, "Failed to Enable gfx
> imu!\n");
> -                             return ret;
> -                     }
> -             }
> -
> +             ret = smu_set_gfx_imu_enable(smu);
> +             if (ret)
> +                     return ret;
>               smu_dpm_set_vcn_enable(smu, true);
>               smu_dpm_set_jpeg_enable(smu, true);
>               smu_set_gfx_cgpg(smu, true);
> @@ -1681,6 +1693,10 @@ static int smu_resume(void *handle)
>               return ret;
>       }
>
> +     ret = smu_set_gfx_imu_enable(smu);
> +     if (ret)
> +             return ret;

>> smu_set_gfx_imu_enable in smu_hw_init is valid when sum->is_apu is true. So such check is still necessary in smu_resume?

>>Regards,
>>Guchun

For both hw_init or resume, it should be ok if not have the smu->is_apu checking. Because we will check the valid of smu->ppt_funcs->set_gfx_power_up_by_imu in the final call path,
so, for dGPU or APUs without IMU that did not implement this interface will return 0.   Thanks.

Tim

>       smu_set_gfx_cgpg(smu, true);
>
>       smu->disable_uclk_switch = 0;
> --
> 2.25.1