drm/amd/pm: correct the checks for fan attributes support

On functionality unsupported, -EOPNOTSUPP will be returned. And we rely
on that to determine the fan attributes support.

Fixes: 79c65f3fcbb128 ("drm/amd/pm: do not expose power implementation details to amdgpu_pm.c")
Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index a8eec91c099554f461e6658a12ae1579946bbbc4..38712009949333737db133516e4d2fc3d989f938 100644 (file)
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -315,8 +315,8 @@ struct amd_pm_funcs {
                                void  *rps,
                                bool  *equal);
 /* export for sysfs */
-       void (*set_fan_control_mode)(void *handle, u32 mode);
-       u32 (*get_fan_control_mode)(void *handle);
+       int (*set_fan_control_mode)(void *handle, u32 mode);
+       int (*get_fan_control_mode)(void *handle, u32 *fan_mode);
        int (*set_fan_speed_pwm)(void *handle, u32 speed);
        int (*get_fan_speed_pwm)(void *handle, u32 *speed);
        int (*force_clock_level)(void *handle, enum pp_clock_type type, uint32_t mask);

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index 728b6e10f30285bb8fa2850021d98bfd31af491c..f0daa66f5b3dd9ecc8d8fe2d69f71a6a1f8c9dda 100644 (file)
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -1087,15 +1087,17 @@ int amdgpu_dpm_get_fan_control_mode(struct amdgpu_device *adev,
                                    uint32_t *fan_mode)
 {
        const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
+       int ret = 0;
 
        if (!pp_funcs->get_fan_control_mode)
                return -EOPNOTSUPP;
 
        mutex_lock(&adev->pm.mutex);
-       *fan_mode = pp_funcs->get_fan_control_mode(adev->powerplay.pp_handle);
+       ret = pp_funcs->get_fan_control_mode(adev->powerplay.pp_handle,
+                                            fan_mode);
        mutex_unlock(&adev->pm.mutex);
 
-       return 0;
+       return ret;
 }
 
 int amdgpu_dpm_set_fan_speed_pwm(struct amdgpu_device *adev,
@@ -1105,7 +1107,7 @@ int amdgpu_dpm_set_fan_speed_pwm(struct amdgpu_device *adev,
        int ret = 0;
 
        if (!pp_funcs->set_fan_speed_pwm)
-               return -EINVAL;
+               return -EOPNOTSUPP;
 
        mutex_lock(&adev->pm.mutex);
        ret = pp_funcs->set_fan_speed_pwm(adev->powerplay.pp_handle,
@@ -1122,7 +1124,7 @@ int amdgpu_dpm_get_fan_speed_pwm(struct amdgpu_device *adev,
        int ret = 0;
 
        if (!pp_funcs->get_fan_speed_pwm)
-               return -EINVAL;
+               return -EOPNOTSUPP;
 
        mutex_lock(&adev->pm.mutex);
        ret = pp_funcs->get_fan_speed_pwm(adev->powerplay.pp_handle,
@@ -1139,7 +1141,7 @@ int amdgpu_dpm_get_fan_speed_rpm(struct amdgpu_device *adev,
        int ret = 0;
 
        if (!pp_funcs->get_fan_speed_rpm)
-               return -EINVAL;
+               return -EOPNOTSUPP;
 
        mutex_lock(&adev->pm.mutex);
        ret = pp_funcs->get_fan_speed_rpm(adev->powerplay.pp_handle,
@@ -1156,7 +1158,7 @@ int amdgpu_dpm_set_fan_speed_rpm(struct amdgpu_device *adev,
        int ret = 0;
 
        if (!pp_funcs->set_fan_speed_rpm)
-               return -EINVAL;
+               return -EOPNOTSUPP;
 
        mutex_lock(&adev->pm.mutex);
        ret = pp_funcs->set_fan_speed_rpm(adev->powerplay.pp_handle,
@@ -1170,16 +1172,17 @@ int amdgpu_dpm_set_fan_control_mode(struct amdgpu_device *adev,
                                    uint32_t mode)
 {
        const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
+       int ret = 0;
 
        if (!pp_funcs->set_fan_control_mode)
                return -EOPNOTSUPP;
 
        mutex_lock(&adev->pm.mutex);
-       pp_funcs->set_fan_control_mode(adev->powerplay.pp_handle,
-                                      mode);
+       ret = pp_funcs->set_fan_control_mode(adev->powerplay.pp_handle,
+                                            mode);
        mutex_unlock(&adev->pm.mutex);
 
-       return 0;
+       return ret;
 }
 
 int amdgpu_dpm_get_power_limit(struct amdgpu_device *adev,

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index d2823aaeca0973f739cb2397750edbe489a00a12..1b03ad7a21ad8421f32ea2061357ec73ac4dbc40 100644 (file)
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -3147,7 +3147,6 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
        struct device *dev = kobj_to_dev(kobj);
        struct amdgpu_device *adev = dev_get_drvdata(dev);
        umode_t effective_mode = attr->mode;
-       uint32_t speed = 0;
 
        /* under multi-vf mode, the hwmon attributes are all not supported */
        if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev))
@@ -3213,15 +3212,15 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
                return 0;
 
        /* mask fan attributes if we have no bindings for this asic to expose */
-       if (((amdgpu_dpm_get_fan_speed_pwm(adev, &speed) == -EINVAL) &&
+       if (((amdgpu_dpm_get_fan_speed_pwm(adev, NULL) == -EOPNOTSUPP) &&
              attr == &sensor_dev_attr_pwm1.dev_attr.attr) || /* can't query fan */
-           ((amdgpu_dpm_get_fan_control_mode(adev, &speed) == -EOPNOTSUPP) &&
+           ((amdgpu_dpm_get_fan_control_mode(adev, NULL) == -EOPNOTSUPP) &&
             attr == &sensor_dev_attr_pwm1_enable.dev_attr.attr)) /* can't query state */
                effective_mode &= ~S_IRUGO;
 
-       if (((amdgpu_dpm_set_fan_speed_pwm(adev, speed) == -EINVAL) &&
+       if (((amdgpu_dpm_set_fan_speed_pwm(adev, U32_MAX) == -EOPNOTSUPP) &&
              attr == &sensor_dev_attr_pwm1.dev_attr.attr) || /* can't manage fan */
-             ((amdgpu_dpm_set_fan_control_mode(adev, speed) == -EOPNOTSUPP) &&
+             ((amdgpu_dpm_set_fan_control_mode(adev, U32_MAX) == -EOPNOTSUPP) &&
              attr == &sensor_dev_attr_pwm1_enable.dev_attr.attr)) /* can't manage state */
                effective_mode &= ~S_IWUSR;
 
@@ -3241,16 +3240,16 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
                return 0;
 
        /* hide max/min values if we can't both query and manage the fan */
-       if (((amdgpu_dpm_set_fan_speed_pwm(adev, speed) == -EINVAL) &&
-             (amdgpu_dpm_get_fan_speed_pwm(adev, &speed) == -EINVAL) &&
-             (amdgpu_dpm_set_fan_speed_rpm(adev, speed) == -EINVAL) &&
-             (amdgpu_dpm_get_fan_speed_rpm(adev, &speed) == -EINVAL)) &&
+       if (((amdgpu_dpm_set_fan_speed_pwm(adev, U32_MAX) == -EOPNOTSUPP) &&
+             (amdgpu_dpm_get_fan_speed_pwm(adev, NULL) == -EOPNOTSUPP) &&
+             (amdgpu_dpm_set_fan_speed_rpm(adev, U32_MAX) == -EOPNOTSUPP) &&
+             (amdgpu_dpm_get_fan_speed_rpm(adev, NULL) == -EOPNOTSUPP)) &&
            (attr == &sensor_dev_attr_pwm1_max.dev_attr.attr ||
             attr == &sensor_dev_attr_pwm1_min.dev_attr.attr))
                return 0;
 
-       if ((amdgpu_dpm_set_fan_speed_rpm(adev, speed) == -EINVAL) &&
-            (amdgpu_dpm_get_fan_speed_rpm(adev, &speed) == -EINVAL) &&
+       if ((amdgpu_dpm_set_fan_speed_rpm(adev, U32_MAX) == -EOPNOTSUPP) &&
+            (amdgpu_dpm_get_fan_speed_rpm(adev, NULL) == -EOPNOTSUPP) &&
             (attr == &sensor_dev_attr_fan1_max.dev_attr.attr ||
             attr == &sensor_dev_attr_fan1_min.dev_attr.attr))
                return 0;

diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
index 92b987fb31d42638bd04615988396954a1682583..23ff0d812e4ba809f33a1e75b2444337af41d3da 100644 (file)
--- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
@@ -6619,6 +6619,9 @@ static int si_dpm_get_fan_speed_pwm(void *handle,
        u64 tmp64;
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+       if (!speed)
+               return -EINVAL;
+
        if (adev->pm.no_fan)
                return -ENOENT;
 
@@ -6669,10 +6672,13 @@ static int si_dpm_set_fan_speed_pwm(void *handle,
        return 0;
 }
 
-static void si_dpm_set_fan_control_mode(void *handle, u32 mode)
+static int si_dpm_set_fan_control_mode(void *handle, u32 mode)
 {
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+       if (mode == U32_MAX)
+               return -EINVAL;
+
        if (mode) {
                /* stop auto-manage */
                if (adev->pm.dpm.fan.ucode_fan_control)
@@ -6685,19 +6691,26 @@ static void si_dpm_set_fan_control_mode(void *handle, u32 mode)
                else
                        si_fan_ctrl_set_default_mode(adev);
        }
+
+       return 0;
 }
 
-static u32 si_dpm_get_fan_control_mode(void *handle)
+static int si_dpm_get_fan_control_mode(void *handle, u32 *fan_mode)
 {
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
        struct si_power_info *si_pi = si_get_pi(adev);
        u32 tmp;
 
+       if (!fan_mode)
+               return -EINVAL;
+
        if (si_pi->fan_is_controlled_by_smc)
                return 0;
 
        tmp = RREG32(CG_FDO_CTRL2) & FDO_PWM_MODE_MASK;
-       return (tmp >> FDO_PWM_MODE_SHIFT);
+       *fan_mode = (tmp >> FDO_PWM_MODE_SHIFT);
+
+       return 0;
 }
 
 #if 0

diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index 89341729744df4aaed00a4633907cbb4f62d298f..76c26ae368f9f6565935b54a708640fe2ffb8b06 100644 (file)
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -488,38 +488,43 @@ static enum amd_pm_state_type pp_dpm_get_current_power_state(void *handle)
        return pm_type;
 }
 
-static void pp_dpm_set_fan_control_mode(void *handle, uint32_t mode)
+static int pp_dpm_set_fan_control_mode(void *handle, uint32_t mode)
 {
        struct pp_hwmgr *hwmgr = handle;
 
        if (!hwmgr || !hwmgr->pm_en)
-               return;
+               return -EOPNOTSUPP;
+
+       if (hwmgr->hwmgr_func->set_fan_control_mode == NULL)
+               return -EOPNOTSUPP;
+
+       if (mode == U32_MAX)
+               return -EINVAL;
 
-       if (hwmgr->hwmgr_func->set_fan_control_mode == NULL) {
-               pr_info_ratelimited("%s was not implemented.\n", __func__);
-               return;
-       }
        mutex_lock(&hwmgr->smu_lock);
        hwmgr->hwmgr_func->set_fan_control_mode(hwmgr, mode);
        mutex_unlock(&hwmgr->smu_lock);
+
+       return 0;
 }
 
-static uint32_t pp_dpm_get_fan_control_mode(void *handle)
+static int pp_dpm_get_fan_control_mode(void *handle, uint32_t *fan_mode)
 {
        struct pp_hwmgr *hwmgr = handle;
-       uint32_t mode = 0;
 
        if (!hwmgr || !hwmgr->pm_en)
-               return 0;
+               return -EOPNOTSUPP;
+
+       if (hwmgr->hwmgr_func->get_fan_control_mode == NULL)
+               return -EOPNOTSUPP;
+
+       if (!fan_mode)
+               return -EINVAL;
 
-       if (hwmgr->hwmgr_func->get_fan_control_mode == NULL) {
-               pr_info_ratelimited("%s was not implemented.\n", __func__);
-               return 0;
-       }
        mutex_lock(&hwmgr->smu_lock);
-       mode = hwmgr->hwmgr_func->get_fan_control_mode(hwmgr);
+       *fan_mode = hwmgr->hwmgr_func->get_fan_control_mode(hwmgr);
        mutex_unlock(&hwmgr->smu_lock);
-       return mode;
+       return 0;
 }
 
 static int pp_dpm_set_fan_speed_pwm(void *handle, uint32_t speed)
@@ -528,12 +533,14 @@ static int pp_dpm_set_fan_speed_pwm(void *handle, uint32_t speed)
        int ret = 0;
 
        if (!hwmgr || !hwmgr->pm_en)
+               return -EOPNOTSUPP;
+
+       if (hwmgr->hwmgr_func->set_fan_speed_pwm == NULL)
+               return -EOPNOTSUPP;
+
+       if (speed == U32_MAX)
                return -EINVAL;
 
-       if (hwmgr->hwmgr_func->set_fan_speed_pwm == NULL) {
-               pr_info_ratelimited("%s was not implemented.\n", __func__);
-               return 0;
-       }
        mutex_lock(&hwmgr->smu_lock);
        ret = hwmgr->hwmgr_func->set_fan_speed_pwm(hwmgr, speed);
        mutex_unlock(&hwmgr->smu_lock);
@@ -546,12 +553,13 @@ static int pp_dpm_get_fan_speed_pwm(void *handle, uint32_t *speed)
        int ret = 0;
 
        if (!hwmgr || !hwmgr->pm_en)
-               return -EINVAL;
+               return -EOPNOTSUPP;
 
-       if (hwmgr->hwmgr_func->get_fan_speed_pwm == NULL) {
-               pr_info_ratelimited("%s was not implemented.\n", __func__);
-               return 0;
-       }
+       if (hwmgr->hwmgr_func->get_fan_speed_pwm == NULL)
+               return -EOPNOTSUPP;
+
+       if (!speed)
+               return -EINVAL;
 
        mutex_lock(&hwmgr->smu_lock);
        ret = hwmgr->hwmgr_func->get_fan_speed_pwm(hwmgr, speed);
@@ -565,9 +573,12 @@ static int pp_dpm_get_fan_speed_rpm(void *handle, uint32_t *rpm)
        int ret = 0;
 
        if (!hwmgr || !hwmgr->pm_en)
-               return -EINVAL;
+               return -EOPNOTSUPP;
 
        if (hwmgr->hwmgr_func->get_fan_speed_rpm == NULL)
+               return -EOPNOTSUPP;
+
+       if (!rpm)
                return -EINVAL;
 
        mutex_lock(&hwmgr->smu_lock);
@@ -582,12 +593,14 @@ static int pp_dpm_set_fan_speed_rpm(void *handle, uint32_t rpm)
        int ret = 0;
 
        if (!hwmgr || !hwmgr->pm_en)
+               return -EOPNOTSUPP;
+
+       if (hwmgr->hwmgr_func->set_fan_speed_rpm == NULL)
+               return -EOPNOTSUPP;
+
+       if (rpm == U32_MAX)
                return -EINVAL;
 
-       if (hwmgr->hwmgr_func->set_fan_speed_rpm == NULL) {
-               pr_info_ratelimited("%s was not implemented.\n", __func__);
-               return 0;
-       }
        mutex_lock(&hwmgr->smu_lock);
        ret = hwmgr->hwmgr_func->set_fan_speed_rpm(hwmgr, rpm);
        mutex_unlock(&hwmgr->smu_lock);

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index c374c30674968694322243fc050087de291f176a..828cb932f6a920c5b6c42e420bfb0dbe470080a9 100644 (file)
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -59,7 +59,7 @@ static int smu_handle_task(struct smu_context *smu,
                           bool lock_needed);
 static int smu_reset(struct smu_context *smu);
 static int smu_set_fan_speed_pwm(void *handle, u32 speed);
-static int smu_set_fan_control_mode(struct smu_context *smu, int value);
+static int smu_set_fan_control_mode(void *handle, u32 value);
 static int smu_set_power_limit(void *handle, uint32_t limit);
 static int smu_set_fan_speed_rpm(void *handle, uint32_t speed);
 static int smu_set_gfx_cgpg(struct smu_context *smu, bool enabled);
@@ -407,7 +407,7 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
        if (smu->user_dpm_profile.fan_mode == AMD_FAN_CTRL_MANUAL ||
            smu->user_dpm_profile.fan_mode == AMD_FAN_CTRL_NONE) {
                ret = smu_set_fan_control_mode(smu, smu->user_dpm_profile.fan_mode);
-               if (ret) {
+               if (ret != -EOPNOTSUPP) {
                        smu->user_dpm_profile.fan_speed_pwm = 0;
                        smu->user_dpm_profile.fan_speed_rpm = 0;
                        smu->user_dpm_profile.fan_mode = AMD_FAN_CTRL_AUTO;
@@ -416,13 +416,13 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
 
                if (smu->user_dpm_profile.fan_speed_pwm) {
                        ret = smu_set_fan_speed_pwm(smu, smu->user_dpm_profile.fan_speed_pwm);
-                       if (ret)
+                       if (ret != -EOPNOTSUPP)
                                dev_err(smu->adev->dev, "Failed to set manual fan speed in pwm\n");
                }
 
                if (smu->user_dpm_profile.fan_speed_rpm) {
                        ret = smu_set_fan_speed_rpm(smu, smu->user_dpm_profile.fan_speed_rpm);
-                       if (ret)
+                       if (ret != -EOPNOTSUPP)
                                dev_err(smu->adev->dev, "Failed to set manual fan speed in rpm\n");
                }
        }
@@ -2218,18 +2218,22 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
        if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
                return -EOPNOTSUPP;
 
+       if (!smu->ppt_funcs->set_fan_speed_rpm)
+               return -EOPNOTSUPP;
+
+       if (speed == U32_MAX)
+               return -EINVAL;
+
        mutex_lock(&smu->mutex);
 
-       if (smu->ppt_funcs->set_fan_speed_rpm) {
-               ret = smu->ppt_funcs->set_fan_speed_rpm(smu, speed);
-               if (!ret && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) {
-                       smu->user_dpm_profile.flags |= SMU_CUSTOM_FAN_SPEED_RPM;
-                       smu->user_dpm_profile.fan_speed_rpm = speed;
+       ret = smu->ppt_funcs->set_fan_speed_rpm(smu, speed);
+       if (!ret && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) {
+               smu->user_dpm_profile.flags |= SMU_CUSTOM_FAN_SPEED_RPM;
+               smu->user_dpm_profile.fan_speed_rpm = speed;
 
-                       /* Override custom PWM setting as they cannot co-exist */
-                       smu->user_dpm_profile.flags &= ~SMU_CUSTOM_FAN_SPEED_PWM;
-                       smu->user_dpm_profile.fan_speed_pwm = 0;
-               }
+               /* Override custom PWM setting as they cannot co-exist */
+               smu->user_dpm_profile.flags &= ~SMU_CUSTOM_FAN_SPEED_PWM;
+               smu->user_dpm_profile.fan_speed_pwm = 0;
        }
 
        mutex_unlock(&smu->mutex);
@@ -2562,60 +2566,65 @@ static int smu_set_power_profile_mode(void *handle,
 }
 
 
-static u32 smu_get_fan_control_mode(void *handle)
+static int smu_get_fan_control_mode(void *handle, u32 *fan_mode)
 {
        struct smu_context *smu = handle;
-       u32 ret = 0;
 
        if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
-               return AMD_FAN_CTRL_NONE;
+               return -EOPNOTSUPP;
+
+       if (!smu->ppt_funcs->get_fan_control_mode)
+               return -EOPNOTSUPP;
+
+       if (!fan_mode)
+               return -EINVAL;
 
        mutex_lock(&smu->mutex);
 
-       if (smu->ppt_funcs->get_fan_control_mode)
-               ret = smu->ppt_funcs->get_fan_control_mode(smu);
+       *fan_mode = smu->ppt_funcs->get_fan_control_mode(smu);
 
        mutex_unlock(&smu->mutex);
 
-       return ret;
+       return 0;
 }
 
-static int smu_set_fan_control_mode(struct smu_context *smu, int value)
+static int smu_set_fan_control_mode(void *handle, u32 value)
 {
+       struct smu_context *smu = handle;
        int ret = 0;
 
        if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
-               return  -EOPNOTSUPP;
+               return -EOPNOTSUPP;
+
+       if (!smu->ppt_funcs->set_fan_control_mode)
+               return -EOPNOTSUPP;
+
+       if (value == U32_MAX)
+               return -EINVAL;
 
        mutex_lock(&smu->mutex);
 
-       if (smu->ppt_funcs->set_fan_control_mode) {
-               ret = smu->ppt_funcs->set_fan_control_mode(smu, value);
-               if (!ret && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE))
-                       smu->user_dpm_profile.fan_mode = value;
-       }
+       ret = smu->ppt_funcs->set_fan_control_mode(smu, value);
+       if (ret)
+               goto out;
 
-       mutex_unlock(&smu->mutex);
+       if (!(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) {
+               smu->user_dpm_profile.fan_mode = value;
 
-       /* reset user dpm fan speed */
-       if (!ret && value != AMD_FAN_CTRL_MANUAL &&
-                       !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) {
-               smu->user_dpm_profile.fan_speed_pwm = 0;
-               smu->user_dpm_profile.fan_speed_rpm = 0;
-               smu->user_dpm_profile.flags &= ~(SMU_CUSTOM_FAN_SPEED_RPM | SMU_CUSTOM_FAN_SPEED_PWM);
+               /* reset user dpm fan speed */
+               if (value != AMD_FAN_CTRL_MANUAL) {
+                       smu->user_dpm_profile.fan_speed_pwm = 0;
+                       smu->user_dpm_profile.fan_speed_rpm = 0;
+                       smu->user_dpm_profile.flags &= ~(SMU_CUSTOM_FAN_SPEED_RPM | SMU_CUSTOM_FAN_SPEED_PWM);
+               }
        }
 
-       return ret;
-}
-
-static void smu_pp_set_fan_control_mode(void *handle, u32 value)
-{
-       struct smu_context *smu = handle;
+out:
+       mutex_unlock(&smu->mutex);
 
-       smu_set_fan_control_mode(smu, value);
+       return ret;
 }
 
-
 static int smu_get_fan_speed_pwm(void *handle, u32 *speed)
 {
        struct smu_context *smu = handle;
@@ -2624,10 +2633,15 @@ static int smu_get_fan_speed_pwm(void *handle, u32 *speed)
        if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
                return -EOPNOTSUPP;
 
+       if (!smu->ppt_funcs->get_fan_speed_pwm)
+               return -EOPNOTSUPP;
+
+       if (!speed)
+               return -EINVAL;
+
        mutex_lock(&smu->mutex);
 
-       if (smu->ppt_funcs->get_fan_speed_pwm)
-               ret = smu->ppt_funcs->get_fan_speed_pwm(smu, speed);
+       ret = smu->ppt_funcs->get_fan_speed_pwm(smu, speed);
 
        mutex_unlock(&smu->mutex);
 
@@ -2642,18 +2656,22 @@ static int smu_set_fan_speed_pwm(void *handle, u32 speed)
        if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
                return -EOPNOTSUPP;
 
+       if (!smu->ppt_funcs->set_fan_speed_pwm)
+               return -EOPNOTSUPP;
+
+       if (speed == U32_MAX)
+               return -EINVAL;
+
        mutex_lock(&smu->mutex);
 
-       if (smu->ppt_funcs->set_fan_speed_pwm) {
-               ret = smu->ppt_funcs->set_fan_speed_pwm(smu, speed);
-               if (!ret && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) {
-                       smu->user_dpm_profile.flags |= SMU_CUSTOM_FAN_SPEED_PWM;
-                       smu->user_dpm_profile.fan_speed_pwm = speed;
+       ret = smu->ppt_funcs->set_fan_speed_pwm(smu, speed);
+       if (!ret && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) {
+               smu->user_dpm_profile.flags |= SMU_CUSTOM_FAN_SPEED_PWM;
+               smu->user_dpm_profile.fan_speed_pwm = speed;
 
-                       /* Override custom RPM setting as they cannot co-exist */
-                       smu->user_dpm_profile.flags &= ~SMU_CUSTOM_FAN_SPEED_RPM;
-                       smu->user_dpm_profile.fan_speed_rpm = 0;
-               }
+               /* Override custom RPM setting as they cannot co-exist */
+               smu->user_dpm_profile.flags &= ~SMU_CUSTOM_FAN_SPEED_RPM;
+               smu->user_dpm_profile.fan_speed_rpm = 0;
        }
 
        mutex_unlock(&smu->mutex);
@@ -2669,10 +2687,15 @@ static int smu_get_fan_speed_rpm(void *handle, uint32_t *speed)
        if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
                return -EOPNOTSUPP;
 
+       if (!smu->ppt_funcs->get_fan_speed_rpm)
+               return -EOPNOTSUPP;
+
+       if (!speed)
+               return -EINVAL;
+
        mutex_lock(&smu->mutex);
 
-       if (smu->ppt_funcs->get_fan_speed_rpm)
-               ret = smu->ppt_funcs->get_fan_speed_rpm(smu, speed);
+       ret = smu->ppt_funcs->get_fan_speed_rpm(smu, speed);
 
        mutex_unlock(&smu->mutex);
 
@@ -3101,7 +3124,7 @@ static int smu_get_prv_buffer_details(void *handle, void **addr, size_t *size)
 
 static const struct amd_pm_funcs swsmu_pm_funcs = {
        /* export for sysfs */
-       .set_fan_control_mode    = smu_pp_set_fan_control_mode,
+       .set_fan_control_mode    = smu_set_fan_control_mode,
        .get_fan_control_mode    = smu_get_fan_control_mode,
        .set_fan_speed_pwm   = smu_set_fan_speed_pwm,
        .get_fan_speed_pwm   = smu_get_fan_speed_pwm,