Re: [PATCH v2 1/2] platform/x86: asus-wmi: Fix inconsistent use of thermal policies

From: Hans de Goede
Date: Thu Nov 07 2024 - 04:53:20 EST


Hi,

On 7-Nov-24 1:38 AM, Armin Wolf wrote:
> When changing the thermal policy using the platform profile API,
> a Vivobook thermal policy is stored in throttle_thermal_policy_mode.
>
> However everywhere else a normal thermal policy is stored inside this
> variable, potentially confusing the platform profile.
>
> Fix this by always storing normal thermal policy values inside
> throttle_thermal_policy_mode and only do the conversion when writing
> the thermal policy to hardware. This also fixes the order in which
> throttle_thermal_policy_switch_next() steps through the thermal modes
> on Vivobook machines.
>
> Tested-by: Casey G Bowman <casey.g.bowman@xxxxxxxxx>
> Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles")
> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Regards,

Hans


> ---
> drivers/platform/x86/asus-wmi.c | 64 +++++++++++----------------------
> 1 file changed, 21 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index ab9342a01a48..ce60835d0303 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -3696,10 +3696,28 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus)
> /* Throttle thermal policy ****************************************************/
> static int throttle_thermal_policy_write(struct asus_wmi *asus)
> {
> - u8 value = asus->throttle_thermal_policy_mode;
> u32 retval;
> + u8 value;
> int err;
>
> + if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO) {
> + switch (asus->throttle_thermal_policy_mode) {
> + case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> + value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO;
> + break;
> + case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
> + value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO;
> + break;
> + case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> + value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO;
> + break;
> + default:
> + return -EINVAL;
> + }
> + } else {
> + value = asus->throttle_thermal_policy_mode;
> + }
> +
> err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev,
> value, &retval);
>
> @@ -3804,46 +3822,6 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
> static DEVICE_ATTR_RW(throttle_thermal_policy);
>
> /* Platform profile ***********************************************************/
> -static int asus_wmi_platform_profile_to_vivo(struct asus_wmi *asus, int mode)
> -{
> - bool vivo;
> -
> - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> -
> - if (vivo) {
> - switch (mode) {
> - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO;
> - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST:
> - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO;
> - case ASUS_THROTTLE_THERMAL_POLICY_SILENT:
> - return ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO;
> - }
> - }
> -
> - return mode;
> -}
> -
> -static int asus_wmi_platform_profile_mode_from_vivo(struct asus_wmi *asus, int mode)
> -{
> - bool vivo;
> -
> - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO;
> -
> - if (vivo) {
> - switch (mode) {
> - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO:
> - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT;
> - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO:
> - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST;
> - case ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO:
> - return ASUS_THROTTLE_THERMAL_POLICY_SILENT;
> - }
> - }
> -
> - return mode;
> -}
> -
> static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof,
> enum platform_profile_option *profile)
> {
> @@ -3853,7 +3831,7 @@ static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof,
> asus = container_of(pprof, struct asus_wmi, platform_profile_handler);
> tp = asus->throttle_thermal_policy_mode;
>
> - switch (asus_wmi_platform_profile_mode_from_vivo(asus, tp)) {
> + switch (tp) {
> case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT:
> *profile = PLATFORM_PROFILE_BALANCED;
> break;
> @@ -3892,7 +3870,7 @@ static int asus_wmi_platform_profile_set(struct platform_profile_handler *pprof,
> return -EOPNOTSUPP;
> }
>
> - asus->throttle_thermal_policy_mode = asus_wmi_platform_profile_to_vivo(asus, tp);
> + asus->throttle_thermal_policy_mode = tp;
> return throttle_thermal_policy_write(asus);
> }
>
> --
> 2.39.5
>