On Wed, 6 Sep 2023, Hans de Goede wrote:
Hi Julius,
On 9/4/23 21:02, Julius Zint wrote:
quoted
On Mon, 4 Sep 2023, Thomas Weißschuh wrote:
quoted
+Cc Hans who ins involved with the backlight subsystem
Hi Julius,
today I stumbled upon a mail from Hans [0], which explains that the
backlight subsystem is not actually a good fit (yet?) for external
displays.
It seems a new API is in the works that would better fit, but I'm not
sure about the state of this API. Maybe Hans can clarify.
This also ties back to my review question how userspace can figure out
to which display a backlight devices applies. So far it can not.
[0] https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/ (local)
Hi Thomas,
thanks for the hint. I will make sure to give this a proper read and
see, if it fits my use case better then the current backlight subsystem.
Note the actual proposal for the new usespace API for display brightness
control is here:
https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com/ (local)
I have finished / stabilized the backlight code refactor which I landed
in 6.1, which is a prerequisite for the above proposal. But I have not
been able to make time to actually implement the above proposal; and
I don't know when I will be able to make time for this.
quoted
Especially since I wasnt able to properly address your other review
comments for now. You are right that the name should align better with
the kernel module and also, that it is possible for multiple displays to
be attached.
In its current state, this would mean that you could only control the
backlight for the first HID device (enough for me :-).
The systemd-backlight@.service uses not only the file name, but also the
full bus path for storing/restoring backlights. I did not yet get around
to see how the desktops handle brightness control, but since the
systemd-backlight@.service already uses the name, its important to stay
the same over multiple boots.
I would be able to get a handle on the underlying USB device and use the
serial to uniquely (and persistently) name the backlight. But it does
feel hacky doing it this way.
So mutter (gnome-shell compositor library) has a similar issue when saving
monitor layouts and I can tell you beforehand that monitor serial numbers
by themselves are not unique enough. Some models just report 123456789
as serial and if you have a dual-monitor setup with 2 such monitors
and name the backlight class device <serial>-vcp-hid or something like that
you will still end up with 2 identical names.
To avoid this when saving monitor layouts mutter saves both the port
to which the monitor is attached (e.g. DP-1 DP-2) and the serialnumber
and on startup / monitor hotplug when it checks to see if it has saved
layout info for the monitor it checks the port+serialnr combination.
So what I think you should do is figure out a way to map which
VCP HID device maps to which drm-connector and then use
the connector-name + serial-nr to generate the backlight device name.
We will need the mapping the a drm-connector object anyway for
the new brightness API proposal from above.
Note this does NOT solve the fact that registering a new backlight
class device for an external monitor on a laptop will hopelessly
confuse userspace, see:
https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfdfb6@redhat.com/ (local)
Regards,
Hans
Thank you for all this additional information. I have watched the talks
and read up upon the mail threads you`ve linked.
I will see if I can make the mapping to the DRM connector and plan to
update this patchset.
Thanks,
Julius