Hi Pavel, Andy, Hans, > > > > > > > > + /* > > > > > > > > + * asus::kbd_backlight still controls a > > > > > > > > base > > > > > > 3-level backlight and when > > > > > > > > + * it is on 0, the RGB is not visible > > > > > > > > at all. > > > > RGB > > should be treated as > > > > > > > > + * an additional step. > > > > > > > > + */ > > > > > > > > Ouch. Lets not do that? If rgb interface is available, hide the > > > > 3 > > > > level one, or something. > > > > I really don't think this is safe or sensible. There are some laptops that default the 3-stage method to off, and this means that the LEDs will not show regardless of multicolor brightness. > > > > > > > > + mc_cdev->led_cdev.name = > > > > > > > > > > > > > > "asus::multicolour::kbd_backlight"; > > > > > > > > Make this "rgb:kbd_backlight" or "inputX:rgb:kbd_backligh" and > > > > document it in Documentation/leds/well-known-leds.txt. Will do. -- 4 hours later -- I've spent a lot of time working on this now. I don't think multicolor LED is suitable for use with the way these keyboards work. The biggest issue is related to the brightness setting. 1. If the ASUS_WMI_DEVID_KBD_BACKLIGHT method defaults to 0 on boot then RGB is not visible I worked around this by setting it to "3" by default in module if ASUS_WMI_DEVID_TUF_RGB_MODE is found. And added a check in the button events to adjust multicolor brightness (+/- 17). This works but now I can't do led notify (led_classdev_notify_brightness_hw_changed). 2. Pattern trigger can't be used for these keyboard modes as the modes are done entirely in hardware via a single switch in the complete command packet. I don't see any way forward with this, and looking at the complexity I don't have time either. 3. Nodes everywhere.. To fully control control these keyboards there are two WMI methods, one for mode/rgb, one for power-state. Splitting each of these parameters out to individual nodes with sensible naming and expectations gives: - keyboard_rgb_apply, new WO node to actually write out data - keyboard_rgb_save, first parameter of packet, retain-on-boot - keyboard_rgb_mode, the factory built-in modes, - keyboard_rgb_speed, speed of certain modes And then for power-state: - keyboard_state_apply, new WO node to actually write out data - keyboard_state_save, first parameter of packet, retain-on-boot - keyboard_state_boot, play boot animation (on boot) - keyboard_state_awake, LEDs visible while awake - keyboard_state_sleep, play suspend animation (while suspended) - keyboard_state_keyboard, unknown effect Quite frankly I would rather use the method I had in the first patch I submitted where mode and state had two nodes each, - keyboard_rgb_mode, WO = "n n n n n n" - keyboard_rgb_mode_index, output = "save/apply, mode, r, g, b, speed" - keyboard_rgb_state, WO = "n n n n n" - keyboard_rgb_state_index, output = "save/apply, boot, awake, sleep, keyboard" A big benefit of this structure is that not being able to read settings back from the keyboard (not possible!) becomes a non-issue because users have to write a full input, not partial, and it will apply right away. Multicolor class could still be used, but from everything I've tried now it really isn't suitable when the proper method for brightness is a separate WMI method (0-3), and when that is 0 it means LEDs are fully off - there's potential for mistakes/issues. Losing led-notif is an issue for users for sure. In short, from dog-fooding the current state inlcuding the trial of using multicolor brightness (and hiding the proper WMI method) I can only conclude that multicolor is not suitable for how these keyboards work. Hans, Andy, can I please revert back to the node + _index pairs taking an array input. Everything will be cleaner and simpler. Cheers, Luke.
