[PATCH 5/7] drm/amd/pm: drop the unnecessary intermediate percent-based transition
Nils Wallménius
nils.wallmenius at gmail.com
Wed Jul 7 06:26:47 UTC 2021
Hi Evan,
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 :-\
Also max() could be used in a bunch of places instead of
if (speed > 255)
speed = 255;
Regards,
Nils
Den ons 7 juli 2021 03:59Evan Quan <evan.quan at amd.com> skrev:
> 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 <evan.quan at amd.com>
> ---
> 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
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210707/59c4788e/attachment-0001.htm>
More information about the amd-gfx
mailing list