Hi, sorry for the (big) delay > Hi Carlos, > > On 3/24/24 7:05 PM, Carlos Ferreira wrote: > > Added support for 4 zone keyboard rgb on omen laptops. > > > > Signed-off-by: Carlos Ferreira <carlosmiguelferreira.2003@xxxxxxxxx> > > Thank you for your patch and sorry for being slow with replying to this. > > There actually already was a previous attemp to add support for > the 4 zone keyboard to hp-wmi by Rishit Bansal: > > https://lore.kernel.org/platform-driver-x86/20230131235027.36304-1-rishitbansal0@xxxxxxxxx/ > > As discussed there we really want to define a new standardized > userspace API for the backlight functionality of these zoned > RGB keyboards. Using driver specific sysfs attributes for this > is undesirable, since that will never get wide support in userspace. > > OTOH if we define and document a new standard userspace API for this > then hopefully standard userspace stacks like KDE and GNOME will > eventually get support for this and then for the next zoned rgb > keyboard things will just work using the new standard API once > kernel support is merged. > > I realize that using a single LED class device with kbd_backlight > in the name to tap into the existing userspace support to at least > control the overall backlight brightness is useful and tempting but > > IMHO this really is a case where we need a new userspace API and then > emulating just a single brightness control for compatilbility with > existing userspace UI code can be done in powerdevil (KDE) or > upower (GNOME and others) in combination with offereing a more > complete DBUS API to also allow controlling the zones separately. > That makes sense. I should post my first implementation using the multicolor led api soon. > Recently another (laptop) driver for Casper Excalibur laptops has > been posted and this also include support for a 4 zone rgb keyboard: > https://lore.kernel.org/platform-driver-x86/20240324181201.87882-2-mustafa.eskieksi@xxxxxxxxx/ > > This driver actually already implements the userspace API proposed in > the discussion surrounding the earlier "[PATCH V3] platform/x86: hp-wmi: > Support omen backlight control wmi-acpi methods" patch. > > This driver creates 4 LED class devices using the multi-color LED API > for RGB. One LED class device per zone. These are named: > > casper:rgb:kbd_zoned_backlight-right > casper:rgb:kbd_zoned_backlight-middle > casper:rgb:kbd_zoned_backlight-left > casper:rgb:kbd_zoned_backlight-corners > > Where as for the HP laptop case I believe the 4 multi-color LED > class devices should have the following names since the zones > are different: > > hp:rgb:kbd_zoned_backlight-main > hp:rgb:kbd_zoned_backlight-wasd > hp:rgb:kbd_zoned_backlight-cursor > hp:rgb:kbd_zoned_backlight-numpad > For this driver i think it should be something more like this: hp:rgb:kbd_zoned_backlight-right hp:rgb:kbd_zoned_backlight-middle hp:rgb:kbd_zoned_backlight-left hp:rgb:kbd_zoned_backlight-wasd > As I mentioned in my review of the Casper Excalibur laptop driver > as part of adding support for these zoned rgb keyboards to various > laptop vendor specific drivers we should also document how these > devices are presented to userspace: > > A separate patch needs to be written to add documentation about > the use of these names for zoned RGB backlit kbds in a new paragraph / > subsection of the "LED Device Naming" section of: > > Documentation/leds/leds-class.rst > > And this should document at least the 2 currently known > zone sets: > > :rgb:kbd_zoned_backlight-right > :rgb:kbd_zoned_backlight-middle > :rgb:kbd_zoned_backlight-left > :rgb:kbd_zoned_backlight-corners > > :rgb:kbd_zoned_backlight-main > :rgb:kbd_zoned_backlight-wasd > :rgb:kbd_zoned_backlight-cursor > :rgb:kbd_zoned_backlight-numpad > > with a comment that in the future different zone names are possible > if keyboards with a different zoning scheme show up. > > Perhaps you can work together with Mustafa on writing a patch for: > Documentation/leds/leds-class.rst ? > I am open to it if it was not done yet. But could you please be a bit more specific about what exactly needs to be documented about my patch? Zone names, brightness control, userspace interaction? > for this and then hopefully Pavel can review + ack this patch > and then we can move forward with both the hp and the casper > laptop zoned rgb keyboard support. > > Regards, > > Hans
