Re: KMS backlight ABI proposition

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 20 Feb 2017, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>> === 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.
>
> I don't accept that bi-directional transfer requires the step range to 
> be the same. Isn't all that is required is acceptance that both sides 
> maintain a copy of the current value in their own number range and that 
> if X is written to then Y may change value (i.e. when mapping between 
> 0..100 and 0..10 then if 0..100 is at 11 and 0..10 gets 1 written then 
> 0..100 is allowed to change to 10).
>
> I'd note also that the mechanisms inside backlight to support 
> sysfs_notify would mean *implementing* bi-directional comms isn't too 
> bloated even if the two sides used different number ranges.

I question the need and usefulness of bi-directional access, and I
question it being "the best". The end goal is to use the connector
property exclusively, and deprecate the sysfs API. If you choose to use
the connector property, you should steer clear from the sysfs. That's
part of the deal here.

The sysfs will still work as ever, it won't break or regress or go away
anytime soon, but the ABI and contract for the connector property will
be, "if you touch the sysfs while using the connector property, you
might get unexpected results reading back the property".

There *are* going to be subtle bugs with the simultaneous operation, and
I know I don't want to be in the receiving end of those bugs.

Raise your hands, who wants to deal with them? Who thinks it's worth it?

>>> 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)
>
> Fixing a large number of steps over an inflexible (lets say 8 level)
> backlight device creates a new problem. User actions to
> increase/decrease the backlight don't work unless the userspace knows
> the hardware step size...

Many of the ACPI backlight interfaces have a limited number of steps,
such as 8 or 16.

However, at least for i915 native backlight, we might *theoretically*
have, say, 5000 steps. But we might have no clue how many user
perceivable distinct steps there are.

> The 0..100 proposal below will encourage the userspace to implement 
> hotkeys that jump by 9 (because 0 is reserved with a special meaning). 
> and thus there will be deadspots where the hot key has no effect.

One brainstormed idea was to provide a way to increase/decrease the
brightness by a user perceivable margin or N%, whichever is the bigger
change. I don't think we explored that in depth, or how feasible that is
with the properties. It might not solve everything, but it could solve
one class of problems with expanding a limited hardware range to 0..100.

>>>  - Uni-directional: KMS -> backlight
>
> See above.
>
>
>>>  - 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!
>
> Do any of those ideas involve adding *new* API to provide information to 
> userspace to help it correct the curves (e.g. somewhat like ALSA)?
>
> It's not that I object to such an approach but I consider it pointless 
> to present fixed range brightness levels if the userspace were to end up 
> responsible for curve correction.

One of the ideas we've discussed is having a property to adjust the
curve in kernel. If the driver knows parameters of the backlight, it
could populate the curve with the information it has, but it would allow
the userspace to adjust or replace it. The idea is that the userspace
could then treat the brightness property as linear wrt perceived
brightness. ("Perceived brightness" is kind of vague too, but let's not
go there just yet.)

>>>  - 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.
>
> I'm somewhat unconvinced by re-ranging the hardware capability but if 
> this is the way we want to go perhaps consider -1..100 as the range. 
> There's a risk of bikeshedding here but -1 is a more obvious "special" 
> value and it offers more flexibility for natural hotkey strides.

There some benefits for "re-ranging" the hardware range:

* It makes sense for hardware ranges that have far more steps than can
  be perceived. Why expose 5000 steps when you can perceive, say, a
  couple of hundred levels, if that. And the userspace will only use
  maybe ten steps.

* Some PWM based backlight allow adjusting the PWM modulation
  frequency. It could be done on the fly. It would be awkward to change
  the max on the fly; not sure if it's even possible for properties.

* There's the idea of letting userspace re-associate the brightness
  properties with the underlying hardware. The max might change. See
  previous point. Any solution must address this.

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.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel