From: Hans de Goede <hdegoede@redhat.com>
To: Antheas Kapenekakis <lkml@antheas.dev>,
platform-driver-x86@vger.kernel.org, linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, "Jiri Kosina" <jikos@kernel.org>,
"Benjamin Tissoires" <bentiss@kernel.org>,
"Corentin Chary" <corentin.chary@gmail.com>,
"Luke D . Jones" <luke@ljones.dev>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements
Date: Mon, 24 Mar 2025 13:10:11 +0100 [thread overview]
Message-ID: <dac78c3d-9ba2-4721-9fb2-06dd2589bc72@redhat.com> (raw)
In-Reply-To: <20250319191320.10092-1-lkml@antheas.dev>
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
next prev parent reply other threads:[~2025-03-24 12:10 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-19 19:13 Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 01/11] HID: asus: refactor init sequence per spec Antheas Kapenekakis
2025-03-20 7:19 ` Luke D. Jones
2025-03-20 9:50 ` Antheas Kapenekakis
2025-03-20 11:47 ` Antheas Kapenekakis
2025-03-20 21:01 ` Luke D. Jones
2025-03-20 21:09 ` Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 02/11] HID: asus: cleanup keyboard backlight check Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 03/11] HID: asus: prevent binding to all HID devices on ROG Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 04/11] HID: asus: rename keyboard3 to Z13_FOLIO Antheas Kapenekakis
2025-03-22 1:31 ` Luke D. Jones
2025-03-19 19:13 ` [PATCH 05/11] HID: asus: add Asus Z13 2025 Fan key Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 06/11] HID: asus: introduce small delay on Asus Z13 RGB init Antheas Kapenekakis
2025-03-20 7:12 ` Luke D. Jones
2025-03-20 8:30 ` Antheas Kapenekakis
2025-03-20 21:03 ` Luke D. Jones
2025-03-19 19:13 ` [PATCH 07/11] platform/x86: asus-wmi: Add support for multiple kbd RGB handlers Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 08/11] HID: asus: listen to the asus-wmi brightness device instead of creating one Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 09/11] platform/x86: asus-wmi: remove unused keyboard backlight quirk Antheas Kapenekakis
2025-03-20 7:10 ` Luke D. Jones
2025-03-20 8:28 ` Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 10/11] platform/x86: asus-wmi: add keyboard brightness event handler Antheas Kapenekakis
2025-03-19 19:13 ` [PATCH 11/11] HID: asus: add support for the asus-wmi brightness handler Antheas Kapenekakis
2025-03-20 10:18 ` kernel test robot
2025-03-19 21:50 ` [PATCH 00/11] HID: asus: hid-asus and asus-wmi backlight unification, Z13 QOL improvements Antheas Kapenekakis
2025-03-20 6:09 ` Luke Jones
2025-03-20 8:26 ` Antheas Kapenekakis
2025-03-24 12:10 ` Hans de Goede [this message]
2025-03-24 12:25 ` Antheas Kapenekakis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dac78c3d-9ba2-4721-9fb2-06dd2589bc72@redhat.com \
--to=hdegoede@redhat.com \
--cc=bentiss@kernel.org \
--cc=corentin.chary@gmail.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkml@antheas.dev \
--cc=luke@ljones.dev \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®