[RFC v2] drm/kms: control display brightness through drm_connector properties

[Sebastian Wick]

On Fri, Sep 30, 2022 at 9:40 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:
>
> On Thu, 29 Sep 2022 20:06:50 +0200
> Sebastian Wick <sebastian.wick at redhat.com> wrote:
>
> > If it is supposed to be a non-linear luminance curve, which one is it?
> > It would be much clearer if user space can control linear luminance
> > and use whatever definition of perceived brightness it wants. The
> > obvious downside of it is that it requires bits to encode changes that
> > users can't perceive. What about backlights which only have a few
> > predefined luminance levels? How would user space differentiate
> > between the continuous and discrete backlight? What about
> > self-emitting displays? They usually increase the dynamic range when
> > they increase in brightness because the black level doesn't rise. They
> > also probably employ some tonemapping to adjust for that. What about
> > the range of the backlight? What about the absolute luminance of the
> > backlight? I want to know about that all in user space.
> >
> > I understand that most of the time the kernel doesn't have answers to
> > those questions right now but the API should account for all of that.
>
> Hi,
>
> if the API accounts for all that, and the kernel doesn't know, then how
> can the API not lie? If the API sometimes lies, how could we ever trust
> it at all?

Make it possible for the API to say "I don't know". I'd much rather
have an API tell me explicitly what it does and doesn't know instead
of having to guess what data I can actually rely on.

For example if the kernel knows the luminance is linear on one display
and doesn't know anything about the other display and it exposes them
both in the same way I can not possibly write any code which relies on
exact control over the luminance for either display.

>
> Personally I have the feeling that if we can even get to the level of
> "each step in the value is a more or less perceivable change", that
> would be good enough. Think of UI, e.g. hotkeys to change brightness.
> You'd expect almost every press to change it a bit.

The nice thing is that you can have that even if you have no further
information about the brightness control and it might be good enough
for some use cases but it isn't for others.

> If an end user wants defined and controlled luminance, I'd suggest they
> need to profile (physically measure) the response of the display at
> hand. This is no different from color profiling displays, but you need
> a measurement device that produces absolute measurements if absolute
> control is what they want.

If that's the kind of user experience you're after, good for you. I
certainly want things to work out of the box which makes this just a
big no-go.

>
> If there ever becomes an industry standard and conformance test
> definitions for luminance levels and backlight control, then things
> could be different. But until that, I believe trying to make one in the
> kernel is futile, because I have got the impression that there is
> practically no consistency between different displays in general.

I'm aware that this is the current situation but it's one that must
change and we should at least try to create an API which still works
when we get more and better data.

>
> Besides, I would expect some backlights to wear over time, grow dimmer
> for the same input value. Without a physical active feedback loop
> (measurements), it just won't work.
>
> If this is mostly for laptop displays, would end users even care?
>
>
> Thanks,
> pq
>
> > On Wed, Sep 28, 2022 at 1:14 PM Ville Syrjälä
> > <ville.syrjala at linux.intel.com> wrote:
> > >
> > > On Wed, Sep 28, 2022 at 01:57:18PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Sep 28, 2022 at 01:04:01PM +0300, Jani Nikula wrote:
> > > > > On Fri, 09 Sep 2022, Hans de Goede <hdegoede at redhat.com> wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > Here is v2 of my "drm/kms: control display brightness through drm_connector properties" RFC:
>
> ...
>
> > > > > > Unlike the /sys/class/backlight/foo/brightness this brightness property
> > > > > > has a clear definition for the value 0. The kernel must ensure that 0
> > > > > > means minimum brightness (so 0 should _never_ turn the backlight off).
> > > > > > If necessary the kernel must enforce a minimum value by adding
> > > > > > an offset to the value seen in the property to ensure this behavior.
> > > > > >
> > > > > > For example if necessary the driver must clamp 0-255 to 10-255, which then
> > > > > > becomes 0-245 on the brightness property, adding 10 internally to writes
> > > > > > done to the brightness property. This adding of an extra offset when
> > > > > > necessary must only be done on the brightness property,
> > > > > > the /sys/class/backlight interface should be left unchanged to not break
> > > > > > userspace which may rely on 0 = off on some systems.
> > > > > >
> > > > > > Note amdgpu already does something like this even for /sys/class/backlight,
> > > > > > see the use of AMDGPU_DM_DEFAULT_MIN_BACKLIGHT in amdgpu.
> > > > > >
> > > > > > Also whenever possible the kernel must ensure that the brightness range
> > > > > > is in perceived brightness, but this cannot always be guaranteed.
> > > > >
> > > > > Do you mean every step should be a visible change?
> > > >
> > > > Hmm. I guess due to this. I'd prefer the opposite tbh so I could
> > > > just put in my opregion BCLM patch. It's annoying to have to
> > > > carry it locally just to have reasonable backlight behaviour
> > >
> > > After second though I guess I'm actually agreeing with Hans here.
> > > The current situation is where small change in the value near one
> > > end of the range does basically nothing, while a small change at
> > > the other of the range causes a massive brightness change. That
> > > is no good.
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
> > >
> >
>