All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting
@ 2021-08-12  4:01 Evan Quan
  2021-08-12  4:01 ` [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings Evan Quan
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Evan Quan @ 2021-08-12  4:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: nils.wallmenius, Lijo.Lazar, Guchun.Chen, Evan Quan

The relationship "PWM = RPM / smu->fan_max_rpm" between fan speed
PWM and RPM is not true for SMU11 ASICs. So, we need a new way to
perform the fan speed RPM setting.

Change-Id: I1afe8102f02ead9a8a07c7105f689ac60a85b0d8
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
v1->v2:
  - hardcode crystal_clock_freq as 25Mhz (Lijo)
---
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  5 +++
 drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  3 ++
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  9 ++---
 .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c |  1 +
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |  1 +
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   |  1 +
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 35 +++++++++++++++++++
 7 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index c2c201b8e3cf..183654f8b564 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -1047,6 +1047,11 @@ struct pptable_funcs {
 	 */
 	int (*set_fan_speed_percent)(struct smu_context *smu, uint32_t speed);
 
+	/**
+	 * @set_fan_speed_rpm: Set a static fan speed in rpm.
+	 */
+	int (*set_fan_speed_rpm)(struct smu_context *smu, uint32_t speed);
+
 	/**
 	 * @set_xgmi_pstate: Set inter-chip global memory interconnect pstate.
 	 * &pstate: Pstate to set. D0 if Nonzero, D3 otherwise.
diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
index 403bc1bf8a77..b9c8a924dca6 100644
--- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
@@ -224,6 +224,9 @@ smu_v11_0_set_fan_control_mode(struct smu_context *smu,
 int smu_v11_0_set_fan_speed_percent(struct smu_context *smu,
 				    uint32_t speed);
 
+int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
+				uint32_t speed);
+
 int smu_v11_0_set_xgmi_pstate(struct smu_context *smu,
 				     uint32_t pstate);
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 71afc2d20b12..e33e67310030 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2187,11 +2187,12 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
 
 	mutex_lock(&smu->mutex);
 
-	if (smu->ppt_funcs->set_fan_speed_percent) {
-		percent = speed * 100 / smu->fan_max_rpm;
-		ret = smu->ppt_funcs->set_fan_speed_percent(smu, percent);
-		if (!ret && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE))
+	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;
+		}
 	}
 
 	mutex_unlock(&smu->mutex);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 6ec8492f71f5..f909cda86299 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -2316,6 +2316,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.get_fan_control_mode = smu_v11_0_get_fan_control_mode,
 	.set_fan_control_mode = smu_v11_0_set_fan_control_mode,
 	.set_fan_speed_percent = smu_v11_0_set_fan_speed_percent,
+	.set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm,
 	.set_xgmi_pstate = smu_v11_0_set_xgmi_pstate,
 	.gfx_off_control = smu_v11_0_gfx_off_control,
 	.register_irq_handler = smu_v11_0_register_irq_handler,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index d7722c229ddd..08b0edcbc8f9 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -3268,6 +3268,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
 	.get_fan_control_mode = smu_v11_0_get_fan_control_mode,
 	.set_fan_control_mode = smu_v11_0_set_fan_control_mode,
 	.set_fan_speed_percent = smu_v11_0_set_fan_speed_percent,
+	.set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm,
 	.set_xgmi_pstate = smu_v11_0_set_xgmi_pstate,
 	.gfx_off_control = smu_v11_0_gfx_off_control,
 	.register_irq_handler = smu_v11_0_register_irq_handler,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 261ef8ca862e..4e28a4ff1817 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -3903,6 +3903,7 @@ static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
 	.get_fan_control_mode = smu_v11_0_get_fan_control_mode,
 	.set_fan_control_mode = smu_v11_0_set_fan_control_mode,
 	.set_fan_speed_percent = smu_v11_0_set_fan_speed_percent,
+	.set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm,
 	.set_xgmi_pstate = smu_v11_0_set_xgmi_pstate,
 	.gfx_off_control = smu_v11_0_gfx_off_control,
 	.register_irq_handler = smu_v11_0_register_irq_handler,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index b5419e8eba89..007f84bdda6f 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1228,6 +1228,41 @@ smu_v11_0_set_fan_speed_percent(struct smu_context *smu, uint32_t speed)
 	return smu_v11_0_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC);
 }
 
+int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
+				uint32_t speed)
+{
+	struct amdgpu_device *adev = smu->adev;
+	/*
+	 * crystal_clock_freq used for fan speed rpm calculation is
+	 * always 25Mhz. So, hardcode it as 2500(in 10K unit).
+	 */
+	uint32_t crystal_clock_freq = 2500;
+	uint32_t tach_period;
+	int ret;
+
+	ret = smu_v11_0_auto_fan_control(smu, 0);
+	if (ret)
+		return ret;
+
+	/*
+	 * To prevent from possible overheat, some ASICs may have requirement
+	 * for minimum fan speed:
+	 * - For some NV10 SKU, the fan speed cannot be set lower than
+	 *   700 RPM.
+	 * - For some Sienna Cichlid SKU, the fan speed cannot be set
+	 *   lower than 500 RPM.
+	 */
+	tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
+	WREG32_SOC15(THM, 0, mmCG_TACH_CTRL,
+		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_TACH_CTRL),
+				   CG_TACH_CTRL, TARGET_PERIOD,
+				   tach_period));
+
+	ret = smu_v11_0_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC_RPM);
+
+	return ret;
+}
+
 int
 smu_v11_0_set_fan_control_mode(struct smu_context *smu,
 			       uint32_t mode)
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings
  2021-08-12  4:01 [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting Evan Quan
@ 2021-08-12  4:01 ` Evan Quan
  2021-08-12  6:05   ` Lazar, Lijo
  2021-08-12  4:01 ` [PATCH V2 3/7] drm/amd/pm: correct the fan speed PWM retrieving Evan Quan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Evan Quan @ 2021-08-12  4:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: nils.wallmenius, Lijo.Lazar, Guchun.Chen, Evan Quan

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@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;
 		}
 	}
 
