KMS backlight ABI proposition

[Stéphane Marchesin]

On Thu, Feb 23, 2017 at 12:40 AM, Jani Nikula
<jani.nikula at linux.intel.com> wrote:
> On Wed, 22 Feb 2017, Stéphane Marchesin <stephane.marchesin at gmail.com> wrote:
>> On Fri, Feb 17, 2017 at 4:58 AM, Martin Peres
>> <martin.peres at linux.intel.com> wrote:
>>> 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.
>>
>> Yes, I don't think we want to normalize anything here. It would
>> essentially be hiding functionality from user space, which then can't
>> expose it in the user interface. As you say, if the backlight slider
>> moves, but the backlight level didn't change, that's weird. On the
>> other hand if user space knows the number of levels it can give you a
>> consistent slider, and normalizing in user space is not that hard
>> (that's how things currently work after all, so people should be used
>> to it).
>
> I listed some of the benefits of normalizing (or re-ranging) in
> [1]. Conversely, I haven't seen good answers on how to gracefully handle
> the brightness range changing on the fly. That is what not normalizing
> would mean. I don't think the current property implementation even
> allows changing the range. And then there'd have to be a way to tell the
> userspace that the range has changed.


Let me reply to your points:
- "And the userspace will only use maybe ten steps." That isn't true,
we use all the steps that are available to do smooth transitions in
Chrome OS.
- "Some PWM based backlight allow adjusting the PWM modulation
frequency." you don't need a motivation for *why* I would want to
change the mod freq on the fly, actually in my experience you
shouldn't since this can lead to flickery backlights.
- "The max might change" again you don't say why except that you want
to change the mod freq. Basically point 3 is like point 2.


>
> In the same message, I mentioned the idea of providing an API to
> increase/decrease brightness. That might be much easier to implement
> than allowing the property range change.
>
> [1] http://marc.info/?i=87mvdei7ug.fsf@intel.com
>
>> Yes the ability to turn off the backlight is important. Some
>> backlights are not stable at low levels, so they don't expose these
>> low levels and effectively level 0 is not off (it is the lowest level
>> which works). So I guess the question is how should that non-linearity
>> be exposed versus the ability to turn it off completely.
>
> You fail to say *why* the ability to turn off the backlight is
> important. I've seen it used as a kind of "light DPMS" that can be done
> using the sysfs interface, but I think that's a hack, really. Here,
> whoever changes the backlight would be doing it using the DRM APIs
> anyway, so it could do actual DPMS anyway. And, of course, not all
> backlight hardware is able to switch off the backlight, and not all
> drivers will be able to say whether 0 is off or not.

Turning the on/off the backlight is much quicker than turning on/off
the display through DPMS. So one thing we do is use that to turn a
screen off/on very quickly.



>
>>> 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.
>>
>> Are we saying that we are putting the kernel in charge of  display vs
>> backlight sequencing? Currently on some ARM boards with separate pwm
>> backlight drivers that's not the case. Don't get me wrong, I think the
>> kernel should be in charge of enforcing sequencing because otherwise
>> user space can damage hardware, I'm just pointing out that right now
>> it isn't the case.
>
> Whenever the kernel is able to enforce the sequencing, it should. I


It probably shouldn't be just "it should". If user space can damage
the hw, then the kernel is broken.

Stéphane


> believe this is the case for most native backlight implementations. And
> in these cases the backlight on/off toggling would really have to be a
> substate of enabled display; can't enable backlight without display
> enabled.
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Technology Center