KMS backlight ABI proposition

Mon Feb 20 20:09:11 UTC 2017

Hi All,

On 17-02-17 13:58, Martin Peres wrote:
> Hey everyone,
>
> We have been working towards exposing the backlight as a KMS property instead of relying on the backlight drivers. We have CC:ed the people we have found to be the more likely to be interested in the discussion but please add everyone you think would have some experience with this issue.
>
> == Introduction ==
>
> We are trying to bring the same level of support for the backlight on both the xf86-video-intel and -modesetting DDX.
>
> Looking into the situation of the backlight, we identified these problems which are almost show-stoppers for -modesetting and wayland compositors:
>
>  - There is no mapping between the backlight driver and DRM-connectors. This means that, in case there are multiple backlight drivers, the userspace has to have knowledge of the machine to know which driver should be used. See the priority list for the intel driver [0].
>
>  - The luminance curve of the backlight drivers is not specified, which can lead to a bad user experience: Little changes in the highest levels but drastic changes in the low levels.
>
>  - Writing to the backlight driver still requires root rights. Given that the xserver and wayland compositors are now running root-less, this means we would need a complex dance involving a setuid helper [1].
>
> Hans de Goede has already given a presentation about these issues at XDC2014. The slides are a good read[2].
>
> [0] https://cgit.freedesktop.org/xorg/driver/xf86-video-intel/tree/src/backlight.c#n259
>
> [1] https://cgit.freedesktop.org/xorg/driver/xf86-video-intel/tree/src/backlight.c#n348
>
> [2] https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/backlight.pdf
>
> == Proposal ==
>
> Since David Hermann already worked on this and proposed what I consider being greats foundations for building towards a solution addressing the issues above, I will just ask you to read his original words:
>
> https://lists.freedesktop.org/archives/dri-devel/2014-September/067984.html
>
> == Open issues ==
>
> Here are the open issues we have identified with the solution proposed by David:
>
>   1) Backlight device interoperability: How far should we support
>      mixing the backlight device and brightness property? Should it be
>      unidirectional or bi-directional? What about the start-up value
>      exposed by the brightness property?
>
>   2) How many steps should be exposed: fixed or driver-dependent?
>
>   3) Expected output curve: power? luminance? Simply monotonically
>      increasing?
>
>   4) Should the userspace be able to turn off the backlight? If so, how
>      should it do it? What can we do to let the userspace distinguish
>      between backlight off or on?
>
>   5) Should we expose to the userspace what is the current backlight
>      power?
>
> Here is our current point of view on the matter:
>
> === 1) Backlight device interoperability ===
>
> Since we need to keep backward compatibility of the backlight, we have to keep the current backlight drivers.
>
> Here are possible options:
>
>  - Exclusive access: Unregister a backlight device when the drm brightness property is requested/used;
>  - Unidirectional access: When writing to the backlight property, update the backlight device;
>  - Bi-directional access: Propagate back changes from the backlight device to the property's value.
>
> Being bi-directional would be of course the best, but this requires that both drivers have the same number of steps, otherwise, we may write a value to the property, but get another one when reading it right after, due to the non-bijective nature of the transformation.
>
> Uni-directional would work in all cases, with the caveat that mixing calls to the KMS property and the backlight device will not be supported (changes mades through the sysfs interface of the backlight driver will not be reflected in the KMS property). At boot time, we should however initialize the value of the backlight property with a value close to what is currently set in the backlight driver.
>
> Giving exclusive access does not sound very good to me, as it would be hard for the userspace to deal with disappearing drivers...
>
> === 2) How many steps should be exposed ===
>
> If the KMS property exposes the same number of steps as the backlight driver, it allows us to get a bijective function between the two interfaces, and allow a bi-directional communication. The downside of this is that it forces the userspace to deal with a variable number of steps which can range from 4 to 1k+. Also, the userspace would be able to handle the case where there are less steps than it would like to expose.
>
> If the KMS property exposes a fixed number of steps (say 100), it becomes easy for the userspace to express the wanted brightness. However, on drivers exposing less than these 100 steps, we cannot guarantee that any change in the value will produce any change. If there is only one possible value (on or off), the user may be trying the change the brightness, a GUI would show what is the expected backlight state, but no change in the luminance would be seen, which is pretty bad.

