On 1/11/2022 4:12 PM, Quan, Evan wrote:
[AMD Official Use Only]



-----Original Message-----
From: Lazar, Lijo <[email protected]>
Sent: Tuesday, January 11, 2022 6:15 PM
To: Quan, Evan <[email protected]>; [email protected]
Cc: Deucher, Alexander <[email protected]>; Chen, Guchun
<[email protected]>
Subject: Re: [PATCH] drm/amd/pm: correct the checks for fan attributes
support



On 1/11/2022 3:36 PM, Quan, Evan wrote:
[AMD Official Use Only]



-----Original Message-----
From: Lazar, Lijo <[email protected]>
Sent: Tuesday, January 11, 2022 4:14 PM
To: Quan, Evan <[email protected]>; [email protected]
Cc: Deucher, Alexander <[email protected]>; Chen, Guchun
<[email protected]>
Subject: Re: [PATCH] drm/amd/pm: correct the checks for fan
attributes support



On 1/11/2022 1:17 PM, Evan Quan wrote:
On functionality unsupported, -EOPNOTSUPP will be returned. And we
rely on that to determine the fan attributes support.

Fixes: 801771de0331 ("drm/amd/pm: do not expose power
implementation
details to
amdgpu_pm.c")

Signed-off-by: Evan Quan <[email protected]>
Change-Id: I95e7e0beebd678a446221a72234cd356e14f0fcd
---
    .../gpu/drm/amd/include/kgd_pp_interface.h    |   4 +-
    drivers/gpu/drm/amd/pm/amdgpu_dpm.c           |  31 ++++-
    drivers/gpu/drm/amd/pm/amdgpu_pm.c            |  21 ++--
    drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c    |  10 +-
    .../gpu/drm/amd/pm/powerplay/amd_powerplay.c  |  59 ++++-----
    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 115
+++++++++----
-----
    6 files changed, 132 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index a8eec91c0995..387120099493 100644
--- 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 68d2e80a673b..fe69785df403 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -1030,15 +1030,20 @@ 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;

+       if (!fan_mode)
+               return -EINVAL;
+

pp_funcs most likely will be there for smu/powerplay subsystem. I
think the checks should be at one layer down -
smu_get_fan_control_mode() and
pp_dpm_get_fan_control_mode()

First one will check if ppt func is implemented and second one will
check if hwmgr func is implemented for the specific ASIC.
[Quan, Evan] Yes, I agree. And if you go through the changes below, you
will see the checks (for the layers down) there.
This patch checks not only those amdgpu_dpm_xxxx APIs, but also those
downstream interfaces(smu_xxxx and pp_dpm_xxxx).


Say you call amdgpu_dpm_get_fan_control_mode(adev, NULL) from
amdgpu_pm

pp_funcs->get_fan_control_mode => this will point to
smu_get_fan_control_mode for all swsmu ASICs. So  "if (!pp_funcs-
get_fan_control_mode)" will be false.

The next statement is NULL check and it will return -EINVAL.

What we want to know is ppt_func is implemented or not for the particualr
swsmu ASIC. Isn't that the case or am I reading it differently?

[Quan, Evan] OK, I get your point now. Hmm, that will be a little tricky.
I suppose the EINVAL check needs to be dispatched in every 
instance(pp_dpm_xxxxx, smu_xxxx, si_dpm_xxxx) then. Any better idea?


Yeah, that is what I meant by "checks should be at one layer down".

Thanks,
Lijo

BR
Evan
Thanks,
Lijo

BR
Evan

Thanks,
Lijo

        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,
@@
-1048,6 +1053,9 @@ int amdgpu_dpm_set_fan_speed_pwm(struct
amdgpu_device *adev,
        int ret = 0;

        if (!pp_funcs->set_fan_speed_pwm)
+               return -EOPNOTSUPP;
+
+       if (speed == U32_MAX)
                return -EINVAL;

        mutex_lock(&adev->pm.mutex);
@@ -1065,6 +1073,9 @@ int amdgpu_dpm_get_fan_speed_pwm(struct
amdgpu_device *adev,
        int ret = 0;

        if (!pp_funcs->get_fan_speed_pwm)
+               return -EOPNOTSUPP;
+
+       if (!speed)
                return -EINVAL;

        mutex_lock(&adev->pm.mutex);
@@ -1082,6 +1093,9 @@ int amdgpu_dpm_get_fan_speed_rpm(struct
amdgpu_device *adev,
        int ret = 0;

        if (!pp_funcs->get_fan_speed_rpm)
+               return -EOPNOTSUPP;
+
+       if (!speed)
                return -EINVAL;

        mutex_lock(&adev->pm.mutex);
@@ -1099,6 +1113,9 @@ int amdgpu_dpm_set_fan_speed_rpm(struct
amdgpu_device *adev,
        int ret = 0;

        if (!pp_funcs->set_fan_speed_rpm)
+               return -EOPNOTSUPP;
+
+       if (speed == U32_MAX)
                return -EINVAL;

        mutex_lock(&adev->pm.mutex);
@@ -1113,16 +1130,20 @@ 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;

+       if (mode == U32_MAX)
+               return -EINVAL;
+
        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 d3eab245e0fe..940cbe751163 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -3197,7 +3197,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))
@@
-3263,15 +3262,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;

@@ -3291,16 +3290,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 92b987fb31d4..7677d3a21f8c 100644
--- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
@@ -6669,7 +6669,7 @@ 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;

@@ -6685,9 +6685,11 @@ 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); @@ -6697,7
+6699,9 @@ static u32 si_dpm_get_fan_control_mode(void *handle)
                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 89341729744d..57bc9405d6a9 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -488,38 +488,37 @@ 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 (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 (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 +527,11 @@ static int pp_dpm_set_fan_speed_pwm(void
*handle,
uint32_t speed)
        int ret = 0;

        if (!hwmgr || !hwmgr->pm_en)
-               return -EINVAL;
+               return -EOPNOTSUPP;
+
+       if (hwmgr->hwmgr_func->set_fan_speed_pwm == NULL)
+               return -EOPNOTSUPP;

-       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 +544,10 @@ 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;

        mutex_lock(&hwmgr->smu_lock);
        ret = hwmgr->hwmgr_func->get_fan_speed_pwm(hwmgr, speed);
@@
-565,10 +561,10 @@ 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 -EINVAL;
+               return -EOPNOTSUPP;

        mutex_lock(&hwmgr->smu_lock);
        ret = hwmgr->hwmgr_func->get_fan_speed_rpm(hwmgr, rpm);
@@ -582,12
+578,11 @@ static int pp_dpm_set_fan_speed_rpm(void *handle,
uint32_t rpm)
        int ret = 0;

        if (!hwmgr || !hwmgr->pm_en)
-               return -EINVAL;
+               return -EOPNOTSUPP;
+
+       if (hwmgr->hwmgr_func->set_fan_speed_rpm == NULL)
+               return -EOPNOTSUPP;

-       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 c374c3067496..474f1f04cc96 100644
--- 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,19 @@ 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;
+
        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 +2563,59 @@ 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;

        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;

        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 +2624,12 @@ 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;
+
        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 +2644,19 @@ 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;
+
        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 +2672,12 @@ 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;
+
        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 +3106,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,

Reply via email to