On 1/10/2022 11:30 AM, Evan Quan wrote:
Before we relied on the return values from the corresponding interfaces.
That is with low efficiency. And the wrong intermediate variable used
makes the fan mode stuck at manual mode which then causes overheating in
3D graphics tests.
Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
Change-Id: Ia93ccf3b929c12e6d10b50c8f3596783ac63f0e3
---
drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 23 +++++++++++++++++++++++
drivers/gpu/drm/amd/pm/amdgpu_pm.c | 20 ++++++++++----------
drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 12 ++++++++++++
3 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index 68d2e80a673b..e732418a9558 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -1547,3 +1547,26 @@ int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
return ret;
}
+
+int amdgpu_dpm_is_fan_operation_supported(struct amdgpu_device *adev,
+ enum fan_operation_id id)
+{
+ const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
+
+ switch (id) {
+ case FAN_CONTROL_MODE_RETRIEVING:
+ return pp_funcs->get_fan_control_mode ? 1 : 0;
+ case FAN_CONTROL_MODE_SETTING:
+ return pp_funcs->set_fan_control_mode ? 1 : 0;
+ case FAN_SPEED_PWM_RETRIEVING:
+ return pp_funcs->get_fan_speed_pwm ? 1 : 0;
+ case FAN_SPEED_PWM_SETTING:
+ return pp_funcs->set_fan_speed_pwm ? 1 : 0;
+ case FAN_SPEED_RPM_RETRIEVING:
+ return pp_funcs->get_fan_speed_rpm ? 1 : 0;
+ case FAN_SPEED_RPM_SETTING:
+ return pp_funcs->set_fan_speed_rpm ? 1 : 0;
+ default:
+ return 0;
+ }
+}
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index d3eab245e0fe..57721750c51a 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -3263,15 +3263,15 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
return 0;
/* mask fan attributes if we have no bindings for this asic to expose */
- if (((amdgpu_dpm_get_fan_speed_pwm(adev, &speed) == -EINVAL) &&
+ if ((!amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_PWM_RETRIEVING) &&
As per the current logic, it's really checking the hardware registers.
For ex: we could have some SKUs that have PMFW based fan control and for
some other SKUs, AIBs could be having a different cooling solution which
doesn't make use of PMFW.
attr == &sensor_dev_attr_pwm1.dev_attr.attr) || /* can't query fan */
- ((amdgpu_dpm_get_fan_control_mode(adev, &speed) == -EOPNOTSUPP) &&
+ (!amdgpu_dpm_is_fan_operation_supported(adev, FAN_CONTROL_MODE_RETRIEVING) &&
attr == &sensor_dev_attr_pwm1_enable.dev_attr.attr)) /* can't query state */
effective_mode &= ~S_IRUGO;
- if (((amdgpu_dpm_set_fan_speed_pwm(adev, speed) == -EINVAL) &&
+ if ((!amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_PWM_SETTING) &&
attr == &sensor_dev_attr_pwm1.dev_attr.attr) || /* can't manage fan */
- ((amdgpu_dpm_set_fan_control_mode(adev, speed) == -EOPNOTSUPP) &&
+ (!amdgpu_dpm_is_fan_operation_supported(adev, FAN_CONTROL_MODE_SETTING) &&
attr == &sensor_dev_attr_pwm1_enable.dev_attr.attr)) /* can't manage state */
effective_mode &= ~S_IWUSR;
@@ -3291,16 +3291,16 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
return 0;
/* hide max/min values if we can't both query and manage the fan */
- if (((amdgpu_dpm_set_fan_speed_pwm(adev, speed) == -EINVAL) &&
- (amdgpu_dpm_get_fan_speed_pwm(adev, &speed) == -EINVAL) &&
- (amdgpu_dpm_set_fan_speed_rpm(adev, speed) == -EINVAL) &&
- (amdgpu_dpm_get_fan_speed_rpm(adev, &speed) == -EINVAL)) &&
+ if ((!amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_PWM_SETTING) &&
+ !amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_PWM_RETRIEVING) &&
+ !amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_RPM_SETTING) &&
+ !amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_RPM_RETRIEVING)) &&
If this is the case, I think we should set pm.no_fan since nothing is
possible.
Thanks,
Lijo
(attr == &sensor_dev_attr_pwm1_max.dev_attr.attr ||
attr == &sensor_dev_attr_pwm1_min.dev_attr.attr))
return 0;
- if ((amdgpu_dpm_set_fan_speed_rpm(adev, speed) == -EINVAL) &&
- (amdgpu_dpm_get_fan_speed_rpm(adev, &speed) == -EINVAL) &&
+ if ((!amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_RPM_SETTING) &&
+ !amdgpu_dpm_is_fan_operation_supported(adev, FAN_SPEED_RPM_RETRIEVING)) &&
(attr == &sensor_dev_attr_fan1_max.dev_attr.attr ||
attr == &sensor_dev_attr_fan1_min.dev_attr.attr))
return 0;
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index ba857ca75392..9e18151a3c46 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -330,6 +330,16 @@ struct amdgpu_pm {
bool pp_force_state_enabled;
};
+enum fan_operation_id
+{
+ FAN_CONTROL_MODE_RETRIEVING = 0,
+ FAN_CONTROL_MODE_SETTING = 1,
+ FAN_SPEED_PWM_RETRIEVING = 2,
+ FAN_SPEED_PWM_SETTING = 3,
+ FAN_SPEED_RPM_RETRIEVING = 4,
+ FAN_SPEED_RPM_SETTING = 5,
+};
+
u32 amdgpu_dpm_get_vblank_time(struct amdgpu_device *adev);
int amdgpu_dpm_read_sensor(struct amdgpu_device *adev, enum amd_pp_sensors sensor,
void *data, uint32_t *size);
@@ -510,4 +520,6 @@ enum pp_smu_status amdgpu_dpm_get_uclk_dpm_states(struct amdgpu_device *adev,
unsigned int *num_states);
int amdgpu_dpm_get_dpm_clock_table(struct amdgpu_device *adev,
struct dpm_clocks *clock_table);
+int amdgpu_dpm_is_fan_operation_supported(struct amdgpu_device *adev,
+ enum fan_operation_id id);
#endif
