Hi, On 9/19/23 19:46, Julius Zint wrote: > > > On Wed, 6 Sep 2023, Hans de Goede wrote: > >> Hi Julius, >> >> On 9/4/23 21:02, Julius Zint wrote: >>> >>> >>> On Mon, 4 Sep 2023, Thomas Weißschuh wrote: >>> >>>> +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@xxxxxxxxxx/ >>>> >>> >>> 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@xxxxxxxxxx/ >> >> 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. >> >>> 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@xxxxxxxxxx/ >> >> Regards, >> >> Hans >> > > Thank you for all this additional information. I have watched the talks > and read up upon the mail threads you`ve linked. Now I wonder which presentation you have watched, did you watch the old XDC2014 presentation ? Note I gave a much more up2date presentation on this at kernel-recipes last year: https://kernel-recipes.org/en/2022/talks/new-userspace-api-for-display-panel-brightness-control/ > I will see if I can make the mapping to the DRM connector and plan to > update this patchset. Sounds good. Regards, Hans
