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

[Jani Nikula]

On Fri, 30 Sep 2022, Sebastian Wick <sebastian.wick at redhat.com> wrote:
> 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.

My idea has been all along that you'd have an API for defining points on
a curve, a kind of mapping, and the kernel would linear intrapolate
between the set points.

If the kernel knows how to pre-fill the information, it would do
so. Otherwise, it would just be linear. The userspace could adjust in
either case.

For i915, in some cases we'd be able to pre-fill the curve, and have a
better brightness adjustment experience. Ville has done something like
this for himself, but IIUC has not polished it into an interface.


BR,
Jani.


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

-- 
Jani Nikula, Intel Open Source Graphics Center