Message-ID: <Zv0YI3qIEg88Dx4c@duo.ucw.cz>
Date: Wed, 2 Oct 2024 11:53:39 +0200
From: Pavel Machek <pavel@....cz>
To: Benjamin Tissoires <bentiss@...nel.org>
Cc: Werner Sembach <wse@...edocomputers.com>, Armin Wolf <W_Armin@....de>,
	Hans de Goede <hdegoede@...hat.com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	dri-devel@...ts.freedesktop.org, jelle@...aa.nl, jikos@...nel.org,
	lee@...nel.org, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
	miguel.ojeda.sandonis@...il.com, ojeda@...nel.org,
	onitake@...il.com, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH 1/1] platform/x86/tuxedo: Add virtual LampArray for
 TUXEDO NB04 devices

On Wed 2024-10-02 10:13:10, Benjamin Tissoires wrote:
> On Oct 01 2024, Pavel Machek wrote:
> > Hi!
> > 
> > > PPS: sorry for pushing that hard on HID-BPF, but I can see that it fits
> > > all of the requirements here:
> > > - need to be dynamic
> > > - still unsure of the userspace implementation, meaning that userspace
> > >   might do something wrong, which might require kernel changes
> > > - possibility to extend later the kernel API
> > > - lots of fun :)
> > 
> > Please don't do this.
> > 
> > We have real drivers for framebuffers. We don't make them emulate
> > USB-display devices.
> 
> Except that they are not framebuffer for multiple reasons. I know you
> disagree, but the DRM maintainers gave a strong NACK already for a

Person not linking the DRM stuff was not a maintainer.

> DRM-like interface, and the auxdisplay would need some sort of tweaking
> that is going too far IMO (I'll explain below why I believe this).



> > Yes, this is a small display, and somewhat unusual with weird pixel
> > positions, but it is common enough that we should have real driver for
> > that, with real API.
> 
> It's not just weird pixel positions. It's also weird shapes. How are you
> going to fit the space bar in a grid, unless you start saying that it
> spans accross several pixels. But then users will want to address
> individual "grouped" pixels, and you'll end up with a weird API. The
> Enter key on non US layouts is also a problem: is it 1 or 2 pixels wide?
> Is is 2 pixel in heights?

Have you seen one of those keyboards?

(Hint: it is LEDs below regular keyboard.)

> The positions of the pixels also depend on the physical layout of the
> keyboard itself. So with the same vendor ID/Product ID, you might have
> different pixel positions if the device is sold in Europe, or in the
> US.

If vendor sells different hardware with same IDs, well 1) that's a
nono, a 2) that's what kernel parameters are for.

> It's also luminance problem IIRC. Some keys might have a different range
> of luminance than others. I remember Bastien Nocera telling me we

Have you seen one of those keyboards?

> But that's just the "easy" part. We can define a kernel API, for sure,
> but then we need users. And there are several problems here:
> 
> - first, users of this new kernel API need to be root to address the
>   LEDs. They probably won't, so they'll rely on a third party daemon for
>   that, or just use uaccess (yay!). But that part is easy

Eventually, desktop environment should talk the interface. (Plus, how
does HID or BPF craziness help with his?)

> - then, user sees a new kernel interface, and they have to implement it.
>   OK, fair enough, but more code to maintain

Yep. At least there's single interface to talk to.

> - but that new kernel API doesn't give enough information: all you have
>   is an approximation of the keyboard layout, which will not match
>   the

Have you seen OpenRGB? It already aproximates keyboard as a grid. Or
maybe we give them enough information.

Below you were just inventing problems.

> - but then, even if you make everyones happy, the GUI project is
>   actually cross-platform (OpenRGB is, Steam is, SDL is). And what is
>   done on Windows is simple: raw access to the HID device. And the
>   raw

Yes, Windows is a mess. We don't want to emulate them.

> I've been through this exact same process with Input and game
> controllers, and even for libratbag for configuring gaming devices. In
> the end, the kernel developer never wins, but the userspace

Yes, we have been in this exact situation. Userland was directly
accessing mice. It was called "gpm" and we moved away from that for
good reasons.

> If you want a 100 lines of code program to control your keyboard, with
> LampArray, you can, as long as you don't require a GUI and don't require
> to be generic. Just write the values directly on the hidraw device,
> and

Haha, no. Kernel part was 400+ lines, no way you can parse that in 100
lines.

> You might agree with me or not, but this is actually not relevant to the
> discussion here IMO: all what Werner is doing (with crazy arrays) is to
> take a proprietary protocol and convert into a HID standard. He is

Yes, we should never have had input subsystem. We should simply
convert all mice to PS/2 standard protocol. ... And yes, we have that,
that's /dev/mice, yet input layer is very useful.

What is relevant that these crazy arrays are not going to be merged,
and better solution is needed.

> I'm also under the impression that you are scared by BPF. BPF is just a
> tool here to "fix" the device with an easy path forward. BPF is
> safer

I should not need to run just-in-time compiler to get support for my
hardware. If you are not scared by BPF, take a look at modern CPU
design, with emphasis on speculation vulnerabilities such as Spectre
and Meltdown.


Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)

Powered by blists - more mailing lists