@@ -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);
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V2 3/7] drm/amd/pm: correct the fan speed PWM retrieving
  2021-08-12  4:01 [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting Evan Quan
  2021-08-12  4:01 ` [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings Evan Quan
@ 2021-08-12  4:01 ` Evan Quan
  2021-08-12  4:01 ` [PATCH V2 4/7] drm/amd/pm: correct the fan speed RPM retrieving Evan Quan
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Evan Quan @ 2021-08-12  4:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: nils.wallmenius, Lijo.Lazar, Guchun.Chen, Evan Quan

The relationship "PWM = RPM / smu->fan_max_rpm" between fan speed
PWM and RPM is not true for SMU11 ASICs. So, we need a new way to
retrieving the fan speed PWM.

Change-Id: Idfe0276d7113b9c921b88fa08085a33fd971d621
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 .../include/asic_reg/thm/thm_11_0_2_offset.h  |  3 ++
 .../include/asic_reg/thm/thm_11_0_2_sh_mask.h |  3 ++
 drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  3 ++
 .../amd/pm/powerplay/hwmgr/vega20_thermal.c   | 24 ++++++++-----
 .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 25 +------------
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 25 +------------
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 25 +------------
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 35 +++++++++++++++++++
 8 files changed, 62 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h b/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h
index a485526f3a51..eca96abef445 100644
--- a/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h
+++ b/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h
@@ -49,4 +49,7 @@
 #define mmTHM_BACO_CNTL                                                                                0x0081
 #define mmTHM_BACO_CNTL_BASE_IDX                                                                       0
 
+#define mmCG_THERMAL_STATUS                                                                            0x006C
+#define mmCG_THERMAL_STATUS_BASE_IDX                                                                   0
+
 #endif
diff --git a/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_sh_mask.h b/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_sh_mask.h
index d130d92aee19..f2f9eae9a68f 100644
--- a/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_sh_mask.h
+++ b/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_sh_mask.h
@@ -92,5 +92,8 @@
 #define THM_TCON_THERM_TRIP__RSVD3_MASK                                                                       0x7FFFC000L
 #define THM_TCON_THERM_TRIP__SW_THERM_TP_MASK                                                                 0x80000000L
 
+#define CG_THERMAL_STATUS__FDO_PWM_DUTY__SHIFT                                                                0x9
+#define CG_THERMAL_STATUS__FDO_PWM_DUTY_MASK                                                                  0x0001FE00L
+
 #endif
 
diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
index b9c8a924dca6..ff31be2eaff9 100644
--- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
@@ -227,6 +227,9 @@ int smu_v11_0_set_fan_speed_percent(struct smu_context *smu,
 int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 				uint32_t speed);
 
+int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
+				    uint32_t *speed);
+
 int smu_v11_0_set_xgmi_pstate(struct smu_context *smu,
 				     uint32_t pstate);
 
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c
index 269dd7e95a44..43d754952bd9 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c
@@ -117,18 +117,24 @@ static int vega20_get_current_rpm(struct pp_hwmgr *hwmgr, uint32_t *current_rpm)
 int vega20_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
 		uint32_t *speed)
 {
-	struct vega20_hwmgr *data = (struct vega20_hwmgr *)(hwmgr->backend);
-	PPTable_t *pp_table = &(data->smc_state_table.pp_table);
-	uint32_t current_rpm, percent = 0;
-	int ret = 0;
+	struct amdgpu_device *adev = hwmgr->adev;
+	uint32_t duty100, duty;
+	uint64_t tmp64;
 
-	ret = vega20_get_current_rpm(hwmgr, &current_rpm);
-	if (ret)
-		return ret;
+	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
+				CG_FDO_CTRL1, FMAX_DUTY100);
+	duty = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_THERMAL_STATUS),
+				CG_THERMAL_STATUS, FDO_PWM_DUTY);
+
+	if (!duty100)
+		return -EINVAL;
 
-	percent = current_rpm * 100 / pp_table->FanMaximumRpm;
+	tmp64 = (uint64_t)duty * 100;
+	do_div(tmp64, duty100);
+	*speed = (uint32_t)tmp64;
 
-	*speed = percent > 100 ? 100 : percent;
+	if (*speed > 100)
+		*speed = 100;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index f909cda86299..74cd957f37da 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -1162,29 +1162,6 @@ static int arcturus_read_sensor(struct smu_context *smu,
 	return ret;
 }
 
-static int arcturus_get_fan_speed_percent(struct smu_context *smu,
-					  uint32_t *speed)
-{
-	int ret;
-	u32 rpm;
-
-	if (!speed)
-		return -EINVAL;
-
-	switch (smu_v11_0_get_fan_control_mode(smu)) {
-	case AMD_FAN_CTRL_AUTO:
-		ret = arcturus_get_smu_metrics_data(smu,
-						    METRICS_CURR_FANSPEED,
-						    &rpm);
-		if (!ret && smu->fan_max_rpm)
-			*speed = rpm * 100 / smu->fan_max_rpm;
-		return ret;
-	default:
-		*speed = smu->user_dpm_profile.fan_speed_percent;
-		return 0;
-	}
-}
-
 static int arcturus_get_fan_parameters(struct smu_context *smu)
 {
 	PPTable_t *pptable = smu->smu_table.driver_pptable;
@@ -2270,7 +2247,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.print_clk_levels = arcturus_print_clk_levels,
 	.force_clk_levels = arcturus_force_clk_levels,
 	.read_sensor = arcturus_read_sensor,
-	.get_fan_speed_percent = arcturus_get_fan_speed_percent,
+	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
 	.get_power_profile_mode = arcturus_get_power_profile_mode,
 	.set_power_profile_mode = arcturus_set_power_profile_mode,
 	.set_performance_level = arcturus_set_performance_level,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 08b0edcbc8f9..c0bd52c307cf 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1668,29 +1668,6 @@ static bool navi10_is_dpm_running(struct smu_context *smu)
 	return !!(feature_enabled & SMC_DPM_FEATURE);
 }
 
-static int navi10_get_fan_speed_percent(struct smu_context *smu,
-					uint32_t *speed)
-{
-	int ret;
-	u32 rpm;
-
-	if (!speed)
-		return -EINVAL;
-
-	switch (smu_v11_0_get_fan_control_mode(smu)) {
-	case AMD_FAN_CTRL_AUTO:
-		ret = navi1x_get_smu_metrics_data(smu,
-						  METRICS_CURR_FANSPEED,
-						  &rpm);
-		if (!ret && smu->fan_max_rpm)
-			*speed = rpm * 100 / smu->fan_max_rpm;
-		return ret;
-	default:
-		*speed = smu->user_dpm_profile.fan_speed_percent;
-		return 0;
-	}
-}
-
 static int navi10_get_fan_parameters(struct smu_context *smu)
 {
 	PPTable_t *pptable = smu->smu_table.driver_pptable;
@@ -3224,7 +3201,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
 	.display_config_changed = navi10_display_config_changed,
 	.notify_smc_display_config = navi10_notify_smc_display_config,
 	.is_dpm_running = navi10_is_dpm_running,
-	.get_fan_speed_percent = navi10_get_fan_speed_percent,
+	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
 	.get_power_profile_mode = navi10_get_power_profile_mode,
 	.set_power_profile_mode = navi10_set_power_profile_mode,
 	.set_watermarks_table = navi10_set_watermarks_table,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 4e28a4ff1817..0b9be89276ab 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1354,29 +1354,6 @@ static bool sienna_cichlid_is_dpm_running(struct smu_context *smu)
 	return !!(feature_enabled & SMC_DPM_FEATURE);
 }
 
-static int sienna_cichlid_get_fan_speed_percent(struct smu_context *smu,
-						uint32_t *speed)
-{
-	int ret;
-	u32 rpm;
-
-	if (!speed)
-		return -EINVAL;
-
-	switch (smu_v11_0_get_fan_control_mode(smu)) {
-	case AMD_FAN_CTRL_AUTO:
-		ret = sienna_cichlid_get_smu_metrics_data(smu,
-							  METRICS_CURR_FANSPEED,
-							  &rpm);
-		if (!ret && smu->fan_max_rpm)
-			*speed = rpm * 100 / smu->fan_max_rpm;
-		return ret;
-	default:
-		*speed = smu->user_dpm_profile.fan_speed_percent;
-		return 0;
-	}
-}
-
 static int sienna_cichlid_get_fan_parameters(struct smu_context *smu)
 {
 	uint16_t *table_member;
@@ -3859,7 +3836,7 @@ static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
 	.display_config_changed = sienna_cichlid_display_config_changed,
 	.notify_smc_display_config = sienna_cichlid_notify_smc_display_config,
 	.is_dpm_running = sienna_cichlid_is_dpm_running,
-	.get_fan_speed_percent = sienna_cichlid_get_fan_speed_percent,
+	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
 	.get_power_profile_mode = sienna_cichlid_get_power_profile_mode,
 	.set_power_profile_mode = sienna_cichlid_set_power_profile_mode,
 	.set_watermarks_table = sienna_cichlid_set_watermarks_table,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 007f84bdda6f..1f3230f232b7 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1263,6 +1263,41 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 	return ret;
 }
 
+int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
+				    uint32_t *speed)
+{
+	struct amdgpu_device *adev = smu->adev;
+	uint32_t duty100, duty;
+	uint64_t tmp64;
+
+	/*
+	 * For pre Sienna Cichlid ASICs, the 0 RPM may be not correctly
+	 * detected via register retrieving. To workaround this, we will
+	 * report the fan speed as 0 PWM if user just requested such.
+	 */
+	if ((smu->user_dpm_profile.flags & SMU_CUSTOM_FAN_SPEED_PWM)
+	     && !smu->user_dpm_profile.fan_speed_percent) {
+		*speed = 0;
+		return 0;
+	}
+
+	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
+				CG_FDO_CTRL1, FMAX_DUTY100);
+	duty = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_THERMAL_STATUS),
+				CG_THERMAL_STATUS, FDO_PWM_DUTY);
+	if (!duty100)
+		return -EINVAL;
+
+	tmp64 = (uint64_t)duty * 100;
+	do_div(tmp64, duty100);
+	*speed = (uint32_t)tmp64;
+
+	if (*speed > 100)
+		*speed = 100;
+
+	return 0;
+}
+
 int
 smu_v11_0_set_fan_control_mode(struct smu_context *smu,
 			       uint32_t mode)
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V2 4/7] drm/amd/pm: correct the fan speed RPM retrieving
  2021-08-12  4:01 [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting Evan Quan
  2021-08-12  4:01 ` [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings Evan Quan
  2021-08-12  4:01 ` [PATCH V2 3/7] drm/amd/pm: correct the fan speed PWM retrieving Evan Quan
@ 2021-08-12  4:01 ` Evan Quan
  2021-08-12  4:01 ` [PATCH V2 5/7] drm/amd/pm: drop the unnecessary intermediate percent-based transition Evan Quan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Evan Quan @ 2021-08-12  4:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: nils.wallmenius, Lijo.Lazar, Guchun.Chen, Evan Quan

The relationship "PWM = RPM / smu->fan_max_rpm" between fan speed
PWM and RPM is not true for SMU11 ASICs. So, we need a new way to
retrieving the fan speed RPM.

Change-Id: Ife4298c8b7ec93ef023a7da27d59654e0444e044
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
v1->v2
  - drop unneeded intermediate varaible (Guchun)
  - hardcode crystal_clock_freq as 25Mhz (Lijo)
---
 .../include/asic_reg/thm/thm_11_0_2_offset.h  |  3 ++
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  5 ++++
 drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  3 ++
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |  7 ++---
 .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 24 ++++++++++++++++
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 24 ++++++++++++++++
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 17 +++++++++++
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 28 +++++++++++++++++++
 8 files changed, 106 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h b/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h
index eca96abef445..8474f419caa5 100644
--- a/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h
+++ b/drivers/gpu/drm/amd/include/asic_reg/thm/thm_11_0_2_offset.h
@@ -38,6 +38,9 @@
 #define mmCG_TACH_CTRL                                                                                 0x006a
 #define mmCG_TACH_CTRL_BASE_IDX                                                                        0
 
+#define mmCG_TACH_STATUS                                                                               0x006b
+#define mmCG_TACH_STATUS_BASE_IDX                                                                      0
+
 #define mmTHM_THERMAL_INT_ENA                                                                          0x000a
 #define mmTHM_THERMAL_INT_ENA_BASE_IDX                                                                 0
 #define mmTHM_THERMAL_INT_CTRL                                                                         0x000b
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 29934a5af44e..c0ac6754f448 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -729,6 +729,11 @@ struct pptable_funcs {
 	 */
 	int (*get_fan_speed_percent)(struct smu_context *smu, uint32_t *speed);
 
+	/**
+	 * @get_fan_speed_rpm: Get the current fan speed in rpm.
+	 */
+	int (*get_fan_speed_rpm)(struct smu_context *smu, uint32_t *speed);
+
 	/**
 	 * @set_watermarks_table: Configure and upload the watermarks tables to
 	 *                        the SMU.
diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
index ff31be2eaff9..bc8d875b526d 100644
--- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
@@ -230,6 +230,9 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
 				    uint32_t *speed);
 
+int smu_v11_0_get_fan_speed_rpm(struct smu_context *smu,
+				uint32_t *speed);
+
 int smu_v11_0_set_xgmi_pstate(struct smu_context *smu,
 				     uint32_t pstate);
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 131ad0dfefbe..979ba6487577 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2627,17 +2627,14 @@ static int smu_get_fan_speed_rpm(void *handle, uint32_t *speed)
 {
 	struct smu_context *smu = handle;
 	int ret = 0;
-	u32 percent;
 
 	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
 		return -EOPNOTSUPP;
 
 	mutex_lock(&smu->mutex);
 
-	if (smu->ppt_funcs->get_fan_speed_percent) {
-		ret = smu->ppt_funcs->get_fan_speed_percent(smu, &percent);
-		*speed = percent * smu->fan_max_rpm / 100;
-	}
+	if (smu->ppt_funcs->get_fan_speed_rpm)
+		ret = smu->ppt_funcs->get_fan_speed_rpm(smu, speed);
 
 	mutex_unlock(&smu->mutex);
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 74cd957f37da..60ce8a813f19 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -1162,6 +1162,29 @@ static int arcturus_read_sensor(struct smu_context *smu,
 	return ret;
 }
 
+static int arcturus_get_fan_speed_rpm(struct smu_context *smu,
+				      uint32_t *speed)
+{
+	int ret = 0;
+
+	if (!speed)
+		return -EINVAL;
+
+	switch (smu_v11_0_get_fan_control_mode(smu)) {
+	case AMD_FAN_CTRL_AUTO:
+		ret = arcturus_get_smu_metrics_data(smu,
+						    METRICS_CURR_FANSPEED,
+						    speed);
+		break;
+	default:
+		ret = smu_v11_0_get_fan_speed_rpm(smu,
+						  speed);
+		break;
+	}
+
+	return ret;
+}
+
 static int arcturus_get_fan_parameters(struct smu_context *smu)
 {
 	PPTable_t *pptable = smu->smu_table.driver_pptable;
@@ -2248,6 +2271,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.force_clk_levels = arcturus_force_clk_levels,
 	.read_sensor = arcturus_read_sensor,
 	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
+	.get_fan_speed_rpm = arcturus_get_fan_speed_rpm,
 	.get_power_profile_mode = arcturus_get_power_profile_mode,
 	.set_power_profile_mode = arcturus_set_power_profile_mode,
 	.set_performance_level = arcturus_set_performance_level,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index c0bd52c307cf..3cb90fe107c5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -1668,6 +1668,29 @@ static bool navi10_is_dpm_running(struct smu_context *smu)
 	return !!(feature_enabled & SMC_DPM_FEATURE);
 }
 
+static int navi10_get_fan_speed_rpm(struct smu_context *smu,
+				    uint32_t *speed)
+{
+	int ret = 0;
+
+	if (!speed)
+		return -EINVAL;
+
+	switch (smu_v11_0_get_fan_control_mode(smu)) {
+	case AMD_FAN_CTRL_AUTO:
+		ret = navi10_get_smu_metrics_data(smu,
+						  METRICS_CURR_FANSPEED,
+						  speed);
+		break;
+	default:
+		ret = smu_v11_0_get_fan_speed_rpm(smu,
+						  speed);
+		break;
+	}
+
+	return ret;
+}
+
 static int navi10_get_fan_parameters(struct smu_context *smu)
 {
 	PPTable_t *pptable = smu->smu_table.driver_pptable;
@@ -3202,6 +3225,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
 	.notify_smc_display_config = navi10_notify_smc_display_config,
 	.is_dpm_running = navi10_is_dpm_running,
 	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
+	.get_fan_speed_rpm = navi10_get_fan_speed_rpm,
 	.get_power_profile_mode = navi10_get_power_profile_mode,
 	.set_power_profile_mode = navi10_set_power_profile_mode,
 	.set_watermarks_table = navi10_set_watermarks_table,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 0b9be89276ab..513373dcaaca 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -1354,6 +1354,22 @@ static bool sienna_cichlid_is_dpm_running(struct smu_context *smu)
 	return !!(feature_enabled & SMC_DPM_FEATURE);
 }
 
+static int sienna_cichlid_get_fan_speed_rpm(struct smu_context *smu,
+					    uint32_t *speed)
+{
+	if (!speed)
+		return -EINVAL;
+
+	/*
+	 * For Sienna_Cichlid and later, the fan speed(rpm) reported
+	 * by pmfw is always trustable(even when the fan control feature
+	 * disabled or 0 RPM kicked in).
+	 */
+	return sienna_cichlid_get_smu_metrics_data(smu,
+						   METRICS_CURR_FANSPEED,
+						   speed);
+}
+
 static int sienna_cichlid_get_fan_parameters(struct smu_context *smu)
 {
 	uint16_t *table_member;
@@ -3837,6 +3853,7 @@ static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
 	.notify_smc_display_config = sienna_cichlid_notify_smc_display_config,
 	.is_dpm_running = sienna_cichlid_is_dpm_running,
 	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
+	.get_fan_speed_rpm = sienna_cichlid_get_fan_speed_rpm,
 	.get_power_profile_mode = sienna_cichlid_get_power_profile_mode,
 	.set_power_profile_mode = sienna_cichlid_set_power_profile_mode,
 	.set_watermarks_table = sienna_cichlid_set_watermarks_table,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 1f3230f232b7..19334bb6a8b0 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1298,6 +1298,34 @@ int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
 	return 0;
 }
 
+int smu_v11_0_get_fan_speed_rpm(struct smu_context *smu,
+				uint32_t *speed)
+{
+	struct amdgpu_device *adev = smu->adev;
+	uint32_t crystal_clock_freq = 2500;
+	uint32_t tach_status;
+	uint64_t tmp64;
+
+	/*
+	 * For pre Sienna Cichlid ASICs, the 0 RPM may be not correctly
+	 * detected via register retrieving. To workaround this, we will
+	 * report the fan speed as 0 RPM if user just requested such.
+	 */
+	if ((smu->user_dpm_profile.flags & SMU_CUSTOM_FAN_SPEED_RPM)
+	     && !smu->user_dpm_profile.fan_speed_rpm) {
+		*speed = 0;
+		return 0;
+	}
+
+	tmp64 = (uint64_t)crystal_clock_freq * 60 * 10000;
+
+	tach_status = RREG32_SOC15(THM, 0, mmCG_TACH_STATUS);
+	do_div(tmp64, tach_status);
+	*speed = (uint32_t)tmp64;
+
+	return 0;
+}
+
 int
 smu_v11_0_set_fan_control_mode(struct smu_context *smu,
 			       uint32_t mode)
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V2 5/7] drm/amd/pm: drop the unnecessary intermediate percent-based transition
  2021-08-12  4:01 [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting Evan Quan
                   ` (2 preceding siblings ...)
  2021-08-12  4:01 ` [PATCH V2 4/7] drm/amd/pm: correct the fan speed RPM retrieving Evan Quan
@ 2021-08-12  4:01 ` Evan Quan
  2021-08-12  4:01 ` [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check Evan Quan
  2021-08-12  4:01 ` [PATCH V2 7/7] drm/amd/pm: correct the address of Arcturus fan related registers Evan Quan
  5 siblings, 0 replies; 17+ messages in thread
From: Evan Quan @ 2021-08-12  4:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: nils.wallmenius, Lijo.Lazar, Guchun.Chen, Evan Quan

Currently, the readout of fan speed pwm is transited into percent-based
and then pwm-based. However, the transition into percent-based is totally
unnecessary and make the final output less accurate.

Change-Id: Ib99e088cda1875b4e2601f7077a178af6fe8a6cb
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
v1->v2:
  - renaming the related functions as set/get_fan_speed_pwm (Lijo, Nils)
  - use macro MIN() for speed(in pwm) comparing against 255 (Nils)
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  2 ++
 .../gpu/drm/amd/include/kgd_pp_interface.h    |  4 +--
 drivers/gpu/drm/amd/pm/amdgpu_pm.c            | 20 +++++------
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       |  8 ++---
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       | 12 +++----
 drivers/gpu/drm/amd/pm/inc/hwmgr.h            |  6 ++--
 drivers/gpu/drm/amd/pm/inc/smu_v11_0.h        |  4 +--
 .../gpu/drm/amd/pm/powerplay/amd_powerplay.c  | 16 ++++-----
 .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c   |  8 ++---
 .../drm/amd/pm/powerplay/hwmgr/smu7_thermal.c | 21 +++++-------
 .../drm/amd/pm/powerplay/hwmgr/smu7_thermal.h |  4 +--
 .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c |  6 ++--
 .../amd/pm/powerplay/hwmgr/vega10_thermal.c   | 18 +++++-----
 .../amd/pm/powerplay/hwmgr/vega10_thermal.h   |  4 +--
 .../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c |  6 ++--
 .../amd/pm/powerplay/hwmgr/vega20_thermal.c   | 16 ++++-----
 .../amd/pm/powerplay/hwmgr/vega20_thermal.h   |  4 +--
 drivers/gpu/drm/amd/pm/powerplay/si_dpm.c     | 19 +++++------
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 34 +++++++------------
 .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c |  4 +--
 .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |  4 +--
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   |  4 +--
 .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    | 22 +++++-------
 23 files changed, 109 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 96e895d6be35..0f278cc3a5f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1271,6 +1271,8 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
 
 #define amdgpu_inc_vram_lost(adev) atomic_inc(&((adev)->vram_lost_counter));
 
+#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
+
 /* Common functions */
 bool amdgpu_device_has_job_running(struct amdgpu_device *adev);
 bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index e38b191c7b7c..bac15c466733 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -306,8 +306,8 @@ struct amd_pm_funcs {
 /* export for sysfs */
 	void (*set_fan_control_mode)(void *handle, u32 mode);
 	u32 (*get_fan_control_mode)(void *handle);
-	int (*set_fan_speed_percent)(void *handle, u32 speed);
-	int (*get_fan_speed_percent)(void *handle, u32 *speed);
+	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);
 	int (*print_clock_levels)(void *handle, enum pp_clock_type type, char *buf);
 	int (*force_performance_level)(void *handle, enum amd_dpm_forced_level level);
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 04c7d82f8b89..2c182ccc80a8 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -2469,10 +2469,8 @@ static ssize_t amdgpu_hwmon_set_pwm1(struct device *dev,
 		return err;
 	}
 
-	value = (value * 100) / 255;
-
-	if (adev->powerplay.pp_funcs->set_fan_speed_percent)
-		err = amdgpu_dpm_set_fan_speed_percent(adev, value);
+	if (adev->powerplay.pp_funcs->set_fan_speed_pwm)
+		err = amdgpu_dpm_set_fan_speed_pwm(adev, value);
 	else
 		err = -EINVAL;
 
@@ -2504,8 +2502,8 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev,
 		return err;
 	}
 
-	if (adev->powerplay.pp_funcs->get_fan_speed_percent)
-		err = amdgpu_dpm_get_fan_speed_percent(adev, &speed);
+	if (adev->powerplay.pp_funcs->get_fan_speed_pwm)
+		err = amdgpu_dpm_get_fan_speed_pwm(adev, &speed);
 	else
 		err = -EINVAL;
 
@@ -2515,8 +2513,6 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev,
 	if (err)
 		return err;
 
-	speed = (speed * 255) / 100;
-
 	return sprintf(buf, "%i\n", speed);
 }
 
