Re: KMS backlight ABI proposition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 24-02-17 09:43, Jani Nikula wrote:
On Thu, 23 Feb 2017, Stéphane Marchesin <stephane.marchesin@xxxxxxxxx> wrote:
On Thu, Feb 23, 2017 at 12:40 AM, Jani Nikula
<jani.nikula@xxxxxxxxxxxxxxx> wrote:
On Wed, 22 Feb 2017, Stéphane Marchesin <stephane.marchesin@xxxxxxxxx> wrote:
On Fri, Feb 17, 2017 at 4:58 AM, Martin Peres
<martin.peres@xxxxxxxxxxxxxxx> 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.

Fair enough. But using, say, 1000 steps is excessive even for that
because you can't distinguish the steps from each other.

- "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 modulation frequency is usually an OEM design choice. The higher the
frequency, less flicker, but also fewer user distinguishable levels of
backlight (signal rise/fall times come into play). Occasionally there
have been requests to be able to adjust the frequency. We can of course
decide that's an implementation detail and not let userspace change it.

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

I guess I wasn't clear enough. If the property max reflects the max of
the underlying backlight class implementation, and userspace
re-associates the property with *another* backlight class implementation
(because the kernel might not always get the association right), it's
likely that the max will not be the same. This re-association was one of
David's original key ideas, so udev could carry the quirks, and
users/developers could use it for debugging.

I don't think we have any good ideas how to solve the property max
changing on the fly. But based on the discussion so far, it's starting
to look like we'll need to study that more thoroughly.

As I mentioned in another part of the thread, I think we can just return
-EBUSY if udev tries to change the binding when a drm-node is open
*and* the backlight/brightness property has been accessed (even if
read only) through that drm-node. We can reset the busy flag when
the (last open) drm-node gets closed.

I'm adding the *and* the backlight/brightness property has been accessed
condition so that udev can still do its thing while a boot splash
screen is active. This assumes that boot splash screens do not
touch the brightness.

Regards,

Hans





(Thanks for requiring the rationale from me like I asked of
you. Really. We can't figure this out otherwise.)

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@xxxxxxxxx

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.

I suppose you can get away with that because you have control over the
overall product and the userspace, and you can ensure this works. What
would you do if the hardware or the kernel driver didn't support
switching off the backlight? I also presume you'd always know if and
when that's the case; in the generic case that's not always possible.

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.

Agreed.

BR,
Jani.


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux