linux-kernel - Re: [PATCH v6 4/5] alienware-wmi: added autodetect_thermal_profile for devices with quirk

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d30b2ce-68e4-4080-9068-d1c0b5a59284@gmx.de>
Date: Sun, 20 Oct 2024 22:39:09 +0200
From: Armin Wolf <W_Armin@....de>
To: Kurt Borja <kuurtb@...il.com>
Cc: hdegoede@...hat.com, ilpo.jarvinen@...ux.intel.com,
 linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v6 4/5] alienware-wmi: added autodetect_thermal_profile
 for devices with quirk_unknown

Am 17.10.24 um 10:16 schrieb Kurt Borja:

> Added autodetect_thermal_profile for devices with quirk_unknown.
> Autodetection is done through basic conditions most devices with WMAX's
> thermal interface meet. Function exits returning 0 in case of errors.
>
> Signed-off-by: Kurt Borja <kuurtb@...il.com>
>
> ---
> I apologize for the late inclusion. This feature can extend support to
> many devices without having to list them in alienware_quirks.
>
> The conditions for selecting the automatic thermal profile are based on
> observations on a lot of *issues* in AWCC open source alternatives.
>
> I observed only Dell's G-Series laptops have WMAX_THERMAL_BALANCED
> avaliable and when it's present none of the other profiles are
> avaliable, except for GMODE. When a model has USTT profiles avaliable
> usually they have all USTT profiles avaliable, except for cool on mostly
> Alienware devices.
>
> I made another implementation of this function, brute-forcing operation
> 0x03 of Thermal_Information, which is the operation that varies the most
> across models. I found the implementation too cumbersome to include in
> this series, but it could potentially extend support of this driver to
> all posible devices with this interface automatically.

I like this patch, automatic configuration is always a nice feature.

Please add support for operation 0x03, this way the driver can work automatically
without users having to submit patches adding quirks for their machines.

Maybe you can use an array for storing the supported thermal mode ids, like this:

enum thermal_modes = {
	THERMAL_MODE_QUIET,
	...
	THERMAL_MODE_LOW_POWER,
	THERMAL_MODE_MAX
};

const enumplatform_profile_option  thermal_mode_to_platform_profile[THERMAL_MODE_MAX] = {
	[THERMAL_MODE_QUIET] =PLATFORM_PROFILE_QUIET, ...
};

const enumthermal_modes platform_profile_to_thermal_mode[PLATFORM_PROFILE_LAST] = {
	[PLATFORM_PROFILE_LOW_POWER] = THERMAL_MODE_LOW_POWER, ...
};


u8 thermal_modes[THERMAL_MODE_MAX] = {};

for (int i = 0; i < THERMAL_MODE_MAX; i++) {
	thermal_modes[i] = call_operation_3(0x06 + i);
	// TODO: Error handling
	if (thermal_modes[i] == 0xFFFFFFFF)
		continue;

	set_bit(supported_profiles, thermal_mode_to_platform_profile[i]);
}

then you can use platform_profile_to_thermal_mode[] when setting the platform profile
and thermal_mode_to_platform_profile[] when getting the platform profile.
I will leave it up to you on how to handle the existence of GMode.

This of course is only a rough idea, you can change anything you want in the above pseudo-code.

Thanks,
Armin Wolf

>
> Another possibility is just including every device I observed into
> alienware_quirks, which I can do but I want to know your opinion first.
> ---
>   drivers/platform/x86/dell/alienware-wmi.c | 42 +++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
> index 37a898273..a11ff4851 100644
> --- a/drivers/platform/x86/dell/alienware-wmi.c
> +++ b/drivers/platform/x86/dell/alienware-wmi.c
> @@ -30,8 +30,11 @@
>   #define WMAX_METHOD_DEEP_SLEEP_STATUS	0x0C
>   #define WMAX_METHOD_THERMAL_INFORMATION	0x14
>   #define WMAX_METHOD_THERMAL_CONTROL	0x15
> +#define WMAX_METHOD_GMODE_STATUS	0x25
>
> +#define WMAX_ARG_GET_DEFAULT_PROF	0x0A
>   #define WMAX_ARG_GET_CURRENT_PROF	0x0B
> +#define WMAX_ARG_GET_GMODE_STATUS	0x02
>
>   #define WMAX_FAILURE_CODE		0xFFFFFFFF
>
> @@ -968,6 +971,42 @@ static int thermal_profile_set_ustt(struct platform_profile_handler *pprof,
>   	return 0;
>   }
>
> +static int autodetect_thermal_profile(void)
> +{
> +	acpi_status status;
> +	u32 in_args;
> +	u32 default_profile;
> +	u32 gmode;
> +
> +	in_args = WMAX_ARG_GET_DEFAULT_PROF;
> +	status = alienware_wmax_command(&in_args, sizeof(in_args),
> +					WMAX_METHOD_THERMAL_INFORMATION, &default_profile);
> +
> +	if (ACPI_FAILURE(status))
> +		return 0;
> +
> +	in_args = WMAX_ARG_GET_GMODE_STATUS;
> +	status = alienware_wmax_command(&in_args, sizeof(in_args),
> +					WMAX_METHOD_GMODE_STATUS, &gmode);
> +
> +	if (ACPI_FAILURE(status))
> +		return 0;
> +
> +	if (default_profile == WMAX_THERMAL_BALANCED && gmode == 1) {
> +		quirks->thermal = WMAX_THERMAL_TABLE_SIMPLE;
> +		quirks->gmode = 1;
> +		return 0;
> +	}
> +
> +	if (default_profile == WMAX_THERMAL_USTT_BALANCED)
> +		quirks->thermal = WMAX_THERMAL_TABLE_USTT;
> +
> +	if (gmode == 0 || gmode == 1)
> +		quirks->gmode = 1;
> +
> +	return 0;
> +}
> +
>   static int create_thermal_profile(void)
>   {
>   	pp_handler.profile_get = thermal_profile_get;
> @@ -1050,6 +1089,9 @@ static int __init alienware_wmi_init(void)
>   			goto fail_prep_deepsleep;
>   	}
>
> +	if (interface == WMAX && quirks == &quirk_unknown)
> +		autodetect_thermal_profile();
> +
>   	if (quirks->thermal > 0) {
>   		ret = create_thermal_profile();
>   		if (ret)