Received-SPF: pass (google.com: domain of linux-kernel+bounces-46375-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <e21a7d87-3059-4a51-af04-1062dac977d2@tuxedocomputers.com>
Date: Wed, 31 Jan 2024 12:41:02 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Future handling of complex RGB devices on Linux
To: Hans de Goede <hdegoede@redhat.com>, Pavel Machek <pavel@ucw.cz>
Cc: Lee Jones <lee@kernel.org>, jikos@kernel.org,
 linux-kernel@vger.kernel.org, Jelle van der Waa <jelle@vdwaa.nl>,
 Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
 "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
 linux-input@vger.kernel.org, ojeda@kernel.org, linux-leds@vger.kernel.org
References: <aac81702-df1e-43a2-bfe9-28e9cb8d2282@tuxedocomputers.com>
 <CANiq72mfP+dOLFR352O0UNVF8m8yTi_VmOY1zzQdTBjPWCRowg@mail.gmail.com>
 <87sf61bm8t.fsf@intel.com> <ZVvHG/Q+V6kCnfKZ@duo.ucw.cz>
 <f4137e34-c7fb-4f21-bc93-1496cbf61fdf@tuxedocomputers.com>
 <8096a042-83bd-4b9f-b633-79e86995c9b8@redhat.com>
 <f416fbca-589b-4f6a-aad6-323b66398273@tuxedocomputers.com>
 <4222268b-ff44-4b7d-bf11-e350594bbe24@redhat.com>
 <ac02143c-d417-49e5-9c6e-150cbda71ba7@tuxedocomputers.com>
 <ZaljwLe7P+dXHEHb@duo.ucw.cz>
 <6bbfdd62-e663-4a45-82f4-445069a8d690@redhat.com>
 <0cdb78b1-7763-4bb6-9582-d70577781e61@tuxedocomputers.com>
 <7228f2c6-fbdd-4e19-b703-103b8535d77d@redhat.com>
 <730bead8-6e1d-4d21-90d2-4ee73155887a@tuxedocomputers.com>
 <952409e1-2f0e-4d7a-a7a9-3b78f2eafec7@redhat.com>
 <9851a06d-956e-4b57-be63-e10ff1fce8b4@tuxedocomputers.com>
 <1bc6d6f0-a13d-4148-80cb-9c13dec7ed32@redhat.com>
 <b70b2ea8-abfd-4d41-b336-3e34e5bdb8c6@tuxedocomputers.com>
 <477d30ee-247e-47e6-bc74-515fd87fdc13@redhat.com>
Content-Language: en-US
From: Werner Sembach <wse@tuxedocomputers.com>
In-Reply-To: <477d30ee-247e-47e6-bc74-515fd87fdc13@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi,

so I combined Hans last draft, with the discussion since then and the comments 
from the OpenRGB maintainers from here 
https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916 and my own experience 
and came up witrh this rough updated draft for the new uapi:

Future handling of complex RGB devices on Linux:

Optional: Provide a basic leds-subsystem driver:
     - The whole device is treated as a singular RGB led in the current leds uapi
         - Backwards compatibility
         - Have something work out-of-the-box and during boot time
     - The driver also registers a misc device with a singluar sysfs attribute 
select_uapi
         - reading this gives back "[leds] none"
         - the current active uapi can be selected by writing it to that attribute
         - switching the uapi means deregistering the device from that entirely 
and registering and initializing it with the new one froms scratch
         - selecting none only does the deregistering

If the device is already reachable by userspace directly, e.g. via hidraw, the 
kernel will only offer this basic implementation and a more complex driver has 
to be implemented in userspace.
     - This driver has to use the select_uapi attribute first and select "none" 
to avoid undefined behaviour caused by accessing the leds upai and hidraw to 
control the lighting at the same time
     - Question: How to best associate the select_uapi attribute to the 
corresponding hidraw (or other) direct access channel? So that a userspace 
driver can reliable check whether or not this has to be set.

Devices not reachable by userspace directly, e.g. because they are controled via 
a wmi interface, can also be implemented in the new rgbledstring-subsystem 
(working title) for more complex control
     - a rgbledstring device provides an ioctl interface (ioctl only no r/w) 
using /dev/rgbledstring0, /dev/rgbledstring1, etc. registered as a misc chardev.
         - get-device-info ioctl which returns in a single struct:
             - char name[64]                     /* Device model name / 
identifier, preferable human readable. For keyboards, if known to the driver, 
physical layout (or even printed layout) should be separated here */
             - enum device_type                  /* e.g. keyboard, mouse, 
lightbar, etc. */
             - char firmware_version_string[64]  /* if known to the driver, 
empty otherwise */
             - char serial_number[64]            /* if known to the driver, 
empty otherwise */
             - enum supported_modes[64]          /* modes supported by the 
firmware e.g. static/direct, breathing, scan, raindrops, etc. */
         - get-mode-info icotl, RFC here: Hans thinks it is better to have the 
modes and their inputs staticly defined and have, if required, something like 
breathing_clevo_1, breathing_clevo_2, breathing_tongfang_1 if the inputs vary 
between vendors. I think a dynamic approach could be useful where userspace just 
queries the struct required for each individual mode.
             - input: a mode from the supported_modes extracted from get-device-info
             - output: static information about the mode, e.g. 
max_animation_speed, max_brightness, etc.
             - output: the struct/a list of attributes and their types required 
to configure the mode
         - set-mode ioctl takes a single struct:
             - enum mode                         /* from supported_modes */
             - union data
                 - char raw[3072]
                 - <all structs returned by get-mode-info>
         - The driver also registers a singluar sysfs attribute select_uapi
             - reading this gives back "[leds] rgbledstring none" or 
"[rgbledstring] none" respectifly
             - Discussion question: should select_uapi instead be use_leds_uapi
                 - if 1: use basic leds driver
                 - if 0: if device is userspace accessible no kernel driver is 
active, if device ist not userspace accessible register rgbledstring (aka 
implicit separation between rgbledstring and none instead of explicit one)

Zone configuration would be seen as a subset of mode configuration, as I suspect 
not every mode needs the zone configuration even on devices that offer it?

The most simple mode would be static/direct and the set-mode struct would look 
like this:
{
     enum mode, /* = static */
     {
         uint8 brightness, /* global brightness, some keyboards offer this */
         uint8 color[<number_of_leds>*3]
     }
}

Question: Are there modes that have a separate setup command that is only 
required once and then a continuous stream of update information? If yes, should 
we reflect that by splitting set-mode into set-mode-setup and set-mode-update 
(with get-mode-info returning one struct for each)? Or should userspace just 
always send setup and update information and it's up to the kernel driver to 
only resend the setup command when something has changed? In the former case 
set-mode-update might be a noop in most cases.

Discussion on this might also happen here: 
https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916#note_1751170108

Regards,

Werner