OLED panel brightness support

[Daniel Vetter]

On Thu, Jul 25, 2019 at 03:35:40PM +0800, Kai-Heng Feng wrote:
> at 00:03, <Mario.Limonciello at dell.com> <Mario.Limonciello at dell.com> wrote:
> 
> > > -----Original Message-----
> > > From: Daniel Vetter <daniel.vetter at ffwll.ch> On Behalf Of Daniel Vetter
> > > Sent: Wednesday, July 24, 2019 6:49 AM
> > > To: Kai-Heng Feng
> > > Cc: dri-devel at lists.freedesktop.org; Anthony Wong; Limonciello, Mario
> > > Subject: Re: OLED panel brightness support
> > > 
> > > 
> > > [EXTERNAL EMAIL]
> > > 
> > > On Tue, Jul 23, 2019 at 06:46:55PM +0800, Kai-Heng Feng wrote:
> > > > Hi,
> > > > 
> > > > Currently, OLED panel brightness [1] is not supported.
> > > > We have a similar Dell system that also affect by lack of OLED brightness
> > > > support.
> > > > 
> > > > I’ve investigated both kernel and user space but I haven’t found a good
> > > > general solution yet.
> > > > Dell systems use EDID descriptor 4 as Dell specific descriptor, which
> > > > reports its panel type and we can know it’s an OLED panel or not.
> > > > 
> > > > My initial thought is to add a new attribute “oled" in drm_sysfs.c [2] to
> > > > let userspace like clutter [3] to control the brightness.
> > > > However other DEs may need to implement their own OLED brightness support
> > > > which isn’t ideal.
> > > > 
> > > > So I’d like to know if there’s any good way to support OLED brightness in
> > > > good old backlight sysfs, to let userspace keep to the current interface.
> > > > 
> > > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=97883
> > > > [2] https://pastebin.ubuntu.com/p/QYrRBppVT9/
> > > > [3] https://gitlab.gnome.org/GNOME/clutter/blob/master/clutter/clutter-
> > > brightness-contrast-effect.c#L559
> > > 
> > > I think the Most Proper Solution would be to integrate the backlight
> > > support into drm, by adding the backlight knobs as kms properties to the
> > > correct connector. This would fix a whole bag of issue:
> > > - no more ill-defined magic for userspace to find the right backlight
> > > - we could have well-defined semantics what happens with the backlight
> > >   across a kms modeset
> > > - issues like this could be solved with a small helper and a bit of code
> > >   in the kernel (there's also other DDC knobs to control backlight, which
> > >   we also don't really expose in a consistent way).
> > > 
> > > Downside is how to roll this out in a backward compatible way, without
> > > breaking the world:
> > > - fbcon/fbdev needs to be taught to not do it's own backlight wrangling
> > >   for kms drivers which handle backlight natively
> > > - we might need an opt-in magic for this, if it turns out that the kms
> > >   driver handling the backlight breaks stuff
> > > - userspace ofc needs to fall back to its current pile of hacks if the
> > >   backlight stuff isn't supported natively.
> > > 
> > > Adding more ill-defined sysfs files, or more magice ordering rules, or
> > > anything else like that doesn't sound too appealing.
> > 
> > Where are you thinking that the opt in magic would occur then?  Userspace?
> > 
> > As KH said, at least on Dell it's a known location in EDID to indicate
> > panel
> > is OLED, so it seems like this could be a kernel time documented magic
> > at least
> > to turn this on in the important scenarios.
> 
> I think what Daniel says is to keep a uniform backlight interface.

Yup. The current approach just doesn't really work, and you end up with
fun like this, where I guess it looks like there's a brightness control,
but it doesn't work.

For opt-in maybe we need a separate per-drm_driver value that indicates
whether this uniform backlight interface is supported. Then userspace
knows that absence of the backlight properties means there's not backlight
support (and it needs to fake it using gamma tables or whatever).

We might also need to set this knob on a per-device/generation/platform
basis, since with long-standing drivers like i915 it'll be a ton of work
to move everything over - essentially we need to implement the current
pile of ad-hoc hacks userspace uses in the kernel, and then improve the
situation going forward.

> The next question is, how do we change the brightness level for OLED
> displays? Is changing gamma value a good way to do it?

There's no overall amplifier knob to set general brightness on these?
-Daniel

> 
> Kai-Heng
> 
> > 
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch