Re: [PATCH 4/7] drm/amd/pm: stage od reset for smu 13.0.6

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 31-Aug-26 10:05 AM, Kevin Wang wrote:
> [Some people who received this message don't often get email from kevin.wang@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> SMU 13.0.6 applies default GFXCLK, UCLK, and FCLK ranges directly from
> PP_OD_RESTORE_DEFAULT_TABLE. This bypasses the staged OD transaction.
> 
> Stage defaults through one helper and let c submit them. Gate UCLK and
> FCLK staging and commit on their DPM features, and require the UCLK
> firmware capability before staging or submitting its limit.
> 
> Signed-off-by: Kevin Wang <kevin.wang@amd.com>
> ---
>   .../drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c  | 85 ++++++++++---------
>   1 file changed, 44 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> index 4dec3a93cb3d..169e38db629d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_6_ppt.c
> @@ -2130,14 +2130,45 @@ static int smu_v13_0_6_set_soft_freq_limited_range(struct smu_context *smu,
>          return ret;
>   }
> 
> +static void smu_v13_0_6_stage_default_dpm_limits(struct smu_context *smu)
> +{
> +       struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
> +       struct smu_13_0_dpm_context *dpm_context = smu_dpm->dpm_context;
> +       struct smu_umd_pstate_table *pstate_table = &smu->pstate_table;
> +
> +       pstate_table->gfxclk_pstate.custom.min =
> +               SMU_DPM_TABLE_MIN(&dpm_context->dpm_tables.gfx_table);
> +       pstate_table->gfxclk_pstate.custom.max =
> +               SMU_DPM_TABLE_MAX(&dpm_context->dpm_tables.gfx_table);
> +
> +       if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_UCLK_BIT) &&
> +           smu_v13_0_6_cap_supported(smu, SMU_CAP(SET_UCLK_MAX))) {
> +               pstate_table->uclk_pstate.custom.min =
> +                       SMU_DPM_TABLE_MIN(&dpm_context->dpm_tables.uclk_table);
> +               pstate_table->uclk_pstate.custom.max =
> +                       SMU_DPM_TABLE_MAX(&dpm_context->dpm_tables.uclk_table);
> +       } else {
> +               pstate_table->uclk_pstate.custom.min = 0;
> +               pstate_table->uclk_pstate.custom.max = 0;
> +       }
> +
> +       if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_FCLK_BIT)) {
> +               pstate_table->fclk_pstate.custom.min =
> +                       SMU_DPM_TABLE_MIN(&dpm_context->dpm_tables.fclk_table);
> +               pstate_table->fclk_pstate.custom.max =
> +                       SMU_DPM_TABLE_MAX(&dpm_context->dpm_tables.fclk_table);
> +       } else {
> +               pstate_table->fclk_pstate.custom.min = 0;
> +               pstate_table->fclk_pstate.custom.max = 0;
> +       }
> +}

