* [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements
@ 2025-03-06 0:56 Kurt Borja
2025-03-06 0:56 ` [PATCH v3 01/10] platform/x86: alienware-wmi-wmax: Rename thermal related symbols Kurt Borja
` (9 more replies)
0 siblings, 10 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel, Guenter Roeck, Jean Delvare,
linux-hwmon
Hi all,
This set mainly adds hwmon and manual fan control support (patches 7-8)
to the alienware-wmi driver, after some improvements.
Aside from some minor changes Ilpo commented on, I added inline helpers
for most awcc operations to make those calls more compact.
Thank you for your feedback :)
---
Changes since v2:
[02/10]
- Move BIT(8) flag comment to AWCC_RESOURCE_ID_MASK definition
[03/10]
- Add awcc_profile_id_to_pprof()
- Add awcc_op_activate_profile()
- Dropped Armin's rev-by because the patch changed a bit
[05/10]
- Dropped __packed attribute from system_description
[07/10]
- Add awcc_op_get_fan_rpm()
- Add awcc_op_get_temperature()
- Use ternary conditional operator on awcc_hwmon_is_visible()
- Check priv->temp_sensors weight to determine hwmon_temp visibility
- Replace U8_MAX with priv->temp_sensors_size in find_nth_bit() calls
- Drop find_nth_bit() failure check due to the visibility change
- Cc+ Jean Delvare <jdelvare@suse.com>
- Cc+ linux-hwmon@vger.kernel.org
[08/10]
- Add a pwm1_enable attribute
- Add awcc_op_get_fan_boost()
- Add awcc_op_set_fan_boost()
- Cc+ Jean Delvare <jdelvare@suse.com>
- Cc+ linux-hwmon@vger.kernel.org
[10/10]
- Reword commit title to reflect path
v2: https://lore.kernel.org/r/20250225222500.23535-1-kuurtb@gmail.com
---
Kurt Borja (10):
platform/x86: alienware-wmi-wmax: Rename thermal related symbols
platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode()
platform/x86: alienware-wmi-wmax: Improve internal AWCC API
platform/x86: alienware-wmi-wmax: Modify supported_thermal_profiles[]
platform/x86: alienware-wmi-wmax: Improve platform profile probe
platform/x86: alienware-wmi-wmax: Add support for the "custom" thermal profile
platform/x86: alienware-wmi-wmax: Add HWMON support
platform/x86: alienware-wmi-wmax: Add support for manual fan control
platform/x86: alienware-wmi-wmax: Add a DebugFS interface
Documentation: wmi: Improve and update alienware-wmi documentation
Documentation/wmi/devices/alienware-wmi.rst | 383 +++-----
drivers/platform/x86/dell/Kconfig | 1 +
drivers/platform/x86/dell/alienware-wmi-wmax.c | 1132 ++++++++++++++++++++----
3 files changed, 1098 insertions(+), 418 deletions(-)
---
base-commit: 5ad6d62c9b183314ec1c64a95a26636e973e736a
change-id: 20250305-hwm-f7bd91902b57
Best regards,
--
~ Kurt
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 01/10] platform/x86: alienware-wmi-wmax: Rename thermal related symbols
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 0:56 ` [PATCH v3 02/10] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode() Kurt Borja
` (8 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
The "thermal" features of the WMAX WMI device are only present on the
host device if the ACPI _UID is "AWCC". Replace WMAX prefixes with
"AWCC" to reflect this relationship.
Thermal profiles with WMAX_PROFILE_BASIC prefix are also renamed to
WMAX_PROFILE_LEGACY because they are only supported in older versions
of this WMI device.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 173 +++++++++++++------------
1 file changed, 87 insertions(+), 86 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index 3d3014b5adf046c94c1ebf39a0e28a92622b40d6..ed70e12d73d7fe5d89f3364c5367820bf47e3c1e 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -24,16 +24,17 @@
#define WMAX_METHOD_DEEP_SLEEP_STATUS 0x0C
#define WMAX_METHOD_BRIGHTNESS 0x3
#define WMAX_METHOD_ZONE_CONTROL 0x4
-#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 AWCC_METHOD_THERMAL_INFORMATION 0x14
+#define AWCC_METHOD_THERMAL_CONTROL 0x15
+#define AWCC_METHOD_GAME_SHIFT_STATUS 0x25
-#define WMAX_FAILURE_CODE 0xFFFFFFFF
-#define WMAX_THERMAL_TABLE_MASK GENMASK(7, 4)
-#define WMAX_THERMAL_MODE_MASK GENMASK(3, 0)
-#define WMAX_SENSOR_ID_MASK BIT(8)
+#define AWCC_THERMAL_MODE_GMODE 0xAB
+
+#define AWCC_FAILURE_CODE 0xFFFFFFFF
+#define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
+#define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
+#define AWCC_SENSOR_ID_MASK BIT(8)
static bool force_platform_profile;
module_param_unsafe(force_platform_profile, bool, 0);
@@ -151,38 +152,38 @@ static const struct dmi_system_id awcc_dmi_table[] __initconst = {
},
};
-enum WMAX_THERMAL_INFORMATION_OPERATIONS {
- WMAX_OPERATION_SYS_DESCRIPTION = 0x02,
- WMAX_OPERATION_LIST_IDS = 0x03,
- WMAX_OPERATION_CURRENT_PROFILE = 0x0B,
+enum AWCC_THERMAL_INFORMATION_OPERATIONS {
+ AWCC_OP_GET_SYSTEM_DESCRIPTION = 0x02,
+ AWCC_OP_GET_RESOURCE_ID = 0x03,
+ AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
};
-enum WMAX_THERMAL_CONTROL_OPERATIONS {
- WMAX_OPERATION_ACTIVATE_PROFILE = 0x01,
+enum AWCC_THERMAL_CONTROL_OPERATIONS {
+ AWCC_OP_ACTIVATE_PROFILE = 0x01,
};
-enum WMAX_GAME_SHIFT_STATUS_OPERATIONS {
- WMAX_OPERATION_TOGGLE_GAME_SHIFT = 0x01,
- WMAX_OPERATION_GET_GAME_SHIFT_STATUS = 0x02,
+enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
+ AWCC_OP_TOGGLE_GAME_SHIFT = 0x01,
+ AWCC_OP_GET_GAME_SHIFT_STATUS = 0x02,
};
-enum WMAX_THERMAL_TABLES {
- WMAX_THERMAL_TABLE_BASIC = 0x90,
- WMAX_THERMAL_TABLE_USTT = 0xA0,
+enum AWCC_THERMAL_TABLES {
+ AWCC_THERMAL_TABLE_LEGACY = 0x90,
+ AWCC_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,
+enum awcc_thermal_profile {
+ AWCC_PROFILE_USTT_BALANCED,
+ AWCC_PROFILE_USTT_BALANCED_PERFORMANCE,
+ AWCC_PROFILE_USTT_COOL,
+ AWCC_PROFILE_USTT_QUIET,
+ AWCC_PROFILE_USTT_PERFORMANCE,
+ AWCC_PROFILE_USTT_LOW_POWER,
+ AWCC_PROFILE_LEGACY_QUIET,
+ AWCC_PROFILE_LEGACY_BALANCED,
+ AWCC_PROFILE_LEGACY_BALANCED_PERFORMANCE,
+ AWCC_PROFILE_LEGACY_PERFORMANCE,
+ AWCC_PROFILE_LAST,
};
struct wmax_led_args {
@@ -210,20 +211,20 @@ struct wmax_u32_args {
struct awcc_priv {
struct wmi_device *wdev;
struct device *ppdev;
- enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
+ enum awcc_thermal_profile supported_thermal_profiles[PLATFORM_PROFILE_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,
+static const enum platform_profile_option awcc_mode_to_platform_profile[AWCC_PROFILE_LAST] = {
+ [AWCC_PROFILE_USTT_BALANCED] = PLATFORM_PROFILE_BALANCED,
+ [AWCC_PROFILE_USTT_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
+ [AWCC_PROFILE_USTT_COOL] = PLATFORM_PROFILE_COOL,
+ [AWCC_PROFILE_USTT_QUIET] = PLATFORM_PROFILE_QUIET,
+ [AWCC_PROFILE_USTT_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
+ [AWCC_PROFILE_USTT_LOW_POWER] = PLATFORM_PROFILE_LOW_POWER,
+ [AWCC_PROFILE_LEGACY_QUIET] = PLATFORM_PROFILE_QUIET,
+ [AWCC_PROFILE_LEGACY_BALANCED] = PLATFORM_PROFILE_BALANCED,
+ [AWCC_PROFILE_LEGACY_BALANCED_PERFORMANCE] = PLATFORM_PROFILE_BALANCED_PERFORMANCE,
+ [AWCC_PROFILE_LEGACY_PERFORMANCE] = PLATFORM_PROFILE_PERFORMANCE,
};
static struct awcc_quirks *awcc;
@@ -444,26 +445,26 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
* Thermal Profile control
* - Provides thermal profile control through the Platform Profile API
*/
-static bool is_wmax_thermal_code(u32 code)
+static bool is_awcc_thermal_mode(u32 code)
{
- if (code & WMAX_SENSOR_ID_MASK)
+ if (code & AWCC_SENSOR_ID_MASK)
return false;
- if ((code & WMAX_THERMAL_MODE_MASK) >= THERMAL_MODE_LAST)
+ if ((code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LAST)
return false;
- if ((code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_BASIC &&
- (code & WMAX_THERMAL_MODE_MASK) >= THERMAL_MODE_BASIC_QUIET)
+ if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_LEGACY &&
+ (code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LEGACY_QUIET)
return true;
- if ((code & WMAX_THERMAL_TABLE_MASK) == WMAX_THERMAL_TABLE_USTT &&
- (code & WMAX_THERMAL_MODE_MASK) <= THERMAL_MODE_USTT_LOW_POWER)
+ if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_USTT &&
+ (code & AWCC_THERMAL_MODE_MASK) <= AWCC_PROFILE_USTT_LOW_POWER)
return true;
return false;
}
-static int wmax_thermal_information(struct wmi_device *wdev, u8 operation,
+static int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
u8 arg, u32 *out_data)
{
struct wmax_u32_args in_args = {
@@ -474,21 +475,21 @@ static int wmax_thermal_information(struct wmi_device *wdev, u8 operation,
};
int ret;
- ret = alienware_wmi_command(wdev, WMAX_METHOD_THERMAL_INFORMATION,
+ ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION,
&in_args, sizeof(in_args), out_data);
if (ret < 0)
return ret;
- if (*out_data == WMAX_FAILURE_CODE)
+ if (*out_data == AWCC_FAILURE_CODE)
return -EBADRQC;
return 0;
}
-static int wmax_thermal_control(struct wmi_device *wdev, u8 profile)
+static int awcc_thermal_control(struct wmi_device *wdev, u8 profile)
{
struct wmax_u32_args in_args = {
- .operation = WMAX_OPERATION_ACTIVATE_PROFILE,
+ .operation = AWCC_OP_ACTIVATE_PROFILE,
.arg1 = profile,
.arg2 = 0,
.arg3 = 0,
@@ -496,18 +497,18 @@ static int wmax_thermal_control(struct wmi_device *wdev, u8 profile)
u32 out_data;
int ret;
- ret = alienware_wmi_command(wdev, WMAX_METHOD_THERMAL_CONTROL,
+ ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL,
&in_args, sizeof(in_args), &out_data);
if (ret)
return ret;
- if (out_data == WMAX_FAILURE_CODE)
+ if (out_data == AWCC_FAILURE_CODE)
return -EBADRQC;
return 0;
}
-static int wmax_game_shift_status(struct wmi_device *wdev, u8 operation,
+static int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
u32 *out_data)
{
struct wmax_u32_args in_args = {
@@ -518,46 +519,46 @@ static int wmax_game_shift_status(struct wmi_device *wdev, u8 operation,
};
int ret;
- ret = alienware_wmi_command(wdev, WMAX_METHOD_GAME_SHIFT_STATUS,
+ ret = alienware_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS,
&in_args, sizeof(in_args), out_data);
if (ret < 0)
return ret;
- if (*out_data == WMAX_FAILURE_CODE)
+ if (*out_data == AWCC_FAILURE_CODE)
return -EOPNOTSUPP;
return 0;
}
-static int thermal_profile_get(struct device *dev,
- enum platform_profile_option *profile)
+static int awcc_platform_profile_get(struct device *dev,
+ enum platform_profile_option *profile)
{
struct awcc_priv *priv = dev_get_drvdata(dev);
u32 out_data;
int ret;
- ret = wmax_thermal_information(priv->wdev, WMAX_OPERATION_CURRENT_PROFILE,
+ ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_CURRENT_PROFILE,
0, &out_data);
if (ret < 0)
return ret;
- if (out_data == WMAX_THERMAL_MODE_GMODE) {
+ if (out_data == AWCC_THERMAL_MODE_GMODE) {
*profile = PLATFORM_PROFILE_PERFORMANCE;
return 0;
}
- if (!is_wmax_thermal_code(out_data))
+ if (!is_awcc_thermal_mode(out_data))
return -ENODATA;
- out_data &= WMAX_THERMAL_MODE_MASK;
- *profile = wmax_mode_to_platform_profile[out_data];
+ out_data &= AWCC_THERMAL_MODE_MASK;
+ *profile = awcc_mode_to_platform_profile[out_data];
return 0;
}
-static int thermal_profile_set(struct device *dev,
- enum platform_profile_option profile)
+static int awcc_platform_profile_set(struct device *dev,
+ enum platform_profile_option profile)
{
struct awcc_priv *priv = dev_get_drvdata(dev);
@@ -565,8 +566,8 @@ static int thermal_profile_set(struct device *dev,
u32 gmode_status;
int ret;
- ret = wmax_game_shift_status(priv->wdev,
- WMAX_OPERATION_GET_GAME_SHIFT_STATUS,
+ ret = awcc_game_shift_status(priv->wdev,
+ AWCC_OP_GET_GAME_SHIFT_STATUS,
&gmode_status);
if (ret < 0)
@@ -574,8 +575,8 @@ static int thermal_profile_set(struct device *dev,
if ((profile == PLATFORM_PROFILE_PERFORMANCE && !gmode_status) ||
(profile != PLATFORM_PROFILE_PERFORMANCE && gmode_status)) {
- ret = wmax_game_shift_status(priv->wdev,
- WMAX_OPERATION_TOGGLE_GAME_SHIFT,
+ ret = awcc_game_shift_status(priv->wdev,
+ AWCC_OP_TOGGLE_GAME_SHIFT,
&gmode_status);
if (ret < 0)
@@ -583,21 +584,21 @@ static int thermal_profile_set(struct device *dev,
}
}
- return wmax_thermal_control(priv->wdev,
+ return awcc_thermal_control(priv->wdev,
priv->supported_thermal_profiles[profile]);
}
-static int thermal_profile_probe(void *drvdata, unsigned long *choices)
+static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
{
enum platform_profile_option profile;
struct awcc_priv *priv = drvdata;
- enum wmax_thermal_mode mode;
+ enum awcc_thermal_profile mode;
u8 sys_desc[4];
u32 first_mode;
u32 out_data;
int ret;
- ret = wmax_thermal_information(priv->wdev, WMAX_OPERATION_SYS_DESCRIPTION,
+ ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
0, (u32 *) &sys_desc);
if (ret < 0)
return ret;
@@ -605,7 +606,7 @@ static int thermal_profile_probe(void *drvdata, unsigned long *choices)
first_mode = sys_desc[0] + sys_desc[1];
for (u32 i = 0; i < sys_desc[3]; i++) {
- ret = wmax_thermal_information(priv->wdev, WMAX_OPERATION_LIST_IDS,
+ ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_RESOURCE_ID,
i + first_mode, &out_data);
if (ret == -EIO)
@@ -614,11 +615,11 @@ static int thermal_profile_probe(void *drvdata, unsigned long *choices)
if (ret == -EBADRQC)
break;
- if (!is_wmax_thermal_code(out_data))
+ if (!is_awcc_thermal_mode(out_data))
continue;
- mode = out_data & WMAX_THERMAL_MODE_MASK;
- profile = wmax_mode_to_platform_profile[mode];
+ mode = out_data & AWCC_THERMAL_MODE_MASK;
+ profile = awcc_mode_to_platform_profile[mode];
priv->supported_thermal_profiles[profile] = out_data;
set_bit(profile, choices);
@@ -629,7 +630,7 @@ static int thermal_profile_probe(void *drvdata, unsigned long *choices)
if (awcc->gmode) {
priv->supported_thermal_profiles[PLATFORM_PROFILE_PERFORMANCE] =
- WMAX_THERMAL_MODE_GMODE;
+ AWCC_THERMAL_MODE_GMODE;
set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
}
@@ -638,9 +639,9 @@ static int thermal_profile_probe(void *drvdata, unsigned long *choices)
}
static const struct platform_profile_ops awcc_platform_profile_ops = {
- .probe = thermal_profile_probe,
- .profile_get = thermal_profile_get,
- .profile_set = thermal_profile_set,
+ .probe = awcc_platform_profile_probe,
+ .profile_get = awcc_platform_profile_get,
+ .profile_set = awcc_platform_profile_set,
};
static int awcc_platform_profile_init(struct wmi_device *wdev)
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 02/10] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode()
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
2025-03-06 0:56 ` [PATCH v3 01/10] platform/x86: alienware-wmi-wmax: Rename thermal related symbols Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 0:56 ` [PATCH v3 03/10] platform/x86: alienware-wmi-wmax: Improve internal AWCC API Kurt Borja
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
Refactor is_awcc_thermal_mode() to use FIELD_GET() instead of bitwise
operations. Drop the check for BIT(8) sensor flag and rename it to
is_awcc_thermal_profile_id().
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 33 +++++++++++++-------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index ed70e12d73d7fe5d89f3364c5367820bf47e3c1e..80aefba5b22d6b4ac18aeb2ca356f8c911150abd 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -34,7 +34,8 @@
#define AWCC_FAILURE_CODE 0xFFFFFFFF
#define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
#define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
-#define AWCC_SENSOR_ID_MASK BIT(8)
+/* Some IDs have a BIT(8) flag that we ignore */
+#define AWCC_RESOURCE_ID_MASK GENMASK(7, 0)
static bool force_platform_profile;
module_param_unsafe(force_platform_profile, bool, 0);
@@ -168,8 +169,8 @@ enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
};
enum AWCC_THERMAL_TABLES {
- AWCC_THERMAL_TABLE_LEGACY = 0x90,
- AWCC_THERMAL_TABLE_USTT = 0xA0,
+ AWCC_THERMAL_TABLE_LEGACY = 0x9,
+ AWCC_THERMAL_TABLE_USTT = 0xA,
};
enum awcc_thermal_profile {
@@ -445,20 +446,18 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
* Thermal Profile control
* - Provides thermal profile control through the Platform Profile API
*/
-static bool is_awcc_thermal_mode(u32 code)
+static bool is_awcc_thermal_profile_id(u8 code)
{
- if (code & AWCC_SENSOR_ID_MASK)
+ u8 table = FIELD_GET(AWCC_THERMAL_TABLE_MASK, code);
+ u8 mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, code);
+
+ if (mode >= AWCC_PROFILE_LAST)
return false;
- if ((code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LAST)
- return false;
-
- if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_LEGACY &&
- (code & AWCC_THERMAL_MODE_MASK) >= AWCC_PROFILE_LEGACY_QUIET)
+ if (table == AWCC_THERMAL_TABLE_LEGACY && mode >= AWCC_PROFILE_LEGACY_QUIET)
return true;
- if ((code & AWCC_THERMAL_TABLE_MASK) == AWCC_THERMAL_TABLE_USTT &&
- (code & AWCC_THERMAL_MODE_MASK) <= AWCC_PROFILE_USTT_LOW_POWER)
+ if (table == AWCC_THERMAL_TABLE_USTT && mode <= AWCC_PROFILE_USTT_LOW_POWER)
return true;
return false;
@@ -548,7 +547,7 @@ static int awcc_platform_profile_get(struct device *dev,
return 0;
}
- if (!is_awcc_thermal_mode(out_data))
+ if (!is_awcc_thermal_profile_id(out_data))
return -ENODATA;
out_data &= AWCC_THERMAL_MODE_MASK;
@@ -597,6 +596,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
u32 first_mode;
u32 out_data;
int ret;
+ u8 id;
ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
0, (u32 *) &sys_desc);
@@ -615,12 +615,13 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
if (ret == -EBADRQC)
break;
- if (!is_awcc_thermal_mode(out_data))
+ id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
+ if (!is_awcc_thermal_profile_id(id))
continue;
- mode = out_data & AWCC_THERMAL_MODE_MASK;
+ mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
profile = awcc_mode_to_platform_profile[mode];
- priv->supported_thermal_profiles[profile] = out_data;
+ priv->supported_thermal_profiles[profile] = id;
set_bit(profile, choices);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 03/10] platform/x86: alienware-wmi-wmax: Improve internal AWCC API
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
2025-03-06 0:56 ` [PATCH v3 01/10] platform/x86: alienware-wmi-wmax: Rename thermal related symbols Kurt Borja
2025-03-06 0:56 ` [PATCH v3 02/10] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode() Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 21:27 ` Armin Wolf
2025-03-06 0:56 ` [PATCH v3 04/10] platform/x86: alienware-wmi-wmax: Modify supported_thermal_profiles[] Kurt Borja
` (6 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
Inline all AWCC WMI helper methods and directly return the newly
introduced __awcc_wmi_command() helper to simplify implementation.
Drop awcc_thermal_control() in favor of awcc_op_activate_profile().
Add awcc_op_get_resource_id() and awcc_profile_id_to_pprof() helpers to
support upcoming changes, as well as a new failure code.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 176 +++++++++++++++----------
1 file changed, 110 insertions(+), 66 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index 80aefba5b22d6b4ac18aeb2ca356f8c911150abd..a43373717bd4580e8f62a7263e67664630165e28 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -32,6 +32,7 @@
#define AWCC_THERMAL_MODE_GMODE 0xAB
#define AWCC_FAILURE_CODE 0xFFFFFFFF
+#define AWCC_FAILURE_CODE_2 0xFFFFFFFE
#define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
#define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
/* Some IDs have a BIT(8) flag that we ignore */
@@ -443,8 +444,7 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
};
/*
- * Thermal Profile control
- * - Provides thermal profile control through the Platform Profile API
+ * AWCC Helpers
*/
static bool is_awcc_thermal_profile_id(u8 code)
{
@@ -463,95 +463,140 @@ static bool is_awcc_thermal_profile_id(u8 code)
return false;
}
-static int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
- u8 arg, u32 *out_data)
+static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
+ struct wmax_u32_args *args, u32 *out)
{
- struct wmax_u32_args in_args = {
+ int ret;
+
+ ret = alienware_wmi_command(wdev, method_id, args, sizeof(*args), out);
+ if (ret)
+ return ret;
+
+ if (*out == AWCC_FAILURE_CODE || *out == AWCC_FAILURE_CODE_2)
+ return -EBADRQC;
+
+ return 0;
+}
+
+static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
+ u8 arg, u32 *out)
+{
+ struct wmax_u32_args args = {
.operation = operation,
.arg1 = arg,
.arg2 = 0,
.arg3 = 0,
};
- int ret;
- ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION,
- &in_args, sizeof(in_args), out_data);
- if (ret < 0)
- return ret;
-
- if (*out_data == AWCC_FAILURE_CODE)
- return -EBADRQC;
-
- return 0;
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
}
-static int awcc_thermal_control(struct wmi_device *wdev, u8 profile)
+static inline int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
+ u32 *out)
{
- struct wmax_u32_args in_args = {
- .operation = AWCC_OP_ACTIVATE_PROFILE,
- .arg1 = profile,
- .arg2 = 0,
- .arg3 = 0,
- };
- u32 out_data;
- int ret;
-
- ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL,
- &in_args, sizeof(in_args), &out_data);
- if (ret)
- return ret;
-
- if (out_data == AWCC_FAILURE_CODE)
- return -EBADRQC;
-
- return 0;
-}
-
-static int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
- u32 *out_data)
-{
- struct wmax_u32_args in_args = {
+ struct wmax_u32_args args = {
.operation = operation,
.arg1 = 0,
.arg2 = 0,
.arg3 = 0,
};
- int ret;
- ret = alienware_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS,
- &in_args, sizeof(in_args), out_data);
- if (ret < 0)
- return ret;
+ return __awcc_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS, &args, out);
+}
- if (*out_data == AWCC_FAILURE_CODE)
- return -EOPNOTSUPP;
+/**
+ * awcc_op_get_resource_id - Get the resource ID at a given index
+ * @wdev: AWCC WMI device
+ * @index: Index
+ * @out: Value returned by the WMI call
+ *
+ * Get the resource ID at a given index. Resource IDs are listed in the
+ * following order:
+ *
+ * - Fan IDs
+ * - Sensor IDs
+ * - Unknown IDs
+ * - Thermal Profile IDs
+ *
+ * The total number of IDs of a given type can be obtained with
+ * AWCC_OP_GET_SYSTEM_DESCRIPTION.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_GET_RESOURCE_ID,
+ .arg1 = index,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
+}
+
+static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_GET_CURRENT_PROFILE,
+ .arg1 = 0,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
+}
+
+static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_ACTIVATE_PROFILE,
+ .arg1 = profile,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+ u32 out;
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
+}
+
+static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profile)
+{
+ switch (id) {
+ case AWCC_THERMAL_MODE_GMODE:
+ *profile = PLATFORM_PROFILE_PERFORMANCE;
+ return 0;
+ default:
+ break;
+ }
+
+ if (!is_awcc_thermal_profile_id(id))
+ return -ENODATA;
+
+ id = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
+ *profile = awcc_mode_to_platform_profile[id];
return 0;
}
+/*
+ * Thermal Profile control
+ * - Provides thermal profile control through the Platform Profile API
+ */
static int awcc_platform_profile_get(struct device *dev,
enum platform_profile_option *profile)
{
struct awcc_priv *priv = dev_get_drvdata(dev);
- u32 out_data;
+ u32 profile_id;
int ret;
- ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_CURRENT_PROFILE,
- 0, &out_data);
-
- if (ret < 0)
+ ret = awcc_op_get_current_profile(priv->wdev, &profile_id);
+ if (ret)
return ret;
- if (out_data == AWCC_THERMAL_MODE_GMODE) {
- *profile = PLATFORM_PROFILE_PERFORMANCE;
- return 0;
- }
-
- if (!is_awcc_thermal_profile_id(out_data))
- return -ENODATA;
-
- out_data &= AWCC_THERMAL_MODE_MASK;
- *profile = awcc_mode_to_platform_profile[out_data];
+ ret = awcc_profile_id_to_pprof(profile_id, profile);
+ if (ret)
+ return ret;
return 0;
}
@@ -583,8 +628,8 @@ static int awcc_platform_profile_set(struct device *dev,
}
}
- return awcc_thermal_control(priv->wdev,
- priv->supported_thermal_profiles[profile]);
+ return awcc_op_activate_profile(priv->wdev,
+ priv->supported_thermal_profiles[profile]);
}
static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
@@ -606,8 +651,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
first_mode = sys_desc[0] + sys_desc[1];
for (u32 i = 0; i < sys_desc[3]; i++) {
- ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_RESOURCE_ID,
- i + first_mode, &out_data);
+ ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
if (ret == -EIO)
return ret;
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 04/10] platform/x86: alienware-wmi-wmax: Modify supported_thermal_profiles[]
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
` (2 preceding siblings ...)
2025-03-06 0:56 ` [PATCH v3 03/10] platform/x86: alienware-wmi-wmax: Improve internal AWCC API Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 0:56 ` [PATCH v3 05/10] platform/x86: alienware-wmi-wmax: Improve platform profile probe Kurt Borja
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
Rename supported_thermal_profiles[] -> supported_profiles[] and change
it's type to u8 because it stores AWCC thermal IDs.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index a43373717bd4580e8f62a7263e67664630165e28..8e91fb4b349b7b62b6f914ac68d5eb1cd30a606e 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -213,7 +213,7 @@ struct wmax_u32_args {
struct awcc_priv {
struct wmi_device *wdev;
struct device *ppdev;
- enum awcc_thermal_profile supported_thermal_profiles[PLATFORM_PROFILE_LAST];
+ u8 supported_profiles[PLATFORM_PROFILE_LAST];
};
static const enum platform_profile_option awcc_mode_to_platform_profile[AWCC_PROFILE_LAST] = {
@@ -628,8 +628,7 @@ static int awcc_platform_profile_set(struct device *dev,
}
}
- return awcc_op_activate_profile(priv->wdev,
- priv->supported_thermal_profiles[profile]);
+ return awcc_op_activate_profile(priv->wdev, priv->supported_profiles[profile]);
}
static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
@@ -665,7 +664,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
profile = awcc_mode_to_platform_profile[mode];
- priv->supported_thermal_profiles[profile] = id;
+ priv->supported_profiles[profile] = id;
set_bit(profile, choices);
}
@@ -674,7 +673,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
return -ENODEV;
if (awcc->gmode) {
- priv->supported_thermal_profiles[PLATFORM_PROFILE_PERFORMANCE] =
+ priv->supported_profiles[PLATFORM_PROFILE_PERFORMANCE] =
AWCC_THERMAL_MODE_GMODE;
set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 05/10] platform/x86: alienware-wmi-wmax: Improve platform profile probe
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
` (3 preceding siblings ...)
2025-03-06 0:56 ` [PATCH v3 04/10] platform/x86: alienware-wmi-wmax: Modify supported_thermal_profiles[] Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 0:56 ` [PATCH v3 06/10] platform/x86: alienware-wmi-wmax: Add support for the "custom" thermal profile Kurt Borja
` (4 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
Get and store the AWCC system description in alienware_awcc_setup()
instead of awcc_platform_profile_probe() and add a check for integer
overflows to avoid misbehaviors.
While at it, replace set_bit() with it's non-atomic version __set_bit()
because `choices` belong to this thread only.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 61 +++++++++++++++++++-------
1 file changed, 46 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index 8e91fb4b349b7b62b6f914ac68d5eb1cd30a606e..a89fac80fa9e819e042f3807eb69dc8ddd9a4841 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -38,6 +38,9 @@
/* Some IDs have a BIT(8) flag that we ignore */
#define AWCC_RESOURCE_ID_MASK GENMASK(7, 0)
+/* Arbitrary limit based on supported models */
+#define AWCC_MAX_RES_COUNT 16
+
static bool force_platform_profile;
module_param_unsafe(force_platform_profile, bool, 0);
MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
@@ -212,6 +215,17 @@ struct wmax_u32_args {
struct awcc_priv {
struct wmi_device *wdev;
+ union {
+ u32 system_description;
+ struct {
+ u8 fan_count;
+ u8 temp_count;
+ u8 unknown_count;
+ u8 profile_count;
+ };
+ u8 res_count[4];
+ };
+
struct device *ppdev;
u8 supported_profiles[PLATFORM_PROFILE_LAST];
};
@@ -636,37 +650,40 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
enum platform_profile_option profile;
struct awcc_priv *priv = drvdata;
enum awcc_thermal_profile mode;
- u8 sys_desc[4];
- u32 first_mode;
+ u8 id, offset = 0;
u32 out_data;
int ret;
- u8 id;
- ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
- 0, (u32 *) &sys_desc);
- if (ret < 0)
- return ret;
-
- first_mode = sys_desc[0] + sys_desc[1];
-
- for (u32 i = 0; i < sys_desc[3]; i++) {
- ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
+ /*
+ * Thermal profile IDs are listed last at offset
+ * fan_count + temp_count + unknown_count
+ */
+ for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count) - 1; i++)
+ offset += priv->res_count[i];
+ for (unsigned int i = 0; i < priv->profile_count; i++) {
+ ret = awcc_op_get_resource_id(priv->wdev, i + offset, &out_data);
if (ret == -EIO)
return ret;
+ /*
+ * Some devices report an incorrect number of thermal profiles
+ * so the resource ID list may end prematurely
+ */
if (ret == -EBADRQC)
break;
id = FIELD_GET(AWCC_RESOURCE_ID_MASK, out_data);
- if (!is_awcc_thermal_profile_id(id))
+ if (!is_awcc_thermal_profile_id(id)) {
+ dev_dbg(&priv->wdev->dev, "Unmapped thermal profile ID 0x%02x\n", id);
continue;
+ }
mode = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
profile = awcc_mode_to_platform_profile[mode];
priv->supported_profiles[profile] = id;
- set_bit(profile, choices);
+ __set_bit(profile, choices);
}
if (bitmap_empty(choices, PLATFORM_PROFILE_LAST))
@@ -676,7 +693,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
priv->supported_profiles[PLATFORM_PROFILE_PERFORMANCE] =
AWCC_THERMAL_MODE_GMODE;
- set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
+ __set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
}
return 0;
@@ -707,6 +724,20 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
if (!priv)
return -ENOMEM;
+ ret = awcc_thermal_information(wdev, AWCC_OP_GET_SYSTEM_DESCRIPTION,
+ 0, &priv->system_description);
+ if (ret < 0)
+ return ret;
+
+ /* Sanity check */
+ for (unsigned int i = 0; i < ARRAY_SIZE(priv->res_count); i++) {
+ if (priv->res_count[i] > AWCC_MAX_RES_COUNT) {
+ dev_err(&wdev->dev, "Malformed system description: 0x%08x\n",
+ priv->system_description);
+ return -ENXIO;
+ }
+ }
+
priv->wdev = wdev;
dev_set_drvdata(&wdev->dev, priv);
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 06/10] platform/x86: alienware-wmi-wmax: Add support for the "custom" thermal profile
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
` (4 preceding siblings ...)
2025-03-06 0:56 ` [PATCH v3 05/10] platform/x86: alienware-wmi-wmax: Improve platform profile probe Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 0:56 ` [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support Kurt Borja
` (3 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
All models with the "AWCC" WMAX device support a "custom" thermal
profile. In some models this profile signals user-space that the user
wants to manually control the fans, which are always unlocked. In other
models it actually unlocks manual fan control.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index a89fac80fa9e819e042f3807eb69dc8ddd9a4841..71fc17e8d103146b8edf53a552ae5ba64414e873 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -29,8 +29,6 @@
#define AWCC_METHOD_THERMAL_CONTROL 0x15
#define AWCC_METHOD_GAME_SHIFT_STATUS 0x25
-#define AWCC_THERMAL_MODE_GMODE 0xAB
-
#define AWCC_FAILURE_CODE 0xFFFFFFFF
#define AWCC_FAILURE_CODE_2 0xFFFFFFFE
#define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
@@ -177,6 +175,11 @@ enum AWCC_THERMAL_TABLES {
AWCC_THERMAL_TABLE_USTT = 0xA,
};
+enum AWCC_SPECIAL_THERMAL_CODES {
+ AWCC_SPECIAL_PROFILE_CUSTOM = 0x00,
+ AWCC_SPECIAL_PROFILE_GMODE = 0xAB,
+};
+
enum awcc_thermal_profile {
AWCC_PROFILE_USTT_BALANCED,
AWCC_PROFILE_USTT_BALANCED_PERFORMANCE,
@@ -577,7 +580,10 @@ static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profile)
{
switch (id) {
- case AWCC_THERMAL_MODE_GMODE:
+ case AWCC_SPECIAL_PROFILE_CUSTOM:
+ *profile = PLATFORM_PROFILE_CUSTOM;
+ return 0;
+ case AWCC_SPECIAL_PROFILE_GMODE:
*profile = PLATFORM_PROFILE_PERFORMANCE;
return 0;
default:
@@ -691,11 +697,17 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
if (awcc->gmode) {
priv->supported_profiles[PLATFORM_PROFILE_PERFORMANCE] =
- AWCC_THERMAL_MODE_GMODE;
+ AWCC_SPECIAL_PROFILE_GMODE;
__set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
}
+ /* Every model supports the "custom" profile */
+ priv->supported_profiles[PLATFORM_PROFILE_CUSTOM] =
+ AWCC_SPECIAL_PROFILE_CUSTOM;
+
+ __set_bit(PLATFORM_PROFILE_CUSTOM, choices);
+
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
` (5 preceding siblings ...)
2025-03-06 0:56 ` [PATCH v3 06/10] platform/x86: alienware-wmi-wmax: Add support for the "custom" thermal profile Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 22:19 ` Armin Wolf
2025-03-06 0:56 ` [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control Kurt Borja
` (2 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel, Guenter Roeck, Jean Delvare,
linux-hwmon
All models with the "AWCC" WMAX device support monitoring fan speed and
temperature sensors. Expose this feature through the HWMON interface.
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: linux-hwmon@vger.kernel.org
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/Kconfig | 1 +
drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++
2 files changed, 432 insertions(+)
diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 100644
--- a/drivers/platform/x86/dell/Kconfig
+++ b/drivers/platform/x86/dell/Kconfig
@@ -43,6 +43,7 @@ config ALIENWARE_WMI_WMAX
bool "Alienware WMAX WMI device driver"
default y
depends on ALIENWARE_WMI
+ depends on HWMON
select ACPI_PLATFORM_PROFILE
help
Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -9,10 +9,13 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/bitfield.h>
+#include <linux/bitmap.h>
#include <linux/bits.h>
#include <linux/dmi.h>
+#include <linux/hwmon.h>
#include <linux/moduleparam.h>
#include <linux/platform_profile.h>
+#include <linux/units.h>
#include <linux/wmi.h>
#include "alienware-wmi.h"
@@ -25,6 +28,7 @@
#define WMAX_METHOD_BRIGHTNESS 0x3
#define WMAX_METHOD_ZONE_CONTROL 0x4
+#define AWCC_METHOD_GET_FAN_SENSORS 0x13
#define AWCC_METHOD_THERMAL_INFORMATION 0x14
#define AWCC_METHOD_THERMAL_CONTROL 0x15
#define AWCC_METHOD_GAME_SHIFT_STATUS 0x25
@@ -39,6 +43,10 @@
/* Arbitrary limit based on supported models */
#define AWCC_MAX_RES_COUNT 16
+static bool force_hwmon;
+module_param_unsafe(force_hwmon, bool, 0);
+MODULE_PARM_DESC(force_hwmon, "Force probing for HWMON support without checking if the WMI backend is available");
+
static bool force_platform_profile;
module_param_unsafe(force_platform_profile, bool, 0);
MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
@@ -48,16 +56,19 @@ module_param_unsafe(force_gmode, bool, 0);
MODULE_PARM_DESC(force_gmode, "Forces G-Mode when performance profile is selected");
struct awcc_quirks {
+ bool hwmon;
bool pprof;
bool gmode;
};
static struct awcc_quirks g_series_quirks = {
+ .hwmon = true,
.pprof = true,
.gmode = true,
};
static struct awcc_quirks generic_quirks = {
+ .hwmon = true,
.pprof = true,
.gmode = false,
};
@@ -155,9 +166,18 @@ static const struct dmi_system_id awcc_dmi_table[] __initconst = {
},
};
+enum AWCC_GET_FAN_SENSORS_OPERATIONS {
+ AWCC_OP_GET_TOTAL_FAN_TEMPS = 0x01,
+ AWCC_OP_GET_FAN_TEMP_ID = 0x02,
+};
+
enum AWCC_THERMAL_INFORMATION_OPERATIONS {
AWCC_OP_GET_SYSTEM_DESCRIPTION = 0x02,
AWCC_OP_GET_RESOURCE_ID = 0x03,
+ AWCC_OP_GET_TEMPERATURE = 0x04,
+ AWCC_OP_GET_FAN_RPM = 0x05,
+ AWCC_OP_GET_FAN_MIN_RPM = 0x08,
+ AWCC_OP_GET_FAN_MAX_RPM = 0x09,
AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
};
@@ -180,6 +200,12 @@ enum AWCC_SPECIAL_THERMAL_CODES {
AWCC_SPECIAL_PROFILE_GMODE = 0xAB,
};
+enum AWCC_TEMP_SENSOR_TYPES {
+ AWCC_TEMP_SENSOR_CPU = 0x01,
+ AWCC_TEMP_SENSOR_GPU = 0x06,
+ AWCC_TEMP_SENSOR_LAST
+};
+
enum awcc_thermal_profile {
AWCC_PROFILE_USTT_BALANCED,
AWCC_PROFILE_USTT_BALANCED_PERFORMANCE,
@@ -216,6 +242,15 @@ struct wmax_u32_args {
u8 arg3;
};
+struct awcc_fan_data {
+ unsigned long *related_temps;
+ unsigned long *auto_channels_temp;
+ u32 total_temps;
+ u32 min_rpm;
+ u32 max_rpm;
+ u8 id;
+};
+
struct awcc_priv {
struct wmi_device *wdev;
union {
@@ -231,6 +266,11 @@ struct awcc_priv {
struct device *ppdev;
u8 supported_profiles[PLATFORM_PROFILE_LAST];
+
+ struct device *hwdev;
+ struct awcc_fan_data **fan_data;
+ unsigned int temp_sensors_size;
+ unsigned long *temp_sensors;
};
static const enum platform_profile_option awcc_mode_to_platform_profile[AWCC_PROFILE_LAST] = {
@@ -495,6 +535,19 @@ static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
return 0;
}
+static inline int awcc_get_fan_sensors(struct wmi_device *wdev, u8 operation,
+ u8 fan_id, u8 index, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = operation,
+ .arg1 = fan_id,
+ .arg2 = index,
+ .arg3 = 0,
+ };
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_GET_FAN_SENSORS, &args, out);
+}
+
static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
u8 arg, u32 *out)
{
@@ -552,6 +605,32 @@ static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32
return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
}
+static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_GET_FAN_RPM,
+ .arg1 = fan_id,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
+}
+
+static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_GET_TEMPERATURE,
+ .arg1 = temp_id,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
+
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
+}
+
static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
{
struct wmax_u32_args args = {
@@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil
return 0;
}
+/*
+ * HWMON
+ * - Provides temperature and fan speed monitoring as well as manual fan
+ * control
+ */
+static umode_t awcc_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ const struct awcc_priv *priv = drvdata;
+ unsigned int temp_count;
+
+ switch (type) {
+ case hwmon_temp:
+ temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size);
+
+ return channel < temp_count ? 0444 : 0;
+ case hwmon_fan:
+ return channel < priv->fan_count ? 0444 : 0;
+ case hwmon_pwm:
+ if (channel >= priv->fan_count)
+ return 0;
+
+ switch (attr) {
+ case hwmon_pwm_enable:
+ return 0644;
+ case hwmon_pwm_auto_channels_temp:
+ return 0444;
+ default:
+ return 0;
+ }
+ default:
+ return 0;
+ }
+}
+
+static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ struct awcc_fan_data *fan;
+ u32 state;
+ int ret;
+ u8 temp;
+
+ switch (type) {
+ case hwmon_temp:
+ temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
+
+ switch (attr) {
+ case hwmon_temp_input:
+ ret = awcc_op_get_temperature(priv->wdev, temp, &state);
+ if (ret)
+ return ret;
+
+ *val = state * MILLIDEGREE_PER_DEGREE;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ break;
+ case hwmon_fan:
+ fan = priv->fan_data[channel];
+
+ switch (attr) {
+ case hwmon_fan_input:
+ ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state);
+ if (ret)
+ return ret;
+
+ *val = state;
+ break;
+ case hwmon_fan_min:
+ *val = fan->min_rpm;
+ break;
+ case hwmon_fan_max:
+ *val = fan->max_rpm;
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ break;
+ case hwmon_pwm:
+ fan = priv->fan_data[channel];
+
+ switch (attr) {
+ case hwmon_pwm_auto_channels_temp:
+ bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ struct awcc_fan_data *fan;
+ u8 temp;
+
+ switch (type) {
+ case hwmon_temp:
+ temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
+
+ switch (temp) {
+ case AWCC_TEMP_SENSOR_CPU:
+ *str = "CPU";
+ break;
+ case AWCC_TEMP_SENSOR_GPU:
+ *str = "GPU";
+ break;
+ default:
+ *str = "Unknown";
+ break;
+ }
+
+ break;
+ case hwmon_fan:
+ fan = priv->fan_data[channel];
+
+ switch (fan->total_temps) {
+ case 0:
+ *str = "Independent Fan";
+ break;
+ case 1:
+ temp = find_first_bit(fan->related_temps, priv->temp_sensors_size);
+
+ switch (temp) {
+ case AWCC_TEMP_SENSOR_CPU:
+ *str = "Processor Fan";
+ break;
+ case AWCC_TEMP_SENSOR_GPU:
+ *str = "Video Fan";
+ break;
+ default:
+ *str = "Unknown Fan";
+ break;
+ }
+
+ break;
+ default:
+ *str = "Shared Fan";
+ break;
+ }
+
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static const struct hwmon_ops awcc_hwmon_ops = {
+ .is_visible = awcc_hwmon_is_visible,
+ .read = awcc_hwmon_read,
+ .read_string = awcc_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(temp,
+ HWMON_T_LABEL | HWMON_T_INPUT,
+ HWMON_T_LABEL | HWMON_T_INPUT,
+ HWMON_T_LABEL | HWMON_T_INPUT,
+ HWMON_T_LABEL | HWMON_T_INPUT,
+ HWMON_T_LABEL | HWMON_T_INPUT,
+ HWMON_T_LABEL | HWMON_T_INPUT
+ ),
+ HWMON_CHANNEL_INFO(fan,
+ HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
+ HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
+ HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
+ HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
+ HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
+ HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
+ ),
+ HWMON_CHANNEL_INFO(pwm,
+ HWMON_PWM_AUTO_CHANNELS_TEMP,
+ HWMON_PWM_AUTO_CHANNELS_TEMP,
+ HWMON_PWM_AUTO_CHANNELS_TEMP,
+ HWMON_PWM_AUTO_CHANNELS_TEMP,
+ HWMON_PWM_AUTO_CHANNELS_TEMP,
+ HWMON_PWM_AUTO_CHANNELS_TEMP
+ ),
+ NULL
+};
+
+static const struct hwmon_chip_info awcc_hwmon_chip_info = {
+ .ops = &awcc_hwmon_ops,
+ .info = awcc_hwmon_info,
+};
+
+static int awcc_hwmon_temps_init(struct wmi_device *wdev)
+{
+ struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
+ unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)];
+ unsigned int i, max_sensor_id = 0;
+ int ret;
+ u32 id;
+
+ for (i = 0; i < priv->temp_count; i++) {
+ /*
+ * Temperature sensors IDs are listed after the fan IDs at
+ * offset `fan_count`
+ */
+ ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id);
+ if (ret)
+ return ret;
+
+ id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
+ if (id > max_sensor_id)
+ max_sensor_id = id;
+
+ __set_bit(id, temp_sensors);
+ }
+
+ /*
+ * We prefer to allocate the bitmap dynamically because usually temp IDs
+ * are small (< 0x30) and only one UL is needed to store it, but there
+ * may be unknown devices that break this rule
+ */
+ priv->temp_sensors_size = max_sensor_id + 1;
+ priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size,
+ GFP_KERNEL);
+ if (!priv->temp_sensors)
+ return -ENOMEM;
+
+ bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size);
+
+ return 0;
+}
+
+static int awcc_hwmon_fans_init(struct wmi_device *wdev)
+{
+ struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
+ u32 id, min_rpm, max_rpm, temp_count, temp_id;
+ unsigned long gather[BITS_TO_LONGS(U8_MAX)];
+ struct awcc_fan_data *fan_data;
+ unsigned int i, j;
+ int ret;
+
+ for (i = 0; i < priv->fan_count; i++) {
+ fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL);
+ if (!fan_data)
+ return -ENOMEM;
+
+ fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev,
+ priv->temp_sensors_size,
+ GFP_KERNEL);
+ if (!priv->temp_sensors)
+ return -ENOMEM;
+
+ fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev,
+ priv->temp_count,
+ GFP_KERNEL);
+ if (!priv->temp_sensors)
+ return -ENOMEM;
+
+ /*
+ * Fan IDs are listed first at offset 0
+ */
+ ret = awcc_op_get_resource_id(wdev, i, &id);
+ if (ret)
+ return ret;
+ id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
+
+ ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id,
+ &min_rpm);
+ if (ret)
+ return ret;
+
+ ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id,
+ &max_rpm);
+ if (ret)
+ return ret;
+
+ ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id,
+ 0, &temp_count);
+ if (ret)
+ return ret;
+
+ for (j = 0; j < temp_count; j++) {
+ ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID,
+ id, j, &temp_id);
+ if (ret)
+ break;
+
+ temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id);
+ if (temp_id < priv->temp_sensors_size)
+ __set_bit(temp_id, fan_data->related_temps);
+ }
+
+ fan_data->id = id;
+ fan_data->min_rpm = min_rpm;
+ fan_data->max_rpm = max_rpm;
+ fan_data->total_temps = bitmap_weight(fan_data->related_temps,
+ priv->temp_sensors_size);
+ bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors,
+ priv->temp_sensors_size);
+ bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count);
+ priv->fan_data[i] = fan_data;
+ }
+
+ return 0;
+}
+
+static int awcc_hwmon_init(struct wmi_device *wdev)
+{
+ struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
+ int ret;
+
+ priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count,
+ sizeof(*priv->fan_data), GFP_KERNEL);
+ if (!priv->fan_data)
+ return -ENOMEM;
+
+ ret = awcc_hwmon_temps_init(wdev);
+ if (ret)
+ return ret;
+
+ ret = awcc_hwmon_fans_init(wdev);
+ if (ret)
+ return ret;
+
+ priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
+ &awcc_hwmon_chip_info, NULL);
+
+ return PTR_ERR_OR_ZERO(priv->hwdev);
+}
+
/*
* Thermal Profile control
* - Provides thermal profile control through the Platform Profile API
@@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
priv->wdev = wdev;
dev_set_drvdata(&wdev->dev, priv);
+ if (awcc->hwmon) {
+ ret = awcc_hwmon_init(wdev);
+ if (ret)
+ return ret;
+ }
+
if (awcc->pprof) {
ret = awcc_platform_profile_init(wdev);
if (ret)
@@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void)
if (id)
awcc = id->driver_data;
+ if (force_hwmon) {
+ if (!awcc)
+ awcc = &empty_quirks;
+
+ awcc->hwmon = true;
+ }
+
if (force_platform_profile) {
if (!awcc)
awcc = &empty_quirks;
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
` (6 preceding siblings ...)
2025-03-06 0:56 ` [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support Kurt Borja
@ 2025-03-06 0:56 ` Kurt Borja
2025-03-06 22:35 ` Armin Wolf
2025-03-06 0:57 ` [PATCH v3 09/10] platform/x86: alienware-wmi-wmax: Add a DebugFS interface Kurt Borja
2025-03-06 0:57 ` [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation Kurt Borja
9 siblings, 1 reply; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:56 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel, Guenter Roeck, Jean Delvare,
linux-hwmon
All models with the "AWCC" WMAX device support a way of manually
controlling fans.
The PWM duty cycle of a fan can't be controlled directly. Instead the
AWCC interface let's us tune a PWM `boost` value, which has the
following empirically discovered, aproximate behavior over the PWM
value:
pwm = pwm_base + (pwm_boost / 255) * (pwm_max - pwm_base)
Where the pwm_base is the locked PWM value controlled by the FW and
pwm_boost is a value between 0 and 255.
Expose this pwm_boost knob as a custom HWMON attribute.
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: linux-hwmon@vger.kernel.org
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 223 ++++++++++++++++++++++++-
1 file changed, 220 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index 20cf3371ee3c0e1ea038b3ca517e831f3b30dc29..de4e8f177aadc9552b05cc732e41ee458b761143 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -13,8 +13,11 @@
#include <linux/bits.h>
#include <linux/dmi.h>
#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/minmax.h>
#include <linux/moduleparam.h>
#include <linux/platform_profile.h>
+#include <linux/pm.h>
#include <linux/units.h>
#include <linux/wmi.h>
#include "alienware-wmi.h"
@@ -179,10 +182,12 @@ enum AWCC_THERMAL_INFORMATION_OPERATIONS {
AWCC_OP_GET_FAN_MIN_RPM = 0x08,
AWCC_OP_GET_FAN_MAX_RPM = 0x09,
AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
+ AWCC_OP_GET_FAN_BOOST = 0x0C,
};
enum AWCC_THERMAL_CONTROL_OPERATIONS {
AWCC_OP_ACTIVATE_PROFILE = 0x01,
+ AWCC_OP_SET_FAN_BOOST = 0x02,
};
enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
@@ -248,6 +253,7 @@ struct awcc_fan_data {
u32 total_temps;
u32 min_rpm;
u32 max_rpm;
+ u8 suspend_cache;
u8 id;
};
@@ -627,6 +633,17 @@ static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u
.arg3 = 0,
};
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
+}
+
+static inline int awcc_op_get_fan_boost(struct wmi_device *wdev, u8 fan_id, u32 *out)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_GET_FAN_BOOST,
+ .arg1 = fan_id,
+ .arg2 = 0,
+ .arg3 = 0,
+ };
return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
}
@@ -656,6 +673,19 @@ static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
}
+static int awcc_op_set_fan_boost(struct wmi_device *wdev, u8 fan_id, u8 boost)
+{
+ struct wmax_u32_args args = {
+ .operation = AWCC_OP_SET_FAN_BOOST,
+ .arg1 = fan_id,
+ .arg2 = boost,
+ .arg3 = 0,
+ };
+ u32 out;
+
+ return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
+}
+
static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profile)
{
switch (id) {
@@ -717,6 +747,7 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
struct awcc_priv *priv = dev_get_drvdata(dev);
+ enum platform_profile_option profile;
struct awcc_fan_data *fan;
u32 state;
int ret;
@@ -765,6 +796,28 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
fan = priv->fan_data[channel];
switch (attr) {
+ case hwmon_pwm_enable:
+ ret = awcc_op_get_current_profile(priv->wdev, &state);
+ if (ret)
+ return ret;
+
+ ret = awcc_profile_id_to_pprof(state, &profile);
+ if (ret)
+ return ret;
+
+ switch (profile) {
+ case PLATFORM_PROFILE_PERFORMANCE:
+ *val = 0;
+ break;
+ case PLATFORM_PROFILE_CUSTOM:
+ *val = 1;
+ break;
+ default:
+ *val = 2;
+ break;
+ }
+
+ break;
case hwmon_pwm_auto_channels_temp:
bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
break;
@@ -840,10 +893,48 @@ static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types ty
return 0;
}
+
+static int awcc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long val)
+{
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ int ret;
+
+ switch (type) {
+ case hwmon_pwm:
+ switch (attr) {
+ case hwmon_pwm_enable:
+ /*
+ * We don't want to duplicate platform profile logic, so
+ * we only allow enabling manual fan control
+ */
+ if (val != 1)
+ return -EINVAL;
+
+ ret = awcc_op_activate_profile(priv->wdev, AWCC_SPECIAL_PROFILE_CUSTOM);
+ if (ret)
+ return ret;
+
+ if (priv->ppdev)
+ platform_profile_notify(priv->ppdev);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
static const struct hwmon_ops awcc_hwmon_ops = {
.is_visible = awcc_hwmon_is_visible,
.read = awcc_hwmon_read,
.read_string = awcc_hwmon_read_string,
+ .write = awcc_hwmon_write,
};
static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
@@ -864,7 +955,7 @@ static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
),
HWMON_CHANNEL_INFO(pwm,
- HWMON_PWM_AUTO_CHANNELS_TEMP,
+ HWMON_PWM_AUTO_CHANNELS_TEMP | HWMON_PWM_ENABLE,
HWMON_PWM_AUTO_CHANNELS_TEMP,
HWMON_PWM_AUTO_CHANNELS_TEMP,
HWMON_PWM_AUTO_CHANNELS_TEMP,
@@ -879,6 +970,75 @@ static const struct hwmon_chip_info awcc_hwmon_chip_info = {
.info = awcc_hwmon_info,
};
+static ssize_t pwm_boost_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int ret, index = to_sensor_dev_attr(attr)->index;
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ struct awcc_fan_data *fan = priv->fan_data[index];
+ u32 boost;
+
+ ret = awcc_op_get_fan_boost(priv->wdev, fan->id, &boost);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%u\n", boost);
+}
+
+static ssize_t pwm_boost_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret, index = to_sensor_dev_attr(attr)->index;
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ struct awcc_fan_data *fan = priv->fan_data[index];
+ unsigned long val;
+
+ ret = kstrtoul(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ ret = awcc_op_set_fan_boost(priv->wdev, fan->id, clamp_val(val, 0, 255));
+
+ return ret ? ret : count;
+}
+
+static SENSOR_DEVICE_ATTR_RW(pwm1_boost, pwm_boost, 0);
+static SENSOR_DEVICE_ATTR_RW(pwm2_boost, pwm_boost, 1);
+static SENSOR_DEVICE_ATTR_RW(pwm3_boost, pwm_boost, 2);
+static SENSOR_DEVICE_ATTR_RW(pwm4_boost, pwm_boost, 3);
+
+static umode_t pwm_boost_attr_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+ struct awcc_priv *priv = dev_get_drvdata(kobj_to_dev(kobj));
+
+ return n < priv->fan_count ? attr->mode : 0;
+}
+
+static bool pwm_boost_group_visible(struct kobject *kobj)
+{
+ return true;
+}
+
+DEFINE_SYSFS_GROUP_VISIBLE(pwm_boost);
+
+static struct attribute *fan_boost_attrs[] = {
+ &sensor_dev_attr_pwm1_boost.dev_attr.attr,
+ &sensor_dev_attr_pwm2_boost.dev_attr.attr,
+ &sensor_dev_attr_pwm3_boost.dev_attr.attr,
+ &sensor_dev_attr_pwm4_boost.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group pwm_boost_group = {
+ .attrs = fan_boost_attrs,
+ .is_visible = SYSFS_GROUP_VISIBLE(pwm_boost),
+};
+
+static const struct attribute_group *awcc_hwmon_groups[] = {
+ &pwm_boost_group,
+ NULL
+};
+
static int awcc_hwmon_temps_init(struct wmi_device *wdev)
{
struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
@@ -1011,12 +1171,50 @@ static int awcc_hwmon_init(struct wmi_device *wdev)
if (ret)
return ret;
- priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
- &awcc_hwmon_chip_info, NULL);
+ priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi",
+ priv, &awcc_hwmon_chip_info,
+ awcc_hwmon_groups);
return PTR_ERR_OR_ZERO(priv->hwdev);
}
+static void awcc_hwmon_suspend(struct device *dev)
+{
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ struct awcc_fan_data *fan;
+ unsigned int i;
+ u32 boost;
+ int ret;
+
+ for (i = 0; i < priv->fan_count; i++) {
+ fan = priv->fan_data[i];
+
+ ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_FAN_BOOST,
+ fan->id, &boost);
+ if (ret)
+ fan->suspend_cache = 0;
+ else
+ fan->suspend_cache = clamp_val(boost, 0, 255);
+
+ awcc_op_set_fan_boost(priv->wdev, fan->id, 0);
+ }
+}
+
+static void awcc_hwmon_resume(struct device *dev)
+{
+
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ struct awcc_fan_data *fan;
+ unsigned int i;
+
+ for (i = 0; i < priv->fan_count; i++) {
+ fan = priv->fan_data[i];
+
+ if (fan->suspend_cache)
+ awcc_op_set_fan_boost(priv->wdev, fan->id, fan->suspend_cache);
+ }
+}
+
/*
* Thermal Profile control
* - Provides thermal profile control through the Platform Profile API
@@ -1233,6 +1431,24 @@ static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
return ret;
}
+static int wmax_wmi_suspend(struct device *dev)
+{
+ if (awcc->hwmon)
+ awcc_hwmon_suspend(dev);
+
+ return 0;
+}
+
+static int wmax_wmi_resume(struct device *dev)
+{
+ if (awcc->hwmon)
+ awcc_hwmon_resume(dev);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_DEV_PM_OPS(wmax_wmi_pm_ops, wmax_wmi_suspend, wmax_wmi_resume);
+
static const struct wmi_device_id alienware_wmax_device_id_table[] = {
{ WMAX_CONTROL_GUID, NULL },
{ },
@@ -1243,6 +1459,7 @@ static struct wmi_driver alienware_wmax_wmi_driver = {
.driver = {
.name = "alienware-wmi-wmax",
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .pm = pm_sleep_ptr(&wmax_wmi_pm_ops),
},
.id_table = alienware_wmax_device_id_table,
.probe = wmax_wmi_probe,
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 09/10] platform/x86: alienware-wmi-wmax: Add a DebugFS interface
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
` (7 preceding siblings ...)
2025-03-06 0:56 ` [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control Kurt Borja
@ 2025-03-06 0:57 ` Kurt Borja
2025-03-06 22:38 ` Armin Wolf
2025-03-06 0:57 ` [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation Kurt Borja
9 siblings, 1 reply; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:57 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
Add a debugfs interface which exposes thermal private data.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
drivers/platform/x86/dell/alienware-wmi-wmax.c | 92 ++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
index de4e8f177aadc9552b05cc732e41ee458b761143..23f8680a212fb9ef2a6f23aafcc2d25738ae4364 100644
--- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
+++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
@@ -11,6 +11,7 @@
#include <linux/bitfield.h>
#include <linux/bitmap.h>
#include <linux/bits.h>
+#include <linux/debugfs.h>
#include <linux/dmi.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
@@ -18,6 +19,7 @@
#include <linux/moduleparam.h>
#include <linux/platform_profile.h>
#include <linux/pm.h>
+#include <linux/seq_file.h>
#include <linux/units.h>
#include <linux/wmi.h>
#include "alienware-wmi.h"
@@ -1343,6 +1345,94 @@ static int awcc_platform_profile_init(struct wmi_device *wdev)
return PTR_ERR_OR_ZERO(priv->ppdev);
}
+/*
+ * DebugFS
+ */
+static int awcc_debugfs_system_description_read(struct seq_file *seq, void *data)
+{
+ struct device *dev = seq->private;
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+
+ seq_printf(seq, "0x%08x\n", priv->system_description);
+
+ return 0;
+}
+
+static int awcc_debugfs_hwmon_data_read(struct seq_file *seq, void *data)
+{
+ struct device *dev = seq->private;
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+ struct awcc_fan_data *fan_data;
+ u8 bit;
+
+ seq_printf(seq, "Number of fans: %u\n", priv->fan_count);
+ seq_printf(seq, "Number of temperature sensors: %u\n\n", priv->temp_count);
+
+ for (u32 i = 0; i < priv->fan_count; i++) {
+ fan_data = priv->fan_data[i];
+
+ seq_printf(seq, "Fan %u:\n", i);
+ seq_printf(seq, " ID: 0x%02x\n", fan_data->id);
+ seq_printf(seq, " Related temperature sensors: ");
+ for_each_set_bit(bit, fan_data->related_temps, priv->temp_sensors_size)
+ seq_printf(seq, "0x%02x ", bit);
+ seq_puts(seq, "\n");
+ }
+
+ seq_puts(seq, "\n");
+
+ seq_printf(seq, "Temperature sensor IDs:\n");
+ for_each_set_bit(bit, priv->temp_sensors, priv->temp_sensors_size)
+ seq_printf(seq, " 0x%02x\n", bit);
+
+ return 0;
+}
+
+static int awcc_debugfs_pprof_data_read(struct seq_file *seq, void *data)
+{
+ struct device *dev = seq->private;
+ struct awcc_priv *priv = dev_get_drvdata(dev);
+
+ seq_printf(seq, "Number of thermal profiles: %u\n\n", priv->profile_count);
+
+ for (u32 i = 0; i < PLATFORM_PROFILE_LAST; i++) {
+ if (!priv->supported_profiles[i])
+ continue;
+
+ seq_printf(seq, "Platform profile %u:\n", i);
+ seq_printf(seq, " ID: 0x%02x\n", priv->supported_profiles[i]);
+ }
+
+ return 0;
+}
+
+static void awcc_debugfs_remove(void *data)
+{
+ struct dentry *root = data;
+
+ debugfs_remove(root);
+}
+
+static void awcc_debugfs_init(struct wmi_device *wdev)
+{
+ struct dentry *root;
+
+ root = debugfs_create_dir("alienware-wmi", NULL);
+
+ debugfs_create_devm_seqfile(&wdev->dev, "system_description", root,
+ awcc_debugfs_system_description_read);
+
+ if (awcc->hwmon)
+ debugfs_create_devm_seqfile(&wdev->dev, "hwmon_data", root,
+ awcc_debugfs_hwmon_data_read);
+
+ if (awcc->pprof)
+ debugfs_create_devm_seqfile(&wdev->dev, "pprof_data", root,
+ awcc_debugfs_pprof_data_read);
+
+ devm_add_action_or_reset(&wdev->dev, awcc_debugfs_remove, root);
+}
+
static int alienware_awcc_setup(struct wmi_device *wdev)
{
struct awcc_priv *priv;
@@ -1381,6 +1471,8 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
return ret;
}
+ awcc_debugfs_init(wdev);
+
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
` (8 preceding siblings ...)
2025-03-06 0:57 ` [PATCH v3 09/10] platform/x86: alienware-wmi-wmax: Add a DebugFS interface Kurt Borja
@ 2025-03-06 0:57 ` Kurt Borja
2025-03-06 23:02 ` Armin Wolf
2025-03-06 23:57 ` Bagas Sanjaya
9 siblings, 2 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-06 0:57 UTC (permalink / raw)
To: Ilpo Järvinen, Armin Wolf
Cc: Kurt Borja, Hans de Goede, platform-driver-x86,
Dell.Client.Kernel, linux-kernel
Use tables to describe method operations instead of using pseudo-code.
Drop unknown method descriptions to avoid redundancy. Drop GPIO section
as it is currently irrelevant to this driver. Update Thermal_Information
method documentation. Add one more helpful developer to the kudos section.
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
Documentation/wmi/devices/alienware-wmi.rst | 383 +++++++++-------------------
1 file changed, 117 insertions(+), 266 deletions(-)
diff --git a/Documentation/wmi/devices/alienware-wmi.rst b/Documentation/wmi/devices/alienware-wmi.rst
index ddc5e561960e05fc7cffe700d7d278e32ff2e7b2..79238051b18bc5de9b502325017cd5c5fcf41748 100644
--- a/Documentation/wmi/devices/alienware-wmi.rst
+++ b/Documentation/wmi/devices/alienware-wmi.rst
@@ -11,7 +11,7 @@ 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.
+with thermal control and overclocking.
It is suspected that the latter is used by Alienware Command Center (AWCC) to
manage manufacturer predefined thermal profiles. The alienware-wmi driver
@@ -69,9 +69,6 @@ data using the `bmfdec <https://github.com/pali/bmfdec>`_ utility:
[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
@@ -87,175 +84,133 @@ ID 0xA0, the argument you would pass to the method is 0xA001.
Thermal Methods
===============
+WMI method GetFanSensors([in] uint32 arg2, [out] uint32 argr)
+-------------------------------------------------------------
+
++--------------------+------------------------------------+--------------------+
+| Operation (Byte 0) | Description | Arguments |
++====================+====================================+====================+
+| 0x01 | Get the number of temperature | - Byte 1: Fan ID |
+| | sensors related with a fan ID | |
++--------------------+------------------------------------+--------------------+
+| 0x02 | Get the temperature sensor IDs | - Byte 1: Fan ID |
+| | related to a fan sensor ID | - Byte 2: Index |
++--------------------+------------------------------------+--------------------+
+
WMI method Thermal_Information([in] uint32 arg2, [out] uint32 argr)
-------------------------------------------------------------------
-::
-
- if BYTE_0(arg2) == 0x01:
- argr = 1
-
- if BYTE_0(arg2) == 0x02:
- argr = SYSTEM_DESCRIPTION
-
- 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 0x02 returns a *system description* buffer with the following
-structure:
-
-::
-
- out[0] -> Number of fans
- out[1] -> Number of sensors
- out[2] -> 0x00
- out[3] -> Number of thermal modes
-
-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.
++--------------------+------------------------------------+--------------------+
+| Operation (Byte 0) | Description | Arguments |
++====================+====================================+====================+
+| 0x01 | Unknown. | - None |
++--------------------+------------------------------------+--------------------+
+| 0x02 | Get system description number with | - None |
+| | the following structure: | |
+| | | |
+| | - Byte 0: Number of fans | |
+| | - Byte 1: Number of temperature | |
+| | sensors | |
+| | - Byte 2: Unknown | |
+| | - Byte 3: Number of thermal | |
+| | profiles | |
++--------------------+------------------------------------+--------------------+
+| 0x03 | List an ID or resource at a given | - Byte 1: Index |
+| | index. Fan IDs, temperature IDs, | |
+| | unknown IDs and thermal profile | |
+| | IDs are listed in that exact | |
+| | order. | |
+| | | |
+| | Operation 0x02 is used to know | |
+| | which indexes map to which | |
+| | resources. | |
+| | | |
+| | **Returns:** ID at a given index | |
++--------------------+------------------------------------+--------------------+
+| 0x04 | Get the current temperature for a | - Byte 1: Sensor |
+| | given temperature sensor. | ID |
++--------------------+------------------------------------+--------------------+
+| 0x05 | Get the current RPM for a given | - Byte 1: Fan ID |
+| | fan. | |
++--------------------+------------------------------------+--------------------+
+| 0x06 | Get fan speed percentage. (not | - Byte 1: Fan ID |
+| | implemented in every model) | |
++--------------------+------------------------------------+--------------------+
+| 0x07 | Unknown. | - Unknown |
++--------------------+------------------------------------+--------------------+
+| 0x08 | Get minimum RPM for a given FAN | - Byte 1: Fan ID |
+| | ID. | |
++--------------------+------------------------------------+--------------------+
+| 0x09 | Get maximum RPM for a given FAN | - Byte 1: Fan ID |
+| | ID. | |
++--------------------+------------------------------------+--------------------+
+| 0x0A | Get balanced thermal profile ID. | - None |
++--------------------+------------------------------------+--------------------+
+| 0x0B | Get current thermal profile ID. | - None |
++--------------------+------------------------------------+--------------------+
+| 0x0C | Get current `boost` value for a | - Byte 1: Fan ID |
+| | given fan ID. | |
++--------------------+------------------------------------+--------------------+
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.
++--------------------+------------------------------------+--------------------+
+| Operation (Byte 0) | Description | Arguments |
++====================+====================================+====================+
+| 0x01 | Activate a given thermal profile. | - Byte 1: Thermal |
+| | | profile ID |
++--------------------+------------------------------------+--------------------+
+| 0x02 | Set a `boost` value for a given | - Byte 1: Fan ID |
+| | fan ID. | - Byte 2: Boost |
++--------------------+------------------------------------+--------------------+
These are the known thermal profile codes:
-::
++------------------------------+----------+------+
+| Thermal Profile | Type | ID |
++==============================+==========+======+
+| Custom | Special | 0x00 |
++------------------------------+----------+------+
+| G-Mode | Special | 0xAB |
++------------------------------+----------+------+
+| Quiet | Legacy | 0x96 |
++------------------------------+----------+------+
+| Balanced | Legacy | 0x97 |
++------------------------------+----------+------+
+| Balanced Performance | Legacy | 0x98 |
++------------------------------+----------+------+
+| Performance | Legacy | 0x99 |
++------------------------------+----------+------+
+| Balanced | USTT | 0xA0 |
++------------------------------+----------+------+
+| Balanced Performance | USTT | 0xA1 |
++------------------------------+----------+------+
+| Cool | USTT | 0xA2 |
++------------------------------+----------+------+
+| Quiet | USTT | 0xA3 |
++------------------------------+----------+------+
+| Performance | USTT | 0xA4 |
++------------------------------+----------+------+
+| Low Power | USTT | 0xA5 |
++------------------------------+----------+------+
- CUSTOM 0x00
+If a model supports the User Selectable Thermal Tables (USTT) profiles, it will
+not support the Legacy profiles and vice-versa.
- 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.
+Every model supports the CUSTOM (0x00) thermal profile. 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()
++--------------------+------------------------------------+--------------------+
+| Operation (Byte 0) | Description | Arguments |
++====================+====================================+====================+
+| 0x01 | Toggle *Game Shift*. | - None |
++--------------------+------------------------------------+--------------------+
+| 0x02 | Get *Game Shift* status. | - None |
++--------------------+------------------------------------+--------------------+
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.
@@ -267,131 +222,27 @@ 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
+Returns the chassis color internal ID.
Acknowledgements
================
-Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ for documenting
-and testing available thermal profile codes.
+Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ and
+`T-Troll <https://github.com/T-Troll/alienfx-tools/>`_ for documenting and
+testing some of this device's functionality, making it possible to generalize
+this driver.
--
2.48.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 03/10] platform/x86: alienware-wmi-wmax: Improve internal AWCC API
2025-03-06 0:56 ` [PATCH v3 03/10] platform/x86: alienware-wmi-wmax: Improve internal AWCC API Kurt Borja
@ 2025-03-06 21:27 ` Armin Wolf
0 siblings, 0 replies; 24+ messages in thread
From: Armin Wolf @ 2025-03-06 21:27 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel
Am 06.03.25 um 01:56 schrieb Kurt Borja:
> Inline all AWCC WMI helper methods and directly return the newly
> introduced __awcc_wmi_command() helper to simplify implementation.
>
> Drop awcc_thermal_control() in favor of awcc_op_activate_profile().
>
> Add awcc_op_get_resource_id() and awcc_profile_id_to_pprof() helpers to
> support upcoming changes, as well as a new failure code.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> drivers/platform/x86/dell/alienware-wmi-wmax.c | 176 +++++++++++++++----------
> 1 file changed, 110 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index 80aefba5b22d6b4ac18aeb2ca356f8c911150abd..a43373717bd4580e8f62a7263e67664630165e28 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -32,6 +32,7 @@
> #define AWCC_THERMAL_MODE_GMODE 0xAB
>
> #define AWCC_FAILURE_CODE 0xFFFFFFFF
> +#define AWCC_FAILURE_CODE_2 0xFFFFFFFE
> #define AWCC_THERMAL_TABLE_MASK GENMASK(7, 4)
> #define AWCC_THERMAL_MODE_MASK GENMASK(3, 0)
> /* Some IDs have a BIT(8) flag that we ignore */
> @@ -443,8 +444,7 @@ const struct attribute_group wmax_deepsleep_attribute_group = {
> };
>
> /*
> - * Thermal Profile control
> - * - Provides thermal profile control through the Platform Profile API
> + * AWCC Helpers
> */
> static bool is_awcc_thermal_profile_id(u8 code)
> {
> @@ -463,95 +463,140 @@ static bool is_awcc_thermal_profile_id(u8 code)
> return false;
> }
>
> -static int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
> - u8 arg, u32 *out_data)
> +static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
> + struct wmax_u32_args *args, u32 *out)
> {
> - struct wmax_u32_args in_args = {
> + int ret;
> +
> + ret = alienware_wmi_command(wdev, method_id, args, sizeof(*args), out);
> + if (ret)
> + return ret;
> +
> + if (*out == AWCC_FAILURE_CODE || *out == AWCC_FAILURE_CODE_2)
> + return -EBADRQC;
> +
> + return 0;
> +}
> +
> +static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
> + u8 arg, u32 *out)
> +{
> + struct wmax_u32_args args = {
> .operation = operation,
> .arg1 = arg,
> .arg2 = 0,
> .arg3 = 0,
> };
> - int ret;
>
> - ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION,
> - &in_args, sizeof(in_args), out_data);
> - if (ret < 0)
> - return ret;
> -
> - if (*out_data == AWCC_FAILURE_CODE)
> - return -EBADRQC;
> -
> - return 0;
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> }
>
> -static int awcc_thermal_control(struct wmi_device *wdev, u8 profile)
> +static inline int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
> + u32 *out)
> {
> - struct wmax_u32_args in_args = {
> - .operation = AWCC_OP_ACTIVATE_PROFILE,
> - .arg1 = profile,
> - .arg2 = 0,
> - .arg3 = 0,
> - };
> - u32 out_data;
> - int ret;
> -
> - ret = alienware_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL,
> - &in_args, sizeof(in_args), &out_data);
> - if (ret)
> - return ret;
> -
> - if (out_data == AWCC_FAILURE_CODE)
> - return -EBADRQC;
> -
> - return 0;
> -}
> -
> -static int awcc_game_shift_status(struct wmi_device *wdev, u8 operation,
> - u32 *out_data)
> -{
> - struct wmax_u32_args in_args = {
> + struct wmax_u32_args args = {
> .operation = operation,
> .arg1 = 0,
> .arg2 = 0,
> .arg3 = 0,
> };
> - int ret;
>
> - ret = alienware_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS,
> - &in_args, sizeof(in_args), out_data);
> - if (ret < 0)
> - return ret;
> + return __awcc_wmi_command(wdev, AWCC_METHOD_GAME_SHIFT_STATUS, &args, out);
> +}
>
> - if (*out_data == AWCC_FAILURE_CODE)
> - return -EOPNOTSUPP;
> +/**
> + * awcc_op_get_resource_id - Get the resource ID at a given index
> + * @wdev: AWCC WMI device
> + * @index: Index
> + * @out: Value returned by the WMI call
> + *
> + * Get the resource ID at a given index. Resource IDs are listed in the
> + * following order:
> + *
> + * - Fan IDs
> + * - Sensor IDs
> + * - Unknown IDs
> + * - Thermal Profile IDs
> + *
> + * The total number of IDs of a given type can be obtained with
> + * AWCC_OP_GET_SYSTEM_DESCRIPTION.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_GET_RESOURCE_ID,
> + .arg1 = index,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> +}
> +
> +static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_GET_CURRENT_PROFILE,
> + .arg1 = 0,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> +}
> +
> +static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_ACTIVATE_PROFILE,
> + .arg1 = profile,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> + u32 out;
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
> +}
> +
> +static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profile)
> +{
> + switch (id) {
> + case AWCC_THERMAL_MODE_GMODE:
> + *profile = PLATFORM_PROFILE_PERFORMANCE;
> + return 0;
> + default:
> + break;
> + }
> +
> + if (!is_awcc_thermal_profile_id(id))
> + return -ENODATA;
> +
> + id = FIELD_GET(AWCC_THERMAL_MODE_MASK, id);
> + *profile = awcc_mode_to_platform_profile[id];
>
> return 0;
> }
>
> +/*
> + * Thermal Profile control
> + * - Provides thermal profile control through the Platform Profile API
> + */
> static int awcc_platform_profile_get(struct device *dev,
> enum platform_profile_option *profile)
> {
> struct awcc_priv *priv = dev_get_drvdata(dev);
> - u32 out_data;
> + u32 profile_id;
> int ret;
>
> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_CURRENT_PROFILE,
> - 0, &out_data);
> -
> - if (ret < 0)
> + ret = awcc_op_get_current_profile(priv->wdev, &profile_id);
> + if (ret)
> return ret;
>
> - if (out_data == AWCC_THERMAL_MODE_GMODE) {
> - *profile = PLATFORM_PROFILE_PERFORMANCE;
> - return 0;
> - }
> -
> - if (!is_awcc_thermal_profile_id(out_data))
> - return -ENODATA;
> -
> - out_data &= AWCC_THERMAL_MODE_MASK;
> - *profile = awcc_mode_to_platform_profile[out_data];
> + ret = awcc_profile_id_to_pprof(profile_id, profile);
> + if (ret)
> + return ret;
>
> return 0;
> }
> @@ -583,8 +628,8 @@ static int awcc_platform_profile_set(struct device *dev,
> }
> }
>
> - return awcc_thermal_control(priv->wdev,
> - priv->supported_thermal_profiles[profile]);
> + return awcc_op_activate_profile(priv->wdev,
> + priv->supported_thermal_profiles[profile]);
> }
>
> static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> @@ -606,8 +651,7 @@ static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)
> first_mode = sys_desc[0] + sys_desc[1];
>
> for (u32 i = 0; i < sys_desc[3]; i++) {
> - ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_RESOURCE_ID,
> - i + first_mode, &out_data);
> + ret = awcc_op_get_resource_id(priv->wdev, i + first_mode, &out_data);
>
> if (ret == -EIO)
> return ret;
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support
2025-03-06 0:56 ` [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support Kurt Borja
@ 2025-03-06 22:19 ` Armin Wolf
2025-03-07 0:35 ` Kurt Borja
0 siblings, 1 reply; 24+ messages in thread
From: Armin Wolf @ 2025-03-06 22:19 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Guenter Roeck, Jean Delvare, linux-hwmon
Am 06.03.25 um 01:56 schrieb Kurt Borja:
> All models with the "AWCC" WMAX device support monitoring fan speed and
> temperature sensors. Expose this feature through the HWMON interface.
>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: linux-hwmon@vger.kernel.org
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> drivers/platform/x86/dell/Kconfig | 1 +
> drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++
> 2 files changed, 432 insertions(+)
>
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -43,6 +43,7 @@ config ALIENWARE_WMI_WMAX
> bool "Alienware WMAX WMI device driver"
> default y
> depends on ALIENWARE_WMI
> + depends on HWMON
> select ACPI_PLATFORM_PROFILE
> help
> Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -9,10 +9,13 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> #include <linux/bits.h>
> #include <linux/dmi.h>
> +#include <linux/hwmon.h>
> #include <linux/moduleparam.h>
> #include <linux/platform_profile.h>
> +#include <linux/units.h>
> #include <linux/wmi.h>
> #include "alienware-wmi.h"
>
> @@ -25,6 +28,7 @@
> #define WMAX_METHOD_BRIGHTNESS 0x3
> #define WMAX_METHOD_ZONE_CONTROL 0x4
>
> +#define AWCC_METHOD_GET_FAN_SENSORS 0x13
> #define AWCC_METHOD_THERMAL_INFORMATION 0x14
> #define AWCC_METHOD_THERMAL_CONTROL 0x15
> #define AWCC_METHOD_GAME_SHIFT_STATUS 0x25
> @@ -39,6 +43,10 @@
> /* Arbitrary limit based on supported models */
> #define AWCC_MAX_RES_COUNT 16
>
> +static bool force_hwmon;
> +module_param_unsafe(force_hwmon, bool, 0);
> +MODULE_PARM_DESC(force_hwmon, "Force probing for HWMON support without checking if the WMI backend is available");
> +
> static bool force_platform_profile;
> module_param_unsafe(force_platform_profile, bool, 0);
> MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
> @@ -48,16 +56,19 @@ module_param_unsafe(force_gmode, bool, 0);
> MODULE_PARM_DESC(force_gmode, "Forces G-Mode when performance profile is selected");
>
> struct awcc_quirks {
> + bool hwmon;
> bool pprof;
> bool gmode;
> };
>
> static struct awcc_quirks g_series_quirks = {
> + .hwmon = true,
> .pprof = true,
> .gmode = true,
> };
>
> static struct awcc_quirks generic_quirks = {
> + .hwmon = true,
> .pprof = true,
> .gmode = false,
> };
> @@ -155,9 +166,18 @@ static const struct dmi_system_id awcc_dmi_table[] __initconst = {
> },
> };
>
> +enum AWCC_GET_FAN_SENSORS_OPERATIONS {
> + AWCC_OP_GET_TOTAL_FAN_TEMPS = 0x01,
> + AWCC_OP_GET_FAN_TEMP_ID = 0x02,
> +};
> +
> enum AWCC_THERMAL_INFORMATION_OPERATIONS {
> AWCC_OP_GET_SYSTEM_DESCRIPTION = 0x02,
> AWCC_OP_GET_RESOURCE_ID = 0x03,
> + AWCC_OP_GET_TEMPERATURE = 0x04,
> + AWCC_OP_GET_FAN_RPM = 0x05,
> + AWCC_OP_GET_FAN_MIN_RPM = 0x08,
> + AWCC_OP_GET_FAN_MAX_RPM = 0x09,
> AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
> };
>
> @@ -180,6 +200,12 @@ enum AWCC_SPECIAL_THERMAL_CODES {
> AWCC_SPECIAL_PROFILE_GMODE = 0xAB,
> };
>
> +enum AWCC_TEMP_SENSOR_TYPES {
> + AWCC_TEMP_SENSOR_CPU = 0x01,
> + AWCC_TEMP_SENSOR_GPU = 0x06,
> + AWCC_TEMP_SENSOR_LAST
> +};
> +
> enum awcc_thermal_profile {
> AWCC_PROFILE_USTT_BALANCED,
> AWCC_PROFILE_USTT_BALANCED_PERFORMANCE,
> @@ -216,6 +242,15 @@ struct wmax_u32_args {
> u8 arg3;
> };
>
> +struct awcc_fan_data {
> + unsigned long *related_temps;
> + unsigned long *auto_channels_temp;
> + u32 total_temps;
> + u32 min_rpm;
> + u32 max_rpm;
> + u8 id;
> +};
> +
> struct awcc_priv {
> struct wmi_device *wdev;
> union {
> @@ -231,6 +266,11 @@ struct awcc_priv {
>
> struct device *ppdev;
> u8 supported_profiles[PLATFORM_PROFILE_LAST];
> +
> + struct device *hwdev;
> + struct awcc_fan_data **fan_data;
> + unsigned int temp_sensors_size;
> + unsigned long *temp_sensors;
> };
>
> static const enum platform_profile_option awcc_mode_to_platform_profile[AWCC_PROFILE_LAST] = {
> @@ -495,6 +535,19 @@ static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
> return 0;
> }
>
> +static inline int awcc_get_fan_sensors(struct wmi_device *wdev, u8 operation,
> + u8 fan_id, u8 index, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = operation,
> + .arg1 = fan_id,
> + .arg2 = index,
> + .arg3 = 0,
> + };
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_GET_FAN_SENSORS, &args, out);
> +}
> +
> static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
> u8 arg, u32 *out)
> {
> @@ -552,6 +605,32 @@ static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32
> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> }
>
> +static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_GET_FAN_RPM,
> + .arg1 = fan_id,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> +}
> +
> +static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_GET_TEMPERATURE,
> + .arg1 = temp_id,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
> +
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> +}
> +
> static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
> {
> struct wmax_u32_args args = {
> @@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil
> return 0;
> }
>
> +/*
> + * HWMON
> + * - Provides temperature and fan speed monitoring as well as manual fan
> + * control
> + */
> +static umode_t awcc_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + const struct awcc_priv *priv = drvdata;
> + unsigned int temp_count;
> +
> + switch (type) {
> + case hwmon_temp:
> + temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size);
> +
> + return channel < temp_count ? 0444 : 0;
> + case hwmon_fan:
> + return channel < priv->fan_count ? 0444 : 0;
> + case hwmon_pwm:
> + if (channel >= priv->fan_count)
> + return 0;
> +
> + switch (attr) {
> + case hwmon_pwm_enable:
Please drop pwm_enable here and only introduce it inside the proper patch.
> + return 0644;
> + case hwmon_pwm_auto_channels_temp:
> + return 0444;
> + default:
> + return 0;
> + }
> + default:
> + return 0;
> + }
> +}
> +
> +static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + struct awcc_fan_data *fan;
> + u32 state;
> + int ret;
> + u8 temp;
> +
> + switch (type) {
> + case hwmon_temp:
> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
> +
> + switch (attr) {
> + case hwmon_temp_input:
> + ret = awcc_op_get_temperature(priv->wdev, temp, &state);
> + if (ret)
> + return ret;
> +
> + *val = state * MILLIDEGREE_PER_DEGREE;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + break;
> + case hwmon_fan:
> + fan = priv->fan_data[channel];
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state);
> + if (ret)
> + return ret;
> +
> + *val = state;
> + break;
> + case hwmon_fan_min:
> + *val = fan->min_rpm;
> + break;
> + case hwmon_fan_max:
> + *val = fan->max_rpm;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + break;
> + case hwmon_pwm:
> + fan = priv->fan_data[channel];
> +
> + switch (attr) {
> + case hwmon_pwm_auto_channels_temp:
> + bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, const char **str)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + struct awcc_fan_data *fan;
> + u8 temp;
> +
> + switch (type) {
> + case hwmon_temp:
> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
> +
> + switch (temp) {
> + case AWCC_TEMP_SENSOR_CPU:
> + *str = "CPU";
> + break;
> + case AWCC_TEMP_SENSOR_GPU:
> + *str = "GPU";
> + break;
> + default:
> + *str = "Unknown";
> + break;
> + }
> +
> + break;
> + case hwmon_fan:
> + fan = priv->fan_data[channel];
> +
> + switch (fan->total_temps) {
> + case 0:
> + *str = "Independent Fan";
> + break;
> + case 1:
> + temp = find_first_bit(fan->related_temps, priv->temp_sensors_size);
> +
> + switch (temp) {
> + case AWCC_TEMP_SENSOR_CPU:
> + *str = "Processor Fan";
> + break;
> + case AWCC_TEMP_SENSOR_GPU:
> + *str = "Video Fan";
> + break;
> + default:
> + *str = "Unknown Fan";
> + break;
> + }
> +
> + break;
> + default:
> + *str = "Shared Fan";
> + break;
> + }
> +
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static const struct hwmon_ops awcc_hwmon_ops = {
> + .is_visible = awcc_hwmon_is_visible,
> + .read = awcc_hwmon_read,
> + .read_string = awcc_hwmon_read_string,
> +};
> +
> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_LABEL | HWMON_T_INPUT,
> + HWMON_T_LABEL | HWMON_T_INPUT,
> + HWMON_T_LABEL | HWMON_T_INPUT,
> + HWMON_T_LABEL | HWMON_T_INPUT,
> + HWMON_T_LABEL | HWMON_T_INPUT,
> + HWMON_T_LABEL | HWMON_T_INPUT
> + ),
> + HWMON_CHANNEL_INFO(fan,
> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
> + ),
> + HWMON_CHANNEL_INFO(pwm,
> + HWMON_PWM_AUTO_CHANNELS_TEMP,
> + HWMON_PWM_AUTO_CHANNELS_TEMP,
> + HWMON_PWM_AUTO_CHANNELS_TEMP,
> + HWMON_PWM_AUTO_CHANNELS_TEMP,
> + HWMON_PWM_AUTO_CHANNELS_TEMP,
> + HWMON_PWM_AUTO_CHANNELS_TEMP
> + ),
Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info
would make sense.
> + NULL
> +};
> +
> +static const struct hwmon_chip_info awcc_hwmon_chip_info = {
> + .ops = &awcc_hwmon_ops,
> + .info = awcc_hwmon_info,
> +};
> +
> +static int awcc_hwmon_temps_init(struct wmi_device *wdev)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
> + unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)];
> + unsigned int i, max_sensor_id = 0;
> + int ret;
> + u32 id;
> +
> + for (i = 0; i < priv->temp_count; i++) {
> + /*
> + * Temperature sensors IDs are listed after the fan IDs at
> + * offset `fan_count`
> + */
> + ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id);
> + if (ret)
> + return ret;
> +
> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
> + if (id > max_sensor_id)
> + max_sensor_id = id;
> +
> + __set_bit(id, temp_sensors);
> + }
> +
> + /*
> + * We prefer to allocate the bitmap dynamically because usually temp IDs
> + * are small (< 0x30) and only one UL is needed to store it, but there
> + * may be unknown devices that break this rule
> + */
Hi,
as far as i know the memory allocator inside the kernel at least allocates 32 bytes, so you are
not saving any memory with this. I suggest you allocate the bitmaps statically.
> + priv->temp_sensors_size = max_sensor_id + 1;
> + priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size,
> + GFP_KERNEL);
> + if (!priv->temp_sensors)
> + return -ENOMEM;
> +
> + bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size);
> +
> + return 0;
> +}
> +
> +static int awcc_hwmon_fans_init(struct wmi_device *wdev)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
> + u32 id, min_rpm, max_rpm, temp_count, temp_id;
> + unsigned long gather[BITS_TO_LONGS(U8_MAX)];
> + struct awcc_fan_data *fan_data;
> + unsigned int i, j;
> + int ret;
> +
> + for (i = 0; i < priv->fan_count; i++) {
> + fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL);
> + if (!fan_data)
> + return -ENOMEM;
> +
> + fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev,
> + priv->temp_sensors_size,
> + GFP_KERNEL);
Same as above
> + if (!priv->temp_sensors)
> + return -ENOMEM;
> +
> + fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev,
> + priv->temp_count,
> + GFP_KERNEL);
We already know that we only ever use the first sizeof(long) bytes from this bitmap,
please do a static allocation here.
> + if (!priv->temp_sensors)
> + return -ENOMEM;
> +
> + /*
> + * Fan IDs are listed first at offset 0
> + */
> + ret = awcc_op_get_resource_id(wdev, i, &id);
> + if (ret)
> + return ret;
> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
> +
> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id,
> + &min_rpm);
> + if (ret)
> + return ret;
> +
> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id,
> + &max_rpm);
> + if (ret)
> + return ret;
> +
> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id,
> + 0, &temp_count);
> + if (ret)
> + return ret;
> +
> + for (j = 0; j < temp_count; j++) {
> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID,
> + id, j, &temp_id);
> + if (ret)
> + break;
> +
> + temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id);
> + if (temp_id < priv->temp_sensors_size)
> + __set_bit(temp_id, fan_data->related_temps);
> + }
> +
> + fan_data->id = id;
> + fan_data->min_rpm = min_rpm;
> + fan_data->max_rpm = max_rpm;
> + fan_data->total_temps = bitmap_weight(fan_data->related_temps,
> + priv->temp_sensors_size);
> + bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors,
> + priv->temp_sensors_size);
Since fan_data->related_temps is only used for determining the fan label after this it would
make sense to determine the fan label here and turn fan_data->related_temps into a local
variable on the stack.
Thanks,
Armin Wolf
> + bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count);
> + priv->fan_data[i] = fan_data;
> + }
> +
> + return 0;
> +}
> +
> +static int awcc_hwmon_init(struct wmi_device *wdev)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
> + int ret;
> +
> + priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count,
> + sizeof(*priv->fan_data), GFP_KERNEL);
> + if (!priv->fan_data)
> + return -ENOMEM;
> +
> + ret = awcc_hwmon_temps_init(wdev);
> + if (ret)
> + return ret;
> +
> + ret = awcc_hwmon_fans_init(wdev);
> + if (ret)
> + return ret;
> +
> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
> + &awcc_hwmon_chip_info, NULL);
> +
> + return PTR_ERR_OR_ZERO(priv->hwdev);
> +}
> +
> /*
> * Thermal Profile control
> * - Provides thermal profile control through the Platform Profile API
> @@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
> priv->wdev = wdev;
> dev_set_drvdata(&wdev->dev, priv);
>
> + if (awcc->hwmon) {
> + ret = awcc_hwmon_init(wdev);
> + if (ret)
> + return ret;
> + }
> +
> if (awcc->pprof) {
> ret = awcc_platform_profile_init(wdev);
> if (ret)
> @@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void)
> if (id)
> awcc = id->driver_data;
>
> + if (force_hwmon) {
> + if (!awcc)
> + awcc = &empty_quirks;
> +
> + awcc->hwmon = true;
> + }
> +
> if (force_platform_profile) {
> if (!awcc)
> awcc = &empty_quirks;
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control
2025-03-06 0:56 ` [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control Kurt Borja
@ 2025-03-06 22:35 ` Armin Wolf
2025-03-07 0:16 ` Kurt Borja
0 siblings, 1 reply; 24+ messages in thread
From: Armin Wolf @ 2025-03-06 22:35 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Guenter Roeck, Jean Delvare, linux-hwmon
Am 06.03.25 um 01:56 schrieb Kurt Borja:
> All models with the "AWCC" WMAX device support a way of manually
> controlling fans.
>
> The PWM duty cycle of a fan can't be controlled directly. Instead the
> AWCC interface let's us tune a PWM `boost` value, which has the
> following empirically discovered, aproximate behavior over the PWM
> value:
>
> pwm = pwm_base + (pwm_boost / 255) * (pwm_max - pwm_base)
>
> Where the pwm_base is the locked PWM value controlled by the FW and
> pwm_boost is a value between 0 and 255.
>
> Expose this pwm_boost knob as a custom HWMON attribute.
>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: linux-hwmon@vger.kernel.org
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> drivers/platform/x86/dell/alienware-wmi-wmax.c | 223 ++++++++++++++++++++++++-
> 1 file changed, 220 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index 20cf3371ee3c0e1ea038b3ca517e831f3b30dc29..de4e8f177aadc9552b05cc732e41ee458b761143 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -13,8 +13,11 @@
> #include <linux/bits.h>
> #include <linux/dmi.h>
> #include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/minmax.h>
> #include <linux/moduleparam.h>
> #include <linux/platform_profile.h>
> +#include <linux/pm.h>
> #include <linux/units.h>
> #include <linux/wmi.h>
> #include "alienware-wmi.h"
> @@ -179,10 +182,12 @@ enum AWCC_THERMAL_INFORMATION_OPERATIONS {
> AWCC_OP_GET_FAN_MIN_RPM = 0x08,
> AWCC_OP_GET_FAN_MAX_RPM = 0x09,
> AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
> + AWCC_OP_GET_FAN_BOOST = 0x0C,
> };
>
> enum AWCC_THERMAL_CONTROL_OPERATIONS {
> AWCC_OP_ACTIVATE_PROFILE = 0x01,
> + AWCC_OP_SET_FAN_BOOST = 0x02,
> };
>
> enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
> @@ -248,6 +253,7 @@ struct awcc_fan_data {
> u32 total_temps;
> u32 min_rpm;
> u32 max_rpm;
> + u8 suspend_cache;
> u8 id;
> };
>
> @@ -627,6 +633,17 @@ static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u
> .arg3 = 0,
> };
>
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> +}
> +
> +static inline int awcc_op_get_fan_boost(struct wmi_device *wdev, u8 fan_id, u32 *out)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_GET_FAN_BOOST,
> + .arg1 = fan_id,
> + .arg2 = 0,
> + .arg3 = 0,
> + };
>
> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
> }
> @@ -656,6 +673,19 @@ static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
> }
>
> +static int awcc_op_set_fan_boost(struct wmi_device *wdev, u8 fan_id, u8 boost)
> +{
> + struct wmax_u32_args args = {
> + .operation = AWCC_OP_SET_FAN_BOOST,
> + .arg1 = fan_id,
> + .arg2 = boost,
> + .arg3 = 0,
> + };
> + u32 out;
> +
> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
> +}
> +
> static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profile)
> {
> switch (id) {
> @@ -717,6 +747,7 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, long *val)
> {
> struct awcc_priv *priv = dev_get_drvdata(dev);
> + enum platform_profile_option profile;
> struct awcc_fan_data *fan;
> u32 state;
> int ret;
> @@ -765,6 +796,28 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> fan = priv->fan_data[channel];
>
> switch (attr) {
> + case hwmon_pwm_enable:
> + ret = awcc_op_get_current_profile(priv->wdev, &state);
> + if (ret)
> + return ret;
> +
> + ret = awcc_profile_id_to_pprof(state, &profile);
> + if (ret)
> + return ret;
> +
> + switch (profile) {
> + case PLATFORM_PROFILE_PERFORMANCE:
The hwmon sysfs docs say that 0 means that the fan is spinning at maximum speed. Does PLATFORM_PROFILE_PERFORMANCE
guarantee that all fans are always spinning at maximum speed?
If no then i suggest to drop support for 0.
> + *val = 0;
> + break;
> + case PLATFORM_PROFILE_CUSTOM:
> + *val = 1;
> + break;
> + default:
> + *val = 2;
> + break;
> + }
> +
> + break;
> case hwmon_pwm_auto_channels_temp:
> bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
> break;
> @@ -840,10 +893,48 @@ static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types ty
> return 0;
> }
>
> +
> +static int awcc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + int ret;
> +
> + switch (type) {
> + case hwmon_pwm:
> + switch (attr) {
> + case hwmon_pwm_enable:
> + /*
> + * We don't want to duplicate platform profile logic, so
> + * we only allow enabling manual fan control
> + */
I do not think that having pwm1_enable brings any benefit, as the pwmX_boost attributes
behave differently than pwmX attributes. I think it would be enough to document that
pwmX_boost settings will only reliably work when the custom platform profile is selected.
> + if (val != 1)
> + return -EINVAL;
> +
> + ret = awcc_op_activate_profile(priv->wdev, AWCC_SPECIAL_PROFILE_CUSTOM);
> + if (ret)
> + return ret;
> +
> + if (priv->ppdev)
> + platform_profile_notify(priv->ppdev);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> static const struct hwmon_ops awcc_hwmon_ops = {
> .is_visible = awcc_hwmon_is_visible,
> .read = awcc_hwmon_read,
> .read_string = awcc_hwmon_read_string,
> + .write = awcc_hwmon_write,
> };
>
> static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
> @@ -864,7 +955,7 @@ static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
> HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
> ),
> HWMON_CHANNEL_INFO(pwm,
> - HWMON_PWM_AUTO_CHANNELS_TEMP,
> + HWMON_PWM_AUTO_CHANNELS_TEMP | HWMON_PWM_ENABLE,
> HWMON_PWM_AUTO_CHANNELS_TEMP,
> HWMON_PWM_AUTO_CHANNELS_TEMP,
> HWMON_PWM_AUTO_CHANNELS_TEMP,
> @@ -879,6 +970,75 @@ static const struct hwmon_chip_info awcc_hwmon_chip_info = {
> .info = awcc_hwmon_info,
> };
>
> +static ssize_t pwm_boost_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + int ret, index = to_sensor_dev_attr(attr)->index;
Please initialize "index" on a separate line, can remember the reverse xmas-tree order for variables.
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + struct awcc_fan_data *fan = priv->fan_data[index];
> + u32 boost;
> +
> + ret = awcc_op_get_fan_boost(priv->wdev, fan->id, &boost);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, "%u\n", boost);
> +}
> +
> +static ssize_t pwm_boost_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int ret, index = to_sensor_dev_attr(attr)->index;
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + struct awcc_fan_data *fan = priv->fan_data[index];
> + unsigned long val;
> +
> + ret = kstrtoul(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + ret = awcc_op_set_fan_boost(priv->wdev, fan->id, clamp_val(val, 0, 255));
> +
> + return ret ? ret : count;
> +}
> +
> +static SENSOR_DEVICE_ATTR_RW(pwm1_boost, pwm_boost, 0);
> +static SENSOR_DEVICE_ATTR_RW(pwm2_boost, pwm_boost, 1);
> +static SENSOR_DEVICE_ATTR_RW(pwm3_boost, pwm_boost, 2);
> +static SENSOR_DEVICE_ATTR_RW(pwm4_boost, pwm_boost, 3);
Since those attributes are working differently than the standard pwm attributes, i suggest to
instead name them fanX_boost.
> +
> +static umode_t pwm_boost_attr_visible(struct kobject *kobj, struct attribute *attr, int n)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(kobj_to_dev(kobj));
> +
> + return n < priv->fan_count ? attr->mode : 0;
> +}
> +
> +static bool pwm_boost_group_visible(struct kobject *kobj)
> +{
> + return true;
> +}
> +
> +DEFINE_SYSFS_GROUP_VISIBLE(pwm_boost);
> +
> +static struct attribute *fan_boost_attrs[] = {
> + &sensor_dev_attr_pwm1_boost.dev_attr.attr,
> + &sensor_dev_attr_pwm2_boost.dev_attr.attr,
> + &sensor_dev_attr_pwm3_boost.dev_attr.attr,
> + &sensor_dev_attr_pwm4_boost.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group pwm_boost_group = {
> + .attrs = fan_boost_attrs,
> + .is_visible = SYSFS_GROUP_VISIBLE(pwm_boost),
> +};
> +
> +static const struct attribute_group *awcc_hwmon_groups[] = {
> + &pwm_boost_group,
> + NULL
> +};
> +
> static int awcc_hwmon_temps_init(struct wmi_device *wdev)
> {
> struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
> @@ -1011,12 +1171,50 @@ static int awcc_hwmon_init(struct wmi_device *wdev)
> if (ret)
> return ret;
>
> - priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
> - &awcc_hwmon_chip_info, NULL);
> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi",
> + priv, &awcc_hwmon_chip_info,
> + awcc_hwmon_groups);
>
> return PTR_ERR_OR_ZERO(priv->hwdev);
> }
>
> +static void awcc_hwmon_suspend(struct device *dev)
> +{
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + struct awcc_fan_data *fan;
> + unsigned int i;
> + u32 boost;
> + int ret;
> +
> + for (i = 0; i < priv->fan_count; i++) {
> + fan = priv->fan_data[i];
> +
> + ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_FAN_BOOST,
> + fan->id, &boost);
> + if (ret)
> + fan->suspend_cache = 0;
Please at least log a warning here that the fan boost value can not be restored properly.
> + else
> + fan->suspend_cache = clamp_val(boost, 0, 255);
> +
> + awcc_op_set_fan_boost(priv->wdev, fan->id, 0);
> + }
> +}
> +
> +static void awcc_hwmon_resume(struct device *dev)
> +{
> +
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + struct awcc_fan_data *fan;
> + unsigned int i;
> +
> + for (i = 0; i < priv->fan_count; i++) {
> + fan = priv->fan_data[i];
> +
> + if (fan->suspend_cache)
How does the driver restore fan boost settings with a value of 0?
Thanks,
Armin Wolf
> + awcc_op_set_fan_boost(priv->wdev, fan->id, fan->suspend_cache);
> + }
> +}
> +
> /*
> * Thermal Profile control
> * - Provides thermal profile control through the Platform Profile API
> @@ -1233,6 +1431,24 @@ static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
> return ret;
> }
>
> +static int wmax_wmi_suspend(struct device *dev)
> +{
> + if (awcc->hwmon)
> + awcc_hwmon_suspend(dev);
> +
> + return 0;
> +}
> +
> +static int wmax_wmi_resume(struct device *dev)
> +{
> + if (awcc->hwmon)
> + awcc_hwmon_resume(dev);
> +
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_DEV_PM_OPS(wmax_wmi_pm_ops, wmax_wmi_suspend, wmax_wmi_resume);
> +
> static const struct wmi_device_id alienware_wmax_device_id_table[] = {
> { WMAX_CONTROL_GUID, NULL },
> { },
> @@ -1243,6 +1459,7 @@ static struct wmi_driver alienware_wmax_wmi_driver = {
> .driver = {
> .name = "alienware-wmi-wmax",
> .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + .pm = pm_sleep_ptr(&wmax_wmi_pm_ops),
> },
> .id_table = alienware_wmax_device_id_table,
> .probe = wmax_wmi_probe,
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 09/10] platform/x86: alienware-wmi-wmax: Add a DebugFS interface
2025-03-06 0:57 ` [PATCH v3 09/10] platform/x86: alienware-wmi-wmax: Add a DebugFS interface Kurt Borja
@ 2025-03-06 22:38 ` Armin Wolf
0 siblings, 0 replies; 24+ messages in thread
From: Armin Wolf @ 2025-03-06 22:38 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel
Am 06.03.25 um 01:57 schrieb Kurt Borja:
> Add a debugfs interface which exposes thermal private data.
>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> drivers/platform/x86/dell/alienware-wmi-wmax.c | 92 ++++++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index de4e8f177aadc9552b05cc732e41ee458b761143..23f8680a212fb9ef2a6f23aafcc2d25738ae4364 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -11,6 +11,7 @@
> #include <linux/bitfield.h>
> #include <linux/bitmap.h>
> #include <linux/bits.h>
> +#include <linux/debugfs.h>
> #include <linux/dmi.h>
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> @@ -18,6 +19,7 @@
> #include <linux/moduleparam.h>
> #include <linux/platform_profile.h>
> #include <linux/pm.h>
> +#include <linux/seq_file.h>
> #include <linux/units.h>
> #include <linux/wmi.h>
> #include "alienware-wmi.h"
> @@ -1343,6 +1345,94 @@ static int awcc_platform_profile_init(struct wmi_device *wdev)
> return PTR_ERR_OR_ZERO(priv->ppdev);
> }
>
> +/*
> + * DebugFS
> + */
> +static int awcc_debugfs_system_description_read(struct seq_file *seq, void *data)
> +{
> + struct device *dev = seq->private;
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> +
> + seq_printf(seq, "0x%08x\n", priv->system_description);
> +
> + return 0;
> +}
> +
> +static int awcc_debugfs_hwmon_data_read(struct seq_file *seq, void *data)
> +{
> + struct device *dev = seq->private;
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> + struct awcc_fan_data *fan_data;
> + u8 bit;
> +
> + seq_printf(seq, "Number of fans: %u\n", priv->fan_count);
> + seq_printf(seq, "Number of temperature sensors: %u\n\n", priv->temp_count);
> +
> + for (u32 i = 0; i < priv->fan_count; i++) {
> + fan_data = priv->fan_data[i];
> +
> + seq_printf(seq, "Fan %u:\n", i);
> + seq_printf(seq, " ID: 0x%02x\n", fan_data->id);
> + seq_printf(seq, " Related temperature sensors: ");
> + for_each_set_bit(bit, fan_data->related_temps, priv->temp_sensors_size)
> + seq_printf(seq, "0x%02x ", bit);
> + seq_puts(seq, "\n");
> + }
> +
> + seq_puts(seq, "\n");
> +
> + seq_printf(seq, "Temperature sensor IDs:\n");
> + for_each_set_bit(bit, priv->temp_sensors, priv->temp_sensors_size)
> + seq_printf(seq, " 0x%02x\n", bit);
> +
> + return 0;
> +}
> +
> +static int awcc_debugfs_pprof_data_read(struct seq_file *seq, void *data)
> +{
> + struct device *dev = seq->private;
> + struct awcc_priv *priv = dev_get_drvdata(dev);
> +
> + seq_printf(seq, "Number of thermal profiles: %u\n\n", priv->profile_count);
> +
> + for (u32 i = 0; i < PLATFORM_PROFILE_LAST; i++) {
> + if (!priv->supported_profiles[i])
> + continue;
> +
> + seq_printf(seq, "Platform profile %u:\n", i);
> + seq_printf(seq, " ID: 0x%02x\n", priv->supported_profiles[i]);
> + }
> +
> + return 0;
> +}
> +
> +static void awcc_debugfs_remove(void *data)
> +{
> + struct dentry *root = data;
> +
> + debugfs_remove(root);
> +}
> +
> +static void awcc_debugfs_init(struct wmi_device *wdev)
> +{
> + struct dentry *root;
> +
> + root = debugfs_create_dir("alienware-wmi", NULL);
Please use a unique name for each driver instance. You can do this by combining the
WMI device name with the driver name.
With that being fixed:
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> +
> + debugfs_create_devm_seqfile(&wdev->dev, "system_description", root,
> + awcc_debugfs_system_description_read);
> +
> + if (awcc->hwmon)
> + debugfs_create_devm_seqfile(&wdev->dev, "hwmon_data", root,
> + awcc_debugfs_hwmon_data_read);
> +
> + if (awcc->pprof)
> + debugfs_create_devm_seqfile(&wdev->dev, "pprof_data", root,
> + awcc_debugfs_pprof_data_read);
> +
> + devm_add_action_or_reset(&wdev->dev, awcc_debugfs_remove, root);
> +}
> +
> static int alienware_awcc_setup(struct wmi_device *wdev)
> {
> struct awcc_priv *priv;
> @@ -1381,6 +1471,8 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
> return ret;
> }
>
> + awcc_debugfs_init(wdev);
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation
2025-03-06 0:57 ` [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation Kurt Borja
@ 2025-03-06 23:02 ` Armin Wolf
2025-03-06 23:57 ` Bagas Sanjaya
1 sibling, 0 replies; 24+ messages in thread
From: Armin Wolf @ 2025-03-06 23:02 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel
Am 06.03.25 um 01:57 schrieb Kurt Borja:
> Use tables to describe method operations instead of using pseudo-code.
> Drop unknown method descriptions to avoid redundancy. Drop GPIO section
> as it is currently irrelevant to this driver. Update Thermal_Information
> method documentation. Add one more helpful developer to the kudos section.
Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
> Documentation/wmi/devices/alienware-wmi.rst | 383 +++++++++-------------------
> 1 file changed, 117 insertions(+), 266 deletions(-)
>
> diff --git a/Documentation/wmi/devices/alienware-wmi.rst b/Documentation/wmi/devices/alienware-wmi.rst
> index ddc5e561960e05fc7cffe700d7d278e32ff2e7b2..79238051b18bc5de9b502325017cd5c5fcf41748 100644
> --- a/Documentation/wmi/devices/alienware-wmi.rst
> +++ b/Documentation/wmi/devices/alienware-wmi.rst
> @@ -11,7 +11,7 @@ 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.
> +with thermal control and overclocking.
>
> It is suspected that the latter is used by Alienware Command Center (AWCC) to
> manage manufacturer predefined thermal profiles. The alienware-wmi driver
> @@ -69,9 +69,6 @@ data using the `bmfdec <https://github.com/pali/bmfdec>`_ utility:
> [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
> @@ -87,175 +84,133 @@ ID 0xA0, the argument you would pass to the method is 0xA001.
> Thermal Methods
> ===============
>
> +WMI method GetFanSensors([in] uint32 arg2, [out] uint32 argr)
> +-------------------------------------------------------------
> +
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Get the number of temperature | - Byte 1: Fan ID |
> +| | sensors related with a fan ID | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Get the temperature sensor IDs | - Byte 1: Fan ID |
> +| | related to a fan sensor ID | - Byte 2: Index |
> ++--------------------+------------------------------------+--------------------+
> +
> WMI method Thermal_Information([in] uint32 arg2, [out] uint32 argr)
> -------------------------------------------------------------------
>
> -::
> -
> - if BYTE_0(arg2) == 0x01:
> - argr = 1
> -
> - if BYTE_0(arg2) == 0x02:
> - argr = SYSTEM_DESCRIPTION
> -
> - 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 0x02 returns a *system description* buffer with the following
> -structure:
> -
> -::
> -
> - out[0] -> Number of fans
> - out[1] -> Number of sensors
> - out[2] -> 0x00
> - out[3] -> Number of thermal modes
> -
> -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.
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Unknown. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Get system description number with | - None |
> +| | the following structure: | |
> +| | | |
> +| | - Byte 0: Number of fans | |
> +| | - Byte 1: Number of temperature | |
> +| | sensors | |
> +| | - Byte 2: Unknown | |
> +| | - Byte 3: Number of thermal | |
> +| | profiles | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x03 | List an ID or resource at a given | - Byte 1: Index |
> +| | index. Fan IDs, temperature IDs, | |
> +| | unknown IDs and thermal profile | |
> +| | IDs are listed in that exact | |
> +| | order. | |
> +| | | |
> +| | Operation 0x02 is used to know | |
> +| | which indexes map to which | |
> +| | resources. | |
> +| | | |
> +| | **Returns:** ID at a given index | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x04 | Get the current temperature for a | - Byte 1: Sensor |
> +| | given temperature sensor. | ID |
> ++--------------------+------------------------------------+--------------------+
> +| 0x05 | Get the current RPM for a given | - Byte 1: Fan ID |
> +| | fan. | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x06 | Get fan speed percentage. (not | - Byte 1: Fan ID |
> +| | implemented in every model) | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x07 | Unknown. | - Unknown |
> ++--------------------+------------------------------------+--------------------+
> +| 0x08 | Get minimum RPM for a given FAN | - Byte 1: Fan ID |
> +| | ID. | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x09 | Get maximum RPM for a given FAN | - Byte 1: Fan ID |
> +| | ID. | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x0A | Get balanced thermal profile ID. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x0B | Get current thermal profile ID. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x0C | Get current `boost` value for a | - Byte 1: Fan ID |
> +| | given fan ID. | |
> ++--------------------+------------------------------------+--------------------+
>
> 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.
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Activate a given thermal profile. | - Byte 1: Thermal |
> +| | | profile ID |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Set a `boost` value for a given | - Byte 1: Fan ID |
> +| | fan ID. | - Byte 2: Boost |
> ++--------------------+------------------------------------+--------------------+
>
> These are the known thermal profile codes:
>
> -::
> ++------------------------------+----------+------+
> +| Thermal Profile | Type | ID |
> ++==============================+==========+======+
> +| Custom | Special | 0x00 |
> ++------------------------------+----------+------+
> +| G-Mode | Special | 0xAB |
> ++------------------------------+----------+------+
> +| Quiet | Legacy | 0x96 |
> ++------------------------------+----------+------+
> +| Balanced | Legacy | 0x97 |
> ++------------------------------+----------+------+
> +| Balanced Performance | Legacy | 0x98 |
> ++------------------------------+----------+------+
> +| Performance | Legacy | 0x99 |
> ++------------------------------+----------+------+
> +| Balanced | USTT | 0xA0 |
> ++------------------------------+----------+------+
> +| Balanced Performance | USTT | 0xA1 |
> ++------------------------------+----------+------+
> +| Cool | USTT | 0xA2 |
> ++------------------------------+----------+------+
> +| Quiet | USTT | 0xA3 |
> ++------------------------------+----------+------+
> +| Performance | USTT | 0xA4 |
> ++------------------------------+----------+------+
> +| Low Power | USTT | 0xA5 |
> ++------------------------------+----------+------+
>
> - CUSTOM 0x00
> +If a model supports the User Selectable Thermal Tables (USTT) profiles, it will
> +not support the Legacy profiles and vice-versa.
>
> - 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.
> +Every model supports the CUSTOM (0x00) thermal profile. 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()
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Toggle *Game Shift*. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Get *Game Shift* status. | - None |
> ++--------------------+------------------------------------+--------------------+
>
> 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.
> @@ -267,131 +222,27 @@ 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
> +Returns the chassis color internal ID.
>
> Acknowledgements
> ================
>
> -Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ for documenting
> -and testing available thermal profile codes.
> +Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ and
> +`T-Troll <https://github.com/T-Troll/alienfx-tools/>`_ for documenting and
> +testing some of this device's functionality, making it possible to generalize
> +this driver.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation
2025-03-06 0:57 ` [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation Kurt Borja
2025-03-06 23:02 ` Armin Wolf
@ 2025-03-06 23:57 ` Bagas Sanjaya
2025-03-07 0:39 ` Kurt Borja
1 sibling, 1 reply; 24+ messages in thread
From: Bagas Sanjaya @ 2025-03-06 23:57 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen, Armin Wolf
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 19035 bytes --]
On Wed, Mar 05, 2025 at 07:57:01PM -0500, Kurt Borja wrote:
> diff --git a/Documentation/wmi/devices/alienware-wmi.rst b/Documentation/wmi/devices/alienware-wmi.rst
> index ddc5e561960e05fc7cffe700d7d278e32ff2e7b2..79238051b18bc5de9b502325017cd5c5fcf41748 100644
> --- a/Documentation/wmi/devices/alienware-wmi.rst
> +++ b/Documentation/wmi/devices/alienware-wmi.rst
> @@ -11,7 +11,7 @@ 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.
> +with thermal control and overclocking.
>
> It is suspected that the latter is used by Alienware Command Center (AWCC) to
> manage manufacturer predefined thermal profiles. The alienware-wmi driver
> @@ -69,9 +69,6 @@ data using the `bmfdec <https://github.com/pali/bmfdec>`_ utility:
> [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
> @@ -87,175 +84,133 @@ ID 0xA0, the argument you would pass to the method is 0xA001.
> Thermal Methods
> ===============
>
> +WMI method GetFanSensors([in] uint32 arg2, [out] uint32 argr)
> +-------------------------------------------------------------
> +
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Get the number of temperature | - Byte 1: Fan ID |
> +| | sensors related with a fan ID | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Get the temperature sensor IDs | - Byte 1: Fan ID |
> +| | related to a fan sensor ID | - Byte 2: Index |
> ++--------------------+------------------------------------+--------------------+
> +
> WMI method Thermal_Information([in] uint32 arg2, [out] uint32 argr)
> -------------------------------------------------------------------
>
> -::
> -
> - if BYTE_0(arg2) == 0x01:
> - argr = 1
> -
> - if BYTE_0(arg2) == 0x02:
> - argr = SYSTEM_DESCRIPTION
> -
> - 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 0x02 returns a *system description* buffer with the following
> -structure:
> -
> -::
> -
> - out[0] -> Number of fans
> - out[1] -> Number of sensors
> - out[2] -> 0x00
> - out[3] -> Number of thermal modes
> -
> -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.
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Unknown. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Get system description number with | - None |
> +| | the following structure: | |
> +| | | |
> +| | - Byte 0: Number of fans | |
> +| | - Byte 1: Number of temperature | |
> +| | sensors | |
> +| | - Byte 2: Unknown | |
> +| | - Byte 3: Number of thermal | |
> +| | profiles | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x03 | List an ID or resource at a given | - Byte 1: Index |
> +| | index. Fan IDs, temperature IDs, | |
> +| | unknown IDs and thermal profile | |
> +| | IDs are listed in that exact | |
> +| | order. | |
> +| | | |
> +| | Operation 0x02 is used to know | |
> +| | which indexes map to which | |
> +| | resources. | |
> +| | | |
> +| | **Returns:** ID at a given index | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x04 | Get the current temperature for a | - Byte 1: Sensor |
> +| | given temperature sensor. | ID |
> ++--------------------+------------------------------------+--------------------+
> +| 0x05 | Get the current RPM for a given | - Byte 1: Fan ID |
> +| | fan. | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x06 | Get fan speed percentage. (not | - Byte 1: Fan ID |
> +| | implemented in every model) | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x07 | Unknown. | - Unknown |
> ++--------------------+------------------------------------+--------------------+
> +| 0x08 | Get minimum RPM for a given FAN | - Byte 1: Fan ID |
> +| | ID. | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x09 | Get maximum RPM for a given FAN | - Byte 1: Fan ID |
> +| | ID. | |
> ++--------------------+------------------------------------+--------------------+
> +| 0x0A | Get balanced thermal profile ID. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x0B | Get current thermal profile ID. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x0C | Get current `boost` value for a | - Byte 1: Fan ID |
> +| | given fan ID. | |
> ++--------------------+------------------------------------+--------------------+
>
> 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.
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Activate a given thermal profile. | - Byte 1: Thermal |
> +| | | profile ID |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Set a `boost` value for a given | - Byte 1: Fan ID |
> +| | fan ID. | - Byte 2: Boost |
> ++--------------------+------------------------------------+--------------------+
>
> These are the known thermal profile codes:
>
> -::
> ++------------------------------+----------+------+
> +| Thermal Profile | Type | ID |
> ++==============================+==========+======+
> +| Custom | Special | 0x00 |
> ++------------------------------+----------+------+
> +| G-Mode | Special | 0xAB |
> ++------------------------------+----------+------+
> +| Quiet | Legacy | 0x96 |
> ++------------------------------+----------+------+
> +| Balanced | Legacy | 0x97 |
> ++------------------------------+----------+------+
> +| Balanced Performance | Legacy | 0x98 |
> ++------------------------------+----------+------+
> +| Performance | Legacy | 0x99 |
> ++------------------------------+----------+------+
> +| Balanced | USTT | 0xA0 |
> ++------------------------------+----------+------+
> +| Balanced Performance | USTT | 0xA1 |
> ++------------------------------+----------+------+
> +| Cool | USTT | 0xA2 |
> ++------------------------------+----------+------+
> +| Quiet | USTT | 0xA3 |
> ++------------------------------+----------+------+
> +| Performance | USTT | 0xA4 |
> ++------------------------------+----------+------+
> +| Low Power | USTT | 0xA5 |
> ++------------------------------+----------+------+
>
> - CUSTOM 0x00
> +If a model supports the User Selectable Thermal Tables (USTT) profiles, it will
> +not support the Legacy profiles and vice-versa.
>
> - 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.
> +Every model supports the CUSTOM (0x00) thermal profile. 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()
> ++--------------------+------------------------------------+--------------------+
> +| Operation (Byte 0) | Description | Arguments |
> ++====================+====================================+====================+
> +| 0x01 | Toggle *Game Shift*. | - None |
> ++--------------------+------------------------------------+--------------------+
> +| 0x02 | Get *Game Shift* status. | - None |
> ++--------------------+------------------------------------+--------------------+
>
> 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.
> @@ -267,131 +222,27 @@ 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
> +Returns the chassis color internal ID.
>
> Acknowledgements
> ================
>
> -Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ for documenting
> -and testing available thermal profile codes.
> +Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ and
> +`T-Troll <https://github.com/T-Troll/alienfx-tools/>`_ for documenting and
> +testing some of this device's functionality, making it possible to generalize
> +this driver.
>
Looks good, thanks!
Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control
2025-03-06 22:35 ` Armin Wolf
@ 2025-03-07 0:16 ` Kurt Borja
2025-03-07 21:18 ` Armin Wolf
0 siblings, 1 reply; 24+ messages in thread
From: Kurt Borja @ 2025-03-07 0:16 UTC (permalink / raw)
To: Armin Wolf, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Guenter Roeck, Jean Delvare, linux-hwmon
Hi Armin,
On Thu Mar 6, 2025 at 5:35 PM -05, Armin Wolf wrote:
> Am 06.03.25 um 01:56 schrieb Kurt Borja:
>
>> All models with the "AWCC" WMAX device support a way of manually
>> controlling fans.
>>
>> The PWM duty cycle of a fan can't be controlled directly. Instead the
>> AWCC interface let's us tune a PWM `boost` value, which has the
>> following empirically discovered, aproximate behavior over the PWM
>> value:
>>
>> pwm = pwm_base + (pwm_boost / 255) * (pwm_max - pwm_base)
>>
>> Where the pwm_base is the locked PWM value controlled by the FW and
>> pwm_boost is a value between 0 and 255.
>>
>> Expose this pwm_boost knob as a custom HWMON attribute.
>>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Jean Delvare <jdelvare@suse.com>
>> Cc: linux-hwmon@vger.kernel.org
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>> drivers/platform/x86/dell/alienware-wmi-wmax.c | 223 ++++++++++++++++++++++++-
>> 1 file changed, 220 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> index 20cf3371ee3c0e1ea038b3ca517e831f3b30dc29..de4e8f177aadc9552b05cc732e41ee458b761143 100644
>> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> @@ -13,8 +13,11 @@
>> #include <linux/bits.h>
>> #include <linux/dmi.h>
>> #include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +#include <linux/minmax.h>
>> #include <linux/moduleparam.h>
>> #include <linux/platform_profile.h>
>> +#include <linux/pm.h>
>> #include <linux/units.h>
>> #include <linux/wmi.h>
>> #include "alienware-wmi.h"
>> @@ -179,10 +182,12 @@ enum AWCC_THERMAL_INFORMATION_OPERATIONS {
>> AWCC_OP_GET_FAN_MIN_RPM = 0x08,
>> AWCC_OP_GET_FAN_MAX_RPM = 0x09,
>> AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
>> + AWCC_OP_GET_FAN_BOOST = 0x0C,
>> };
>>
>> enum AWCC_THERMAL_CONTROL_OPERATIONS {
>> AWCC_OP_ACTIVATE_PROFILE = 0x01,
>> + AWCC_OP_SET_FAN_BOOST = 0x02,
>> };
>>
>> enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
>> @@ -248,6 +253,7 @@ struct awcc_fan_data {
>> u32 total_temps;
>> u32 min_rpm;
>> u32 max_rpm;
>> + u8 suspend_cache;
>> u8 id;
>> };
>>
>> @@ -627,6 +633,17 @@ static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u
>> .arg3 = 0,
>> };
>>
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> +}
>> +
>> +static inline int awcc_op_get_fan_boost(struct wmi_device *wdev, u8 fan_id, u32 *out)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = AWCC_OP_GET_FAN_BOOST,
>> + .arg1 = fan_id,
>> + .arg2 = 0,
>> + .arg3 = 0,
>> + };
>>
>> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> }
>> @@ -656,6 +673,19 @@ static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
>> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
>> }
>>
>> +static int awcc_op_set_fan_boost(struct wmi_device *wdev, u8 fan_id, u8 boost)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = AWCC_OP_SET_FAN_BOOST,
>> + .arg1 = fan_id,
>> + .arg2 = boost,
>> + .arg3 = 0,
>> + };
>> + u32 out;
>> +
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
>> +}
>> +
>> static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profile)
>> {
>> switch (id) {
>> @@ -717,6 +747,7 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>> u32 attr, int channel, long *val)
>> {
>> struct awcc_priv *priv = dev_get_drvdata(dev);
>> + enum platform_profile_option profile;
>> struct awcc_fan_data *fan;
>> u32 state;
>> int ret;
>> @@ -765,6 +796,28 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>> fan = priv->fan_data[channel];
>>
>> switch (attr) {
>> + case hwmon_pwm_enable:
>> + ret = awcc_op_get_current_profile(priv->wdev, &state);
>> + if (ret)
>> + return ret;
>> +
>> + ret = awcc_profile_id_to_pprof(state, &profile);
>> + if (ret)
>> + return ret;
>> +
>> + switch (profile) {
>> + case PLATFORM_PROFILE_PERFORMANCE:
>
> The hwmon sysfs docs say that 0 means that the fan is spinning at maximum speed. Does PLATFORM_PROFILE_PERFORMANCE
> guarantee that all fans are always spinning at maximum speed?
Yes PERFORMANCE is full-speed for all devices I know. Manual fan control
is completely disabled for that profile too.
In fact I'm thinking about adding a module parameter to suppress this
behavior. Not everyone may like that. That's outside the scope of these
series tho.
>
> If no then i suggest to drop support for 0.
>
>> + *val = 0;
>> + break;
>> + case PLATFORM_PROFILE_CUSTOM:
>> + *val = 1;
>> + break;
>> + default:
>> + *val = 2;
>> + break;
>> + }
>> +
>> + break;
>> case hwmon_pwm_auto_channels_temp:
>> bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
>> break;
>> @@ -840,10 +893,48 @@ static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types ty
>> return 0;
>> }
>>
>> +
>> +static int awcc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>> + u32 attr, int channel, long val)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>> + int ret;
>> +
>> + switch (type) {
>> + case hwmon_pwm:
>> + switch (attr) {
>> + case hwmon_pwm_enable:
>> + /*
>> + * We don't want to duplicate platform profile logic, so
>> + * we only allow enabling manual fan control
>> + */
>
> I do not think that having pwm1_enable brings any benefit, as the pwmX_boost attributes
> behave differently than pwmX attributes. I think it would be enough to document that
> pwmX_boost settings will only reliably work when the custom platform profile is selected.
Now I realise I completely forgot about the admin-guide documentation!
I'll include it in the next revision. Is this path ok?
Documentation/admin-guide/laptops/alienware-wmi.rst
Or should I add driver specific ABI documentation? (or both ofc)
I don't want to name the file alienware-laptop because this driver is
compatible with Dell G-Series too.
>
>> + if (val != 1)
>> + return -EINVAL;
>> +
>> + ret = awcc_op_activate_profile(priv->wdev, AWCC_SPECIAL_PROFILE_CUSTOM);
>> + if (ret)
>> + return ret;
>> +
>> + if (priv->ppdev)
>> + platform_profile_notify(priv->ppdev);
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static const struct hwmon_ops awcc_hwmon_ops = {
>> .is_visible = awcc_hwmon_is_visible,
>> .read = awcc_hwmon_read,
>> .read_string = awcc_hwmon_read_string,
>> + .write = awcc_hwmon_write,
>> };
>>
>> static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>> @@ -864,7 +955,7 @@ static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>> HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
>> ),
>> HWMON_CHANNEL_INFO(pwm,
>> - HWMON_PWM_AUTO_CHANNELS_TEMP,
>> + HWMON_PWM_AUTO_CHANNELS_TEMP | HWMON_PWM_ENABLE,
>> HWMON_PWM_AUTO_CHANNELS_TEMP,
>> HWMON_PWM_AUTO_CHANNELS_TEMP,
>> HWMON_PWM_AUTO_CHANNELS_TEMP,
>> @@ -879,6 +970,75 @@ static const struct hwmon_chip_info awcc_hwmon_chip_info = {
>> .info = awcc_hwmon_info,
>> };
>>
>> +static ssize_t pwm_boost_show(struct device *dev, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + int ret, index = to_sensor_dev_attr(attr)->index;
>
> Please initialize "index" on a separate line, can remember the reverse xmas-tree order for variables.
Ack.
>
>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>> + struct awcc_fan_data *fan = priv->fan_data[index];
>> + u32 boost;
>> +
>> + ret = awcc_op_get_fan_boost(priv->wdev, fan->id, &boost);
>> + if (ret)
>> + return ret;
>> +
>> + return sysfs_emit(buf, "%u\n", boost);
>> +}
>> +
>> +static ssize_t pwm_boost_store(struct device *dev, struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + int ret, index = to_sensor_dev_attr(attr)->index;
>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>> + struct awcc_fan_data *fan = priv->fan_data[index];
>> + unsigned long val;
>> +
>> + ret = kstrtoul(buf, 0, &val);
>> + if (ret)
>> + return ret;
>> +
>> + ret = awcc_op_set_fan_boost(priv->wdev, fan->id, clamp_val(val, 0, 255));
>> +
>> + return ret ? ret : count;
>> +}
>> +
>> +static SENSOR_DEVICE_ATTR_RW(pwm1_boost, pwm_boost, 0);
>> +static SENSOR_DEVICE_ATTR_RW(pwm2_boost, pwm_boost, 1);
>> +static SENSOR_DEVICE_ATTR_RW(pwm3_boost, pwm_boost, 2);
>> +static SENSOR_DEVICE_ATTR_RW(pwm4_boost, pwm_boost, 3);
>
> Since those attributes are working differently than the standard pwm attributes, i suggest to
> instead name them fanX_boost.
I went for pwm*_boost because we also export pwm*_auto_channels_temp,
but I'm ok with fan*_boost too.
>
>> +
>> +static umode_t pwm_boost_attr_visible(struct kobject *kobj, struct attribute *attr, int n)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(kobj_to_dev(kobj));
>> +
>> + return n < priv->fan_count ? attr->mode : 0;
>> +}
>> +
>> +static bool pwm_boost_group_visible(struct kobject *kobj)
>> +{
>> + return true;
>> +}
>> +
>> +DEFINE_SYSFS_GROUP_VISIBLE(pwm_boost);
>> +
>> +static struct attribute *fan_boost_attrs[] = {
>> + &sensor_dev_attr_pwm1_boost.dev_attr.attr,
>> + &sensor_dev_attr_pwm2_boost.dev_attr.attr,
>> + &sensor_dev_attr_pwm3_boost.dev_attr.attr,
>> + &sensor_dev_attr_pwm4_boost.dev_attr.attr,
>> + NULL
>> +};
>> +
>> +static const struct attribute_group pwm_boost_group = {
>> + .attrs = fan_boost_attrs,
>> + .is_visible = SYSFS_GROUP_VISIBLE(pwm_boost),
>> +};
>> +
>> +static const struct attribute_group *awcc_hwmon_groups[] = {
>> + &pwm_boost_group,
>> + NULL
>> +};
>> +
>> static int awcc_hwmon_temps_init(struct wmi_device *wdev)
>> {
>> struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>> @@ -1011,12 +1171,50 @@ static int awcc_hwmon_init(struct wmi_device *wdev)
>> if (ret)
>> return ret;
>>
>> - priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
>> - &awcc_hwmon_chip_info, NULL);
>> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi",
>> + priv, &awcc_hwmon_chip_info,
>> + awcc_hwmon_groups);
>>
>> return PTR_ERR_OR_ZERO(priv->hwdev);
>> }
>>
>> +static void awcc_hwmon_suspend(struct device *dev)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>> + struct awcc_fan_data *fan;
>> + unsigned int i;
>> + u32 boost;
>> + int ret;
>> +
>> + for (i = 0; i < priv->fan_count; i++) {
>> + fan = priv->fan_data[i];
>> +
>> + ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_FAN_BOOST,
>> + fan->id, &boost);
>> + if (ret)
>> + fan->suspend_cache = 0;
>
> Please at least log a warning here that the fan boost value can not be restored properly.
Ack.
Is not propagating errors a good approach here? My idea was to try to
turn off fans no matter what.
>
>> + else
>> + fan->suspend_cache = clamp_val(boost, 0, 255);
>> +
>> + awcc_op_set_fan_boost(priv->wdev, fan->id, 0);
>> + }
>> +}
>> +
>> +static void awcc_hwmon_resume(struct device *dev)
>> +{
>> +
>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>> + struct awcc_fan_data *fan;
>> + unsigned int i;
>> +
>> + for (i = 0; i < priv->fan_count; i++) {
>> + fan = priv->fan_data[i];
>> +
>> + if (fan->suspend_cache)
>
> How does the driver restore fan boost settings with a value of 0?
We set to 0 when suspending so I don't think it's necessary to restore
to 0 again when resuming.
>
> Thanks,
> Armin Wolf
>
>> + awcc_op_set_fan_boost(priv->wdev, fan->id, fan->suspend_cache);
>> + }
>> +}
>> +
>> /*
>> * Thermal Profile control
>> * - Provides thermal profile control through the Platform Profile API
>> @@ -1233,6 +1431,24 @@ static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
>> return ret;
>> }
>>
>> +static int wmax_wmi_suspend(struct device *dev)
>> +{
>> + if (awcc->hwmon)
>> + awcc_hwmon_suspend(dev);
>> +
>> + return 0;
>> +}
>> +
>> +static int wmax_wmi_resume(struct device *dev)
>> +{
>> + if (awcc->hwmon)
>> + awcc_hwmon_resume(dev);
>> +
>> + return 0;
>> +}
>> +
>> +DEFINE_SIMPLE_DEV_PM_OPS(wmax_wmi_pm_ops, wmax_wmi_suspend, wmax_wmi_resume);
>> +
>> static const struct wmi_device_id alienware_wmax_device_id_table[] = {
>> { WMAX_CONTROL_GUID, NULL },
>> { },
>> @@ -1243,6 +1459,7 @@ static struct wmi_driver alienware_wmax_wmi_driver = {
>> .driver = {
>> .name = "alienware-wmi-wmax",
>> .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> + .pm = pm_sleep_ptr(&wmax_wmi_pm_ops),
>> },
>> .id_table = alienware_wmax_device_id_table,
>> .probe = wmax_wmi_probe,
>>
--
~ Kurt
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support
2025-03-06 22:19 ` Armin Wolf
@ 2025-03-07 0:35 ` Kurt Borja
2025-03-07 21:09 ` Armin Wolf
0 siblings, 1 reply; 24+ messages in thread
From: Kurt Borja @ 2025-03-07 0:35 UTC (permalink / raw)
To: Armin Wolf, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Guenter Roeck, Jean Delvare, linux-hwmon
On Thu Mar 6, 2025 at 5:19 PM -05, Armin Wolf wrote:
> Am 06.03.25 um 01:56 schrieb Kurt Borja:
>
>> All models with the "AWCC" WMAX device support monitoring fan speed and
>> temperature sensors. Expose this feature through the HWMON interface.
>>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Cc: Jean Delvare <jdelvare@suse.com>
>> Cc: linux-hwmon@vger.kernel.org
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>> drivers/platform/x86/dell/Kconfig | 1 +
>> drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++
>> 2 files changed, 432 insertions(+)
>>
>> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
>> index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 100644
>> --- a/drivers/platform/x86/dell/Kconfig
>> +++ b/drivers/platform/x86/dell/Kconfig
>> @@ -43,6 +43,7 @@ config ALIENWARE_WMI_WMAX
>> bool "Alienware WMAX WMI device driver"
>> default y
>> depends on ALIENWARE_WMI
>> + depends on HWMON
>> select ACPI_PLATFORM_PROFILE
>> help
>> Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
>> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> index 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 100644
>> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>> @@ -9,10 +9,13 @@
>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>
>> #include <linux/bitfield.h>
>> +#include <linux/bitmap.h>
>> #include <linux/bits.h>
>> #include <linux/dmi.h>
>> +#include <linux/hwmon.h>
>> #include <linux/moduleparam.h>
>> #include <linux/platform_profile.h>
>> +#include <linux/units.h>
>> #include <linux/wmi.h>
>> #include "alienware-wmi.h"
>>
>> @@ -25,6 +28,7 @@
>> #define WMAX_METHOD_BRIGHTNESS 0x3
>> #define WMAX_METHOD_ZONE_CONTROL 0x4
>>
>> +#define AWCC_METHOD_GET_FAN_SENSORS 0x13
>> #define AWCC_METHOD_THERMAL_INFORMATION 0x14
>> #define AWCC_METHOD_THERMAL_CONTROL 0x15
>> #define AWCC_METHOD_GAME_SHIFT_STATUS 0x25
>> @@ -39,6 +43,10 @@
>> /* Arbitrary limit based on supported models */
>> #define AWCC_MAX_RES_COUNT 16
>>
>> +static bool force_hwmon;
>> +module_param_unsafe(force_hwmon, bool, 0);
>> +MODULE_PARM_DESC(force_hwmon, "Force probing for HWMON support without checking if the WMI backend is available");
>> +
>> static bool force_platform_profile;
>> module_param_unsafe(force_platform_profile, bool, 0);
>> MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
>> @@ -48,16 +56,19 @@ module_param_unsafe(force_gmode, bool, 0);
>> MODULE_PARM_DESC(force_gmode, "Forces G-Mode when performance profile is selected");
>>
>> struct awcc_quirks {
>> + bool hwmon;
>> bool pprof;
>> bool gmode;
>> };
>>
>> static struct awcc_quirks g_series_quirks = {
>> + .hwmon = true,
>> .pprof = true,
>> .gmode = true,
>> };
>>
>> static struct awcc_quirks generic_quirks = {
>> + .hwmon = true,
>> .pprof = true,
>> .gmode = false,
>> };
>> @@ -155,9 +166,18 @@ static const struct dmi_system_id awcc_dmi_table[] __initconst = {
>> },
>> };
>>
>> +enum AWCC_GET_FAN_SENSORS_OPERATIONS {
>> + AWCC_OP_GET_TOTAL_FAN_TEMPS = 0x01,
>> + AWCC_OP_GET_FAN_TEMP_ID = 0x02,
>> +};
>> +
>> enum AWCC_THERMAL_INFORMATION_OPERATIONS {
>> AWCC_OP_GET_SYSTEM_DESCRIPTION = 0x02,
>> AWCC_OP_GET_RESOURCE_ID = 0x03,
>> + AWCC_OP_GET_TEMPERATURE = 0x04,
>> + AWCC_OP_GET_FAN_RPM = 0x05,
>> + AWCC_OP_GET_FAN_MIN_RPM = 0x08,
>> + AWCC_OP_GET_FAN_MAX_RPM = 0x09,
>> AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
>> };
>>
>> @@ -180,6 +200,12 @@ enum AWCC_SPECIAL_THERMAL_CODES {
>> AWCC_SPECIAL_PROFILE_GMODE = 0xAB,
>> };
>>
>> +enum AWCC_TEMP_SENSOR_TYPES {
>> + AWCC_TEMP_SENSOR_CPU = 0x01,
>> + AWCC_TEMP_SENSOR_GPU = 0x06,
>> + AWCC_TEMP_SENSOR_LAST
>> +};
>> +
>> enum awcc_thermal_profile {
>> AWCC_PROFILE_USTT_BALANCED,
>> AWCC_PROFILE_USTT_BALANCED_PERFORMANCE,
>> @@ -216,6 +242,15 @@ struct wmax_u32_args {
>> u8 arg3;
>> };
>>
>> +struct awcc_fan_data {
>> + unsigned long *related_temps;
>> + unsigned long *auto_channels_temp;
>> + u32 total_temps;
>> + u32 min_rpm;
>> + u32 max_rpm;
>> + u8 id;
>> +};
>> +
>> struct awcc_priv {
>> struct wmi_device *wdev;
>> union {
>> @@ -231,6 +266,11 @@ struct awcc_priv {
>>
>> struct device *ppdev;
>> u8 supported_profiles[PLATFORM_PROFILE_LAST];
>> +
>> + struct device *hwdev;
>> + struct awcc_fan_data **fan_data;
>> + unsigned int temp_sensors_size;
>> + unsigned long *temp_sensors;
>> };
>>
>> static const enum platform_profile_option awcc_mode_to_platform_profile[AWCC_PROFILE_LAST] = {
>> @@ -495,6 +535,19 @@ static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
>> return 0;
>> }
>>
>> +static inline int awcc_get_fan_sensors(struct wmi_device *wdev, u8 operation,
>> + u8 fan_id, u8 index, u32 *out)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = operation,
>> + .arg1 = fan_id,
>> + .arg2 = index,
>> + .arg3 = 0,
>> + };
>> +
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_GET_FAN_SENSORS, &args, out);
>> +}
>> +
>> static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
>> u8 arg, u32 *out)
>> {
>> @@ -552,6 +605,32 @@ static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32
>> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> }
>>
>> +static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = AWCC_OP_GET_FAN_RPM,
>> + .arg1 = fan_id,
>> + .arg2 = 0,
>> + .arg3 = 0,
>> + };
>> +
>> +
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> +}
>> +
>> +static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out)
>> +{
>> + struct wmax_u32_args args = {
>> + .operation = AWCC_OP_GET_TEMPERATURE,
>> + .arg1 = temp_id,
>> + .arg2 = 0,
>> + .arg3 = 0,
>> + };
>> +
>> +
>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>> +}
>> +
>> static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
>> {
>> struct wmax_u32_args args = {
>> @@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil
>> return 0;
>> }
>>
>> +/*
>> + * HWMON
>> + * - Provides temperature and fan speed monitoring as well as manual fan
>> + * control
>> + */
>> +static umode_t awcc_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
>> + u32 attr, int channel)
>> +{
>> + const struct awcc_priv *priv = drvdata;
>> + unsigned int temp_count;
>> +
>> + switch (type) {
>> + case hwmon_temp:
>> + temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size);
>> +
>> + return channel < temp_count ? 0444 : 0;
>> + case hwmon_fan:
>> + return channel < priv->fan_count ? 0444 : 0;
>> + case hwmon_pwm:
>> + if (channel >= priv->fan_count)
>> + return 0;
>> +
>> + switch (attr) {
>> + case hwmon_pwm_enable:
>
> Please drop pwm_enable here and only introduce it inside the proper patch.
Thanks, mb.
>
>> + return 0644;
>> + case hwmon_pwm_auto_channels_temp:
>> + return 0444;
>> + default:
>> + return 0;
>> + }
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> +static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>> + u32 attr, int channel, long *val)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>> + struct awcc_fan_data *fan;
>> + u32 state;
>> + int ret;
>> + u8 temp;
>> +
>> + switch (type) {
>> + case hwmon_temp:
>> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
>> +
>> + switch (attr) {
>> + case hwmon_temp_input:
>> + ret = awcc_op_get_temperature(priv->wdev, temp, &state);
>> + if (ret)
>> + return ret;
>> +
>> + *val = state * MILLIDEGREE_PER_DEGREE;
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + break;
>> + case hwmon_fan:
>> + fan = priv->fan_data[channel];
>> +
>> + switch (attr) {
>> + case hwmon_fan_input:
>> + ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state);
>> + if (ret)
>> + return ret;
>> +
>> + *val = state;
>> + break;
>> + case hwmon_fan_min:
>> + *val = fan->min_rpm;
>> + break;
>> + case hwmon_fan_max:
>> + *val = fan->max_rpm;
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + break;
>> + case hwmon_pwm:
>> + fan = priv->fan_data[channel];
>> +
>> + switch (attr) {
>> + case hwmon_pwm_auto_channels_temp:
>> + bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
>> + u32 attr, int channel, const char **str)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>> + struct awcc_fan_data *fan;
>> + u8 temp;
>> +
>> + switch (type) {
>> + case hwmon_temp:
>> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
>> +
>> + switch (temp) {
>> + case AWCC_TEMP_SENSOR_CPU:
>> + *str = "CPU";
>> + break;
>> + case AWCC_TEMP_SENSOR_GPU:
>> + *str = "GPU";
>> + break;
>> + default:
>> + *str = "Unknown";
>> + break;
>> + }
>> +
>> + break;
>> + case hwmon_fan:
>> + fan = priv->fan_data[channel];
>> +
>> + switch (fan->total_temps) {
>> + case 0:
>> + *str = "Independent Fan";
>> + break;
>> + case 1:
>> + temp = find_first_bit(fan->related_temps, priv->temp_sensors_size);
>> +
>> + switch (temp) {
>> + case AWCC_TEMP_SENSOR_CPU:
>> + *str = "Processor Fan";
>> + break;
>> + case AWCC_TEMP_SENSOR_GPU:
>> + *str = "Video Fan";
>> + break;
>> + default:
>> + *str = "Unknown Fan";
>> + break;
>> + }
>> +
>> + break;
>> + default:
>> + *str = "Shared Fan";
>> + break;
>> + }
>> +
>> + break;
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct hwmon_ops awcc_hwmon_ops = {
>> + .is_visible = awcc_hwmon_is_visible,
>> + .read = awcc_hwmon_read,
>> + .read_string = awcc_hwmon_read_string,
>> +};
>> +
>> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>> + HWMON_CHANNEL_INFO(temp,
>> + HWMON_T_LABEL | HWMON_T_INPUT,
>> + HWMON_T_LABEL | HWMON_T_INPUT,
>> + HWMON_T_LABEL | HWMON_T_INPUT,
>> + HWMON_T_LABEL | HWMON_T_INPUT,
>> + HWMON_T_LABEL | HWMON_T_INPUT,
>> + HWMON_T_LABEL | HWMON_T_INPUT
>> + ),
>> + HWMON_CHANNEL_INFO(fan,
>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
>> + ),
>> + HWMON_CHANNEL_INFO(pwm,
>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>> + HWMON_PWM_AUTO_CHANNELS_TEMP
>> + ),
>
> Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info
> would make sense.
IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead
of doing that? I might be wrong tho.
I'm fine either way.
>
>> + NULL
>> +};
>> +
>> +static const struct hwmon_chip_info awcc_hwmon_chip_info = {
>> + .ops = &awcc_hwmon_ops,
>> + .info = awcc_hwmon_info,
>> +};
>> +
>> +static int awcc_hwmon_temps_init(struct wmi_device *wdev)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>> + unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)];
>> + unsigned int i, max_sensor_id = 0;
>> + int ret;
>> + u32 id;
>> +
>> + for (i = 0; i < priv->temp_count; i++) {
>> + /*
>> + * Temperature sensors IDs are listed after the fan IDs at
>> + * offset `fan_count`
>> + */
>> + ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id);
>> + if (ret)
>> + return ret;
>> +
>> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
>> + if (id > max_sensor_id)
>> + max_sensor_id = id;
>> +
>> + __set_bit(id, temp_sensors);
>> + }
>> +
>> + /*
>> + * We prefer to allocate the bitmap dynamically because usually temp IDs
>> + * are small (< 0x30) and only one UL is needed to store it, but there
>> + * may be unknown devices that break this rule
>> + */
>
> Hi,
>
> as far as i know the memory allocator inside the kernel at least allocates 32 bytes, so you are
bytes? bits maybe?
> not saving any memory with this. I suggest you allocate the bitmaps statically.
The thing is - We don't know before hand how big an ID can be.
Technically the upper limit is U8_MAX which would require 4 ULs to
store, which is a lot. However I haven't seen temp IDs bigger than 0x6,
so this way only one UL is allocated for most devices.
I would be very grateful if Dell could help us on this one :')
>
>> + priv->temp_sensors_size = max_sensor_id + 1;
>> + priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size,
>> + GFP_KERNEL);
>> + if (!priv->temp_sensors)
>> + return -ENOMEM;
>> +
>> + bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size);
>> +
>> + return 0;
>> +}
>> +
>> +static int awcc_hwmon_fans_init(struct wmi_device *wdev)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>> + u32 id, min_rpm, max_rpm, temp_count, temp_id;
>> + unsigned long gather[BITS_TO_LONGS(U8_MAX)];
>> + struct awcc_fan_data *fan_data;
>> + unsigned int i, j;
>> + int ret;
>> +
>> + for (i = 0; i < priv->fan_count; i++) {
>> + fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL);
>> + if (!fan_data)
>> + return -ENOMEM;
>> +
>> + fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev,
>> + priv->temp_sensors_size,
>> + GFP_KERNEL);
>
> Same as above
>
>> + if (!priv->temp_sensors)
>> + return -ENOMEM;
>> +
>> + fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev,
>> + priv->temp_count,
>> + GFP_KERNEL);
>
> We already know that we only ever use the first sizeof(long) bytes from this bitmap,
> please do a static allocation here.
Right, ofc! I forgot about the limit I imposed.
>
>> + if (!priv->temp_sensors)
>> + return -ENOMEM;
>> +
>> + /*
>> + * Fan IDs are listed first at offset 0
>> + */
>> + ret = awcc_op_get_resource_id(wdev, i, &id);
>> + if (ret)
>> + return ret;
>> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
>> +
>> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id,
>> + &min_rpm);
>> + if (ret)
>> + return ret;
>> +
>> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id,
>> + &max_rpm);
>> + if (ret)
>> + return ret;
>> +
>> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id,
>> + 0, &temp_count);
>> + if (ret)
>> + return ret;
>> +
>> + for (j = 0; j < temp_count; j++) {
>> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID,
>> + id, j, &temp_id);
>> + if (ret)
>> + break;
>> +
>> + temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id);
>> + if (temp_id < priv->temp_sensors_size)
>> + __set_bit(temp_id, fan_data->related_temps);
>> + }
>> +
>> + fan_data->id = id;
>> + fan_data->min_rpm = min_rpm;
>> + fan_data->max_rpm = max_rpm;
>> + fan_data->total_temps = bitmap_weight(fan_data->related_temps,
>> + priv->temp_sensors_size);
>> + bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors,
>> + priv->temp_sensors_size);
>
> Since fan_data->related_temps is only used for determining the fan label after this it would
> make sense to determine the fan label here and turn fan_data->related_temps into a local
> variable on the stack.
Ack.
>
> Thanks,
> Armin Wolf
Thank you very much! Very helpful feedback :)
>
>> + bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count);
>> + priv->fan_data[i] = fan_data;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int awcc_hwmon_init(struct wmi_device *wdev)
>> +{
>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>> + int ret;
>> +
>> + priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count,
>> + sizeof(*priv->fan_data), GFP_KERNEL);
>> + if (!priv->fan_data)
>> + return -ENOMEM;
>> +
>> + ret = awcc_hwmon_temps_init(wdev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = awcc_hwmon_fans_init(wdev);
>> + if (ret)
>> + return ret;
>> +
>> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
>> + &awcc_hwmon_chip_info, NULL);
>> +
>> + return PTR_ERR_OR_ZERO(priv->hwdev);
>> +}
>> +
>> /*
>> * Thermal Profile control
>> * - Provides thermal profile control through the Platform Profile API
>> @@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
>> priv->wdev = wdev;
>> dev_set_drvdata(&wdev->dev, priv);
>>
>> + if (awcc->hwmon) {
>> + ret = awcc_hwmon_init(wdev);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> if (awcc->pprof) {
>> ret = awcc_platform_profile_init(wdev);
>> if (ret)
>> @@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void)
>> if (id)
>> awcc = id->driver_data;
>>
>> + if (force_hwmon) {
>> + if (!awcc)
>> + awcc = &empty_quirks;
>> +
>> + awcc->hwmon = true;
>> + }
>> +
>> if (force_platform_profile) {
>> if (!awcc)
>> awcc = &empty_quirks;
>>
--
~ Kurt
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation
2025-03-06 23:57 ` Bagas Sanjaya
@ 2025-03-07 0:39 ` Kurt Borja
0 siblings, 0 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-07 0:39 UTC (permalink / raw)
To: Bagas Sanjaya, Ilpo Järvinen, Armin Wolf
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel
On Thu Mar 6, 2025 at 6:57 PM -05, Bagas Sanjaya wrote:
> On Wed, Mar 05, 2025 at 07:57:01PM -0500, Kurt Borja wrote:
>> diff --git a/Documentation/wmi/devices/alienware-wmi.rst b/Documentation/wmi/devices/alienware-wmi.rst
>> index ddc5e561960e05fc7cffe700d7d278e32ff2e7b2..79238051b18bc5de9b502325017cd5c5fcf41748 100644
>> --- a/Documentation/wmi/devices/alienware-wmi.rst
>> +++ b/Documentation/wmi/devices/alienware-wmi.rst
>> @@ -11,7 +11,7 @@ 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.
>> +with thermal control and overclocking.
>>
>> It is suspected that the latter is used by Alienware Command Center (AWCC) to
>> manage manufacturer predefined thermal profiles. The alienware-wmi driver
>> @@ -69,9 +69,6 @@ data using the `bmfdec <https://github.com/pali/bmfdec>`_ utility:
>> [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
>> @@ -87,175 +84,133 @@ ID 0xA0, the argument you would pass to the method is 0xA001.
>> Thermal Methods
>> ===============
>>
>> +WMI method GetFanSensors([in] uint32 arg2, [out] uint32 argr)
>> +-------------------------------------------------------------
>> +
>> ++--------------------+------------------------------------+--------------------+
>> +| Operation (Byte 0) | Description | Arguments |
>> ++====================+====================================+====================+
>> +| 0x01 | Get the number of temperature | - Byte 1: Fan ID |
>> +| | sensors related with a fan ID | |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x02 | Get the temperature sensor IDs | - Byte 1: Fan ID |
>> +| | related to a fan sensor ID | - Byte 2: Index |
>> ++--------------------+------------------------------------+--------------------+
>> +
>> WMI method Thermal_Information([in] uint32 arg2, [out] uint32 argr)
>> -------------------------------------------------------------------
>>
>> -::
>> -
>> - if BYTE_0(arg2) == 0x01:
>> - argr = 1
>> -
>> - if BYTE_0(arg2) == 0x02:
>> - argr = SYSTEM_DESCRIPTION
>> -
>> - 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 0x02 returns a *system description* buffer with the following
>> -structure:
>> -
>> -::
>> -
>> - out[0] -> Number of fans
>> - out[1] -> Number of sensors
>> - out[2] -> 0x00
>> - out[3] -> Number of thermal modes
>> -
>> -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.
>> ++--------------------+------------------------------------+--------------------+
>> +| Operation (Byte 0) | Description | Arguments |
>> ++====================+====================================+====================+
>> +| 0x01 | Unknown. | - None |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x02 | Get system description number with | - None |
>> +| | the following structure: | |
>> +| | | |
>> +| | - Byte 0: Number of fans | |
>> +| | - Byte 1: Number of temperature | |
>> +| | sensors | |
>> +| | - Byte 2: Unknown | |
>> +| | - Byte 3: Number of thermal | |
>> +| | profiles | |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x03 | List an ID or resource at a given | - Byte 1: Index |
>> +| | index. Fan IDs, temperature IDs, | |
>> +| | unknown IDs and thermal profile | |
>> +| | IDs are listed in that exact | |
>> +| | order. | |
>> +| | | |
>> +| | Operation 0x02 is used to know | |
>> +| | which indexes map to which | |
>> +| | resources. | |
>> +| | | |
>> +| | **Returns:** ID at a given index | |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x04 | Get the current temperature for a | - Byte 1: Sensor |
>> +| | given temperature sensor. | ID |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x05 | Get the current RPM for a given | - Byte 1: Fan ID |
>> +| | fan. | |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x06 | Get fan speed percentage. (not | - Byte 1: Fan ID |
>> +| | implemented in every model) | |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x07 | Unknown. | - Unknown |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x08 | Get minimum RPM for a given FAN | - Byte 1: Fan ID |
>> +| | ID. | |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x09 | Get maximum RPM for a given FAN | - Byte 1: Fan ID |
>> +| | ID. | |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x0A | Get balanced thermal profile ID. | - None |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x0B | Get current thermal profile ID. | - None |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x0C | Get current `boost` value for a | - Byte 1: Fan ID |
>> +| | given fan ID. | |
>> ++--------------------+------------------------------------+--------------------+
>>
>> 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.
>> ++--------------------+------------------------------------+--------------------+
>> +| Operation (Byte 0) | Description | Arguments |
>> ++====================+====================================+====================+
>> +| 0x01 | Activate a given thermal profile. | - Byte 1: Thermal |
>> +| | | profile ID |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x02 | Set a `boost` value for a given | - Byte 1: Fan ID |
>> +| | fan ID. | - Byte 2: Boost |
>> ++--------------------+------------------------------------+--------------------+
>>
>> These are the known thermal profile codes:
>>
>> -::
>> ++------------------------------+----------+------+
>> +| Thermal Profile | Type | ID |
>> ++==============================+==========+======+
>> +| Custom | Special | 0x00 |
>> ++------------------------------+----------+------+
>> +| G-Mode | Special | 0xAB |
>> ++------------------------------+----------+------+
>> +| Quiet | Legacy | 0x96 |
>> ++------------------------------+----------+------+
>> +| Balanced | Legacy | 0x97 |
>> ++------------------------------+----------+------+
>> +| Balanced Performance | Legacy | 0x98 |
>> ++------------------------------+----------+------+
>> +| Performance | Legacy | 0x99 |
>> ++------------------------------+----------+------+
>> +| Balanced | USTT | 0xA0 |
>> ++------------------------------+----------+------+
>> +| Balanced Performance | USTT | 0xA1 |
>> ++------------------------------+----------+------+
>> +| Cool | USTT | 0xA2 |
>> ++------------------------------+----------+------+
>> +| Quiet | USTT | 0xA3 |
>> ++------------------------------+----------+------+
>> +| Performance | USTT | 0xA4 |
>> ++------------------------------+----------+------+
>> +| Low Power | USTT | 0xA5 |
>> ++------------------------------+----------+------+
>>
>> - CUSTOM 0x00
>> +If a model supports the User Selectable Thermal Tables (USTT) profiles, it will
>> +not support the Legacy profiles and vice-versa.
>>
>> - 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.
>> +Every model supports the CUSTOM (0x00) thermal profile. 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()
>> ++--------------------+------------------------------------+--------------------+
>> +| Operation (Byte 0) | Description | Arguments |
>> ++====================+====================================+====================+
>> +| 0x01 | Toggle *Game Shift*. | - None |
>> ++--------------------+------------------------------------+--------------------+
>> +| 0x02 | Get *Game Shift* status. | - None |
>> ++--------------------+------------------------------------+--------------------+
>>
>> 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.
>> @@ -267,131 +222,27 @@ 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
>> +Returns the chassis color internal ID.
>>
>> Acknowledgements
>> ================
>>
>> -Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ for documenting
>> -and testing available thermal profile codes.
>> +Kudos to `AlexIII <https://github.com/AlexIII/tcc-g15>`_ and
>> +`T-Troll <https://github.com/T-Troll/alienfx-tools/>`_ for documenting and
>> +testing some of this device's functionality, making it possible to generalize
>> +this driver.
>>
>
> Looks good, thanks!
>
> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
Thank you for taking a look!
--
~ Kurt
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support
2025-03-07 0:35 ` Kurt Borja
@ 2025-03-07 21:09 ` Armin Wolf
2025-03-07 23:59 ` Guenter Roeck
0 siblings, 1 reply; 24+ messages in thread
From: Armin Wolf @ 2025-03-07 21:09 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Guenter Roeck, Jean Delvare, linux-hwmon
Am 07.03.25 um 01:35 schrieb Kurt Borja:
> On Thu Mar 6, 2025 at 5:19 PM -05, Armin Wolf wrote:
>> Am 06.03.25 um 01:56 schrieb Kurt Borja:
>>
>>> All models with the "AWCC" WMAX device support monitoring fan speed and
>>> temperature sensors. Expose this feature through the HWMON interface.
>>>
>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>> Cc: Jean Delvare <jdelvare@suse.com>
>>> Cc: linux-hwmon@vger.kernel.org
>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>>> ---
>>> drivers/platform/x86/dell/Kconfig | 1 +
>>> drivers/platform/x86/dell/alienware-wmi-wmax.c | 431 +++++++++++++++++++++++++
>>> 2 files changed, 432 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
>>> index f8a0dffcaab7c3b423472c5b9093011334a698c8..85a57c01aaada5d899cd8252e77ed6043da5cbdf 100644
>>> --- a/drivers/platform/x86/dell/Kconfig
>>> +++ b/drivers/platform/x86/dell/Kconfig
>>> @@ -43,6 +43,7 @@ config ALIENWARE_WMI_WMAX
>>> bool "Alienware WMAX WMI device driver"
>>> default y
>>> depends on ALIENWARE_WMI
>>> + depends on HWMON
>>> select ACPI_PLATFORM_PROFILE
>>> help
>>> Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
>>> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>>> index 71fc17e8d103146b8edf53a552ae5ba64414e873..20cf3371ee3c0e1ea038b3ca517e831f3b30dc29 100644
>>> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
>>> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>>> @@ -9,10 +9,13 @@
>>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>
>>> #include <linux/bitfield.h>
>>> +#include <linux/bitmap.h>
>>> #include <linux/bits.h>
>>> #include <linux/dmi.h>
>>> +#include <linux/hwmon.h>
>>> #include <linux/moduleparam.h>
>>> #include <linux/platform_profile.h>
>>> +#include <linux/units.h>
>>> #include <linux/wmi.h>
>>> #include "alienware-wmi.h"
>>>
>>> @@ -25,6 +28,7 @@
>>> #define WMAX_METHOD_BRIGHTNESS 0x3
>>> #define WMAX_METHOD_ZONE_CONTROL 0x4
>>>
>>> +#define AWCC_METHOD_GET_FAN_SENSORS 0x13
>>> #define AWCC_METHOD_THERMAL_INFORMATION 0x14
>>> #define AWCC_METHOD_THERMAL_CONTROL 0x15
>>> #define AWCC_METHOD_GAME_SHIFT_STATUS 0x25
>>> @@ -39,6 +43,10 @@
>>> /* Arbitrary limit based on supported models */
>>> #define AWCC_MAX_RES_COUNT 16
>>>
>>> +static bool force_hwmon;
>>> +module_param_unsafe(force_hwmon, bool, 0);
>>> +MODULE_PARM_DESC(force_hwmon, "Force probing for HWMON support without checking if the WMI backend is available");
>>> +
>>> static bool force_platform_profile;
>>> module_param_unsafe(force_platform_profile, bool, 0);
>>> MODULE_PARM_DESC(force_platform_profile, "Forces auto-detecting thermal profiles without checking if WMI thermal backend is available");
>>> @@ -48,16 +56,19 @@ module_param_unsafe(force_gmode, bool, 0);
>>> MODULE_PARM_DESC(force_gmode, "Forces G-Mode when performance profile is selected");
>>>
>>> struct awcc_quirks {
>>> + bool hwmon;
>>> bool pprof;
>>> bool gmode;
>>> };
>>>
>>> static struct awcc_quirks g_series_quirks = {
>>> + .hwmon = true,
>>> .pprof = true,
>>> .gmode = true,
>>> };
>>>
>>> static struct awcc_quirks generic_quirks = {
>>> + .hwmon = true,
>>> .pprof = true,
>>> .gmode = false,
>>> };
>>> @@ -155,9 +166,18 @@ static const struct dmi_system_id awcc_dmi_table[] __initconst = {
>>> },
>>> };
>>>
>>> +enum AWCC_GET_FAN_SENSORS_OPERATIONS {
>>> + AWCC_OP_GET_TOTAL_FAN_TEMPS = 0x01,
>>> + AWCC_OP_GET_FAN_TEMP_ID = 0x02,
>>> +};
>>> +
>>> enum AWCC_THERMAL_INFORMATION_OPERATIONS {
>>> AWCC_OP_GET_SYSTEM_DESCRIPTION = 0x02,
>>> AWCC_OP_GET_RESOURCE_ID = 0x03,
>>> + AWCC_OP_GET_TEMPERATURE = 0x04,
>>> + AWCC_OP_GET_FAN_RPM = 0x05,
>>> + AWCC_OP_GET_FAN_MIN_RPM = 0x08,
>>> + AWCC_OP_GET_FAN_MAX_RPM = 0x09,
>>> AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
>>> };
>>>
>>> @@ -180,6 +200,12 @@ enum AWCC_SPECIAL_THERMAL_CODES {
>>> AWCC_SPECIAL_PROFILE_GMODE = 0xAB,
>>> };
>>>
>>> +enum AWCC_TEMP_SENSOR_TYPES {
>>> + AWCC_TEMP_SENSOR_CPU = 0x01,
>>> + AWCC_TEMP_SENSOR_GPU = 0x06,
>>> + AWCC_TEMP_SENSOR_LAST
>>> +};
>>> +
>>> enum awcc_thermal_profile {
>>> AWCC_PROFILE_USTT_BALANCED,
>>> AWCC_PROFILE_USTT_BALANCED_PERFORMANCE,
>>> @@ -216,6 +242,15 @@ struct wmax_u32_args {
>>> u8 arg3;
>>> };
>>>
>>> +struct awcc_fan_data {
>>> + unsigned long *related_temps;
>>> + unsigned long *auto_channels_temp;
>>> + u32 total_temps;
>>> + u32 min_rpm;
>>> + u32 max_rpm;
>>> + u8 id;
>>> +};
>>> +
>>> struct awcc_priv {
>>> struct wmi_device *wdev;
>>> union {
>>> @@ -231,6 +266,11 @@ struct awcc_priv {
>>>
>>> struct device *ppdev;
>>> u8 supported_profiles[PLATFORM_PROFILE_LAST];
>>> +
>>> + struct device *hwdev;
>>> + struct awcc_fan_data **fan_data;
>>> + unsigned int temp_sensors_size;
>>> + unsigned long *temp_sensors;
>>> };
>>>
>>> static const enum platform_profile_option awcc_mode_to_platform_profile[AWCC_PROFILE_LAST] = {
>>> @@ -495,6 +535,19 @@ static int __awcc_wmi_command(struct wmi_device *wdev, u32 method_id,
>>> return 0;
>>> }
>>>
>>> +static inline int awcc_get_fan_sensors(struct wmi_device *wdev, u8 operation,
>>> + u8 fan_id, u8 index, u32 *out)
>>> +{
>>> + struct wmax_u32_args args = {
>>> + .operation = operation,
>>> + .arg1 = fan_id,
>>> + .arg2 = index,
>>> + .arg3 = 0,
>>> + };
>>> +
>>> + return __awcc_wmi_command(wdev, AWCC_METHOD_GET_FAN_SENSORS, &args, out);
>>> +}
>>> +
>>> static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation,
>>> u8 arg, u32 *out)
>>> {
>>> @@ -552,6 +605,32 @@ static inline int awcc_op_get_resource_id(struct wmi_device *wdev, u8 index, u32
>>> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>>> }
>>>
>>> +static inline int awcc_op_get_fan_rpm(struct wmi_device *wdev, u8 fan_id, u32 *out)
>>> +{
>>> + struct wmax_u32_args args = {
>>> + .operation = AWCC_OP_GET_FAN_RPM,
>>> + .arg1 = fan_id,
>>> + .arg2 = 0,
>>> + .arg3 = 0,
>>> + };
>>> +
>>> +
>>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>>> +}
>>> +
>>> +static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u32 *out)
>>> +{
>>> + struct wmax_u32_args args = {
>>> + .operation = AWCC_OP_GET_TEMPERATURE,
>>> + .arg1 = temp_id,
>>> + .arg2 = 0,
>>> + .arg3 = 0,
>>> + };
>>> +
>>> +
>>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>>> +}
>>> +
>>> static inline int awcc_op_get_current_profile(struct wmi_device *wdev, u32 *out)
>>> {
>>> struct wmax_u32_args args = {
>>> @@ -599,6 +678,345 @@ static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profil
>>> return 0;
>>> }
>>>
>>> +/*
>>> + * HWMON
>>> + * - Provides temperature and fan speed monitoring as well as manual fan
>>> + * control
>>> + */
>>> +static umode_t awcc_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
>>> + u32 attr, int channel)
>>> +{
>>> + const struct awcc_priv *priv = drvdata;
>>> + unsigned int temp_count;
>>> +
>>> + switch (type) {
>>> + case hwmon_temp:
>>> + temp_count = bitmap_weight(priv->temp_sensors, priv->temp_sensors_size);
>>> +
>>> + return channel < temp_count ? 0444 : 0;
>>> + case hwmon_fan:
>>> + return channel < priv->fan_count ? 0444 : 0;
>>> + case hwmon_pwm:
>>> + if (channel >= priv->fan_count)
>>> + return 0;
>>> +
>>> + switch (attr) {
>>> + case hwmon_pwm_enable:
>> Please drop pwm_enable here and only introduce it inside the proper patch.
> Thanks, mb.
>
>>> + return 0644;
>>> + case hwmon_pwm_auto_channels_temp:
>>> + return 0444;
>>> + default:
>>> + return 0;
>>> + }
>>> + default:
>>> + return 0;
>>> + }
>>> +}
>>> +
>>> +static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>> + u32 attr, int channel, long *val)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + struct awcc_fan_data *fan;
>>> + u32 state;
>>> + int ret;
>>> + u8 temp;
>>> +
>>> + switch (type) {
>>> + case hwmon_temp:
>>> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
>>> +
>>> + switch (attr) {
>>> + case hwmon_temp_input:
>>> + ret = awcc_op_get_temperature(priv->wdev, temp, &state);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + *val = state * MILLIDEGREE_PER_DEGREE;
>>> + break;
>>> + default:
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + break;
>>> + case hwmon_fan:
>>> + fan = priv->fan_data[channel];
>>> +
>>> + switch (attr) {
>>> + case hwmon_fan_input:
>>> + ret = awcc_op_get_fan_rpm(priv->wdev, fan->id, &state);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + *val = state;
>>> + break;
>>> + case hwmon_fan_min:
>>> + *val = fan->min_rpm;
>>> + break;
>>> + case hwmon_fan_max:
>>> + *val = fan->max_rpm;
>>> + break;
>>> + default:
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + break;
>>> + case hwmon_pwm:
>>> + fan = priv->fan_data[channel];
>>> +
>>> + switch (attr) {
>>> + case hwmon_pwm_auto_channels_temp:
>>> + bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
>>> + break;
>>> + default:
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + break;
>>> + default:
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
>>> + u32 attr, int channel, const char **str)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + struct awcc_fan_data *fan;
>>> + u8 temp;
>>> +
>>> + switch (type) {
>>> + case hwmon_temp:
>>> + temp = find_nth_bit(priv->temp_sensors, priv->temp_sensors_size, channel);
>>> +
>>> + switch (temp) {
>>> + case AWCC_TEMP_SENSOR_CPU:
>>> + *str = "CPU";
>>> + break;
>>> + case AWCC_TEMP_SENSOR_GPU:
>>> + *str = "GPU";
>>> + break;
>>> + default:
>>> + *str = "Unknown";
>>> + break;
>>> + }
>>> +
>>> + break;
>>> + case hwmon_fan:
>>> + fan = priv->fan_data[channel];
>>> +
>>> + switch (fan->total_temps) {
>>> + case 0:
>>> + *str = "Independent Fan";
>>> + break;
>>> + case 1:
>>> + temp = find_first_bit(fan->related_temps, priv->temp_sensors_size);
>>> +
>>> + switch (temp) {
>>> + case AWCC_TEMP_SENSOR_CPU:
>>> + *str = "Processor Fan";
>>> + break;
>>> + case AWCC_TEMP_SENSOR_GPU:
>>> + *str = "Video Fan";
>>> + break;
>>> + default:
>>> + *str = "Unknown Fan";
>>> + break;
>>> + }
>>> +
>>> + break;
>>> + default:
>>> + *str = "Shared Fan";
>>> + break;
>>> + }
>>> +
>>> + break;
>>> + default:
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct hwmon_ops awcc_hwmon_ops = {
>>> + .is_visible = awcc_hwmon_is_visible,
>>> + .read = awcc_hwmon_read,
>>> + .read_string = awcc_hwmon_read_string,
>>> +};
>>> +
>>> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>>> + HWMON_CHANNEL_INFO(temp,
>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>> + HWMON_T_LABEL | HWMON_T_INPUT
>>> + ),
>>> + HWMON_CHANNEL_INFO(fan,
>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
>>> + ),
>>> + HWMON_CHANNEL_INFO(pwm,
>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> + HWMON_PWM_AUTO_CHANNELS_TEMP
>>> + ),
>> Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info
>> would make sense.
> IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead
> of doing that? I might be wrong tho.
>
> I'm fine either way.
>
If Guenter is fine with your current approach then you can keep it.
>>> + NULL
>>> +};
>>> +
>>> +static const struct hwmon_chip_info awcc_hwmon_chip_info = {
>>> + .ops = &awcc_hwmon_ops,
>>> + .info = awcc_hwmon_info,
>>> +};
>>> +
>>> +static int awcc_hwmon_temps_init(struct wmi_device *wdev)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>>> + unsigned long temp_sensors[BITS_TO_LONGS(U8_MAX)];
>>> + unsigned int i, max_sensor_id = 0;
>>> + int ret;
>>> + u32 id;
>>> +
>>> + for (i = 0; i < priv->temp_count; i++) {
>>> + /*
>>> + * Temperature sensors IDs are listed after the fan IDs at
>>> + * offset `fan_count`
>>> + */
>>> + ret = awcc_op_get_resource_id(wdev, i + priv->fan_count, &id);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
>>> + if (id > max_sensor_id)
>>> + max_sensor_id = id;
>>> +
>>> + __set_bit(id, temp_sensors);
>>> + }
>>> +
>>> + /*
>>> + * We prefer to allocate the bitmap dynamically because usually temp IDs
>>> + * are small (< 0x30) and only one UL is needed to store it, but there
>>> + * may be unknown devices that break this rule
>>> + */
>> Hi,
>>
>> as far as i know the memory allocator inside the kernel at least allocates 32 bytes, so you are
> bytes? bits maybe?
I am sure it is bytes.
>> not saving any memory with this. I suggest you allocate the bitmaps statically.
> The thing is - We don't know before hand how big an ID can be.
>
> Technically the upper limit is U8_MAX which would require 4 ULs to
> store, which is a lot. However I haven't seen temp IDs bigger than 0x6,
> so this way only one UL is allocated for most devices.
>
> I would be very grateful if Dell could help us on this one :')
>
I think wasting 3 * 8 = 24 bytes is OK, especially if you allocate at least 32 bytes
for a dynamic bitmap either way.
Thanks,
Armin Wolf
>>> + priv->temp_sensors_size = max_sensor_id + 1;
>>> + priv->temp_sensors = devm_bitmap_zalloc(&wdev->dev, priv->temp_sensors_size,
>>> + GFP_KERNEL);
>>> + if (!priv->temp_sensors)
>>> + return -ENOMEM;
>>> +
>>> + bitmap_copy(priv->temp_sensors, temp_sensors, priv->temp_sensors_size);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int awcc_hwmon_fans_init(struct wmi_device *wdev)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>>> + u32 id, min_rpm, max_rpm, temp_count, temp_id;
>>> + unsigned long gather[BITS_TO_LONGS(U8_MAX)];
>>> + struct awcc_fan_data *fan_data;
>>> + unsigned int i, j;
>>> + int ret;
>>> +
>>> + for (i = 0; i < priv->fan_count; i++) {
>>> + fan_data = devm_kzalloc(&wdev->dev, sizeof(*fan_data), GFP_KERNEL);
>>> + if (!fan_data)
>>> + return -ENOMEM;
>>> +
>>> + fan_data->related_temps = devm_bitmap_zalloc(&wdev->dev,
>>> + priv->temp_sensors_size,
>>> + GFP_KERNEL);
>> Same as above
>>
>>> + if (!priv->temp_sensors)
>>> + return -ENOMEM;
>>> +
>>> + fan_data->auto_channels_temp = devm_bitmap_zalloc(&wdev->dev,
>>> + priv->temp_count,
>>> + GFP_KERNEL);
>> We already know that we only ever use the first sizeof(long) bytes from this bitmap,
>> please do a static allocation here.
> Right, ofc! I forgot about the limit I imposed.
>
>>> + if (!priv->temp_sensors)
>>> + return -ENOMEM;
>>> +
>>> + /*
>>> + * Fan IDs are listed first at offset 0
>>> + */
>>> + ret = awcc_op_get_resource_id(wdev, i, &id);
>>> + if (ret)
>>> + return ret;
>>> + id = FIELD_GET(AWCC_RESOURCE_ID_MASK, id);
>>> +
>>> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MIN_RPM, id,
>>> + &min_rpm);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = awcc_thermal_information(wdev, AWCC_OP_GET_FAN_MAX_RPM, id,
>>> + &max_rpm);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_TOTAL_FAN_TEMPS, id,
>>> + 0, &temp_count);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + for (j = 0; j < temp_count; j++) {
>>> + ret = awcc_get_fan_sensors(wdev, AWCC_OP_GET_FAN_TEMP_ID,
>>> + id, j, &temp_id);
>>> + if (ret)
>>> + break;
>>> +
>>> + temp_id = FIELD_GET(AWCC_RESOURCE_ID_MASK, temp_id);
>>> + if (temp_id < priv->temp_sensors_size)
>>> + __set_bit(temp_id, fan_data->related_temps);
>>> + }
>>> +
>>> + fan_data->id = id;
>>> + fan_data->min_rpm = min_rpm;
>>> + fan_data->max_rpm = max_rpm;
>>> + fan_data->total_temps = bitmap_weight(fan_data->related_temps,
>>> + priv->temp_sensors_size);
>>> + bitmap_gather(gather, fan_data->related_temps, priv->temp_sensors,
>>> + priv->temp_sensors_size);
>> Since fan_data->related_temps is only used for determining the fan label after this it would
>> make sense to determine the fan label here and turn fan_data->related_temps into a local
>> variable on the stack.
> Ack.
>
>> Thanks,
>> Armin Wolf
> Thank you very much! Very helpful feedback :)
>
>>> + bitmap_copy(fan_data->auto_channels_temp, gather, priv->temp_count);
>>> + priv->fan_data[i] = fan_data;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int awcc_hwmon_init(struct wmi_device *wdev)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>>> + int ret;
>>> +
>>> + priv->fan_data = devm_kcalloc(&wdev->dev, priv->fan_count,
>>> + sizeof(*priv->fan_data), GFP_KERNEL);
>>> + if (!priv->fan_data)
>>> + return -ENOMEM;
>>> +
>>> + ret = awcc_hwmon_temps_init(wdev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = awcc_hwmon_fans_init(wdev);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
>>> + &awcc_hwmon_chip_info, NULL);
>>> +
>>> + return PTR_ERR_OR_ZERO(priv->hwdev);
>>> +}
>>> +
>>> /*
>>> * Thermal Profile control
>>> * - Provides thermal profile control through the Platform Profile API
>>> @@ -753,6 +1171,12 @@ static int alienware_awcc_setup(struct wmi_device *wdev)
>>> priv->wdev = wdev;
>>> dev_set_drvdata(&wdev->dev, priv);
>>>
>>> + if (awcc->hwmon) {
>>> + ret = awcc_hwmon_init(wdev);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> if (awcc->pprof) {
>>> ret = awcc_platform_profile_init(wdev);
>>> if (ret)
>>> @@ -833,6 +1257,13 @@ int __init alienware_wmax_wmi_init(void)
>>> if (id)
>>> awcc = id->driver_data;
>>>
>>> + if (force_hwmon) {
>>> + if (!awcc)
>>> + awcc = &empty_quirks;
>>> +
>>> + awcc->hwmon = true;
>>> + }
>>> +
>>> if (force_platform_profile) {
>>> if (!awcc)
>>> awcc = &empty_quirks;
>>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control
2025-03-07 0:16 ` Kurt Borja
@ 2025-03-07 21:18 ` Armin Wolf
0 siblings, 0 replies; 24+ messages in thread
From: Armin Wolf @ 2025-03-07 21:18 UTC (permalink / raw)
To: Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Guenter Roeck, Jean Delvare, linux-hwmon
Am 07.03.25 um 01:16 schrieb Kurt Borja:
> Hi Armin,
>
> On Thu Mar 6, 2025 at 5:35 PM -05, Armin Wolf wrote:
>> Am 06.03.25 um 01:56 schrieb Kurt Borja:
>>
>>> All models with the "AWCC" WMAX device support a way of manually
>>> controlling fans.
>>>
>>> The PWM duty cycle of a fan can't be controlled directly. Instead the
>>> AWCC interface let's us tune a PWM `boost` value, which has the
>>> following empirically discovered, aproximate behavior over the PWM
>>> value:
>>>
>>> pwm = pwm_base + (pwm_boost / 255) * (pwm_max - pwm_base)
>>>
>>> Where the pwm_base is the locked PWM value controlled by the FW and
>>> pwm_boost is a value between 0 and 255.
>>>
>>> Expose this pwm_boost knob as a custom HWMON attribute.
>>>
>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>> Cc: Jean Delvare <jdelvare@suse.com>
>>> Cc: linux-hwmon@vger.kernel.org
>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>>> ---
>>> drivers/platform/x86/dell/alienware-wmi-wmax.c | 223 ++++++++++++++++++++++++-
>>> 1 file changed, 220 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>>> index 20cf3371ee3c0e1ea038b3ca517e831f3b30dc29..de4e8f177aadc9552b05cc732e41ee458b761143 100644
>>> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
>>> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
>>> @@ -13,8 +13,11 @@
>>> #include <linux/bits.h>
>>> #include <linux/dmi.h>
>>> #include <linux/hwmon.h>
>>> +#include <linux/hwmon-sysfs.h>
>>> +#include <linux/minmax.h>
>>> #include <linux/moduleparam.h>
>>> #include <linux/platform_profile.h>
>>> +#include <linux/pm.h>
>>> #include <linux/units.h>
>>> #include <linux/wmi.h>
>>> #include "alienware-wmi.h"
>>> @@ -179,10 +182,12 @@ enum AWCC_THERMAL_INFORMATION_OPERATIONS {
>>> AWCC_OP_GET_FAN_MIN_RPM = 0x08,
>>> AWCC_OP_GET_FAN_MAX_RPM = 0x09,
>>> AWCC_OP_GET_CURRENT_PROFILE = 0x0B,
>>> + AWCC_OP_GET_FAN_BOOST = 0x0C,
>>> };
>>>
>>> enum AWCC_THERMAL_CONTROL_OPERATIONS {
>>> AWCC_OP_ACTIVATE_PROFILE = 0x01,
>>> + AWCC_OP_SET_FAN_BOOST = 0x02,
>>> };
>>>
>>> enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
>>> @@ -248,6 +253,7 @@ struct awcc_fan_data {
>>> u32 total_temps;
>>> u32 min_rpm;
>>> u32 max_rpm;
>>> + u8 suspend_cache;
>>> u8 id;
>>> };
>>>
>>> @@ -627,6 +633,17 @@ static inline int awcc_op_get_temperature(struct wmi_device *wdev, u8 temp_id, u
>>> .arg3 = 0,
>>> };
>>>
>>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>>> +}
>>> +
>>> +static inline int awcc_op_get_fan_boost(struct wmi_device *wdev, u8 fan_id, u32 *out)
>>> +{
>>> + struct wmax_u32_args args = {
>>> + .operation = AWCC_OP_GET_FAN_BOOST,
>>> + .arg1 = fan_id,
>>> + .arg2 = 0,
>>> + .arg3 = 0,
>>> + };
>>>
>>> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>>> }
>>> @@ -656,6 +673,19 @@ static inline int awcc_op_activate_profile(struct wmi_device *wdev, u8 profile)
>>> return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
>>> }
>>>
>>> +static int awcc_op_set_fan_boost(struct wmi_device *wdev, u8 fan_id, u8 boost)
>>> +{
>>> + struct wmax_u32_args args = {
>>> + .operation = AWCC_OP_SET_FAN_BOOST,
>>> + .arg1 = fan_id,
>>> + .arg2 = boost,
>>> + .arg3 = 0,
>>> + };
>>> + u32 out;
>>> +
>>> + return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_CONTROL, &args, &out);
>>> +}
>>> +
>>> static int awcc_profile_id_to_pprof(u32 id, enum platform_profile_option *profile)
>>> {
>>> switch (id) {
>>> @@ -717,6 +747,7 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>> u32 attr, int channel, long *val)
>>> {
>>> struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + enum platform_profile_option profile;
>>> struct awcc_fan_data *fan;
>>> u32 state;
>>> int ret;
>>> @@ -765,6 +796,28 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>> fan = priv->fan_data[channel];
>>>
>>> switch (attr) {
>>> + case hwmon_pwm_enable:
>>> + ret = awcc_op_get_current_profile(priv->wdev, &state);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = awcc_profile_id_to_pprof(state, &profile);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + switch (profile) {
>>> + case PLATFORM_PROFILE_PERFORMANCE:
>> The hwmon sysfs docs say that 0 means that the fan is spinning at maximum speed. Does PLATFORM_PROFILE_PERFORMANCE
>> guarantee that all fans are always spinning at maximum speed?
> Yes PERFORMANCE is full-speed for all devices I know. Manual fan control
> is completely disabled for that profile too.
>
> In fact I'm thinking about adding a module parameter to suppress this
> behavior. Not everyone may like that. That's outside the scope of these
> series tho.
I see no problem in exposing PLATFORM_PROFILE_PERFORMANCE as a normal platform profile,
if a user does not want to use it then they can simply not select it.
>> If no then i suggest to drop support for 0.
>>
>>> + *val = 0;
>>> + break;
>>> + case PLATFORM_PROFILE_CUSTOM:
>>> + *val = 1;
>>> + break;
>>> + default:
>>> + *val = 2;
>>> + break;
>>> + }
>>> +
>>> + break;
>>> case hwmon_pwm_auto_channels_temp:
>>> bitmap_copy(val, fan->auto_channels_temp, BITS_PER_LONG);
>>> break;
>>> @@ -840,10 +893,48 @@ static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types ty
>>> return 0;
>>> }
>>>
>>> +
>>> +static int awcc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>>> + u32 attr, int channel, long val)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + int ret;
>>> +
>>> + switch (type) {
>>> + case hwmon_pwm:
>>> + switch (attr) {
>>> + case hwmon_pwm_enable:
>>> + /*
>>> + * We don't want to duplicate platform profile logic, so
>>> + * we only allow enabling manual fan control
>>> + */
>> I do not think that having pwm1_enable brings any benefit, as the pwmX_boost attributes
>> behave differently than pwmX attributes. I think it would be enough to document that
>> pwmX_boost settings will only reliably work when the custom platform profile is selected.
> Now I realise I completely forgot about the admin-guide documentation!
> I'll include it in the next revision. Is this path ok?
>
> Documentation/admin-guide/laptops/alienware-wmi.rst
>
> Or should I add driver specific ABI documentation? (or both ofc)
>
> I don't want to name the file alienware-laptop because this driver is
> compatible with Dell G-Series too.
The name "alienware-wmi.rst" is OK. I suggest that you create driver specific ABI documentation for
the boost attributes and reference them inside the laptop documentation.
>>> + if (val != 1)
>>> + return -EINVAL;
>>> +
>>> + ret = awcc_op_activate_profile(priv->wdev, AWCC_SPECIAL_PROFILE_CUSTOM);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (priv->ppdev)
>>> + platform_profile_notify(priv->ppdev);
>>> + break;
>>> + default:
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + break;
>>> + default:
>>> + return -EOPNOTSUPP;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static const struct hwmon_ops awcc_hwmon_ops = {
>>> .is_visible = awcc_hwmon_is_visible,
>>> .read = awcc_hwmon_read,
>>> .read_string = awcc_hwmon_read_string,
>>> + .write = awcc_hwmon_write,
>>> };
>>>
>>> static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>>> @@ -864,7 +955,7 @@ static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>>> HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
>>> ),
>>> HWMON_CHANNEL_INFO(pwm,
>>> - HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> + HWMON_PWM_AUTO_CHANNELS_TEMP | HWMON_PWM_ENABLE,
>>> HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> HWMON_PWM_AUTO_CHANNELS_TEMP,
>>> @@ -879,6 +970,75 @@ static const struct hwmon_chip_info awcc_hwmon_chip_info = {
>>> .info = awcc_hwmon_info,
>>> };
>>>
>>> +static ssize_t pwm_boost_show(struct device *dev, struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + int ret, index = to_sensor_dev_attr(attr)->index;
>> Please initialize "index" on a separate line, can remember the reverse xmas-tree order for variables.
> Ack.
>
>>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + struct awcc_fan_data *fan = priv->fan_data[index];
>>> + u32 boost;
>>> +
>>> + ret = awcc_op_get_fan_boost(priv->wdev, fan->id, &boost);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return sysfs_emit(buf, "%u\n", boost);
>>> +}
>>> +
>>> +static ssize_t pwm_boost_store(struct device *dev, struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + int ret, index = to_sensor_dev_attr(attr)->index;
>>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + struct awcc_fan_data *fan = priv->fan_data[index];
>>> + unsigned long val;
>>> +
>>> + ret = kstrtoul(buf, 0, &val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = awcc_op_set_fan_boost(priv->wdev, fan->id, clamp_val(val, 0, 255));
>>> +
>>> + return ret ? ret : count;
>>> +}
>>> +
>>> +static SENSOR_DEVICE_ATTR_RW(pwm1_boost, pwm_boost, 0);
>>> +static SENSOR_DEVICE_ATTR_RW(pwm2_boost, pwm_boost, 1);
>>> +static SENSOR_DEVICE_ATTR_RW(pwm3_boost, pwm_boost, 2);
>>> +static SENSOR_DEVICE_ATTR_RW(pwm4_boost, pwm_boost, 3);
>> Since those attributes are working differently than the standard pwm attributes, i suggest to
>> instead name them fanX_boost.
> I went for pwm*_boost because we also export pwm*_auto_channels_temp,
> but I'm ok with fan*_boost too.
>
Having pwmX_auto_channels_temp as the only pwm attribute is fine, the fsteutates driver does
something similar. Naming the attributes fanX_boost would better fir inside the fan-centric
hwmon API of this driver.
>>> +
>>> +static umode_t pwm_boost_attr_visible(struct kobject *kobj, struct attribute *attr, int n)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(kobj_to_dev(kobj));
>>> +
>>> + return n < priv->fan_count ? attr->mode : 0;
>>> +}
>>> +
>>> +static bool pwm_boost_group_visible(struct kobject *kobj)
>>> +{
>>> + return true;
>>> +}
>>> +
>>> +DEFINE_SYSFS_GROUP_VISIBLE(pwm_boost);
>>> +
>>> +static struct attribute *fan_boost_attrs[] = {
>>> + &sensor_dev_attr_pwm1_boost.dev_attr.attr,
>>> + &sensor_dev_attr_pwm2_boost.dev_attr.attr,
>>> + &sensor_dev_attr_pwm3_boost.dev_attr.attr,
>>> + &sensor_dev_attr_pwm4_boost.dev_attr.attr,
>>> + NULL
>>> +};
>>> +
>>> +static const struct attribute_group pwm_boost_group = {
>>> + .attrs = fan_boost_attrs,
>>> + .is_visible = SYSFS_GROUP_VISIBLE(pwm_boost),
>>> +};
>>> +
>>> +static const struct attribute_group *awcc_hwmon_groups[] = {
>>> + &pwm_boost_group,
>>> + NULL
>>> +};
>>> +
>>> static int awcc_hwmon_temps_init(struct wmi_device *wdev)
>>> {
>>> struct awcc_priv *priv = dev_get_drvdata(&wdev->dev);
>>> @@ -1011,12 +1171,50 @@ static int awcc_hwmon_init(struct wmi_device *wdev)
>>> if (ret)
>>> return ret;
>>>
>>> - priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi", priv,
>>> - &awcc_hwmon_chip_info, NULL);
>>> + priv->hwdev = devm_hwmon_device_register_with_info(&wdev->dev, "alienware_wmi",
>>> + priv, &awcc_hwmon_chip_info,
>>> + awcc_hwmon_groups);
>>>
>>> return PTR_ERR_OR_ZERO(priv->hwdev);
>>> }
>>>
>>> +static void awcc_hwmon_suspend(struct device *dev)
>>> +{
>>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + struct awcc_fan_data *fan;
>>> + unsigned int i;
>>> + u32 boost;
>>> + int ret;
>>> +
>>> + for (i = 0; i < priv->fan_count; i++) {
>>> + fan = priv->fan_data[i];
>>> +
>>> + ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_FAN_BOOST,
>>> + fan->id, &boost);
>>> + if (ret)
>>> + fan->suspend_cache = 0;
>> Please at least log a warning here that the fan boost value can not be restored properly.
> Ack.
>
> Is not propagating errors a good approach here? My idea was to try to
> turn off fans no matter what.
In this case it should be OK, failing to restore the fan boost value is not critical.
>>> + else
>>> + fan->suspend_cache = clamp_val(boost, 0, 255);
>>> +
>>> + awcc_op_set_fan_boost(priv->wdev, fan->id, 0);
>>> + }
>>> +}
>>> +
>>> +static void awcc_hwmon_resume(struct device *dev)
>>> +{
>>> +
>>> + struct awcc_priv *priv = dev_get_drvdata(dev);
>>> + struct awcc_fan_data *fan;
>>> + unsigned int i;
>>> +
>>> + for (i = 0; i < priv->fan_count; i++) {
>>> + fan = priv->fan_data[i];
>>> +
>>> + if (fan->suspend_cache)
>> How does the driver restore fan boost settings with a value of 0?
> We set to 0 when suspending so I don't think it's necessary to restore
> to 0 again when resuming.
>
I understand, in this case everything should be fine.
Thanks,
Armin Wolf
>> Thanks,
>> Armin Wolf
>>
>>> + awcc_op_set_fan_boost(priv->wdev, fan->id, fan->suspend_cache);
>>> + }
>>> +}
>>> +
>>> /*
>>> * Thermal Profile control
>>> * - Provides thermal profile control through the Platform Profile API
>>> @@ -1233,6 +1431,24 @@ static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
>>> return ret;
>>> }
>>>
>>> +static int wmax_wmi_suspend(struct device *dev)
>>> +{
>>> + if (awcc->hwmon)
>>> + awcc_hwmon_suspend(dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int wmax_wmi_resume(struct device *dev)
>>> +{
>>> + if (awcc->hwmon)
>>> + awcc_hwmon_resume(dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +DEFINE_SIMPLE_DEV_PM_OPS(wmax_wmi_pm_ops, wmax_wmi_suspend, wmax_wmi_resume);
>>> +
>>> static const struct wmi_device_id alienware_wmax_device_id_table[] = {
>>> { WMAX_CONTROL_GUID, NULL },
>>> { },
>>> @@ -1243,6 +1459,7 @@ static struct wmi_driver alienware_wmax_wmi_driver = {
>>> .driver = {
>>> .name = "alienware-wmi-wmax",
>>> .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>>> + .pm = pm_sleep_ptr(&wmax_wmi_pm_ops),
>>> },
>>> .id_table = alienware_wmax_device_id_table,
>>> .probe = wmax_wmi_probe,
>>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support
2025-03-07 21:09 ` Armin Wolf
@ 2025-03-07 23:59 ` Guenter Roeck
2025-03-08 4:54 ` Kurt Borja
0 siblings, 1 reply; 24+ messages in thread
From: Guenter Roeck @ 2025-03-07 23:59 UTC (permalink / raw)
To: Armin Wolf, Kurt Borja, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Jean Delvare, linux-hwmon
On 3/7/25 13:09, Armin Wolf wrote:
> Am 07.03.25 um 01:35 schrieb Kurt Borja:
...
>>>> +static const struct hwmon_ops awcc_hwmon_ops = {
>>>> + .is_visible = awcc_hwmon_is_visible,
>>>> + .read = awcc_hwmon_read,
>>>> + .read_string = awcc_hwmon_read_string,
>>>> +};
>>>> +
>>>> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>>>> + HWMON_CHANNEL_INFO(temp,
>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>> + HWMON_T_LABEL | HWMON_T_INPUT
>>>> + ),
>>>> + HWMON_CHANNEL_INFO(fan,
>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
>>>> + ),
>>>> + HWMON_CHANNEL_INFO(pwm,
>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP
>>>> + ),
>>> Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info
>>> would make sense.
>> IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead
>> of doing that? I might be wrong tho.
>>
>> I'm fine either way.
>>
> If Guenter is fine with your current approach then you can keep it.
>
In drivers/hwmon, I prefer static descriptions such as the above and using
the is_visible() function to determine if sensor attributes should actually
be created. However, as I have mentioned several times, I do not comment on style
questions like this (or, for that matter, non-standard sysfs attributes) outside
drivers/hwmon, so you can do or request whatever you like.
Guenter
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support
2025-03-07 23:59 ` Guenter Roeck
@ 2025-03-08 4:54 ` Kurt Borja
0 siblings, 0 replies; 24+ messages in thread
From: Kurt Borja @ 2025-03-08 4:54 UTC (permalink / raw)
To: Guenter Roeck, Armin Wolf, Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Dell.Client.Kernel,
linux-kernel, Jean Delvare, linux-hwmon
On Fri Mar 7, 2025 at 6:59 PM -05, Guenter Roeck wrote:
> On 3/7/25 13:09, Armin Wolf wrote:
>> Am 07.03.25 um 01:35 schrieb Kurt Borja:
> ...
>>>>> +static const struct hwmon_ops awcc_hwmon_ops = {
>>>>> + .is_visible = awcc_hwmon_is_visible,
>>>>> + .read = awcc_hwmon_read,
>>>>> + .read_string = awcc_hwmon_read_string,
>>>>> +};
>>>>> +
>>>>> +static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>>>>> + HWMON_CHANNEL_INFO(temp,
>>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>>> + HWMON_T_LABEL | HWMON_T_INPUT,
>>>>> + HWMON_T_LABEL | HWMON_T_INPUT
>>>>> + ),
>>>>> + HWMON_CHANNEL_INFO(fan,
>>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>>>>> + HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
>>>>> + ),
>>>>> + HWMON_CHANNEL_INFO(pwm,
>>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP,
>>>>> + HWMON_PWM_AUTO_CHANNELS_TEMP
>>>>> + ),
>>>> Since the number of fans and temperature sensors is only known at runtime creating awcc_hwmon_info
>>>> would make sense.
>>> IIRC Guenter asked another dev to add more CHANNEL_INFO entries instead
>>> of doing that? I might be wrong tho.
>>>
>>> I'm fine either way.
>>>
>> If Guenter is fine with your current approach then you can keep it.
>>
>
> In drivers/hwmon, I prefer static descriptions such as the above and using
> the is_visible() function to determine if sensor attributes should actually
> be created. However, as I have mentioned several times, I do not comment on style
> questions like this (or, for that matter, non-standard sysfs attributes) outside
> drivers/hwmon, so you can do or request whatever you like.
>
> Guenter
Thank you for clarifying!
If there are no objections, then I prefer to keep it this way. With the
few extra HWMON_CHANNEL_INFO entries.
--
~ Kurt
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2025-03-08 4:55 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-06 0:56 [PATCH v3 00/10] platform/x86: alienware-wmi-wmax: HWMON support + DebugFS + Improvements Kurt Borja
2025-03-06 0:56 ` [PATCH v3 01/10] platform/x86: alienware-wmi-wmax: Rename thermal related symbols Kurt Borja
2025-03-06 0:56 ` [PATCH v3 02/10] platform/x86: alienware-wmi-wmax: Refactor is_awcc_thermal_mode() Kurt Borja
2025-03-06 0:56 ` [PATCH v3 03/10] platform/x86: alienware-wmi-wmax: Improve internal AWCC API Kurt Borja
2025-03-06 21:27 ` Armin Wolf
2025-03-06 0:56 ` [PATCH v3 04/10] platform/x86: alienware-wmi-wmax: Modify supported_thermal_profiles[] Kurt Borja
2025-03-06 0:56 ` [PATCH v3 05/10] platform/x86: alienware-wmi-wmax: Improve platform profile probe Kurt Borja
2025-03-06 0:56 ` [PATCH v3 06/10] platform/x86: alienware-wmi-wmax: Add support for the "custom" thermal profile Kurt Borja
2025-03-06 0:56 ` [PATCH v3 07/10] platform/x86: alienware-wmi-wmax: Add HWMON support Kurt Borja
2025-03-06 22:19 ` Armin Wolf
2025-03-07 0:35 ` Kurt Borja
2025-03-07 21:09 ` Armin Wolf
2025-03-07 23:59 ` Guenter Roeck
2025-03-08 4:54 ` Kurt Borja
2025-03-06 0:56 ` [PATCH v3 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control Kurt Borja
2025-03-06 22:35 ` Armin Wolf
2025-03-07 0:16 ` Kurt Borja
2025-03-07 21:18 ` Armin Wolf
2025-03-06 0:57 ` [PATCH v3 09/10] platform/x86: alienware-wmi-wmax: Add a DebugFS interface Kurt Borja
2025-03-06 22:38 ` Armin Wolf
2025-03-06 0:57 ` [PATCH v3 10/10] Documentation: wmi: Improve and update alienware-wmi documentation Kurt Borja
2025-03-06 23:02 ` Armin Wolf
2025-03-06 23:57 ` Bagas Sanjaya
2025-03-07 0:39 ` Kurt Borja
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox