* [PATCH v7 1/4] alienware-wmi: fixed indentation and clean up
2024-10-24 6:15 [PATCH v7 0/4] Dell AWCC platform_profile support Kurt Borja
@ 2024-10-24 6:17 ` Kurt Borja
2024-10-24 6:18 ` [PATCH v7 2/4] alienware-wmi: alienware_wmax_command() is now input size agnostic Kurt Borja
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Kurt Borja @ 2024-10-24 6:17 UTC (permalink / raw)
To: kuurtb; +Cc: W_Armin, hdegoede, ilpo.jarvinen, linux-kernel,
platform-driver-x86
Fixed inconsistent indentation and removed unnecessary (acpi_size) and
(u32 *) casts.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
---
v7:
- Unchanged
v6:
- Unchanged
---
drivers/platform/x86/dell/alienware-wmi.c | 134 +++++++++++-----------
1 file changed, 67 insertions(+), 67 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index f5ee62ce1..16a3fe9ac 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -116,68 +116,68 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
static const struct dmi_system_id alienware_quirks[] __initconst = {
{
- .callback = dmi_matched,
- .ident = "Alienware X51 R3",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51 R3"),
- },
- .driver_data = &quirk_x51_r3,
- },
+ .callback = dmi_matched,
+ .ident = "Alienware X51 R3",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51 R3"),
+ },
+ .driver_data = &quirk_x51_r3,
+ },
{
- .callback = dmi_matched,
- .ident = "Alienware X51 R2",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51 R2"),
- },
- .driver_data = &quirk_x51_r1_r2,
- },
+ .callback = dmi_matched,
+ .ident = "Alienware X51 R2",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51 R2"),
+ },
+ .driver_data = &quirk_x51_r1_r2,
+ },
{
- .callback = dmi_matched,
- .ident = "Alienware X51 R1",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
- DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51"),
- },
- .driver_data = &quirk_x51_r1_r2,
- },
+ .callback = dmi_matched,
+ .ident = "Alienware X51 R1",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Alienware X51"),
+ },
+ .driver_data = &quirk_x51_r1_r2,
+ },
{
- .callback = dmi_matched,
- .ident = "Alienware ASM100",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
- DMI_MATCH(DMI_PRODUCT_NAME, "ASM100"),
- },
- .driver_data = &quirk_asm100,
- },
+ .callback = dmi_matched,
+ .ident = "Alienware ASM100",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ASM100"),
+ },
+ .driver_data = &quirk_asm100,
+ },
{
- .callback = dmi_matched,
- .ident = "Alienware ASM200",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
- DMI_MATCH(DMI_PRODUCT_NAME, "ASM200"),
- },
- .driver_data = &quirk_asm200,
- },
+ .callback = dmi_matched,
+ .ident = "Alienware ASM200",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ASM200"),
+ },
+ .driver_data = &quirk_asm200,
+ },
{
- .callback = dmi_matched,
- .ident = "Alienware ASM201",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
- DMI_MATCH(DMI_PRODUCT_NAME, "ASM201"),
- },
- .driver_data = &quirk_asm201,
- },
- {
- .callback = dmi_matched,
- .ident = "Dell Inc. Inspiron 5675",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
- DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 5675"),
- },
- .driver_data = &quirk_inspiron5675,
- },
+ .callback = dmi_matched,
+ .ident = "Alienware ASM201",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "ASM201"),
+ },
+ .driver_data = &quirk_asm201,
+ },
+ {
+ .callback = dmi_matched,
+ .ident = "Dell Inc. Inspiron 5675",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 5675"),
+ },
+ .driver_data = &quirk_inspiron5675,
+ },
{}
};
@@ -221,8 +221,8 @@ static struct platform_zone *zone_data;
static struct platform_driver platform_driver = {
.driver = {
- .name = "alienware-wmi",
- }
+ .name = "alienware-wmi",
+ }
};
static struct attribute_group zone_attribute_group = {
@@ -292,7 +292,7 @@ static int alienware_update_led(struct platform_zone *zone)
guid = WMAX_CONTROL_GUID;
method_id = WMAX_METHOD_ZONE_CONTROL;
- input.length = (acpi_size) sizeof(wmax_basic_args);
+ input.length = sizeof(wmax_basic_args);
input.pointer = &wmax_basic_args;
} else {
legacy_args.colors = zone->colors;
@@ -306,7 +306,7 @@ static int alienware_update_led(struct platform_zone *zone)
guid = LEGACY_CONTROL_GUID;
method_id = zone->location + 1;
- input.length = (acpi_size) sizeof(legacy_args);
+ input.length = sizeof(legacy_args);
input.pointer = &legacy_args;
}
pr_debug("alienware-wmi: guid %s method %d\n", guid, method_id);
@@ -358,7 +358,7 @@ static int wmax_brightness(int brightness)
.led_mask = 0xFF,
.percentage = brightness,
};
- input.length = (acpi_size) sizeof(args);
+ input.length = sizeof(args);
input.pointer = &args;
status = wmi_evaluate_method(WMAX_CONTROL_GUID, 0,
WMAX_METHOD_BRIGHTNESS, &input, NULL);
@@ -508,7 +508,7 @@ static acpi_status alienware_wmax_command(struct wmax_basic_args *in_args,
struct acpi_buffer input;
struct acpi_buffer output;
- input.length = (acpi_size) sizeof(*in_args);
+ input.length = sizeof(*in_args);
input.pointer = in_args;
if (out_data) {
output.length = ACPI_ALLOCATE_BUFFER;
@@ -542,7 +542,7 @@ static ssize_t show_hdmi_cable(struct device *dev,
};
status =
alienware_wmax_command(&in_args, WMAX_METHOD_HDMI_CABLE,
- (u32 *) &out_data);
+ &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 0)
return sysfs_emit(buf, "[unconnected] connected unknown\n");
@@ -563,7 +563,7 @@ static ssize_t show_hdmi_source(struct device *dev,
};
status =
alienware_wmax_command(&in_args, WMAX_METHOD_HDMI_STATUS,
- (u32 *) &out_data);
+ &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 1)
@@ -643,7 +643,7 @@ static ssize_t show_amplifier_status(struct device *dev,
};
status =
alienware_wmax_command(&in_args, WMAX_METHOD_AMPLIFIER_CABLE,
- (u32 *) &out_data);
+ &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 0)
return sysfs_emit(buf, "[unconnected] connected unknown\n");
@@ -695,7 +695,7 @@ static ssize_t show_deepsleep_status(struct device *dev,
.arg = 0,
};
status = alienware_wmax_command(&in_args, WMAX_METHOD_DEEP_SLEEP_STATUS,
- (u32 *) &out_data);
+ &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 0)
return sysfs_emit(buf, "[disabled] s5 s5_s4\n");
--
2.47.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 2/4] alienware-wmi: alienware_wmax_command() is now input size agnostic
2024-10-24 6:15 [PATCH v7 0/4] Dell AWCC platform_profile support Kurt Borja
2024-10-24 6:17 ` [PATCH v7 1/4] alienware-wmi: fixed indentation and clean up Kurt Borja
@ 2024-10-24 6:18 ` Kurt Borja
2024-10-24 6:19 ` [PATCH v7 3/4] alienware-wmi: added platform profile support Kurt Borja
2024-10-24 6:21 ` [PATCH v7 4/4] alienware-wmi: WMAX interface documentation Kurt Borja
3 siblings, 0 replies; 7+ messages in thread
From: Kurt Borja @ 2024-10-24 6:18 UTC (permalink / raw)
To: kuurtb; +Cc: W_Armin, hdegoede, ilpo.jarvinen, linux-kernel,
platform-driver-x86
alienware_wmax_command() now takes void * and size_t instead of struct
wmax_basic_args to extend support to new WMAX methods. Also int *out_data
was changed to u32 *out_data, because new interface specifies u32 as output
parameter and all previous callers would pass u32 * regardless.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
---
v7:
- Unchanged
v6:
- Unchanged
---
drivers/platform/x86/dell/alienware-wmi.c | 29 ++++++++++++-----------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index 16a3fe9ac..b27f3b64c 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -500,15 +500,15 @@ static void alienware_zone_exit(struct platform_device *dev)
kfree(zone_attrs);
}
-static acpi_status alienware_wmax_command(struct wmax_basic_args *in_args,
- u32 command, int *out_data)
+static acpi_status alienware_wmax_command(void *in_args, size_t in_size,
+ u32 command, u32 *out_data)
{
acpi_status status;
union acpi_object *obj;
struct acpi_buffer input;
struct acpi_buffer output;
- input.length = sizeof(*in_args);
+ input.length = in_size;
input.pointer = in_args;
if (out_data) {
output.length = ACPI_ALLOCATE_BUFFER;
@@ -541,8 +541,8 @@ static ssize_t show_hdmi_cable(struct device *dev,
.arg = 0,
};
status =
- alienware_wmax_command(&in_args, WMAX_METHOD_HDMI_CABLE,
- &out_data);
+ alienware_wmax_command(&in_args, sizeof(in_args),
+ WMAX_METHOD_HDMI_CABLE, &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 0)
return sysfs_emit(buf, "[unconnected] connected unknown\n");
@@ -562,8 +562,8 @@ static ssize_t show_hdmi_source(struct device *dev,
.arg = 0,
};
status =
- alienware_wmax_command(&in_args, WMAX_METHOD_HDMI_STATUS,
- &out_data);
+ alienware_wmax_command(&in_args, sizeof(in_args),
+ WMAX_METHOD_HDMI_STATUS, &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 1)
@@ -589,7 +589,8 @@ static ssize_t toggle_hdmi_source(struct device *dev,
args.arg = 3;
pr_debug("alienware-wmi: setting hdmi to %d : %s", args.arg, buf);
- status = alienware_wmax_command(&args, WMAX_METHOD_HDMI_SOURCE, NULL);
+ status = alienware_wmax_command(&args, sizeof(args),
+ WMAX_METHOD_HDMI_SOURCE, NULL);
if (ACPI_FAILURE(status))
pr_err("alienware-wmi: HDMI toggle failed: results: %u\n",
@@ -642,8 +643,8 @@ static ssize_t show_amplifier_status(struct device *dev,
.arg = 0,
};
status =
- alienware_wmax_command(&in_args, WMAX_METHOD_AMPLIFIER_CABLE,
- &out_data);
+ alienware_wmax_command(&in_args, sizeof(in_args),
+ WMAX_METHOD_AMPLIFIER_CABLE, &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 0)
return sysfs_emit(buf, "[unconnected] connected unknown\n");
@@ -694,8 +695,8 @@ static ssize_t show_deepsleep_status(struct device *dev,
struct wmax_basic_args in_args = {
.arg = 0,
};
- status = alienware_wmax_command(&in_args, WMAX_METHOD_DEEP_SLEEP_STATUS,
- &out_data);
+ status = alienware_wmax_command(&in_args, sizeof(in_args),
+ WMAX_METHOD_DEEP_SLEEP_STATUS, &out_data);
if (ACPI_SUCCESS(status)) {
if (out_data == 0)
return sysfs_emit(buf, "[disabled] s5 s5_s4\n");
@@ -723,8 +724,8 @@ static ssize_t toggle_deepsleep(struct device *dev,
args.arg = 2;
pr_debug("alienware-wmi: setting deep sleep to %d : %s", args.arg, buf);
- status = alienware_wmax_command(&args, WMAX_METHOD_DEEP_SLEEP_CONTROL,
- NULL);
+ status = alienware_wmax_command(&args, sizeof(args),
+ WMAX_METHOD_DEEP_SLEEP_CONTROL, NULL);
if (ACPI_FAILURE(status))
pr_err("alienware-wmi: deep sleep control failed: results: %u\n",
--
2.47.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 3/4] alienware-wmi: added platform profile support
2024-10-24 6:15 [PATCH v7 0/4] Dell AWCC platform_profile support Kurt Borja
2024-10-24 6:17 ` [PATCH v7 1/4] alienware-wmi: fixed indentation and clean up Kurt Borja
2024-10-24 6:18 ` [PATCH v7 2/4] alienware-wmi: alienware_wmax_command() is now input size agnostic Kurt Borja
@ 2024-10-24 6:19 ` Kurt Borja
2024-10-24 7:39 ` Ilpo Järvinen
2024-10-24 6:21 ` [PATCH v7 4/4] alienware-wmi: WMAX interface documentation Kurt Borja
3 siblings, 1 reply; 7+ messages in thread
From: Kurt Borja @ 2024-10-24 6:19 UTC (permalink / raw)
To: kuurtb; +Cc: W_Armin, hdegoede, ilpo.jarvinen, linux-kernel,
platform-driver-x86
Implements platform profile support for Dell laptops with new WMAX thermal
interface, present on some Alienware X-Series, Alienware M-Series and
Dell's G-Series laptops.
This implementation automatically detects available thermal profiles and
GMODE + Game Shift availability, which is a feature of G-Series laptops.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
v7:
- Method operations are now clearly listed as separate enums
- wmax_thermal_modes are now listed without codes in order to support
autodetection, as well as getting and setting thermal profiles
cleanly through arrays
- Added wmax_mode_to_platform_profile[]
- Added struct wmax_u32_args to replace bit mask approach of
constructing arguments for wmax methods
- create_thermal_profile now autodetects available thermal codes
through operation 0x03 of THERMAL_INFORMATION method. These are
codes are stored in supported_thermal_profiles[]
- thermal_profile_get now uses wmax_mode_to_platform_profile[] instead of
switch-case approach
- thermal_profile_set now uses supported_thermal_profiles[] instead of
switch-case approach
- When gmode is autodetected, thermal_profile_set also sets Game Shift
status accordingly
v6:
- Fixed alignment on some function definitions
- Fixed braces on if statment
- Removed quirk thermal_ustt
- Now quirk thermal can take values defined in enum WMAX_THERMAL_TABLE.
- Proper removal of thermal_profile
---
This refactor was done in order to autodetect available thermal modes
efficently through operation 0x03.
This new array approach exploits the fact that the first 4 bits of every
thermal code are consecutive from 0 to 9, however next 4 bits are not
consecutive (available thermal codes are documented in patch 4/4) so full
codes are dynamically probed based on rules found in is_wmax_thermal_code().
---
drivers/platform/x86/dell/Kconfig | 1 +
drivers/platform/x86/dell/alienware-wmi.c | 282 ++++++++++++++++++++++
2 files changed, 283 insertions(+)
diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
index 68a49788a..b06d634cd 100644
--- a/drivers/platform/x86/dell/Kconfig
+++ b/drivers/platform/x86/dell/Kconfig
@@ -21,6 +21,7 @@ config ALIENWARE_WMI
depends on LEDS_CLASS
depends on NEW_LEDS
depends on ACPI_WMI
+ select ACPI_PLATFORM_PROFILE
help
This is a driver for controlling Alienware BIOS driven
features. It exposes an interface for controlling the AlienFX
diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index b27f3b64c..9ce6e794a 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -8,8 +8,11 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/acpi.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/platform_profile.h>
#include <linux/dmi.h>
#include <linux/leds.h>
@@ -25,6 +28,13 @@
#define WMAX_METHOD_AMPLIFIER_CABLE 0x6
#define WMAX_METHOD_DEEP_SLEEP_CONTROL 0x0B
#define WMAX_METHOD_DEEP_SLEEP_STATUS 0x0C
+#define WMAX_METHOD_THERMAL_INFORMATION 0x14
+#define WMAX_METHOD_THERMAL_CONTROL 0x15
+#define WMAX_METHOD_GAME_SHIFT_STATUS 0x25
+
+#define WMAX_THERMAL_MODE_GMODE 0xAB
+
+#define WMAX_FAILURE_CODE 0xFFFFFFFF
MODULE_AUTHOR("Mario Limonciello <mario.limonciello@outlook.com>");
MODULE_DESCRIPTION("Alienware special feature control");
@@ -49,11 +59,59 @@ enum WMAX_CONTROL_STATES {
WMAX_SUSPEND = 3,
};
+enum WMAX_THERMAL_INFORMATION_OPERATIONS {
+ WMAX_OPERATION_LIST_IDS = 0x03,
+ WMAX_OPERATION_CURRENT_PROFILE = 0x0B,
+};
+
+enum WMAX_THERMAL_CONTROL_OPERATIONS {
+ WMAX_OPERATION_ACTIVATE_PROFILE = 0x01,
+};
+
+enum WMAX_GAMESHIFT_STATUS_OPERATIONS {
+ WMAX_OPERATION_TOGGLE_GAME_SHIFT = 0x01,
+ WMAX_OPERATION_GET_GAME_SHIFT_STATUS = 0x02,
+};
+
+enum WMAX_THERMAL_TABLES {
+ WMAX_THERMAL_TABLE_BASIC = 0x90,
+ WMAX_THERMAL_TABLE_USTT = 0xA0,
+};
+
+enum wmax_thermal_mode {
+ THERMAL_MODE_USTT_BALANCED,
+ THERMAL_MODE_USTT_BALANCED_PERFORMANCE,
+ THERMAL_MODE_USTT_COOL,
+ THERMAL_MODE_USTT_QUIET,
+ THERMAL_MODE_USTT_PERFORMANCE,
+ THERMAL_MODE_USTT_LOW_POWER,
+ THERMAL_MODE_BASIC_QUIET,
+ THERMAL_MODE_BASIC_BALANCED,
+ THERMAL_MODE_BASIC_BALANCED_PERFORMANCE,
+ THERMAL_MODE_BASIC_PERFORMANCE,
+ THERMAL_MODE_LAST,
+};
+
+static const enum platform_profile_option wmax_mode_to_platform_profile[THERMAL_MODE_LAST] = {
+ [THERMAL_MODE_USTT_BALANCED] = PLATFORM_PROFILE_BALANCED,
+ [THERMAL_MODE_USTT_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
+ [THERMAL_MODE_USTT_COOL] = PLATFORM_PROFILE_COOL,
+ [THERMAL_MODE_USTT_QUIET] = PLATFORM_PROFILE_QUIET,
+ [THERMAL_MODE_USTT_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
+ [THERMAL_MODE_USTT_LOW_POWER] = PLATFORM_PROFILE_LOW_POWER,
+ [THERMAL_MODE_BASIC_QUIET] = PLATFORM_PROFILE_QUIET,
+ [THERMAL_MODE_BASIC_BALANCED] = PLATFORM_PROFILE_BALANCED,
+ [THERMAL_MODE_BASIC_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
+ [THERMAL_MODE_BASIC_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
+};
+
struct quirk_entry {
u8 num_zones;
u8 hdmi_mux;
u8 amplifier;
u8 deepslp;
+ u8 thermal;
+ u8 gmode;
};
static struct quirk_entry *quirks;
@@ -64,6 +122,8 @@ static struct quirk_entry quirk_inspiron5675 = {
.hdmi_mux = 0,
.amplifier = 0,
.deepslp = 0,
+ .thermal = 0,
+ .gmode = 0,
};
static struct quirk_entry quirk_unknown = {
@@ -71,6 +131,8 @@ static struct quirk_entry quirk_unknown = {
.hdmi_mux = 0,
.amplifier = 0,
.deepslp = 0,
+ .thermal = 0,
+ .gmode = 0,
};
static struct quirk_entry quirk_x51_r1_r2 = {
@@ -78,6 +140,8 @@ static struct quirk_entry quirk_x51_r1_r2 = {
.hdmi_mux = 0,
.amplifier = 0,
.deepslp = 0,
+ .thermal = 0,
+ .gmode = 0,
};
static struct quirk_entry quirk_x51_r3 = {
@@ -85,6 +149,8 @@ static struct quirk_entry quirk_x51_r3 = {
.hdmi_mux = 0,
.amplifier = 1,
.deepslp = 0,
+ .thermal = 0,
+ .gmode = 0,
};
static struct quirk_entry quirk_asm100 = {
@@ -92,6 +158,8 @@ static struct quirk_entry quirk_asm100 = {
.hdmi_mux = 1,
.amplifier = 0,
.deepslp = 0,
+ .thermal = 0,
+ .gmode = 0,
};
static struct quirk_entry quirk_asm200 = {
@@ -99,6 +167,8 @@ static struct quirk_entry quirk_asm200 = {
.hdmi_mux = 1,
.amplifier = 0,
.deepslp = 1,
+ .thermal = 0,
+ .gmode = 0,
};
static struct quirk_entry quirk_asm201 = {
@@ -106,6 +176,8 @@ static struct quirk_entry quirk_asm201 = {
.hdmi_mux = 1,
.amplifier = 1,
.deepslp = 1,
+ .thermal = 0,
+ .gmode = 0,
};
static int __init dmi_matched(const struct dmi_system_id *dmi)
@@ -214,10 +286,19 @@ struct wmax_led_args {
u8 state;
} __packed;
+struct wmax_u32_args {
+ u8 operation;
+ u8 arg1;
+ u8 arg2;
+ u8 arg3;
+};
+
static struct platform_device *platform_device;
static struct device_attribute *zone_dev_attrs;
static struct attribute **zone_attrs;
static struct platform_zone *zone_data;
+static struct platform_profile_handler pp_handler;
+static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
static struct platform_driver platform_driver = {
.driver = {
@@ -761,6 +842,198 @@ static int create_deepsleep(struct platform_device *dev)
return ret;
}
+/*
+ * Thermal Profile control
+ * - Provides thermal profile control through the Platform Profile API
+ */
+#define WMAX_THERMAL_TABLE_MASK GENMASK(7, 4)
+#define WMAX_THERMAL_MODE_MASK GENMASK(3, 0)
+
+static bool is_wmax_thermal_code(u32 code)
+{
+ return ((code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_BASIC ||
+ (code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_USTT) &&
+ (code & WMAX_THERMAL_MODE_MASK) < THERMAL_MODE_LAST;
+}
+
+static int wmax_thermal_information(u8 operation, u8 arg, u32 *out_data)
+{
+ acpi_status status;
+ struct wmax_u32_args in_args = {
+ .operation = operation,
+ .arg1 = arg,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+ status = alienware_wmax_command(&in_args, sizeof(in_args),
+ WMAX_METHOD_THERMAL_INFORMATION,
+ out_data);
+
+ if (ACPI_FAILURE(status))
+ return -EIO;
+
+ if (*out_data == WMAX_FAILURE_CODE)
+ return -EBADRQC;
+
+ return 0;
+}
+
+static int wmax_thermal_control(u8 operation, u8 arg)
+{
+ acpi_status status;
+ struct wmax_u32_args in_args = {
+ .operation = operation,
+ .arg1 = arg,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+ u32 out_data;
+
+ status = alienware_wmax_command(&in_args, sizeof(in_args),
+ WMAX_METHOD_THERMAL_CONTROL,
+ &out_data);
+
+ if (ACPI_FAILURE(status))
+ return -EIO;
+
+ if (out_data == WMAX_FAILURE_CODE)
+ return -EBADRQC;
+
+ return 0;
+}
+
+static int wmax_gameshift_status(u8 operation, u32 *out_data)
+{
+ acpi_status status;
+ struct wmax_u32_args in_args = {
+ .operation = operation,
+ .arg1 = 0,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+ status = alienware_wmax_command(&in_args, sizeof(in_args),
+ WMAX_METHOD_GAME_SHIFT_STATUS,
+ out_data);
+
+ if (ACPI_FAILURE(status))
+ return -EIO;
+
+ if (*out_data == WMAX_FAILURE_CODE)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+static int thermal_profile_get(struct platform_profile_handler *pprof,
+ enum platform_profile_option *profile)
+{
+ u32 out_data;
+ int ret;
+
+ ret = wmax_thermal_information(WMAX_OPERATION_CURRENT_PROFILE,
+ 0, &out_data);
+
+ if (ret < 0)
+ return ret;
+
+ if (!is_wmax_thermal_code(out_data))
+ return -ENODATA;
+
+ out_data &= WMAX_THERMAL_MODE_MASK;
+ *profile = wmax_mode_to_platform_profile[out_data];
+
+ return 0;
+}
+
+static int thermal_profile_set(struct platform_profile_handler *pprof,
+ enum platform_profile_option profile)
+{
+ if (quirks->gmode == 1) {
+ u32 gmode_status;
+ int ret;
+
+ ret = wmax_gameshift_status(WMAX_OPERATION_GET_GAME_SHIFT_STATUS,
+ &gmode_status);
+
+ if (ret < 0)
+ return ret;
+
+ if ((profile == PLATFORM_PROFILE_PERFORMANCE && !gmode_status) ||
+ (profile != PLATFORM_PROFILE_PERFORMANCE && gmode_status)) {
+ ret = wmax_gameshift_status(WMAX_OPERATION_TOGGLE_GAME_SHIFT,
+ &gmode_status);
+
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ return wmax_thermal_control(WMAX_OPERATION_ACTIVATE_PROFILE,
+ supported_thermal_profiles[profile]);
+}
+
+static int create_thermal_profile(void)
+{
+ u32 out_data;
+ u32 gmode_status;
+ enum wmax_thermal_mode mode;
+ enum platform_profile_option profile;
+ int ret;
+
+ for (u8 i = 0x2; i <= 0xD; i++) {
+ ret = wmax_thermal_information(WMAX_OPERATION_LIST_IDS,
+ i, &out_data);
+
+ if (ret == -EIO)
+ return 0;
+
+ if (ret == -EBADRQC)
+ break;
+
+ if (!is_wmax_thermal_code(out_data))
+ continue;
+
+ mode = out_data & WMAX_THERMAL_MODE_MASK;
+ profile = wmax_mode_to_platform_profile[mode];
+ supported_thermal_profiles[profile] = out_data;
+
+ set_bit(profile, pp_handler.choices);
+ }
+
+ if (bitmap_empty(pp_handler.choices, PLATFORM_PROFILE_LAST))
+ return 0;
+
+ ret = wmax_gameshift_status(WMAX_OPERATION_GET_GAME_SHIFT_STATUS,
+ &gmode_status);
+
+ if (!ret) {
+ supported_thermal_profiles[PLATFORM_PROFILE_PERFORMANCE] =
+ WMAX_THERMAL_MODE_GMODE;
+
+ set_bit(PLATFORM_PROFILE_PERFORMANCE, pp_handler.choices);
+ quirks->gmode = 1;
+ }
+
+ pp_handler.profile_get = thermal_profile_get;
+ pp_handler.profile_set = thermal_profile_set;
+
+ ret = platform_profile_register(&pp_handler);
+ if (ret < 0)
+ return ret;
+
+ quirks->thermal = 1;
+
+ return 0;
+}
+
+static void remove_thermal_profile(void)
+{
+ if (quirks->thermal > 0)
+ platform_profile_remove();
+}
+
static int __init alienware_wmi_init(void)
{
int ret;
@@ -808,6 +1081,12 @@ static int __init alienware_wmi_init(void)
goto fail_prep_deepsleep;
}
+ if (interface == WMAX && quirks == &quirk_unknown) {
+ ret = create_thermal_profile();
+ if (ret)
+ goto fail_prep_thermal_profile;
+ }
+
ret = alienware_zone_init(platform_device);
if (ret)
goto fail_prep_zones;
@@ -816,6 +1095,8 @@ static int __init alienware_wmi_init(void)
fail_prep_zones:
alienware_zone_exit(platform_device);
+ remove_thermal_profile();
+fail_prep_thermal_profile:
fail_prep_deepsleep:
fail_prep_amplifier:
fail_prep_hdmi:
@@ -835,6 +1116,7 @@ static void __exit alienware_wmi_exit(void)
if (platform_device) {
alienware_zone_exit(platform_device);
remove_hdmi(platform_device);
+ remove_thermal_profile();
platform_device_unregister(platform_device);
platform_driver_unregister(&platform_driver);
}
--
2.47.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v7 3/4] alienware-wmi: added platform profile support
2024-10-24 6:19 ` [PATCH v7 3/4] alienware-wmi: added platform profile support Kurt Borja
@ 2024-10-24 7:39 ` Ilpo Järvinen
2024-10-24 9:02 ` Kurt Borja
0 siblings, 1 reply; 7+ messages in thread
From: Ilpo Järvinen @ 2024-10-24 7:39 UTC (permalink / raw)
To: Kurt Borja; +Cc: W_Armin, Hans de Goede, LKML, platform-driver-x86
On Thu, 24 Oct 2024, Kurt Borja wrote:
> Implements platform profile support for Dell laptops with new WMAX thermal
> interface, present on some Alienware X-Series, Alienware M-Series and
> Dell's G-Series laptops.
>
> This implementation automatically detects available thermal profiles and
> GMODE + Game Shift availability, which is a feature of G-Series laptops.
>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> v7:
> - Method operations are now clearly listed as separate enums
> - wmax_thermal_modes are now listed without codes in order to support
> autodetection, as well as getting and setting thermal profiles
> cleanly through arrays
> - Added wmax_mode_to_platform_profile[]
> - Added struct wmax_u32_args to replace bit mask approach of
> constructing arguments for wmax methods
> - create_thermal_profile now autodetects available thermal codes
> through operation 0x03 of THERMAL_INFORMATION method. These are
> codes are stored in supported_thermal_profiles[]
> - thermal_profile_get now uses wmax_mode_to_platform_profile[] instead of
> switch-case approach
> - thermal_profile_set now uses supported_thermal_profiles[] instead of
> switch-case approach
> - When gmode is autodetected, thermal_profile_set also sets Game Shift
> status accordingly
> v6:
> - Fixed alignment on some function definitions
> - Fixed braces on if statment
> - Removed quirk thermal_ustt
> - Now quirk thermal can take values defined in enum WMAX_THERMAL_TABLE.
> - Proper removal of thermal_profile
> ---
> This refactor was done in order to autodetect available thermal modes
> efficently through operation 0x03.
>
> This new array approach exploits the fact that the first 4 bits of every
> thermal code are consecutive from 0 to 9, however next 4 bits are not
> consecutive (available thermal codes are documented in patch 4/4) so full
> codes are dynamically probed based on rules found in is_wmax_thermal_code().
IMO, both of these paragraphs contain information would be useful in the
commit message (with minor rephrasing).
> ---
> drivers/platform/x86/dell/Kconfig | 1 +
> drivers/platform/x86/dell/alienware-wmi.c | 282 ++++++++++++++++++++++
> 2 files changed, 283 insertions(+)
>
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index 68a49788a..b06d634cd 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -21,6 +21,7 @@ config ALIENWARE_WMI
> depends on LEDS_CLASS
> depends on NEW_LEDS
> depends on ACPI_WMI
> + select ACPI_PLATFORM_PROFILE
> help
> This is a driver for controlling Alienware BIOS driven
> features. It exposes an interface for controlling the AlienFX
> diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
> index b27f3b64c..9ce6e794a 100644
> --- a/drivers/platform/x86/dell/alienware-wmi.c
> +++ b/drivers/platform/x86/dell/alienware-wmi.c
> @@ -8,8 +8,11 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/acpi.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/platform_profile.h>
> #include <linux/dmi.h>
> #include <linux/leds.h>
>
> @@ -25,6 +28,13 @@
> #define WMAX_METHOD_AMPLIFIER_CABLE 0x6
> #define WMAX_METHOD_DEEP_SLEEP_CONTROL 0x0B
> #define WMAX_METHOD_DEEP_SLEEP_STATUS 0x0C
> +#define WMAX_METHOD_THERMAL_INFORMATION 0x14
> +#define WMAX_METHOD_THERMAL_CONTROL 0x15
> +#define WMAX_METHOD_GAME_SHIFT_STATUS 0x25
> +
> +#define WMAX_THERMAL_MODE_GMODE 0xAB
> +
> +#define WMAX_FAILURE_CODE 0xFFFFFFFF
>
> MODULE_AUTHOR("Mario Limonciello <mario.limonciello@outlook.com>");
> MODULE_DESCRIPTION("Alienware special feature control");
> @@ -49,11 +59,59 @@ enum WMAX_CONTROL_STATES {
> WMAX_SUSPEND = 3,
> };
>
> +enum WMAX_THERMAL_INFORMATION_OPERATIONS {
> + WMAX_OPERATION_LIST_IDS = 0x03,
> + WMAX_OPERATION_CURRENT_PROFILE = 0x0B,
> +};
> +
> +enum WMAX_THERMAL_CONTROL_OPERATIONS {
> + WMAX_OPERATION_ACTIVATE_PROFILE = 0x01,
> +};
> +
> +enum WMAX_GAMESHIFT_STATUS_OPERATIONS {
> + WMAX_OPERATION_TOGGLE_GAME_SHIFT = 0x01,
> + WMAX_OPERATION_GET_GAME_SHIFT_STATUS = 0x02,
> +};
> +
> +enum WMAX_THERMAL_TABLES {
> + WMAX_THERMAL_TABLE_BASIC = 0x90,
> + WMAX_THERMAL_TABLE_USTT = 0xA0,
> +};
> +
> +enum wmax_thermal_mode {
> + THERMAL_MODE_USTT_BALANCED,
> + THERMAL_MODE_USTT_BALANCED_PERFORMANCE,
> + THERMAL_MODE_USTT_COOL,
> + THERMAL_MODE_USTT_QUIET,
> + THERMAL_MODE_USTT_PERFORMANCE,
> + THERMAL_MODE_USTT_LOW_POWER,
> + THERMAL_MODE_BASIC_QUIET,
> + THERMAL_MODE_BASIC_BALANCED,
> + THERMAL_MODE_BASIC_BALANCED_PERFORMANCE,
> + THERMAL_MODE_BASIC_PERFORMANCE,
> + THERMAL_MODE_LAST,
> +};
> +
> +static const enum platform_profile_option wmax_mode_to_platform_profile[THERMAL_MODE_LAST] = {
> + [THERMAL_MODE_USTT_BALANCED] = PLATFORM_PROFILE_BALANCED,
> + [THERMAL_MODE_USTT_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> + [THERMAL_MODE_USTT_COOL] = PLATFORM_PROFILE_COOL,
> + [THERMAL_MODE_USTT_QUIET] = PLATFORM_PROFILE_QUIET,
> + [THERMAL_MODE_USTT_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
> + [THERMAL_MODE_USTT_LOW_POWER] = PLATFORM_PROFILE_LOW_POWER,
> + [THERMAL_MODE_BASIC_QUIET] = PLATFORM_PROFILE_QUIET,
> + [THERMAL_MODE_BASIC_BALANCED] = PLATFORM_PROFILE_BALANCED,
> + [THERMAL_MODE_BASIC_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> + [THERMAL_MODE_BASIC_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
Please align the values.
I know they are a bit long so if you don't like how far they end up, align
at least all except the PLATFORM_PROFILE_BALANCED_PERFORMANCE lines which
already is a major improvement over the current layout.
> +};
> +
> struct quirk_entry {
> u8 num_zones;
> u8 hdmi_mux;
> u8 amplifier;
> u8 deepslp;
> + u8 thermal;
> + u8 gmode;
I wonder what is the benefit of setting these to zeros down below? It's
the default they initialize anyway. It's also a bit misleading to
indicate they're 0.
IMO it would be better to just add a comment to these two entries here
that they're autodetected and not initialized them explicitly at all.
Also, both of these behave like booleans so if there's no plan to use more
than 0/1 (false/true), just make them bool (and adapt the related code
accordingly).
> };
>
> static struct quirk_entry *quirks;
> @@ -64,6 +122,8 @@ static struct quirk_entry quirk_inspiron5675 = {
> .hdmi_mux = 0,
> .amplifier = 0,
> .deepslp = 0,
> + .thermal = 0,
> + .gmode = 0,
> };
>
> static struct quirk_entry quirk_unknown = {
> @@ -71,6 +131,8 @@ static struct quirk_entry quirk_unknown = {
> .hdmi_mux = 0,
> .amplifier = 0,
> .deepslp = 0,
> + .thermal = 0,
> + .gmode = 0,
> };
>
> static struct quirk_entry quirk_x51_r1_r2 = {
> @@ -78,6 +140,8 @@ static struct quirk_entry quirk_x51_r1_r2 = {
> .hdmi_mux = 0,
> .amplifier = 0,
> .deepslp = 0,
> + .thermal = 0,
> + .gmode = 0,
> };
>
> static struct quirk_entry quirk_x51_r3 = {
> @@ -85,6 +149,8 @@ static struct quirk_entry quirk_x51_r3 = {
> .hdmi_mux = 0,
> .amplifier = 1,
> .deepslp = 0,
> + .thermal = 0,
> + .gmode = 0,
> };
>
> static struct quirk_entry quirk_asm100 = {
> @@ -92,6 +158,8 @@ static struct quirk_entry quirk_asm100 = {
> .hdmi_mux = 1,
> .amplifier = 0,
> .deepslp = 0,
> + .thermal = 0,
> + .gmode = 0,
> };
>
> static struct quirk_entry quirk_asm200 = {
> @@ -99,6 +167,8 @@ static struct quirk_entry quirk_asm200 = {
> .hdmi_mux = 1,
> .amplifier = 0,
> .deepslp = 1,
> + .thermal = 0,
> + .gmode = 0,
> };
>
> static struct quirk_entry quirk_asm201 = {
> @@ -106,6 +176,8 @@ static struct quirk_entry quirk_asm201 = {
> .hdmi_mux = 1,
> .amplifier = 1,
> .deepslp = 1,
> + .thermal = 0,
> + .gmode = 0,
> };
>
> static int __init dmi_matched(const struct dmi_system_id *dmi)
> @@ -214,10 +286,19 @@ struct wmax_led_args {
> u8 state;
> } __packed;
>
> +struct wmax_u32_args {
> + u8 operation;
> + u8 arg1;
> + u8 arg2;
> + u8 arg3;
> +};
> +
> static struct platform_device *platform_device;
> static struct device_attribute *zone_dev_attrs;
> static struct attribute **zone_attrs;
> static struct platform_zone *zone_data;
> +static struct platform_profile_handler pp_handler;
> +static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
>
> static struct platform_driver platform_driver = {
> .driver = {
> @@ -761,6 +842,198 @@ static int create_deepsleep(struct platform_device *dev)
> return ret;
> }
>
> +/*
> + * Thermal Profile control
> + * - Provides thermal profile control through the Platform Profile API
> + */
> +#define WMAX_THERMAL_TABLE_MASK GENMASK(7, 4)
> +#define WMAX_THERMAL_MODE_MASK GENMASK(3, 0)
> +
> +static bool is_wmax_thermal_code(u32 code)
> +{
> + return ((code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_BASIC ||
> + (code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_USTT) &&
This line is misindented...
> + (code & WMAX_THERMAL_MODE_MASK) < THERMAL_MODE_LAST;
...But I think this would be cleaner/easier to follow if less is attempted
inside a single statement:
if ((code & WMAX_THERMAL_MODE_MASK) >= THERMAL_MODE_LAST)
return false;
return ...the rest of the logic...;
Or in the opposite order if you want to do the table check before the mode
check.
> +}
> +
> +static int wmax_thermal_information(u8 operation, u8 arg, u32 *out_data)
Can this 'arg' parameter be named
> +{
> + acpi_status status;
> + struct wmax_u32_args in_args = {
> + .operation = operation,
> + .arg1 = arg,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> + status = alienware_wmax_command(&in_args, sizeof(in_args),
> + WMAX_METHOD_THERMAL_INFORMATION,
> + out_data);
> +
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + if (*out_data == WMAX_FAILURE_CODE)
> + return -EBADRQC;
> +
> + return 0;
> +}
> +
> +static int wmax_thermal_control(u8 operation, u8 arg)
> +{
> + acpi_status status;
> + struct wmax_u32_args in_args = {
> + .operation = operation,
> + .arg1 = arg,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> + u32 out_data;
> +
> + status = alienware_wmax_command(&in_args, sizeof(in_args),
> + WMAX_METHOD_THERMAL_CONTROL,
> + &out_data);
> +
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + if (out_data == WMAX_FAILURE_CODE)
> + return -EBADRQC;
> +
> + return 0;
> +}
> +
> +static int wmax_gameshift_status(u8 operation, u32 *out_data)
> +{
> + acpi_status status;
> + struct wmax_u32_args in_args = {
> + .operation = operation,
> + .arg1 = 0,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> + status = alienware_wmax_command(&in_args, sizeof(in_args),
> + WMAX_METHOD_GAME_SHIFT_STATUS,
> + out_data);
> +
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + if (*out_data == WMAX_FAILURE_CODE)
> + return -EOPNOTSUPP;
> +
> + return 0;
> +}
> +
> +static int thermal_profile_get(struct platform_profile_handler *pprof,
> + enum platform_profile_option *profile)
> +{
> + u32 out_data;
> + int ret;
> +
> + ret = wmax_thermal_information(WMAX_OPERATION_CURRENT_PROFILE,
> + 0, &out_data);
> +
> + if (ret < 0)
> + return ret;
> +
> + if (!is_wmax_thermal_code(out_data))
> + return -ENODATA;
> +
> + out_data &= WMAX_THERMAL_MODE_MASK;
> + *profile = wmax_mode_to_platform_profile[out_data];
> +
> + return 0;
> +}
> +
> +static int thermal_profile_set(struct platform_profile_handler *pprof,
> + enum platform_profile_option profile)
> +{
> + if (quirks->gmode == 1) {
> + u32 gmode_status;
> + int ret;
> +
> + ret = wmax_gameshift_status(WMAX_OPERATION_GET_GAME_SHIFT_STATUS,
> + &gmode_status);
> +
> + if (ret < 0)
> + return ret;
> +
> + if ((profile == PLATFORM_PROFILE_PERFORMANCE && !gmode_status) ||
> + (profile != PLATFORM_PROFILE_PERFORMANCE && gmode_status)) {
> + ret = wmax_gameshift_status(WMAX_OPERATION_TOGGLE_GAME_SHIFT,
> + &gmode_status);
> +
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> + return wmax_thermal_control(WMAX_OPERATION_ACTIVATE_PROFILE,
> + supported_thermal_profiles[profile]);
If somebody has to look at this change from changelog history some years
from now, it would be beneficial for the commit message to contain a few
words to explain how this "game shift" thing relates to anything what is
being done here. It's not exactly obvious from the naming alone and
marketting may have already moved to the next term making it eventually
hard to find some information about the term.
You have some material in the docs change and the most relevant
fragment(s) could perhaps be used in the commit message as well
(especially as it seems to be based on some guesswork on its actual
effect).
If you reorder the patches 3 and 4, the commit message of this change
could also ref to the doc for further information (but I'd prefer to have
the relevant parts also in the commit message so the reasoning is
communicated as per today's understanding, the doc files may get
rewritten/changed over time).
In any case, thanks a lot for working on this!
--
i.
> +}
> +
> +static int create_thermal_profile(void)
> +{
> + u32 out_data;
> + u32 gmode_status;
> + enum wmax_thermal_mode mode;
> + enum platform_profile_option profile;
> + int ret;
> +
> + for (u8 i = 0x2; i <= 0xD; i++) {
> + ret = wmax_thermal_information(WMAX_OPERATION_LIST_IDS,
> + i, &out_data);
> +
> + if (ret == -EIO)
> + return 0;
> +
> + if (ret == -EBADRQC)
> + break;
> +
> + if (!is_wmax_thermal_code(out_data))
> + continue;
> +
> + mode = out_data & WMAX_THERMAL_MODE_MASK;
> + profile = wmax_mode_to_platform_profile[mode];
> + supported_thermal_profiles[profile] = out_data;
> +
> + set_bit(profile, pp_handler.choices);
> + }
> +
> + if (bitmap_empty(pp_handler.choices, PLATFORM_PROFILE_LAST))
> + return 0;
> +
> + ret = wmax_gameshift_status(WMAX_OPERATION_GET_GAME_SHIFT_STATUS,
> + &gmode_status);
> +
> + if (!ret) {
> + supported_thermal_profiles[PLATFORM_PROFILE_PERFORMANCE] =
> + WMAX_THERMAL_MODE_GMODE;
> +
> + set_bit(PLATFORM_PROFILE_PERFORMANCE, pp_handler.choices);
> + quirks->gmode = 1;
> + }
> +
> + pp_handler.profile_get = thermal_profile_get;
> + pp_handler.profile_set = thermal_profile_set;
> +
> + ret = platform_profile_register(&pp_handler);
> + if (ret < 0)
> + return ret;
> +
> + quirks->thermal = 1;
> +
> + return 0;
> +}
> +
> +static void remove_thermal_profile(void)
> +{
> + if (quirks->thermal > 0)
> + platform_profile_remove();
> +}
> +
> static int __init alienware_wmi_init(void)
> {
> int ret;
> @@ -808,6 +1081,12 @@ static int __init alienware_wmi_init(void)
> goto fail_prep_deepsleep;
> }
>
> + if (interface == WMAX && quirks == &quirk_unknown) {
> + ret = create_thermal_profile();
> + if (ret)
> + goto fail_prep_thermal_profile;
> + }
> +
> ret = alienware_zone_init(platform_device);
> if (ret)
> goto fail_prep_zones;
> @@ -816,6 +1095,8 @@ static int __init alienware_wmi_init(void)
>
> fail_prep_zones:
> alienware_zone_exit(platform_device);
> + remove_thermal_profile();
> +fail_prep_thermal_profile:
> fail_prep_deepsleep:
> fail_prep_amplifier:
> fail_prep_hdmi:
> @@ -835,6 +1116,7 @@ static void __exit alienware_wmi_exit(void)
> if (platform_device) {
> alienware_zone_exit(platform_device);
> remove_hdmi(platform_device);
> + remove_thermal_profile();
> platform_device_unregister(platform_device);
> platform_driver_unregister(&platform_driver);
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v7 3/4] alienware-wmi: added platform profile support
2024-10-24 7:39 ` Ilpo Järvinen
@ 2024-10-24 9:02 ` Kurt Borja
0 siblings, 0 replies; 7+ messages in thread
From: Kurt Borja @ 2024-10-24 9:02 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: W_Armin, Hans de Goede, LKML, platform-driver-x86
On Thu, Oct 24, 2024 at 10:39:55AM +0300, Ilpo Järvinen wrote:
> On Thu, 24 Oct 2024, Kurt Borja wrote:
>
> > Implements platform profile support for Dell laptops with new WMAX thermal
> > interface, present on some Alienware X-Series, Alienware M-Series and
> > Dell's G-Series laptops.
> >
> > This implementation automatically detects available thermal profiles and
> > GMODE + Game Shift availability, which is a feature of G-Series laptops.
> >
> > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > ---
> > v7:
> > - Method operations are now clearly listed as separate enums
> > - wmax_thermal_modes are now listed without codes in order to support
> > autodetection, as well as getting and setting thermal profiles
> > cleanly through arrays
> > - Added wmax_mode_to_platform_profile[]
> > - Added struct wmax_u32_args to replace bit mask approach of
> > constructing arguments for wmax methods
> > - create_thermal_profile now autodetects available thermal codes
> > through operation 0x03 of THERMAL_INFORMATION method. These are
> > codes are stored in supported_thermal_profiles[]
> > - thermal_profile_get now uses wmax_mode_to_platform_profile[] instead of
> > switch-case approach
> > - thermal_profile_set now uses supported_thermal_profiles[] instead of
> > switch-case approach
> > - When gmode is autodetected, thermal_profile_set also sets Game Shift
> > status accordingly
> > v6:
> > - Fixed alignment on some function definitions
> > - Fixed braces on if statment
> > - Removed quirk thermal_ustt
> > - Now quirk thermal can take values defined in enum WMAX_THERMAL_TABLE.
> > - Proper removal of thermal_profile
> > ---
> > This refactor was done in order to autodetect available thermal modes
> > efficently through operation 0x03.
> >
> > This new array approach exploits the fact that the first 4 bits of every
> > thermal code are consecutive from 0 to 9, however next 4 bits are not
> > consecutive (available thermal codes are documented in patch 4/4) so full
> > codes are dynamically probed based on rules found in is_wmax_thermal_code().
>
> IMO, both of these paragraphs contain information would be useful in the
> commit message (with minor rephrasing).
I will make sure the next commit message contains key details about
methods and operations used.
>
> > ---
> > drivers/platform/x86/dell/Kconfig | 1 +
> > drivers/platform/x86/dell/alienware-wmi.c | 282 ++++++++++++++++++++++
> > 2 files changed, 283 insertions(+)
> >
> > diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> > index 68a49788a..b06d634cd 100644
> > --- a/drivers/platform/x86/dell/Kconfig
> > +++ b/drivers/platform/x86/dell/Kconfig
> > @@ -21,6 +21,7 @@ config ALIENWARE_WMI
> > depends on LEDS_CLASS
> > depends on NEW_LEDS
> > depends on ACPI_WMI
> > + select ACPI_PLATFORM_PROFILE
> > help
> > This is a driver for controlling Alienware BIOS driven
> > features. It exposes an interface for controlling the AlienFX
> > diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
> > index b27f3b64c..9ce6e794a 100644
> > --- a/drivers/platform/x86/dell/alienware-wmi.c
> > +++ b/drivers/platform/x86/dell/alienware-wmi.c
> > @@ -8,8 +8,11 @@
> > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >
> > #include <linux/acpi.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > #include <linux/module.h>
> > #include <linux/platform_device.h>
> > +#include <linux/platform_profile.h>
> > #include <linux/dmi.h>
> > #include <linux/leds.h>
> >
> > @@ -25,6 +28,13 @@
> > #define WMAX_METHOD_AMPLIFIER_CABLE 0x6
> > #define WMAX_METHOD_DEEP_SLEEP_CONTROL 0x0B
> > #define WMAX_METHOD_DEEP_SLEEP_STATUS 0x0C
> > +#define WMAX_METHOD_THERMAL_INFORMATION 0x14
> > +#define WMAX_METHOD_THERMAL_CONTROL 0x15
> > +#define WMAX_METHOD_GAME_SHIFT_STATUS 0x25
> > +
> > +#define WMAX_THERMAL_MODE_GMODE 0xAB
> > +
> > +#define WMAX_FAILURE_CODE 0xFFFFFFFF
> >
> > MODULE_AUTHOR("Mario Limonciello <mario.limonciello@outlook.com>");
> > MODULE_DESCRIPTION("Alienware special feature control");
> > @@ -49,11 +59,59 @@ enum WMAX_CONTROL_STATES {
> > WMAX_SUSPEND = 3,
> > };
> >
> > +enum WMAX_THERMAL_INFORMATION_OPERATIONS {
> > + WMAX_OPERATION_LIST_IDS = 0x03,
> > + WMAX_OPERATION_CURRENT_PROFILE = 0x0B,
> > +};
> > +
> > +enum WMAX_THERMAL_CONTROL_OPERATIONS {
> > + WMAX_OPERATION_ACTIVATE_PROFILE = 0x01,
> > +};
> > +
> > +enum WMAX_GAMESHIFT_STATUS_OPERATIONS {
> > + WMAX_OPERATION_TOGGLE_GAME_SHIFT = 0x01,
> > + WMAX_OPERATION_GET_GAME_SHIFT_STATUS = 0x02,
> > +};
> > +
> > +enum WMAX_THERMAL_TABLES {
> > + WMAX_THERMAL_TABLE_BASIC = 0x90,
> > + WMAX_THERMAL_TABLE_USTT = 0xA0,
> > +};
> > +
> > +enum wmax_thermal_mode {
> > + THERMAL_MODE_USTT_BALANCED,
> > + THERMAL_MODE_USTT_BALANCED_PERFORMANCE,
> > + THERMAL_MODE_USTT_COOL,
> > + THERMAL_MODE_USTT_QUIET,
> > + THERMAL_MODE_USTT_PERFORMANCE,
> > + THERMAL_MODE_USTT_LOW_POWER,
> > + THERMAL_MODE_BASIC_QUIET,
> > + THERMAL_MODE_BASIC_BALANCED,
> > + THERMAL_MODE_BASIC_BALANCED_PERFORMANCE,
> > + THERMAL_MODE_BASIC_PERFORMANCE,
> > + THERMAL_MODE_LAST,
> > +};
> > +
> > +static const enum platform_profile_option wmax_mode_to_platform_profile[THERMAL_MODE_LAST] = {
> > + [THERMAL_MODE_USTT_BALANCED] = PLATFORM_PROFILE_BALANCED,
> > + [THERMAL_MODE_USTT_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> > + [THERMAL_MODE_USTT_COOL] = PLATFORM_PROFILE_COOL,
> > + [THERMAL_MODE_USTT_QUIET] = PLATFORM_PROFILE_QUIET,
> > + [THERMAL_MODE_USTT_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
> > + [THERMAL_MODE_USTT_LOW_POWER] = PLATFORM_PROFILE_LOW_POWER,
> > + [THERMAL_MODE_BASIC_QUIET] = PLATFORM_PROFILE_QUIET,
> > + [THERMAL_MODE_BASIC_BALANCED] = PLATFORM_PROFILE_BALANCED,
> > + [THERMAL_MODE_BASIC_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
> > + [THERMAL_MODE_BASIC_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
>
> Please align the values.
>
> I know they are a bit long so if you don't like how far they end up, align
> at least all except the PLATFORM_PROFILE_BALANCED_PERFORMANCE lines which
> already is a major improvement over the current layout.
I will fix it.
>
> > +};
> > +
> > struct quirk_entry {
> > u8 num_zones;
> > u8 hdmi_mux;
> > u8 amplifier;
> > u8 deepslp;
> > + u8 thermal;
> > + u8 gmode;
>
> I wonder what is the benefit of setting these to zeros down below? It's
> the default they initialize anyway. It's also a bit misleading to
> indicate they're 0.
Agreed, I will remove them.
>
> IMO it would be better to just add a comment to these two entries here
> that they're autodetected and not initialized them explicitly at all.
Agreed.
>
> Also, both of these behave like booleans so if there's no plan to use more
> than 0/1 (false/true), just make them bool (and adapt the related code
> accordingly).
I will change them to bools.
>
> > };
> >
> > static struct quirk_entry *quirks;
> > @@ -64,6 +122,8 @@ static struct quirk_entry quirk_inspiron5675 = {
> > .hdmi_mux = 0,
> > .amplifier = 0,
> > .deepslp = 0,
> > + .thermal = 0,
> > + .gmode = 0,
> > };
> >
> > static struct quirk_entry quirk_unknown = {
> > @@ -71,6 +131,8 @@ static struct quirk_entry quirk_unknown = {
> > .hdmi_mux = 0,
> > .amplifier = 0,
> > .deepslp = 0,
> > + .thermal = 0,
> > + .gmode = 0,
> > };
> >
> > static struct quirk_entry quirk_x51_r1_r2 = {
> > @@ -78,6 +140,8 @@ static struct quirk_entry quirk_x51_r1_r2 = {
> > .hdmi_mux = 0,
> > .amplifier = 0,
> > .deepslp = 0,
> > + .thermal = 0,
> > + .gmode = 0,
> > };
> >
> > static struct quirk_entry quirk_x51_r3 = {
> > @@ -85,6 +149,8 @@ static struct quirk_entry quirk_x51_r3 = {
> > .hdmi_mux = 0,
> > .amplifier = 1,
> > .deepslp = 0,
> > + .thermal = 0,
> > + .gmode = 0,
> > };
> >
> > static struct quirk_entry quirk_asm100 = {
> > @@ -92,6 +158,8 @@ static struct quirk_entry quirk_asm100 = {
> > .hdmi_mux = 1,
> > .amplifier = 0,
> > .deepslp = 0,
> > + .thermal = 0,
> > + .gmode = 0,
> > };
> >
> > static struct quirk_entry quirk_asm200 = {
> > @@ -99,6 +167,8 @@ static struct quirk_entry quirk_asm200 = {
> > .hdmi_mux = 1,
> > .amplifier = 0,
> > .deepslp = 1,
> > + .thermal = 0,
> > + .gmode = 0,
> > };
> >
> > static struct quirk_entry quirk_asm201 = {
> > @@ -106,6 +176,8 @@ static struct quirk_entry quirk_asm201 = {
> > .hdmi_mux = 1,
> > .amplifier = 1,
> > .deepslp = 1,
> > + .thermal = 0,
> > + .gmode = 0,
> > };
> >
> > static int __init dmi_matched(const struct dmi_system_id *dmi)
> > @@ -214,10 +286,19 @@ struct wmax_led_args {
> > u8 state;
> > } __packed;
> >
> > +struct wmax_u32_args {
> > + u8 operation;
> > + u8 arg1;
> > + u8 arg2;
> > + u8 arg3;
> > +};
> > +
> > static struct platform_device *platform_device;
> > static struct device_attribute *zone_dev_attrs;
> > static struct attribute **zone_attrs;
> > static struct platform_zone *zone_data;
> > +static struct platform_profile_handler pp_handler;
> > +static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
> >
> > static struct platform_driver platform_driver = {
> > .driver = {
> > @@ -761,6 +842,198 @@ static int create_deepsleep(struct platform_device *dev)
> > return ret;
> > }
> >
> > +/*
> > + * Thermal Profile control
> > + * - Provides thermal profile control through the Platform Profile API
> > + */
> > +#define WMAX_THERMAL_TABLE_MASK GENMASK(7, 4)
> > +#define WMAX_THERMAL_MODE_MASK GENMASK(3, 0)
> > +
> > +static bool is_wmax_thermal_code(u32 code)
> > +{
> > + return ((code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_BASIC ||
> > + (code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_USTT) &&
>
> This line is misindented...
>
> > + (code & WMAX_THERMAL_MODE_MASK) < THERMAL_MODE_LAST;
>
> ...But I think this would be cleaner/easier to follow if less is attempted
> inside a single statement:
>
> if ((code & WMAX_THERMAL_MODE_MASK) >= THERMAL_MODE_LAST)
> return false;
>
> return ...the rest of the logic...;
>
> Or in the opposite order if you want to do the table check before the mode
> check.
I will do it like this.
>
> > +}
> > +
> > +static int wmax_thermal_information(u8 operation, u8 arg, u32 *out_data)
>
> Can this 'arg' parameter be named
I named it `arg` because Thermal_Information method has various operations
in which `arg` could take different meanings and I wanted this wrapper
to be as general as possible.
But then again this should be clear from the commit message.
>
> > +{
> > + acpi_status status;
> > + struct wmax_u32_args in_args = {
> > + .operation = operation,
> > + .arg1 = arg,
> > + .arg2 = 0,
> > + .arg3 = 0,
> > + };
> > +
> > + status = alienware_wmax_command(&in_args, sizeof(in_args),
> > + WMAX_METHOD_THERMAL_INFORMATION,
> > + out_data);
> > +
> > + if (ACPI_FAILURE(status))
> > + return -EIO;
> > +
> > + if (*out_data == WMAX_FAILURE_CODE)
> > + return -EBADRQC;
> > +
> > + return 0;
> > +}
> > +
> > +static int wmax_thermal_control(u8 operation, u8 arg)
> > +{
> > + acpi_status status;
> > + struct wmax_u32_args in_args = {
> > + .operation = operation,
> > + .arg1 = arg,
> > + .arg2 = 0,
> > + .arg3 = 0,
> > + };
> > + u32 out_data;
> > +
> > + status = alienware_wmax_command(&in_args, sizeof(in_args),
> > + WMAX_METHOD_THERMAL_CONTROL,
> > + &out_data);
> > +
> > + if (ACPI_FAILURE(status))
> > + return -EIO;
> > +
> > + if (out_data == WMAX_FAILURE_CODE)
> > + return -EBADRQC;
> > +
> > + return 0;
> > +}
> > +
> > +static int wmax_gameshift_status(u8 operation, u32 *out_data)
> > +{
> > + acpi_status status;
> > + struct wmax_u32_args in_args = {
> > + .operation = operation,
> > + .arg1 = 0,
> > + .arg2 = 0,
> > + .arg3 = 0,
> > + };
> > +
> > + status = alienware_wmax_command(&in_args, sizeof(in_args),
> > + WMAX_METHOD_GAME_SHIFT_STATUS,
> > + out_data);
> > +
> > + if (ACPI_FAILURE(status))
> > + return -EIO;
> > +
> > + if (*out_data == WMAX_FAILURE_CODE)
> > + return -EOPNOTSUPP;
> > +
> > + return 0;
> > +}
> > +
> > +static int thermal_profile_get(struct platform_profile_handler *pprof,
> > + enum platform_profile_option *profile)
> > +{
> > + u32 out_data;
> > + int ret;
> > +
> > + ret = wmax_thermal_information(WMAX_OPERATION_CURRENT_PROFILE,
> > + 0, &out_data);
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (!is_wmax_thermal_code(out_data))
> > + return -ENODATA;
> > +
> > + out_data &= WMAX_THERMAL_MODE_MASK;
> > + *profile = wmax_mode_to_platform_profile[out_data];
> > +
> > + return 0;
> > +}
> > +
> > +static int thermal_profile_set(struct platform_profile_handler *pprof,
> > + enum platform_profile_option profile)
> > +{
> > + if (quirks->gmode == 1) {
> > + u32 gmode_status;
> > + int ret;
> > +
> > + ret = wmax_gameshift_status(WMAX_OPERATION_GET_GAME_SHIFT_STATUS,
> > + &gmode_status);
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + if ((profile == PLATFORM_PROFILE_PERFORMANCE && !gmode_status) ||
> > + (profile != PLATFORM_PROFILE_PERFORMANCE && gmode_status)) {
> > + ret = wmax_gameshift_status(WMAX_OPERATION_TOGGLE_GAME_SHIFT,
> > + &gmode_status);
> > +
> > + if (ret < 0)
> > + return ret;
> > + }
> > + }
> > +
> > + return wmax_thermal_control(WMAX_OPERATION_ACTIVATE_PROFILE,
> > + supported_thermal_profiles[profile]);
>
> If somebody has to look at this change from changelog history some years
> from now, it would be beneficial for the commit message to contain a few
> words to explain how this "game shift" thing relates to anything what is
> being done here. It's not exactly obvious from the naming alone and
> marketting may have already moved to the next term making it eventually
> hard to find some information about the term.
>
> You have some material in the docs change and the most relevant
> fragment(s) could perhaps be used in the commit message as well
> (especially as it seems to be based on some guesswork on its actual
> effect).
>
> If you reorder the patches 3 and 4, the commit message of this change
> could also ref to the doc for further information (but I'd prefer to have
> the relevant parts also in the commit message so the reasoning is
> communicated as per today's understanding, the doc files may get
> rewritten/changed over time).
Yes, I too prefer to make it clear how it works and why I added it in
the commit message.
>
>
> In any case, thanks a lot for working on this!
I apologize for the repetitive mistakes.
Thanks for your feedback!
Kurt
>
> --
> i.
>
> > +}
> > +
> > +static int create_thermal_profile(void)
> > +{
> > + u32 out_data;
> > + u32 gmode_status;
> > + enum wmax_thermal_mode mode;
> > + enum platform_profile_option profile;
> > + int ret;
> > +
> > + for (u8 i = 0x2; i <= 0xD; i++) {
> > + ret = wmax_thermal_information(WMAX_OPERATION_LIST_IDS,
> > + i, &out_data);
> > +
> > + if (ret == -EIO)
> > + return 0;
> > +
> > + if (ret == -EBADRQC)
> > + break;
> > +
> > + if (!is_wmax_thermal_code(out_data))
> > + continue;
> > +
> > + mode = out_data & WMAX_THERMAL_MODE_MASK;
> > + profile = wmax_mode_to_platform_profile[mode];
> > + supported_thermal_profiles[profile] = out_data;
> > +
> > + set_bit(profile, pp_handler.choices);
> > + }
> > +
> > + if (bitmap_empty(pp_handler.choices, PLATFORM_PROFILE_LAST))
> > + return 0;
> > +
> > + ret = wmax_gameshift_status(WMAX_OPERATION_GET_GAME_SHIFT_STATUS,
> > + &gmode_status);
> > +
> > + if (!ret) {
> > + supported_thermal_profiles[PLATFORM_PROFILE_PERFORMANCE] =
> > + WMAX_THERMAL_MODE_GMODE;
> > +
> > + set_bit(PLATFORM_PROFILE_PERFORMANCE, pp_handler.choices);
> > + quirks->gmode = 1;
> > + }
> > +
> > + pp_handler.profile_get = thermal_profile_get;
> > + pp_handler.profile_set = thermal_profile_set;
> > +
> > + ret = platform_profile_register(&pp_handler);
> > + if (ret < 0)
> > + return ret;
> > +
> > + quirks->thermal = 1;
> > +
> > + return 0;
> > +}
> > +
> > +static void remove_thermal_profile(void)
> > +{
> > + if (quirks->thermal > 0)
> > + platform_profile_remove();
> > +}
> > +
> > static int __init alienware_wmi_init(void)
> > {
> > int ret;
> > @@ -808,6 +1081,12 @@ static int __init alienware_wmi_init(void)
> > goto fail_prep_deepsleep;
> > }
> >
> > + if (interface == WMAX && quirks == &quirk_unknown) {
> > + ret = create_thermal_profile();
> > + if (ret)
> > + goto fail_prep_thermal_profile;
> > + }
> > +
> > ret = alienware_zone_init(platform_device);
> > if (ret)
> > goto fail_prep_zones;
> > @@ -816,6 +1095,8 @@ static int __init alienware_wmi_init(void)
> >
> > fail_prep_zones:
> > alienware_zone_exit(platform_device);
> > + remove_thermal_profile();
> > +fail_prep_thermal_profile:
> > fail_prep_deepsleep:
> > fail_prep_amplifier:
> > fail_prep_hdmi:
> > @@ -835,6 +1116,7 @@ static void __exit alienware_wmi_exit(void)
> > if (platform_device) {
> > alienware_zone_exit(platform_device);
> > remove_hdmi(platform_device);
> > + remove_thermal_profile();
> > platform_device_unregister(platform_device);
> > platform_driver_unregister(&platform_driver);
> > }
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v7 4/4] alienware-wmi: WMAX interface documentation
2024-10-24 6:15 [PATCH v7 0/4] Dell AWCC platform_profile support Kurt Borja
` (2 preceding siblings ...)
2024-10-24 6:19 ` [PATCH v7 3/4] alienware-wmi: added platform profile support Kurt Borja
@ 2024-10-24 6:21 ` Kurt Borja
3 siblings, 0 replies; 7+ messages in thread
From: Kurt Borja @ 2024-10-24 6:21 UTC (permalink / raw)
To: kuurtb; +Cc: W_Armin, hdegoede, ilpo.jarvinen, linux-kernel,
platform-driver-x86
Added documentation for new WMAX interface, present on some Alienware
X-Series, Alienware M-Series and Dell's G-Series laptops.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
---
v7:
- Added GameShiftStatus method to documentation
- Added remark about operation 0x03 of Thermal_Information method
- Removed undocumented methods
v6:
- Fixed typos
- Included new file in MAINTAINERS
---
Documentation/wmi/devices/alienware-wmi.rst | 388 ++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 389 insertions(+)
create mode 100644 Documentation/wmi/devices/alienware-wmi.rst
diff --git a/Documentation/wmi/devices/alienware-wmi.rst b/Documentation/wmi/devices/alienware-wmi.rst
new file mode 100644
index 000000000..36a67ff9a
--- /dev/null
+++ b/Documentation/wmi/devices/alienware-wmi.rst
@@ -0,0 +1,388 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+==============================================
+Dell AWCC WMI interface driver (alienware-wmi)
+==============================================
+
+Introduction
+============
+
+The WMI device WMAX has been implemented for many Alienware and Dell's G-Series
+models. Throughout these models, two implementations have been identified. The
+first one, used by older systems, deals with HDMI, brightness, RGB, amplifier
+and deep sleep control. The second one used by newer systems deals primarily
+with thermal, overclocking, and GPIO control.
+
+It is suspected that the latter is used by Alienware Command Center (AWCC) to
+manage manufacturer predefined thermal profiles. The alienware-wmi driver
+exposes Thermal_Information and Thermal_Control methods through the Platform
+Profile API to mimic AWCC's behavior.
+
+This newer interface, named AWCCMethodFunction has been reverse engineered, as
+Dell has not provided any official documentation. We will try to describe to the
+best of our ability its discovered inner workings.
+
+.. note::
+ The following method description may be incomplete and some operations have
+ different implementations between devices.
+
+WMI interface description
+-------------------------
+
+The WMI interface description can be decoded from the embedded binary MOF (bmof)
+data using the `bmfdec <https://github.com/pali/bmfdec>`_ utility:
+
+::
+
+ [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"), Description("WMI Function"), guid("{A70591CE-A997-11DA-B012-B622A1EF5492}")]
+ class AWCCWmiMethodFunction {
+ [key, read] string InstanceName;
+ [read] boolean Active;
+
+ [WmiMethodId(13), Implemented, read, write, Description("Return Overclocking Report.")] void Return_OverclockingReport([out] uint32 argr);
+ [WmiMethodId(14), Implemented, read, write, Description("Set OCUIBIOS Control.")] void Set_OCUIBIOSControl([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(15), Implemented, read, write, Description("Clear OC FailSafe Flag.")] void Clear_OCFailSafeFlag([out] uint32 argr);
+ [WmiMethodId(19), Implemented, read, write, Description("Get Fan Sensors.")] void GetFanSensors([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(20), Implemented, read, write, Description("Thermal Information.")] void Thermal_Information([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(21), Implemented, read, write, Description("Thermal Control.")] void Thermal_Control([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(23), Implemented, read, write, Description("MemoryOCControl.")] void MemoryOCControl([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(26), Implemented, read, write, Description("System Information.")] void SystemInformation([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(28), Implemented, read, write, Description("Power Information.")] void PowerInformation([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(32), Implemented, read, write, Description("FW Update GPIO toggle.")] void FWUpdateGPIOtoggle([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(33), Implemented, read, write, Description("Read Total of GPIOs.")] void ReadTotalofGPIOs([out] uint32 argr);
+ [WmiMethodId(34), Implemented, read, write, Description("Read GPIO pin Status.")] void ReadGPIOpPinStatus([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(35), Implemented, read, write, Description("Read Chassis Color.")] void ReadChassisColor([out] uint32 argr);
+ [WmiMethodId(36), Implemented, read, write, Description("Read Platform Properties.")] void ReadPlatformProperties([out] uint32 argr);
+ [WmiMethodId(37), Implemented, read, write, Description("Game Shift Status.")] void GameShiftStatus([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(128), Implemented, read, write, Description("Caldera SW installation.")] void CalderaSWInstallation([out] uint32 argr);
+ [WmiMethodId(129), Implemented, read, write, Description("Caldera SW is released.")] void CalderaSWReleased([out] uint32 argr);
+ [WmiMethodId(130), Implemented, read, write, Description("Caldera Connection Status.")] void CalderaConnectionStatus([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(131), Implemented, read, write, Description("Surprise Unplugged Flag Status.")] void SurpriseUnpluggedFlagStatus([out] uint32 argr);
+ [WmiMethodId(132), Implemented, read, write, Description("Clear Surprise Unplugged Flag.")] void ClearSurpriseUnpluggedFlag([out] uint32 argr);
+ [WmiMethodId(133), Implemented, read, write, Description("Cancel Undock Request.")] void CancelUndockRequest([out] uint32 argr);
+ [WmiMethodId(135), Implemented, read, write, Description("Devices in Caldera.")] void DevicesInCaldera([in] uint32 arg2, [out] uint32 argr);
+ [WmiMethodId(136), Implemented, read, write, Description("Notify BIOS for SW ready to disconnect Caldera.")] void NotifyBIOSForSWReadyToDisconnectCaldera([out] uint32 argr);
+ [WmiMethodId(160), Implemented, read, write, Description("Tobii SW installation.")] void TobiiSWinstallation([out] uint32 argr);
+ [WmiMethodId(161), Implemented, read, write, Description("Tobii SW Released.")] void TobiiSWReleased([out] uint32 argr);
+ [WmiMethodId(162), Implemented, read, write, Description("Tobii Camera Power Reset.")] void TobiiCameraPowerReset([out] uint32 argr);
+ [WmiMethodId(163), Implemented, read, write, Description("Tobii Camera Power On.")] void TobiiCameraPowerOn([out] uint32 argr);
+ [WmiMethodId(164), Implemented, read, write, Description("Tobii Camera Power Off.")] void TobiiCameraPowerOff([out] uint32 argr);
+ };
+
+Some of these methods get quite intricate so we will describe them using
+pseudo-code that vaguely resembles the original ASL code.
+
+Methods not described in the following document have unknown behavior.
+
+Argument Structure
+------------------
+
+All input arguments have type **uint32** and their structure is very similar
+between methods. Usually, the first byte corresponds to a specific *operation*
+the method performs, and the subsequent bytes correspond to *arguments* passed
+to this *operation*. For example, if an operation has code 0x01 and requires an
+ID 0xA0, the argument you would pass to the method is 0xA001.
+
+
+Thermal Methods
+===============
+
+WMI method Thermal_Information([in] uint32 arg2, [out] uint32 argr)
+-------------------------------------------------------------------
+
+::
+
+ if BYTE_0(arg2) == 0x01:
+ argr = 1
+
+ if BYTE_0(arg2) == 0x02:
+ argr = UNKNOWN_CONSTANT
+
+ if BYTE_0(arg2) == 0x03:
+ if BYTE_1(arg2) == 0x00:
+ argr = FAN_ID_0
+
+ if BYTE_1(arg2) == 0x01:
+ argr = FAN_ID_1
+
+ if BYTE_1(arg2) == 0x02:
+ argr = FAN_ID_2
+
+ if BYTE_1(arg2) == 0x03:
+ argr = FAN_ID_3
+
+ if BYTE_1(arg2) == 0x04:
+ argr = SENSOR_ID_CPU | 0x0100
+
+ if BYTE_1(arg2) == 0x05:
+ argr = SENSOR_ID_GPU | 0x0100
+
+ if BYTE_1(arg2) == 0x06:
+ argr = THERMAL_MODE_QUIET_ID
+
+ if BYTE_1(arg2) == 0x07:
+ argr = THERMAL_MODE_BALANCED_ID
+
+ if BYTE_1(arg2) == 0x08:
+ argr = THERMAL_MODE_BALANCED_PERFORMANCE_ID
+
+ if BYTE_1(arg2) == 0x09:
+ argr = THERMAL_MODE_PERFORMANCE_ID
+
+ if BYTE_1(arg2) == 0x0A:
+ argr = THERMAL_MODE_LOW_POWER_ID
+
+ if BYTE_1(arg2) == 0x0B:
+ argr = THERMAL_MODE_GMODE_ID
+
+ else:
+ argr = 0xFFFFFFFF
+
+ if BYTE_0(arg2) == 0x04:
+ if is_valid_sensor(BYTE_1(arg2)):
+ argr = SENSOR_TEMP_C
+ else:
+ argr = 0xFFFFFFFF
+
+ if BYTE_0(arg2) == 0x05:
+ if is_valid_fan(BYTE_1(arg2)):
+ argr = FAN_RPM()
+
+ if BYTE_0(arg2) == 0x06:
+ skip
+
+ if BYTE_0(arg2) == 0x07:
+ argr = 0
+
+ If BYTE_0(arg2) == 0x08:
+ if is_valid_fan(BYTE_1(arg2)):
+ argr = 0
+ else:
+ argr = 0xFFFFFFFF
+
+ if BYTE_0(arg2) == 0x09:
+ if is_valid_fan(BYTE_1(arg2)):
+ argr = FAN_UNKNOWN_STAT_0()
+
+ else:
+ argr = 0xFFFFFFFF
+
+ if BYTE_0(arg2) == 0x0A:
+ argr = THERMAL_MODE_BALANCED_ID
+
+ if BYTE_0(arg2) == 0x0B:
+ argr = CURRENT_THERMAL_MODE()
+
+ if BYTE_0(arg2) == 0x0C:
+ if is_valid_fan(BYTE_1(arg2)):
+ argr = FAN_UNKNOWN_STAT_1()
+ else:
+ argr = 0xFFFFFFFF
+
+Operation 0x03 list all available fan IDs, sensor IDs and thermal profile
+codes in order, but different models may have different number of fans and
+thermal profiles. These are the known ranges:
+
+* Fan IDs: from 2 up to 4
+* Sensor IDs: 2
+* Thermal profile codes: from 1 up to 7
+
+In total BYTE_1(ARG2) may range from 0x5 up to 0xD depending on the model.
+
+WMI method Thermal_Control([in] uint32 arg2, [out] uint32 argr)
+---------------------------------------------------------------
+
+::
+
+ if BYTE_0(arg2) == 0x01:
+ if is_valid_thermal_profile(BYTE_1(arg2)):
+ SET_THERMAL_PROFILE(BYTE_1(arg2))
+ argr = 0
+
+ if BYTE_0(arg2) == 0x02:
+ if is_valid_fan(BYTE_1(arg2)):
+ SET_FAN_SPEED_MULTIPLIER(BYTE_2(arg2))
+ argr = 0
+ else:
+ argr = 0xFFFFFFFF
+
+.. note::
+ While you can manually change the fan speed multiplier with this method,
+ Dell's BIOS tends to overwrite this changes anyway.
+
+These are the known thermal profile codes:
+
+::
+
+ CUSTOM 0x00
+
+ BALANCED_USTT 0xA0
+ BALANCED_PERFORMANCE_USTT 0xA1
+ COOL_USTT 0xA2
+ QUIET_USTT 0xA3
+ PERFORMANCE_USTT 0xA4
+ LOW_POWER_USTT 0xA5
+
+ QUIET 0x96
+ BALANCED 0x97
+ BALANCED_PERFORMANCE 0x98
+ PERFORMANCE 0x99
+
+ GMODE 0xAB
+
+Usually if a model doesn't support the first four profiles they will support
+the User Selectable Thermal Tables (USTT) profiles and vice-versa.
+
+GMODE replaces PERFORMANCE in G-Series laptops.
+
+WMI method GameShiftStatus([in] uint32 arg2, [out] uint32 argr)
+---------------------------------------------------------------
+
+::
+
+ if BYTE_0(arg2) == 0x1:
+ TOGGLE_GAME_SHIFT()
+ argr = GET_GAME_SHIFT_STATUS()
+
+ if BYTE_0(arg2) == 0x2:
+ argr = GET_GAME_SHIFT_STATUS()
+
+Game Shift Status does not change the fan speed profile but it could be some
+sort of CPU/GPU power profile. Benchmarks have not been done.
+
+This method is only present on Dell's G-Series laptops and it's implementation
+implies GMODE thermal profile is available, even if operation 0x03 of
+Thermal_Information does not list it.
+
+G-key on Dell's G-Series laptops also changes Game Shift status, so both are
+directly related.
+
+WMI method GetFanSensors([in] uint32 arg2, [out] uint32 argr)
+-------------------------------------------------------------
+
+::
+
+ if BYTE_0(arg2) == 0x1:
+ if is_valid_fan(BYTE_1(arg2)):
+ argr = 1
+ else:
+ argr = 0
+
+ if BYTE_0(arg2) == 0x2:
+ if is_valid_fan(BYTE_1(arg2)):
+ if BYTE_2(arg2) == 0:
+ argr == SENSOR_ID
+ else
+ argr == 0xFFFFFFFF
+ else:
+ argr = 0
+
+Overclocking Methods
+====================
+
+.. warning::
+ These methods have not been tested and are only partially reverse
+ engineered.
+
+WMI method Return_OverclockingReport([out] uint32 argr)
+-------------------------------------------------------
+
+::
+
+ CSMI (0xE3, 0x99)
+ argr = 0
+
+CSMI is an unknown operation.
+
+WMI method Set_OCUIBIOSControl([in] uint32 arg2, [out] uint32 argr)
+-------------------------------------------------------------------
+
+::
+
+ CSMI (0xE3, 0x99)
+ argr = 0
+
+CSMI is an unknown operation.
+
+WMI method Clear_OCFailSafeFlag([out] uint32 argr)
+--------------------------------------------------
+
+::
+
+ CSMI (0xE3, 0x99)
+ argr = 0
+
+CSMI is an unknown operation.
+
+
+WMI method MemoryOCControl([in] uint32 arg2, [out] uint32 argr)
+---------------------------------------------------------------
+
+AWCC supports memory overclocking, but this method is very intricate and has
+not been deciphered yet.
+
+GPIO methods
+============
+
+These methods are probably related to some kind of firmware update system,
+through a GPIO device.
+
+.. warning::
+ These methods have not been tested and are only partially reverse
+ engineered.
+
+WMI method FWUpdateGPIOtoggle([in] uint32 arg2, [out] uint32 argr)
+------------------------------------------------------------------
+
+::
+
+ if BYTE_0(arg2) == 0:
+ if BYTE_1(arg2) == 1:
+ SET_PIN_A_HIGH()
+ else:
+ SET_PIN_A_LOW()
+
+ if BYTE_0(arg2) == 1:
+ if BYTE_1(arg2) == 1:
+ SET_PIN_B_HIGH()
+
+ else:
+ SET_PIN_B_LOW()
+
+ else:
+ argr = 1
+
+WMI method ReadTotalofGPIOs([out] uint32 argr)
+----------------------------------------------
+
+::
+
+ argr = 0x02
+
+WMI method ReadGPIOpPinStatus([in] uint32 arg2, [out] uint32 argr)
+------------------------------------------------------------------
+
+::
+
+ if BYTE_0(arg2) == 0:
+ argr = PIN_A_STATUS
+
+ if BYTE_0(arg2) == 1:
+ argr = PIN_B_STATUS
+
+Other information Methods
+=========================
+
+WMI method ReadChassisColor([out] uint32 argr)
+----------------------------------------------
+
+::
+
+ argr = CHASSIS_COLOR_ID
+
+Acknowledgements
+================
+
+Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ for documenting
+and testing available thermal profile codes.
+
diff --git a/MAINTAINERS b/MAINTAINERS
index c27f31907..25f6de4c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -792,6 +792,7 @@ F: drivers/perf/alibaba_uncore_drw_pmu.c
ALIENWARE WMI DRIVER
L: Dell.Client.Kernel@dell.com
S: Maintained
+F: Documentation/wmi/devices/alienware-wmi.rst
F: drivers/platform/x86/dell/alienware-wmi.c
ALLEGRO DVT VIDEO IP CORE DRIVER
--
2.47.0
^ permalink raw reply related [flat|nested] 7+ messages in thread