[PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm

[Chen, Guchun]

Re: Do we need to handle this differently for BOCO?

No. feature mask applies both for BOCO and BACO.

I have a discussion with Evan, we shall focus on BACO bit instead of the whole feature mask to simply this patch. So will try to update it.

Regards,
Guchun

-----Original Message-----
From: Alex Deucher <alexdeucher at gmail.com> 
Sent: Monday, July 11, 2022 11:30 PM
To: Chen, Guchun <Guchun.Chen at amd.com>
Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>; Quan, Evan <Evan.Quan at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: use cached SMU feature mask in runpm

On Mon, Jul 11, 2022 at 9:58 AM Guchun Chen <guchun.chen at amd.com> wrote:
>
> SMU will perform dpm disablement when entering BACO, and enable them 
> later on, so talking to SMU to get enabled features in runpm cycle as 
> BACO support check is not reliable. Hence, use a cached value to fix 
> it.
>
> Signed-off-by: Guchun Chen <guchun.chen at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       | 4 ++++
>  drivers/gpu/drm/amd/pm/amdgpu_dpm.c           | 9 +++++++++
>  drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       | 1 +
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 5 +++++
>  drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 3 +++
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 8 +++++++-
>  6 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 1cc9260e75de..dc2e78bb7224 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2478,6 +2478,10 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>         }
>
>         adev->in_runpm = true;
> +
> +       /* cache SMU feature mask */
> +       amdgpu_dpm_set_cached_feature_mask(adev);
> +
>         if (amdgpu_device_supports_px(drm_dev))
>                 drm_dev->switch_power_state = 
> DRM_SWITCH_POWER_CHANGING;
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c 
> b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 956b6ce81c84..211f73a987d6 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -1702,3 +1702,12 @@ int amdgpu_dpm_get_dpm_clock_table(struct 
> amdgpu_device *adev,
>
>         return ret;
>  }
> +
> +void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev) {
> +       struct smu_context *smu = adev->powerplay.pp_handle;
> +
> +       mutex_lock(&adev->pm.mutex);
> +       smu_set_cached_enabled_mask(smu);
> +       mutex_unlock(&adev->pm.mutex); }
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h 
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 524fb09437e5..e9c002a561c2 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -543,4 +543,5 @@ enum pp_smu_status amdgpu_dpm_get_uclk_dpm_states(struct amdgpu_device *adev,
>                                                   unsigned int 
> *num_states);  int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
>                                    struct dpm_clocks *clock_table);
> +void amdgpu_dpm_set_cached_feature_mask(struct amdgpu_device *adev);
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index fd79b213fab4..e8ead58a00b4 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -3130,3 +3130,8 @@ int smu_send_hbm_bad_channel_flag(struct 
> smu_context *smu, uint32_t size)
>
>         return ret;
>  }
> +
> +void smu_set_cached_enabled_mask(struct smu_context *smu) {
> +       smu_feature_get_enabled_mask(smu, &smu->cache_enabled_mask); }
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h 
> b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index b81c657c7386..678123b5e2bf 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -568,6 +568,8 @@ struct smu_context
>         u32 param_reg;
>         u32 msg_reg;
>         u32 resp_reg;
> +
> +       uint64_t cache_enabled_mask;
>  };
>
>  struct i2c_adapter;
> @@ -1465,5 +1467,6 @@ int smu_stb_collect_info(struct smu_context 
> *smu, void *buff, uint32_t size);  void 
> amdgpu_smu_stb_debug_fs_init(struct amdgpu_device *adev);  int 
> smu_send_hbm_bad_pages_num(struct smu_context *smu, uint32_t size);  
> int smu_send_hbm_bad_channel_flag(struct smu_context *smu, uint32_t 
> size);
> +void smu_set_cached_enabled_mask(struct smu_context *smu);
>  #endif
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 15e4298c7cc8..b3087085622a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -499,7 +499,13 @@ int smu_cmn_feature_is_enabled(struct smu_context *smu,
>         uint64_t enabled_features;
>         int feature_id;
>
> -       if (__smu_get_enabled_features(smu, &enabled_features)) {
> +       /* SMU will perform dpm disablement when entering BACO, and enable
> +        * them later on, so talking to SMU to get enabled features in runpm
> +        * stage is not reliable. Use a cache value for this instead to fix it.
> +        */
> +       if (adev->in_runpm) {
> +               enabled_features = smu->cache_enabled_mask;

Do we need to handle this differently for BOCO?

Alex

> +       } else if (__smu_get_enabled_features(smu, &enabled_features)) 
> + {
>                 dev_err(adev->dev, "Failed to retrieve enabled ppfeatures!\n");
>                 return 0;
>         }
> --
> 2.17.1
>