The default values shouldn't be put as custom min/max limits. The custom 
limits will keep non-zero values to indicate any custom limit is set or 
not. It's not used to keep the default limits.
> +
>   static int smu_v13_0_6_usr_edit_dpm_table(struct smu_context *smu,
>                                            enum PP_OD_DPM_TABLE_COMMAND type,
>                                            long input[], uint32_t size)
>   {
>          struct smu_dpm_context *smu_dpm = &(smu->smu_dpm);
>          struct smu_13_0_dpm_context *dpm_context = smu_dpm->dpm_context;
> -       struct smu_dpm_table *uclk_table = &dpm_context->dpm_tables.uclk_table;
> -       struct smu_dpm_table *fclk_table = &dpm_context->dpm_tables.fclk_table;
>          struct smu_umd_pstate_table *pstate_table = &smu->pstate_table;
>          uint32_t min_clk;
>          uint32_t max_clk;
> @@ -2258,42 +2289,8 @@ static int smu_v13_0_6_usr_edit_dpm_table(struct smu_context *smu,
>                          dev_err(smu->adev->dev,
>                                  "Input parameter number not correct\n");
>                          return -EINVAL;
> -               } else {
> -                       /* Use the default frequencies for manual and determinism mode */
> -                       min_clk = SMU_DPM_TABLE_MIN(
> -                               &dpm_context->dpm_tables.gfx_table);
> -                       max_clk = SMU_DPM_TABLE_MAX(
> -                               &dpm_context->dpm_tables.gfx_table);
> -
> -                       ret = smu_v13_0_6_set_soft_freq_limited_range(
> -                               smu, SMU_GFXCLK, min_clk, max_clk, false);
> -
> -                       if (ret)
> -                               return ret;
> -
> -                       if (SMU_DPM_TABLE_MAX(uclk_table) !=
> -                           pstate_table->uclk_pstate.curr.max) {
> -                               min_clk = SMU_DPM_TABLE_MIN(&dpm_context->dpm_tables.uclk_table);
> -                               max_clk = SMU_DPM_TABLE_MAX(&dpm_context->dpm_tables.uclk_table);
> -                               ret = smu_v13_0_6_set_soft_freq_limited_range(smu,
> -                                                                             SMU_UCLK, min_clk,
> -                                                                             max_clk, false);
> -                               if (ret)
> -                                       return ret;
> -                       }
> -
> -                       if (SMU_DPM_TABLE_MAX(fclk_table) !=
> -                           pstate_table->fclk_pstate.curr.max) {
> -                               max_clk = SMU_DPM_TABLE_MAX(&dpm_context->dpm_tables.fclk_table);
> -                               min_clk = SMU_DPM_TABLE_MIN(&dpm_context->dpm_tables.fclk_table);
> -                               ret = smu_v13_0_6_set_soft_freq_limited_range(smu,
> -                                                                             SMU_FCLK, min_clk,
> -                                                                             max_clk, false);
> -                               if (ret)
> -                                       return ret;
> -                       }
> -                       smu_v13_0_reset_custom_level(smu);
>                  }
> +               smu_v13_0_6_stage_default_dpm_limits(smu);
>                  break;
>          case PP_OD_COMMIT_DPM_TABLE:
>                  if (size != 0) {
> @@ -2318,8 +2315,10 @@ static int smu_v13_0_6_usr_edit_dpm_table(struct smu_context *smu,
>                          if (ret)
>                                  return ret;
> 
> -                       if (pstate_table->fclk_pstate.custom.max) {
> -                               min_clk = pstate_table->fclk_pstate.curr.min;
> +                       if (pstate_table->fclk_pstate.custom.max &&
> +                           smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_FCLK_BIT)) {
> +                               min_clk = pstate_table->fclk_pstate.custom.min ?:
> +                                       pstate_table->fclk_pstate.curr.min;

This is intentional to always enforce the min limit, there is no custom 
min limit allowed for FCLK.

>                                  max_clk = pstate_table->fclk_pstate.custom.max;
>                                  ret = smu_v13_0_6_set_soft_freq_limited_range(smu,
>                                                                                SMU_FCLK, min_clk,
> @@ -2328,10 +2327,14 @@ static int smu_v13_0_6_usr_edit_dpm_table(struct smu_context *smu,
>                                          return ret;
>                          }
> 
> -                       if (!pstate_table->uclk_pstate.custom.max)
> +                       if (!pstate_table->uclk_pstate.custom.max ||
> +                           !smu_cmn_feature_is_enabled(smu, SMU_FEATURE_DPM_UCLK_BIT) ||
> +                           !smu_v13_0_6_cap_supported(smu,
> +                                                            SMU_CAP(SET_UCLK_MAX)))
>                                  return 0;
> 
> -                       min_clk = pstate_table->uclk_pstate.curr.min;
> +                       min_clk = pstate_table->uclk_pstate.custom.min ?:

Same here, there is no custom min allowed for UCLK.

Thanks,
Lijo

> +                               pstate_table->uclk_pstate.curr.min;
>                          max_clk = pstate_table->uclk_pstate.custom.max;
>                          return smu_v13_0_6_set_soft_freq_limited_range(
>                                  smu, SMU_UCLK, min_clk, max_clk, false);
> --
> 2.55.0
>