KMS backlight ABI proposition

[Hans de Goede]

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 ?

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

Regards,

Hans