* [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 2/6] amdgpu/pm: clean up smu_get_power_limit function signature
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
add two new powerplay enums (limit_level, type)
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 11`
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 7bc7492f37b9..6689164e62f2 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -194,6 +194,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_type
+{
+ PP_PWR_TYPE_SUSTAINED,
+ PP_PWR_TYPE_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 2e1286f6a2ad..c827f0ae5afa 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2907,8 +2907,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_type power_type = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
uint32_t max_limit = 0;
ssize_t size;
int r;
@@ -2925,7 +2925,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, power_type);
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,
@@ -2947,8 +2947,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_type power_type = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -2964,7 +2964,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, power_type);
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,
@@ -2986,8 +2986,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_type power_type = to_sensor_dev_attr(attr)->index;
+ uint32_t limit;
ssize_t size;
int r;
@@ -3003,7 +3003,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, power_type);
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 71adb9e76a95..026d9b6d5ad3 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1264,7 +1264,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_type pp_power_type);
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 1b8321d12c8a..cfa680850887 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2175,14 +2175,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_type pp_power_type)
{
- 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(pp_power_type) {
+ case PP_PWR_TYPE_SUSTAINED:
+ limit_type = SMU_DEFAULT_PPT_LIMIT;
+ break;
+ case PP_PWR_TYPE_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.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 2/6] amdgpu/pm: clean up smu_get_power_limit function signature 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.