<div dir="auto"><div>Hi Evan,<div dir="auto"> </div><div dir="auto">Bit of a drive by comment but I think that maybe all the *_fan_speed_percent() function names are a bit confusing if they no longer operate on percents but on a duty cycle unit of 0-255. No good idea what to call them though :-\</div><div dir="auto"> </div><div dir="auto">Also max() could be used in a bunch of places instead of</div><div dir="auto"> </div><div dir="auto"> if (speed > 255)</div> speed = 255; Regards,</div><div dir="auto">Nils <div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">Den ons 7 juli 2021 03:59Evan Quan <<a href="mailto:evan.quan@amd.com">evan.quan@amd.com</a>> skrev: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Currently, the readout of fan speed pwm is transited into percent-based and then pwm-based. However, the transition into percent-based is totally unnecessary and make the final output less accurate. Change-Id: Ib99e088cda1875b4e2601f7077a178af6fe8a6cb Signed-off-by: Evan Quan <<a href="mailto:evan.quan@amd.com" target="_blank" rel="noreferrer">evan.quan@amd.com</a>> --- drivers/gpu/drm/amd/pm/amdgpu_pm.c | 4 ---- .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 4 ++-- .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c | 12 ++++++------ .../gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 2 +- .../drm/amd/pm/powerplay/hwmgr/vega10_thermal.c | 10 +++++----- .../gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c | 2 +- .../drm/amd/pm/powerplay/hwmgr/vega20_thermal.c | 12 ++++++------ drivers/gpu/drm/amd/pm/powerplay/si_dpm.c | 10 +++++----- drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 12 ++---------- drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 14 +++++++------- 10 files changed, 35 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c index 769f58d5ae1a..e9c98e3f4cfb 100644 --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c @@ -2469,8 +2469,6 @@ static ssize_t amdgpu_hwmon_set_pwm1(struct device *dev, return err; } - value = (value * 100) / 255; - if (adev->powerplay.pp_funcs->set_fan_speed_percent) err = amdgpu_dpm_set_fan_speed_percent(adev, value); else @@ -2515,8 +2513,6 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev, if (err) return err; - speed = (speed * 255) / 100; - return sprintf(buf, "%i\n", speed); } diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c index 0541bfc81c1b..aa353a628c50 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c @@ -3212,7 +3212,7 @@ static int smu7_force_dpm_level(struct pp_hwmgr *hwmgr, if (!ret) { if (level == AMD_DPM_FORCED_LEVEL_PROFILE_PEAK && hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) - smu7_fan_ctrl_set_fan_speed_percent(hwmgr, 100); + smu7_fan_ctrl_set_fan_speed_percent(hwmgr, 255); else if (level != AMD_DPM_FORCED_LEVEL_PROFILE_PEAK && hwmgr->dpm_level == AMD_DPM_FORCED_LEVEL_PROFILE_PEAK) smu7_fan_ctrl_reset_fan_speed_to_default(hwmgr); } @@ -4988,7 +4988,7 @@ static void smu7_set_fan_control_mode(struct pp_hwmgr *hwmgr, uint32_t mode) { switch (mode) { case AMD_FAN_CTRL_NONE: - smu7_fan_ctrl_set_fan_speed_percent(hwmgr, 100); + smu7_fan_ctrl_set_fan_speed_percent(hwmgr, 255); break; case AMD_FAN_CTRL_MANUAL: if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps, diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c index 6cfe148ed45b..70ccc127e3fd 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c @@ -70,12 +70,12 @@ int smu7_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr, return -EINVAL; - tmp64 = (uint64_t)duty * 100; + tmp64 = (uint64_t)duty * 255; do_div(tmp64, duty100); *speed = (uint32_t)tmp64; - if (*speed > 100) - *speed = 100; + if (*speed > 255) + *speed = 255; return 0; } @@ -214,8 +214,8 @@ int smu7_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr, if (hwmgr->thermal_controller.fanInfo.bNoFan) return 0; - if (speed > 100) - speed = 100; + if (speed > 255) + speed = 255; if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl)) smu7_fan_ctrl_stop_smc_fan_control(hwmgr); @@ -227,7 +227,7 @@ int smu7_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr, return -EINVAL; tmp64 = (uint64_t)speed * duty100; - do_div(tmp64, 100); + do_div(tmp64, 255); duty = (uint32_t)tmp64; PHM_WRITE_VFPF_INDIRECT_FIELD(hwmgr->device, CGS_IND_REG__SMC, diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c index 25979106fd25..44c5e2588046 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c @@ -4199,7 +4199,7 @@ static void vega10_set_fan_control_mode(struct pp_hwmgr *hwmgr, uint32_t mode) switch (mode) { case AMD_FAN_CTRL_NONE: - vega10_fan_ctrl_set_fan_speed_percent(hwmgr, 100); + vega10_fan_ctrl_set_fan_speed_percent(hwmgr, 255); break; case AMD_FAN_CTRL_MANUAL: if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl)) diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c index 9b46b27bd30c..6b4c4294afca 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c @@ -78,11 +78,11 @@ int vega10_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr, if (hwmgr->thermal_controller. advanceFanControlParameters.usMaxFanRPM != 0) - percent = current_rpm * 100 / + percent = current_rpm * 255 / hwmgr->thermal_controller. advanceFanControlParameters.usMaxFanRPM; - *speed = percent > 100 ? 100 : percent; + *speed = percent > 255 ? 255 : percent; return 0; } @@ -257,8 +257,8 @@ int vega10_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr, if (hwmgr->thermal_controller.fanInfo.bNoFan) return 0; - if (speed > 100) - speed = 100; + if (speed > 255) + speed = 255; if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl)) vega10_fan_ctrl_stop_smc_fan_control(hwmgr); @@ -270,7 +270,7 @@ int vega10_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr, return -EINVAL; tmp64 = (uint64_t)speed * duty100; - do_div(tmp64, 100); + do_div(tmp64, 255); duty = (uint32_t)tmp64; WREG32_SOC15(THM, 0, mmCG_FDO_CTRL0, diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c index 0791309586c5..cbe5f8027ee0 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c @@ -2769,7 +2769,7 @@ static void vega20_set_fan_control_mode(struct pp_hwmgr *hwmgr, uint32_t mode) { switch (mode) { case AMD_FAN_CTRL_NONE: - vega20_fan_ctrl_set_fan_speed_percent(hwmgr, 100); + vega20_fan_ctrl_set_fan_speed_percent(hwmgr, 255); break; case AMD_FAN_CTRL_MANUAL: if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl)) diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c index 43d754952bd9..eb007c00d7c6 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c @@ -129,12 +129,12 @@ int vega20_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr, if (!duty100) return -EINVAL; - tmp64 = (uint64_t)duty * 100; + tmp64 = (uint64_t)duty * 255; do_div(tmp64, duty100); *speed = (uint32_t)tmp64; - if (*speed > 100) - *speed = 100; + if (*speed > 255) + *speed = 255; return 0; } @@ -147,8 +147,8 @@ int vega20_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr, uint32_t duty; uint64_t tmp64; - if (speed > 100) - speed = 100; + if (speed > 255) + speed = 255; if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl)) vega20_fan_ctrl_stop_smc_fan_control(hwmgr); @@ -160,7 +160,7 @@ int vega20_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr, return -EINVAL; tmp64 = (uint64_t)speed * duty100; - do_div(tmp64, 100); + do_div(tmp64, 255); duty = (uint32_t)tmp64; WREG32_SOC15(THM, 0, mmCG_FDO_CTRL0, diff --git a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c index 15c0b8af376f..96ca359c10a5 100644 --- a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c +++ b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c @@ -6555,12 +6555,12 @@ static int si_dpm_get_fan_speed_percent(void *handle, if (duty100 == 0) return -EINVAL; - tmp64 = (u64)duty * 100; + tmp64 = (u64)duty * 255; do_div(tmp64, duty100); *speed = (u32)tmp64; - if (*speed > 100) - *speed = 100; + if (*speed > 255) + *speed = 255; return 0; } @@ -6580,7 +6580,7 @@ static int si_dpm_set_fan_speed_percent(void *handle, if (si_pi->fan_is_controlled_by_smc) return -EINVAL; - if (speed > 100) + if (speed > 255) return -EINVAL; duty100 = (RREG32(CG_FDO_CTRL1) & FMAX_DUTY100_MASK) >> FMAX_DUTY100_SHIFT; @@ -6589,7 +6589,7 @@ static int si_dpm_set_fan_speed_percent(void *handle, return -EINVAL; tmp64 = (u64)speed * duty100; - do_div(tmp64, 100); + do_div(tmp64, 255); duty = (u32)tmp64; tmp = RREG32(CG_FDO_CTRL0) & ~FDO_STATIC_DUTY_MASK; diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c index 54fb3d7d23ee..94c15526ad21 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c @@ -2565,23 +2565,17 @@ static int smu_get_fan_speed_percent(void *handle, u32 *speed) { struct smu_context *smu = handle; int ret = 0; - uint32_t percent; if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) return -EOPNOTSUPP; mutex_lock(&smu->mutex); - if (smu->ppt_funcs->get_fan_speed_percent) { - ret = smu->ppt_funcs->get_fan_speed_percent(smu, &percent); - if (!ret) { - *speed = percent > 100 ? 100 : percent; - } - } + if (smu->ppt_funcs->get_fan_speed_percent) + ret = smu->ppt_funcs->get_fan_speed_percent(smu, speed); mutex_unlock(&smu->mutex); - return ret; } @@ -2596,8 +2590,6 @@ static int smu_set_fan_speed_percent(void *handle, u32 speed) mutex_lock(&smu->mutex); if (smu->ppt_funcs->set_fan_speed_percent) { - if (speed > 100) - speed = 100; ret = smu->ppt_funcs->set_fan_speed_percent(smu, speed); if (!ret && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) { smu->user_dpm_profile.custom_fan_speed |= SMU_CUSTOM_FAN_SPEED_PWM; diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c index 0cdf55a0dba2..f0ae0920c07e 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c @@ -1191,8 +1191,8 @@ smu_v11_0_set_fan_speed_percent(struct smu_context *smu, uint32_t speed) uint32_t duty100, duty; uint64_t tmp64; - if (speed > 100) - speed = 100; + if (speed > 255) + speed = 255; if (smu_v11_0_auto_fan_control(smu, 0)) return -EINVAL; @@ -1203,7 +1203,7 @@ smu_v11_0_set_fan_speed_percent(struct smu_context *smu, uint32_t speed) return -EINVAL; tmp64 = (uint64_t)speed * duty100; - do_div(tmp64, 100); + do_div(tmp64, 255); duty = (uint32_t)tmp64; WREG32_SOC15(THM, 0, mmCG_FDO_CTRL0, @@ -1274,12 +1274,12 @@ int smu_v11_0_get_fan_speed_percent(struct smu_context *smu, if (!duty100) return -EINVAL; - tmp64 = (uint64_t)duty * 100; + tmp64 = (uint64_t)duty * 255; do_div(tmp64, duty100); *speed = (uint32_t)tmp64; - if (*speed > 100) - *speed = 100; + if (*speed > 255) + *speed = 255; return 0; } @@ -1320,7 +1320,7 @@ smu_v11_0_set_fan_control_mode(struct smu_context *smu, switch (mode) { case AMD_FAN_CTRL_NONE: - ret = smu_v11_0_set_fan_speed_percent(smu, 100); + ret = smu_v11_0_set_fan_speed_percent(smu, 255); break; case AMD_FAN_CTRL_MANUAL: ret = smu_v11_0_auto_fan_control(smu, 0); -- 2.29.0 _______________________________________________ amd-gfx mailing list <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank" rel="noreferrer">amd-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </blockquote></div></div></div>