* [PATCH 0/3] amdgpu/pm: Implement parallel sysfs_emit solution for navi10
@ 2021-12-08 6:36 Darren Powell
2021-12-08 6:36 ` [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset Darren Powell
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Darren Powell @ 2021-12-08 6:36 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
== Description ==
Scnprintf use within the kernel is not recommended, but simple sysfs_emit replacement has
not been successful due to the page alignment requirement of the function. This patch
set implements a new api "emit_clock_levels" to facilitate passing both the base and
offset to the device rather than just the write pointer.
This patch is only implemented for navi10 for some clocks to seek comment on the
implementation before extending the implementation to other clock values and devices.
Needs to be verified on a platform that supports the overclocking
== Patch Summary ==
linux: (git@gitlab.freedesktop.org:agd5f) origin/amd-staging-drm-next @ be0118f90e18
+ 351ae7734898 amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset
+ 76c13c49fc6b amdgpu/pm: insert attempt to call emit_clock_levels into amdgpu_get_pp_od_clk_voltage
+ 28c533371023 amdgpu/pm: Add Error Handling to emit_clocks stack
== System Summary ==
* DESKTOP(AMD FX-8350 + NAVI10(731F/ca), BIOS: F2)
+ ISO(Ubuntu 20.04.3 LTS)
+ Kernel(5.13.0-20211128-be0118f90e18-fdoagd5f)
== Test ==
LOGFILE=pp_clk.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
pp_dpm_mclk
pp_dpm_pcie
pp_dpm_socclk
pp_dpm_fclk
pp_dpm_dcefclk
pp_dpm_vclk
pp_dpm_dclk "
for f in $FILES
do
echo === $f === >> $LOGFILE
cat $HWMON_DIR/device/$f >> $LOGFILE
done
cat $LOGFILE
Darren Powell (3):
amdgpu/pm: Implement new API function "emit" that accepts buffer base
and write offset
amdgpu/pm: insert attempt to call emit_clock_levels into
amdgpu_get_pp_od_clk_voltage
amdgpu/pm: Add Error Handling to emit_clocks stack
.../gpu/drm/amd/include/kgd_pp_interface.h | 1 +
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 65 ++++--
drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 3 +
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 18 ++
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 49 ++++-
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 188 ++++++++++++++++++
6 files changed, 301 insertions(+), 23 deletions(-)
base-commit: be0118f90e18e3fb8191f17f51a9addc93d0f185
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset
2021-12-08 6:36 [PATCH 0/3] amdgpu/pm: Implement parallel sysfs_emit solution for navi10 Darren Powell
@ 2021-12-08 6:36 ` Darren Powell
2021-12-09 1:48 ` Quan, Evan
2021-12-08 6:36 ` [PATCH 2/3] amdgpu/pm: insert attempt to call emit_clock_levels into amdgpu_get_pp_od_clk_voltage Darren Powell
2021-12-08 6:36 ` [PATCH 3/3] amdgpu/pm: Add Error Handling to emit_clocks stack Darren Powell
2 siblings, 1 reply; 6+ messages in thread
From: Darren Powell @ 2021-12-08 6:36 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
- new power management function emit_clk_levels implemented by navi10_emit_clk_levels()
This function currently duplicates the functionality of navi10_print_clk_levels, where
snprintf is used write to the sysfs buffer. The first implementation to use sysfs_emit
was unsuccessful due to requirement that buf must be aligned to a specific memory
boundary. This solution passes the buffer base and write offset down the stack.
- new power management function emit_clock_levels implemented by smu_emit_ppclk_levels()
This function combines implementation of smu_print_ppclk_levels and
smu_print_smuclk_levels and calls emit_clk_levels
- new helper function smu_convert_to_smuclk called by smu_print_ppclk_levels and
smu_emit_ppclk_levels
- emit_clock_levels currently returns the length of the string written to the buffer,
maintaining the behaviour of print_clock_levels, but will be upgraded to return
error values in a subsequent patch
- failure of the emit_clock_levels at the top level (amdgpu_get_pp_dpm_clock) triggers a
fallback to the print_clock_levels, which can be removed after emit_clock_levels is
implemented across all platforms.
== Test ==
LOGFILE=pp_clk.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
pp_dpm_mclk
pp_dpm_pcie
pp_dpm_socclk
pp_dpm_fclk
pp_dpm_dcefclk
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/include/kgd_pp_interface.h | 1 +
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 20 +-
drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 3 +
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 10 +
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 50 ++++-
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 189 ++++++++++++++++++
6 files changed, 262 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index bac15c466733..43f43a4f321b 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -310,6 +310,7 @@ struct amd_pm_funcs {
int (*get_fan_speed_pwm)(void *handle, u32 *speed);
int (*force_clock_level)(void *handle, enum pp_clock_type type, uint32_t mask);
int (*print_clock_levels)(void *handle, enum pp_clock_type type, char *buf);
+ int (*emit_clock_levels)(void *handle, enum pp_clock_type type, char *buf, int *offset);
int (*force_performance_level)(void *handle, enum amd_dpm_forced_level level);
int (*get_sclk_od)(void *handle);
int (*set_sclk_od)(void *handle, uint32_t value);
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 49df4c20f09e..a1169a2397ca 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -1056,8 +1056,8 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev,
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
- ssize_t size;
- int ret;
+ int size = 0;
+ int ret = 0;
if (amdgpu_in_reset(adev))
return -EPERM;
@@ -1070,10 +1070,18 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev,
return ret;
}
- if (adev->powerplay.pp_funcs->print_clock_levels)
- size = amdgpu_dpm_print_clock_levels(adev, type, buf);
- else
- size = sysfs_emit(buf, "\n");
+ ret = -EOPNOTSUPP;
+ if (adev->powerplay.pp_funcs->emit_clock_levels) {
+ ret = amdgpu_dpm_emit_clock_levels(adev, type, buf, &size);
+ }
+
+ if (ret < 0) {
+ if (adev->powerplay.pp_funcs->print_clock_levels) {
+ size = amdgpu_dpm_print_clock_levels(adev, type, buf);
+ }
+ else
+ size = sysfs_emit(buf, "\n");
+ }
pm_runtime_mark_last_busy(ddev->dev);
pm_runtime_put_autosuspend(ddev->dev);
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 16e3f72d31b9..95add7afcc86 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -310,6 +310,9 @@ enum amdgpu_pcie_gen {
#define amdgpu_dpm_print_clock_levels(adev, type, buf) \
((adev)->powerplay.pp_funcs->print_clock_levels((adev)->powerplay.pp_handle, type, buf))
+#define amdgpu_dpm_emit_clock_levels(adev, type, buf, offset) \
+ ((adev)->powerplay.pp_funcs->emit_clock_levels((adev)->powerplay.pp_handle, type, buf, offset))
+
#define amdgpu_dpm_force_clock_level(adev, type, level) \
((adev)->powerplay.pp_funcs->force_clock_level((adev)->powerplay.pp_handle, type, level))
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index f738f7dc20c9..dc1325783c10 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -620,6 +620,16 @@ struct pptable_funcs {
*/
int (*print_clk_levels)(struct smu_context *smu, enum smu_clk_type clk_type, char *buf);
+ /**
+ * @emit_clk_levels: Print DPM clock levels for a clock domain
+ * to buffer using sysfs_emit_at. Star current level.
+ *
+ * Used for sysfs interfaces.
+ * &buf: sysfs buffer
+ * &offset: offset within buffer to start printing
+ */
+ int (*emit_clk_levels)(struct smu_context *smu, enum smu_clk_type clk_type, char *buf, int *offset);
+
/**
* @force_clk_levels: Set a range of allowed DPM levels for a clock
* domain.
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index e156add7b560..b0638d8308d2 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2397,11 +2397,8 @@ static int smu_print_smuclk_levels(struct smu_context *smu, enum smu_clk_type cl
return ret;
}
-static int smu_print_ppclk_levels(void *handle,
- enum pp_clock_type type,
- char *buf)
+static enum smu_clk_type smu_convert_to_smuclk(enum pp_clock_type type)
{
- struct smu_context *smu = handle;
enum smu_clk_type clk_type;
switch (type) {
@@ -2434,12 +2431,54 @@ static int smu_print_ppclk_levels(void *handle,
case OD_CCLK:
clk_type = SMU_OD_CCLK; break;
default:
- return -EINVAL;
+ clk_type = SMU_CLK_COUNT; break;
}
+ return clk_type;
+}
+
+static int smu_print_ppclk_levels(void *handle,
+ enum pp_clock_type type,
+ char *buf)
+{
+ struct smu_context *smu = handle;
+ enum smu_clk_type clk_type;
+
+ clk_type = smu_convert_to_smuclk(type);
+ if (clk_type == SMU_CLK_COUNT)
+ return -EINVAL;
+
return smu_print_smuclk_levels(smu, clk_type, buf);
}
+static int smu_emit_ppclk_levels(void *handle, enum pp_clock_type type, char *buf, int *offset)
+{
+ struct smu_context *smu = handle;
+ enum smu_clk_type clk_type;
+ int ret = 0;
+
+ clk_type = smu_convert_to_smuclk(type);
+ if (clk_type == SMU_CLK_COUNT)
+ return -EINVAL;
+
+ if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&smu->mutex);
+
+ if (smu->ppt_funcs->emit_clk_levels) {
+ ret = smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf, offset);
+ }
+ else {
+ ret = -EOPNOTSUPP;
+ }
+
+ mutex_unlock(&smu->mutex);
+
+ return ret;
+
+}
+
static int smu_od_edit_dpm_table(void *handle,
enum PP_OD_DPM_TABLE_COMMAND type,
long *input, uint32_t size)
@@ -3117,6 +3156,7 @@ static const struct amd_pm_funcs swsmu_pm_funcs = {
.get_fan_speed_pwm = smu_get_fan_speed_pwm,
.force_clock_level = smu_force_ppclk_levels,
.print_clock_levels = smu_print_ppclk_levels,
+ .emit_clock_levels = smu_emit_ppclk_levels,
.force_performance_level = smu_force_performance_level,
.read_sensor = smu_read_sensor,
.get_performance_level = smu_get_performance_level,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 60a557068ea4..e43c7e950a55 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1261,6 +1261,194 @@ static void navi10_od_setting_get_range(struct smu_11_0_overdrive_table *od_tabl
*max = od_table->max[setting];
}
+static int navi10_emit_clk_levels(struct smu_context *smu,
+ enum smu_clk_type clk_type, char *buf, int *offset)
+{
+ uint16_t *curve_settings;
+ int ret = 0;
+ uint32_t cur_value = 0, value = 0;
+ uint32_t freq_values[3] = {0};
+ uint32_t i, levels, mark_index = 0, count = 0;
+ struct smu_table_context *table_context = &smu->smu_table;
+ uint32_t gen_speed, lane_width;
+ struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
+ struct smu_11_0_dpm_context *dpm_context = smu_dpm->dpm_context;
+ PPTable_t *pptable = (PPTable_t *)table_context->driver_pptable;
+ OverDriveTable_t *od_table =
+ (OverDriveTable_t *)table_context->overdrive_table;
+ struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
+ uint32_t min_value, max_value;
+ int save_offset = *offset;
+
+ switch (clk_type) {
+ case SMU_GFXCLK:
+ case SMU_SCLK:
+ case SMU_SOCCLK:
+ case SMU_MCLK:
+ case SMU_UCLK:
+ case SMU_FCLK:
+ case SMU_VCLK:
+ case SMU_DCLK:
+ case SMU_DCEFCLK:
+ ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
+ if (ret)
+ return 0;
+
+ ret = smu_v11_0_get_dpm_level_count(smu, clk_type, &count);
+ if (ret)
+ return 0;
+
+ if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
+ for (i = 0; i < count; i++) {
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
+ if (ret)
+ return (*offset - save_offset);
+
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i, value,
+ cur_value == value ? "*" : "");
+
+ }
+ } else {
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
+ if (ret)
+ return 0;
+ ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_values[2]);
+ if (ret)
+ return 0;
+
+ freq_values[1] = cur_value;
+ mark_index = cur_value == freq_values[0] ? 0 :
+ cur_value == freq_values[2] ? 2 : 1;
+
+ levels = 3;
+ if (mark_index != 1) {
+ levels = 2;
+ freq_values[1] = freq_values[2];
+ }
+
+ for (i = 0; i < levels; i++) {
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i, freq_values[i],
+ i == mark_index ? "*" : "");
+ }
+ }
+ 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);
+ for (i = 0; i < NUM_LINK_LEVELS; i++) {
+ *offset += sysfs_emit_at(buf, *offset, "%d: %s %s %dMhz %s\n", i,
+ (dpm_context->dpm_tables.pcie_table.pcie_gen[i] == 0) ? "2.5GT/s," :
+ (dpm_context->dpm_tables.pcie_table.pcie_gen[i] == 1) ? "5.0GT/s," :
+ (dpm_context->dpm_tables.pcie_table.pcie_gen[i] == 2) ? "8.0GT/s," :
+ (dpm_context->dpm_tables.pcie_table.pcie_gen[i] == 3) ? "16.0GT/s," : "",
+ (dpm_context->dpm_tables.pcie_table.pcie_lane[i] == 1) ? "x1" :
+ (dpm_context->dpm_tables.pcie_table.pcie_lane[i] == 2) ? "x2" :
+ (dpm_context->dpm_tables.pcie_table.pcie_lane[i] == 3) ? "x4" :
+ (dpm_context->dpm_tables.pcie_table.pcie_lane[i] == 4) ? "x8" :
+ (dpm_context->dpm_tables.pcie_table.pcie_lane[i] == 5) ? "x12" :
+ (dpm_context->dpm_tables.pcie_table.pcie_lane[i] == 6) ? "x16" : "",
+ pptable->LclkFreq[i],
+ (gen_speed == dpm_context->dpm_tables.pcie_table.pcie_gen[i]) &&
+ (lane_width == dpm_context->dpm_tables.pcie_table.pcie_lane[i]) ?
+ "*" : "");
+ }
+ break;
+ case SMU_OD_SCLK:
+ if (!smu->od_enabled || !od_table || !od_settings)
+ break;
+ if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_LIMITS))
+ break;
+ *offset += sysfs_emit_at(buf, *offset, "OD_SCLK:\n0: %uMhz\n1: %uMhz\n",
+ od_table->GfxclkFmin, od_table->GfxclkFmax);
+ break;
+ case SMU_OD_MCLK:
+ if (!smu->od_enabled || !od_table || !od_settings)
+ break;
+ if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_UCLK_MAX))
+ break;
+ *offset += sysfs_emit_at(buf, *offset, "OD_MCLK:\n1: %uMHz\n", od_table->UclkFmax);
+ break;
+ case SMU_OD_VDDC_CURVE:
+ if (!smu->od_enabled || !od_table || !od_settings)
+ break;
+ if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_CURVE))
+ break;
+ *offset += sysfs_emit_at(buf, *offset, "OD_VDDC_CURVE:\n");
+ for (i = 0; i < 3; i++) {
+ switch (i) {
+ case 0:
+ curve_settings = &od_table->GfxclkFreq1;
+ break;
+ case 1:
+ curve_settings = &od_table->GfxclkFreq2;
+ break;
+ case 2:
+ curve_settings = &od_table->GfxclkFreq3;
+ break;
+ default:
+ break;
+ }
+ *offset += sysfs_emit_at(buf, *offset, "%d: %uMHz %umV\n",
+ i, curve_settings[0],
+ curve_settings[1] / NAVI10_VOLTAGE_SCALE);
+ }
+ break;
+ case SMU_OD_RANGE:
+ if (!smu->od_enabled || !od_table || !od_settings)
+ break;
+ *offset += sysfs_emit_at(buf, *offset, "%s:\n", "OD_RANGE");
+
+ if (navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_LIMITS)) {
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_GFXCLKFMIN,
+ &min_value, NULL);
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_GFXCLKFMAX,
+ NULL, &max_value);
+ *offset+= sysfs_emit_at(buf, *offset, "SCLK: %7uMhz %10uMhz\n",
+ min_value, max_value);
+ }
+
+ if (navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_UCLK_MAX)) {
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_UCLKFMAX,
+ &min_value, &max_value);
+ *offset+= sysfs_emit_at(buf, *offset, "MCLK: %7uMhz %10uMhz\n",
+ min_value, max_value);
+ }
+
+ if (navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_CURVE)) {
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P1,
+ &min_value, &max_value);
+ *offset += sysfs_emit_at(buf, *offset, "VDDC_CURVE_SCLK[0]: %7uMhz %10uMhz\n",
+ min_value, max_value);
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P1,
+ &min_value, &max_value);
+ *offset += sysfs_emit_at(buf, *offset, "VDDC_CURVE_VOLT[0]: %7dmV %11dmV\n",
+ min_value, max_value);
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P2,
+ &min_value, &max_value);
+ *offset += sysfs_emit_at(buf, *offset, "VDDC_CURVE_SCLK[1]: %7uMhz %10uMhz\n",
+ min_value, max_value);
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P2,
+ &min_value, &max_value);
+ *offset += sysfs_emit_at(buf, *offset, "VDDC_CURVE_VOLT[1]: %7dmV %11dmV\n",
+ min_value, max_value);
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P3,
+ &min_value, &max_value);
+ *offset += sysfs_emit_at(buf, *offset, "VDDC_CURVE_SCLK[2]: %7uMhz %10uMhz\n",
+ min_value, max_value);
+ navi10_od_setting_get_range(od_settings, SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P3,
+ &min_value, &max_value);
+ *offset += sysfs_emit_at(buf, *offset, "VDDC_CURVE_VOLT[2]: %7dmV %11dmV\n",
+ min_value, max_value);
+ }
+
+ break;
+ default:
+ break;
+ }
+
+ return (*offset - save_offset);
+}
+
static int navi10_print_clk_levels(struct smu_context *smu,
enum smu_clk_type clk_type, char *buf)
{
@@ -3245,6 +3433,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
.i2c_init = navi10_i2c_control_init,
.i2c_fini = navi10_i2c_control_fini,
.print_clk_levels = navi10_print_clk_levels,
+ .emit_clk_levels = navi10_emit_clk_levels,
.force_clk_levels = navi10_force_clk_levels,
.populate_umd_state_clk = navi10_populate_umd_state_clk,
.get_clock_by_type_with_latency = navi10_get_clock_by_type_with_latency,
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] amdgpu/pm: insert attempt to call emit_clock_levels into amdgpu_get_pp_od_clk_voltage
2021-12-08 6:36 [PATCH 0/3] amdgpu/pm: Implement parallel sysfs_emit solution for navi10 Darren Powell
2021-12-08 6:36 ` [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset Darren Powell
@ 2021-12-08 6:36 ` Darren Powell
2021-12-08 6:36 ` [PATCH 3/3] amdgpu/pm: Add Error Handling to emit_clocks stack Darren Powell
2 siblings, 0 replies; 6+ messages in thread
From: Darren Powell @ 2021-12-08 6:36 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
Use new API function emit_clock_levels to display to overclocking values, with a fallback
to the print_clock_levels if the first call fails.
== Test ==
LOGFILE=pp_clk.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"
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>
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 46 ++++++++++++++++++++++--------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index a1169a2397ca..04992a3bc1d2 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -904,8 +904,17 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
{
struct drm_device *ddev = dev_get_drvdata(dev);
struct amdgpu_device *adev = drm_to_adev(ddev);
- ssize_t size;
+ int size = 0;
int ret;
+ enum pp_clock_type od_clocks[6] = {
+ OD_SCLK,
+ OD_MCLK,
+ OD_VDDC_CURVE,
+ OD_RANGE,
+ OD_VDDGFX_OFFSET,
+ OD_CCLK,
+ };
+ uint clk_index;
if (amdgpu_in_reset(adev))
return -EPERM;
@@ -918,16 +927,29 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
return ret;
}
- if (adev->powerplay.pp_funcs->print_clock_levels) {
- size = amdgpu_dpm_print_clock_levels(adev, OD_SCLK, buf);
- size += amdgpu_dpm_print_clock_levels(adev, OD_MCLK, buf+size);
- size += amdgpu_dpm_print_clock_levels(adev, OD_VDDC_CURVE, buf+size);
- size += amdgpu_dpm_print_clock_levels(adev, OD_VDDGFX_OFFSET, buf+size);
- size += amdgpu_dpm_print_clock_levels(adev, OD_RANGE, buf+size);
- size += amdgpu_dpm_print_clock_levels(adev, OD_CCLK, buf+size);
- } else {
- size = sysfs_emit(buf, "\n");
+ ret = -EOPNOTSUPP;
+ if (adev->powerplay.pp_funcs->emit_clock_levels) {
+ ret = amdgpu_dpm_emit_clock_levels(adev, od_clocks[0], buf, &size);
+ }
+
+ if (ret > 0) {
+ /* Proceed with emit for other od clocks if the first call succeeds */
+ for(clk_index = 1 ; clk_index < 6 ; clk_index++)
+ amdgpu_dpm_emit_clock_levels(adev, od_clocks[clk_index], buf, &size);
}
+ else {
+ if (adev->powerplay.pp_funcs->print_clock_levels) {
+ size = amdgpu_dpm_print_clock_levels(adev, OD_SCLK, buf);
+ size += amdgpu_dpm_print_clock_levels(adev, OD_MCLK, buf+size);
+ size += amdgpu_dpm_print_clock_levels(adev, OD_VDDC_CURVE, buf+size);
+ size += amdgpu_dpm_print_clock_levels(adev, OD_VDDGFX_OFFSET, buf+size);
+ size += amdgpu_dpm_print_clock_levels(adev, OD_RANGE, buf+size);
+ size += amdgpu_dpm_print_clock_levels(adev, OD_CCLK, buf+size);
+ } else {
+ size = sysfs_emit(buf, "\n");
+ }
+ }
+
pm_runtime_mark_last_busy(ddev->dev);
pm_runtime_put_autosuspend(ddev->dev);
@@ -2075,8 +2097,8 @@ static int default_attr_update(struct amdgpu_device *adev, struct amdgpu_device_
} else if (DEVICE_ATTR_IS(pp_od_clk_voltage)) {
*states = ATTR_STATE_UNSUPPORTED;
if ((is_support_sw_smu(adev) && adev->smu.od_enabled) ||
- (is_support_sw_smu(adev) && adev->smu.is_apu) ||
- (!is_support_sw_smu(adev) && hwmgr->od_enabled))
+ (is_support_sw_smu(adev) && adev->smu.is_apu) ||
+ (!is_support_sw_smu(adev) && hwmgr->od_enabled) )
*states = ATTR_STATE_SUPPORTED;
} else if (DEVICE_ATTR_IS(mem_busy_percent)) {
if (adev->flags & AMD_IS_APU || asic_type == CHIP_VEGA10)
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] amdgpu/pm: Add Error Handling to emit_clocks stack
2021-12-08 6:36 [PATCH 0/3] amdgpu/pm: Implement parallel sysfs_emit solution for navi10 Darren Powell
2021-12-08 6:36 ` [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset Darren Powell
2021-12-08 6:36 ` [PATCH 2/3] amdgpu/pm: insert attempt to call emit_clock_levels into amdgpu_get_pp_od_clk_voltage Darren Powell
@ 2021-12-08 6:36 ` Darren Powell
2 siblings, 0 replies; 6+ messages in thread
From: Darren Powell @ 2021-12-08 6:36 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
Previous implementation of print_clocks required return of bytes written
to calling function via return value. Passing this value in by reference
allows us now to pass back error codes up the calling stack.
- Errors from get_current_clk_freq, get_dpm_level_count & get_dpm_freq
now passed back up the stack
- Added -EOPNOTSUPP when encountering for OD clocks
!od_enabled
missing od_table or od_settings
- OD_RANGE continues to print any subset of the ODCAP settings it can find
without reporting error for any missing
- smu_emit_ppclk returns ENOENT if emit_clk_levels is not supported by the
device
- modified calling logic so fallback to print_clock_levels is only attempted
if emit_clk_levels is not (yet) supported by the device
== Test ==
LOGFILE=pp_clk.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
pp_dpm_mclk
pp_dpm_pcie
pp_dpm_socclk
pp_dpm_fclk
pp_dpm_dcefclk
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>
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 19 ++++++++---------
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 10 ++++++++-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 9 ++++----
.../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 21 +++++++++----------
4 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 04992a3bc1d2..741e991baaca 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -927,17 +927,16 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
return ret;
}
- ret = -EOPNOTSUPP;
+ ret = -ENOENT;
if (adev->powerplay.pp_funcs->emit_clock_levels) {
- ret = amdgpu_dpm_emit_clock_levels(adev, od_clocks[0], buf, &size);
+ for(clk_index = 0 ; clk_index < 6 ; clk_index++) {
+ ret = amdgpu_dpm_emit_clock_levels(adev, od_clocks[clk_index], buf, &size);
+ if (ret)
+ break;
+ }
}
- if (ret > 0) {
- /* Proceed with emit for other od clocks if the first call succeeds */
- for(clk_index = 1 ; clk_index < 6 ; clk_index++)
- amdgpu_dpm_emit_clock_levels(adev, od_clocks[clk_index], buf, &size);
- }
- else {
+ if (ret == -ENOENT) {
if (adev->powerplay.pp_funcs->print_clock_levels) {
size = amdgpu_dpm_print_clock_levels(adev, OD_SCLK, buf);
size += amdgpu_dpm_print_clock_levels(adev, OD_MCLK, buf+size);
@@ -1092,12 +1091,12 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev,
return ret;
}
- ret = -EOPNOTSUPP;
+ ret = -ENOENT;
if (adev->powerplay.pp_funcs->emit_clock_levels) {
ret = amdgpu_dpm_emit_clock_levels(adev, type, buf, &size);
}
- if (ret < 0) {
+ if (ret == -ENOENT) {
if (adev->powerplay.pp_funcs->print_clock_levels) {
size = amdgpu_dpm_print_clock_levels(adev, type, buf);
}
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index dc1325783c10..7db6bec158d9 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -617,6 +617,11 @@ struct pptable_funcs {
* to buffer. Star current level.
*
* Used for sysfs interfaces.
+ * Return: Number of characters written to the buffer
+ *
+ * On Success the function will return 0 or a positive value to
+ * record the number of characters written to the buffer. A negative
+ * value is returned on error.
*/
int (*print_clk_levels)(struct smu_context *smu, enum smu_clk_type clk_type, char *buf);
@@ -626,7 +631,10 @@ struct pptable_funcs {
*
* Used for sysfs interfaces.
* &buf: sysfs buffer
- * &offset: offset within buffer to start printing
+ * &offset: offset within buffer to start printing, which is updated by the
+ * function.
+ *
+ * Return: 0 on Success or Negative to indicate an error occurred.
*/
int (*emit_clk_levels)(struct smu_context *smu, enum smu_clk_type clk_type, char *buf, int *offset);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index b0638d8308d2..6dab167928b0 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2464,16 +2464,15 @@ static int smu_emit_ppclk_levels(void *handle, enum pp_clock_type type, char *bu
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
- mutex_lock(&smu->mutex);
if (smu->ppt_funcs->emit_clk_levels) {
+ mutex_lock(&smu->mutex);
ret = smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf, offset);
+ mutex_unlock(&smu->mutex);
}
- else {
- ret = -EOPNOTSUPP;
- }
+ else
+ ret = -ENOENT;
- mutex_unlock(&smu->mutex);
return ret;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index e43c7e950a55..a9ba1cb35ddf 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1278,7 +1278,6 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
(OverDriveTable_t *)table_context->overdrive_table;
struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
uint32_t min_value, max_value;
- int save_offset = *offset;
switch (clk_type) {
case SMU_GFXCLK:
@@ -1292,17 +1291,17 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
case SMU_DCEFCLK:
ret = navi10_get_current_clk_freq_by_table(smu, clk_type, &cur_value);
if (ret)
- return 0;
+ return ret;
ret = smu_v11_0_get_dpm_level_count(smu, clk_type, &count);
if (ret)
- return 0;
+ return ret;
if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
for (i = 0; i < count; i++) {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
if (ret)
- return (*offset - save_offset);
+ return ret;
*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i, value,
cur_value == value ? "*" : "");
@@ -1311,10 +1310,10 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
} else {
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, 0, &freq_values[0]);
if (ret)
- return 0;
+ return ret;
ret = smu_v11_0_get_dpm_freq_by_index(smu, clk_type, count - 1, &freq_values[2]);
if (ret)
- return 0;
+ return ret;
freq_values[1] = cur_value;
mark_index = cur_value == freq_values[0] ? 0 :
@@ -1355,7 +1354,7 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
break;
case SMU_OD_SCLK:
if (!smu->od_enabled || !od_table || !od_settings)
- break;
+ return -EOPNOTSUPP;
if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_LIMITS))
break;
*offset += sysfs_emit_at(buf, *offset, "OD_SCLK:\n0: %uMhz\n1: %uMhz\n",
@@ -1363,14 +1362,14 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
break;
case SMU_OD_MCLK:
if (!smu->od_enabled || !od_table || !od_settings)
- break;
+ return -EOPNOTSUPP;
if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_UCLK_MAX))
break;
*offset += sysfs_emit_at(buf, *offset, "OD_MCLK:\n1: %uMHz\n", od_table->UclkFmax);
break;
case SMU_OD_VDDC_CURVE:
if (!smu->od_enabled || !od_table || !od_settings)
- break;
+ return -EOPNOTSUPP;
if (!navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_CURVE))
break;
*offset += sysfs_emit_at(buf, *offset, "OD_VDDC_CURVE:\n");
@@ -1395,7 +1394,7 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
break;
case SMU_OD_RANGE:
if (!smu->od_enabled || !od_table || !od_settings)
- break;
+ return -EOPNOTSUPP;
*offset += sysfs_emit_at(buf, *offset, "%s:\n", "OD_RANGE");
if (navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_GFXCLK_LIMITS)) {
@@ -1446,7 +1445,7 @@ static int navi10_emit_clk_levels(struct smu_context *smu,
break;
}
- return (*offset - save_offset);
+ return 0;
}
static int navi10_print_clk_levels(struct smu_context *smu,
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset
2021-12-08 6:36 ` [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset Darren Powell
@ 2021-12-09 1:48 ` Quan, Evan
2021-12-09 5:18 ` Powell, Darren
0 siblings, 1 reply; 6+ messages in thread
From: Quan, Evan @ 2021-12-09 1:48 UTC (permalink / raw)
To: Powell, Darren, amd-gfx; +Cc: Powell, Darren
[AMD Official Use Only]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Darren Powell
> Sent: Wednesday, December 8, 2021 2:36 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Powell, Darren <Darren.Powell@amd.com>
> Subject: [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that
> accepts buffer base and write offset
>
> - new power management function emit_clk_levels implemented by
> navi10_emit_clk_levels()
> This function currently duplicates the functionality of
> navi10_print_clk_levels, where
> snprintf is used write to the sysfs buffer. The first implementation to use
> sysfs_emit
> was unsuccessful due to requirement that buf must be aligned to a
> specific memory
> boundary. This solution passes the buffer base and write offset down the
> stack.
> - new power management function emit_clock_levels implemented by
> smu_emit_ppclk_levels()
> This function combines implementation of smu_print_ppclk_levels and
> smu_print_smuclk_levels and calls emit_clk_levels
> - new helper function smu_convert_to_smuclk called by
> smu_print_ppclk_levels and
> smu_emit_ppclk_levels
> - emit_clock_levels currently returns the length of the string written to the
> buffer,
> maintaining the behaviour of print_clock_levels, but will be upgraded to
> return
> error values in a subsequent patch
> - failure of the emit_clock_levels at the top level
> (amdgpu_get_pp_dpm_clock) triggers a
> fallback to the print_clock_levels, which can be removed after
> emit_clock_levels is
> implemented across all platforms.
>
> == Test ==
> LOGFILE=pp_clk.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
> pp_dpm_mclk
> pp_dpm_pcie
> pp_dpm_socclk
> pp_dpm_fclk
> pp_dpm_dcefclk
> 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/include/kgd_pp_interface.h | 1 +
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 20 +-
> drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 3 +
> drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 10 +
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 50 ++++-
> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 189
> ++++++++++++++++++
> 6 files changed, 262 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> index bac15c466733..43f43a4f321b 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -310,6 +310,7 @@ struct amd_pm_funcs {
> int (*get_fan_speed_pwm)(void *handle, u32 *speed);
> int (*force_clock_level)(void *handle, enum pp_clock_type type,
> uint32_t mask);
> int (*print_clock_levels)(void *handle, enum pp_clock_type type,
> char *buf);
> + int (*emit_clock_levels)(void *handle, enum pp_clock_type type,
> char *buf, int *offset);
> int (*force_performance_level)(void *handle, enum
> amd_dpm_forced_level level);
> int (*get_sclk_od)(void *handle);
> int (*set_sclk_od)(void *handle, uint32_t value);
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 49df4c20f09e..a1169a2397ca 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -1056,8 +1056,8 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct
> device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - ssize_t size;
> - int ret;
> + int size = 0;
> + int ret = 0;
>
> if (amdgpu_in_reset(adev))
> return -EPERM;
> @@ -1070,10 +1070,18 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct
> device *dev,
> return ret;
> }
>
> - if (adev->powerplay.pp_funcs->print_clock_levels)
> - size = amdgpu_dpm_print_clock_levels(adev, type, buf);
> - else
> - size = sysfs_emit(buf, "\n");
> + ret = -EOPNOTSUPP;
> + if (adev->powerplay.pp_funcs->emit_clock_levels) {
> + ret = amdgpu_dpm_emit_clock_levels(adev, type, buf,
> &size);
> + }
The whole idea seems fine to me. However, we are trying to do some cleanups to avoid spiking into power internals(as above via adev->powerplay.pp_funcs->emit_clock_levels).
Check the patch series below:
https://lists.freedesktop.org/archives/amd-gfx/2021-December/072096.html
So, it would be better if you can rebase the patches here based on that. Or you can wait a while to let me land them first.
BR
Evan
> +
> + if (ret < 0) {
> + if (adev->powerplay.pp_funcs->print_clock_levels) {
> + size = amdgpu_dpm_print_clock_levels(adev, type,
> buf);
> + }
> + else
> + size = sysfs_emit(buf, "\n");
> + }
>
> pm_runtime_mark_last_busy(ddev->dev);
> pm_runtime_put_autosuspend(ddev->dev);
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 16e3f72d31b9..95add7afcc86 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -310,6 +310,9 @@ enum amdgpu_pcie_gen {
> #define amdgpu_dpm_print_clock_levels(adev, type, buf) \
> ((adev)->powerplay.pp_funcs->print_clock_levels((adev)-
> >powerplay.pp_handle, type, buf))
>
> +#define amdgpu_dpm_emit_clock_levels(adev, type, buf, offset) \
> + ((adev)->powerplay.pp_funcs->emit_clock_levels((adev)-
> >powerplay.pp_handle, type, buf, offset))
> +
> #define amdgpu_dpm_force_clock_level(adev, type, level) \
> ((adev)->powerplay.pp_funcs->force_clock_level((adev)-
> >powerplay.pp_handle, type, level))
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index f738f7dc20c9..dc1325783c10 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -620,6 +620,16 @@ struct pptable_funcs {
> */
> int (*print_clk_levels)(struct smu_context *smu, enum
> smu_clk_type clk_type, char *buf);
>
> + /**
> + * @emit_clk_levels: Print DPM clock levels for a clock domain
> + * to buffer using sysfs_emit_at. Star current level.
> + *
> + * Used for sysfs interfaces.
> + * &buf: sysfs buffer
> + * &offset: offset within buffer to start printing
> + */
> + int (*emit_clk_levels)(struct smu_context *smu, enum
> smu_clk_type clk_type, char *buf, int *offset);
> +
> /**
> * @force_clk_levels: Set a range of allowed DPM levels for a clock
> * domain.
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index e156add7b560..b0638d8308d2 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -2397,11 +2397,8 @@ static int smu_print_smuclk_levels(struct
> smu_context *smu, enum smu_clk_type cl
> return ret;
> }
>
> -static int smu_print_ppclk_levels(void *handle,
> - enum pp_clock_type type,
> - char *buf)
> +static enum smu_clk_type smu_convert_to_smuclk(enum pp_clock_type
> type)
> {
> - struct smu_context *smu = handle;
> enum smu_clk_type clk_type;
>
> switch (type) {
> @@ -2434,12 +2431,54 @@ static int smu_print_ppclk_levels(void *handle,
> case OD_CCLK:
> clk_type = SMU_OD_CCLK; break;
> default:
> - return -EINVAL;
> + clk_type = SMU_CLK_COUNT; break;
> }
>
> + return clk_type;
> +}
> +
> +static int smu_print_ppclk_levels(void *handle,
> + enum pp_clock_type type,
> + char *buf)
> +{
> + struct smu_context *smu = handle;
> + enum smu_clk_type clk_type;
> +
> + clk_type = smu_convert_to_smuclk(type);
> + if (clk_type == SMU_CLK_COUNT)
> + return -EINVAL;
> +
> return smu_print_smuclk_levels(smu, clk_type, buf);
> }
>
> +static int smu_emit_ppclk_levels(void *handle, enum pp_clock_type type,
> char *buf, int *offset)
> +{
> + struct smu_context *smu = handle;
> + enum smu_clk_type clk_type;
> + int ret = 0;
> +
> + clk_type = smu_convert_to_smuclk(type);
> + if (clk_type == SMU_CLK_COUNT)
> + return -EINVAL;
> +
> + if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&smu->mutex);
> +
> + if (smu->ppt_funcs->emit_clk_levels) {
> + ret = smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf,
> offset);
> + }
> + else {
> + ret = -EOPNOTSUPP;
> + }
> +
> + mutex_unlock(&smu->mutex);
> +
> + return ret;
> +
> +}
> +
> static int smu_od_edit_dpm_table(void *handle,
> enum PP_OD_DPM_TABLE_COMMAND
> type,
> long *input, uint32_t size)
> @@ -3117,6 +3156,7 @@ static const struct amd_pm_funcs
> swsmu_pm_funcs = {
> .get_fan_speed_pwm = smu_get_fan_speed_pwm,
> .force_clock_level = smu_force_ppclk_levels,
> .print_clock_levels = smu_print_ppclk_levels,
> + .emit_clock_levels = smu_emit_ppclk_levels,
> .force_performance_level = smu_force_performance_level,
> .read_sensor = smu_read_sensor,
> .get_performance_level = smu_get_performance_level,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 60a557068ea4..e43c7e950a55 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -1261,6 +1261,194 @@ static void navi10_od_setting_get_range(struct
> smu_11_0_overdrive_table *od_tabl
> *max = od_table->max[setting];
> }
>
> +static int navi10_emit_clk_levels(struct smu_context *smu,
> + enum smu_clk_type clk_type, char *buf, int *offset)
> +{
> + uint16_t *curve_settings;
> + int ret = 0;
> + uint32_t cur_value = 0, value = 0;
> + uint32_t freq_values[3] = {0};
> + uint32_t i, levels, mark_index = 0, count = 0;
> + struct smu_table_context *table_context = &smu->smu_table;
> + uint32_t gen_speed, lane_width;
> + struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
> + struct smu_11_0_dpm_context *dpm_context = smu_dpm-
> >dpm_context;
> + PPTable_t *pptable = (PPTable_t *)table_context->driver_pptable;
> + OverDriveTable_t *od_table =
> + (OverDriveTable_t *)table_context->overdrive_table;
> + struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
> + uint32_t min_value, max_value;
> + int save_offset = *offset;
> +
> + switch (clk_type) {
> + case SMU_GFXCLK:
> + case SMU_SCLK:
> + case SMU_SOCCLK:
> + case SMU_MCLK:
> + case SMU_UCLK:
> + case SMU_FCLK:
> + case SMU_VCLK:
> + case SMU_DCLK:
> + case SMU_DCEFCLK:
> + ret = navi10_get_current_clk_freq_by_table(smu, clk_type,
> &cur_value);
> + if (ret)
> + return 0;
> +
> + ret = smu_v11_0_get_dpm_level_count(smu, clk_type,
> &count);
> + if (ret)
> + return 0;
> +
> + if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
> + for (i = 0; i < count; i++) {
> + ret =
> smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
> + if (ret)
> + return (*offset - save_offset);
> +
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i, value,
> + cur_value == value ? "*" : "");
> +
> + }
> + } else {
> + ret = smu_v11_0_get_dpm_freq_by_index(smu,
> clk_type, 0, &freq_values[0]);
> + if (ret)
> + return 0;
> + ret = smu_v11_0_get_dpm_freq_by_index(smu,
> clk_type, count - 1, &freq_values[2]);
> + if (ret)
> + return 0;
> +
> + freq_values[1] = cur_value;
> + mark_index = cur_value == freq_values[0] ? 0 :
> + cur_value == freq_values[2] ? 2 : 1;
> +
> + levels = 3;
> + if (mark_index != 1) {
> + levels = 2;
> + freq_values[1] = freq_values[2];
> + }
> +
> + for (i = 0; i < levels; i++) {
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i, freq_values[i],
> + i == mark_index ? "*" : "");
> + }
> + }
> + 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);
> + for (i = 0; i < NUM_LINK_LEVELS; i++) {
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %s %s %dMhz %s\n", i,
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 0) ? "2.5GT/s," :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 1) ? "5.0GT/s," :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 2) ? "8.0GT/s," :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 3) ? "16.0GT/s," : "",
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 1) ? "x1" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 2) ? "x2" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 3) ? "x4" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 4) ? "x8" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 5) ? "x12" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 6) ? "x16" : "",
> + pptable->LclkFreq[i],
> + (gen_speed == dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i]) &&
> + (lane_width == dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i]) ?
> + "*" : "");
> + }
> + break;
> + case SMU_OD_SCLK:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + if (!navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_LIMITS))
> + break;
> + *offset += sysfs_emit_at(buf, *offset,
> "OD_SCLK:\n0: %uMhz\n1: %uMhz\n",
> + od_table->GfxclkFmin, od_table-
> >GfxclkFmax);
> + break;
> + case SMU_OD_MCLK:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + if (!navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_UCLK_MAX))
> + break;
> + *offset += sysfs_emit_at(buf, *offset,
> "OD_MCLK:\n1: %uMHz\n", od_table->UclkFmax);
> + break;
> + case SMU_OD_VDDC_CURVE:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + if (!navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_CURVE))
> + break;
> + *offset += sysfs_emit_at(buf, *offset,
> "OD_VDDC_CURVE:\n");
> + for (i = 0; i < 3; i++) {
> + switch (i) {
> + case 0:
> + curve_settings = &od_table->GfxclkFreq1;
> + break;
> + case 1:
> + curve_settings = &od_table->GfxclkFreq2;
> + break;
> + case 2:
> + curve_settings = &od_table->GfxclkFreq3;
> + break;
> + default:
> + break;
> + }
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMHz %umV\n",
> + i, curve_settings[0],
> + curve_settings[1] /
> NAVI10_VOLTAGE_SCALE);
> + }
> + break;
> + case SMU_OD_RANGE:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + *offset += sysfs_emit_at(buf, *offset, "%s:\n",
> "OD_RANGE");
> +
> + if (navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_LIMITS)) {
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_GFXCLKFMIN,
> + &min_value, NULL);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_GFXCLKFMAX,
> + NULL, &max_value);
> + *offset+= sysfs_emit_at(buf, *offset,
> "SCLK: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + }
> +
> + if (navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_UCLK_MAX)) {
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_UCLKFMAX,
> + &min_value, &max_value);
> + *offset+= sysfs_emit_at(buf, *offset,
> "MCLK: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + }
> +
> + if (navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_CURVE)) {
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P1,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_SCLK[0]: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P1,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_VOLT[0]: %7dmV %11dmV\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P2,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_SCLK[1]: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P2,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_VOLT[1]: %7dmV %11dmV\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P3,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_SCLK[2]: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P3,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_VOLT[2]: %7dmV %11dmV\n",
> + min_value, max_value);
> + }
> +
> + break;
> + default:
> + break;
> + }
> +
> + return (*offset - save_offset);
> +}
> +
> static int navi10_print_clk_levels(struct smu_context *smu,
> enum smu_clk_type clk_type, char *buf)
> {
> @@ -3245,6 +3433,7 @@ static const struct pptable_funcs navi10_ppt_funcs
> = {
> .i2c_init = navi10_i2c_control_init,
> .i2c_fini = navi10_i2c_control_fini,
> .print_clk_levels = navi10_print_clk_levels,
> + .emit_clk_levels = navi10_emit_clk_levels,
> .force_clk_levels = navi10_force_clk_levels,
> .populate_umd_state_clk = navi10_populate_umd_state_clk,
> .get_clock_by_type_with_latency =
> navi10_get_clock_by_type_with_latency,
> --
> 2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset
2021-12-09 1:48 ` Quan, Evan
@ 2021-12-09 5:18 ` Powell, Darren
0 siblings, 0 replies; 6+ messages in thread
From: Powell, Darren @ 2021-12-09 5:18 UTC (permalink / raw)
To: Quan, Evan, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 23210 bytes --]
[AMD Official Use Only]
> The whole idea seems fine to me. However, we are trying to do some cleanups to avoid spiking into power internals(as above via adev->powerplay.pp_funcs->emit_clock_levels).
> Check the patch series below:
> https://lists.freedesktop.org/archives/amd-gfx/2021-December/072096.html
> So, it would be better if you can rebase the patches here based on that. Or you can wait a while to let me land them first.
> BR
> Evan
I can wait for it to land and rebase then, will send v2 of series once this happens
Darren
________________________________
From: Quan, Evan <Evan.Quan@amd.com>
Sent: Wednesday, December 8, 2021 8:48 PM
To: Powell, Darren <Darren.Powell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Powell, Darren <Darren.Powell@amd.com>
Subject: RE: [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset
[AMD Official Use Only]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Darren Powell
> Sent: Wednesday, December 8, 2021 2:36 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Powell, Darren <Darren.Powell@amd.com>
> Subject: [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that
> accepts buffer base and write offset
>
> - new power management function emit_clk_levels implemented by
> navi10_emit_clk_levels()
> This function currently duplicates the functionality of
> navi10_print_clk_levels, where
> snprintf is used write to the sysfs buffer. The first implementation to use
> sysfs_emit
> was unsuccessful due to requirement that buf must be aligned to a
> specific memory
> boundary. This solution passes the buffer base and write offset down the
> stack.
> - new power management function emit_clock_levels implemented by
> smu_emit_ppclk_levels()
> This function combines implementation of smu_print_ppclk_levels and
> smu_print_smuclk_levels and calls emit_clk_levels
> - new helper function smu_convert_to_smuclk called by
> smu_print_ppclk_levels and
> smu_emit_ppclk_levels
> - emit_clock_levels currently returns the length of the string written to the
> buffer,
> maintaining the behaviour of print_clock_levels, but will be upgraded to
> return
> error values in a subsequent patch
> - failure of the emit_clock_levels at the top level
> (amdgpu_get_pp_dpm_clock) triggers a
> fallback to the print_clock_levels, which can be removed after
> emit_clock_levels is
> implemented across all platforms.
>
> == Test ==
> LOGFILE=pp_clk.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
> pp_dpm_mclk
> pp_dpm_pcie
> pp_dpm_socclk
> pp_dpm_fclk
> pp_dpm_dcefclk
> 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/include/kgd_pp_interface.h | 1 +
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 20 +-
> drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 3 +
> drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 10 +
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 50 ++++-
> .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 189
> ++++++++++++++++++
> 6 files changed, 262 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> index bac15c466733..43f43a4f321b 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -310,6 +310,7 @@ struct amd_pm_funcs {
> int (*get_fan_speed_pwm)(void *handle, u32 *speed);
> int (*force_clock_level)(void *handle, enum pp_clock_type type,
> uint32_t mask);
> int (*print_clock_levels)(void *handle, enum pp_clock_type type,
> char *buf);
> + int (*emit_clock_levels)(void *handle, enum pp_clock_type type,
> char *buf, int *offset);
> int (*force_performance_level)(void *handle, enum
> amd_dpm_forced_level level);
> int (*get_sclk_od)(void *handle);
> int (*set_sclk_od)(void *handle, uint32_t value);
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 49df4c20f09e..a1169a2397ca 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -1056,8 +1056,8 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct
> device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - ssize_t size;
> - int ret;
> + int size = 0;
> + int ret = 0;
>
> if (amdgpu_in_reset(adev))
> return -EPERM;
> @@ -1070,10 +1070,18 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct
> device *dev,
> return ret;
> }
>
> - if (adev->powerplay.pp_funcs->print_clock_levels)
> - size = amdgpu_dpm_print_clock_levels(adev, type, buf);
> - else
> - size = sysfs_emit(buf, "\n");
> + ret = -EOPNOTSUPP;
> + if (adev->powerplay.pp_funcs->emit_clock_levels) {
> + ret = amdgpu_dpm_emit_clock_levels(adev, type, buf,
> &size);
> + }
The whole idea seems fine to me. However, we are trying to do some cleanups to avoid spiking into power internals(as above via adev->powerplay.pp_funcs->emit_clock_levels).
Check the patch series below:
https://lists.freedesktop.org/archives/amd-gfx/2021-December/072096.html
So, it would be better if you can rebase the patches here based on that. Or you can wait a while to let me land them first.
BR
Evan
> +
> + if (ret < 0) {
> + if (adev->powerplay.pp_funcs->print_clock_levels) {
> + size = amdgpu_dpm_print_clock_levels(adev, type,
> buf);
> + }
> + else
> + size = sysfs_emit(buf, "\n");
> + }
>
> pm_runtime_mark_last_busy(ddev->dev);
> pm_runtime_put_autosuspend(ddev->dev);
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 16e3f72d31b9..95add7afcc86 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -310,6 +310,9 @@ enum amdgpu_pcie_gen {
> #define amdgpu_dpm_print_clock_levels(adev, type, buf) \
> ((adev)->powerplay.pp_funcs->print_clock_levels((adev)-
> >powerplay.pp_handle, type, buf))
>
> +#define amdgpu_dpm_emit_clock_levels(adev, type, buf, offset) \
> + ((adev)->powerplay.pp_funcs->emit_clock_levels((adev)-
> >powerplay.pp_handle, type, buf, offset))
> +
> #define amdgpu_dpm_force_clock_level(adev, type, level) \
> ((adev)->powerplay.pp_funcs->force_clock_level((adev)-
> >powerplay.pp_handle, type, level))
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index f738f7dc20c9..dc1325783c10 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -620,6 +620,16 @@ struct pptable_funcs {
> */
> int (*print_clk_levels)(struct smu_context *smu, enum
> smu_clk_type clk_type, char *buf);
>
> + /**
> + * @emit_clk_levels: Print DPM clock levels for a clock domain
> + * to buffer using sysfs_emit_at. Star current level.
> + *
> + * Used for sysfs interfaces.
> + * &buf: sysfs buffer
> + * &offset: offset within buffer to start printing
> + */
> + int (*emit_clk_levels)(struct smu_context *smu, enum
> smu_clk_type clk_type, char *buf, int *offset);
> +
> /**
> * @force_clk_levels: Set a range of allowed DPM levels for a clock
> * domain.
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index e156add7b560..b0638d8308d2 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -2397,11 +2397,8 @@ static int smu_print_smuclk_levels(struct
> smu_context *smu, enum smu_clk_type cl
> return ret;
> }
>
> -static int smu_print_ppclk_levels(void *handle,
> - enum pp_clock_type type,
> - char *buf)
> +static enum smu_clk_type smu_convert_to_smuclk(enum pp_clock_type
> type)
> {
> - struct smu_context *smu = handle;
> enum smu_clk_type clk_type;
>
> switch (type) {
> @@ -2434,12 +2431,54 @@ static int smu_print_ppclk_levels(void *handle,
> case OD_CCLK:
> clk_type = SMU_OD_CCLK; break;
> default:
> - return -EINVAL;
> + clk_type = SMU_CLK_COUNT; break;
> }
>
> + return clk_type;
> +}
> +
> +static int smu_print_ppclk_levels(void *handle,
> + enum pp_clock_type type,
> + char *buf)
> +{
> + struct smu_context *smu = handle;
> + enum smu_clk_type clk_type;
> +
> + clk_type = smu_convert_to_smuclk(type);
> + if (clk_type == SMU_CLK_COUNT)
> + return -EINVAL;
> +
> return smu_print_smuclk_levels(smu, clk_type, buf);
> }
>
> +static int smu_emit_ppclk_levels(void *handle, enum pp_clock_type type,
> char *buf, int *offset)
> +{
> + struct smu_context *smu = handle;
> + enum smu_clk_type clk_type;
> + int ret = 0;
> +
> + clk_type = smu_convert_to_smuclk(type);
> + if (clk_type == SMU_CLK_COUNT)
> + return -EINVAL;
> +
> + if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&smu->mutex);
> +
> + if (smu->ppt_funcs->emit_clk_levels) {
> + ret = smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf,
> offset);
> + }
> + else {
> + ret = -EOPNOTSUPP;
> + }
> +
> + mutex_unlock(&smu->mutex);
> +
> + return ret;
> +
> +}
> +
> static int smu_od_edit_dpm_table(void *handle,
> enum PP_OD_DPM_TABLE_COMMAND
> type,
> long *input, uint32_t size)
> @@ -3117,6 +3156,7 @@ static const struct amd_pm_funcs
> swsmu_pm_funcs = {
> .get_fan_speed_pwm = smu_get_fan_speed_pwm,
> .force_clock_level = smu_force_ppclk_levels,
> .print_clock_levels = smu_print_ppclk_levels,
> + .emit_clock_levels = smu_emit_ppclk_levels,
> .force_performance_level = smu_force_performance_level,
> .read_sensor = smu_read_sensor,
> .get_performance_level = smu_get_performance_level,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 60a557068ea4..e43c7e950a55 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -1261,6 +1261,194 @@ static void navi10_od_setting_get_range(struct
> smu_11_0_overdrive_table *od_tabl
> *max = od_table->max[setting];
> }
>
> +static int navi10_emit_clk_levels(struct smu_context *smu,
> + enum smu_clk_type clk_type, char *buf, int *offset)
> +{
> + uint16_t *curve_settings;
> + int ret = 0;
> + uint32_t cur_value = 0, value = 0;
> + uint32_t freq_values[3] = {0};
> + uint32_t i, levels, mark_index = 0, count = 0;
> + struct smu_table_context *table_context = &smu->smu_table;
> + uint32_t gen_speed, lane_width;
> + struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
> + struct smu_11_0_dpm_context *dpm_context = smu_dpm-
> >dpm_context;
> + PPTable_t *pptable = (PPTable_t *)table_context->driver_pptable;
> + OverDriveTable_t *od_table =
> + (OverDriveTable_t *)table_context->overdrive_table;
> + struct smu_11_0_overdrive_table *od_settings = smu->od_settings;
> + uint32_t min_value, max_value;
> + int save_offset = *offset;
> +
> + switch (clk_type) {
> + case SMU_GFXCLK:
> + case SMU_SCLK:
> + case SMU_SOCCLK:
> + case SMU_MCLK:
> + case SMU_UCLK:
> + case SMU_FCLK:
> + case SMU_VCLK:
> + case SMU_DCLK:
> + case SMU_DCEFCLK:
> + ret = navi10_get_current_clk_freq_by_table(smu, clk_type,
> &cur_value);
> + if (ret)
> + return 0;
> +
> + ret = smu_v11_0_get_dpm_level_count(smu, clk_type,
> &count);
> + if (ret)
> + return 0;
> +
> + if (!navi10_is_support_fine_grained_dpm(smu, clk_type)) {
> + for (i = 0; i < count; i++) {
> + ret =
> smu_v11_0_get_dpm_freq_by_index(smu, clk_type, i, &value);
> + if (ret)
> + return (*offset - save_offset);
> +
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i, value,
> + cur_value == value ? "*" : "");
> +
> + }
> + } else {
> + ret = smu_v11_0_get_dpm_freq_by_index(smu,
> clk_type, 0, &freq_values[0]);
> + if (ret)
> + return 0;
> + ret = smu_v11_0_get_dpm_freq_by_index(smu,
> clk_type, count - 1, &freq_values[2]);
> + if (ret)
> + return 0;
> +
> + freq_values[1] = cur_value;
> + mark_index = cur_value == freq_values[0] ? 0 :
> + cur_value == freq_values[2] ? 2 : 1;
> +
> + levels = 3;
> + if (mark_index != 1) {
> + levels = 2;
> + freq_values[1] = freq_values[2];
> + }
> +
> + for (i = 0; i < levels; i++) {
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMhz %s\n", i, freq_values[i],
> + i == mark_index ? "*" : "");
> + }
> + }
> + 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);
> + for (i = 0; i < NUM_LINK_LEVELS; i++) {
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %s %s %dMhz %s\n", i,
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 0) ? "2.5GT/s," :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 1) ? "5.0GT/s," :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 2) ? "8.0GT/s," :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i] == 3) ? "16.0GT/s," : "",
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 1) ? "x1" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 2) ? "x2" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 3) ? "x4" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 4) ? "x8" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 5) ? "x12" :
> + (dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i] == 6) ? "x16" : "",
> + pptable->LclkFreq[i],
> + (gen_speed == dpm_context-
> >dpm_tables.pcie_table.pcie_gen[i]) &&
> + (lane_width == dpm_context-
> >dpm_tables.pcie_table.pcie_lane[i]) ?
> + "*" : "");
> + }
> + break;
> + case SMU_OD_SCLK:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + if (!navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_LIMITS))
> + break;
> + *offset += sysfs_emit_at(buf, *offset,
> "OD_SCLK:\n0: %uMhz\n1: %uMhz\n",
> + od_table->GfxclkFmin, od_table-
> >GfxclkFmax);
> + break;
> + case SMU_OD_MCLK:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + if (!navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_UCLK_MAX))
> + break;
> + *offset += sysfs_emit_at(buf, *offset,
> "OD_MCLK:\n1: %uMHz\n", od_table->UclkFmax);
> + break;
> + case SMU_OD_VDDC_CURVE:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + if (!navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_CURVE))
> + break;
> + *offset += sysfs_emit_at(buf, *offset,
> "OD_VDDC_CURVE:\n");
> + for (i = 0; i < 3; i++) {
> + switch (i) {
> + case 0:
> + curve_settings = &od_table->GfxclkFreq1;
> + break;
> + case 1:
> + curve_settings = &od_table->GfxclkFreq2;
> + break;
> + case 2:
> + curve_settings = &od_table->GfxclkFreq3;
> + break;
> + default:
> + break;
> + }
> + *offset += sysfs_emit_at(buf, *offset,
> "%d: %uMHz %umV\n",
> + i, curve_settings[0],
> + curve_settings[1] /
> NAVI10_VOLTAGE_SCALE);
> + }
> + break;
> + case SMU_OD_RANGE:
> + if (!smu->od_enabled || !od_table || !od_settings)
> + break;
> + *offset += sysfs_emit_at(buf, *offset, "%s:\n",
> "OD_RANGE");
> +
> + if (navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_LIMITS)) {
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_GFXCLKFMIN,
> + &min_value, NULL);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_GFXCLKFMAX,
> + NULL, &max_value);
> + *offset+= sysfs_emit_at(buf, *offset,
> "SCLK: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + }
> +
> + if (navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_UCLK_MAX)) {
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_UCLKFMAX,
> + &min_value, &max_value);
> + *offset+= sysfs_emit_at(buf, *offset,
> "MCLK: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + }
> +
> + if (navi10_od_feature_is_supported(od_settings,
> SMU_11_0_ODCAP_GFXCLK_CURVE)) {
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P1,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_SCLK[0]: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P1,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_VOLT[0]: %7dmV %11dmV\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P2,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_SCLK[1]: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P2,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_VOLT[1]: %7dmV %11dmV\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEFREQ_P3,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_SCLK[2]: %7uMhz %10uMhz\n",
> + min_value, max_value);
> + navi10_od_setting_get_range(od_settings,
> SMU_11_0_ODSETTING_VDDGFXCURVEVOLTAGE_P3,
> + &min_value, &max_value);
> + *offset += sysfs_emit_at(buf, *offset,
> "VDDC_CURVE_VOLT[2]: %7dmV %11dmV\n",
> + min_value, max_value);
> + }
> +
> + break;
> + default:
> + break;
> + }
> +
> + return (*offset - save_offset);
> +}
> +
> static int navi10_print_clk_levels(struct smu_context *smu,
> enum smu_clk_type clk_type, char *buf)
> {
> @@ -3245,6 +3433,7 @@ static const struct pptable_funcs navi10_ppt_funcs
> = {
> .i2c_init = navi10_i2c_control_init,
> .i2c_fini = navi10_i2c_control_fini,
> .print_clk_levels = navi10_print_clk_levels,
> + .emit_clk_levels = navi10_emit_clk_levels,
> .force_clk_levels = navi10_force_clk_levels,
> .populate_umd_state_clk = navi10_populate_umd_state_clk,
> .get_clock_by_type_with_latency =
> navi10_get_clock_by_type_with_latency,
> --
> 2.34.1
[-- Attachment #2: Type: text/html, Size: 55507 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-09 16:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08 6:36 [PATCH 0/3] amdgpu/pm: Implement parallel sysfs_emit solution for navi10 Darren Powell
2021-12-08 6:36 ` [PATCH 1/3] amdgpu/pm: Implement new API function "emit" that accepts buffer base and write offset Darren Powell
2021-12-09 1:48 ` Quan, Evan
2021-12-09 5:18 ` Powell, Darren
2021-12-08 6:36 ` [PATCH 2/3] amdgpu/pm: insert attempt to call emit_clock_levels into amdgpu_get_pp_od_clk_voltage Darren Powell
2021-12-08 6:36 ` [PATCH 3/3] amdgpu/pm: Add Error Handling to emit_clocks stack Darren Powell
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.