On 5/9/2022 9:28 AM, Darren Powell wrote:
added a check to populate and use SCLK shim table freq_values only
if using dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL or
AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM
removed clocks.num_levels from calculation of shim table size
removed unsafe accesses to shim table freq_values
output gfx_table values if using other dpm levels
added check for freq_match when using freq_values for when now == min_clk
== Test ==
LOGFILE=aldebaran-sclk.test.log
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | awk '{print $9}'`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" > $LOGFILE
FILES="pp_od_clk_voltage
pp_dpm_sclk"
for f in $FILES
do
echo === $f === >> $LOGFILE
cat $HWMON_DIR/device/$f >> $LOGFILE
done
cat $LOGFILE
Signed-off-by: Darren Powell <darren.powell@xxxxxxx>
---
.../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 60 +++++++++----------
1 file changed, 29 insertions(+), 31 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 6a4fca47ae53..a653668e8402 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -740,9 +740,8 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
struct smu_13_0_dpm_table *single_dpm_table;
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
struct smu_13_0_dpm_context *dpm_context = NULL;
- uint32_t display_levels;
uint32_t freq_values[3] = {0};
- uint32_t min_clk, max_clk;
+ uint32_t min_clk, max_clk, display_levels = 0;
smu_cmn_get_sysfs_buf(&buf, &size);
@@ -765,46 +764,45 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
return ret;
}
- single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
- ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
- return ret;
- }
-
- display_levels = clocks.num_levels;
-
- 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;
-
- /* fine-grained dpm has only 2 levels */
- if (now > min_clk && now < max_clk) {
- display_levels = clocks.num_levels + 1;
- freq_values[2] = max_clk;
- freq_values[1] = now;
- }
+ if ((smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL &&
+ smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM)) {
+ single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
+ ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
+ if (ret) {
+ dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
+ return ret;
+ }
There are only two levels for GFX clock in aldebaran - min and max.
Regardless of the mode, gfxclk_pstate.curr.min/max should reflect the
current min/max level.
Could you explain the issue you are seeing? It's not so clear from the
commit message.
Thanks,
Lijo
- /*
- * For DPM disabled case, there will be only one clock level.
- * And it's safe to assume that is always the current clock.
- */
- if (display_levels == clocks.num_levels) {
for (i = 0; i < clocks.num_levels; i++)
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
- freq_values[i],
+ clocks.data[i].clocks_in_khz / 1000,
(clocks.num_levels == 1) ?
"*" :
(aldebaran_freqs_in_same_level(
- freq_values[i], now) ?
+ clocks.data[i].clocks_in_khz / 1000, now) ?
"*" :
""));
} else {
+ /* fine-grained dpm has only 2 levels */
+ display_levels = 2;
+
+ 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;
+
+ if (now > min_clk && now < max_clk) {
+ display_levels++;
+ freq_values[2] = max_clk;
+ freq_values[1] = now;
+ }
+
for (i = 0; i < display_levels; i++)
size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
- freq_values[i], i == 1 ? "*" : "");
+ freq_values[i],
+ aldebaran_freqs_in_same_level(freq_values[i], now) ?
+ "*" : "");
}
break;
