<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> [AMD Public Use]<o:p></o:p> <o:p> </o:p> Hi Kevin,<o:p></o:p> <o:p> </o:p> This case is for determinism mode - there it uses the max frequency passed and min_clk is the default min clock.<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Lijo<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com> Sent: Monday, May 17, 2021 2:32 PM To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com> Subject: Re: [PATCH v2 2/3] drm/amd/pm: Fix showing incorrect frequencies on aldebaran<o:p></o:p> </div> </div> <o:p> </o:p> [AMD Public Use]<o:p></o:p> <o:p> </o:p> <div> <div> Hi Lijo,<o:p></o:p> <o:p> </o:p> + pstate_table->gfxclk_pstate.curr.min = min_clk;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.max = max;<o:p></o:p> <o:p> </o:p> min_clk and max,<o:p></o:p> it seems it is coding error, is right?<o:p></o:p> <o:p> </o:p> Best Regards,<o:p></o:p> Kevin<o:p></o:p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> From: Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>> Sent: Monday, May 17, 2021 4:39 PM To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>> Cc: Zhang, Hawking <<a href="mailto:Hawking.Zhang@amd.com">Hawking.Zhang@amd.com</a>>; Wang, Kevin(Yang) <<a href="mailto:Kevin1.Wang@amd.com">Kevin1.Wang@amd.com</a>>; Chen, Guchun <<a href="mailto:Guchun.Chen@amd.com">Guchun.Chen@amd.com</a>>; Feng, Kenneth <<a href="mailto:Kenneth.Feng@amd.com">Kenneth.Feng@amd.com</a>> Subject: [PATCH v2 2/3] drm/amd/pm: Fix showing incorrect frequencies on aldebaran <o:p></o:p> <div> <o:p></o:p> </div> </div> <div> [AMD Public Use]<o:p></o:p> <o:p> </o:p> <div> v1: Use the current and custom pstate frequencies to track the current and<o:p></o:p> user-set min/max values in manual and determinism mode. Previously, only<o:p></o:p> actual_* value was used to track the currrent and user requested value.<o:p></o:p> The value will get reassigned whenever user requests a new value with<o:p></o:p> pp_od_clk_voltage node. Hence it will show incorrect values when user<o:p></o:p> requests an invalid value or tries a partial request without committing<o:p></o:p> the values. Separating out to custom and current variable fixes such<o:p></o:p> issues.<o:p></o:p> <o:p></o:p> v2: Remove redundant if-else check<o:p></o:p> <o:p></o:p> Signed-off-by: Lijo Lazar <a href="mailto:lijo.lazar@amd.com"> lijo.lazar@amd.com</a><o:p></o:p> ---<o:p></o:p> .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 65 ++++++++++++-------<o:p></o:p> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 6 ++<o:p></o:p> 2 files changed, 46 insertions(+), 25 deletions(-)<o:p></o:p> <o:p></o:p> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c<o:p></o:p> index 5d04a1dfdfd8..d27ed2954705 100644<o:p></o:p> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c<o:p></o:p> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c<o:p></o:p> @@ -78,8 +78,6 @@<o:p></o:p> #define smnPCIE_ESM_CTRL 0x111003D0<o:p></o:p> -#define CLOCK_VALID (1 << 31)<o:p></o:p> -<o:p></o:p> static const struct cmn2asic_msg_mapping aldebaran_message_map[SMU_MSG_MAX_COUNT] = {<o:p></o:p> MSG_MAP(TestMessage, PPSMC_MSG_TestMessage, 0),<o:p></o:p> MSG_MAP(GetSmuVersion, PPSMC_MSG_GetSmuVersion, 1),<o:p></o:p> @@ -455,12 +453,18 @@ static int aldebaran_populate_umd_state_clk(struct smu_context *smu)<o:p></o:p> pstate_table->gfxclk_pstate.min = gfx_table->min;<o:p></o:p> pstate_table->gfxclk_pstate.peak = gfx_table->max;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.min = gfx_table->min;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.max = gfx_table->max;<o:p></o:p> pstate_table->uclk_pstate.min = mem_table->min;<o:p></o:p> pstate_table->uclk_pstate.peak = mem_table->max;<o:p></o:p> + pstate_table->uclk_pstate.curr.min = mem_table->min;<o:p></o:p> + pstate_table->uclk_pstate.curr.max = mem_table->max;<o:p></o:p> pstate_table->socclk_pstate.min = soc_table->min;<o:p></o:p> pstate_table->socclk_pstate.peak = soc_table->max;<o:p></o:p> + pstate_table->socclk_pstate.curr.min = soc_table->min;<o:p></o:p> + pstate_table->socclk_pstate.curr.max = soc_table->max;<o:p></o:p> if (gfx_table->count > ALDEBARAN_UMD_PSTATE_GFXCLK_LEVEL &&<o:p></o:p> mem_table->count > ALDEBARAN_UMD_PSTATE_MCLK_LEVEL &&<o:p></o:p> @@ -669,6 +673,7 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,<o:p></o:p> {<o:p></o:p> int i, now, size = 0;<o:p></o:p> int ret = 0;<o:p></o:p> + struct smu_umd_pstate_table *pstate_table = &smu->pstate_table;<o:p></o:p> struct pp_clock_levels_with_latency clocks;<o:p></o:p> struct smu_13_0_dpm_table *single_dpm_table;<o:p></o:p> struct smu_dpm_context *smu_dpm = &smu->smu_dpm;<o:p></o:p> @@ -703,12 +708,8 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,<o:p></o:p> display_levels = clocks.num_levels;<o:p></o:p> - min_clk = smu->gfx_actual_hard_min_freq & CLOCK_VALID ?<o:p></o:p> - smu->gfx_actual_hard_min_freq & ~CLOCK_VALID :<o:p></o:p> - single_dpm_table->dpm_levels[0].value;<o:p></o:p> - max_clk = smu->gfx_actual_soft_max_freq & CLOCK_VALID ?<o:p></o:p> - smu->gfx_actual_soft_max_freq & ~CLOCK_VALID :<o:p></o:p> - single_dpm_table->dpm_levels[1].value;<o:p></o:p> + min_clk = pstate_table->gfxclk_pstate.curr.min;<o:p></o:p> + max_clk = pstate_table->gfxclk_pstate.curr.max;<o:p></o:p> freq_values[0] = min_clk;<o:p></o:p> freq_values[1] = max_clk;<o:p></o:p> @@ -1134,9 +1135,6 @@ static int aldebaran_set_performance_level(struct smu_context *smu,<o:p></o:p> && (level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM))<o:p></o:p> smu_cmn_send_smc_msg(smu, SMU_MSG_DisableDeterminism, NULL);<o:p></o:p> - /* Reset user min/max gfx clock */<o:p></o:p> - smu->gfx_actual_hard_min_freq = 0;<o:p></o:p> - smu->gfx_actual_soft_max_freq = 0;<o:p></o:p> switch (level) {<o:p></o:p> @@ -1163,6 +1161,7 @@ static int aldebaran_set_soft_freq_limited_range(struct smu_context *smu,<o:p></o:p> {<o:p></o:p> struct smu_dpm_context *smu_dpm = &(smu->smu_dpm);<o:p></o:p> struct smu_13_0_dpm_context *dpm_context = smu_dpm->dpm_context;<o:p></o:p> + struct smu_umd_pstate_table *pstate_table = &smu->pstate_table;<o:p></o:p> struct amdgpu_device *adev = smu->adev;<o:p></o:p> uint32_t min_clk;<o:p></o:p> uint32_t max_clk;<o:p></o:p> @@ -1176,14 +1175,20 @@ static int aldebaran_set_soft_freq_limited_range(struct smu_context *smu,<o:p></o:p> return -EINVAL;<o:p></o:p> if (smu_dpm->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) {<o:p></o:p> - min_clk = max(min, dpm_context->dpm_tables.gfx_table.min);<o:p></o:p> - max_clk = min(max, dpm_context->dpm_tables.gfx_table.max);<o:p></o:p> - ret = smu_v13_0_set_soft_freq_limited_range(smu, SMU_GFXCLK,<o:p></o:p> - min_clk, max_clk);<o:p></o:p> + if (min >= max) {<o:p></o:p> + dev_err(smu->adev->dev,<o:p></o:p> + "Minimum GFX clk should be less than the maximum allowed clock\n");<o:p></o:p> + return -EINVAL;<o:p></o:p> + }<o:p></o:p> + if ((min == pstate_table->gfxclk_pstate.curr.min) &&<o:p></o:p> + (max == pstate_table->gfxclk_pstate.curr.max))<o:p></o:p> + return 0;<o:p></o:p> + ret = smu_v13_0_set_soft_freq_limited_range(smu, SMU_GFXCLK,<o:p></o:p> + min, max);<o:p></o:p> if (!ret) {<o:p></o:p> - smu->gfx_actual_hard_min_freq = min_clk | CLOCK_VALID;<o:p></o:p> - smu->gfx_actual_soft_max_freq = max_clk | CLOCK_VALID;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.min = min;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.max = max;<o:p></o:p> }<o:p></o:p> return ret;<o:p></o:p> }<o:p></o:p> @@ -1209,10 +1214,8 @@ static int aldebaran_set_soft_freq_limited_range(struct smu_context *smu,<o:p></o:p> dev_err(adev->dev,<o:p></o:p> "Failed to enable determinism at GFX clock %d MHz\n", max);<o:p></o:p> } else {<o:p></o:p> - smu->gfx_actual_hard_min_freq =<o:p></o:p> - min_clk | CLOCK_VALID;<o:p></o:p> - smu->gfx_actual_soft_max_freq =<o:p></o:p> - max | CLOCK_VALID;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.min = min_clk;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.max = max;<o:p></o:p> }<o:p></o:p> }<o:p></o:p> }<o:p></o:p> @@ -1225,6 +1228,7 @@ static int aldebaran_usr_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_<o:p></o:p> {<o:p></o:p> struct smu_dpm_context *smu_dpm = &(smu->smu_dpm);<o:p></o:p> struct smu_13_0_dpm_context *dpm_context = smu_dpm->dpm_context;<o:p></o:p> + struct smu_umd_pstate_table *pstate_table = &smu->pstate_table;<o:p></o:p> uint32_t min_clk;<o:p></o:p> uint32_t max_clk;<o:p></o:p> int ret = 0;<o:p></o:p> @@ -1245,16 +1249,20 @@ static int aldebaran_usr_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_<o:p></o:p> if (input[1] < dpm_context->dpm_tables.gfx_table.min) {<o:p></o:p> dev_warn(smu->adev->dev, "Minimum GFX clk (%ld) MHz specified is less than the minimum allowed (%d) MHz\n",<o:p></o:p> input[1], dpm_context->dpm_tables.gfx_table.min);<o:p></o:p> + pstate_table->gfxclk_pstate.custom.min =<o:p></o:p> + pstate_table->gfxclk_pstate.curr.min;<o:p></o:p> return -EINVAL;<o:p></o:p> }<o:p></o:p> - smu->gfx_actual_hard_min_freq = input[1];<o:p></o:p> + pstate_table->gfxclk_pstate.custom.min = input[1];<o:p></o:p> } else if (input[0] == 1) {<o:p></o:p> if (input[1] > dpm_context->dpm_tables.gfx_table.max) {<o:p></o:p> dev_warn(smu->adev->dev, "Maximum GFX clk (%ld) MHz specified is greater than the maximum allowed (%d) MHz\n",<o:p></o:p> input[1], dpm_context->dpm_tables.gfx_table.max);<o:p></o:p> + pstate_table->gfxclk_pstate.custom.max =<o:p></o:p> + pstate_table->gfxclk_pstate.curr.max;<o:p></o:p> return -EINVAL;<o:p></o:p> }<o:p></o:p> - smu->gfx_actual_soft_max_freq = input[1];<o:p></o:p> + pstate_table->gfxclk_pstate.custom.max = input[1];<o:p></o:p> } else {<o:p></o:p> return -EINVAL;<o:p></o:p> }<o:p></o:p> @@ -1276,8 +1284,15 @@ static int aldebaran_usr_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_<o:p></o:p> dev_err(smu->adev->dev, "Input parameter number not correct\n");<o:p></o:p> return -EINVAL;<o:p></o:p> } else {<o:p></o:p> - min_clk = smu->gfx_actual_hard_min_freq;<o:p></o:p> - max_clk = smu->gfx_actual_soft_max_freq;<o:p></o:p> + if (!pstate_table->gfxclk_pstate.custom.min)<o:p></o:p> + pstate_table->gfxclk_pstate.custom.min =<o:p></o:p> + pstate_table->gfxclk_pstate.curr.min;<o:p></o:p> + if (!pstate_table->gfxclk_pstate.custom.max)<o:p></o:p> + pstate_table->gfxclk_pstate.custom.max =<o:p></o:p> + pstate_table->gfxclk_pstate.curr.max;<o:p></o:p> + min_clk = pstate_table->gfxclk_pstate.custom.min;<o:p></o:p> + max_clk = pstate_table->gfxclk_pstate.custom.max;<o:p></o:p> +<o:p></o:p> return aldebaran_set_soft_freq_limited_range(smu, SMU_GFXCLK, min_clk, max_clk);<o:p></o:p> }<o:p></o:p> break;<o:p></o:p> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c<o:p></o:p> index 0864083e7435..6b3374432e1d 100644<o:p></o:p> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c<o:p></o:p> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c<o:p></o:p> @@ -1626,6 +1626,8 @@ int smu_v13_0_set_performance_level(struct smu_context *smu,<o:p></o:p> sclk_max);<o:p></o:p> if (ret)<o:p></o:p> return ret;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.min = sclk_min;<o:p></o:p> + pstate_table->gfxclk_pstate.curr.max = sclk_max;<o:p></o:p> }<o:p></o:p> if (mclk_min && mclk_max) {<o:p></o:p> @@ -1635,6 +1637,8 @@ int smu_v13_0_set_performance_level(struct smu_context *smu,<o:p></o:p> mclk_max);<o:p></o:p> if (ret)<o:p></o:p> return ret;<o:p></o:p> + pstate_table->uclk_pstate.curr.min = mclk_min;<o:p></o:p> + pstate_table->uclk_pstate.curr.max = mclk_max;<o:p></o:p> }<o:p></o:p> if (socclk_min && socclk_max) {<o:p></o:p> @@ -1644,6 +1648,8 @@ int smu_v13_0_set_performance_level(struct smu_context *smu,<o:p></o:p> socclk_max);<o:p></o:p> if (ret)<o:p></o:p> return ret;<o:p></o:p> + pstate_table->socclk_pstate.curr.min = socclk_min;<o:p></o:p> + pstate_table->socclk_pstate.curr.max = socclk_max;<o:p></o:p> }<o:p></o:p> return ret;<o:p></o:p> -- <o:p></o:p> 2.17.1<o:p></o:p> <o:p></o:p> </div> </div> </div> </div> </body> </html>