Re: [PATCH v2 0/4] platform/x86: Add Lenovo Gaming Series WMI Drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 02.01.25 um 19:27 schrieb Derek John Clark:

On Wed, Jan 1, 2025 at 8:01 PM Mario Limonciello <superm1@xxxxxxxxxx> wrote:


On 1/1/25 18:47, Derek J. Clark wrote:
Adds support for the Lenovo "Gaming Series" of laptop hardware that use
WMI interfaces that control various power settings. There are multiple WMI
interfaces that work in concert to provide getting and setting values as
well as validation of input. Currently only the "GameZone", "Other
Mode", and "LENOVO_CAPABILITY_DATA_01" interfaces are implemented, but
I attempted to structure the driver so that adding the "Custom Mode",
"Lighting", and other data block interfaces would be trivial in a later
patches.

This driver is distinct from, but should be considered a replacement for
this patch:
https://lore.kernel.org/all/20241118100503.14228-1-jonmail@xxxxxxx/

This driver attempts to standardize the exposed sysfs by mirroring the
asus-armoury driver currently under review. As such, a lot of
inspiration has been drawn from that driver.
https://lore.kernel.org/all/20240930000046.51388-1-luke@xxxxxxxxxx/

The drivers have been tested by me on the Lenovo Legion Go.

v2:
- Broke up initial patch into a 4 patch series.
- Removed all references to "Legion" in documentation, Kconfig,
    driver structs, functions, etc. Everything now refers either to the
    interface being used or the Lenovo "Gaming Series" of laptop hardware.
- Fixed all Acked changes requested by Mario and Armin.
- Capability Data is now cached before kset creation for each attribute.
    If the lenovo-wmi-capdata01 interface is not present, fails to grab
    valid data, doesn't include the requested attribute id page, or the
    data block indicates the attribute is not supported, the attribute will
    not be created in sysfs.
- The sysfs path for the firmware-attributes class was moved from
    lenovo-legion-wmi to lenovo-wmi-other.

- The Other Mode WMI interface no longer relies on gamezone as
    discussed. However; this creates a problem that should be discussed
    here. The current_value attribute is now only accurate when the
    "custom" profile is set on the device. Previously it would report the
    value from the Capability Data 01 instance related to the currently
    selected profile, which reported an accurate accounting of the current
    system state in all cases. I submitted this as-is since we discussed
    removing that dependency, but I am not a fan of the current_value
    attribute being incorrect for 3 of the 4 available profiles, especially
    when the data is available. There is also no way to -ENOTSUPP or
    similar when not in custom mode as that would also require us to know
    the state of the gamezone interface. What I would prefer to do would be
    to make the gamezone interface optional by treating custom as the
    default mode in the current_value functions, then only update the mode
    if a callback to get the current fan profile is a success. That way the
    logic will work with or without the GameZone interface, but it will be
    greatly improved if it is present.

I agree there needs to be /some/ sort of dependency.
One thing I was thinking you could do is use:

wmi_has_guid() to tell whether or not the "GZ" interface is even present
from the "Other" driver.  Move the GUID for the GZ interface into a
common header both drivers include.

However that only helps in the case of a system that supports custom but
not GZ.  I think you still will need some sort of symbol to either get a
pointer to the platform profile class or tell if the profile for the
driver is set to custom.

I personally don't see a problem with a simple symbol like this:

bool lenovo_wmi_gamezone_is_custom(void);

You could then have your logic in all the store and show call a helper
something like this:

