Message-ID: <dac78c3d-9ba2-4721-9fb2-06dd2589bc72@redhat.com>
Date: Mon, 24 Mar 2025 13:10:11 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Antheas Kapenekakis <lkml@...heas.dev>,
 platform-driver-x86@...r.kernel.org, linux-input@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Jiri Kosina <jikos@...nel.org>,
 Benjamin Tissoires <bentiss@...nel.org>,
 Corentin Chary <corentin.chary@...il.com>, "Luke D . Jones"
 <luke@...nes.dev>, Ilpo Järvinen
 <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight
 unification, Z13 QOL improvements

Hi Antheas,

On 19-Mar-25 20:13, Antheas Kapenekakis wrote:
> This is a three part series that does the following: first, it cleans up
> the hid-asus driver initialization, preventing excess renames and dmesg
> errors on ROG devices. Then, it adds support for the Z13 2025 keyboard,
> by fixing its keyboard to not be stuck in BIOS mode and enabling its fan
> key. Finally, the bigger piece of this series is the unification of the
> backlight controls between hid-asus and asus-wmi.

Thank you for your work on this.

> This requires some context. First, some ROG devices expose both WMI and
> HID controls for RGB. In addition, some ROG devices (such as the Z13)
> have two AURA devices where both have backlight controls (lightbar and
> keyboard). Under Windows, Armoury Crate exposes a single brightness control
> for all Aura devices.
> 
> However, currently in the linux kernel this is not the case, with asus-wmi
> and hid-asus relying on a quirk system to figure out which should control
> the backlight. But what about the other one? There might be silent
> regressions such as part of the RGB of the device not responding properly.
> 
> In the Z13, this is the case, with a race condition causing the lightbar
> to control the asus::kbd_backlight device most of the time, with the
> keyboard being renamed to asus::kbd_backlight_1 and not doing anything
> under KDE controls.
> 
> Here, we should note that most backlight handlers are hardcoded to check
> for backlight once, and for one backlight, during boot, so any other
> solution would require a large rewrite of userspace.

Note that work is actually ongoing to add support for multiple kbd
backlights to upower:

https://gitlab.freedesktop.org/upower/upower/-/merge_requests/258

But that is intended for when there are 2 kbds with a controllable backlight,
e.g. a docked laptop with a gaming kbd with RGB backlight connected to the dock.

Where as here we seem to have 2 controls which ideally should be set to
the same value if I understand things correctly ?

> Even when brightness controls are fixed, we still have the problem of the
> backlight key being on/off when controlled by KDE and 0/33/66/100 when
> the device has a WMI keyboard. Ideally, we would like the 0/33/66/100 to
> be done under hid as well, regardless of whether the backlight of the
> device is controlled by WMI or HID.

Hmm, ideally we want this sort of policy to be in userspace, this sounds
more like it is a keycode problem and we maybe need KEY_KBDILLUMCYCLE next
to the existing KEY_KBDILLUMTOGGLE. For the existing toggle doing on/off
obviously is the correct userspace behavior.

Anyways I can see how Asus is special here and on laptops the cycling is
typically handled by the EC and we have chosen to emulate EC behavior in
the kernel before to keep things consistent amongst models.

Still generally speaking we do prefer to just send keypresses when possible
and let userspace set the policy, but I guess we can make an exception here.

> Therefore, this is what the third part of this series does. It sets up
> asus-wmi to expose accepting listeners for the asus::kbd_backlight device
> and being the one that sets it up. Then, it makes hid-asus devices
> register a listener there, so that all of them are controlled by
> asus::kbd_backlight. Finally, it adds an event handler for keyboard keys,
> so that HID led controls are handled by the kernel instead of userspace.
> This way, even when userspace is not active the key works, and we get the
> desired behavior of 0/33/66/100 across all Aura devices (currently, that
> is keyboards, and embedded devices such as lightbars). This results
> removing the quirk system as well, eliminating a point of failure.

I've taken a quick look at the new API between asus-wmi and asus-hid and
this looks good to me, thank you for your work on this.

Regards,

Hans

Powered by blists - more mailing lists