| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <D7T8UQ8PJ7BH.3DGKE2REW8025@gmail.com>
Date: Sat, 15 Feb 2025 13:51:14 -0500
From: "Kurt Borja" <kuurtb@...il.com>
To: "Armin Wolf" <W_Armin@....de>, <jlee@...e.com>,
<basak.sb2006@...il.com>, <rayanmargham4@...il.com>
Cc: <hdegoede@...hat.com>, <ilpo.jarvinen@...ux.intel.com>,
<platform-driver-x86@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v2 1/3] platform/x86: acer-wmi: Fix setting of fan
behavior
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@....de>
> ---
> 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@...il.com>
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
Powered by blists - more mailing lists