<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <p style="font-family:Arial;font-size:10pt;color:#008000;margin:15pt;" align="Left"> [Public]<br> </p> <br> <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> 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.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Please give more specific example?</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Darren </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <div> <div>= HWMON Mapping =</div> <div>== Read ==</div> </div> <div></div> <pre style="font-family:monospace, monospace;background-color:rgb(248, 249, 250);border:1px solid rgb(234, 236, 240);padding:1em;line-height:1.3em;font-size:14px">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" </pre> <div> <div>== Write ==</div> </div> <pre style="font-family:monospace, monospace;background-color:rgb(248, 249, 250);border:1px solid rgb(234, 236, 240);padding:1em;line-height:1.3em;font-size:14px">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 ) </pre> <p style="margin:0.5em 0px;color:rgb(34, 34, 34);font-family:sans-serif;font-size:14px;background-color:rgb(255, 255, 255)"> <br> </p> <div>= Summary =</div> <pre style="font-family:monospace, monospace;background-color:rgb(248, 249, 250);border:1px solid rgb(234, 236, 240);padding:1em;line-height:1.3em;font-size:14px">power1 => PP_PWR_WINDOW_DEFAULT ( label ("slowPPT")) power2 => PP_PWR_WINDOW_FAST ( label ("fastPPT")) </pre> <pre style="font-family:monospace, monospace;background-color:rgb(248, 249, 250);border:1px solid rgb(234, 236, 240);padding:1em;line-height:1.3em;font-size:14px">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</pre> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Lazar, Lijo <Lijo.Lazar@amd.com><br> <b>Sent:</b> Tuesday, June 1, 2021 12:56 AM<br> <b>To:</b> Powell, Darren <Darren.Powell@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> <b>Subject:</b> RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature</font> <div> </div> </div> <style>  </style> <div lang="EN-US" link="#0563C1" vlink="#954F72" style="word-wrap:break-word"> <div class="x_WordSection1"> <p class="x_msipheaderc10f11a2" style="margin:0in"><span style="font-size:10.0pt; font-family:"Arial",sans-serif; color:green">[Public]</span></p> <p class="x_MsoNormal"> </p> <p class="x_MsoNormal">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.</p> <p class="x_MsoNormal"> </p> <p class="x_MsoNormal">Thanks,</p> <p class="x_MsoNormal">Lijo</p> <p class="x_MsoNormal"> </p> <div> <div style="border:none; border-top:solid #E1E1E1 1.0pt; padding:3.0pt 0in 0in 0in"> <p class="x_MsoNormal"><b>From:</b> Powell, Darren <Darren.Powell@amd.com> <br> <b>Sent:</b> Tuesday, June 1, 2021 4:50 AM<br> <b>To:</b> Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org<br> <b>Subject:</b> Re: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature</p> </div> </div> <p class="x_MsoNormal"> </p> <p style="margin:15.0pt"><span style="font-size:10.0pt; font-family:"Arial",sans-serif; color:green">[Public]</span></p> <p class="x_MsoNormal"> </p> <div> <div> <p class="x_MsoNormal"><span style="font-family:"Segoe UI",sans-serif; color:black"><br> <span style="background:white">>< > 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.</span><br> <span style="background:white">>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.</span></span><span style="font-size:12.0pt; color:black"></span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black">I think you mean something more like this?</span></p> </div> <div> <p class="x_MsoNormal"><span style="font-family:"Segoe UI",sans-serif; color:black; background:white">+ enum pp_power_constraints</span><span style="font-size:12.0pt; color:black"><br> </span><span style="font-family:"Segoe UI",sans-serif; color:black; background:white">+{</span><span style="font-family:"Segoe UI",sans-serif; color:black"><br> <span style="background:white">+ PP_PWR_CONSTRAINT_DEFAULT,</span><br> <span style="background:white">+ PP_PWR_CONSTRAINT_FASTWINDOW,</span><br> <span style="background:white">+};</span><br> <span style="background:white">+</span></span><span style="font-size:12.0pt; color:black"></span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p> </div> <div> <p class="x_MsoNormal"><span style="font-size:12.0pt; color:black"> </span></p> </div> <div class="x_MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="x_divRplyFwdMsg"> <p class="x_MsoNormal"><b><span style="color:black">From:</span></b><span style="color:black"> Lazar, Lijo <<a href="mailto:Lijo.Lazar@amd.com">Lijo.Lazar@amd.com</a>><br> <b>Sent:</b> Monday, May 31, 2021 2:04 AM<br> <b>To:</b> 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>><br> <b>Subject:</b> RE: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature</span> </p> <div> <p class="x_MsoNormal"> </p> </div> </div> <div> <div> <p class="x_MsoNormal">[Public]<br> <br> <br> <br> -----Original Message-----<br> From: Powell, Darren <<a href="mailto:Darren.Powell@amd.com">Darren.Powell@amd.com</a>> <br> Sent: Saturday, May 29, 2021 4:36 AM<br> To: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br> Cc: Powell, Darren <<a href="mailto:Darren.Powell@amd.com">Darren.Powell@amd.com</a>><br> Subject: [PATCH 2/6] amdgpu/pm: clean up smu_get_power_limit function signature<br> <br> 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<br> <br> * Test<br> 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}<br> <br> lspci -nn | grep "VGA\|Display" ; \<br> echo "=== power1 cap ===" ; cat $HWMON_DIR/power1_cap ; \<br> echo "=== power1 cap max ===" ; cat $HWMON_DIR/power1_cap_max ; \<br> echo "=== power1 cap def ===" ; cat $HWMON_DIR/power1_cap_default<br> <br> Signed-off-by: Darren Powell <<a href="mailto:darren.powell@amd.com">darren.powell@amd.com</a>><br> ---<br> .../gpu/drm/amd/include/kgd_pp_interface.h | 14 ++++++++<br> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 18 +++++-----<br> drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +-<br> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 34 +++++++++++++++++--<br> 4 files changed, 57 insertions(+), 12 deletions(-)<br> <br> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h<br> index b1cd52a9d684..ddbf802ea8ad 100644<br> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h<br> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h<br> @@ -192,6 +192,20 @@ enum pp_df_cstate {<br> DF_CSTATE_ALLOW,<br> };<br> <br> +enum pp_power_limit_level<br> +{<br> + PP_PWR_LIMIT_MIN = -1,<br> + PP_PWR_LIMIT_CURRENT,<br> + PP_PWR_LIMIT_DEFAULT,<br> + PP_PWR_LIMIT_MAX,<br> +};<br> +<br> + enum pp_power_sample_window<br> +{<br> + PP_PWR_WINDOW_DEFAULT,<br> + PP_PWR_WINDOW_FAST,<br> +};<br> +<br> <br> < > 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. <br> 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.<br> <br> Thanks,<br> Lijo<br> <br> #define PP_GROUP_MASK 0xF0000000<br> #define PP_GROUP_SHIFT 28<br> <br> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c<br> index 13da377888d2..f7b45803431d 100644<br> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c<br> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c<br> @@ -2717,8 +2717,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev, {<br> struct amdgpu_device *adev = dev_get_drvdata(dev);<br> const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;<br> - int limit_type = to_sensor_dev_attr(attr)->index;<br> - uint32_t limit = limit_type << 24;<br> + enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;<br> + uint32_t limit;<br> uint32_t max_limit = 0;<br> ssize_t size;<br> int r;<br> @@ -2735,7 +2735,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_max(struct device *dev,<br> }<br> <br> if (is_support_sw_smu(adev)) {<br> - smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_MAX);<br> + smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_MAX, <br> +sample_window);<br> size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);<br> } else if (pp_funcs && pp_funcs->get_power_limit) {<br> pp_funcs->get_power_limit(adev->powerplay.pp_handle,<br> @@ -2757,8 +2757,8 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev, {<br> struct amdgpu_device *adev = dev_get_drvdata(dev);<br> const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;<br> - int limit_type = to_sensor_dev_attr(attr)->index;<br> - uint32_t limit = limit_type << 24;<br> + enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;<br> + uint32_t limit;<br> ssize_t size;<br> int r;<br> <br> @@ -2774,7 +2774,7 @@ static ssize_t amdgpu_hwmon_show_power_cap(struct device *dev,<br> }<br> <br> if (is_support_sw_smu(adev)) {<br> - smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_CURRENT);<br> + smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_CURRENT, <br> +sample_window);<br> size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);<br> } else if (pp_funcs && pp_funcs->get_power_limit) {<br> pp_funcs->get_power_limit(adev->powerplay.pp_handle,<br> @@ -2796,8 +2796,8 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev, {<br> struct amdgpu_device *adev = dev_get_drvdata(dev);<br> const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;<br> - int limit_type = to_sensor_dev_attr(attr)->index;<br> - uint32_t limit = limit_type << 24;<br> + enum pp_power_sample_window sample_window = to_sensor_dev_attr(attr)->index;<br> + uint32_t limit;<br> ssize_t size;<br> int r;<br> <br> @@ -2813,7 +2813,7 @@ static ssize_t amdgpu_hwmon_show_power_cap_default(struct device *dev,<br> }<br> <br> if (is_support_sw_smu(adev)) {<br> - smu_get_power_limit(&adev->smu, &limit, SMU_PPT_LIMIT_DEFAULT);<br> + smu_get_power_limit(&adev->smu, &limit, PP_PWR_LIMIT_DEFAULT, <br> +sample_window);<br> size = snprintf(buf, PAGE_SIZE, "%u\n", limit * 1000000);<br> } else if (pp_funcs && pp_funcs->get_power_limit) {<br> pp_funcs->get_power_limit(adev->powerplay.pp_handle,<br> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h<br> index 523f9d2982e9..b97b960c2eac 100644<br> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h<br> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h<br> @@ -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,<br> uint32_t *limit,<br> - enum smu_ppt_limit_level limit_level);<br> + enum pp_power_limit_level pp_limit_level,<br> + enum pp_power_sample_window sample_window);<br> <br> 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<br> index 8aff67a667fa..44c1baa2748d 100644<br> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c<br> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c<br> @@ -2168,14 +2168,44 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)<br> <br> int smu_get_power_limit(struct smu_context *smu,<br> uint32_t *limit,<br> - enum smu_ppt_limit_level limit_level)<br> + enum pp_power_limit_level pp_limit_level,<br> + enum pp_power_sample_window sample_window)<br> {<br> - uint32_t limit_type = *limit >> 24;<br> + enum smu_ppt_limit_level limit_level;<br> + uint32_t limit_type;<br> int ret = 0;<br> <br> if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)<br> return -EOPNOTSUPP;<br> <br> + switch(sample_window) {<br> + case PP_PWR_WINDOW_DEFAULT:<br> + limit_type = SMU_DEFAULT_PPT_LIMIT;<br> + break;<br> + case PP_PWR_WINDOW_FAST:<br> + limit_type = SMU_FAST_PPT_LIMIT;<br> + break;<br> + default:<br> + return -EOPNOTSUPP;<br> + break;<br> + }<br> +<br> + switch(pp_limit_level){<br> + case PP_PWR_LIMIT_CURRENT:<br> + limit_level = SMU_PPT_LIMIT_CURRENT;<br> + break;<br> + case PP_PWR_LIMIT_DEFAULT:<br> + limit_level = SMU_PPT_LIMIT_DEFAULT;<br> + break;<br> + case PP_PWR_LIMIT_MAX:<br> + limit_level = SMU_PPT_LIMIT_MAX;<br> + break;<br> + case PP_PWR_LIMIT_MIN:<br> + default:<br> + return -EOPNOTSUPP;<br> + break;<br> + }<br> +<br> mutex_lock(&smu->mutex);<br> <br> if (limit_type != SMU_DEFAULT_PPT_LIMIT) {<br> --<br> 2.25.1</p> </div> </div> </div> </div> </div> </div> </body> </html>