* [PATCH v1 1/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for arcturus
@ 2022-05-13 3:15 Darren Powell
2022-05-13 3:15 ` [PATCH v1 2/4] amdgpu/pm: Optimize arcturus_emit_clk_levels Darren Powell
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Darren Powell @ 2022-05-13 3:15 UTC (permalink / raw)
To: amd-gfx
Cc: kevin1.wang, kenneth.feng, lijo.lazar, Darren Powell, evan.quan,
lang.yu, david.nieto
Replace print_clock_levels with emit_clock_levels for arcturus
* replace .print_clk_levels with .emit_clk_levels in arcturus_ppt_funcs
* added extra parameter int *offset
* removed var size, uses arg *offset instead
* removed call to smu_cmn_get_sysfs_buf
* errors are returned to caller
* returns 0 on success
additional incidental changes
* changed type of var i, now to remove comparing mismatch types
* renamed var s/now/cur_value/
* switch statement default now returns -EINVAL
* RAS Recovery returns -EBUSY
Based on
commit aa93bbdd1492 ("amdgpu/pm: Implement emit_clk_levels for navi10")
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 54 +++++++++----------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 201563072189..163adab2843d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -756,29 +756,28 @@ static int arcturus_get_current_clk_freq_by_table(struct smu_context *smu,
value);
}
-static int arcturus_print_clk_levels(struct smu_context *smu,
- enum smu_clk_type type, char *buf)
+static int arcturus_emit_clk_levels(struct smu_context *smu,
+ enum smu_clk_type type, char *buf, int *offset)
{
- int i, now, size = 0;
int ret = 0;
struct pp_clock_levels_with_latency clocks;
struct smu_11_0_dpm_table *single_dpm_table;
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
struct smu_11_0_dpm_context *dpm_context = NULL;
uint32_t gen_speed, lane_width;
+ uint32_t i, cur_value = 0;
- smu_cmn_get_sysfs_buf(&buf, &size);
if (amdgpu_ras_intr_triggered()) {
- size += sysfs_emit_at(buf, size, "unavailable\n");
- return size;
+ *offset += sysfs_emit_at(buf, *offset, "unavailable\n");
+ return -EBUSY;
}
dpm_context = smu_dpm->dpm_context;
switch (type) {
case SMU_SCLK:
- ret = arcturus_get_current_clk_freq_by_table(smu, SMU_GFXCLK, &now);
+ ret = arcturus_get_current_clk_freq_by_table(smu, SMU_GFXCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current gfx clk Failed!");
return ret;
@@ -796,16 +795,16 @@ static int arcturus_print_clk_levels(struct smu_context *smu,
* And it's safe to assume that is always the current clock.
*/
for (i = 0; i < clocks.num_levels; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
clocks.data[i].clocks_in_khz / 1000,
(clocks.num_levels == 1) ? "*" :
(arcturus_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_MCLK:
- ret = arcturus_get_current_clk_freq_by_table(smu, SMU_UCLK, &now);
+ ret = arcturus_get_current_clk_freq_by_table(smu, SMU_UCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current mclk Failed!");
return ret;
@@ -819,16 +818,16 @@ static int arcturus_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < clocks.num_levels; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, clocks.data[i].clocks_in_khz / 1000,
(clocks.num_levels == 1) ? "*" :
(arcturus_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_SOCCLK:
- ret = arcturus_get_current_clk_freq_by_table(smu, SMU_SOCCLK, &now);
+ ret = arcturus_get_current_clk_freq_by_table(smu, SMU_SOCCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current socclk Failed!");
return ret;
@@ -842,16 +841,16 @@ static int arcturus_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < clocks.num_levels; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, clocks.data[i].clocks_in_khz / 1000,
(clocks.num_levels == 1) ? "*" :
(arcturus_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_FCLK:
- ret = arcturus_get_current_clk_freq_by_table(smu, SMU_FCLK, &now);
+ ret = arcturus_get_current_clk_freq_by_table(smu, SMU_FCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current fclk Failed!");
return ret;
@@ -865,16 +864,16 @@ static int arcturus_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < single_dpm_table->count; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, single_dpm_table->dpm_levels[i].value,
(clocks.num_levels == 1) ? "*" :
(arcturus_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_VCLK:
- ret = arcturus_get_current_clk_freq_by_table(smu, SMU_VCLK, &now);
+ ret = arcturus_get_current_clk_freq_by_table(smu, SMU_VCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current vclk Failed!");
return ret;
@@ -888,16 +887,16 @@ static int arcturus_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < single_dpm_table->count; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, single_dpm_table->dpm_levels[i].value,
(clocks.num_levels == 1) ? "*" :
(arcturus_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_DCLK:
- ret = arcturus_get_current_clk_freq_by_table(smu, SMU_DCLK, &now);
+ ret = arcturus_get_current_clk_freq_by_table(smu, SMU_DCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current dclk Failed!");
return ret;
@@ -911,18 +910,18 @@ static int arcturus_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < single_dpm_table->count; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, single_dpm_table->dpm_levels[i].value,
(clocks.num_levels == 1) ? "*" :
(arcturus_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_PCIE:
gen_speed = smu_v11_0_get_current_pcie_link_speed_level(smu);
lane_width = smu_v11_0_get_current_pcie_link_width_level(smu);
- size += sysfs_emit_at(buf, size, "0: %s %s %dMhz *\n",
+ *offset += sysfs_emit_at(buf, *offset, "0: %s %s %dMhz *\n",
(gen_speed == 0) ? "2.5GT/s," :
(gen_speed == 1) ? "5.0GT/s," :
(gen_speed == 2) ? "8.0GT/s," :
@@ -937,10 +936,11 @@ static int arcturus_print_clk_levels(struct smu_context *smu,
break;
default:
+ return -EINVAL;
break;
}
- return size;
+ return 0;
}
static int arcturus_upload_dpm_level(struct smu_context *smu,
@@ -2423,7 +2423,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
.set_default_dpm_table = arcturus_set_default_dpm_table,
.populate_umd_state_clk = arcturus_populate_umd_state_clk,
.get_thermal_temperature_range = arcturus_get_thermal_temperature_range,
- .print_clk_levels = arcturus_print_clk_levels,
+ .emit_clk_levels = arcturus_emit_clk_levels,
.force_clk_levels = arcturus_force_clk_levels,
.read_sensor = arcturus_read_sensor,
.get_fan_speed_pwm = arcturus_get_fan_speed_pwm,
base-commit: 8bb14fbec5ae45c31cbefe217db2418cc683612f
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 2/4] amdgpu/pm: Optimize arcturus_emit_clk_levels
2022-05-13 3:15 [PATCH v1 1/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for arcturus Darren Powell
@ 2022-05-13 3:15 ` Darren Powell
2022-05-13 3:15 ` [PATCH v1 3/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for aldebaran Darren Powell
2022-05-13 3:15 ` [PATCH v1 4/4] amdgpu/pm: Optimize aldebaran_emit_clk_levels Darren Powell
2 siblings, 0 replies; 5+ messages in thread
From: Darren Powell @ 2022-05-13 3:15 UTC (permalink / raw)
To: amd-gfx
Cc: kevin1.wang, kenneth.feng, lijo.lazar, Darren Powell, evan.quan,
lang.yu, david.nieto
Optimized arcturus_emit_clk_levels
split into two switch statements to gather vars, then use common output
reduce size of string literals by creating static string var
removed impossible error messages for failure of get_clk_table
added clock_mhz to remove double divide by 1000
changed unsafe loop iterator from single_dpm_table->count to clocks.num_levels
in case MAX_DPM_LEVELS > PP_MAX_CLOCK_LEVELS in future code
simplified code to output detect frequency level match to current level
combined common output code into single switch case
arcturus_get_clk_table cannot fail so convert to void function
arcturus_freqs_in_same_level now returns bool
== Test ==
LOGFILE=arcturus.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_dpm_sclk
pp_dpm_mclk
pp_dpm_pcie
pp_dpm_socclk
pp_dpm_fclk
pp_dpm_vclk
pp_dpm_dclk "
for f in $FILES
do
echo === $f === >> $LOGFILE
cat $HWMON_DIR/device/$f >> $LOGFILE
done
cat $LOGFILE
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 144 ++++++------------
1 file changed, 49 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 163adab2843d..df8d870125da 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -569,9 +569,9 @@ static int arcturus_populate_umd_state_clk(struct smu_context *smu)
return 0;
}
-static int arcturus_get_clk_table(struct smu_context *smu,
- struct pp_clock_levels_with_latency *clocks,
- struct smu_11_0_dpm_table *dpm_table)
+static void arcturus_get_clk_table(struct smu_context *smu,
+ struct pp_clock_levels_with_latency *clocks,
+ struct smu_11_0_dpm_table *dpm_table)
{
int i, count;
@@ -584,11 +584,11 @@ static int arcturus_get_clk_table(struct smu_context *smu,
clocks->data[i].latency_in_us = 0;
}
- return 0;
+ return;
}
-static int arcturus_freqs_in_same_level(int32_t frequency1,
- int32_t frequency2)
+static bool arcturus_freqs_in_same_level(int32_t frequency1,
+ int32_t frequency2)
{
return (abs(frequency1 - frequency2) <= EPSILON);
}
@@ -766,6 +766,9 @@ static int arcturus_emit_clk_levels(struct smu_context *smu,
struct smu_11_0_dpm_context *dpm_context = NULL;
uint32_t gen_speed, lane_width;
uint32_t i, cur_value = 0;
+ bool freq_match;
+ unsigned int clock_mhz;
+ static const char attempt_string[] = "Attempt to get current";
if (amdgpu_ras_intr_triggered()) {
@@ -779,148 +782,100 @@ static int arcturus_emit_clk_levels(struct smu_context *smu,
case SMU_SCLK:
ret = arcturus_get_current_clk_freq_by_table(smu, SMU_GFXCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current gfx clk Failed!");
+ dev_err(smu->adev->dev, "%s gfx clk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
- ret = arcturus_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
- return ret;
- }
-
- /*
- * For DPM disabled case, there will be only one clock level.
- * And it's safe to assume that is always the current clock.
- */
- for (i = 0; i < clocks.num_levels; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
- clocks.data[i].clocks_in_khz / 1000,
- (clocks.num_levels == 1) ? "*" :
- (arcturus_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ arcturus_get_clk_table(smu, &clocks, single_dpm_table);
break;
case SMU_MCLK:
ret = arcturus_get_current_clk_freq_by_table(smu, SMU_UCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current mclk Failed!");
+ dev_err(smu->adev->dev, "%s mclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
- ret = arcturus_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get memory clk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < clocks.num_levels; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, clocks.data[i].clocks_in_khz / 1000,
- (clocks.num_levels == 1) ? "*" :
- (arcturus_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ arcturus_get_clk_table(smu, &clocks, single_dpm_table);
break;
case SMU_SOCCLK:
ret = arcturus_get_current_clk_freq_by_table(smu, SMU_SOCCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current socclk Failed!");
+ dev_err(smu->adev->dev, "%s socclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.soc_table);
- ret = arcturus_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get socclk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < clocks.num_levels; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, clocks.data[i].clocks_in_khz / 1000,
- (clocks.num_levels == 1) ? "*" :
- (arcturus_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ arcturus_get_clk_table(smu, &clocks, single_dpm_table);
break;
case SMU_FCLK:
ret = arcturus_get_current_clk_freq_by_table(smu, SMU_FCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current fclk Failed!");
+ dev_err(smu->adev->dev, "%s fclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
- ret = arcturus_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get fclk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < single_dpm_table->count; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, single_dpm_table->dpm_levels[i].value,
- (clocks.num_levels == 1) ? "*" :
- (arcturus_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ arcturus_get_clk_table(smu, &clocks, single_dpm_table);
break;
case SMU_VCLK:
ret = arcturus_get_current_clk_freq_by_table(smu, SMU_VCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current vclk Failed!");
+ dev_err(smu->adev->dev, "%s vclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
- ret = arcturus_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get vclk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < single_dpm_table->count; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, single_dpm_table->dpm_levels[i].value,
- (clocks.num_levels == 1) ? "*" :
- (arcturus_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ arcturus_get_clk_table(smu, &clocks, single_dpm_table);
break;
case SMU_DCLK:
ret = arcturus_get_current_clk_freq_by_table(smu, SMU_DCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current dclk Failed!");
+ dev_err(smu->adev->dev, "%s dclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
- ret = arcturus_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get dclk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < single_dpm_table->count; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, single_dpm_table->dpm_levels[i].value,
- (clocks.num_levels == 1) ? "*" :
- (arcturus_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ arcturus_get_clk_table(smu, &clocks, single_dpm_table);
break;
case SMU_PCIE:
gen_speed = smu_v11_0_get_current_pcie_link_speed_level(smu);
lane_width = smu_v11_0_get_current_pcie_link_width_level(smu);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ switch (type) {
+ case SMU_SCLK:
+ case SMU_MCLK:
+ case SMU_SOCCLK:
+ case SMU_FCLK:
+ case SMU_VCLK:
+ case SMU_DCLK:
+ /*
+ * For DPM disabled case, there will be only one clock level.
+ * And it's safe to assume that is always the current clock.
+ */
+ for (i = 0; i < clocks.num_levels; i++) {
+ clock_mhz = clocks.data[i].clocks_in_khz / 1000;
+ freq_match = arcturus_freqs_in_same_level(clock_mhz, cur_value);
+ freq_match |= (clocks.num_levels == 1);
+
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
+ clock_mhz,
+ freq_match ? "*" : "");
+ }
+ break;
+ case SMU_PCIE:
*offset += sysfs_emit_at(buf, *offset, "0: %s %s %dMhz *\n",
(gen_speed == 0) ? "2.5GT/s," :
(gen_speed == 1) ? "5.0GT/s," :
@@ -937,7 +892,6 @@ static int arcturus_emit_clk_levels(struct smu_context *smu,
default:
return -EINVAL;
- break;
}
return 0;
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 3/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for aldebaran
2022-05-13 3:15 [PATCH v1 1/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for arcturus Darren Powell
2022-05-13 3:15 ` [PATCH v1 2/4] amdgpu/pm: Optimize arcturus_emit_clk_levels Darren Powell
@ 2022-05-13 3:15 ` Darren Powell
2022-05-13 3:15 ` [PATCH v1 4/4] amdgpu/pm: Optimize aldebaran_emit_clk_levels Darren Powell
2 siblings, 0 replies; 5+ messages in thread
From: Darren Powell @ 2022-05-13 3:15 UTC (permalink / raw)
To: amd-gfx
Cc: kevin1.wang, kenneth.feng, lijo.lazar, Darren Powell, evan.quan,
lang.yu, david.nieto
Replace print_clock_levels with emit_clock_levels for aldebaran
* replace .print_clk_levels with .emit_clk_levels in aldebaran_ppt_funcs
* added extra parameter int *offset
* removed var size, uses arg *offset instead
* removed call to smu_cmn_get_sysfs_buf
* errors are returned to caller
* returns 0 on success
additional incidental changes
* changed type of vars i, now to remove comparing mismatch types
* renamed var s/now/cur_value/
* switch statement default now returns -EINVAL
* RAS Recovery returns -EBUSY
Based on
commit aa93bbdd1492 ("amdgpu/pm: Implement emit_clk_levels for navi10")
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 66 +++++++++----------
1 file changed, 32 insertions(+), 34 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 38af648cb857..e593878bc173 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -729,25 +729,22 @@ static int aldebaran_get_current_clk_freq_by_table(struct smu_context *smu,
value);
}
-static int aldebaran_print_clk_levels(struct smu_context *smu,
- enum smu_clk_type type, char *buf)
+static int aldebaran_emit_clk_levels(struct smu_context *smu,
+ enum smu_clk_type type, char *buf, int *offset)
{
- 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;
struct smu_13_0_dpm_context *dpm_context = NULL;
- uint32_t display_levels;
+ uint32_t i, display_levels;
uint32_t freq_values[3] = {0};
- uint32_t min_clk, max_clk;
-
- smu_cmn_get_sysfs_buf(&buf, &size);
+ uint32_t min_clk, max_clk, cur_value = 0;
if (amdgpu_ras_intr_triggered()) {
- size += sysfs_emit_at(buf, size, "unavailable\n");
- return size;
+ *offset += sysfs_emit_at(buf, *offset, "unavailable\n");
+ return -EBUSY;
}
dpm_context = smu_dpm->dpm_context;
@@ -755,10 +752,10 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
switch (type) {
case SMU_OD_SCLK:
- size += sysfs_emit_at(buf, size, "%s:\n", "GFXCLK");
+ *offset += sysfs_emit_at(buf, *offset, "%s:\n", "GFXCLK");
fallthrough;
case SMU_SCLK:
- ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_GFXCLK, &now);
+ ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_GFXCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current gfx clk Failed!");
return ret;
@@ -780,10 +777,10 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
freq_values[1] = max_clk;
/* fine-grained dpm has only 2 levels */
- if (now > min_clk && now < max_clk) {
+ if (cur_value > min_clk && cur_value < max_clk) {
display_levels = clocks.num_levels + 1;
freq_values[2] = max_clk;
- freq_values[1] = now;
+ freq_values[1] = cur_value;
}
/*
@@ -792,27 +789,27 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
*/
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,
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
freq_values[i],
(clocks.num_levels == 1) ?
"*" :
(aldebaran_freqs_in_same_level(
- freq_values[i], now) ?
+ freq_values[i], cur_value) ?
"*" :
""));
} else {
for (i = 0; i < display_levels; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
freq_values[i], i == 1 ? "*" : "");
}
break;
case SMU_OD_MCLK:
- size += sysfs_emit_at(buf, size, "%s:\n", "MCLK");
+ *offset += sysfs_emit_at(buf, *offset, "%s:\n", "MCLK");
fallthrough;
case SMU_MCLK:
- ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_UCLK, &now);
+ ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_UCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current mclk Failed!");
return ret;
@@ -826,16 +823,16 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < clocks.num_levels; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, clocks.data[i].clocks_in_khz / 1000,
(clocks.num_levels == 1) ? "*" :
(aldebaran_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_SOCCLK:
- ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_SOCCLK, &now);
+ ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_SOCCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current socclk Failed!");
return ret;
@@ -849,16 +846,16 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < clocks.num_levels; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, clocks.data[i].clocks_in_khz / 1000,
(clocks.num_levels == 1) ? "*" :
(aldebaran_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_FCLK:
- ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_FCLK, &now);
+ ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_FCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current fclk Failed!");
return ret;
@@ -872,16 +869,16 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < single_dpm_table->count; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, single_dpm_table->dpm_levels[i].value,
(clocks.num_levels == 1) ? "*" :
(aldebaran_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_VCLK:
- ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_VCLK, &now);
+ ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_VCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current vclk Failed!");
return ret;
@@ -895,16 +892,16 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < single_dpm_table->count; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, single_dpm_table->dpm_levels[i].value,
(clocks.num_levels == 1) ? "*" :
(aldebaran_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
case SMU_DCLK:
- ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_DCLK, &now);
+ ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_DCLK, &cur_value);
if (ret) {
dev_err(smu->adev->dev, "Attempt to get current dclk Failed!");
return ret;
@@ -918,19 +915,20 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
}
for (i = 0; i < single_dpm_table->count; i++)
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n",
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
i, single_dpm_table->dpm_levels[i].value,
(clocks.num_levels == 1) ? "*" :
(aldebaran_freqs_in_same_level(
clocks.data[i].clocks_in_khz / 1000,
- now) ? "*" : ""));
+ cur_value) ? "*" : ""));
break;
default:
+ return -EINVAL;
break;
}
- return size;
+ return 0;
}
static int aldebaran_upload_dpm_level(struct smu_context *smu,
@@ -2048,7 +2046,7 @@ static const struct pptable_funcs aldebaran_ppt_funcs = {
.set_default_dpm_table = aldebaran_set_default_dpm_table,
.populate_umd_state_clk = aldebaran_populate_umd_state_clk,
.get_thermal_temperature_range = aldebaran_get_thermal_temperature_range,
- .print_clk_levels = aldebaran_print_clk_levels,
+ .emit_clk_levels = aldebaran_emit_clk_levels,
.force_clk_levels = aldebaran_force_clk_levels,
.read_sensor = aldebaran_read_sensor,
.set_performance_level = aldebaran_set_performance_level,
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 4/4] amdgpu/pm: Optimize aldebaran_emit_clk_levels
2022-05-13 3:15 [PATCH v1 1/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for arcturus Darren Powell
2022-05-13 3:15 ` [PATCH v1 2/4] amdgpu/pm: Optimize arcturus_emit_clk_levels Darren Powell
2022-05-13 3:15 ` [PATCH v1 3/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for aldebaran Darren Powell
@ 2022-05-13 3:15 ` Darren Powell
2022-05-13 7:48 ` Quan, Evan
2 siblings, 1 reply; 5+ messages in thread
From: Darren Powell @ 2022-05-13 3:15 UTC (permalink / raw)
To: amd-gfx
Cc: kevin1.wang, kenneth.feng, lijo.lazar, Darren Powell, evan.quan,
lang.yu, david.nieto
aldebaran_get_clk_table cannot fail so convert to void function
aldebaran_freqs_in_same_level now returns bool
aldebaran_emit_clk_levels optimized:
split into two switch statements to gather vars, then use common output
removed impossible error messages for failure of get_clk_table
reduce size of string literals by creating static string var
changed unsafe loop iterator from single_dpm_table->count to clocks.num_levels
in case MAX_DPM_LEVELS > PP_MAX_CLOCK_LEVELS in future code
added clock_mhz to remove double divide by 1000
collapse duplicate case statements in second switch statement
simplified code to output detect frequency level match to current level
== Test ==
LOGFILE=aldebaran.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_dpm_sclk
pp_dpm_mclk
pp_dpm_pcie
pp_dpm_socclk
pp_dpm_fclk
pp_dpm_vclk
pp_dpm_dclk "
for f in $FILES
do
echo === $f === >> $LOGFILE
cat $HWMON_DIR/device/$f >> $LOGFILE
done
cat $LOGFILE
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 173 +++++++-----------
1 file changed, 62 insertions(+), 111 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 e593878bc173..23a87bfb4429 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -545,9 +545,9 @@ static int aldebaran_populate_umd_state_clk(struct smu_context *smu)
return 0;
}
-static int aldebaran_get_clk_table(struct smu_context *smu,
- struct pp_clock_levels_with_latency *clocks,
- struct smu_13_0_dpm_table *dpm_table)
+static void aldebaran_get_clk_table(struct smu_context *smu,
+ struct pp_clock_levels_with_latency *clocks,
+ struct smu_13_0_dpm_table *dpm_table)
{
int i, count;
@@ -560,11 +560,11 @@ static int aldebaran_get_clk_table(struct smu_context *smu,
clocks->data[i].latency_in_us = 0;
}
- return 0;
+ return;
}
-static int aldebaran_freqs_in_same_level(int32_t frequency1,
- int32_t frequency2)
+static bool aldebaran_freqs_in_same_level(int32_t frequency1,
+ int32_t frequency2)
{
return (abs(frequency1 - frequency2) <= EPSILON);
}
@@ -738,9 +738,12 @@ static int aldebaran_emit_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 i, display_levels;
+ uint32_t i, cur_value = 0;
uint32_t freq_values[3] = {0};
- uint32_t min_clk, max_clk, cur_value = 0;
+ uint32_t min_clk, max_clk, display_levels;
+ bool freq_match;
+ unsigned int clock_mhz;
+ static const char attempt_string[] = "Attempt to get current";
if (amdgpu_ras_intr_triggered()) {
*offset += sysfs_emit_at(buf, *offset, "unavailable\n");
@@ -750,23 +753,18 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
dpm_context = smu_dpm->dpm_context;
switch (type) {
-
case SMU_OD_SCLK:
*offset += sysfs_emit_at(buf, *offset, "%s:\n", "GFXCLK");
fallthrough;
case SMU_SCLK:
ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_GFXCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current gfx clk Failed!");
+ dev_err(smu->adev->dev, "%s gfx clk Failed!", attempt_string);
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;
- }
+ aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
display_levels = clocks.num_levels;
@@ -782,152 +780,105 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
freq_values[2] = max_clk;
freq_values[1] = cur_value;
}
-
- /*
- * 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++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
- freq_values[i],
- (clocks.num_levels == 1) ?
- "*" :
- (aldebaran_freqs_in_same_level(
- freq_values[i], cur_value) ?
- "*" :
- ""));
- } else {
- for (i = 0; i < display_levels; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
- freq_values[i], i == 1 ? "*" : "");
- }
-
break;
-
case SMU_OD_MCLK:
*offset += sysfs_emit_at(buf, *offset, "%s:\n", "MCLK");
fallthrough;
case SMU_MCLK:
ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_UCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current mclk Failed!");
+ dev_err(smu->adev->dev, "%s mclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
- ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get memory clk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < clocks.num_levels; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, clocks.data[i].clocks_in_khz / 1000,
- (clocks.num_levels == 1) ? "*" :
- (aldebaran_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
break;
-
case SMU_SOCCLK:
ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_SOCCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current socclk Failed!");
+ dev_err(smu->adev->dev, "%s socclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.soc_table);
- ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get socclk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < clocks.num_levels; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, clocks.data[i].clocks_in_khz / 1000,
- (clocks.num_levels == 1) ? "*" :
- (aldebaran_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
break;
-
case SMU_FCLK:
ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_FCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current fclk Failed!");
+ dev_err(smu->adev->dev, "%s fclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
- ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get fclk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < single_dpm_table->count; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, single_dpm_table->dpm_levels[i].value,
- (clocks.num_levels == 1) ? "*" :
- (aldebaran_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
break;
-
case SMU_VCLK:
ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_VCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current vclk Failed!");
+ dev_err(smu->adev->dev, "%s vclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
- ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get vclk levels Failed!");
- return ret;
- }
-
- for (i = 0; i < single_dpm_table->count; i++)
- *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, single_dpm_table->dpm_levels[i].value,
- (clocks.num_levels == 1) ? "*" :
- (aldebaran_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
break;
-
case SMU_DCLK:
ret = aldebaran_get_current_clk_freq_by_table(smu, SMU_DCLK, &cur_value);
if (ret) {
- dev_err(smu->adev->dev, "Attempt to get current dclk Failed!");
+ dev_err(smu->adev->dev, "%s dclk Failed!", attempt_string);
return ret;
}
single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
- ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
- if (ret) {
- dev_err(smu->adev->dev, "Attempt to get dclk levels Failed!");
- return ret;
+ aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (type) {
+ case SMU_OD_SCLK:
+ case SMU_SCLK:
+ /*
+ * 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++) {
+ clock_mhz = freq_values[i];
+ freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
+ freq_match |= (clocks.num_levels == 1);
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
+ clock_mhz,
+ freq_match ? "*" : "");
+ }
+ } else {
+ for (i = 0; i < display_levels; i++)
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
+ freq_values[i], i == 1 ? "*" : "");
}
- for (i = 0; i < single_dpm_table->count; i++)
+ break;
+ case SMU_OD_MCLK:
+ case SMU_MCLK:
+ case SMU_SOCCLK:
+ case SMU_FCLK:
+ case SMU_VCLK:
+ case SMU_DCLK:
+ for (i = 0; i < clocks.num_levels; i++) {
+ clock_mhz = clocks.data[i].clocks_in_khz / 1000;
+ freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
+ freq_match |= (clocks.num_levels == 1);
*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
- i, single_dpm_table->dpm_levels[i].value,
- (clocks.num_levels == 1) ? "*" :
- (aldebaran_freqs_in_same_level(
- clocks.data[i].clocks_in_khz / 1000,
- cur_value) ? "*" : ""));
+ i, clock_mhz,
+ freq_match ? "*" : "");
+ }
break;
-
default:
return -EINVAL;
- break;
}
-
return 0;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH v1 4/4] amdgpu/pm: Optimize aldebaran_emit_clk_levels
2022-05-13 3:15 ` [PATCH v1 4/4] amdgpu/pm: Optimize aldebaran_emit_clk_levels Darren Powell
@ 2022-05-13 7:48 ` Quan, Evan
0 siblings, 0 replies; 5+ messages in thread
From: Quan, Evan @ 2022-05-13 7:48 UTC (permalink / raw)
To: Powell, Darren, amd-gfx
Cc: Yu, Lang, Lazar, Lijo, Feng, Kenneth, Wang, Yang(Kevin), david.nieto
[AMD Official Use Only - General]
Series is acked-by: Evan Quan <evan.quan@amd.com>
> -----Original Message-----
> From: Powell, Darren <Darren.Powell@amd.com>
> Sent: Friday, May 13, 2022 11:15 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan@amd.com>; Wang, Yang(Kevin)
> <KevinYang.Wang@amd.com>; david.nieto@amd.com; Lazar, Lijo
> <Lijo.Lazar@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Yu,
> Lang <Lang.Yu@amd.com>; Powell, Darren <Darren.Powell@amd.com>
> Subject: [PATCH v1 4/4] amdgpu/pm: Optimize aldebaran_emit_clk_levels
>
> aldebaran_get_clk_table cannot fail so convert to void function
> aldebaran_freqs_in_same_level now returns bool
> aldebaran_emit_clk_levels optimized:
> split into two switch statements to gather vars, then use common output
> removed impossible error messages for failure of get_clk_table
> reduce size of string literals by creating static string var
> changed unsafe loop iterator from single_dpm_table->count to
> clocks.num_levels
> in case MAX_DPM_LEVELS > PP_MAX_CLOCK_LEVELS in future code
> added clock_mhz to remove double divide by 1000
> collapse duplicate case statements in second switch statement
> simplified code to output detect frequency level match to current level
>
> == Test ==
> LOGFILE=aldebaran.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_dpm_sclk
> pp_dpm_mclk
> pp_dpm_pcie
> pp_dpm_socclk
> pp_dpm_fclk
> pp_dpm_vclk
> pp_dpm_dclk "
>
> for f in $FILES
> do
> echo === $f === >> $LOGFILE
> cat $HWMON_DIR/device/$f >> $LOGFILE
> done
> cat $LOGFILE
>
> Signed-off-by: Darren Powell <darren.powell@amd.com>
> ---
> .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 173 +++++++-----------
> 1 file changed, 62 insertions(+), 111 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 e593878bc173..23a87bfb4429 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -545,9 +545,9 @@ static int aldebaran_populate_umd_state_clk(struct
> smu_context *smu)
> return 0;
> }
>
> -static int aldebaran_get_clk_table(struct smu_context *smu,
> - struct pp_clock_levels_with_latency *clocks,
> - struct smu_13_0_dpm_table *dpm_table)
> +static void aldebaran_get_clk_table(struct smu_context *smu,
> + struct pp_clock_levels_with_latency
> *clocks,
> + struct smu_13_0_dpm_table *dpm_table)
> {
> int i, count;
>
> @@ -560,11 +560,11 @@ static int aldebaran_get_clk_table(struct
> smu_context *smu,
> clocks->data[i].latency_in_us = 0;
> }
>
> - return 0;
> + return;
> }
>
> -static int aldebaran_freqs_in_same_level(int32_t frequency1,
> - int32_t frequency2)
> +static bool aldebaran_freqs_in_same_level(int32_t frequency1,
> + int32_t frequency2)
> {
> return (abs(frequency1 - frequency2) <= EPSILON);
> }
> @@ -738,9 +738,12 @@ static int aldebaran_emit_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 i, display_levels;
> + uint32_t i, cur_value = 0;
> uint32_t freq_values[3] = {0};
> - uint32_t min_clk, max_clk, cur_value = 0;
> + uint32_t min_clk, max_clk, display_levels;
> + bool freq_match;
> + unsigned int clock_mhz;
> + static const char attempt_string[] = "Attempt to get current";
>
> if (amdgpu_ras_intr_triggered()) {
> *offset += sysfs_emit_at(buf, *offset, "unavailable\n");
> @@ -750,23 +753,18 @@ static int aldebaran_emit_clk_levels(struct
> smu_context *smu,
> dpm_context = smu_dpm->dpm_context;
>
> switch (type) {
> -
> case SMU_OD_SCLK:
> *offset += sysfs_emit_at(buf, *offset, "%s:\n", "GFXCLK");
> fallthrough;
> case SMU_SCLK:
> ret = aldebaran_get_current_clk_freq_by_table(smu,
> SMU_GFXCLK, &cur_value);
> if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get current
> gfx clk Failed!");
> + dev_err(smu->adev->dev, "%s gfx clk Failed!",
> attempt_string);
> 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;
> - }
> + aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
>
> display_levels = clocks.num_levels;
>
> @@ -782,152 +780,105 @@ static int aldebaran_emit_clk_levels(struct
> smu_context *smu,
> freq_values[2] = max_clk;
> freq_values[1] = cur_value;
> }
> -
> - /*
> - * 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++)
> - *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i,
> - freq_values[i],
> - (clocks.num_levels == 1) ?
> - "*" :
> -
> (aldebaran_freqs_in_same_level(
> - freq_values[i],
> cur_value) ?
> - "*" :
> - ""));
> - } else {
> - for (i = 0; i < display_levels; i++)
> - *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i,
> - freq_values[i], i == 1 ? "*" :
> "");
> - }
> -
> break;
> -
> case SMU_OD_MCLK:
> *offset += sysfs_emit_at(buf, *offset, "%s:\n", "MCLK");
> fallthrough;
> case SMU_MCLK:
> ret = aldebaran_get_current_clk_freq_by_table(smu,
> SMU_UCLK, &cur_value);
> if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get current
> mclk Failed!");
> + dev_err(smu->adev->dev, "%s mclk Failed!",
> attempt_string);
> return ret;
> }
>
> single_dpm_table = &(dpm_context-
> >dpm_tables.uclk_table);
> - ret = aldebaran_get_clk_table(smu, &clocks,
> single_dpm_table);
> - if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get memory
> clk levels Failed!");
> - return ret;
> - }
> -
> - for (i = 0; i < clocks.num_levels; i++)
> - *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n",
> - i, clocks.data[i].clocks_in_khz / 1000,
> - (clocks.num_levels == 1) ? "*" :
> - (aldebaran_freqs_in_same_level(
> -
> clocks.data[i].clocks_in_khz / 1000,
> -
> cur_value) ? "*" : ""));
> + aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> break;
> -
> case SMU_SOCCLK:
> ret = aldebaran_get_current_clk_freq_by_table(smu,
> SMU_SOCCLK, &cur_value);
> if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get current
> socclk Failed!");
> + dev_err(smu->adev->dev, "%s socclk Failed!",
> attempt_string);
> return ret;
> }
>
> single_dpm_table = &(dpm_context-
> >dpm_tables.soc_table);
> - ret = aldebaran_get_clk_table(smu, &clocks,
> single_dpm_table);
> - if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get socclk
> levels Failed!");
> - return ret;
> - }
> -
> - for (i = 0; i < clocks.num_levels; i++)
> - *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n",
> - i, clocks.data[i].clocks_in_khz / 1000,
> - (clocks.num_levels == 1) ? "*" :
> - (aldebaran_freqs_in_same_level(
> -
> clocks.data[i].clocks_in_khz / 1000,
> -
> cur_value) ? "*" : ""));
> + aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> break;
> -
> case SMU_FCLK:
> ret = aldebaran_get_current_clk_freq_by_table(smu,
> SMU_FCLK, &cur_value);
> if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get current
> fclk Failed!");
> + dev_err(smu->adev->dev, "%s fclk Failed!",
> attempt_string);
> return ret;
> }
>
> single_dpm_table = &(dpm_context-
> >dpm_tables.fclk_table);
> - ret = aldebaran_get_clk_table(smu, &clocks,
> single_dpm_table);
> - if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get fclk levels
> Failed!");
> - return ret;
> - }
> -
> - for (i = 0; i < single_dpm_table->count; i++)
> - *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n",
> - i, single_dpm_table-
> >dpm_levels[i].value,
> - (clocks.num_levels == 1) ? "*" :
> - (aldebaran_freqs_in_same_level(
> -
> clocks.data[i].clocks_in_khz / 1000,
> -
> cur_value) ? "*" : ""));
> + aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> break;
> -
> case SMU_VCLK:
> ret = aldebaran_get_current_clk_freq_by_table(smu,
> SMU_VCLK, &cur_value);
> if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get current
> vclk Failed!");
> + dev_err(smu->adev->dev, "%s vclk Failed!",
> attempt_string);
> return ret;
> }
>
> single_dpm_table = &(dpm_context-
> >dpm_tables.vclk_table);
> - ret = aldebaran_get_clk_table(smu, &clocks,
> single_dpm_table);
> - if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get vclk
> levels Failed!");
> - return ret;
> - }
> -
> - for (i = 0; i < single_dpm_table->count; i++)
> - *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n",
> - i, single_dpm_table-
> >dpm_levels[i].value,
> - (clocks.num_levels == 1) ? "*" :
> - (aldebaran_freqs_in_same_level(
> -
> clocks.data[i].clocks_in_khz / 1000,
> -
> cur_value) ? "*" : ""));
> + aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> break;
> -
> case SMU_DCLK:
> ret = aldebaran_get_current_clk_freq_by_table(smu,
> SMU_DCLK, &cur_value);
> if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get current
> dclk Failed!");
> + dev_err(smu->adev->dev, "%s dclk Failed!",
> attempt_string);
> return ret;
> }
>
> single_dpm_table = &(dpm_context-
> >dpm_tables.dclk_table);
> - ret = aldebaran_get_clk_table(smu, &clocks,
> single_dpm_table);
> - if (ret) {
> - dev_err(smu->adev->dev, "Attempt to get dclk
> levels Failed!");
> - return ret;
> + aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + switch (type) {
> + case SMU_OD_SCLK:
> + case SMU_SCLK:
> + /*
> + * 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++) {
> + clock_mhz = freq_values[i];
> + freq_match =
> aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> + freq_match |= (clocks.num_levels == 1);
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i,
> + clock_mhz,
> + freq_match ? "*" :
> "");
> + }
> + } else {
> + for (i = 0; i < display_levels; i++)
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i,
> + freq_values[i], i == 1 ?
> "*" : "");
> }
>
> - for (i = 0; i < single_dpm_table->count; i++)
> + break;
> + case SMU_OD_MCLK:
> + case SMU_MCLK:
> + case SMU_SOCCLK:
> + case SMU_FCLK:
> + case SMU_VCLK:
> + case SMU_DCLK:
> + for (i = 0; i < clocks.num_levels; i++) {
> + clock_mhz = clocks.data[i].clocks_in_khz / 1000;
> + freq_match =
> aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> + freq_match |= (clocks.num_levels == 1);
> *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n",
> - i, single_dpm_table-
> >dpm_levels[i].value,
> - (clocks.num_levels == 1) ? "*" :
> - (aldebaran_freqs_in_same_level(
> -
> clocks.data[i].clocks_in_khz / 1000,
> -
> cur_value) ? "*" : ""));
> + i, clock_mhz,
> + freq_match ? "*" : "");
> + }
> break;
> -
> default:
> return -EINVAL;
> - break;
> }
> -
> return 0;
> }
>
> --
> 2.35.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-13 7:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13 3:15 [PATCH v1 1/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for arcturus Darren Powell
2022-05-13 3:15 ` [PATCH v1 2/4] amdgpu/pm: Optimize arcturus_emit_clk_levels Darren Powell
2022-05-13 3:15 ` [PATCH v1 3/4] amdgpu/pm: Replace print_clock_levels with emit_clock_levels for aldebaran Darren Powell
2022-05-13 3:15 ` [PATCH v1 4/4] amdgpu/pm: Optimize aldebaran_emit_clk_levels Darren Powell
2022-05-13 7:48 ` Quan, Evan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.