KMS backlight ABI proposition

[Martin Peres]

On 22/02/17 17:05, Jani Nikula wrote:
> On Mon, 20 Feb 2017, Daniel Thompson <daniel.thompson at 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.)

One way would be to just expose a table containing trip points with
known (raw_value, luminance %) points. The userspace may just
allow changing between them or may want to interpolate if it needs
more trip points. This way, we don't have to deal with any curves in
the kernel space.

If this table were user-writable, this would allow people to calibrate
their monitor, as wanted (using a colourhug, or whatever).

>
>>>>   - 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.

Ah, right! I had forgotten about this!

What I said was that since it is a rare event, we could issue
a hotplug event to work around this issue. The userspace would
be forced to re-probe.

>
> 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.

Yeah, -1 is a good idea.

>
>
> BR,
> Jani.
>
>