KMS backlight ABI proposition

[Martin Peres]

On 24/02/17 11:59, Hans de Goede wrote:
> Hi,
>
> On 24-02-17 10:48, Hans de Goede wrote:
>> Hi,
>>
>> On 24-02-17 10:46, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 24-02-17 10:34, Jani Nikula wrote:
>>>> On Fri, 24 Feb 2017, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>> On 24-02-17 09:43, Jani Nikula wrote:
>>>>>> I don't think we have any good ideas how to solve the property max
>>>>>> changing on the fly. But based on the discussion so far, it's
>>>>>> starting
>>>>>> to look like we'll need to study that more thoroughly.
>>>>>
>>>>> As I mentioned in another part of the thread, I think we can just
>>>>> return
>>>>> -EBUSY if udev tries to change the binding when a drm-node is open
>>>>> *and* the backlight/brightness property has been accessed (even if
>>>>> read only) through that drm-node. We can reset the busy flag when
>>>>> the (last open) drm-node gets closed.
>>>>
>>>> That's certainly easier than coming up with ways to notify the
>>>> userspace
>>>> about property range changes. The obvious downside is that you have to
>>>> kill X to do the re-association. That's fine for when everything just
>>>> works (i.e. everything happens before the drm node is opened), but for
>>>> debugging it's a bit painful. Can we live with that?
>>>
>>> Sure, debugging udev rules is somewhat tricky in general (requires
>>> manually triggering udev). When asking users to test udev rule / hwdb
>>> patches I usually just ask them to reboot.
>>>
>>>>> I'm adding the *and* the backlight/brightness property has been
>>>>> accessed
>>>>> condition so that udev can still do its thing while a boot splash
>>>>> screen is active. This assumes that boot splash screens do not
>>>>> touch the brightness.
>>>>
>>>> I presume udev will have to do its job before the drm node is opened, I
>>>> think relying on the userspace not touching the brightness property is
>>>> going to be racy/flaky.
>>>
>>> Making sure that udev does its job before we show the splashscreen
>>> is tricky, note the idea is that the splashscreen *never* touches
>>> the brightness property and by the time regular X / wayland launches
>>> udev should have had plenty of time to complete its job.
>>>
>>> I do not know how hard it will be to add the code to detect triggering
>>> the property being touched, but that is the best I can come up with
>>> to still allow the bootsplash (e.g. plymouth) to use the drm-node.
>>
>> p.s.
>>
>> I realize this aint pretty, alternatively we could just document that
>> root may change the back-end of the property and that in that case
>> the max value may change and that it is up to userspace to make sure
>> that it deals with this (e.g. by not changing the backend at a bad
>> time).
>
> Sorry for all the mails, I just realized that this (making it userspace's
> problem entirely) is exactly what the input subsystem does. Things like
> setkeycodes, but esp. also the fixing of min/max value for absolute axis
> for touchpads are done by /lib/udev/hwdb.d/60-evdev.hwdb and the
> unwritten rule there is that udev needs to do this before wayland or
> xorg start using the touchpad and queries the min/max values (which it
> does once and then caches the results).
>
> I just checked the implementation of the EVIOCSABS ioctl and it does
> not check that the device is not in use while the changes are applied.
>
> So I think the best solution here is to just document that changing the
> backend and thus the max value while it is being used is not the best
> idea, but is allowed by the kernel.

What's wrong with sending a hotplug event upon changing the backlight 
driver? This would work even when X is running!

Martin