On Sat Feb 15, 2025 at 12:45 PM -05, Armin Wolf wrote: > After studying the linuwu_sense driver > (https://github.com/0x7375646F/Linuwu-Sense) i was able to understand > the meaning of the SetGamingFanBehavior() WMI method: > > - the first 16-bit are a bitmap of all fans affected by a fan behavior > change request. > > - the next 8 bits contain four fan mode fields (2-bit), each being > associated with a bit inside the fan bitmap. > > There are three fan modes: auto, turbo and custom. > > Use this newfound knowledge to fix the turbo fan handling by setting > the correct bits before calling SetGamingFanBehavior(). Also check > the result of the WMI method call and return an error should the ACPI > firmware signal failure. > > Signed-off-by: Armin Wolf <W_Armin@xxxxxx> > --- > drivers/platform/x86/acer-wmi.c | 75 +++++++++++++++++++++++---------- > 1 file changed, 52 insertions(+), 23 deletions(-) > > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c > index 69336bd778ee..f20a882e3650 100644 > --- a/drivers/platform/x86/acer-wmi.c > +++ b/drivers/platform/x86/acer-wmi.c > @@ -68,10 +68,19 @@ MODULE_LICENSE("GPL"); > #define ACER_WMID_SET_GAMING_LED_METHODID 2 > #define ACER_WMID_GET_GAMING_LED_METHODID 4 > #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5 > -#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14 > +#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14 > #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22 > #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23 > > +#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0) > +#define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16) > + > +#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0) > +#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3) > + > +#define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0) > +#define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6) > + > #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0) > #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0) > #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8) > @@ -121,6 +130,12 @@ enum acer_wmi_predator_v4_sensor_id { > ACER_WMID_SENSOR_GPU_TEMPERATURE = 0x0A, > }; > > +enum acer_wmi_gaming_fan_mode { > + ACER_WMID_FAN_MODE_AUTO = 0x01, > + ACER_WMID_FAN_MODE_TURBO = 0x02, > + ACER_WMID_FAN_MODE_CUSTOM = 0x03, > +}; > + > enum acer_wmi_predator_v4_oc { > ACER_WMID_OC_NORMAL = 0x0000, > ACER_WMID_OC_TURBO = 0x0002, > @@ -1565,9 +1580,6 @@ static acpi_status WMID_gaming_set_u64(u64 value, u32 cap) > case ACER_CAP_TURBO_LED: > method_id = ACER_WMID_SET_GAMING_LED_METHODID; > break; > - case ACER_CAP_TURBO_FAN: > - method_id = ACER_WMID_SET_GAMING_FAN_BEHAVIOR; > - break; > default: > return AE_BAD_PARAMETER; > } > @@ -1618,25 +1630,42 @@ static int WMID_gaming_get_sys_info(u32 command, u64 *out) > return 0; > } > > +static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap) > +{ > + acpi_status status; > + u64 input = 0; > + u64 result; > + > + input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap); > + input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK, mode_bitmap); > + > + status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID, input, > + &result); > + if (ACPI_FAILURE(status)) > + return -EIO; > + > + /* TODO: Proper error handling */ > + pr_notice("Fan behavior return status: %llu\n", result); I guess this is missing some ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK handling right? This shouldn't mess with testing tho. > + > + return 0; > +} > + > static void WMID_gaming_set_fan_mode(u8 fan_mode) > { > - /* fan_mode = 1 is used for auto, fan_mode = 2 used for turbo*/ > - u64 gpu_fan_config1 = 0, gpu_fan_config2 = 0; > - int i; > - > - if (quirks->cpu_fans > 0) > - gpu_fan_config2 |= 1; > - for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i) > - gpu_fan_config2 |= 1 << (i + 1); I agree on with your explaination in the previous thread, so after the TODO is addressed: Reviewed-by: Kurt Borja <kuurtb@xxxxxxxxx> I do wonder tho, isn't there a WMI operation to get the bitmap of available fans? Like in the case of available thermal profiles and sensors. -- ~ Kurt > - for (i = 0; i < quirks->gpu_fans; ++i) > - gpu_fan_config2 |= 1 << (i + 3); > - if (quirks->cpu_fans > 0) > - gpu_fan_config1 |= fan_mode; > - for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i) > - gpu_fan_config1 |= fan_mode << (2 * i + 2); > - for (i = 0; i < quirks->gpu_fans; ++i) > - gpu_fan_config1 |= fan_mode << (2 * i + 6); > - WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN); > + u16 mode_bitmap = 0; > + u16 fan_bitmap = 0; > + > + if (quirks->cpu_fans > 0) { > + fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_CPU; > + mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK, fan_mode); > + } > + > + if (quirks->gpu_fans > 0) { > + fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_GPU; > + mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK, fan_mode); > + } > + > + WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap); > } > > static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value) > @@ -1923,7 +1952,7 @@ static int acer_toggle_turbo(void) > WMID_gaming_set_u64(0x1, ACER_CAP_TURBO_LED); > > /* Set FAN mode to auto */ > - WMID_gaming_set_fan_mode(0x1); > + WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_AUTO); > > /* Set OC to normal */ > if (has_cap(ACER_CAP_TURBO_OC)) { > @@ -1937,7 +1966,7 @@ static int acer_toggle_turbo(void) > WMID_gaming_set_u64(0x10001, ACER_CAP_TURBO_LED); > > /* Set FAN mode to turbo */ > - WMID_gaming_set_fan_mode(0x2); > + WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_TURBO); > > /* Set OC to turbo mode */ > if (has_cap(ACER_CAP_TURBO_OC)) { > -- > 2.39.5