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@xxxxxxx> --- .../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