So 1 and 2 are closely related the problem is that if we expose a fixed number of steps
we need to convert in both directions, and if userspace tries to increment by doing read,
add 1, write and we expose 1-100 but the hardware has only 4 levels then the read will
keep returning the same value. Note I've fixed bugs like this already so this is a real
problem. As already mentioned in the thread this can be fixed by caching the last written
value on both sides and invalidating both caches on a new write to one side.

> === 3) and 4) ===
>
> These issues are not handled at all by the backlight device sysfs interface.
>
> But since David already had to add an in-kernel interface to access the backlight devices [0], we could add capabilities to the drivers while keeping the backward compatibility.
>
> From the in-kernel interface, it is already possible to turn on and off the backlight for sure (when supported, but this is also reported properly). However, what is not supported is to know what the value 0 means (lowest setting possible but not turned off, or no power at all).
>
> It was brought up that we could simply not allow the backlight to be turned off, and just request DPMS to reach this state. However, I do not think it is a good idea as some panels (like the one from the OLPC) switch to e-paper mode when the backlight is set to 0 and are perfectly readable.
>
> I would suggest we design an interface that will allow good drivers to expose as many features as possible, but yet gracefully degrade if information is not present.
>
> Over time, drivers will improve to expose information about the platform, and the user experience will improve as a result.
>
> [0] https://lists.freedesktop.org/archives/dri-devel/2014-September/067987.html
>
> ===  5) Exposing the current backlight power?  ===
>
> The backlight_current interface in the backlight devices is meant to expose the currently-used backlight value, regardless of the wanted value that should be used when the backlight is not off.
>
> My current stance on this is that this should not be needed. The userspace should describe the intent of the user (wanted backlight level) and trust the KMS property to turn off the backlight when entering DPMS.
>
> == Current KMS ABI proposal ==
>
> The current ABI proposal has mostly been proposed by Jani Nikula, as a result of his experience and our discussions.
>
> It takes the following approach:
>
>  - Fixed number of steps (I think we should change it to expose the same number of steps)
>  - Uni-directional: KMS -> backlight
>  - Do not deal yet with 3) and 4): I have ideas, but I have been procrastinating long-enough to send this email and we already have much to discuss!
>  - Does not expose the current backlight power as we want to let the kernel deal with DPMS on its own
>
> === ABI proposal ===
>
> The brightness property MUST have values 0...100 inclusive.
>
> The display brightness MUST be a monotonically increasing function of
> the brightness property.

What does "monotonically increasing" actually mean ? I would prefer
to clearly define that we are talking about perceived brightness here,
for 2 reasons:

1) This is what the user actually wants
2) Some of the x86 firmware interfaces only give us perceived brightness
and no way to get back to any other unit of measire.

> Brightness property value 1 MUST mean the minimum supported visible
> brightness.
>
> Brightness property value 100 MUST mean the maximum supported
> brightness.
>
> Brightness property value 0 SHOULD mean backlight off or equivalent for
> non-backlight brightness adjustment, typically completely
> black. Brightness property value 0 MUST NOT switch the display or pipe
> off [1].
>
> If the hardware is not capable of supporting zero brightness, and the
> driver knows this, value 0 MUST be equal to value 1.

Ok.

> If the driver does not know whether the hardware is capable of
> supporting zero brightness, the driver SHOULD err on the side of 0 not
> being off rather than 1 meaning off. In this case, value 0 is likely
> different from value 1, and the minimum brightness can only be reached
> via property value 0 [2].

I agree, but this needs rewording (I had to read it 3 times).

> If the brightness gets changed outside of the property interface,
> reading the property value MAY be out of sync with the actual brightness
> [3].

This is for when the panel is off ? Otherwise it seems like a bad idea
to me.

> [1] Must be able to support displays which are visible even with the
> backlight switched off.
>
> [2] The main downside corner case with this is that if the driver
> doesn't know whether it can switch off the backlight, 0 might end up
> meaning the minimum visible, and 1 is the second lowest visible, and
> with a userspace that avoids black display, the user can't use the
> lowest brightness setting.
>
> [3] This is not unlike the "brightness" property in the backlight class
> sysfs interface. The intention is that the drm interface does not have
> an equivalent of "actual_brightness".

Regards,

Hans