@@ -3349,13 +3345,13 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
 
 	if (!is_support_sw_smu(adev)) {
 		/* mask fan attributes if we have no bindings for this asic to expose */
-		if ((!adev->powerplay.pp_funcs->get_fan_speed_percent &&
+		if ((!adev->powerplay.pp_funcs->get_fan_speed_pwm &&
 		     attr == &sensor_dev_attr_pwm1.dev_attr.attr) || /* can't query fan */
 		    (!adev->powerplay.pp_funcs->get_fan_control_mode &&
 		     attr == &sensor_dev_attr_pwm1_enable.dev_attr.attr)) /* can't query state */
 			effective_mode &= ~S_IRUGO;
 
-		if ((!adev->powerplay.pp_funcs->set_fan_speed_percent &&
+		if ((!adev->powerplay.pp_funcs->set_fan_speed_pwm &&
 		     attr == &sensor_dev_attr_pwm1.dev_attr.attr) || /* can't manage fan */
 		    (!adev->powerplay.pp_funcs->set_fan_control_mode &&
 		     attr == &sensor_dev_attr_pwm1_enable.dev_attr.attr)) /* can't manage state */
@@ -3379,8 +3375,8 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
 
 	if (!is_support_sw_smu(adev)) {
 		/* hide max/min values if we can't both query and manage the fan */
-		if ((!adev->powerplay.pp_funcs->set_fan_speed_percent &&
-		     !adev->powerplay.pp_funcs->get_fan_speed_percent) &&
+		if ((!adev->powerplay.pp_funcs->set_fan_speed_pwm &&
+		     !adev->powerplay.pp_funcs->get_fan_speed_pwm) &&
 		     (!adev->powerplay.pp_funcs->set_fan_speed_rpm &&
 		     !adev->powerplay.pp_funcs->get_fan_speed_rpm) &&
 		    (attr == &sensor_dev_attr_pwm1_max.dev_attr.attr ||
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index d03e6fa2bf1a..98f1b3d8c1d5 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -280,11 +280,11 @@ enum amdgpu_pcie_gen {
 #define amdgpu_dpm_get_fan_control_mode(adev) \
 		((adev)->powerplay.pp_funcs->get_fan_control_mode((adev)->powerplay.pp_handle))
 
-#define amdgpu_dpm_set_fan_speed_percent(adev, s) \
-		((adev)->powerplay.pp_funcs->set_fan_speed_percent((adev)->powerplay.pp_handle, (s)))
+#define amdgpu_dpm_set_fan_speed_pwm(adev, s) \
+		((adev)->powerplay.pp_funcs->set_fan_speed_pwm((adev)->powerplay.pp_handle, (s)))
 
-#define amdgpu_dpm_get_fan_speed_percent(adev, s) \
-		((adev)->powerplay.pp_funcs->get_fan_speed_percent((adev)->powerplay.pp_handle, (s)))
+#define amdgpu_dpm_get_fan_speed_pwm(adev, s) \
+		((adev)->powerplay.pp_funcs->get_fan_speed_pwm((adev)->powerplay.pp_handle, (s)))
 
 #define amdgpu_dpm_get_fan_speed_rpm(adev, s) \
 		((adev)->powerplay.pp_funcs->get_fan_speed_rpm)((adev)->powerplay.pp_handle, (s))
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index c0ac6754f448..715b4225f5ee 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -231,7 +231,7 @@ enum smu_memory_pool_size
 struct smu_user_dpm_profile {
 	uint32_t fan_mode;
 	uint32_t power_limit;
-	uint32_t fan_speed_percent;
+	uint32_t fan_speed_pwm;
 	uint32_t fan_speed_rpm;
 	uint32_t flags;
 	uint32_t user_od;
@@ -543,7 +543,7 @@ struct smu_context
 	struct work_struct interrupt_work;
 
 	unsigned fan_max_rpm;
-	unsigned manual_fan_speed_percent;
+	unsigned manual_fan_speed_pwm;
 
 	uint32_t gfx_default_hard_min_freq;
 	uint32_t gfx_default_soft_max_freq;
@@ -725,9 +725,9 @@ struct pptable_funcs {
 	bool (*is_dpm_running)(struct smu_context *smu);
 
 	/**
-	 * @get_fan_speed_percent: Get the current fan speed in percent.
+	 * @get_fan_speed_pwm: Get the current fan speed in PWM.
 	 */
-	int (*get_fan_speed_percent)(struct smu_context *smu, uint32_t *speed);
+	int (*get_fan_speed_pwm)(struct smu_context *smu, uint32_t *speed);
 
 	/**
 	 * @get_fan_speed_rpm: Get the current fan speed in rpm.
@@ -1051,9 +1051,9 @@ struct pptable_funcs {
 	int (*set_fan_control_mode)(struct smu_context *smu, uint32_t mode);
 
 	/**
-	 * @set_fan_speed_percent: Set a static fan speed in percent.
+	 * @set_fan_speed_pwm: Set a static fan speed in PWM.
 	 */
-	int (*set_fan_speed_percent)(struct smu_context *smu, uint32_t speed);
+	int (*set_fan_speed_pwm)(struct smu_context *smu, uint32_t speed);
 
 	/**
 	 * @set_fan_speed_rpm: Set a static fan speed in rpm.
diff --git a/drivers/gpu/drm/amd/pm/inc/hwmgr.h b/drivers/gpu/drm/amd/pm/inc/hwmgr.h
index 490371bd2520..8ed01071fe5a 100644
--- a/drivers/gpu/drm/amd/pm/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/pm/inc/hwmgr.h
@@ -278,9 +278,9 @@ struct pp_hwmgr_func {
 	int (*get_fan_speed_info)(struct pp_hwmgr *hwmgr, struct phm_fan_speed_info *fan_speed_info);
 	void (*set_fan_control_mode)(struct pp_hwmgr *hwmgr, uint32_t mode);
 	uint32_t (*get_fan_control_mode)(struct pp_hwmgr *hwmgr);
-	int (*set_fan_speed_percent)(struct pp_hwmgr *hwmgr, uint32_t percent);
-	int (*get_fan_speed_percent)(struct pp_hwmgr *hwmgr, uint32_t *speed);
-	int (*set_fan_speed_rpm)(struct pp_hwmgr *hwmgr, uint32_t percent);
+	int (*set_fan_speed_pwm)(struct pp_hwmgr *hwmgr, uint32_t speed);
+	int (*get_fan_speed_pwm)(struct pp_hwmgr *hwmgr, uint32_t *speed);
+	int (*set_fan_speed_rpm)(struct pp_hwmgr *hwmgr, uint32_t speed);
 	int (*get_fan_speed_rpm)(struct pp_hwmgr *hwmgr, uint32_t *speed);
 	int (*reset_fan_speed_to_default)(struct pp_hwmgr *hwmgr);
 	int (*uninitialize_thermal_controller)(struct pp_hwmgr *hwmgr);
diff --git a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
index bc8d875b526d..cbdae8a2c698 100644
--- a/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
+++ b/drivers/gpu/drm/amd/pm/inc/smu_v11_0.h
@@ -221,13 +221,13 @@ int
 smu_v11_0_set_fan_control_mode(struct smu_context *smu,
 			       uint32_t mode);
 
-int smu_v11_0_set_fan_speed_percent(struct smu_context *smu,
+int smu_v11_0_set_fan_speed_pwm(struct smu_context *smu,
 				    uint32_t speed);
 
 int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 				uint32_t speed);
 
-int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
+int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
 				    uint32_t *speed);
 
 int smu_v11_0_get_fan_speed_rpm(struct smu_context *smu,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index d2a38246a78a..321215003643 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -533,7 +533,7 @@ static uint32_t pp_dpm_get_fan_control_mode(void *handle)
 	return mode;
 }
 
-static int pp_dpm_set_fan_speed_percent(void *handle, uint32_t percent)
+static int pp_dpm_set_fan_speed_pwm(void *handle, uint32_t speed)
 {
 	struct pp_hwmgr *hwmgr = handle;
 	int ret = 0;
@@ -541,17 +541,17 @@ static int pp_dpm_set_fan_speed_percent(void *handle, uint32_t percent)
 	if (!hwmgr || !hwmgr->pm_en)
 		return -EINVAL;
 
-	if (hwmgr->hwmgr_func->set_fan_speed_percent == NULL) {
+	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_percent(hwmgr, percent);
+	ret = hwmgr->hwmgr_func->set_fan_speed_pwm(hwmgr, speed);
 	mutex_unlock(&hwmgr->smu_lock);
 	return ret;
 }
 
-static int pp_dpm_get_fan_speed_percent(void *handle, uint32_t *speed)
+static int pp_dpm_get_fan_speed_pwm(void *handle, uint32_t *speed)
 {
 	struct pp_hwmgr *hwmgr = handle;
 	int ret = 0;
@@ -559,13 +559,13 @@ static int pp_dpm_get_fan_speed_percent(void *handle, uint32_t *speed)
 	if (!hwmgr || !hwmgr->pm_en)
 		return -EINVAL;
 
-	if (hwmgr->hwmgr_func->get_fan_speed_percent == NULL) {
+	if (hwmgr->hwmgr_func->get_fan_speed_pwm == NULL) {
 		pr_info_ratelimited("%s was not implemented.\n", __func__);
 		return 0;
 	}
 
 	mutex_lock(&hwmgr->smu_lock);
-	ret = hwmgr->hwmgr_func->get_fan_speed_percent(hwmgr, speed);
+	ret = hwmgr->hwmgr_func->get_fan_speed_pwm(hwmgr, speed);
 	mutex_unlock(&hwmgr->smu_lock);
 	return ret;
 }
@@ -1691,8 +1691,8 @@ static const struct amd_pm_funcs pp_dpm_funcs = {
 	.dispatch_tasks = pp_dpm_dispatch_tasks,
 	.set_fan_control_mode = pp_dpm_set_fan_control_mode,
 	.get_fan_control_mode = pp_dpm_get_fan_control_mode,
-	.set_fan_speed_percent = pp_dpm_set_fan_speed_percent,
-	.get_fan_speed_percent = pp_dpm_get_fan_speed_percent,
+	.set_fan_speed_pwm = pp_dpm_set_fan_speed_pwm,
+	.get_fan_speed_pwm = pp_dpm_get_fan_speed_pwm,
 	.get_fan_speed_rpm = pp_dpm_get_fan_speed_rpm,
 	.set_fan_speed_rpm = pp_dpm_set_fan_speed_rpm,
 	.get_pp_num_states = pp_dpm_get_pp_num_states,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index 0541bfc81c1b..5562ed2d3080 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -3212,7 +3212,7 @@ static int smu7_force_dpm_level(struct pp_hwmgr *hwmgr,
 
 	if (!ret) {
 		if (level == AMD_DPM_FORCED_LEVEL_PROFILE_PEAK && hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_PROFILE_PEAK)
-			smu7_fan_ctrl_set_fan_speed_percent(hwmgr, 100);
+			smu7_fan_ctrl_set_fan_speed_pwm(hwmgr, 255);
 		else if (level != AMD_DPM_FORCED_LEVEL_PROFILE_PEAK && hwmgr->dpm_level == AMD_DPM_FORCED_LEVEL_PROFILE_PEAK)
 			smu7_fan_ctrl_reset_fan_speed_to_default(hwmgr);
 	}
@@ -4988,7 +4988,7 @@ static void smu7_set_fan_control_mode(struct pp_hwmgr *hwmgr, uint32_t mode)
 {
 	switch (mode) {
 	case AMD_FAN_CTRL_NONE:
-		smu7_fan_ctrl_set_fan_speed_percent(hwmgr, 100);
+		smu7_fan_ctrl_set_fan_speed_pwm(hwmgr, 255);
 		break;
 	case AMD_FAN_CTRL_MANUAL:
 		if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
@@ -5692,8 +5692,8 @@ static const struct pp_hwmgr_func smu7_hwmgr_funcs = {
 	.set_max_fan_rpm_output = smu7_set_max_fan_rpm_output,
 	.stop_thermal_controller = smu7_thermal_stop_thermal_controller,
 	.get_fan_speed_info = smu7_fan_ctrl_get_fan_speed_info,
-	.get_fan_speed_percent = smu7_fan_ctrl_get_fan_speed_percent,
-	.set_fan_speed_percent = smu7_fan_ctrl_set_fan_speed_percent,
+	.get_fan_speed_pwm = smu7_fan_ctrl_get_fan_speed_pwm,
+	.set_fan_speed_pwm = smu7_fan_ctrl_set_fan_speed_pwm,
 	.reset_fan_speed_to_default = smu7_fan_ctrl_reset_fan_speed_to_default,
 	.get_fan_speed_rpm = smu7_fan_ctrl_get_fan_speed_rpm,
 	.set_fan_speed_rpm = smu7_fan_ctrl_set_fan_speed_rpm,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c
index 6cfe148ed45b..a6c3610db23e 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c
@@ -51,7 +51,7 @@ int smu7_fan_ctrl_get_fan_speed_info(struct pp_hwmgr *hwmgr,
 	return 0;
 }
 
-int smu7_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
+int smu7_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t *speed)
 {
 	uint32_t duty100;
@@ -70,12 +70,9 @@ int smu7_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
 		return -EINVAL;
 
 
-	tmp64 = (uint64_t)duty * 100;
+	tmp64 = (uint64_t)duty * 255;
 	do_div(tmp64, duty100);
-	*speed = (uint32_t)tmp64;
-
-	if (*speed > 100)
-		*speed = 100;
+	*speed = MIN((uint32_t)tmp64, 255);
 
 	return 0;
 }
@@ -199,12 +196,11 @@ int smu7_fan_ctrl_stop_smc_fan_control(struct pp_hwmgr *hwmgr)
 }
 
 /**
- * smu7_fan_ctrl_set_fan_speed_percent - Set Fan Speed in percent.
+ * smu7_fan_ctrl_set_fan_speed_pwm - Set Fan Speed in PWM.
  * @hwmgr: the address of the powerplay hardware manager.
- * @speed: is the percentage value (0% - 100%) to be set.
- * Exception: Fails is the 100% setting appears to be 0.
+ * @speed: is the pwm value (0 - 255) to be set.
  */
-int smu7_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
+int smu7_fan_ctrl_set_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t speed)
 {
 	uint32_t duty100;
@@ -214,8 +210,7 @@ int smu7_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
 	if (hwmgr->thermal_controller.fanInfo.bNoFan)
 		return 0;
 
-	if (speed > 100)
-		speed = 100;
+	speed = MIN(speed, 255);
 
 	if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl))
 		smu7_fan_ctrl_stop_smc_fan_control(hwmgr);
@@ -227,7 +222,7 @@ int smu7_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
 		return -EINVAL;
 
 	tmp64 = (uint64_t)speed * duty100;
-	do_div(tmp64, 100);
+	do_div(tmp64, 255);
 	duty = (uint32_t)tmp64;
 
 	PHM_WRITE_VFPF_INDIRECT_FIELD(hwmgr->device, CGS_IND_REG__SMC,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.h
index 42c1ba0fad78..a386a437e1f0 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.h
@@ -41,10 +41,10 @@
 extern int smu7_thermal_get_temperature(struct pp_hwmgr *hwmgr);
 extern int smu7_thermal_stop_thermal_controller(struct pp_hwmgr *hwmgr);
 extern int smu7_fan_ctrl_get_fan_speed_info(struct pp_hwmgr *hwmgr, struct phm_fan_speed_info *fan_speed_info);
-extern int smu7_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr, uint32_t *speed);
+extern int smu7_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr, uint32_t *speed);
 extern int smu7_fan_ctrl_set_default_mode(struct pp_hwmgr *hwmgr);
 extern int smu7_fan_ctrl_set_static_mode(struct pp_hwmgr *hwmgr, uint32_t mode);
-extern int smu7_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr, uint32_t speed);
+extern int smu7_fan_ctrl_set_fan_speed_pwm(struct pp_hwmgr *hwmgr, uint32_t speed);
 extern int smu7_fan_ctrl_reset_fan_speed_to_default(struct pp_hwmgr *hwmgr);
 extern int smu7_thermal_ctrl_uninitialize_thermal_controller(struct pp_hwmgr *hwmgr);
 extern int smu7_fan_ctrl_set_fan_speed_rpm(struct pp_hwmgr *hwmgr, uint32_t speed);
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
index 25979106fd25..39475ba2de8e 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
@@ -4199,7 +4199,7 @@ static void vega10_set_fan_control_mode(struct pp_hwmgr *hwmgr, uint32_t mode)
 
 	switch (mode) {
 	case AMD_FAN_CTRL_NONE:
-		vega10_fan_ctrl_set_fan_speed_percent(hwmgr, 100);
+		vega10_fan_ctrl_set_fan_speed_pwm(hwmgr, 255);
 		break;
 	case AMD_FAN_CTRL_MANUAL:
 		if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl))
@@ -5523,8 +5523,8 @@ static const struct pp_hwmgr_func vega10_hwmgr_funcs = {
 	.force_dpm_level = vega10_dpm_force_dpm_level,
 	.stop_thermal_controller = vega10_thermal_stop_thermal_controller,
 	.get_fan_speed_info = vega10_fan_ctrl_get_fan_speed_info,
-	.get_fan_speed_percent = vega10_fan_ctrl_get_fan_speed_percent,
-	.set_fan_speed_percent = vega10_fan_ctrl_set_fan_speed_percent,
+	.get_fan_speed_pwm = vega10_fan_ctrl_get_fan_speed_pwm,
+	.set_fan_speed_pwm = vega10_fan_ctrl_set_fan_speed_pwm,
 	.reset_fan_speed_to_default =
 			vega10_fan_ctrl_reset_fan_speed_to_default,
 	.get_fan_speed_rpm = vega10_fan_ctrl_get_fan_speed_rpm,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
index 9b46b27bd30c..dad3e3741a4e 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
@@ -64,7 +64,7 @@ int vega10_fan_ctrl_get_fan_speed_info(struct pp_hwmgr *hwmgr,
 	return 0;
 }
 
-int vega10_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
+int vega10_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t *speed)
 {
 	uint32_t current_rpm;
@@ -78,11 +78,11 @@ int vega10_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
 
 	if (hwmgr->thermal_controller.
 			advanceFanControlParameters.usMaxFanRPM != 0)
-		percent = current_rpm * 100 /
+		percent = current_rpm * 255 /
 			hwmgr->thermal_controller.
 			advanceFanControlParameters.usMaxFanRPM;
 
-	*speed = percent > 100 ? 100 : percent;
+	*speed = MIN(percent, 255);
 
 	return 0;
 }
@@ -241,12 +241,11 @@ int vega10_fan_ctrl_stop_smc_fan_control(struct pp_hwmgr *hwmgr)
 }
 
 /**
- * vega10_fan_ctrl_set_fan_speed_percent - Set Fan Speed in percent.
+ * vega10_fan_ctrl_set_fan_speed_pwm - Set Fan Speed in PWM.
  * @hwmgr:  the address of the powerplay hardware manager.
- * @speed: is the percentage value (0% - 100%) to be set.
- * Exception: Fails is the 100% setting appears to be 0.
+ * @speed: is the percentage value (0 - 255) to be set.
  */
-int vega10_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
+int vega10_fan_ctrl_set_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t speed)
 {
 	struct amdgpu_device *adev = hwmgr->adev;
@@ -257,8 +256,7 @@ int vega10_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
 	if (hwmgr->thermal_controller.fanInfo.bNoFan)
 		return 0;
 
-	if (speed > 100)
-		speed = 100;
+	speed = MIN(speed, 255);
 
 	if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl))
 		vega10_fan_ctrl_stop_smc_fan_control(hwmgr);
@@ -270,7 +268,7 @@ int vega10_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
 		return -EINVAL;
 
 	tmp64 = (uint64_t)speed * duty100;
-	do_div(tmp64, 100);
+	do_div(tmp64, 255);
 	duty = (uint32_t)tmp64;
 
 	WREG32_SOC15(THM, 0, mmCG_FDO_CTRL0,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.h
index 4a0ede7c1f07..6850a21a2991 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.h
@@ -54,12 +54,12 @@ extern int vega10_thermal_get_temperature(struct pp_hwmgr *hwmgr);
 extern int vega10_thermal_stop_thermal_controller(struct pp_hwmgr *hwmgr);
 extern int vega10_fan_ctrl_get_fan_speed_info(struct pp_hwmgr *hwmgr,
 		struct phm_fan_speed_info *fan_speed_info);
-extern int vega10_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
+extern int vega10_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t *speed);
 extern int vega10_fan_ctrl_set_default_mode(struct pp_hwmgr *hwmgr);
 extern int vega10_fan_ctrl_set_static_mode(struct pp_hwmgr *hwmgr,
 		uint32_t mode);
-extern int vega10_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
+extern int vega10_fan_ctrl_set_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t speed);
 extern int vega10_fan_ctrl_reset_fan_speed_to_default(struct pp_hwmgr *hwmgr);
 extern int vega10_thermal_ctrl_uninitialize_thermal_controller(
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
index 0791309586c5..eab76a416b5f 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c
@@ -2769,7 +2769,7 @@ static void vega20_set_fan_control_mode(struct pp_hwmgr *hwmgr, uint32_t mode)
 {
 	switch (mode) {
 	case AMD_FAN_CTRL_NONE:
-		vega20_fan_ctrl_set_fan_speed_percent(hwmgr, 100);
+		vega20_fan_ctrl_set_fan_speed_pwm(hwmgr, 255);
 		break;
 	case AMD_FAN_CTRL_MANUAL:
 		if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl))
@@ -4409,8 +4409,8 @@ static const struct pp_hwmgr_func vega20_hwmgr_funcs = {
 	.register_irq_handlers = smu9_register_irq_handlers,
 	.disable_smc_firmware_ctf = vega20_thermal_disable_alert,
 	/* fan control related */
-	.get_fan_speed_percent = vega20_fan_ctrl_get_fan_speed_percent,
-	.set_fan_speed_percent = vega20_fan_ctrl_set_fan_speed_percent,
+	.get_fan_speed_pwm = vega20_fan_ctrl_get_fan_speed_pwm,
+	.set_fan_speed_pwm = vega20_fan_ctrl_set_fan_speed_pwm,
 	.get_fan_speed_info = vega20_fan_ctrl_get_fan_speed_info,
 	.get_fan_speed_rpm = vega20_fan_ctrl_get_fan_speed_rpm,
 	.set_fan_speed_rpm = vega20_fan_ctrl_set_fan_speed_rpm,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c
index 43d754952bd9..f4f4efdbda79 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.c
@@ -114,7 +114,7 @@ static int vega20_get_current_rpm(struct pp_hwmgr *hwmgr, uint32_t *current_rpm)
 	return 0;
 }
 
-int vega20_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
+int vega20_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t *speed)
 {
 	struct amdgpu_device *adev = hwmgr->adev;
@@ -129,17 +129,14 @@ int vega20_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
 	if (!duty100)
 		return -EINVAL;
 
-	tmp64 = (uint64_t)duty * 100;
+	tmp64 = (uint64_t)duty * 255;
 	do_div(tmp64, duty100);
-	*speed = (uint32_t)tmp64;
-
-	if (*speed > 100)
-		*speed = 100;
+	*speed = MIN((uint32_t)tmp64, 255);
 
 	return 0;
 }
 
-int vega20_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
+int vega20_fan_ctrl_set_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t speed)
 {
 	struct amdgpu_device *adev = hwmgr->adev;
@@ -147,8 +144,7 @@ int vega20_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
 	uint32_t duty;
 	uint64_t tmp64;
 
-	if (speed > 100)
-		speed = 100;
+	speed = MIN(speed, 255);
 
 	if (PP_CAP(PHM_PlatformCaps_MicrocodeFanControl))
 		vega20_fan_ctrl_stop_smc_fan_control(hwmgr);
@@ -160,7 +156,7 @@ int vega20_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
 		return -EINVAL;
 
 	tmp64 = (uint64_t)speed * duty100;
-	do_div(tmp64, 100);
+	do_div(tmp64, 255);
 	duty = (uint32_t)tmp64;
 
 	WREG32_SOC15(THM, 0, mmCG_FDO_CTRL0,
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.h b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.h
index 2d1769bbd24e..b18d09cf761e 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.h
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega20_thermal.h
@@ -56,9 +56,9 @@ extern int vega20_fan_ctrl_get_fan_speed_rpm(struct pp_hwmgr *hwmgr,
 		uint32_t *speed);
 extern int vega20_fan_ctrl_set_fan_speed_rpm(struct pp_hwmgr *hwmgr,
 		uint32_t speed);
-extern int vega20_fan_ctrl_get_fan_speed_percent(struct pp_hwmgr *hwmgr,
+extern int vega20_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t *speed);
-extern int vega20_fan_ctrl_set_fan_speed_percent(struct pp_hwmgr *hwmgr,
+extern int vega20_fan_ctrl_set_fan_speed_pwm(struct pp_hwmgr *hwmgr,
 		uint32_t speed);
 extern int vega20_fan_ctrl_stop_smc_fan_control(struct pp_hwmgr *hwmgr);
 extern int vega20_fan_ctrl_start_smc_fan_control(struct pp_hwmgr *hwmgr);
diff --git a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
index 15c0b8af376f..bdbbeb959c68 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/si_dpm.c
@@ -6539,7 +6539,7 @@ static int si_fan_ctrl_stop_smc_fan_control(struct amdgpu_device *adev)
 	}
 }
 
-static int si_dpm_get_fan_speed_percent(void *handle,
+static int si_dpm_get_fan_speed_pwm(void *handle,
 				      u32 *speed)
 {
 	u32 duty, duty100;
@@ -6555,17 +6555,14 @@ static int si_dpm_get_fan_speed_percent(void *handle,
 	if (duty100 == 0)
 		return -EINVAL;
 
-	tmp64 = (u64)duty * 100;
+	tmp64 = (u64)duty * 255;
 	do_div(tmp64, duty100);
-	*speed = (u32)tmp64;
-
-	if (*speed > 100)
-		*speed = 100;
+	*speed = MIN((u32)tmp64, 255);
 
 	return 0;
 }
 
-static int si_dpm_set_fan_speed_percent(void *handle,
+static int si_dpm_set_fan_speed_pwm(void *handle,
 				      u32 speed)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -6580,7 +6577,7 @@ static int si_dpm_set_fan_speed_percent(void *handle,
 	if (si_pi->fan_is_controlled_by_smc)
 		return -EINVAL;
 
-	if (speed > 100)
+	if (speed > 255)
 		return -EINVAL;
 
 	duty100 = (RREG32(CG_FDO_CTRL1) & FMAX_DUTY100_MASK) >> FMAX_DUTY100_SHIFT;
@@ -6589,7 +6586,7 @@ static int si_dpm_set_fan_speed_percent(void *handle,
 		return -EINVAL;
 
 	tmp64 = (u64)speed * duty100;
-	do_div(tmp64, 100);
+	do_div(tmp64, 255);
 	duty = (u32)tmp64;
 
 	tmp = RREG32(CG_FDO_CTRL0) & ~FDO_STATIC_DUTY_MASK;
@@ -8059,8 +8056,8 @@ static const struct amd_pm_funcs si_dpm_funcs = {
 	.vblank_too_short = &si_dpm_vblank_too_short,
 	.set_fan_control_mode = &si_dpm_set_fan_control_mode,
 	.get_fan_control_mode = &si_dpm_get_fan_control_mode,
-	.set_fan_speed_percent = &si_dpm_set_fan_speed_percent,
-	.get_fan_speed_percent = &si_dpm_get_fan_speed_percent,
+	.set_fan_speed_pwm = &si_dpm_set_fan_speed_pwm,
+	.get_fan_speed_pwm = &si_dpm_get_fan_speed_pwm,
 	.check_state_equal = &si_check_state_equal,
 	.get_vce_clock_state = amdgpu_get_vce_clock_state,
 	.read_sensor = &si_dpm_read_sensor,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 979ba6487577..c825330524e1 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -58,7 +58,7 @@ static int smu_handle_task(struct smu_context *smu,
 			   enum amd_pp_task task_id,
 			   bool lock_needed);
 static int smu_reset(struct smu_context *smu);
-static int smu_set_fan_speed_percent(void *handle, u32 speed);
+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_power_limit(void *handle, uint32_t limit);
 static int smu_set_fan_speed_rpm(void *handle, uint32_t speed);
@@ -410,8 +410,8 @@ static void smu_restore_dpm_user_profile(struct smu_context *smu)
 			return;
 		}
 
-		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 && smu->user_dpm_profile.fan_speed_pwm) {
+			ret = smu_set_fan_speed_pwm(smu, smu->user_dpm_profile.fan_speed_pwm);
 			if (ret)
 				dev_err(smu->adev->dev, "Failed to set manual fan speed in percent\n");
 		}
@@ -2558,7 +2558,7 @@ 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.fan_speed_percent = 0;
+		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);
 	}
@@ -2574,31 +2574,25 @@ static void smu_pp_set_fan_control_mode(void *handle, u32 value)
 }
 
 
-static int smu_get_fan_speed_percent(void *handle, u32 *speed)
+static int smu_get_fan_speed_pwm(void *handle, u32 *speed)
 {
 	struct smu_context *smu = handle;
 	int ret = 0;
-	uint32_t percent;
 
 	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
 		return -EOPNOTSUPP;
 
 	mutex_lock(&smu->mutex);
 
-	if (smu->ppt_funcs->get_fan_speed_percent) {
-		ret = smu->ppt_funcs->get_fan_speed_percent(smu, &percent);
-		if (!ret) {
-			*speed = percent > 100 ? 100 : percent;
-		}
-	}
+	if (smu->ppt_funcs->get_fan_speed_pwm)
+		ret = smu->ppt_funcs->get_fan_speed_pwm(smu, speed);
 
 	mutex_unlock(&smu->mutex);
 
-
 	return ret;
 }
 
-static int smu_set_fan_speed_percent(void *handle, u32 speed)
+static int smu_set_fan_speed_pwm(void *handle, u32 speed)
 {
 	struct smu_context *smu = handle;
 	int ret = 0;
@@ -2608,13 +2602,11 @@ static int smu_set_fan_speed_percent(void *handle, u32 speed)
 
 	mutex_lock(&smu->mutex);
 
-	if (smu->ppt_funcs->set_fan_speed_percent) {
-		if (speed > 100)
-			speed = 100;
-		ret = smu->ppt_funcs->set_fan_speed_percent(smu, speed);
+	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_percent = speed;
+			smu->user_dpm_profile.fan_speed_pwm = speed;
 		}
 	}
 
@@ -3051,8 +3043,8 @@ static const struct amd_pm_funcs swsmu_pm_funcs = {
 	/* export for sysfs */
 	.set_fan_control_mode    = smu_pp_set_fan_control_mode,
 	.get_fan_control_mode    = smu_get_fan_control_mode,
-	.set_fan_speed_percent   = smu_set_fan_speed_percent,
-	.get_fan_speed_percent   = smu_get_fan_speed_percent,
+	.set_fan_speed_pwm   = smu_set_fan_speed_pwm,
+	.get_fan_speed_pwm   = smu_get_fan_speed_pwm,
 	.force_clock_level       = smu_force_ppclk_levels,
 	.print_clock_levels      = smu_print_ppclk_levels,
 	.force_performance_level = smu_force_performance_level,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 60ce8a813f19..d090a999baf4 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -2270,7 +2270,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.print_clk_levels = arcturus_print_clk_levels,
 	.force_clk_levels = arcturus_force_clk_levels,
 	.read_sensor = arcturus_read_sensor,
-	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
+	.get_fan_speed_pwm = smu_v11_0_get_fan_speed_pwm,
 	.get_fan_speed_rpm = arcturus_get_fan_speed_rpm,
 	.get_power_profile_mode = arcturus_get_power_profile_mode,
 	.set_power_profile_mode = arcturus_set_power_profile_mode,
@@ -2316,7 +2316,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.display_clock_voltage_request = smu_v11_0_display_clock_voltage_request,
 	.get_fan_control_mode = smu_v11_0_get_fan_control_mode,
 	.set_fan_control_mode = smu_v11_0_set_fan_control_mode,
-	.set_fan_speed_percent = smu_v11_0_set_fan_speed_percent,
+	.set_fan_speed_pwm = smu_v11_0_set_fan_speed_pwm,
 	.set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm,
 	.set_xgmi_pstate = smu_v11_0_set_xgmi_pstate,
 	.gfx_off_control = smu_v11_0_gfx_off_control,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index 3cb90fe107c5..4ebf3ee16201 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -3224,7 +3224,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
 	.display_config_changed = navi10_display_config_changed,
 	.notify_smc_display_config = navi10_notify_smc_display_config,
 	.is_dpm_running = navi10_is_dpm_running,
-	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
+	.get_fan_speed_pwm = smu_v11_0_get_fan_speed_pwm,
 	.get_fan_speed_rpm = navi10_get_fan_speed_rpm,
 	.get_power_profile_mode = navi10_get_power_profile_mode,
 	.set_power_profile_mode = navi10_set_power_profile_mode,
@@ -3268,7 +3268,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
 	.display_clock_voltage_request = smu_v11_0_display_clock_voltage_request,
 	.get_fan_control_mode = smu_v11_0_get_fan_control_mode,
 	.set_fan_control_mode = smu_v11_0_set_fan_control_mode,
-	.set_fan_speed_percent = smu_v11_0_set_fan_speed_percent,
+	.set_fan_speed_pwm = smu_v11_0_set_fan_speed_pwm,
 	.set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm,
 	.set_xgmi_pstate = smu_v11_0_set_xgmi_pstate,
 	.gfx_off_control = smu_v11_0_gfx_off_control,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 513373dcaaca..1f4252f41b2a 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -3852,7 +3852,7 @@ static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
 	.display_config_changed = sienna_cichlid_display_config_changed,
 	.notify_smc_display_config = sienna_cichlid_notify_smc_display_config,
 	.is_dpm_running = sienna_cichlid_is_dpm_running,
-	.get_fan_speed_percent = smu_v11_0_get_fan_speed_percent,
+	.get_fan_speed_pwm = smu_v11_0_get_fan_speed_pwm,
 	.get_fan_speed_rpm = sienna_cichlid_get_fan_speed_rpm,
 	.get_power_profile_mode = sienna_cichlid_get_power_profile_mode,
 	.set_power_profile_mode = sienna_cichlid_set_power_profile_mode,
@@ -3896,7 +3896,7 @@ static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
 	.display_clock_voltage_request = smu_v11_0_display_clock_voltage_request,
 	.get_fan_control_mode = smu_v11_0_get_fan_control_mode,
 	.set_fan_control_mode = smu_v11_0_set_fan_control_mode,
-	.set_fan_speed_percent = smu_v11_0_set_fan_speed_percent,
+	.set_fan_speed_pwm = smu_v11_0_set_fan_speed_pwm,
 	.set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm,
 	.set_xgmi_pstate = smu_v11_0_set_xgmi_pstate,
 	.gfx_off_control = smu_v11_0_gfx_off_control,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 19334bb6a8b0..9001952442ba 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1200,14 +1200,13 @@ smu_v11_0_set_fan_static_mode(struct smu_context *smu, uint32_t mode)
 }
 
 int
-smu_v11_0_set_fan_speed_percent(struct smu_context *smu, uint32_t speed)
+smu_v11_0_set_fan_speed_pwm(struct smu_context *smu, uint32_t speed)
 {
 	struct amdgpu_device *adev = smu->adev;
 	uint32_t duty100, duty;
 	uint64_t tmp64;
 
-	if (speed > 100)
-		speed = 100;
+	speed = MIN(speed, 255);
 
 	if (smu_v11_0_auto_fan_control(smu, 0))
 		return -EINVAL;
@@ -1218,7 +1217,7 @@ smu_v11_0_set_fan_speed_percent(struct smu_context *smu, uint32_t speed)
 		return -EINVAL;
 
 	tmp64 = (uint64_t)speed * duty100;
-	do_div(tmp64, 100);
+	do_div(tmp64, 255);
 	duty = (uint32_t)tmp64;
 
 	WREG32_SOC15(THM, 0, mmCG_FDO_CTRL0,
@@ -1263,8 +1262,8 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 	return ret;
 }
 
-int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
-				    uint32_t *speed)
+int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
+				uint32_t *speed)
 {
 	struct amdgpu_device *adev = smu->adev;
 	uint32_t duty100, duty;
@@ -1276,7 +1275,7 @@ int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
 	 * report the fan speed as 0 PWM if user just requested such.
 	 */
 	if ((smu->user_dpm_profile.flags & SMU_CUSTOM_FAN_SPEED_PWM)
-	     && !smu->user_dpm_profile.fan_speed_percent) {
+	     && !smu->user_dpm_profile.fan_speed_pwm) {
 		*speed = 0;
 		return 0;
 	}
@@ -1288,12 +1287,9 @@ int smu_v11_0_get_fan_speed_percent(struct smu_context *smu,
 	if (!duty100)
 		return -EINVAL;
 
-	tmp64 = (uint64_t)duty * 100;
+	tmp64 = (uint64_t)duty * 255;
 	do_div(tmp64, duty100);
-	*speed = (uint32_t)tmp64;
-
-	if (*speed > 100)
-		*speed = 100;
+	*speed = MIN((uint32_t)tmp64, 255);
 
 	return 0;
 }
@@ -1334,7 +1330,7 @@ smu_v11_0_set_fan_control_mode(struct smu_context *smu,
 
 	switch (mode) {
 	case AMD_FAN_CTRL_NONE:
-		ret = smu_v11_0_set_fan_speed_percent(smu, 100);
+		ret = smu_v11_0_set_fan_speed_pwm(smu, 255);
 		break;
 	case AMD_FAN_CTRL_MANUAL:
 		ret = smu_v11_0_auto_fan_control(smu, 0);
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check
  2021-08-12  4:01 [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting Evan Quan
                   ` (3 preceding siblings ...)
  2021-08-12  4:01 ` [PATCH V2 5/7] drm/amd/pm: drop the unnecessary intermediate percent-based transition Evan Quan
@ 2021-08-12  4:01 ` Evan Quan
  2021-08-12  6:15   ` Lazar, Lijo
  2021-08-12  4:01 ` [PATCH V2 7/7] drm/amd/pm: correct the address of Arcturus fan related registers Evan Quan
  5 siblings, 1 reply; 17+ messages in thread
From: Evan Quan @ 2021-08-12  4:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: nils.wallmenius, Lijo.Lazar, Guchun.Chen, Evan Quan

As the fan control was guarded under manual mode before fan speed
RPM/PWM setting. Thus the extra check is totally redundant.

Change-Id: Ia9d776141ec4aa39255accbf00d7e7ed81c8424d
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
index 9001952442ba..20ece0963f51 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
@@ -1208,9 +1208,6 @@ smu_v11_0_set_fan_speed_pwm(struct smu_context *smu, uint32_t speed)
 
 	speed = MIN(speed, 255);
 
-	if (smu_v11_0_auto_fan_control(smu, 0))
-		return -EINVAL;
-
 	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
 				CG_FDO_CTRL1, FMAX_DUTY100);
 	if (!duty100)
@@ -1237,11 +1234,6 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 	 */
 	uint32_t crystal_clock_freq = 2500;
 	uint32_t tach_period;
-	int ret;
-
-	ret = smu_v11_0_auto_fan_control(smu, 0);
-	if (ret)
-		return ret;
 
 	/*
 	 * To prevent from possible overheat, some ASICs may have requirement
@@ -1257,9 +1249,7 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
 				   CG_TACH_CTRL, TARGET_PERIOD,
 				   tach_period));
 
-	ret = smu_v11_0_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC_RPM);
-
-	return ret;
+	return smu_v11_0_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC_RPM);
 }
 
 int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH V2 7/7] drm/amd/pm: correct the address of Arcturus fan related registers
  2021-08-12  4:01 [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting Evan Quan
                   ` (4 preceding siblings ...)
  2021-08-12  4:01 ` [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check Evan Quan
@ 2021-08-12  4:01 ` Evan Quan
  5 siblings, 0 replies; 17+ messages in thread
From: Evan Quan @ 2021-08-12  4:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: nils.wallmenius, Lijo.Lazar, Guchun.Chen, Evan Quan

These registers have different address from other SMU V11 ASICs.

Change-Id: Iaeb0438331eed9b0313933da25622f8e4c048fab
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
v1->v2:
  - cover the ASIC specific details in arcturus_ppt.c (Lijo)
---
 .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 138 +++++++++++++++++-
 1 file changed, 133 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index d090a999baf4..1aad6c23db4d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -81,6 +81,24 @@
 
 #define smnPCIE_ESM_CTRL			0x111003D0
 
+#define mmCG_FDO_CTRL0_ARCT			0x8B
+#define mmCG_FDO_CTRL0_ARCT_BASE_IDX		0
+
+#define mmCG_FDO_CTRL1_ARCT			0x8C
+#define mmCG_FDO_CTRL1_ARCT_BASE_IDX		0
+
+#define mmCG_FDO_CTRL2_ARCT			0x8D
+#define mmCG_FDO_CTRL2_ARCT_BASE_IDX		0
+
+#define mmCG_TACH_CTRL_ARCT			0x8E
+#define mmCG_TACH_CTRL_ARCT_BASE_IDX		0
+
+#define mmCG_TACH_STATUS_ARCT			0x8F
+#define mmCG_TACH_STATUS_ARCT_BASE_IDX		0
+
+#define mmCG_THERMAL_STATUS_ARCT		0x90
+#define mmCG_THERMAL_STATUS_ARCT_BASE_IDX	0
+
 static const struct cmn2asic_msg_mapping arcturus_message_map[SMU_MSG_MAX_COUNT] = {
 	MSG_MAP(TestMessage,			     PPSMC_MSG_TestMessage,			0),
 	MSG_MAP(GetSmuVersion,			     PPSMC_MSG_GetSmuVersion,			1),
@@ -1162,9 +1180,28 @@ static int arcturus_read_sensor(struct smu_context *smu,
 	return ret;
 }
 
+static int arcturus_set_fan_static_mode(struct smu_context *smu,
+					uint32_t mode)
+{
+	struct amdgpu_device *adev = smu->adev;
+
+	WREG32_SOC15(THM, 0, mmCG_FDO_CTRL2_ARCT,
+		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL2_ARCT),
+				   CG_FDO_CTRL2, TMIN, 0));
+	WREG32_SOC15(THM, 0, mmCG_FDO_CTRL2_ARCT,
+		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL2_ARCT),
+				   CG_FDO_CTRL2, FDO_PWM_MODE, mode));
+
+	return 0;
+}
+
 static int arcturus_get_fan_speed_rpm(struct smu_context *smu,
 				      uint32_t *speed)
 {
+	struct amdgpu_device *adev = smu->adev;
+	uint32_t crystal_clock_freq = 2500;
+	uint32_t tach_status;
+	uint64_t tmp64;
 	int ret = 0;
 
 	if (!speed)
@@ -1177,14 +1214,105 @@ static int arcturus_get_fan_speed_rpm(struct smu_context *smu,
 						    speed);
 		break;
 	default:
-		ret = smu_v11_0_get_fan_speed_rpm(smu,
-						  speed);
+		/*
+		 * For pre Sienna Cichlid ASICs, the 0 RPM may be not correctly
+		 * detected via register retrieving. To workaround this, we will
+		 * report the fan speed as 0 RPM if user just requested such.
+		 */
+		if ((smu->user_dpm_profile.flags & SMU_CUSTOM_FAN_SPEED_RPM)
+		     && !smu->user_dpm_profile.fan_speed_rpm) {
+			*speed = 0;
+			return 0;
+		}
+
+		tmp64 = (uint64_t)crystal_clock_freq * 60 * 10000;
+		tach_status = RREG32_SOC15(THM, 0, mmCG_TACH_STATUS_ARCT);
+		do_div(tmp64, tach_status);
+		*speed = (uint32_t)tmp64;
+
 		break;
 	}
 
 	return ret;
 }
 
+static int arcturus_set_fan_speed_pwm(struct smu_context *smu,
+				      uint32_t speed)
+{
+	struct amdgpu_device *adev = smu->adev;
+	uint32_t duty100, duty;
+	uint64_t tmp64;
+
+	speed = MIN(speed, 255);
+
+	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1_ARCT),
+				CG_FDO_CTRL1, FMAX_DUTY100);
+	if (!duty100)
+		return -EINVAL;
+
+	tmp64 = (uint64_t)speed * duty100;
+	do_div(tmp64, 255);
+	duty = (uint32_t)tmp64;
+
+	WREG32_SOC15(THM, 0, mmCG_FDO_CTRL0_ARCT,
+		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL0_ARCT),
+				   CG_FDO_CTRL0, FDO_STATIC_DUTY, duty));
+
+	return arcturus_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC);
+}
+
+static int arcturus_set_fan_speed_rpm(struct smu_context *smu,
+				      uint32_t speed)
+{
+	struct amdgpu_device *adev = smu->adev;
+	/*
+	 * crystal_clock_freq used for fan speed rpm calculation is
+	 * always 25Mhz. So, hardcode it as 2500(in 10K unit).
+	 */
+	uint32_t crystal_clock_freq = 2500;
+	uint32_t tach_period;
+
+	tach_period = 60 * crystal_clock_freq * 10000 / (8 * speed);
+	WREG32_SOC15(THM, 0, mmCG_TACH_CTRL_ARCT,
+		     REG_SET_FIELD(RREG32_SOC15(THM, 0, mmCG_TACH_CTRL_ARCT),
+				   CG_TACH_CTRL, TARGET_PERIOD,
+				   tach_period));
+
+	return arcturus_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC_RPM);
+}
+
+static int arcturus_get_fan_speed_pwm(struct smu_context *smu,
+				      uint32_t *speed)
+{
+	struct amdgpu_device *adev = smu->adev;
+	uint32_t duty100, duty;
+	uint64_t tmp64;
+
+	/*
+	 * For pre Sienna Cichlid ASICs, the 0 RPM may be not correctly
+	 * detected via register retrieving. To workaround this, we will
+	 * report the fan speed as 0 PWM if user just requested such.
+	 */
+	if ((smu->user_dpm_profile.flags & SMU_CUSTOM_FAN_SPEED_PWM)
+	     && !smu->user_dpm_profile.fan_speed_pwm) {
+		*speed = 0;
+		return 0;
+	}
+
+	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1_ARCT),
+				CG_FDO_CTRL1, FMAX_DUTY100);
+	duty = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_THERMAL_STATUS_ARCT),
+				CG_THERMAL_STATUS, FDO_PWM_DUTY);
+	if (!duty100)
+		return -EINVAL;
+
+	tmp64 = (uint64_t)duty * 255;
+	do_div(tmp64, duty100);
+	*speed = MIN((uint32_t)tmp64, 255);
+
+	return 0;
+}
+
 static int arcturus_get_fan_parameters(struct smu_context *smu)
 {
 	PPTable_t *pptable = smu->smu_table.driver_pptable;
@@ -2270,7 +2398,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.print_clk_levels = arcturus_print_clk_levels,
 	.force_clk_levels = arcturus_force_clk_levels,
 	.read_sensor = arcturus_read_sensor,
-	.get_fan_speed_pwm = smu_v11_0_get_fan_speed_pwm,
+	.get_fan_speed_pwm = arcturus_get_fan_speed_pwm,
 	.get_fan_speed_rpm = arcturus_get_fan_speed_rpm,
 	.get_power_profile_mode = arcturus_get_power_profile_mode,
 	.set_power_profile_mode = arcturus_set_power_profile_mode,
@@ -2316,8 +2444,8 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
 	.display_clock_voltage_request = smu_v11_0_display_clock_voltage_request,
 	.get_fan_control_mode = smu_v11_0_get_fan_control_mode,
 	.set_fan_control_mode = smu_v11_0_set_fan_control_mode,
-	.set_fan_speed_pwm = smu_v11_0_set_fan_speed_pwm,
-	.set_fan_speed_rpm = smu_v11_0_set_fan_speed_rpm,
+	.set_fan_speed_pwm = arcturus_set_fan_speed_pwm,
+	.set_fan_speed_rpm = arcturus_set_fan_speed_rpm,
 	.set_xgmi_pstate = smu_v11_0_set_xgmi_pstate,
 	.gfx_off_control = smu_v11_0_gfx_off_control,
 	.register_irq_handler = smu_v11_0_register_irq_handler,
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings
  2021-08-12  4:01 ` [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings Evan Quan
@ 2021-08-12  6:05   ` Lazar, Lijo
  2021-08-12  6:51     ` Quan, Evan
  0 siblings, 1 reply; 17+ messages in thread
From: Lazar, Lijo @ 2021-08-12  6:05 UTC (permalink / raw)
  To: Evan Quan, amd-gfx; +Cc: nils.wallmenius, Guchun.Chen



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@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.

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);
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check
  2021-08-12  4:01 ` [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check Evan Quan
@ 2021-08-12  6:15   ` Lazar, Lijo
  2021-08-12  6:46     ` Quan, Evan
  0 siblings, 1 reply; 17+ messages in thread
From: Lazar, Lijo @ 2021-08-12  6:15 UTC (permalink / raw)
  To: Evan Quan, amd-gfx; +Cc: nils.wallmenius, Guchun.Chen



On 8/12/2021 9:31 AM, Evan Quan wrote:
> As the fan control was guarded under manual mode before fan speed
> RPM/PWM setting. Thus the extra check is totally redundant.
> 
> Change-Id: Ia9d776141ec4aa39255accbf00d7e7ed81c8424d
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 12 +-----------
>   1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index 9001952442ba..20ece0963f51 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -1208,9 +1208,6 @@ smu_v11_0_set_fan_speed_pwm(struct smu_context *smu, uint32_t speed)
>   
>   	speed = MIN(speed, 255);
>   
> -	if (smu_v11_0_auto_fan_control(smu, 0))
> -		return -EINVAL;
> -

There is a FAN_CONTROL_NONE mode where it is set to full speed. Is that 
affected by this change?

Thanks,
Lijo

>   	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
>   				CG_FDO_CTRL1, FMAX_DUTY100);
>   	if (!duty100)
> @@ -1237,11 +1234,6 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
>   	 */
>   	uint32_t crystal_clock_freq = 2500;
>   	uint32_t tach_period;
> -	int ret;
> -
> -	ret = smu_v11_0_auto_fan_control(smu, 0);
> -	if (ret)
> -		return ret;
>   
>   	/*
>   	 * To prevent from possible overheat, some ASICs may have requirement
> @@ -1257,9 +1249,7 @@ int smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
>   				   CG_TACH_CTRL, TARGET_PERIOD,
>   				   tach_period));
>   
> -	ret = smu_v11_0_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC_RPM);
> -
> -	return ret;
> +	return smu_v11_0_set_fan_static_mode(smu, FDO_PWM_MODE_STATIC_RPM);
>   }
>   
>   int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check
  2021-08-12  6:15   ` Lazar, Lijo
@ 2021-08-12  6:46     ` Quan, Evan
  2021-08-12  7:38       ` Lazar, Lijo
  0 siblings, 1 reply; 17+ messages in thread
From: Quan, Evan @ 2021-08-12  6:46 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, August 12, 2021 2:16 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@amd.com>
> Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode
> check
> 
> 
> 
> On 8/12/2021 9:31 AM, Evan Quan wrote:
> > As the fan control was guarded under manual mode before fan speed
> > RPM/PWM setting. Thus the extra check is totally redundant.
> >
> > Change-Id: Ia9d776141ec4aa39255accbf00d7e7ed81c8424d
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > ---
> >   drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 12 +-----------
> >   1 file changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > index 9001952442ba..20ece0963f51 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> > @@ -1208,9 +1208,6 @@ smu_v11_0_set_fan_speed_pwm(struct
> smu_context
> > *smu, uint32_t speed)
> >
> >   	speed = MIN(speed, 255);
> >
> > -	if (smu_v11_0_auto_fan_control(smu, 0))
> > -		return -EINVAL;
> > -
> 
> There is a FAN_CONTROL_NONE mode where it is set to full speed. Is that
> affected by this change?
[Quan, Evan] This check was designed to block "auto" mode(like If it was under auto mode, these manual settings will be not permitted).
The FAN_CONTROL_NONE mode is not affected with and without this check.

BR
Evan
> 
> Thanks,
> Lijo
> 
> >   	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0,
> mmCG_FDO_CTRL1),
> >   				CG_FDO_CTRL1, FMAX_DUTY100);
> >   	if (!duty100)
> > @@ -1237,11 +1234,6 @@ int smu_v11_0_set_fan_speed_rpm(struct
> smu_context *smu,
> >   	 */
> >   	uint32_t crystal_clock_freq = 2500;
> >   	uint32_t tach_period;
> > -	int ret;
> > -
> > -	ret = smu_v11_0_auto_fan_control(smu, 0);
> > -	if (ret)
> > -		return ret;
> >
> >   	/*
> >   	 * To prevent from possible overheat, some ASICs may have
> > requirement @@ -1257,9 +1249,7 @@ int
> smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
> >   				   CG_TACH_CTRL, TARGET_PERIOD,
> >   				   tach_period));
> >
> > -	ret = smu_v11_0_set_fan_static_mode(smu,
> FDO_PWM_MODE_STATIC_RPM);
> > -
> > -	return ret;
> > +	return smu_v11_0_set_fan_static_mode(smu,
> FDO_PWM_MODE_STATIC_RPM);
> >   }
> >
> >   int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings
  2021-08-12  6:05   ` Lazar, Lijo
@ 2021-08-12  6:51     ` Quan, Evan
  2021-08-12  7:52       ` Lazar, Lijo
  0 siblings, 1 reply; 17+ messages in thread
From: Quan, Evan @ 2021-08-12  6:51 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, August 12, 2021 2:05 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@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@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?
> 
> 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);
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check
  2021-08-12  6:46     ` Quan, Evan
@ 2021-08-12  7:38       ` Lazar, Lijo
  2021-08-12  8:19         ` Quan, Evan
  0 siblings, 1 reply; 17+ messages in thread
From: Lazar, Lijo @ 2021-08-12  7:38 UTC (permalink / raw)
  To: Quan, Evan, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun



On 8/12/2021 12:16 PM, Quan, Evan wrote:
> [AMD Official Use Only]
> 
> 
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Thursday, August 12, 2021 2:16 PM
>> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@amd.com>
>> Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode
>> check
>>
>>
>>
>> On 8/12/2021 9:31 AM, Evan Quan wrote:
>>> As the fan control was guarded under manual mode before fan speed
>>> RPM/PWM setting. Thus the extra check is totally redundant.
>>>
>>> Change-Id: Ia9d776141ec4aa39255accbf00d7e7ed81c8424d
>>> Signed-off-by: Evan Quan <evan.quan@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 12 +-----------
>>>    1 file changed, 1 insertion(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>> index 9001952442ba..20ece0963f51 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>> @@ -1208,9 +1208,6 @@ smu_v11_0_set_fan_speed_pwm(struct
>> smu_context
>>> *smu, uint32_t speed)
>>>
>>>    	speed = MIN(speed, 255);
>>>
>>> -	if (smu_v11_0_auto_fan_control(smu, 0))
>>> -		return -EINVAL;
>>> -
>>
>> There is a FAN_CONTROL_NONE mode where it is set to full speed. Is that
>> affected by this change?
> [Quan, Evan] This check was designed to block "auto" mode(like If it was under auto mode, these manual settings will be not permitted).
> The FAN_CONTROL_NONE mode is not affected with and without this check.
> 

To set FAN_CONTROL_NONE, basically also need to turn off auto mode and 
manually set to 100% speed. If we take out turning off auto mode here, 
probably that part needs to be handled elsewhere.

Thanks,
Lijo

> BR
> Evan
>>
>> Thanks,
>> Lijo
>>
>>>    	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0,
>> mmCG_FDO_CTRL1),
>>>    				CG_FDO_CTRL1, FMAX_DUTY100);
>>>    	if (!duty100)
>>> @@ -1237,11 +1234,6 @@ int smu_v11_0_set_fan_speed_rpm(struct
>> smu_context *smu,
>>>    	 */
>>>    	uint32_t crystal_clock_freq = 2500;
>>>    	uint32_t tach_period;
>>> -	int ret;
>>> -
>>> -	ret = smu_v11_0_auto_fan_control(smu, 0);
>>> -	if (ret)
>>> -		return ret;
>>>
>>>    	/*
>>>    	 * To prevent from possible overheat, some ASICs may have
>>> requirement @@ -1257,9 +1249,7 @@ int
>> smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
>>>    				   CG_TACH_CTRL, TARGET_PERIOD,
>>>    				   tach_period));
>>>
>>> -	ret = smu_v11_0_set_fan_static_mode(smu,
>> FDO_PWM_MODE_STATIC_RPM);
>>> -
>>> -	return ret;
>>> +	return smu_v11_0_set_fan_static_mode(smu,
>> FDO_PWM_MODE_STATIC_RPM);
>>>    }
>>>
>>>    int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
>>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings
  2021-08-12  6:51     ` Quan, Evan
@ 2021-08-12  7:52       ` Lazar, Lijo
  2021-08-12  8:49         ` Quan, Evan
  0 siblings, 1 reply; 17+ messages in thread
From: Lazar, Lijo @ 2021-08-12  7:52 UTC (permalink / raw)
  To: Quan, Evan, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun



On 8/12/2021 12:21 PM, Quan, Evan wrote:
> [AMD Official Use Only]
> 
> 
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Thursday, August 12, 2021 2:05 PM
>> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@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@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.

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);
>>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check
  2021-08-12  7:38       ` Lazar, Lijo
@ 2021-08-12  8:19         ` Quan, Evan
  2021-08-12  9:09           ` Lazar, Lijo
  0 siblings, 1 reply; 17+ messages in thread
From: Quan, Evan @ 2021-08-12  8:19 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, August 12, 2021 3:38 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@amd.com>
> Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode
> check
> 
> 
> 
> On 8/12/2021 12:16 PM, Quan, Evan wrote:
> > [AMD Official Use Only]
> >
> >
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> >> Sent: Thursday, August 12, 2021 2:16 PM
> >> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> >> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@amd.com>
> >> Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual
> mode
> >> check
> >>
> >>
> >>
> >> On 8/12/2021 9:31 AM, Evan Quan wrote:
> >>> As the fan control was guarded under manual mode before fan speed
> >>> RPM/PWM setting. Thus the extra check is totally redundant.
> >>>
> >>> Change-Id: Ia9d776141ec4aa39255accbf00d7e7ed81c8424d
> >>> Signed-off-by: Evan Quan <evan.quan@amd.com>
> >>> ---
> >>>    drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 12 +----------
> -
> >>>    1 file changed, 1 insertion(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>> index 9001952442ba..20ece0963f51 100644
> >>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> >>> @@ -1208,9 +1208,6 @@ smu_v11_0_set_fan_speed_pwm(struct
> >> smu_context
> >>> *smu, uint32_t speed)
> >>>
> >>>    	speed = MIN(speed, 255);
> >>>
> >>> -	if (smu_v11_0_auto_fan_control(smu, 0))
> >>> -		return -EINVAL;
> >>> -
> >>
> >> There is a FAN_CONTROL_NONE mode where it is set to full speed. Is
> >> that affected by this change?
> > [Quan, Evan] This check was designed to block "auto" mode(like If it was
> under auto mode, these manual settings will be not permitted).
> > The FAN_CONTROL_NONE mode is not affected with and without this
> check.
> >
> 
> To set FAN_CONTROL_NONE, basically also need to turn off auto mode and
> manually set to 100% speed. If we take out turning off auto mode here,
> probably that part needs to be handled elsewhere.
[Quan, Evan] OK, I see.  Will add that for AMD_FAN_CTRL_NONE in smu_v11_0_set_fan_control_mode(). That's the right place for mode switching.

BR
Evan
> 
> Thanks,
> Lijo
> 
> > BR
> > Evan
> >>
> >> Thanks,
> >> Lijo
> >>
> >>>    	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0,
> >> mmCG_FDO_CTRL1),
> >>>    				CG_FDO_CTRL1, FMAX_DUTY100);
> >>>    	if (!duty100)
> >>> @@ -1237,11 +1234,6 @@ int smu_v11_0_set_fan_speed_rpm(struct
> >> smu_context *smu,
> >>>    	 */
> >>>    	uint32_t crystal_clock_freq = 2500;
> >>>    	uint32_t tach_period;
> >>> -	int ret;
> >>> -
> >>> -	ret = smu_v11_0_auto_fan_control(smu, 0);
> >>> -	if (ret)
> >>> -		return ret;
> >>>
> >>>    	/*
> >>>    	 * To prevent from possible overheat, some ASICs may have
> >>> requirement @@ -1257,9 +1249,7 @@ int
> >> smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
> >>>    				   CG_TACH_CTRL, TARGET_PERIOD,
> >>>    				   tach_period));
> >>>
> >>> -	ret = smu_v11_0_set_fan_static_mode(smu,
> >> FDO_PWM_MODE_STATIC_RPM);
> >>> -
> >>> -	return ret;
> >>> +	return smu_v11_0_set_fan_static_mode(smu,
> >> FDO_PWM_MODE_STATIC_RPM);
> >>>    }
> >>>
> >>>    int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
> >>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings
  2021-08-12  7:52       ` Lazar, Lijo
@ 2021-08-12  8:49         ` Quan, Evan
  2021-08-12  9:11           ` Lazar, Lijo
  0 siblings, 1 reply; 17+ messages in thread
From: Quan, Evan @ 2021-08-12  8:49 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, August 12, 2021 3:53 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@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@amd.com>
> >> Sent: Thursday, August 12, 2021 2:05 PM
> >> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> >> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@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@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
> 
> 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);
> >>>

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check
  2021-08-12  8:19         ` Quan, Evan
@ 2021-08-12  9:09           ` Lazar, Lijo
  0 siblings, 0 replies; 17+ messages in thread
From: Lazar, Lijo @ 2021-08-12  9:09 UTC (permalink / raw)
  To: Quan, Evan, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun



On 8/12/2021 1:49 PM, Quan, Evan wrote:
> [AMD Official Use Only]
> 
> 
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Thursday, August 12, 2021 3:38 PM
>> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@amd.com>
>> Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode
>> check
>>
>>
>>
>> On 8/12/2021 12:16 PM, Quan, Evan wrote:
>>> [AMD Official Use Only]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>> Sent: Thursday, August 12, 2021 2:16 PM
>>>> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
>>>> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@amd.com>
>>>> Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual
>> mode
>>>> check
>>>>
>>>>
>>>>
>>>> On 8/12/2021 9:31 AM, Evan Quan wrote:
>>>>> As the fan control was guarded under manual mode before fan speed
>>>>> RPM/PWM setting. Thus the extra check is totally redundant.
>>>>>
>>>>> Change-Id: Ia9d776141ec4aa39255accbf00d7e7ed81c8424d
>>>>> Signed-off-by: Evan Quan <evan.quan@amd.com>
>>>>> ---
>>>>>     drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 12 +----------
>> -
>>>>>     1 file changed, 1 insertion(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>>>> index 9001952442ba..20ece0963f51 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>>>> @@ -1208,9 +1208,6 @@ smu_v11_0_set_fan_speed_pwm(struct
>>>> smu_context
>>>>> *smu, uint32_t speed)
>>>>>
>>>>>     	speed = MIN(speed, 255);
>>>>>
>>>>> -	if (smu_v11_0_auto_fan_control(smu, 0))
>>>>> -		return -EINVAL;
>>>>> -
>>>>
>>>> There is a FAN_CONTROL_NONE mode where it is set to full speed. Is
>>>> that affected by this change?
>>> [Quan, Evan] This check was designed to block "auto" mode(like If it was
>> under auto mode, these manual settings will be not permitted).
>>> The FAN_CONTROL_NONE mode is not affected with and without this
>> check.
>>>
>>
>> To set FAN_CONTROL_NONE, basically also need to turn off auto mode and
>> manually set to 100% speed. If we take out turning off auto mode here,
>> probably that part needs to be handled elsewhere.
> [Quan, Evan] OK, I see.  Will add that for AMD_FAN_CTRL_NONE in smu_v11_0_set_fan_control_mode(). That's the right place for mode switching.
> 
Yes, that is the right place.

Thanks,
Lijo

> BR
> Evan
>>
>> Thanks,
>> Lijo
>>
>>> BR
>>> Evan
>>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>>>     	duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0,
>>>> mmCG_FDO_CTRL1),
>>>>>     				CG_FDO_CTRL1, FMAX_DUTY100);
>>>>>     	if (!duty100)
>>>>> @@ -1237,11 +1234,6 @@ int smu_v11_0_set_fan_speed_rpm(struct
>>>> smu_context *smu,
>>>>>     	 */
>>>>>     	uint32_t crystal_clock_freq = 2500;
>>>>>     	uint32_t tach_period;
>>>>> -	int ret;
>>>>> -
>>>>> -	ret = smu_v11_0_auto_fan_control(smu, 0);
>>>>> -	if (ret)
>>>>> -		return ret;
>>>>>
>>>>>     	/*
>>>>>     	 * To prevent from possible overheat, some ASICs may have
>>>>> requirement @@ -1257,9 +1249,7 @@ int
>>>> smu_v11_0_set_fan_speed_rpm(struct smu_context *smu,
>>>>>     				   CG_TACH_CTRL, TARGET_PERIOD,
>>>>>     				   tach_period));
>>>>>
>>>>> -	ret = smu_v11_0_set_fan_static_mode(smu,
>>>> FDO_PWM_MODE_STATIC_RPM);
>>>>> -
>>>>> -	return ret;
>>>>> +	return smu_v11_0_set_fan_static_mode(smu,
>>>> FDO_PWM_MODE_STATIC_RPM);
>>>>>     }
>>>>>
>>>>>     int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu,
>>>>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings
  2021-08-12  8:49         ` Quan, Evan
@ 2021-08-12  9:11           ` Lazar, Lijo
  0 siblings, 0 replies; 17+ messages in thread
From: Lazar, Lijo @ 2021-08-12  9:11 UTC (permalink / raw)
  To: Quan, Evan, amd-gfx; +Cc: nils.wallmenius, Chen, Guchun



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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-08-12  9:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12  4:01 [PATCH V2 1/7] drm/amd/pm: correct the fan speed RPM setting Evan Quan
2021-08-12  4:01 ` [PATCH V2 2/7] drm/amd/pm: record the RPM and PWM based fan speed settings Evan Quan
2021-08-12  6:05   ` Lazar, Lijo
2021-08-12  6:51     ` Quan, Evan
2021-08-12  7:52       ` Lazar, Lijo
2021-08-12  8:49         ` Quan, Evan
2021-08-12  9:11           ` Lazar, Lijo
2021-08-12  4:01 ` [PATCH V2 3/7] drm/amd/pm: correct the fan speed PWM retrieving Evan Quan
2021-08-12  4:01 ` [PATCH V2 4/7] drm/amd/pm: correct the fan speed RPM retrieving Evan Quan
2021-08-12  4:01 ` [PATCH V2 5/7] drm/amd/pm: drop the unnecessary intermediate percent-based transition Evan Quan
2021-08-12  4:01 ` [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check Evan Quan
2021-08-12  6:15   ` Lazar, Lijo
2021-08-12  6:46     ` Quan, Evan
2021-08-12  7:38       ` Lazar, Lijo
2021-08-12  8:19         ` Quan, Evan
2021-08-12  9:09           ` Lazar, Lijo
2021-08-12  4:01 ` [PATCH V2 7/7] drm/amd/pm: correct the address of Arcturus fan related registers Evan Quan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.