[PATCH] drm/amd/pm: correct the checks for fan attributes support
Lazar, Lijo
lijo.lazar at amd.com
Tue Jan 11 10:14:47 UTC 2022
On 1/11/2022 3:36 PM, Quan, Evan wrote:
> [AMD Official Use Only]
>
>
>
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>> Sent: Tuesday, January 11, 2022 4:14 PM
>> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Chen, Guchun
>> <Guchun.Chen at amd.com>
>> 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 <evan.quan at amd.com>
>>> 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?
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,
>>>
More information about the amd-gfx
mailing list