[PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings
Lazar, Lijo
lijo.lazar at amd.com
Thu Aug 12 09:11:23 UTC 2021
On 8/12/2021 2:19 PM, Quan, Evan wrote:
> [AMD Official Use Only]
>
>
>
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>> Sent: Thursday, August 12, 2021 3:53 PM
>> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: nils.wallmenius at gmail.com; Chen, Guchun <Guchun.Chen at amd.com>
>> Subject: Re: [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based
>> fan speed settings
>>
>>
>>
>> On 8/12/2021 12:21 PM, Quan, Evan wrote:
>>> [AMD Official Use Only]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>>>> Sent: Thursday, August 12, 2021 2:05 PM
>>>> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
>>>> Cc: nils.wallmenius at gmail.com; Chen, Guchun <Guchun.Chen at amd.com>
>>>> Subject: Re: [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM
>> based
>>>> fan speed settings
>>>>
>>>>
>>>>
>>>> On 8/12/2021 9:31 AM, Evan Quan wrote:
>>>>> As the relationship "PWM = RPM / smu->fan_max_rpm" between fan
>>>> speed
>>>>> PWM and RPM is not true for SMU11 ASICs. So, both the RPM and PWM
>>>>> settings need to be saved.
>>>>>
>>>>> Change-Id: I318c134d442273d518b805339cdf383e151b935d
>>>>> Signed-off-by: Evan Quan <evan.quan at amd.com>
>>>>> --
>>>>> v1->v2:
>>>>> - coding style and logging prints optimization (Guchun)
>>>>> - reuse existing flags (Lijo)
>>>>> ---
>>>>> drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +++
>>>>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 22
>>>> ++++++++++++++++------
>>>>> 2 files changed, 19 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
>>>>> b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
>>>>> index 183654f8b564..29934a5af44e 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
>>>>> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
>>>>> @@ -34,6 +34,8 @@
>>>>> #define SMU_FW_NAME_LEN 0x24
>>>>>
>>>>> #define SMU_DPM_USER_PROFILE_RESTORE (1 << 0)
>>>>> +#define SMU_CUSTOM_FAN_SPEED_RPM (1 << 1)
>>>>> +#define SMU_CUSTOM_FAN_SPEED_PWM (1 << 2)
>>>>>
>>>>> // Power Throttlers
>>>>> #define SMU_THROTTLER_PPT0_BIT 0
>>>>> @@ -230,6 +232,7 @@ struct smu_user_dpm_profile {
>>>>> uint32_t fan_mode;
>>>>> uint32_t power_limit;
>>>>> uint32_t fan_speed_percent;
>>>>> + uint32_t fan_speed_rpm;
>>>>> uint32_t flags;
>>>>> uint32_t user_od;
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> index e33e67310030..131ad0dfefbe 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> @@ -413,7 +413,13 @@ static void
>> smu_restore_dpm_user_profile(struct
>>>> smu_context *smu)
>>>>> if (!ret && smu->user_dpm_profile.fan_speed_percent) {
>>>>> ret = smu_set_fan_speed_percent(smu, smu-
>>>>> user_dpm_profile.fan_speed_percent);
>>>>> if (ret)
>>>>> - dev_err(smu->adev->dev, "Failed to set
>>>> manual fan speed\n");
>>>>> + dev_err(smu->adev->dev, "Failed to set
>>>> manual fan speed in percent\n");
>>>>> + }
>>>>> +
>>>>> + if (!ret && smu->user_dpm_profile.fan_speed_rpm) {
>>>>> + ret = smu_set_fan_speed_rpm(smu, smu-
>>>>> user_dpm_profile.fan_speed_rpm);
>>>>> + if (ret)
>>>>> + dev_err(smu->adev->dev, "Failed to set
>>>> manual fan speed in
>>>>> +rpm\n");
>>>>> }
>>>>> }
>>>>>
>>>>> @@ -2179,7 +2185,6 @@ static int smu_set_gfx_cgpg(struct
>> smu_context
>>>> *smu, bool enabled)
>>>>> static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
>>>>> {
>>>>> struct smu_context *smu = handle;
>>>>> - u32 percent;
>>>>> int ret = 0;
>>>>>
>>>>> if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) @@ -
>>>> 2190,8
>>>>> +2195,8 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t
>>>> speed)
>>>>> 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) {
>>>>> - percent = speed * 100 / smu->fan_max_rpm;
>>>>> - smu->user_dpm_profile.fan_speed_percent =
>>>> percent;
>>>>> + smu->user_dpm_profile.flags |=
>>>> SMU_CUSTOM_FAN_SPEED_RPM;
>>>>> + smu->user_dpm_profile.fan_speed_rpm = speed;
>>>>
>>>> Sorry, missed this when you posted v1. Either RPM or PWM mode is
>>>> allowed at a time, is that right? If so, shall we make the pwm to 0
>>>> and reset PWM flag when RPM is set and vice versa? This helps during
>>>> restore where one is not overwritten with the other.
>>> [Quan, Evan] Sounds reasonable to me. But I suppose we also need to
>> prompt some warnings when user trying to set these two modes at the same
>> time.
>>> Instead of performing these silently. Right?
>>
>> Not sure on the warnings part. For ex: user may set the manual mode,
>> choose a pwm based fan speed and may later switch to a precise rpm based
>> speed. Since it's driven by user, we may not need to warn.
> [Quan, Evan] Hmm. How about the followings? A notice in the description part for related APIs?
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 04c7d82f8b89..112ee5f5d855 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -3174,6 +3174,9 @@ static ssize_t amdgpu_hwmon_show_mclk_label(struct device *dev,
> *
> * - fan[1-\*]_enable: Enable or disable the sensors.1: Enable 0: Disable
> *
> + * NOTE: DO NOT set the fan speed via "pwm1" and "fan[1-\*]_target" interfaces at the same time.
> + * That will get the former one overridden.
> + *
> * hwmon interfaces for GPU clocks:
> *
> * - freq1_input: the gfx/compute clock in hertz
Looks fine.
Thanks,
Lijo
>>
>> Thanks,
>> Lijo
>>
>>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>>> }
>>>>> }
>>>>>
>>>>> @@ -2552,8 +2557,11 @@ static int smu_set_fan_control_mode(struct
>>>>> smu_context *smu, int 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.flags &
>>>> SMU_DPM_USER_PROFILE_RESTORE)) {
>>>>> smu->user_dpm_profile.fan_speed_percent = 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;
>>>>> }
>>>>> @@ -2604,8 +2612,10 @@ static int smu_set_fan_speed_percent(void
>>>> *handle, u32 speed)
>>>>> 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))
>>>>> + 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_percent =
>>>> speed;
>>>>> + }
>>>>> }
>>>>>
>>>>> mutex_unlock(&smu->mutex);
>>>>>
More information about the amd-gfx
mailing list