linux-kernel - Re: [PATCH] platform/x86: acer-wmi: Add support for Acer Helios 300 RGB keyboard backlight

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210519110542.GA24621@duo.ucw.cz>
Date:   Wed, 19 May 2021 13:05:42 +0200
From:   Pavel Machek <pavel@....cz>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     Jafar Akhondali <jafar.akhoondali@...il.com>, jlee@...e.com,
        mgross@...ux.intel.com, platform-driver-x86@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] platform/x86: acer-wmi: Add support for Acer Helios 300
 RGB keyboard backlight

Hi!
> >>> The Acer helios 300 provides gaming functions WMI that is available in
> >>> Windows, however this was not implemented in Linux. The process of finding
> >>> the related method was done by decompiling PredatorSense(official Acer
> >>> gaming functions software for Predator series) and decompiling WQ
> >>> buffers. This patch provides a gaming interface which will then expose a
> >>> character device named "acer-gkbbl". This character device accepts 16
> >>> bytes long config, which is specific for the backlight method. The
> >>> meaning of each bytes ordered by bit position is as follows:
> >>>
> >>> Bit 0 -> Backlight modes:
> >>> 1: Breath
> >>> 2: Neon
> >>> 3: Wave
> >>> 4: Shifting
> >>> 5: Zoom
> >>> Bit 1 -> Animation Speed: from 1 to 9 ( 1 is slowest, 9 is fastest)
> >>> Bit 2 -> Brightness from 0 to 100 ( 0 is no backlight, 100 is brightest)
> >>> Bit 3 -> Unknown. Wave effect uses 8, other modes must use 0
> >>> Bit 4 -> Animation Direction:
> >>> 1: Right-to-Left
> >>> 2: Left-to-Right
> >>> Bit 5 -> Red Color Selection
> >>> Bit 6 -> Green Color Selection
> >>> Bit 7 -> Blue Color Selection
> >>> Bit 8 -> Currently unknown, or not used in known model
> >>> Bit 9 -> Currently unknown, or not used in known model
> >>> Bit 10 -> Currently unknown, or not used in known model
> >>> Bit 11 -> Currently unknown, or not used in known model
> >>> Bit 12 -> Currently unknown, or not used in known model
> >>> Bit 13 -> Currently unknown, or not used in known model
> >>> Bit 14 -> Currently unknown, or not used in known model
> >>> Bit 15 -> Currently unknown, or not used in known model
> >>>
> >>> Filling this config is out of scope for the kernel module, and this module
> >>> only acts as an interface.
> >>>
> >>> Currently, I'm not sure with the method for communicating with user-space,
> >>> but since leds.h subsystem wouldn't fit for complex actions such as this
> >>> complex config, I couldn't find any better method than char dev.
> >>
> >> Thank you for your patch, given that there is no existing kernel
> >> interface which is a good match for the features exported by this
> >> keyboard I'm fine with just having a raw interface where userspace
> >> writes GAMING_KBBL_CONFIG_LEN bytes as you suggest.
> > 
> > Keyboard backlight goes through LED interface (so please cc the mailing list) and
> > no, passing raw bytes to hardware is not an acceptable interface.
> > 
> >> But lets not use a classdev + chardev for this please, you can
> >> just add a binary write-only sysfs-atrribute under the wmi-dev for
> >> this with a name like (for example) gaming_kbd_backlight_config
> >> and then userspace can write to that without needing a class + char
> >> dev just for this single write.
> > 
> > NAK. We have existing interfaces for this.
> 
> I don't think we have existing interfaces for things like wave / zoom /
> neon effects.  I guess this could use the new CONFIG_LEDS_CLASS_MULTICOLOR
> API to set the color + overall brightness, combined with a custom sysfs attribute
> under the led_classdev's device's dir for selecting the things for which there
> is no standard API ?

We do have RGB LEDs, and some of them can do hardware-accelerated
effects. We do have pattern trigger.

We do have other hardware (Mauro is just trying to merge support) that
has limited set of effects.

Yes, some work remains to be done. But no, introducing custom
interface for this is not acceptable.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)