[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4cb33604-28c6-ac51-0162-2e5a027f02a0@redhat.com>
Date: Tue, 2 Aug 2022 14:13:06 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: "Luke D. Jones" <luke@...nes.dev>
Cc: corentin.chary@...il.com, markgross@...nel.org,
linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH] asus-wmi: Add support for TUF laptop keyboard RGB
Hi,
On 8/2/22 13:09, Hans de Goede wrote:
> Hi Luke,
>
> On 8/2/22 06:59, Luke D. Jones wrote:
>> Adds support for TUF laptop RGB control. This creates two sysfs
>> paths to add control of basic keyboard LEDs, and power states.
>>
>> /sys/devices/platform/asus-nb-wmi/tuf_krgb_mode has the following
>> as input options via U8 "n n n n n n":
>> - Save or set, if set, then settings revert on cold boot
>> - Mode, 0-8 for regular modes (if supported), 10-12 for "warning" styles
>> - Red, 0-255
>> - Green, 0-255
>> - Blue, 0-255
>> - Speed, 0 = Slow, 1 = Medium, 2 = Fast
>>
>> /sys/devices/platform/asus-nb-wmi/tuf_krgb_state has the following
>> as input options via boolean "b b b b b":
>> - Save or set, if set, then settings revert on cold boot
>> - Boot, if true, the keyboard displays animation on boot
>> - Awake, if true, the keyboard LED's are on while device is awake
>> - Sleep, if true, the keyboard shows animation while device is suspended
>> - Keybaord, appears to have no effect
>
> Typo in Keybaord here.
>
> Thank you for your patch. I really appreciate your continued
> efforts to make Asus laptops work well with Linux.
>
> For keyboard backlight support Linux has standardized on
> using the /sys/class/leds API. So I'm afraid that this patch
> will need to be rewritten to use the standard LED API
> and then specifically the somewhat new multicolor LED API
> at least for setting the RGB values (within the current mode)
>
> Any extra functionality can then be added as extra sysfs
> attributes under the /sys/class/leds/asus::kbd_backlight
> device, see e.g. the use of kbd_led_groups in:
> drivers/platform/x86/dell/dell-laptop.c
>
> Note the kbd_backlight part of the name is important this
> will allow upower to recognize this as a keyboard backlight
> and will then enable desktop-environments which use
> upower for kbd backlight control to at least control
> the overall brightness of the kbd-backlight.
>
> I realize that this means that you need to redo a whole
> bunch of work here; and I presume also in your
> asusctl userspace utility, sorry about that. But it
> really is important that standard userspace APIs are
> used for things like this where ever possible.
>
> Regards,
>
> Hans
p.s.
For more info on the multi-color LED API see:
https://www.kernel.org/doc/html/latest/leds/leds-class-multicolor.html
https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.9-Multi-Color-LEDs
>
>
>
>
>>
>> Signed-off-by: Luke D. Jones <luke@...nes.dev>
>> ---
>> drivers/platform/x86/asus-wmi.c | 168 +++++++++++++++++++++
>> include/linux/platform_data/x86/asus-wmi.h | 6 +
>> 2 files changed, 174 insertions(+)
>>
>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
>> index 62ce198a3463..09277bd98249 100644
>> --- a/drivers/platform/x86/asus-wmi.c
>> +++ b/drivers/platform/x86/asus-wmi.c
>> @@ -234,6 +234,9 @@ struct asus_wmi {
>> bool dgpu_disable_available;
>> bool dgpu_disable;
>>
>> + bool tuf_kb_rgb_mode_available;
>> + bool tuf_kb_rgb_state_available;
>> +
>> bool throttle_thermal_policy_available;
>> u8 throttle_thermal_policy_mode;
>>
>> @@ -734,6 +737,153 @@ static ssize_t egpu_enable_store(struct device *dev,
>>
>> static DEVICE_ATTR_RW(egpu_enable);
>>
>> +/* TUF Laptop Keyboard RGB Modes **********************************************/
>> +static int tuf_kb_rgb_mode_check_present(struct asus_wmi *asus)
>> +{
>> + u32 result;
>> + int err;
>> +
>> + asus->tuf_kb_rgb_mode_available = false;
>> +
>> + err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_MODE, &result);
>> + if (err) {
>> + if (err == -ENODEV)
>> + return 0;
>> + return err;
>> + }
>> +
>> + if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>> + asus->tuf_kb_rgb_mode_available = true;
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t tuf_kb_rgb_mode_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + int err;
>> + u32 ret;
>> + u8 res, tmp, arg_num;
>> + char *data, *part, *end;
>> + u8 cmd, mode, r, g, b, speed;
>> +
>> + data = end = kstrdup(buf, GFP_KERNEL);
>> + cmd = mode = r = g = b = speed = arg_num = 0;
>> +
>> + while ((part = strsep(&end, " ")) != NULL) {
>> + if (part == NULL)
>> + return -1;
>> +
>> + res = kstrtou8(part, 10, &tmp);
>> + if (res)
>> + return -1;
>> +
>> + if (arg_num == 0)
>> + // apply : set
>> + cmd = tmp == 1 ? 0xb5 : 0xb4;
>> + else if (arg_num == 1)
>> + // From 0-8 are valid modes with 10-12 being "warning"
>> + // style modes. All models have "pulse" mode 10.
>> + mode = (tmp <= 12 && tmp != 9) ? tmp : 10;
>> + else if (arg_num == 2)
>> + r = tmp;
>> + else if (arg_num == 3)
>> + g = tmp;
>> + else if (arg_num == 4)
>> + b = tmp;
>> + else if (arg_num == 5) {
>> + if (tmp == 0)
>> + speed = 0xe1;
>> + else if (tmp == 1)
>> + speed = 0xeb;
>> + else if (tmp == 2)
>> + speed = 0xf5;
>> + }
>> +
>> + arg_num += 1;
>> + }
>> +
>> + err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS, ASUS_WMI_DEVID_TUF_RGB_MODE,
>> + cmd | (mode << 8) | (r << 16) | (g << 24), (b) | (speed << 8), &ret);
>> + if (err)
>> + return err;
>> +
>> + kfree(data);
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR_WO(tuf_kb_rgb_mode);
>> +
>> +/* TUF Laptop Keyboard RGB States *********************************************/
>> +static int tuf_kb_rgb_state_check_present(struct asus_wmi *asus)
>> +{
>> + u32 result;
>> + int err;
>> +
>> + asus->tuf_kb_rgb_state_available = false;
>> +
>> + err = asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_TUF_RGB_STATE, &result);
>> + if (err) {
>> + if (err == -ENODEV)
>> + return 0;
>> + return err;
>> + }
>> +
>> + if (result & ASUS_WMI_DSTS_PRESENCE_BIT)
>> + asus->tuf_kb_rgb_state_available = true;
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t tuf_kb_rgb_state_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + int err;
>> + u32 ret;
>> + bool tmp;
>> + char *data, *part, *end;
>> + u8 save, flags, res, arg_num;
>> +
>> + save = flags = arg_num = 0;
>> + data = end = kstrdup(buf, GFP_KERNEL);
>> +
>> + while ((part = strsep(&end, " ")) != NULL) {
>> + if (part == NULL)
>> + return -1;
>> +
>> + res = kstrtobool(part, &tmp);
>> + if (res)
>> + return -1;
>> +
>> + if (tmp) {
>> + if (arg_num == 0) // save : set
>> + save = tmp == 0 ? 0x0100 : 0x0000;
>> + else if (arg_num == 1)
>> + flags |= 0x02; // boot
>> + else if (arg_num == 2)
>> + flags |= 0x08; // awake
>> + else if (arg_num == 3)
>> + flags |= 0x20; // sleep
>> + else if (arg_num == 4)
>> + flags |= 0x80; // keyboard
>> + }
>> +
>> + arg_num += 1;
>> + }
>> +
>> + err = asus_wmi_evaluate_method3(ASUS_WMI_METHODID_DEVS,
>> + ASUS_WMI_DEVID_TUF_RGB_STATE, 0xBD | save | (flags << 16), 0, &ret);
>> + if (err)
>> + return err;
>> +
>> + kfree(data);
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR_WO(tuf_kb_rgb_state);
>> +
>> /* Battery ********************************************************************/
>>
>> /* The battery maximum charging percentage */
>> @@ -3258,6 +3408,8 @@ static struct attribute *platform_attributes[] = {
>> &dev_attr_touchpad.attr,
>> &dev_attr_egpu_enable.attr,
>> &dev_attr_dgpu_disable.attr,
>> + &dev_attr_tuf_kb_rgb_mode.attr,
>> + &dev_attr_tuf_kb_rgb_state.attr,
>> &dev_attr_lid_resume.attr,
>> &dev_attr_als_enable.attr,
>> &dev_attr_fan_boost_mode.attr,
>> @@ -3286,6 +3438,12 @@ static umode_t asus_sysfs_is_visible(struct kobject *kobj,
>> devid = ASUS_WMI_DEVID_ALS_ENABLE;
>> else if (attr == &dev_attr_egpu_enable.attr)
>> ok = asus->egpu_enable_available;
>> + else if (attr == &dev_attr_tuf_kb_rgb_mode.attr)
>> + ok = asus->tuf_kb_rgb_mode_available;
>> + else if (attr == &dev_attr_tuf_kb_rgb_state.attr)
>> + ok = asus->tuf_kb_rgb_state_available;
>> + else if (attr == &dev_attr_dgpu_disable.attr)
>> + ok = asus->dgpu_disable_available;
>> else if (attr == &dev_attr_dgpu_disable.attr)
>> ok = asus->dgpu_disable_available;
>> else if (attr == &dev_attr_fan_boost_mode.attr)
>> @@ -3557,6 +3715,14 @@ static int asus_wmi_add(struct platform_device *pdev)
>> if (err)
>> goto fail_dgpu_disable;
>>
>> + err = tuf_kb_rgb_mode_check_present(asus);
>> + if (err)
>> + goto fail_tuf_kb_rgb_mode;
>> +
>> + err = tuf_kb_rgb_state_check_present(asus);
>> + if (err)
>> + goto fail_tuf_kb_rgb_state;
>> +
>> err = fan_boost_mode_check_present(asus);
>> if (err)
>> goto fail_fan_boost_mode;
>> @@ -3671,6 +3837,8 @@ static int asus_wmi_add(struct platform_device *pdev)
>> fail_fan_boost_mode:
>> fail_egpu_enable:
>> fail_dgpu_disable:
>> +fail_tuf_kb_rgb_mode:
>> +fail_tuf_kb_rgb_state:
>> fail_platform:
>> fail_panel_od:
>> kfree(asus);
>> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
>> index a571b47ff362..af4191fb0508 100644
>> --- a/include/linux/platform_data/x86/asus-wmi.h
>> +++ b/include/linux/platform_data/x86/asus-wmi.h
>> @@ -98,6 +98,12 @@
>> /* dgpu on/off */
>> #define ASUS_WMI_DEVID_DGPU 0x00090020
>>
>> +/* TUF laptop RGB modes/colours */
>> +#define ASUS_WMI_DEVID_TUF_RGB_MODE 0x00100056
>> +
>> +/* TUF laptop RGB power/state */
>> +#define ASUS_WMI_DEVID_TUF_RGB_STATE 0x00100057
>> +
>> /* DSTS masks */
>> #define ASUS_WMI_DSTS_STATUS_BIT 0x00000001
>> #define ASUS_WMI_DSTS_UNKNOWN_BIT 0x00000002
Powered by blists - more mailing lists