On Sun, 24 Nov 2024, Armin Wolf wrote:
> After looking at the ACPI AML code, it seems that the command 0x0000
> used with ACER_WMID_GET_GAMING_SYS_INFO_METHODID returns a bitmap of
> all supported sensors available through the 0x0001 command.
>
> Add support for this command and the additional temperature sensors.
I had to read quite a bit of code before understanding how this changed
what was a few hard-coded sensor IDs embedded inside the old wmi commands
into a more generalized approach using what turned out to be a field of
sensors. Perhaps you could explain more here.
> This also fixed detection of fan sensors should the fan not spin
> during device initialization.
This too comes out of the woods because the old way of doing things is
entirely overlooked by this commit message.
> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
> ---
> drivers/platform/x86/acer-wmi.c | 129 ++++++++++++++++++++++++--------
> 1 file changed, 96 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index dd57787466b9..7b549920eba7 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -30,6 +30,7 @@
> #include <linux/input/sparse-keymap.h>
> #include <acpi/video.h>
> #include <linux/hwmon.h>
> +#include <linux/units.h>
> #include <linux/bitfield.h>
>
> MODULE_AUTHOR("Carlos Corbacho");
> @@ -70,7 +71,8 @@ MODULE_LICENSE("GPL");
>
> #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
>
> -#define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)
> +#define ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK GENMASK(24, 8)
> +#define ACER_PREDATOR_V4_SUPPORTED_SENSORS_BIT_MASK GENMASK(40, 24)
>
> /*
> * Acer ACPI method GUIDs
> @@ -98,9 +100,17 @@ enum acer_wmi_event_ids {
> };
>
> enum acer_wmi_predator_v4_sys_info_command {
> - ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS = 0x02,
> - ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED = 0x0201,
> - ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED = 0x0601,
> + ACER_WMID_CMD_GET_PREDATOR_V4_SUPPORTED_SENSORS = 0x0000,
> + ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING = 0x0001,
> + ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS = 0x0002,
> +};
> +
> +enum acer_wmi_predator_v4_sensor_id {
> + ACER_WMID_SENSOR_CPU_TEMPERATURE = 0x01,
> + ACER_WMID_SENSOR_CPU_FAN_SPEED = 0x02,
> + ACER_WMID_SENSOR_EXTERNAL_TEMPERATURE_2 = 0x03,
> + ACER_WMID_SENSOR_GPU_FAN_SPEED = 0x06,
> + ACER_WMID_SENSOR_GPU_TEMPERATURE = 0x0A,
Please align values in both enums.
> };
>
> static const struct key_entry acer_wmi_keymap[] __initconst = {
> @@ -271,6 +281,7 @@ static u16 commun_func_bitmap;
> static u8 commun_fn_key_number;
> static bool cycle_gaming_thermal_profile = true;
> static bool predator_v4;
> +static u64 supported_sensors;
>
> module_param(mailled, int, 0444);
> module_param(brightness, int, 0444);
> @@ -1513,6 +1524,24 @@ static acpi_status WMID_gaming_get_u64(u64 *value, u32 cap)
> return status;
> }
>
> +static int WMID_gaming_get_sys_info(u32 command, u64 *out)
> +{
> + acpi_status status;
> + u64 result;
> +
> + status = WMI_gaming_execute_u64(ACER_WMID_GET_GAMING_SYS_INFO_METHODID, command, &result);
> + if (ACPI_FAILURE(status))
> + return -EIO;
> +
> + /* The lower 8 bits must be zero for the operation to have succeeded */
> + if (result & 0xff)
Could we name this field and use FIELD_GET()?
> + return -EIO;
> +
> + *out = result;
> +
> + return 0;
> +}
> +
> static void WMID_gaming_set_fan_mode(u8 fan_mode)
> {
> /* fan_mode = 1 is used for auto, fan_mode = 2 used for turbo*/
> @@ -1760,26 +1789,6 @@ static int acer_gsensor_event(void)
> return 0;
> }
>
> -static int acer_get_fan_speed(int fan)
> -{
> - if (quirks->predator_v4) {
> - acpi_status status;
> - u64 fanspeed;
> -
> - status = WMI_gaming_execute_u64(
> - ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
> - fan == 0 ? ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED :
> - ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED,
> - &fanspeed);
> -
> - if (ACPI_FAILURE(status))
> - return -EIO;
> -
> - return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
> - }
> - return -EOPNOTSUPP;
> -}
> -
> /*
> * Predator series turbo button
> */
> @@ -2671,43 +2680,86 @@ static void __init create_debugfs(void)
> &interface->debug.wmid_devices);
> }
>
> +static const enum acer_wmi_predator_v4_sensor_id acer_wmi_temp_channel_to_sensor_id[] = {
> + [0] = ACER_WMID_SENSOR_CPU_TEMPERATURE,
> + [1] = ACER_WMID_SENSOR_GPU_TEMPERATURE,
> + [2] = ACER_WMID_SENSOR_EXTERNAL_TEMPERATURE_2,
> +};
> +
> +static const enum acer_wmi_predator_v4_sensor_id acer_wmi_fan_channel_to_sensor_id[] = {
> + [0] = ACER_WMID_SENSOR_CPU_FAN_SPEED,
> + [1] = ACER_WMID_SENSOR_GPU_FAN_SPEED,
> +};
> +
> static umode_t acer_wmi_hwmon_is_visible(const void *data,
> enum hwmon_sensor_types type, u32 attr,
> int channel)
> {
> + enum acer_wmi_predator_v4_sensor_id sensor_id;
> + const u64 *supported_sensors = data;
> +
> switch (type) {
> + case hwmon_temp:
> + sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
> + break;
> case hwmon_fan:
> - if (acer_get_fan_speed(channel) >= 0)
> - return 0444;
> + sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
> break;
> default:
> return 0;
> }
>
> + if (*supported_sensors & BIT(sensor_id - 1))
> + return 0444;
> +
> return 0;
> }
>
> static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, long *val)
> {
> + enum acer_wmi_predator_v4_sensor_id sensor_id;
> + u64 command, result;
> int ret;
>
> switch (type) {
> + case hwmon_temp:
> + sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
> + command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING | (sensor_id << 8);
Please use FIELD_PREP() and a named define.
> +
> + ret = WMID_gaming_get_sys_info(command, &result);
> + if (ret < 0)
> + return ret;
> +
> + result = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
> + *val = result * MILLIDEGREE_PER_DEGREE;
> + return 0;
> case hwmon_fan:
> - ret = acer_get_fan_speed(channel);
> + sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
> + command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING | (sensor_id << 8);
Ditto.
--
i.
> +
> + ret = WMID_gaming_get_sys_info(command, &result);
> if (ret < 0)
> return ret;
> - *val = ret;
> - break;
> +
> + *val = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
> + return 0;
> default:
> return -EOPNOTSUPP;
> }
> -
> - return 0;
> }
>
> static const struct hwmon_channel_info *const acer_wmi_hwmon_info[] = {
> - HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT), NULL
> + HWMON_CHANNEL_INFO(temp,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT,
> + HWMON_T_INPUT
> + ),
> + HWMON_CHANNEL_INFO(fan,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT
> + ),
> + NULL
> };
>
> static const struct hwmon_ops acer_wmi_hwmon_ops = {
> @@ -2724,9 +2776,20 @@ static int acer_wmi_hwmon_init(void)
> {
> struct device *dev = &acer_platform_device->dev;
> struct device *hwmon;
> + u64 result;
> + int ret;
> +
> + ret = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_SUPPORTED_SENSORS, &result);
> + if (ret < 0)
> + return ret;
> +
> + /* Return early if no sensors are available */
> + supported_sensors = FIELD_GET(ACER_PREDATOR_V4_SUPPORTED_SENSORS_BIT_MASK, result);
> + if (!supported_sensors)
> + return 0;
>
> hwmon = devm_hwmon_device_register_with_info(dev, "acer",
> - &acer_platform_driver,
> + &supported_sensors,
> &acer_wmi_hwmon_chip_info,
> NULL);
>
> --
> 2.39.5
>
