[PATCH 2/3] drm/amd/pm: Fix showing incorrect frequencies on aldebaran
Chen, Guchun
Guchun.Chen at amd.com
Thu May 13 13:34:59 UTC 2021
[AMD Public Use]
3 nit-picks inline.
Regards,
Guchun
-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Lijo Lazar
Sent: Thursday, May 13, 2021 5:48 PM
To: amd-gfx at lists.freedesktop.org
Cc: Wang, Kevin(Yang) <Kevin1.Wang at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>
Subject: [PATCH 2/3] drm/amd/pm: Fix showing incorrect frequencies on aldebaran
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.
Signed-off-by: Lijo Lazar <lijo.lazar at amd.com>
---
.../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 65 ++++++++++++-------
.../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 18 ++++-
2 files changed, 55 insertions(+), 28 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..755bddaf6c4b 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
@@ -1624,8 +1624,12 @@ int smu_v13_0_set_performance_level(struct
smu_context *smu,
SMU_GFXCLK,
sclk_min,
sclk_max);
- if (ret)
+ if (ret) {
return ret;
+ } else {
+ pstate_table->gfxclk_pstate.curr.min = sclk_min;
+ pstate_table->gfxclk_pstate.curr.max = sclk_max;
+ }
[Guchun] We may simply code like below. Drop "else" part, as it's redundant.
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) {
@@ -1633,8 +1637,12 @@ int smu_v13_0_set_performance_level(struct
smu_context *smu,
SMU_MCLK,
mclk_min,
mclk_max);
- if (ret)
+ if (ret) {
return ret;
+ } else {
+ pstate_table->uclk_pstate.curr.min = mclk_min;
+ pstate_table->uclk_pstate.curr.max = mclk_max;
+ }
[Guchun]Same as above.
}
if (socclk_min && socclk_max) {
@@ -1642,8 +1650,12 @@ int smu_v13_0_set_performance_level(struct
smu_context *smu,
SMU_SOCCLK,
socclk_min,
socclk_max);
- if (ret)
+ if (ret) {
return ret;
+ } else {
+ pstate_table->socclk_pstate.curr.min = socclk_min;
+ pstate_table->socclk_pstate.curr.max = socclk_max;
+ }
[Guchun]Same as above.
}
return ret;
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cguchun.chen%40amd.com%7C2a60ef3fa0994b00fb2c08d915f42cce%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637564960746162621%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Rlv%2F%2B%2F7t9fc7vxXhs9vqipWMKhXbKYLj8hHhSh48lo0%3D&reserved=0
More information about the amd-gfx
mailing list