KMS backlight ABI proposition

[Hans de Goede]

Hi,

On 23-02-17 09:55, Jani Nikula wrote:
> On Wed, 22 Feb 2017, Hans de Goede <hdegoede at redhat.com> wrote:
>> My first thought was that your proposal is reasonable, but on second
>> thought I foresee trouble here with e.g. the backlight level save / restore
>> code in systemd still using the sysfs interface, while the desktop
>> environment has moved on to the property, I believe we really need to
>> do some sort of bi-directional syncing here ...
>
> FWIW I think systemd should have no business changing the backlight. The
> save/restore should belong in the desktop environments... But I think we
> could dodge this one by having the initial sysfs value synced back to
> the property on initialization, but not otherwise. The systemd stuff,
> IIRC, happens before/after X.

We would also need to sync from the property back to sysfs when the drm-node
gets closed.

>>> We can bikeshed the meaning of 0 or -1, I don't mind. The point is, we
>>> need to define what the drivers should aim for, with the potentially
>>> limited information they have available, to provide as smooth and
>>> unified an experience as possible.
>>>
>>> One benefit of -1 is that we might get away with adding that as a
>>> special case later on, if we define 0 properly. And if the drivers know
>>> they don't support off, they could have range 0..100 instead.
>>
>> I really believe that we need to define the ABI as 0 meaning minimal
>> brightness which keeps the screen readable (which for the epaper
>> example would be no brightness, but normally would be some minimal
>> level).
>
> We can define anything, but it doesn't mean it makes sense for the
> drivers or that the drivers can implement it. By your above definition,
> the right thing for the driver to do in the epaper case is to switch off
> the backlight at 0, because switching it off completely can save more
> power than just setting duty cycle to 0. That's the whole point for
> epaper.

Correct.

> And this contradicts with what you say below about backlight
> on/off being orthogonal.

Not really this would just mean that for epaper as a special
case dpms on and backlight level 0 would result in the backlight
getting turned off as that still keeps the screen readable, this
can all be dealt with in the kernel without userspace needing to
know.

>> Yes we do not get this right in some cases, but let at least define
>> it properly in the ABI. Add a fat disclaimer for all I care that
>> in some cases the driver is unable to guarantee this, but lets
>> clearly define what 0 means and then try to get as much drivers
>> to adhere to that as possible.
>>
>> As for -1 meaning turning stuff off, I'm against that, on/off
>> vs brightness setting really are 2 almost orthogonal controls,
>> in many cases in hardware they are truely orthogona, with both
>> an enable pin as well as a pwm input for the brightness level,
>> and driving the enable low will get the pwm input to effectively
>> be ignored.
>
> I think the crux with the on/off/minimum etc. is to have an ABI that
> works sensibly also for drivers/hardware that is not able to switch
> backlight off,

Not being able to turn backlight off is covered by my proposal,
since 0 would keep the backlight on for a lcd panel.

> or doesn't know whether the minimum is off or not.

As I said in this case, I'm fine with adding a disclaimer for this
case to the ABI specification for the new brightness property I just
want the main text to clearly define how the driver should behave if
it does know. This also means we may add quirks for (some) models
where the VBT get things wrong and actually override the minimum
pwm from the VBT with a sensible value which actually works on the model
in question. Maybe even allow the interface to bind the backlight to
also provide a (hidden from userspace) minimum value to use when
the property is set to 0.

> And we need to have recommendations for drivers on what to do in the
 > imperfect reality.

I would say they need to do a best effort to make 0 be minimum brightness
and not off, like the i915 driver is doing now, where it tries to make 0
be minimum brightness by reading info from the VBT and if that info is
borked 0 often becomes off.

Regards,

Hans