[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <21edd314-b168-4766-9654-c7ea08d7ac4b@gmx.de>
Date: Mon, 9 Dec 2024 03:44:01 +0100
From: Armin Wolf <W_Armin@....de>
To: Kurt Borja <kuurtb@...il.com>
Cc: jlee@...e.com, farhan.anwar8@...il.com, rayanmargham4@...il.com,
hdegoede@...hat.com, ilpo.jarvinen@...ux.intel.com,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/5] platform/x86: acer-wmi: Implement proper hwmon
support
Am 08.12.24 um 21:20 schrieb Kurt Borja:
> On Fri, Nov 29, 2024 at 08:33:58PM +0100, 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 sensor indices available through the 0x0001 command.
>>
>> Those sensor indices seem to include both temperature and fan speed
>> sensors, with only the fan speed sensors being currently supported.
>>
>> Use the output of this new command to implement reliable sensor
>> detection. This fixes detection of fans which do not spin during
>> probe, as fans are currently being ignored if their speed is 0.
>>
>> Also add support for the new temperature sensor ids.
>>
>> Signed-off-by: Armin Wolf <W_Armin@....de>
>> ---
>> drivers/platform/x86/acer-wmi.c | 114 ++++++++++++++++++++++----------
>> 1 file changed, 80 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>> index ac4500f33b8c..2c1ea6155bd3 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");
>> @@ -71,7 +72,10 @@ MODULE_LICENSE("GPL");
>> #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
>>
>> #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
> Hi Armin!
>
> This macro is defined twice.
Good catch, i will send a v4 patch series to correct this.
Thanks,
Armin Wolf
>
>> -#define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)
>> +#define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
> Here ^.
>
> ~ Kurt
>
>> +#define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
>> +#define ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK GENMASK_ULL(23, 8)
>> +#define ACER_PREDATOR_V4_SUPPORTED_SENSORS_BIT_MASK GENMASK_ULL(39, 24)
>>
>> /*
>> * Acer ACPI method GUIDs
>> @@ -99,9 +103,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,
>> };
>>
>> static const struct key_entry acer_wmi_keymap[] __initconst = {
>> @@ -272,6 +284,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);
>> @@ -1779,27 +1792,6 @@ static int acer_gsensor_event(void)
>> return 0;
>> }
>>
>> -static int acer_get_fan_speed(int fan)
>> -{
>> - u64 fanspeed;
>> - u32 command;
>> - int ret;
>> -
>> - if (!quirks->predator_v4)
>> - return -EOPNOTSUPP;
>> -
>> - if (fan == 0)
>> - command = ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED;
>> - else
>> - command = ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED;
>> -
>> - ret = WMID_gaming_get_sys_info(command, &fanspeed);
>> - if (ret < 0)
>> - return ret;
>> -
>> - return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
>> -}
>> -
>> /*
>> * Predator series turbo button
>> */
>> @@ -2688,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)
>> {
>> + u64 command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING;
>> + u64 result;
>> int ret;
>>
>> switch (type) {
>> + case hwmon_temp:
>> + command |= FIELD_PREP(ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK,
>> + acer_wmi_temp_channel_to_sensor_id[channel]);
>> +
>> + 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);
>> + command |= FIELD_PREP(ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK,
>> + acer_wmi_fan_channel_to_sensor_id[channel]);
>> +
>> + 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 = {
>> @@ -2741,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
>>
Powered by blists - more mailing lists