KMS backlight ABI proposition

[Martin Peres]

On 24/02/17 12:44, Hans de Goede wrote:
> Hi,
>
> On 24-02-17 11:23, Martin Peres wrote:
>> 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!
>
> Propagating that is going to be tricky, does randr even allow changing
> the range of a property after it has been registered ?

It does, unless the property was set immutable.

>
> With that said, yes sending a change uevent when this changes probably
> is a good idea regardless.

I think that solves the problem pretty nicely, so yes :)
Will wait until monday to send an updated email with the new proposal.

Dave, I would really like to hear more about your thoughts on this. Did 
we explain sufficiently well why we think pushing work onto the 
userspace will not work?

Martin