* [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API
@ 2021-05-28 23:06 Darren Powell
2021-05-28 23:06 ` [PATCH 1/6] amdgpu/pm: reorder definition of swsmu_pm_funcs for readability Darren Powell
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Darren Powell @ 2021-05-28 23:06 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
=== Description ===
modify smu_get_power_limit to implement Powerplay API
v2: rewrote the patchset to use two enums as args to get_power
=== Test System ===
* DESKTOP(AMD FX-8350 + NAVI10(731F/ca), BIOS: F2)
+ ISO(Ubuntu 20.04.1 LTS)
+ Kernel(5.11.0-custom-fdoagd5f)
=== Patch Summary ===
linux: (git@gitlab.freedesktop.org:agd5f) origin/amd-staging-drm-next @ 3ac16cf10525
+ 212a8ab5269d amdgpu/pm: reorder definition of swsmu_pm_funcs for readability
+ 50adb18c2670 amdgpu/pm: clean up smu_get_power_limit function signature
+ ab31cfcad254 amdgpu/pm: modify Powerplay API get_power_limit to use new pp_power enums
+ a5e2a4209a3c amdgpu/pm: modify and add smu_get_power_limit to Powerplay API
+ 6b732f665a9c amdgpu/pm: handle return value for get_power_limit
+ c1e3e0963996 amdgpu/pm: add kernel documentation for smu_get_power_limit
=== Tests ===
==== get_power_limit Test ====
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
==== Documentation Test ====
* Insert temp documentation
** Documentation/gpu/amdgpu.rst
vi Documentation/gpu/amdgpu.rst
** added text to start
------------START------------
Documentation Testing
=====================
Power Limit
-----------
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:doc: amdgpu_pp_power
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:identifiers: pp_power_limit_level
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:identifiers: pp_power_sample_window
.. kernel-doc:: drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
:identifiers: smu_get_power_limit
-------------END-------------
* Setup
cd ~/workspace/linux
. sphinx_2.4.4/bin/activate
* Build
export SPHINXDOCLOG=sphinx.build.log
cp $SPHINXDOCLOG{,.old}
time make -j 8 htmldocs |& tee $SPHINXDOCLOG
* View
firefox file:///home/dapowell/workspace/linux/Documentation/output/gpu/amdgpu.html
Darren Powell (6):
amdgpu/pm: reorder definition of swsmu_pm_funcs for readability
amdgpu/pm: clean up smu_get_power_limit function signature
amdgpu/pm: modify Powerplay API get_power_limit to use new pp_power
enums
amdgpu/pm: modify and add smu_get_power_limit to Powerplay API
amdgpu/pm: handle return value for get_power_limit
amdgpu/pm: add kernel documentation for smu_get_power_limit
.../gpu/drm/amd/include/kgd_pp_interface.h | 47 ++++++++-
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 64 +++++++------
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 5 +-
.../gpu/drm/amd/pm/powerplay/amd_powerplay.c | 33 ++++---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 96 +++++++++++++------
5 files changed, 172 insertions(+), 73 deletions(-)
base-commit: 3ac16cf105253e17c4e63d4216bd4012cd5b3145
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/6] amdgpu/pm: reorder definition of swsmu_pm_funcs for readability
2021-05-28 23:06 [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
@ 2021-05-28 23:06 ` Darren Powell
2021-05-28 23:06 ` [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature Darren Powell
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Darren Powell @ 2021-05-28 23:06 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
Match the order of definition to the structure's declaration to
help with locating included and missing functions of the API
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 48 +++++++++++------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 285849cef9f2..8aff67a667fa 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2962,6 +2962,8 @@ static const struct amd_pm_funcs swsmu_pm_funcs = {
.get_fan_control_mode = smu_get_fan_control_mode,
.set_fan_speed_percent = smu_set_fan_speed_percent,
.get_fan_speed_percent = smu_get_fan_speed_percent,
+ .force_clock_level = smu_force_ppclk_levels,
+ .print_clock_levels = smu_print_ppclk_levels,
.force_performance_level = smu_force_performance_level,
.read_sensor = smu_read_sensor,
.get_performance_level = smu_get_performance_level,
@@ -2974,38 +2976,36 @@ static const struct amd_pm_funcs swsmu_pm_funcs = {
.switch_power_profile = smu_switch_power_profile,
/* export to amdgpu */
.dispatch_tasks = smu_handle_dpm_task,
+ .load_firmware = smu_load_microcode,
.set_powergating_by_smu = smu_dpm_set_power_gate,
.set_power_limit = smu_set_power_limit,
+ .get_power_profile_mode = smu_get_power_profile_mode,
+ .set_power_profile_mode = smu_set_power_profile_mode,
.odn_edit_dpm_table = smu_od_edit_dpm_table,
.set_mp1_state = smu_set_mp1_state,
+ .gfx_state_change_set = smu_gfx_state_change_set,
/* export to DC */
- .get_sclk = smu_get_sclk,
- .get_mclk = smu_get_mclk,
- .enable_mgpu_fan_boost = smu_enable_mgpu_fan_boost,
- .get_asic_baco_capability = smu_get_baco_capability,
- .set_asic_baco_state = smu_baco_set_state,
- .get_ppfeature_status = smu_sys_get_pp_feature_mask,
- .set_ppfeature_status = smu_sys_set_pp_feature_mask,
- .asic_reset_mode_2 = smu_mode2_reset,
- .set_df_cstate = smu_set_df_cstate,
- .set_xgmi_pstate = smu_set_xgmi_pstate,
- .get_gpu_metrics = smu_sys_get_gpu_metrics,
- .set_power_profile_mode = smu_set_power_profile_mode,
- .get_power_profile_mode = smu_get_power_profile_mode,
- .force_clock_level = smu_force_ppclk_levels,
- .print_clock_levels = smu_print_ppclk_levels,
- .get_uclk_dpm_states = smu_get_uclk_dpm_states,
- .get_dpm_clock_table = smu_get_dpm_clock_table,
- .display_configuration_change = smu_display_configuration_change,
- .get_clock_by_type_with_latency = smu_get_clock_by_type_with_latency,
- .display_clock_voltage_request = smu_display_clock_voltage_request,
- .set_active_display_count = smu_set_display_count,
- .set_min_deep_sleep_dcefclk = smu_set_deep_sleep_dcefclk,
+ .get_sclk = smu_get_sclk,
+ .get_mclk = smu_get_mclk,
+ .display_configuration_change = smu_display_configuration_change,
+ .get_clock_by_type_with_latency = smu_get_clock_by_type_with_latency,
+ .display_clock_voltage_request = smu_display_clock_voltage_request,
+ .enable_mgpu_fan_boost = smu_enable_mgpu_fan_boost,
+ .set_active_display_count = smu_set_display_count,
+ .set_min_deep_sleep_dcefclk = smu_set_deep_sleep_dcefclk,
+ .get_asic_baco_capability = smu_get_baco_capability,
+ .set_asic_baco_state = smu_baco_set_state,
+ .get_ppfeature_status = smu_sys_get_pp_feature_mask,
+ .set_ppfeature_status = smu_sys_set_pp_feature_mask,
+ .asic_reset_mode_2 = smu_mode2_reset,
+ .set_df_cstate = smu_set_df_cstate,
+ .set_xgmi_pstate = smu_set_xgmi_pstate,
+ .get_gpu_metrics = smu_sys_get_gpu_metrics,
.set_watermarks_for_clock_ranges = smu_set_watermarks_for_clock_ranges,
.display_disable_memory_clock_switch = smu_display_disable_memory_clock_switch,
.get_max_sustainable_clocks_by_dc = smu_get_max_sustainable_clocks_by_dc,
- .load_firmware = smu_load_microcode,
- .gfx_state_change_set = smu_gfx_state_change_set,
+ .get_uclk_dpm_states = smu_get_uclk_dpm_states,
+ .get_dpm_clock_table = smu_get_dpm_clock_table,
.get_smu_prv_buf_details = smu_get_prv_buffer_details,
};
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-05-28 23:06 [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
2021-05-28 23:06 ` [PATCH 1/6] amdgpu/pm: reorder definition of swsmu_pm_funcs for readability Darren Powell
@ 2021-05-28 23:06 ` Darren Powell
2021-05-31 6:04 ` Lazar, Lijo
2021-05-28 23:06 ` [PATCH 3/6] amdgpu/pm: modify Powerplay API get_power_limit to use new pp_power enums Darren Powell
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Darren Powell @ 2021-05-28 23:06 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
add two new powerplay enums (limit_level, sample_window)
add enums to smu_get_power_limit signature
remove input bitfield stuffing of output variable limit
update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
{
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
{
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
{
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type {
#if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu);
bool smu_mode2_reset_is_support(struct smu_context *smu);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/6] amdgpu/pm: modify Powerplay API get_power_limit to use new pp_power enums
2021-05-28 23:06 [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
2021-05-28 23:06 ` [PATCH 1/6] amdgpu/pm: reorder definition of swsmu_pm_funcs for readability Darren Powell
2021-05-28 23:06 ` [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature Darren Powell
@ 2021-05-28 23:06 ` Darren Powell
2021-05-28 23:06 ` [PATCH 4/6] amdgpu/pm: modify and add smu_get_power_limit to Powerplay API Darren Powell
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Darren Powell @ 2021-05-28 23:06 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
updated {amd_pm_funcs}->get_power_limit() signature
rewrote pp_get_power_limit to use new enums
pp_get_power_limit now returns -EOPNOTSUPP for unknown power limit
update calls to {amd_pm_funcs}->get_power_limit()
* Test Notes
* testing hardware was NAVI10 (tests SMU path)
** needs testing on VANGOGH
** needs testing on SMU < 11
** ie, one of
TOPAZ, FIJI, TONGA, POLARIS10, POLARIS11, POLARIS12, VEGAM, CARRIZO,
STONEY, VEGA10, VEGA12,VEGA20, RAVEN, BONAIRE, HAWAII
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 5 +--
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 27 ++++++++-------
.../gpu/drm/amd/pm/powerplay/amd_powerplay.c | 33 ++++++++++++-------
3 files changed, 41 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index ddbf802ea8ad..369a72f03e92 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -305,8 +305,9 @@ struct amd_pm_funcs {
uint32_t block_type, bool gate);
int (*set_clockgating_by_smu)(void *handle, uint32_t msg_id);
int (*set_power_limit)(void *handle, uint32_t n);
- int (*get_power_limit)(void *handle, uint32_t *limit, uint32_t *max_limit,
- bool default_limit);
+ int (*get_power_limit)(void *handle, uint32_t *limit,
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
int (*get_power_profile_mode)(void *handle, char *buf);
int (*set_power_profile_mode)(void *handle, long *input, uint32_t size);
int (*set_fine_grain_clk_vol)(void *handle, uint32_t type, long *input, uint32_t size);
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index f7b45803431d..0098c8b55bb4 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2718,8 +2718,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ enum pp_power_limit_level pp_limit_level = PP_PWR_LIMIT_MAX;
uint32_t limit;
- uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,12 +2735,13 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX, sample_window);
+ smu_get_power_limit(&adev->smu, &limit,
+ pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
- pp_funcs->get_power_limit(adev->powerplay.pp_handle,
- &limit, &max_limit, true);
- size = snprintf(buf, PAGE_SIZE, "%u\n", max_limit * 1000000);
+ pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
+ pp_limit_level, sample_window);
+ size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else {
size = snprintf(buf, PAGE_SIZE, "\n");
}
@@ -2758,6 +2759,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ enum pp_power_limit_level pp_limit_level = PP_PWR_LIMIT_CURRENT;
uint32_t limit;
ssize_t size;
int r;
@@ -2774,11 +2776,12 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT, sample_window);
+ smu_get_power_limit(&adev->smu, &limit,
+ pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
- pp_funcs->get_power_limit(adev->powerplay.pp_handle,
- &limit, NULL, false);
+ pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
+ pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else {
size = snprintf(buf, PAGE_SIZE, "\n");
@@ -2797,6 +2800,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ enum pp_power_limit_level pp_limit_level = PP_PWR_LIMIT_DEFAULT;
uint32_t limit;
ssize_t size;
int r;
@@ -2813,11 +2817,12 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT, sample_window);
+ smu_get_power_limit(&adev->smu, &limit,
+ pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
- pp_funcs->get_power_limit(adev->powerplay.pp_handle,
- &limit, NULL, true);
+ pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
+ pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else {
size = snprintf(buf, PAGE_SIZE, "\n");
diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index c73504e998e5..7ef7d2db3629 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -1035,31 +1035,42 @@ static int pp_set_power_limit(void *handle, uint32_t limit)
}
static int pp_get_power_limit(void *handle, uint32_t *limit,
- uint32_t *max_limit, bool default_limit)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
struct pp_hwmgr *hwmgr = handle;
+ int ret = 0;
if (!hwmgr || !hwmgr->pm_en ||!limit)
return -EINVAL;
+ if (sample_window != PP_PWR_WINDOW_DEFAULT)
+ return -EOPNOTSUPP;
+
mutex_lock(&hwmgr->smu_lock);
- if (default_limit) {
- *limit = hwmgr->default_power_limit;
- if (max_limit) {
- *max_limit = *limit;
+ switch (pp_limit_level) {
+ case PP_PWR_LIMIT_CURRENT:
+ *limit = hwmgr->power_limit;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ *limit = hwmgr->default_power_limit;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ *limit = hwmgr->default_power_limit;
if (hwmgr->od_enabled) {
- *max_limit *= (100 + hwmgr->platform_descriptor.TDPODLimit);
- *max_limit /= 100;
+ *limit *= (100 + hwmgr->platform_descriptor.TDPODLimit);
+ *limit /= 100;
}
- }
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ break;
}
- else
- *limit = hwmgr->power_limit;
mutex_unlock(&hwmgr->smu_lock);
- return 0;
+ return ret;
}
static int pp_display_configuration_change(void *handle,
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/6] amdgpu/pm: modify and add smu_get_power_limit to Powerplay API
2021-05-28 23:06 [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
` (2 preceding siblings ...)
2021-05-28 23:06 ` [PATCH 3/6] amdgpu/pm: modify Powerplay API get_power_limit to use new pp_power enums Darren Powell
@ 2021-05-28 23:06 ` Darren Powell
2021-05-28 23:06 ` [PATCH 5/6] amdgpu/pm: handle return value for get_power_limit Darren Powell
2021-05-28 23:06 ` [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit Darren Powell
5 siblings, 0 replies; 16+ messages in thread
From: Darren Powell @ 2021-05-28 23:06 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
modify args of smu_get_power_limit to match Powerplay API .get_power_limit
add smu_get_power_limit to Powerplay API swsmu_pm_funcs
remove special handling of smu in amdgpu_hwmon_show_power_cap*
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++---------------
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 2 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 4 +++-
3 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 0098c8b55bb4..44715848705a 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2734,11 +2734,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
return r;
}
- if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit,
- pp_limit_level, sample_window);
- size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
- } else if (pp_funcs && pp_funcs->get_power_limit) {
+ if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
@@ -2775,11 +2771,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
return r;
}
- if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit,
- pp_limit_level, sample_window);
- size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
- } else if (pp_funcs && pp_funcs->get_power_limit) {
+ if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
@@ -2816,11 +2808,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
return r;
}
- if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit,
- pp_limit_level, sample_window);
- size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
- } else if (pp_funcs && pp_funcs->get_power_limit) {
+ if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
pp_limit_level, sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index b97b960c2eac..9636a023387f 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1260,7 +1260,7 @@ enum smu_cmn2asic_mapping_type {
[profile] = {1, (workload)}
#if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4)
-int smu_get_power_limit(struct smu_context *smu,
+int smu_get_power_limit(void *handle,
uint32_t *limit,
enum pp_power_limit_level pp_limit_level,
enum pp_power_sample_window sample_window);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 44c1baa2748d..5671abd58bcf 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2166,11 +2166,12 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
return ret;
}
-int smu_get_power_limit(struct smu_context *smu,
+int smu_get_power_limit(void *handle,
uint32_t *limit,
enum pp_power_limit_level pp_limit_level,
enum pp_power_sample_window sample_window)
{
+ struct smu_context *smu = handle;
enum smu_ppt_limit_level limit_level;
uint32_t limit_type;
int ret = 0;
@@ -3009,6 +3010,7 @@ static const struct amd_pm_funcs swsmu_pm_funcs = {
.load_firmware = smu_load_microcode,
.set_powergating_by_smu = smu_dpm_set_power_gate,
.set_power_limit = smu_set_power_limit,
+ .get_power_limit = smu_get_power_limit,
.get_power_profile_mode = smu_get_power_profile_mode,
.set_power_profile_mode = smu_set_power_profile_mode,
.odn_edit_dpm_table = smu_od_edit_dpm_table,
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/6] amdgpu/pm: handle return value for get_power_limit
2021-05-28 23:06 [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
` (3 preceding siblings ...)
2021-05-28 23:06 ` [PATCH 4/6] amdgpu/pm: modify and add smu_get_power_limit to Powerplay API Darren Powell
@ 2021-05-28 23:06 ` Darren Powell
2021-05-28 23:06 ` [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit Darren Powell
5 siblings, 0 replies; 16+ messages in thread
From: Darren Powell @ 2021-05-28 23:06 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 39 ++++++++++++++++++------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 44715848705a..aa138abe6e1d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2734,13 +2734,16 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
return r;
}
- if (pp_funcs && pp_funcs->get_power_limit) {
- pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
- pp_limit_level, sample_window);
+ if (pp_funcs && pp_funcs->get_power_limit)
+ r = pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
+ pp_limit_level, sample_window);
+ else
+ r = -ENODATA;
+
+ if (!r)
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
- } else {
+ else
size = snprintf(buf, PAGE_SIZE, "\n");
- }
pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -2771,13 +2774,16 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
return r;
}
- if (pp_funcs && pp_funcs->get_power_limit) {
- pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
- pp_limit_level, sample_window);
+ if (pp_funcs && pp_funcs->get_power_limit)
+ r = pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
+ pp_limit_level, sample_window);
+ else
+ r = -ENODATA;
+
+ if (!r)
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
- } else {
+ else
size = snprintf(buf, PAGE_SIZE, "\n");
- }
pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -2808,13 +2814,16 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
return r;
}
- if (pp_funcs && pp_funcs->get_power_limit) {
- pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
- pp_limit_level, sample_window);
+ if (pp_funcs && pp_funcs->get_power_limit)
+ r = pp_funcs->get_power_limit(adev->powerplay.pp_handle, &limit,
+ pp_limit_level, sample_window);
+ else
+ r = -ENODATA;
+
+ if (!r)
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
- } else {
+ else
size = snprintf(buf, PAGE_SIZE, "\n");
- }
pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit
2021-05-28 23:06 [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
` (4 preceding siblings ...)
2021-05-28 23:06 ` [PATCH 5/6] amdgpu/pm: handle return value for get_power_limit Darren Powell
@ 2021-05-28 23:06 ` Darren Powell
2021-06-01 8:12 ` Quan, Evan
5 siblings, 1 reply; 16+ messages in thread
From: Darren Powell @ 2021-05-28 23:06 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
added doc tag "amdgpu_pp_power" with description
added tags for enums pp_power_limit_level, pp_power_sample_window
added tag for function smu_get_power_limit
Test:
* Temporary insertion into Documentation/gpu/amdgpu.rst
------------START------------
Power Limit
-----------
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:doc: amdgpu_pp_power
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:identifiers: pp_power_limit_level
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:identifiers: pp_power_sample_window
.. kernel-doc:: drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
:identifiers: smu_get_power_limit
-------------END-------------
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 30 ++++++++++++++++++-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 10 +++++++
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 369a72f03e92..46d2fc434e24 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,26 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+/**
+ * DOC: amdgpu_pp_power
+ *
+ * APU power is managed to system-level requirements through the PPT
+ * (package power tracking) feature. PPT is intended to limit power to the
+ * requirements of the power source and could be dynamically updated to
+ * maximize APU performance within the system power budget.
+ *
+ * Two windows of power measurement can be requested, where supported, with
+ * :c:type:`enum pp_power_sample_window <pp_power_sample_window>`.
+ */
+
+/**
+ * enum pp_power_limit_level - Used to query the power limits
+ *
+ * @PP_PWR_LIMIT_MIN: Minimum Power Limit
+ * @PP_PWR_LIMIT_CURRENT: Current Power Limit
+ * @PP_PWR_LIMIT_DEFAULT: Default Power Limit
+ * @PP_PWR_LIMIT_MAX: Maximum Power Limit
+ */
enum pp_power_limit_level
{
PP_PWR_LIMIT_MIN = -1,
@@ -200,7 +220,15 @@ enum pp_power_limit_level
PP_PWR_LIMIT_MAX,
};
- enum pp_power_sample_window
+/**
+ * enum pp_power_sample_window - Used to specify the window size of the requested power
+ *
+ * @PP_PWR_WINDOW_DEFAULT: manages the configurable, thermally significant
+ * moving average of APU power (default ~5000 ms).
+ * @PP_PWR_WINDOW_FAST: manages the ~10 ms moving average of APU power,
+ * where supported.
+ */
+enum pp_power_sample_window
{
PP_PWR_WINDOW_DEFAULT,
PP_PWR_WINDOW_FAST,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 5671abd58bcf..b7a9037a2dbc 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2166,6 +2166,16 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
return ret;
}
+/**
+ * smu_get_power_limit - Request one of the SMU Power Limits
+ *
+ * @handle: pointer to smu context
+ * @limit: requested limit is written back to this variable
+ * @pp_limit_level: &pp_power_limit_level which power limit to return
+ * @sample_window: &pp_power_sample_window measurement window
+ * Return: 0 on success, <0 on error
+ *
+ */
int smu_get_power_limit(void *handle,
uint32_t *limit,
enum pp_power_limit_level pp_limit_level,
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-05-28 23:06 ` [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature Darren Powell
@ 2021-05-31 6:04 ` Lazar, Lijo
2021-05-31 23:20 ` Powell, Darren
0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-05-31 6:04 UTC (permalink / raw)
To: Powell, Darren, amd-gfx
[Public]
-----Original Message-----
From: Powell, Darren <Darren.Powell@amd.com>
Sent: Saturday, May 29, 2021 4:36 AM
To: amd-gfx@lists.freedesktop.org
Cc: Powell, Darren <Darren.Powell@amd.com>
Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
add two new powerplay enums (limit_level, sample_window) add enums to smu_get_power_limit signature remove input bitfield stuffing of output variable limit update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1` AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10` HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
Thanks,
Lijo
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type { #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu); bool smu_mode2_reset_is_support(struct smu_context *smu); diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-05-31 6:04 ` Lazar, Lijo
@ 2021-05-31 23:20 ` Powell, Darren
2021-06-01 4:56 ` Lazar, Lijo
0 siblings, 1 reply; 16+ messages in thread
From: Powell, Darren @ 2021-05-31 23:20 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx
[-- Attachment #1.1: Type: text/plain, Size: 8939 bytes --]
[Public]
>< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
>It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
I think you mean something more like this?
+ enum pp_power_constraints
+{
+ PP_PWR_CONSTRAINT_DEFAULT,
+ PP_PWR_CONSTRAINT_FASTWINDOW,
+};
+
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Monday, May 31, 2021 2:04 AM
To: Powell, Darren <Darren.Powell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
-----Original Message-----
From: Powell, Darren <Darren.Powell@amd.com>
Sent: Saturday, May 29, 2021 4:36 AM
To: amd-gfx@lists.freedesktop.org
Cc: Powell, Darren <Darren.Powell@amd.com>
Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
add two new powerplay enums (limit_level, sample_window) add enums to smu_get_power_limit signature remove input bitfield stuffing of output variable limit update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1` AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10` HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
Thanks,
Lijo
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type { #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu); bool smu_mode2_reset_is_support(struct smu_context *smu); diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
[-- Attachment #1.2: Type: text/html, Size: 20763 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-05-31 23:20 ` Powell, Darren
@ 2021-06-01 4:56 ` Lazar, Lijo
2021-06-02 3:55 ` Powell, Darren
0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-06-01 4:56 UTC (permalink / raw)
To: Powell, Darren, amd-gfx
[-- Attachment #1.1: Type: text/plain, Size: 9629 bytes --]
[Public]
May be just call it power_limit or power_cap similar to hwmon. The various limits correspond to hwmon power[1-*]_cap and levels correspond to min/ max etc.
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com>
Sent: Tuesday, June 1, 2021 4:50 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
>< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
>It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
I think you mean something more like this?
+ enum pp_power_constraints
+{
+ PP_PWR_CONSTRAINT_DEFAULT,
+ PP_PWR_CONSTRAINT_FASTWINDOW,
+};
+
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Monday, May 31, 2021 2:04 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
-----Original Message-----
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Saturday, May 29, 2021 4:36 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
add two new powerplay enums (limit_level, sample_window) add enums to smu_get_power_limit signature remove input bitfield stuffing of output variable limit update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1` AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10` HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com<mailto:darren.powell@amd.com>>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
Thanks,
Lijo
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type { #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu); bool smu_mode2_reset_is_support(struct smu_context *smu); diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
[-- Attachment #1.2: Type: text/html, Size: 20772 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit
2021-05-28 23:06 ` [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit Darren Powell
@ 2021-06-01 8:12 ` Quan, Evan
0 siblings, 0 replies; 16+ messages in thread
From: Quan, Evan @ 2021-06-01 8:12 UTC (permalink / raw)
To: Powell, Darren, amd-gfx; +Cc: Powell, Darren
[AMD Official Use Only]
Series is reviewed-by: Evan Quan <evan.quan@amd.com>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Darren Powell
> Sent: Saturday, May 29, 2021 7:06 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Powell, Darren <Darren.Powell@amd.com>
> Subject: [PATCH 6/6] amdgpu/pm: add kernel documentation for
> smu_get_power_limit
>
> added doc tag "amdgpu_pp_power" with description
> added tags for enums pp_power_limit_level, pp_power_sample_window
> added tag for function smu_get_power_limit
>
> Test:
> * Temporary insertion into Documentation/gpu/amdgpu.rst
> ------------START------------
> Power Limit
> -----------
> .. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
> :doc: amdgpu_pp_power
>
> .. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
> :identifiers: pp_power_limit_level
>
> .. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
> :identifiers: pp_power_sample_window
>
> .. kernel-doc:: drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> :identifiers: smu_get_power_limit
> -------------END-------------
>
> Signed-off-by: Darren Powell <darren.powell@amd.com>
> ---
> .../gpu/drm/amd/include/kgd_pp_interface.h | 30
> ++++++++++++++++++-
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 10 +++++++
> 2 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> index 369a72f03e92..46d2fc434e24 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -192,6 +192,26 @@ enum pp_df_cstate {
> DF_CSTATE_ALLOW,
> };
>
> +/**
> + * DOC: amdgpu_pp_power
> + *
> + * APU power is managed to system-level requirements through the PPT
> + * (package power tracking) feature. PPT is intended to limit power to the
> + * requirements of the power source and could be dynamically updated to
> + * maximize APU performance within the system power budget.
> + *
> + * Two windows of power measurement can be requested, where
> supported, with
> + * :c:type:`enum pp_power_sample_window
> <pp_power_sample_window>`.
> + */
> +
> +/**
> + * enum pp_power_limit_level - Used to query the power limits
> + *
> + * @PP_PWR_LIMIT_MIN: Minimum Power Limit
> + * @PP_PWR_LIMIT_CURRENT: Current Power Limit
> + * @PP_PWR_LIMIT_DEFAULT: Default Power Limit
> + * @PP_PWR_LIMIT_MAX: Maximum Power Limit
> + */
> enum pp_power_limit_level
> {
> PP_PWR_LIMIT_MIN = -1,
> @@ -200,7 +220,15 @@ enum pp_power_limit_level
> PP_PWR_LIMIT_MAX,
> };
>
> - enum pp_power_sample_window
> +/**
> + * enum pp_power_sample_window - Used to specify the window size of
> the requested power
> + *
> + * @PP_PWR_WINDOW_DEFAULT: manages the configurable, thermally
> significant
> + * moving average of APU power (default ~5000 ms).
> + * @PP_PWR_WINDOW_FAST: manages the ~10 ms moving average of
> APU power,
> + * where supported.
> + */
> +enum pp_power_sample_window
> {
> PP_PWR_WINDOW_DEFAULT,
> PP_PWR_WINDOW_FAST,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 5671abd58bcf..b7a9037a2dbc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -2166,6 +2166,16 @@ static int smu_set_fan_speed_rpm(void *handle,
> uint32_t speed)
> return ret;
> }
>
> +/**
> + * smu_get_power_limit - Request one of the SMU Power Limits
> + *
> + * @handle: pointer to smu context
> + * @limit: requested limit is written back to this variable
> + * @pp_limit_level: &pp_power_limit_level which power limit to return
> + * @sample_window: &pp_power_sample_window measurement window
> + * Return: 0 on success, <0 on error
> + *
> + */
> int smu_get_power_limit(void *handle,
> uint32_t *limit,
> enum pp_power_limit_level pp_limit_level,
> --
> 2.25.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&data=04%7C01%7Cevan.quan%40amd.com%7C171d1673dbaf4dad5
> dbd08d9222d473c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
> 37578400133814296%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=
> 1L62QveKX%2B2UjfHPF9TUE8BREmMYbJkHUpQPlzmi324%3D&reserved
> =0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-06-01 4:56 ` Lazar, Lijo
@ 2021-06-02 3:55 ` Powell, Darren
2021-06-02 4:32 ` Lazar, Lijo
0 siblings, 1 reply; 16+ messages in thread
From: Powell, Darren @ 2021-06-02 3:55 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx
[-- Attachment #1.1: Type: text/plain, Size: 12245 bytes --]
[Public]
I'm not sure exactly what you are looking for. The enums sample_window and limit_level map to power{1,2} and cap{min,max,default,current} respectively. I added the enums to make the function signatures more readable and stop the use of value as an input and output variable.
Please give more specific example?
Darren
= HWMON Mapping =
== Read ==
SENSOR(power1_average) amdgpu_hwmon_show_power_avg(0) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power1_cap_max) amdgpu_hwmon_show_power_cap_max(0) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_min) amdgpu_hwmon_show_power_cap_min(0) 0
SENSOR(power1_cap) amdgpu_hwmon_show_power_cap(0) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_default) amdgpu_hwmon_show_power_cap_default(0) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_label) amdgpu_hwmon_show_power_label(0) "slowPPT"
SENSOR(power2_average) amdgpu_hwmon_show_power_avg(1) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power2_cap_max) amdgpu_hwmon_show_power_cap_max(1) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_min) amdgpu_hwmon_show_power_cap_min(1) 0
SENSOR(power2_cap) amdgpu_hwmon_show_power_cap(1) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_default) amdgpu_hwmon_show_power_cap_default(1) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_FAST)
SENSOR(power2_label) amdgpu_hwmon_show_power_label(1) "fastPPT"
== Write ==
SENSOR(power1_cap) amdgpu_hwmon_set_power_cap(0,value) set_power_limit( (0<<24) || value )
SENSOR(power2_cap) amdgpu_hwmon_set_power_cap(1,value) set_power_limit( (1<<24) || value )
= Summary =
power1 => PP_PWR_WINDOW_DEFAULT ( label ("slowPPT"))
power2 => PP_PWR_WINDOW_FAST ( label ("fastPPT"))
power_avg => AMDGPU_PP_SENSOR_GPU_POWER
power_cap_max => PP_PWR_LIMIT_MAX
power_cap_min => PP_PWR_LIMIT_MIN (optimized to 0)
power_cap => PP_PWR_LIMIT_CURRENT
power_cap_default => PP_PWR_LIMIT_DEFAULT
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Tuesday, June 1, 2021 12:56 AM
To: Powell, Darren <Darren.Powell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
May be just call it power_limit or power_cap similar to hwmon. The various limits correspond to hwmon power[1-*]_cap and levels correspond to min/ max etc.
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com>
Sent: Tuesday, June 1, 2021 4:50 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
>< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
>It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
I think you mean something more like this?
+ enum pp_power_constraints
+{
+ PP_PWR_CONSTRAINT_DEFAULT,
+ PP_PWR_CONSTRAINT_FASTWINDOW,
+};
+
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Monday, May 31, 2021 2:04 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
-----Original Message-----
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Saturday, May 29, 2021 4:36 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
add two new powerplay enums (limit_level, sample_window) add enums to smu_get_power_limit signature remove input bitfield stuffing of output variable limit update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1` AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10` HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com<mailto:darren.powell@amd.com>>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
Thanks,
Lijo
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type { #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu); bool smu_mode2_reset_is_support(struct smu_context *smu); diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
[-- Attachment #1.2: Type: text/html, Size: 24868 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-06-02 3:55 ` Powell, Darren
@ 2021-06-02 4:32 ` Lazar, Lijo
2021-06-03 2:47 ` Powell, Darren
0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-06-02 4:32 UTC (permalink / raw)
To: Powell, Darren, amd-gfx
[-- Attachment #1.1: Type: text/plain, Size: 13372 bytes --]
[Public]
I'm looking for an appropriate name that can accommodate more limits like
enum pp_power_limit
{
PP_PWR_LIMIT_DEFAULT,
PP_PWR_LIMIT_FAST,
PP_PWR_LIMIT_APU,
PP_PWR_LIMIT_PLATFORM,
};
Or simply different limits where the meaning may change based on ASIC - LIMIT2 could be platform power limit for ASIC1 or LIMIT2 could be max power allocation for a domain (say memory) for ASIC2
enum pp_power_limit
{
PP_PWR_LIMIT_0,
PP_PWR_LIMIT_1,
PP_PWR_LIMIT_2,
PP_PWR_LIMIT_3,
};
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com>
Sent: Wednesday, June 2, 2021 9:25 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
I'm not sure exactly what you are looking for. The enums sample_window and limit_level map to power{1,2} and cap{min,max,default,current} respectively. I added the enums to make the function signatures more readable and stop the use of value as an input and output variable.
Please give more specific example?
Darren
= HWMON Mapping =
== Read ==
SENSOR(power1_average) amdgpu_hwmon_show_power_avg(0) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power1_cap_max) amdgpu_hwmon_show_power_cap_max(0) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_min) amdgpu_hwmon_show_power_cap_min(0) 0
SENSOR(power1_cap) amdgpu_hwmon_show_power_cap(0) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_default) amdgpu_hwmon_show_power_cap_default(0) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_label) amdgpu_hwmon_show_power_label(0) "slowPPT"
SENSOR(power2_average) amdgpu_hwmon_show_power_avg(1) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power2_cap_max) amdgpu_hwmon_show_power_cap_max(1) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_min) amdgpu_hwmon_show_power_cap_min(1) 0
SENSOR(power2_cap) amdgpu_hwmon_show_power_cap(1) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_default) amdgpu_hwmon_show_power_cap_default(1) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_FAST)
SENSOR(power2_label) amdgpu_hwmon_show_power_label(1) "fastPPT"
== Write ==
SENSOR(power1_cap) amdgpu_hwmon_set_power_cap(0,value) set_power_limit( (0<<24) || value )
SENSOR(power2_cap) amdgpu_hwmon_set_power_cap(1,value) set_power_limit( (1<<24) || value )
= Summary =
power1 => PP_PWR_WINDOW_DEFAULT ( label ("slowPPT"))
power2 => PP_PWR_WINDOW_FAST ( label ("fastPPT"))
power_avg => AMDGPU_PP_SENSOR_GPU_POWER
power_cap_max => PP_PWR_LIMIT_MAX
power_cap_min => PP_PWR_LIMIT_MIN (optimized to 0)
power_cap => PP_PWR_LIMIT_CURRENT
power_cap_default => PP_PWR_LIMIT_DEFAULT
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Tuesday, June 1, 2021 12:56 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
May be just call it power_limit or power_cap similar to hwmon. The various limits correspond to hwmon power[1-*]_cap and levels correspond to min/ max etc.
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Tuesday, June 1, 2021 4:50 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
>< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
>It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
I think you mean something more like this?
+ enum pp_power_constraints
+{
+ PP_PWR_CONSTRAINT_DEFAULT,
+ PP_PWR_CONSTRAINT_FASTWINDOW,
+};
+
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Monday, May 31, 2021 2:04 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
-----Original Message-----
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Saturday, May 29, 2021 4:36 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
add two new powerplay enums (limit_level, sample_window) add enums to smu_get_power_limit signature remove input bitfield stuffing of output variable limit update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1` AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10` HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com<mailto:darren.powell@amd.com>>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
Thanks,
Lijo
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type { #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu); bool smu_mode2_reset_is_support(struct smu_context *smu); diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
[-- Attachment #1.2: Type: text/html, Size: 32324 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-06-02 4:32 ` Lazar, Lijo
@ 2021-06-03 2:47 ` Powell, Darren
2021-06-03 4:23 ` Lazar, Lijo
0 siblings, 1 reply; 16+ messages in thread
From: Powell, Darren @ 2021-06-03 2:47 UTC (permalink / raw)
To: Lazar, Lijo, amd-gfx; +Cc: Quan, Evan
[-- Attachment #1.1: Type: text/plain, Size: 14499 bytes --]
[Public]
I think both of those candidates have drawbacks
** option #1 has a clash with the limit_level
eg.
enum pp_power_limit_level limit_level;
enum pp_power_limit power_limit;
power_limit = PP_PWR_LIMIT_DEFAULT; // name clash
** option #2 doesn't describe the usage, and might as well just be a uint
power_limit = PP_PWR_LIMIT_0; // no use in reading code later
Would you accept "category" as a suitable solution?
eg.
enum pp_power_category
{
PP_PWR_CATEGORY_DEFAULT,
PP_PWR_CATEGORY_FAST,
PP_PWR_CATEGORY_APU,
PP_PWR_CATEGORY_PLATFORM,
};
Other possibilities may be
** class
** subsystem
** type
** element
Thanks
Darren
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Wednesday, June 2, 2021 12:32 AM
To: Powell, Darren <Darren.Powell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
I’m looking for an appropriate name that can accommodate more limits like
enum pp_power_limit
{
PP_PWR_LIMIT_DEFAULT,
PP_PWR_LIMIT_FAST,
PP_PWR_LIMIT_APU,
PP_PWR_LIMIT_PLATFORM,
};
Or simply different limits where the meaning may change based on ASIC - LIMIT2 could be platform power limit for ASIC1 or LIMIT2 could be max power allocation for a domain (say memory) for ASIC2
enum pp_power_limit
{
PP_PWR_LIMIT_0,
PP_PWR_LIMIT_1,
PP_PWR_LIMIT_2,
PP_PWR_LIMIT_3,
};
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com>
Sent: Wednesday, June 2, 2021 9:25 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
I'm not sure exactly what you are looking for. The enums sample_window and limit_level map to power{1,2} and cap{min,max,default,current} respectively. I added the enums to make the function signatures more readable and stop the use of value as an input and output variable.
Please give more specific example?
Darren
= HWMON Mapping =
== Read ==
SENSOR(power1_average) amdgpu_hwmon_show_power_avg(0) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power1_cap_max) amdgpu_hwmon_show_power_cap_max(0) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_min) amdgpu_hwmon_show_power_cap_min(0) 0
SENSOR(power1_cap) amdgpu_hwmon_show_power_cap(0) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_default) amdgpu_hwmon_show_power_cap_default(0) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_label) amdgpu_hwmon_show_power_label(0) "slowPPT"
SENSOR(power2_average) amdgpu_hwmon_show_power_avg(1) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power2_cap_max) amdgpu_hwmon_show_power_cap_max(1) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_min) amdgpu_hwmon_show_power_cap_min(1) 0
SENSOR(power2_cap) amdgpu_hwmon_show_power_cap(1) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_default) amdgpu_hwmon_show_power_cap_default(1) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_FAST)
SENSOR(power2_label) amdgpu_hwmon_show_power_label(1) "fastPPT"
== Write ==
SENSOR(power1_cap) amdgpu_hwmon_set_power_cap(0,value) set_power_limit( (0<<24) || value )
SENSOR(power2_cap) amdgpu_hwmon_set_power_cap(1,value) set_power_limit( (1<<24) || value )
= Summary =
power1 => PP_PWR_WINDOW_DEFAULT ( label ("slowPPT"))
power2 => PP_PWR_WINDOW_FAST ( label ("fastPPT"))
power_avg => AMDGPU_PP_SENSOR_GPU_POWER
power_cap_max => PP_PWR_LIMIT_MAX
power_cap_min => PP_PWR_LIMIT_MIN (optimized to 0)
power_cap => PP_PWR_LIMIT_CURRENT
power_cap_default => PP_PWR_LIMIT_DEFAULT
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Tuesday, June 1, 2021 12:56 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
May be just call it power_limit or power_cap similar to hwmon. The various limits correspond to hwmon power[1-*]_cap and levels correspond to min/ max etc.
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Tuesday, June 1, 2021 4:50 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
>< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
>It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
I think you mean something more like this?
+ enum pp_power_constraints
+{
+ PP_PWR_CONSTRAINT_DEFAULT,
+ PP_PWR_CONSTRAINT_FASTWINDOW,
+};
+
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Monday, May 31, 2021 2:04 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
-----Original Message-----
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Saturday, May 29, 2021 4:36 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
add two new powerplay enums (limit_level, sample_window) add enums to smu_get_power_limit signature remove input bitfield stuffing of output variable limit update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1` AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10` HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com<mailto:darren.powell@amd.com>>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
Thanks,
Lijo
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type { #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu); bool smu_mode2_reset_is_support(struct smu_context *smu); diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
[-- Attachment #1.2: Type: text/html, Size: 32394 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
2021-06-03 2:47 ` Powell, Darren
@ 2021-06-03 4:23 ` Lazar, Lijo
0 siblings, 0 replies; 16+ messages in thread
From: Lazar, Lijo @ 2021-06-03 4:23 UTC (permalink / raw)
To: Powell, Darren, amd-gfx; +Cc: Quan, Evan
[-- Attachment #1.1: Type: text/plain, Size: 15853 bytes --]
[Public]
I guess, you are suggesting to have levels to indicate limits/average and types/class to indicate different levels. I'm fine with that.
enum pp_power_level
{
PP_PWR_LIMIT_MIN = -1,
PP_PWR_LIMIT_CURRENT,
PP_PWR_LIMIT_DEFAULT,
PP_PWR_LIMIT_MAX,
};
enum pp_power_type
{
PP_PWR_TYPE_SUSTAINED, => Sustained Power is the default in ASICs
PP_PWR_TYPE_FAST,
PP_PWR_TYPE_PLATFORM,
PP_PWR_TYPE_APU,
};
Do note that for some ASICs FW uses power types like below (indicated in throttler bits) which we won't be able to explain with meaningful names.
#define THROTTLER_PPT0_BIT 13
#define THROTTLER_PPT1_BIT 14
#define THROTTLER_PPT2_BIT 15
#define THROTTLER_PPT3_BIT 16
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com>
Sent: Thursday, June 3, 2021 8:18 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Quan, Evan <Evan.Quan@amd.com>
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
I think both of those candidates have drawbacks
** option #1 has a clash with the limit_level
eg.
enum pp_power_limit_level limit_level;
enum pp_power_limit power_limit;
power_limit = PP_PWR_LIMIT_DEFAULT; // name clash
** option #2 doesn't describe the usage, and might as well just be a uint
power_limit = PP_PWR_LIMIT_0; // no use in reading code later
Would you accept "category" as a suitable solution?
eg.
enum pp_power_category
{
PP_PWR_CATEGORY_DEFAULT,
PP_PWR_CATEGORY_FAST,
PP_PWR_CATEGORY_APU,
PP_PWR_CATEGORY_PLATFORM,
};
Other possibilities may be
** class
** subsystem
** type
** element
Thanks
Darren
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Wednesday, June 2, 2021 12:32 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
I'm looking for an appropriate name that can accommodate more limits like
enum pp_power_limit
{
PP_PWR_LIMIT_DEFAULT,
PP_PWR_LIMIT_FAST,
PP_PWR_LIMIT_APU,
PP_PWR_LIMIT_PLATFORM,
};
Or simply different limits where the meaning may change based on ASIC - LIMIT2 could be platform power limit for ASIC1 or LIMIT2 could be max power allocation for a domain (say memory) for ASIC2
enum pp_power_limit
{
PP_PWR_LIMIT_0,
PP_PWR_LIMIT_1,
PP_PWR_LIMIT_2,
PP_PWR_LIMIT_3,
};
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Wednesday, June 2, 2021 9:25 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
I'm not sure exactly what you are looking for. The enums sample_window and limit_level map to power{1,2} and cap{min,max,default,current} respectively. I added the enums to make the function signatures more readable and stop the use of value as an input and output variable.
Please give more specific example?
Darren
= HWMON Mapping =
== Read ==
SENSOR(power1_average) amdgpu_hwmon_show_power_avg(0) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power1_cap_max) amdgpu_hwmon_show_power_cap_max(0) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_min) amdgpu_hwmon_show_power_cap_min(0) 0
SENSOR(power1_cap) amdgpu_hwmon_show_power_cap(0) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_cap_default) amdgpu_hwmon_show_power_cap_default(0) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_DEFAULT)
SENSOR(power1_label) amdgpu_hwmon_show_power_label(0) "slowPPT"
SENSOR(power2_average) amdgpu_hwmon_show_power_avg(1) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)
SENSOR(power2_cap_max) amdgpu_hwmon_show_power_cap_max(1) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_min) amdgpu_hwmon_show_power_cap_min(1) 0
SENSOR(power2_cap) amdgpu_hwmon_show_power_cap(1) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_FAST)
SENSOR(power2_cap_default) amdgpu_hwmon_show_power_cap_default(1) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_FAST)
SENSOR(power2_label) amdgpu_hwmon_show_power_label(1) "fastPPT"
== Write ==
SENSOR(power1_cap) amdgpu_hwmon_set_power_cap(0,value) set_power_limit( (0<<24) || value )
SENSOR(power2_cap) amdgpu_hwmon_set_power_cap(1,value) set_power_limit( (1<<24) || value )
= Summary =
power1 => PP_PWR_WINDOW_DEFAULT ( label ("slowPPT"))
power2 => PP_PWR_WINDOW_FAST ( label ("fastPPT"))
power_avg => AMDGPU_PP_SENSOR_GPU_POWER
power_cap_max => PP_PWR_LIMIT_MAX
power_cap_min => PP_PWR_LIMIT_MIN (optimized to 0)
power_cap => PP_PWR_LIMIT_CURRENT
power_cap_default => PP_PWR_LIMIT_DEFAULT
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Tuesday, June 1, 2021 12:56 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
May be just call it power_limit or power_cap similar to hwmon. The various limits correspond to hwmon power[1-*]_cap and levels correspond to min/ max etc.
Thanks,
Lijo
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Tuesday, June 1, 2021 4:50 AM
To: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
>< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
>It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
I think you mean something more like this?
+ enum pp_power_constraints
+{
+ PP_PWR_CONSTRAINT_DEFAULT,
+ PP_PWR_CONSTRAINT_FASTWINDOW,
+};
+
________________________________
From: Lazar, Lijo <Lijo.Lazar@amd.com<mailto:Lijo.Lazar@amd.com>>
Sent: Monday, May 31, 2021 2:04 AM
To: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
[Public]
-----Original Message-----
From: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Sent: Saturday, May 29, 2021 4:36 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Powell, Darren <Darren.Powell@amd.com<mailto:Darren.Powell@amd.com>>
Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
add two new powerplay enums (limit_level, sample_window) add enums to smu_get_power_limit signature remove input bitfield stuffing of output variable limit update calls to smu_get_power_limit
* Test
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1` AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | cut -d " " -f 10` HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}
lspci -nn | grep "VGA\|Display" ; \
echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \
echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \
echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default
Signed-off-by: Darren Powell <darren.powell@amd.com<mailto:darren.powell@amd.com>>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----
drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--
4 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index b1cd52a9d684..ddbf802ea8ad 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -192,6 +192,20 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+enum pp_power_limit_level
+{
+ PP_PWR_LIMIT_MIN = -1,
+ PP_PWR_LIMIT_CURRENT,
+ PP_PWR_LIMIT_DEFAULT,
+ PP_PWR_LIMIT_MAX,
+};
+
+ enum pp_power_sample_window
+{
+ PP_PWR_WINDOW_DEFAULT,
+ PP_PWR_WINDOW_FAST,
+};
+
< > The limits are not limited to sample window. There are limits like APU only limit, platform limit and totally obscure ones like PPT0/PPT1 etc.
It's better that the new enum takes care of those as well in case there is a need to make them available through sysfs.
Thanks,
Lijo
#define PP_GROUP_MASK 0xF0000000
#define PP_GROUP_SHIFT 28
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 13da377888d2..f7b45803431d 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
@@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {
struct amdgpu_device *adev = dev_get_drvdata(dev);
const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
- int limit_type = to_sensor_dev_attr(attr)->index;
- uint32_t limit = limit_type << 24;
+ enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,
}
if (is_support_sw_smu(adev)) {
- smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);
+ smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT,
+sample_window);
size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);
} else if (pp_funcs && pp_funcs->get_power_limit) {
pp_funcs->get_power_limit(adev->powerplay.pp_handle,
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 523f9d2982e9..b97b960c2eac 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1262,7 +1262,8 @@ enum smu_cmn2asic_mapping_type { #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level);
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window);
bool smu_mode1_reset_is_support(struct smu_context *smu); bool smu_mode2_reset_is_support(struct smu_context *smu); diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 8aff67a667fa..44c1baa2748d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
int smu_get_power_limit(struct smu_context *smu,
uint32_t *limit,
- enum smu_ppt_limit_level limit_level)
+ enum pp_power_limit_level pp_limit_level,
+ enum pp_power_sample_window sample_window)
{
- uint32_t limit_type = *limit >> 24;
+ enum smu_ppt_limit_level limit_level;
+ uint32_t limit_type;
int ret = 0;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
return -EOPNOTSUPP;
+ switch(sample_window) {
+ case PP_PWR_WINDOW_DEFAULT:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_WINDOW_FAST:
+ limit_type = SMU_FAST_PPT_LIMIT;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
+ switch(pp_limit_level){
+ case PP_PWR_LIMIT_CURRENT:
+ limit_level = SMU_PPT_LIMIT_CURRENT;
+ break;
+ case PP_PWR_LIMIT_DEFAULT:
+ limit_level = SMU_PPT_LIMIT_DEFAULT;
+ break;
+ case PP_PWR_LIMIT_MAX:
+ limit_level = SMU_PPT_LIMIT_MAX;
+ break;
+ case PP_PWR_LIMIT_MIN:
+ default:
+ return -EOPNOTSUPP;
+ break;
+ }
+
mutex_lock(&smu->mutex);
if (limit_type != SMU_DEFAULT_PPT_LIMIT) {
--
2.25.1
[-- Attachment #1.2: Type: text/html, Size: 42272 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit
2021-06-06 5:00 [PATCH v3 0/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
@ 2021-06-06 5:00 ` Darren Powell
0 siblings, 0 replies; 16+ messages in thread
From: Darren Powell @ 2021-06-06 5:00 UTC (permalink / raw)
To: amd-gfx; +Cc: Darren Powell
added doc tag "amdgpu_pp_power" with description
added tags for enums pp_power_limit_level, pp_power_type
added tag for function smu_get_power_limit
Test:
* Temporary insertion into Documentation/gpu/amdgpu.rst
------------START------------
Power Limit
-----------
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:doc: amdgpu_pp_power
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:identifiers: pp_power_limit_level
.. kernel-doc:: drivers/gpu/drm/amd/include/kgd_pp_interface.h
:identifiers: pp_power_type
.. kernel-doc:: drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
:identifiers: smu_get_power_limit
-------------END-------------
Signed-off-by: Darren Powell <darren.powell@amd.com>
---
.../gpu/drm/amd/include/kgd_pp_interface.h | 28 +++++++++++++++++++
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 10 +++++++
2 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 62559723bcb9..9f73a2f586d8 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -194,6 +194,26 @@ enum pp_df_cstate {
DF_CSTATE_ALLOW,
};
+/**
+ * DOC: amdgpu_pp_power
+ *
+ * APU power is managed to system-level requirements through the PPT
+ * (package power tracking) feature. PPT is intended to limit power to the
+ * requirements of the power source and could be dynamically updated to
+ * maximize APU performance within the system power budget.
+ *
+ * Two types of power measurement can be requested, where supported, with
+ * :c:type:`enum pp_power_type <pp_power_type>`.
+ */
+
+/**
+ * enum pp_power_limit_level - Used to query the power limits
+ *
+ * @PP_PWR_LIMIT_MIN: Minimum Power Limit
+ * @PP_PWR_LIMIT_CURRENT: Current Power Limit
+ * @PP_PWR_LIMIT_DEFAULT: Default Power Limit
+ * @PP_PWR_LIMIT_MAX: Maximum Power Limit
+ */
enum pp_power_limit_level
{
PP_PWR_LIMIT_MIN = -1,
@@ -202,6 +222,14 @@ enum pp_power_limit_level
PP_PWR_LIMIT_MAX,
};
+/**
+ * enum pp_power_type - Used to specify the type of the requested power
+ *
+ * @PP_PWR_TYPE_SUSTAINED: manages the configurable, thermally significant
+ * moving average of APU power (default ~5000 ms).
+ * @PP_PWR_TYPE_FAST: manages the ~10 ms moving average of APU power,
+ * where supported.
+ */
enum pp_power_type
{
PP_PWR_TYPE_SUSTAINED,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index c9b921cd48cd..b4ea8b233240 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2173,6 +2173,16 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
return ret;
}
+/**
+ * smu_get_power_limit - Request one of the SMU Power Limits
+ *
+ * @handle: pointer to smu context
+ * @limit: requested limit is written back to this variable
+ * @pp_limit_level: &pp_power_limit_level which limit of the power to return
+ * @pp_power_type: &pp_power_type type of power
+ * Return: 0 on success, <0 on error
+ *
+ */
int smu_get_power_limit(void *handle,
uint32_t *limit,
enum pp_power_limit_level pp_limit_level,
--
2.31.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-06-06 5:00 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-28 23:06 [PATCH v2 1/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
2021-05-28 23:06 ` [PATCH 1/6] amdgpu/pm: reorder definition of swsmu_pm_funcs for readability Darren Powell
2021-05-28 23:06 ` [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature Darren Powell
2021-05-31 6:04 ` Lazar, Lijo
2021-05-31 23:20 ` Powell, Darren
2021-06-01 4:56 ` Lazar, Lijo
2021-06-02 3:55 ` Powell, Darren
2021-06-02 4:32 ` Lazar, Lijo
2021-06-03 2:47 ` Powell, Darren
2021-06-03 4:23 ` Lazar, Lijo
2021-05-28 23:06 ` [PATCH 3/6] amdgpu/pm: modify Powerplay API get_power_limit to use new pp_power enums Darren Powell
2021-05-28 23:06 ` [PATCH 4/6] amdgpu/pm: modify and add smu_get_power_limit to Powerplay API Darren Powell
2021-05-28 23:06 ` [PATCH 5/6] amdgpu/pm: handle return value for get_power_limit Darren Powell
2021-05-28 23:06 ` [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit Darren Powell
2021-06-01 8:12 ` Quan, Evan
2021-06-06 5:00 [PATCH v3 0/6] Modify smu_get_power_limit to implement Powerplay API Darren Powell
2021-06-06 5:00 ` [PATCH 6/6] amdgpu/pm: add kernel documentation for smu_get_power_limit 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.