<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="#0563C1" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> [Public]<o:p></o:p> <o:p> </o:p> I’m looking for an appropriate name that can accommodate more limits like <o:p></o:p> <o:p> </o:p> enum pp_power_limit<o:p></o:p> {<o:p></o:p> PP_PWR_LIMIT_DEFAULT,<o:p></o:p> PP_PWR_LIMIT_FAST, <o:p></o:p> PP_PWR_LIMIT_APU,<o:p></o:p> PP_PWR_LIMIT_PLATFORM,<o:p></o:p> };<o:p></o:p> <o:p> </o:p> 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<o:p></o:p> <o:p> </o:p> enum pp_power_limit<o:p></o:p> {<o:p></o:p> PP_PWR_LIMIT_0,<o:p></o:p> PP_PWR_LIMIT_1,<o:p></o:p> PP_PWR_LIMIT_2,<o:p></o:p> PP_PWR_LIMIT_3,<o:p></o:p> };<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Lijo<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> 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<o:p></o:p> </div> </div> <o:p> </o:p> [Public]<o:p></o:p> <o:p> </o:p> <div> <div> 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.<o:p></o:p> </div> <div> Please give more specific example?<o:p></o:p> </div> <div> Darren <o:p></o:p> </div> <div> <o:p> </o:p> </div> <div> <div> <div> = HWMON Mapping =<o:p></o:p> </div> <div> == Read ==<o:p></o:p> </div> </div> <div style="border:solid #EAECF0 1.0pt;padding:12.0pt 12.0pt 12.0pt 12.0pt"> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power1_average) amdgpu_hwmon_show_power_avg(0) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power1_cap_max) amdgpu_hwmon_show_power_cap_max(0) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_DEFAULT)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power1_cap_min) amdgpu_hwmon_show_power_cap_min(0) 0<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power1_cap) amdgpu_hwmon_show_power_cap(0) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_DEFAULT)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power1_cap_default) amdgpu_hwmon_show_power_cap_default(0) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_DEFAULT)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power1_label) amdgpu_hwmon_show_power_label(0) "slowPPT"<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power2_average) amdgpu_hwmon_show_power_avg(1) amdgpu_dpm_read_sensor(AMDGPU_PP_SENSOR_GPU_POWER)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power2_cap_max) amdgpu_hwmon_show_power_cap_max(1) get_power_limit(PP_PWR_LIMIT_MAX, PP_PWR_WINDOW_FAST)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power2_cap_min) amdgpu_hwmon_show_power_cap_min(1) 0<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power2_cap) amdgpu_hwmon_show_power_cap(1) get_power_limit(PP_PWR_LIMIT_CURRENT, PP_PWR_WINDOW_FAST)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power2_cap_default) amdgpu_hwmon_show_power_cap_default(1) get_power_limit(PP_PWR_LIMIT_DEFAULT, PP_PWR_WINDOW_FAST)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power2_label) amdgpu_hwmon_show_power_label(1) "fastPPT"<o:p></o:p></pre> </div> <div> <div> == Write ==<o:p></o:p> </div> </div> <div style="border:solid #EAECF0 1.0pt;padding:12.0pt 12.0pt 12.0pt 12.0pt"> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power1_cap) amdgpu_hwmon_set_power_cap(0,value) set_power_limit( (0<<24) || value )<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">SENSOR(power2_cap) amdgpu_hwmon_set_power_cap(1,value) set_power_limit( (1<<24) || value )<o:p></o:p></pre> </div> <o:p> </o:p> <div> = Summary =<o:p></o:p> </div> <div style="border:solid #EAECF0 1.0pt;padding:12.0pt 12.0pt 12.0pt 12.0pt"> <pre style="line-height:15.6pt;background:#F8F9FA">power1 => PP_PWR_WINDOW_DEFAULT ( label ("slowPPT"))<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">power2 => PP_PWR_WINDOW_FAST ( label ("fastPPT"))<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">power_avg => AMDGPU_PP_SENSOR_GPU_POWER<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">power_cap_max => PP_PWR_LIMIT_MAX<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">power_cap_min => PP_PWR_LIMIT_MIN (optimized to 0)<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">power_cap => PP_PWR_LIMIT_CURRENT<o:p></o:p></pre> <pre style="line-height:15.6pt;background:#F8F9FA">power_cap_default => PP_PWR_LIMIT_DEFAULT<o:p></o:p></pre> </div> <o:p> </o:p> </div> <div> <o:p> </o:p> </div> <div> <o:p> </o:p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> From: Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>> Sent: Tuesday, June 1, 2021 12:56 AM To: Powell, Darren <<a href="mailto:Darren.Powell@amd.com">Darren.Powell@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>> Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature <o:p></o:p> <div> <o:p></o:p> </div> </div> <div> <div> [Public]<o:p></o:p> <o:p></o:p> 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.<o:p></o:p> <o:p></o:p> Thanks,<o:p></o:p> Lijo<o:p></o:p> <o:p></o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Powell, Darren <<a href="mailto:Darren.Powell@amd.com">Darren.Powell@amd.com</a>> Sent: Tuesday, June 1, 2021 4:50 AM To: Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature<o:p></o:p> </div> </div> <o:p></o:p> [Public]<o:p></o:p> <o:p></o:p> <div> <div> >< > 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.<o:p></o:p> </div> <div> <o:p></o:p> </div> <div> I think you mean something more like this?<o:p></o:p> </div> <div> + enum pp_power_constraints +{ + PP_PWR_CONSTRAINT_DEFAULT, + PP_PWR_CONSTRAINT_FASTWINDOW, +}; +<o:p></o:p> </div> <div> <o:p></o:p> </div> <div> <o:p></o:p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="x_divRplyFwdMsg"> From: Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>> Sent: Monday, May 31, 2021 2:04 AM To: Powell, Darren <<a href="mailto:Darren.Powell@amd.com">Darren.Powell@amd.com</a>>; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>> Subject: RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature <o:p></o:p> <div> <o:p></o:p> </div> </div> <div> <div> [Public] -----Original Message----- From: Powell, Darren <<a href="mailto:Darren.Powell@amd.com">Darren.Powell@amd.com</a>> Sent: Saturday, May 29, 2021 4:36 AM To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Cc: Powell, Darren <<a href="mailto:Darren.Powell@amd.com">Darren.Powell@amd.com</a>> 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 <<a href="mailto:darren.powell@amd.com">darren.powell@amd.com</a>> --- .../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<o:p></o:p> </div> </div> </div> </div> </div> </div> </div> </body> </html>