[PATCH 5/7] drm/amd/pm: drop the unnecessary intermediate percent-based transition

Lazar, Lijo lijo.lazar at amd.com
Wed Jul 7 09:07:47 UTC 2021


As mentioned in another patch, renaming function to set_/get_ pwm will 
clarify things better. The scale is 0-255 in hwmon as well.

Thanks,
Lijo

On 7/7/2021 7:26 AM, Evan Quan wrote:
> 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);
> 


More information about the amd-gfx mailing list