static bool lenovo_wmi_custom_mode() {
         if (!wmi_has_guid(GZ_GUID)
                 return true;

         if (!IS_REACHABLE(CONFIG_LENOVO_WMI_GAMEZONE))
                 return true;

         return lenovo_wmi_gamezone_is_custom();
}
I agree with checking wmi_has_guid() before calling anything across
interfaces.

Please do not use wmi_has_guid() for this as WMI devices can disappear
at any time.

As far as using a bool to determine if we are in custom,
that seems to me like that would be a half measure. Since we would be
calling across interfaces anyway there is a benefit to getting the
full scope, where knowing only if we are in custom or not would just
add the ability to exit early. What I would prefer is knowing the
specific state of the hardware as it will allow me to call the
specific method ID as related to the current profile. I'll elaborate a
bit on what I mean.

Each attribute ID corresponds to a specific fan profile mode for a
specific attribute. It is used as both the data block ID in
LENOVO_CAPABILITY_DATA_01, and as the first argument when using
GetFeatureValue/SetFeatureValue on the Other Mode interface. I map
these with the lenovo_wmi_attr_id struct. The fan mode value provided
by the gamezone interface corresponds directly to the mode value in
the ID. For example, ID 0x01010100 would provide the capability data
for the CPU device (0x01), SPPT (0x01), in Quiet mode (0x01). There is
no type ID for these attributes (0x00) like there are on some
unimplemented attributes. Balanced mode is 0x02, Performance is 0x03,
Extreme mode (Which the Go doesn't use and there is no analogue for in
the kernel atm) is 0xE0, and custom mode is 0xFF. When the
GetSmartFanMode method ID is called on the gamezone interface it
returns one of these values, corresponding to the current state of the
hardware. This allows us to call GetFeatureValue for the current
profile. Currently we are always calling the custom mode method ID
(0x0101FF00) in GetFeatureValue.

If we want to avoid an additional wmi call in GZ, then grabbing it
from the platform profile and translating it back would maybe suffice.
In that case I would need to implement the
LENOVO_GAMEZONE_SMART_FAN_MODE_EVENT GUID
"D320289E-8FEA-41E0-86F9-611D83151B5F" to ensure that the profile is
updated properly when the hardware is switched profiles using the
physical buttons. This is probably a good idea anyway, but some
guidance on implementing that would be nice as I think it would be an
additional driver and then we have more cross referencing.

I attached a prototype WMI driver for another device which had a similar problem.
The solution was to provide a notifier so other event consumers can be notified
when an WMI event was received.

Example event consumer callback code:

	static int uniwill_wmi_notify_call(struct notifier_block *nb, unsigned long action, void *data)
	{
        	if (action != UNIWILL_OSD_PERF_MODE_CHANGED)
                	return NOTIFY_DONE;

        	platform_profile_cycle();

        	return NOTIFY_OK;
	}

I would also suggest that you use a notifier for communicating with the gamezone
interface. Then you just have to submit commands (as action values) in the form of events
which will then be processed by the available gamezone drivers (the result can be stored in *data).

Those gamezone drivers can then return NOTIFY_STOP which will ensure that only a single gamezone
driver can successfully process a given command.

All in all the patch series seems to progress nicely. I am confident that we will solve the remaining issues.

Thanks,
Armin Wolf


The simplest solution IMO would be to do something closer to what I
was doing in v1 just for current_value_show, where we instantiate the
mode variable as SMARTFAN_MODE_CUSTOM (0xFF) then check if the gz
interface is present. If it is, pass the mode variable as a pointer to
GZ where it can call GetSmartFanMode and update the value. Otherwise,
bypass that block and treat it as custom. This does add an additional
WMI call, but only when reading the current_value.

- I did extensive testing of this firmware-attributes interface and its
    ability to retain the value set by the user. The SPL, SPPT, FPPT, and
    platform profile all retain the users last setting when resuming from
    suspend, a full reboot, and a full shutdown. The only time the values
    are not preserved is when the user manually selects a new platform
    profile using either the pprof interface or the manual selection
    button on the device, in which case you would not expect them to be
    retained as they were intentionally changed. Based on the previous
    discussion it may be the case that older BIOS' will preserve the
    settings even after changing profiles, though I haven't confirmed
    this.
This is good to hear considering the concerns raised by some others.

But FWIW we have nothing in the firmware attributes API documentation
that mandates what the firmware does for storage of settings across a
power cycle so this is currently up to the platform to decide.
v1:
https://lore.kernel.org/platform-driver-x86/CAFqHKTna+kJpHLo5s4Fm1TmHcSSqSTr96JHDm0DJ0dxsZMkixA@xxxxxxxxxxxxxx/T/#t

Suggested-by: Mario Limonciello <superm1@xxxxxxxxxx>
Signed-off-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx>

Derek J. Clark (4):
    platform/x86: Add lenovo-wmi drivers Documentation
    platform/x86: Add Lenovo GameZone WMI Driver
    platform/x86: Add Lenovo Capability Data 01 WMI Driver
    platform/x86: Add Lenovo Other Mode WMI Driver

   Documentation/wmi/devices/lenovo-wmi.rst    | 104 ++++++
   MAINTAINERS                                 |   9 +
   drivers/platform/x86/Kconfig                |  34 ++
   drivers/platform/x86/Makefile               |   3 +
   drivers/platform/x86/lenovo-wmi-capdata01.c | 131 +++++++
   drivers/platform/x86/lenovo-wmi-gamezone.c  | 203 +++++++++++
   drivers/platform/x86/lenovo-wmi-other.c     | 385 ++++++++++++++++++++
   drivers/platform/x86/lenovo-wmi.h           | 241 ++++++++++++
   8 files changed, 1110 insertions(+)
   create mode 100644 Documentation/wmi/devices/lenovo-wmi.rst
   create mode 100644 drivers/platform/x86/lenovo-wmi-capdata01.c
   create mode 100644 drivers/platform/x86/lenovo-wmi-gamezone.c
   create mode 100644 drivers/platform/x86/lenovo-wmi-other.c
   create mode 100644 drivers/platform/x86/lenovo-wmi.h

// SPDX-License-Identifier: GPL-2.0-or-later
/*
 * Linux hotkey driver for Uniwill notebooks.
 *
 * Copyright (C) 2024 Armin Wolf <W_Armin@xxxxxx>
 */

#include <linux/acpi.h>
#include <linux/device.h>
#include <linux/errno.h>
#include <linux/export.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/notifier.h>
#include <linux/printk.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/wmi.h>

#include "uniwill-wmi.h"

#define DRIVER_NAME		"uniwill-wmi"
#define UNIWILL_EVENT_GUID	"ABBC0F72-8EA1-11D1-00A0-C90629100000"

struct uniwill_wmi_data {
	struct mutex input_lock;	/* Protects input sequence during notify */
	struct input_dev *input_device;
};

static BLOCKING_NOTIFIER_HEAD(uniwill_wmi_chain_head);

static const struct key_entry uniwill_wmi_keymap[] = {
	/* Reported via keyboard controller */
	{ KE_IGNORE,	UNIWILL_KEY_CAPSLOCK,			{ KEY_CAPSLOCK }},
	{ KE_IGNORE,	UNIWILL_KEY_NUMLOCK,			{ KEY_NUMLOCK }},
	{ KE_IGNORE,	UNIWILL_KEY_SCROLLLOCK,			{ KEY_SCROLLLOCK }},

	{ KE_IGNORE,	UNIWILL_KEY_TOUCHPAD_ON,		{ KEY_TOUCHPAD_ON }},
	{ KE_IGNORE,	UNIWILL_KEY_TOUCHPAD_OFF,		{ KEY_TOUCHPAD_OFF }},

	/* Reported via "video bus" */
	{ KE_IGNORE,	UNIWILL_KEY_BRIGHTNESSUP,		{ KEY_BRIGHTNESSUP }},
	{ KE_IGNORE,	UNIWILL_KEY_BRIGHTNESSDOWN,		{ KEY_BRIGHTNESSDOWN }},

	/*
	 * Reported in automatic mode when rfkill state changes.
	 * We ignore those events since uniwill-laptop always puts
	 * the EC into manual mode.
	 */
	{ KE_IGNORE,	UNIWILL_OSD_RADIOON,			{.sw = { SW_RFKILL_ALL, 1 }}},
	{ KE_IGNORE,	UNIWILL_OSD_RADIOOFF,			{.sw = { SW_RFKILL_ALL, 0 }}},

	/* Reported via keyboard controller */
	{ KE_IGNORE,	UNIWILL_KEY_MUTE,			{ KEY_MUTE }},
	{ KE_IGNORE,	UNIWILL_KEY_VOLUMEDOWN,			{ KEY_VOLUMEDOWN }},
	{ KE_IGNORE,	UNIWILL_KEY_VOLUMEUP,			{ KEY_VOLUMEUP }},

	{ KE_IGNORE,	UNIWILL_OSD_LIGHTBAR_ON,		{ KEY_RESERVED }},
	{ KE_IGNORE,	UNIWILL_OSD_LIGHTBAR_OFF,		{ KEY_RESERVED }},

	{ KE_KEY,	UNIWILL_OSD_KB_LED_LEVEL0,		{ KEY_KBDILLUMTOGGLE }},
	{ KE_KEY,	UNIWILL_OSD_KB_LED_LEVEL1,		{ KEY_KBDILLUMTOGGLE }},
	{ KE_KEY,	UNIWILL_OSD_KB_LED_LEVEL2,		{ KEY_KBDILLUMTOGGLE }},
	{ KE_KEY,	UNIWILL_OSD_KB_LED_LEVEL3,		{ KEY_KBDILLUMTOGGLE }},
	{ KE_KEY,	UNIWILL_OSD_KB_LED_LEVEL4,		{ KEY_KBDILLUMTOGGLE }},

	{ KE_IGNORE,	UNIWILL_OSD_SUPER_KEY_LOCK_ENABLE,	{ KEY_RESERVED }},
	{ KE_IGNORE,	UNIWILL_OSD_SUPER_KEY_LOCK_DISABLE,	{ KEY_RESERVED }},

	/*
	 * Not reported by other means when in manual mode,
	 * handled automatically when in automatic mode
	 */
	{ KE_KEY,	UNIWILL_KEY_RFKILL,			{ KEY_RFKILL }},

	{ KE_IGNORE,	UNIWILL_OSD_SUPER_KEY_LOCK_TOGGLE,	{ KEY_RESERVED }},
	{ KE_IGNORE,	UNIWILL_OSD_LIGHTBAR_STATE_CHANGED,	{ KEY_RESERVED }},
	{ KE_IGNORE,	UNIWILL_OSD_FAN_BOOST_STATE_CHANGED,	{ KEY_RESERVED }},
	{ KE_IGNORE,	UNIWILL_OSD_DC_ADAPTER_CHANGED,		{ KEY_RESERVED }},

	{ KE_IGNORE,	UNIWILL_OSD_PERF_MODE_CHANGED,		{ KEY_RESERVED }},

	/*
	 * Not reported by other means when in manual mode,
	 * handled automatically when in automatic mode
	 */
	{ KE_KEY,	UNIWILL_KEY_KBDILLUMDOWN,		{ KEY_KBDILLUMDOWN }},
	{ KE_KEY,	UNIWILL_KEY_KBDILLUMUP,			{ KEY_KBDILLUMUP }},
	{ KE_KEY,	UNIWILL_KEY_FN_LOCK,			{ KEY_FN_ESC }},

	{ KE_KEY,	UNIWILL_KEY_KBDILLUMTOGGLE,		{ KEY_KBDILLUMTOGGLE }},

	{ KE_IGNORE,	UNIWILL_OSD_KBD_BACKLIGHT_CHANGED,	{ KEY_RESERVED }},

	{ KE_END }
};

int uniwill_wmi_register_notifier(struct notifier_block *nb)
{
	return blocking_notifier_chain_register(&uniwill_wmi_chain_head, nb);
}
EXPORT_SYMBOL_NS_GPL(uniwill_wmi_register_notifier, UNIWILL);

int uniwill_wmi_unregister_notifier(struct notifier_block *nb)
{
	return blocking_notifier_chain_unregister(&uniwill_wmi_chain_head, nb);
}
EXPORT_SYMBOL_NS_GPL(uniwill_wmi_unregister_notifier, UNIWILL);

static void devm_uniwill_wmi_unregister_notifier(void *data)
{
	struct notifier_block *nb = data;

	uniwill_wmi_unregister_notifier(nb);
}

int devm_uniwill_wmi_register_notifier(struct device *dev, struct notifier_block *nb)
{
	int ret;

	ret = uniwill_wmi_register_notifier(nb);
	if (ret < 0)
		return ret;

	return devm_add_action_or_reset(dev, devm_uniwill_wmi_unregister_notifier, nb);
}
EXPORT_SYMBOL_NS_GPL(devm_uniwill_wmi_register_notifier, UNIWILL);

static void uniwill_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
{
	struct uniwill_wmi_data *data = dev_get_drvdata(&wdev->dev);
	u32 value;
	int ret;

	if (obj->type != ACPI_TYPE_INTEGER)
		return;

	value = obj->integer.value;

	dev_dbg(&wdev->dev, "Received WMI event %u\n", value);

	ret = blocking_notifier_call_chain(&uniwill_wmi_chain_head, value, NULL);
	if (ret == NOTIFY_BAD)
		return;

	mutex_lock(&data->input_lock);
	sparse_keymap_report_event(data->input_device, value, 1, true);
	mutex_unlock(&data->input_lock);
}

static int uniwill_wmi_probe(struct wmi_device *wdev, const void *context)
{
	struct uniwill_wmi_data *data;
	int ret;

	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
	if (!data)
		return -ENOMEM;

	ret = devm_mutex_init(&wdev->dev, &data->input_lock);
	if (ret < 0)
		return ret;

	dev_set_drvdata(&wdev->dev, data);

	data->input_device = devm_input_allocate_device(&wdev->dev);
	if (!data->input_device)
		return -ENOMEM;

	ret = sparse_keymap_setup(data->input_device, uniwill_wmi_keymap, NULL);
	if (ret < 0)
		return ret;

	data->input_device->name = "Uniwill WMI hotkeys";
	data->input_device->phys = "wmi/input0";
	data->input_device->id.bustype = BUS_HOST;

	return input_register_device(data->input_device);
}

/*
 * We cannot fully trust this GUID since Uniwill just copied the WMI GUID
 * from the Windows driver example, and others probably did the same.
 *
 * Because of this we cannot use this WMI GUID for autoloading.
 */
static const struct wmi_device_id uniwill_wmi_id_table[] = {
	{ UNIWILL_EVENT_GUID, NULL },
	{ }
};

static struct wmi_driver uniwill_wmi_driver = {
	.driver = {
		.name = DRIVER_NAME,
		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
	},
	.id_table = uniwill_wmi_id_table,
	.probe = uniwill_wmi_probe,
	.notify = uniwill_wmi_notify,
	.no_singleton = true,
};
module_wmi_driver(uniwill_wmi_driver);

MODULE_AUTHOR("Armin Wolf <W_Armin@xxxxxx>");
MODULE_DESCRIPTION("Uniwill notebook hotkey driver");
MODULE_LICENSE("GPL");
/* SPDX-License-Identifier: GPL-2.0-or-later */
/*
 * Linux hotkey driver for Uniwill notebooks.
 *
 * Copyright (C) 2024 Armin Wolf <W_Armin@xxxxxx>
 */

#ifndef UNIWILL_WMI_H
#define UNIWILL_WMI_H

#define UNIWILL_KEY_CAPSLOCK			0x01
#define UNIWILL_KEY_NUMLOCK			0x02
#define UNIWILL_KEY_SCROLLLOCK			0x03

#define UNIWILL_KEY_TOUCHPAD_ON			0x04
#define UNIWILL_KEY_TOUCHPAD_OFF		0x05

#define UNIWILL_KEY_BRIGHTNESSUP		0x14
#define UNIWILL_KEY_BRIGHTNESSDOWN		0x15

#define UNIWILL_OSD_RADIOON			0x1A
#define UNIWILL_OSD_RADIOOFF			0x1B

#define UNIWILL_KEY_MUTE			0x35
#define UNIWILL_KEY_VOLUMEDOWN			0x36
#define UNIWILL_KEY_VOLUMEUP			0x37

#define UNIWILL_OSD_LIGHTBAR_ON			0x39
#define UNIWILL_OSD_LIGHTBAR_OFF		0x3A

#define UNIWILL_OSD_KB_LED_LEVEL0		0x3B
#define UNIWILL_OSD_KB_LED_LEVEL1		0x3C
#define UNIWILL_OSD_KB_LED_LEVEL2		0x3D
#define UNIWILL_OSD_KB_LED_LEVEL3		0x3E
#define UNIWILL_OSD_KB_LED_LEVEL4		0x3F

#define UNIWILL_OSD_SUPER_KEY_LOCK_ENABLE	0x40
#define UNIWILL_OSD_SUPER_KEY_LOCK_DISABLE	0x41

#define UNIWILL_KEY_RFKILL			0xA4

#define UNIWILL_OSD_SUPER_KEY_LOCK_TOGGLE	0xA5

#define UNIWILL_OSD_LIGHTBAR_STATE_CHANGED	0xA6

#define UNIWILL_OSD_FAN_BOOST_STATE_CHANGED	0xA7

#define UNIWILL_OSD_DC_ADAPTER_CHANGED		0xAB

#define UNIWILL_OSD_PERF_MODE_CHANGED		0xB0

#define UNIWILL_KEY_KBDILLUMDOWN		0xB1
#define UNIWILL_KEY_KBDILLUMUP			0xB2

#define UNIWILL_KEY_FN_LOCK			0xB8
#define UNIWILL_KEY_KBDILLUMTOGGLE		0xB9

#define UNIWILL_OSD_KBD_BACKLIGHT_CHANGED	0xF0

struct notifier_block;

int uniwill_wmi_register_notifier(struct notifier_block *nb);
int uniwill_wmi_unregister_notifier(struct notifier_block *nb);
int devm_uniwill_wmi_register_notifier(struct device *dev, struct notifier_block *nb);

#endif /* UNIWILL_WMI_H */

[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux