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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 5/19/21 1:05 PM, Pavel Machek wrote:
> 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.

Can you provide a link to the patches from Mauro, those might be
helpful for Jafar to take a look at to get some idea how he can
implement this using the LED API.

Regards,

Hans