<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> [AMD Public Use] <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> ok, it is fine for me,</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> and some code blocks need to have a blank line to match kernel coding style.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> with that fixes, Series is </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Reviewed-by: Kevin Wang <kevin1.wang@amd.com></div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Best Regards, Kevin</div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="divRplyFwdMsg" dir="ltr">From: Lazar, Lijo <Lijo.Lazar@amd.com> Sent: Monday, May 17, 2021 5:04 PM To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <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 <div> </div> </div> <div lang="EN-US" style="word-wrap:break-word"> <div class="x_WordSection1"> [AMD Public Use] Hi Kevin, This case is for determinism mode - there it uses the max frequency passed and min_clk is the default min clock. Thanks, Lijo <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 </div> </div> [AMD Public Use] <div> <div> Hi Lijo, + pstate_table->gfxclk_pstate.curr.min = min_clk; + pstate_table->gfxclk_pstate.curr.max = max; min_clk and max, it seems it is coding error, is right? Best Regards, Kevin </div> <div class="x_MsoNormal" align="center" style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="x_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 <div> </div> </div> <div> [AMD Public Use] <div> v1: Use the current and custom pstate frequencies to track the current and user-set min/max values in manual and determinism mode. Previously, only actual_* value was used to track the currrent and user requested value. The value will get reassigned whenever user requests a new value with pp_od_clk_voltage node. Hence it will show incorrect values when user requests an invalid value or tries a partial request without committing the values. Separating out to custom and current variable fixes such issues. v2: Remove redundant if-else check Signed-off-by: Lijo Lazar <a href="mailto:lijo.lazar@amd.com">lijo.lazar@amd.com</a> --- .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 65 ++++++++++++------- .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 6 ++ 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c index 5d04a1dfdfd8..d27ed2954705 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c @@ -78,8 +78,6 @@ #define smnPCIE_ESM_CTRL 0x111003D0 -#define CLOCK_VALID (1 << 31) - static const struct cmn2asic_msg_mapping aldebaran_message_map[SMU_MSG_MAX_COUNT] = { MSG_MAP(TestMessage, PPSMC_MSG_TestMessage, 0), MSG_MAP(GetSmuVersion, PPSMC_MSG_GetSmuVersion, 1), @@ -455,12 +453,18 @@ static int aldebaran_populate_umd_state_clk(struct smu_context *smu) pstate_table->gfxclk_pstate.min = gfx_table->min; pstate_table->gfxclk_pstate.peak = gfx_table->max; + pstate_table->gfxclk_pstate.curr.min = gfx_table->min; + pstate_table->gfxclk_pstate.curr.max = gfx_table->max; pstate_table->uclk_pstate.min = mem_table->min; pstate_table->uclk_pstate.peak = mem_table->max; + pstate_table->uclk_pstate.curr.min = mem_table->min; + pstate_table->uclk_pstate.curr.max = mem_table->max; pstate_table->socclk_pstate.min = soc_table->min; pstate_table->socclk_pstate.peak = soc_table->max; + pstate_table->socclk_pstate.curr.min = soc_table->min; + pstate_table->socclk_pstate.curr.max = soc_table->max; if (gfx_table->count > ALDEBARAN_UMD_PSTATE_GFXCLK_LEVEL && mem_table->count > ALDEBARAN_UMD_PSTATE_MCLK_LEVEL && @@ -669,6 +673,7 @@ static int aldebaran_print_clk_levels(struct smu_context *smu, { int i, now, size = 0; int ret = 0; + struct smu_umd_pstate_table *pstate_table = &smu->pstate_table; struct pp_clock_levels_with_latency clocks; struct smu_13_0_dpm_table *single_dpm_table; struct smu_dpm_context *smu_dpm = &smu->smu_dpm; @@ -703,12 +708,8 @@ static int aldebaran_print_clk_levels(struct smu_context *smu, display_levels = clocks.num_levels; - min_clk = smu->gfx_actual_hard_min_freq & CLOCK_VALID ? - smu->gfx_actual_hard_min_freq & ~CLOCK_VALID : - single_dpm_table->dpm_levels[0].value; - max_clk = smu->gfx_actual_soft_max_freq & CLOCK_VALID ? - smu->gfx_actual_soft_max_freq & ~CLOCK_VALID : - single_dpm_table->dpm_levels[1].value; + min_clk = pstate_table->gfxclk_pstate.curr.min; + max_clk = pstate_table->gfxclk_pstate.curr.max; freq_values[0] = min_clk; freq_values[1] = max_clk; @@ -1134,9 +1135,6 @@ static int aldebaran_set_performance_level(struct smu_context *smu, && (level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM)) smu_cmn_send_smc_msg(smu, SMU_MSG_DisableDeterminism, NULL); - /* Reset user min/max gfx clock */ - smu->gfx_actual_hard_min_freq = 0; - smu->gfx_actual_soft_max_freq = 0; switch (level) { @@ -1163,6 +1161,7 @@ static int aldebaran_set_soft_freq_limited_range(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; struct amdgpu_device *adev = smu->adev; uint32_t min_clk; uint32_t max_clk; @@ -1176,14 +1175,20 @@ static int aldebaran_set_soft_freq_limited_range(struct smu_context *smu, return -EINVAL; if (smu_dpm->dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL) { - min_clk = max(min, dpm_context->dpm_tables.gfx_table.min); - max_clk = min(max, dpm_context->dpm_tables.gfx_table.max); - ret = smu_v13_0_set_soft_freq_limited_range(smu, SMU_GFXCLK, - min_clk, max_clk); + if (min >= max) { + dev_err(smu->adev->dev, + "Minimum GFX clk should be less than the maximum allowed clock\n"); + return -EINVAL; + } + if ((min == pstate_table->gfxclk_pstate.curr.min) && + (max == pstate_table->gfxclk_pstate.curr.max)) + return 0; + ret = smu_v13_0_set_soft_freq_limited_range(smu, SMU_GFXCLK, + min, max); if (!ret) { - smu->gfx_actual_hard_min_freq = min_clk | CLOCK_VALID; - smu->gfx_actual_soft_max_freq = max_clk | CLOCK_VALID; + pstate_table->gfxclk_pstate.curr.min = min; + pstate_table->gfxclk_pstate.curr.max = max; } return ret; } @@ -1209,10 +1214,8 @@ static int aldebaran_set_soft_freq_limited_range(struct smu_context *smu, dev_err(adev->dev, "Failed to enable determinism at GFX clock %d MHz\n", max); } else { - smu->gfx_actual_hard_min_freq = - min_clk | CLOCK_VALID; - smu->gfx_actual_soft_max_freq = - max | CLOCK_VALID; + pstate_table->gfxclk_pstate.curr.min = min_clk; + pstate_table->gfxclk_pstate.curr.max = max; } } } @@ -1225,6 +1228,7 @@ static int aldebaran_usr_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_ { 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; uint32_t min_clk; uint32_t max_clk; int ret = 0; @@ -1245,16 +1249,20 @@ static int aldebaran_usr_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_ if (input[1] < dpm_context->dpm_tables.gfx_table.min) { dev_warn(smu->adev->dev, "Minimum GFX clk (%ld) MHz specified is less than the minimum allowed (%d) MHz\n", input[1], dpm_context->dpm_tables.gfx_table.min); + pstate_table->gfxclk_pstate.custom.min = + pstate_table->gfxclk_pstate.curr.min; return -EINVAL; } - smu->gfx_actual_hard_min_freq = input[1]; + pstate_table->gfxclk_pstate.custom.min = input[1]; } else if (input[0] == 1) { if (input[1] > dpm_context->dpm_tables.gfx_table.max) { dev_warn(smu->adev->dev, "Maximum GFX clk (%ld) MHz specified is greater than the maximum allowed (%d) MHz\n", input[1], dpm_context->dpm_tables.gfx_table.max); + pstate_table->gfxclk_pstate.custom.max = + pstate_table->gfxclk_pstate.curr.max; return -EINVAL; } - smu->gfx_actual_soft_max_freq = input[1]; + pstate_table->gfxclk_pstate.custom.max = input[1]; } else { return -EINVAL; } @@ -1276,8 +1284,15 @@ static int aldebaran_usr_edit_dpm_table(struct smu_context *smu, enum PP_OD_DPM_ dev_err(smu->adev->dev, "Input parameter number not correct\n"); return -EINVAL; } else { - min_clk = smu->gfx_actual_hard_min_freq; - max_clk = smu->gfx_actual_soft_max_freq; + if (!pstate_table->gfxclk_pstate.custom.min) + pstate_table->gfxclk_pstate.custom.min = + pstate_table->gfxclk_pstate.curr.min; + if (!pstate_table->gfxclk_pstate.custom.max) + pstate_table->gfxclk_pstate.custom.max = + pstate_table->gfxclk_pstate.curr.max; + min_clk = pstate_table->gfxclk_pstate.custom.min; + max_clk = pstate_table->gfxclk_pstate.custom.max; + return aldebaran_set_soft_freq_limited_range(smu, SMU_GFXCLK, min_clk, max_clk); } break; 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 index 0864083e7435..6b3374432e1d 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c @@ -1626,6 +1626,8 @@ int smu_v13_0_set_performance_level(struct smu_context *smu, sclk_max); if (ret) return ret; + pstate_table->gfxclk_pstate.curr.min = sclk_min; + pstate_table->gfxclk_pstate.curr.max = sclk_max; } if (mclk_min && mclk_max) { @@ -1635,6 +1637,8 @@ int smu_v13_0_set_performance_level(struct smu_context *smu, mclk_max); if (ret) return ret; + pstate_table->uclk_pstate.curr.min = mclk_min; + pstate_table->uclk_pstate.curr.max = mclk_max; } if (socclk_min && socclk_max) { @@ -1644,6 +1648,8 @@ int smu_v13_0_set_performance_level(struct smu_context *smu, socclk_max); if (ret) return ret; + pstate_table->socclk_pstate.curr.min = socclk_min; + pstate_table->socclk_pstate.curr.max = socclk_max; } return ret; -- 2.17.1 </div> </div> </div> </div> </div> </div> </body> </html>