Re: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm
From: Zhu, Rex Sent: Thursday, September 20, 2018 10:43 AM To: Deucher, Alexander; amd-gfx@lists.freedesktop.org; Quan, Evan Subject: RE: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm Hi Alex and Evan, For the fan control via sysfs, I think we need to clarify the use case. We support manual/auto fan control mode. User can set the mode through pwm_enable. 1 mean manual. 2 mean auto User can set fan speed via pwm1 and fan1_input. For pwm1, user set the percentage value (0% - 100%) And can get the pwm1’s range via sysfs: pwm1_min, pwm1_max. the range is [0, 255]. In driver, we transfer to [0% - 100%] For fan1_input, user set the fan’s resolution per minute No way for user to get the range. On Tonga, the range is (0, 6000]. Not support zero-rpm on tonga. Do we need to add new sysfs to expose the RPM range or just print the range in dmesg if user’s setting is out of range? Yes, expose the rpm limits via fan1_min and max. See: https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface for more info on standard sysfs interfaces for hwmon. Another question is: Currently, the default fan control mode is auto. When user change the fan speed via pwm1 or fan1_input, we switch to manual mode automatically. So if user want to change back to auto fan control mode, they need to echo 2 to pwm_enable. I think we should reject any changes unless the user selects manual (1) first. Best Regards Rex From: Deucher, Alexander Sent: Thursday, September 20, 2018 9:52 PM To: Zhu, Rex ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm Series is: Reviewed-by: Alex Deucher mailto:alexander.deuc...@amd.com>> From: amd-gfx mailto:amd-gfx-boun...@lists.freedesktop.org>> on behalf of Rex Zhu mailto:rex@amd.com>> Sent: Thursday, September 20, 2018 3:14:25 AM To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Cc: Zhu, Rex Subject: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm The minRPM speed maybe equal to zero. so need to check input RPM not equal to 0, otherwise cause divide-by-zero driver crash. Signed-off-by: Rex Zhu mailto:rex@amd.com>> --- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c index 44527755..d61a9b4 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c @@ -260,6 +260,7 @@ int smu7_fan_ctrl_set_fan_speed_rpm(struct pp_hwmgr *hwmgr, uint32_t speed) if (hwmgr->thermal_controller.fanInfo.bNoFan || (hwmgr->thermal_controller.fanInfo. ucTachometerPulsesPerRevolution == 0) || + speed == 0 || (speed < hwmgr->thermal_controller.fanInfo.ulMinRPM) || (speed > hwmgr->thermal_controller.fanInfo.ulMaxRPM)) return 0; -- 1.9.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm
Hi Alex and Evan, For the fan control via sysfs, I think we need to clarify the use case. We support manual/auto fan control mode. User can set the mode through pwm_enable. 1 mean manual. 2 mean auto User can set fan speed via pwm1 and fan1_input. For pwm1, user set the percentage value (0% - 100%) And can get the pwm1's range via sysfs: pwm1_min, pwm1_max. the range is [0, 255]. In driver, we transfer to [0% - 100%] For fan1_input, user set the fan's resolution per minute No way for user to get the range. On Tonga, the range is (0, 6000]. Not support zero-rpm on tonga. Do we need to add new sysfs to expose the RPM range or just print the range in dmesg if user's setting is out of range? Another question is: Currently, the default fan control mode is auto. When user change the fan speed via pwm1 or fan1_input, we switch to manual mode automatically. So if user want to change back to auto fan control mode, they need to echo 2 to pwm_enable. Best Regards Rex From: Deucher, Alexander Sent: Thursday, September 20, 2018 9:52 PM To: Zhu, Rex ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm Series is: Reviewed-by: Alex Deucher mailto:alexander.deuc...@amd.com>> From: amd-gfx mailto:amd-gfx-boun...@lists.freedesktop.org>> on behalf of Rex Zhu mailto:rex@amd.com>> Sent: Thursday, September 20, 2018 3:14:25 AM To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Cc: Zhu, Rex Subject: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm The minRPM speed maybe equal to zero. so need to check input RPM not equal to 0, otherwise cause divide-by-zero driver crash. Signed-off-by: Rex Zhu mailto:rex@amd.com>> --- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c index 44527755..d61a9b4 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c @@ -260,6 +260,7 @@ int smu7_fan_ctrl_set_fan_speed_rpm(struct pp_hwmgr *hwmgr, uint32_t speed) if (hwmgr->thermal_controller.fanInfo.bNoFan || (hwmgr->thermal_controller.fanInfo. ucTachometerPulsesPerRevolution == 0) || + speed == 0 || (speed < hwmgr->thermal_controller.fanInfo.ulMinRPM) || (speed > hwmgr->thermal_controller.fanInfo.ulMaxRPM)) return 0; -- 1.9.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm
Series is: Reviewed-by: Alex Deucher From: amd-gfx on behalf of Rex Zhu Sent: Thursday, September 20, 2018 3:14:25 AM To: amd-gfx@lists.freedesktop.org Cc: Zhu, Rex Subject: [PATCH 1/3] drm/amd/pp: Avoid divide-by-zero in smu7_fan_ctrl_set_fan_speed_rpm The minRPM speed maybe equal to zero. so need to check input RPM not equal to 0, otherwise cause divide-by-zero driver crash. Signed-off-by: Rex Zhu --- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c index 44527755..d61a9b4 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c @@ -260,6 +260,7 @@ int smu7_fan_ctrl_set_fan_speed_rpm(struct pp_hwmgr *hwmgr, uint32_t speed) if (hwmgr->thermal_controller.fanInfo.bNoFan || (hwmgr->thermal_controller.fanInfo. ucTachometerPulsesPerRevolution == 0) || + speed == 0 || (speed < hwmgr->thermal_controller.fanInfo.ulMinRPM) || (speed > hwmgr->thermal_controller.fanInfo.ulMaxRPM)) return 0; -- 1.9.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
