Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <79c27c71-a86b-4b30-aa1a-96b2f7bf26b4@tuxedocomputers.com>
Date:   Wed, 11 Oct 2023 21:21:41 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] leds: rgb: Implement per-key keyboard backlight for
 several TUXEDO devices
Content-Language: en-US
To:     Pavel Machek <pavel@ucw.cz>, Lee Jones <lee@kernel.org>
Cc:     linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org
References: <20231011190017.1230898-1-wse@tuxedocomputers.com>
From:   Werner Sembach <wse@tuxedocomputers.com>
In-Reply-To: <20231011190017.1230898-1-wse@tuxedocomputers.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

Am 11.10.23 um 21:00 schrieb Werner Sembach:
> From: Christoffer Sandberg <cs@tuxedo.de>
>
> Implement per-key keyboard backlight in the leds sysfs interface for
> several TUXEDO devices using the ite8291 controller.
>
> There are however some known short comings:
> - The sysfs leds interface does only allow to write one key at a time. The
> controller however can only update row wise or the whole keyboard at once
> (whole keyboard update is currently not implemented). This means that even
> when you want to updated a whole row, the whole row is actually updated
> once for each key. So you actually write up to 18x as much as would be
> required.
> - When you want to update the brightness of the whole keyboard you have to
> write 126 sysfs entries, which inherently is somewhat slow, especially when
> using a slider that is live updating the brightness.
> - While the controller manages up to 126 leds, not all are actually
> populated. However the unused are not grouped at the end but also in
> between. To not have dead sysfs entries, this would require manual testing
> for each implemented device to see which leds are used and some kind of
> bitmap in the driver to sort out the unconnected ones.
> - It is not communicated to userspace which led entry controls which key
> exactly
>
> Co-developed-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Christoffer Sandberg <cs@tuxedo.de>

The first time I submit a whole module, so please let me know if it made any 
mistakes e.g. I'm unsure if I need add myself explicitly as a maintainer, if 
MODULE_AUTHOR has to be a human, or if i have to split this up into smaller junks.

Also please let me know if i somehow misinterpreted the current API and the 
shortcomings can actually be avoided.

I have not yet looked deeply into triggers, but one idea i had is to only have 
one kbd_backlight by default that just makes the whole keyboard the same color 
and brightness. In addition to that a trigger per_key_control, that, when set, 
adds 125*3 subleds to write the whole keyboard in rainbow colors with a single 
echo to multi_intensity.

The keyboard also supports hardware color effects like color cycle, which 
continuously and smoothly cycles through up to 7 colors. Could this also be 
implemented with a trigger? That trigger would need to add a new entry nr_colors 
and also respectively additional subleds or additional multi_intensity_* entries.

An additional though I had was that it would be nice if the driver could somehow 
communicate the physical location of the key to the userspace for UIs to 
automatically generate a keyboard view to graphically set individual colors.

Kind regards,

Werner Sembach