Re: [PATCH 0/1] platform/x86/tuxedo: Add virtual LampArray for TUXEDO NB04

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Benjamin,

Am 27.09.24 um 18:08 schrieb Benjamin Tissoires:
On Sep 26 2024, Werner Sembach wrote:
Hi,
took some time but now a first working draft of the suggested new way of
handling per-key RGB keyboard backlights is finished. See:
https://lore.kernel.org/all/1fb08a74-62c7-4d0c-ba5d-648e23082dcb@xxxxxxxxxxxxxxxxxxx/
First time for me sending a whole new driver to the LKML, so please excuse
mistakes I might have made.

Known bugs:
- The device has a lightbar which is currently not implemented and
   therefore stuck to blue once the first backlight control command is send.

What is still missing:
- The leds fallback
- Lightbar control

Some general noob questions:

Initially I though it would be nice to have 2 modules, one jsut being the
wmi initialization and utility stuff and one just being the backlight logic
stuff, being loaded automatically via module_alias, but that would still
require me to create the virtual hid device during the wmi_ab probe, and
that already needs the ll_driver, so i guess I have to do it statically
like i did now?
Or in other words: I would have liked to have a module dependency graph
like this:
     tuxedo_nb04_lamp_array depends on tuxedo_nb04_platform (combining *_wmi_init and *_wmi_utility)
but if i currently split it into modules i would get this:
     tuxedo_nb04_wmi_ab_init dpends on tuxedo_nb04_wmi_ab_lamp_array depends on tuxedo_nb04_wmi_utility
On more general question to you: how much confident are you about your
LampArray implementation?

If you still need to add/fix stuff in it, I would advise you to have a
simple HID device, with bare minimum functionality, and then add the
LampArray functionality on top through HID-BPF. This way you can fix
LampArray out of band with the kernel, while having a more stable kernel
module. This should be possible with v6.11+.

Another solution is to still have your wmi-to-hid module, and then a
HID kernel module in drivers/hid that supports LampArray.

But I would strongly suggest while you are figuring out the userspace
part to stick to HID-BPF, and then once you are happy we can move to a
full kernel module.

I don't expect this patch to get merged right away, but like i wrote, wanted to collect some feedback on it to already start refining it.

With this driver now functional I have something to build and test userspace against while waiting on the feedback and the undoubtly following discussion of details to get it right ^^.

Until now I only tested with a very simple, self built command line binary, looping some patterns. My next step is to try the work in progress implementetion for LampArray in OpenRGB: https://gitlab.com/CalcProgrammer1/OpenRGB/-/merge_requests/2348

Regards,

Werner


Cheers,
Benjamin

Currently after creating the virtual hdev in the wmi init probe function I
have to keep track of it and manually destroy it during the wmi init
remove. Can this be automated devm_kzalloc-style?

Kind regards,
Werner Sembach





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux