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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx> 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@xxxxxxx>
---
 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux