On 8/12/2021 2:19 PM, Quan, Evan wrote:
[AMD Official Use Only]
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Sent: Thursday, August 12, 2021 3:53 PM
To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: nils.wallmenius@xxxxxxxxx; Chen, Guchun <Guchun.Chen@xxxxxxx>
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@xxxxxxx>
Sent: Thursday, August 12, 2021 2:05 PM
To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: nils.wallmenius@xxxxxxxxx; Chen, Guchun <Guchun.Chen@xxxxxxx>
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@xxxxxxx>
--
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);
