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

[Hans de Goede]

Hi Pekka,

On 4/11/22 13:34, Pekka Paalanen wrote:
> On Mon, 11 Apr 2022 12:18:30 +0200
> Hans de Goede <hdegoede at redhat.com> wrote:
> 
>> Hi,
>>
>> On 4/8/22 17:11, Alex Deucher wrote:
>>> On Fri, Apr 8, 2022 at 10:56 AM Hans de Goede <hdegoede at redhat.com> wrote:  
>>>>
> 
> ...
> 
>>> So set it to a level we can guarantee can call it 0.  If we have the
>>> flag we are just punting on the problem in my opinion.  
>>
>> Right this an impossible problem to solve so the intent is indeed
>> to punt this to userspace, which IMHO is the best thing we can do
>> here.  The idea behind the "bl_brightness_0_is_min_brightness:
>> ro, boolean" property is to provide a hint to userspace to help
>> userspace deal with this (and if userspace ends up using e.g.
>> systemd's hwdb for this to avoid unnecessary entries in hwdb).
>>
>>>  The kernel
>>> driver would seem to have a better idea what is a valid minimum than
>>> some arbitrary userspace application.  
>>
>> If the kernel driver knows the valid minimum then it can make 0
>> actually be that valid minimum as you suggest and it can set the
>> hint flag to let userspace know this. OTOH there are many cases
>> where the kernel's guess is just as bad as userspace's guess and
>> there are too many laptops where this is the case to quirk
>> ourselves out of this situation.
>>
>>> Plus then if we need a
>>> workaround for what is the minimum valid brightness, we can fix it one
>>> place rather than letting every application try and fix it.  
>>
>> I wish we could solve this in the kernel, but at least on
>> laptops with Intel integrated gfx many vendors don't bother
>> to put a non 0 value in the minimum duty-cycle field of the
>> VBT, so there really is no good way to solve this.
>>
>> If the userspace folks ever want to do a database for this,
>> I would expect them to do something with hwdb + udev which
>> can then be shared between the different desktop-environments.
> 
> Hi Hans,
> 
> assuming that it is impossible to reach a reasonable user experience by
> having a quirk database in the kernel in order to offer a consistent
> definition of bl_brightness=0, then should you not be recommending a
> userspace hwdb solution with full steam, rather than adding a hint in
> the kernel that might be just enough to have no-one ever bother
> investing in a proper solution?

The problem is we already lack the manpower for a quirk database,
and even if we ever get the manpower then it would still be good
to avoid the work necessary to add models to the database where
the kernel already knows how things work, see below.

As for no-one ever bothering coming up with a full-steam hwdb
solution for the cases where the kernel has no idea what
bl_brightness=0 means, yes that is likely, but that already
is the status quo, the hint will at least allow using the
full brightness range on devices where the kernel knows
(with certainty) that this is correct.

> Re-reading your "bl_brightness_0_is_min_brightness" definition, it
> seems to be specified as exposing a certain condition in the system.
> When it is true, you imply that userspace can safely use value 0 as min
> brightness, but that is assuming the hint is correct. How likely
> is the hint incorrect?

It should never be incorrect, there are cases when the kernel knows
reliably that bl_brightness=0 means minimum brightness
(and NOT backlight off).

> If the hint can be incorrect, does this hint
> actually give anything to userspace, or would userspace still choose to
> be safer than sorry and ignore the hint by assuming the worst?

If the hint is incorrect then that would be a kernel bug and that
should be fixed in the kernel. The whole idea behind the hind is
that userspace can absolutely trust it to be correct when set to true
(false basically means that the kernel does not know of 0=off or not).

> Is this situation much different to the quirk database libinput needs
> to work beautifully out of the box?

libinput's quirk database is relatively pretty small and a lot
of effort is done to fix things in generic ways where possible,
to avoid growing it.

As a general rule quirks should only be used to handle exceptions
to general rules, the problem here is that bl_brightness=0 being
backlight off is not a true exception it happens quite often.

Which is also why I believe that a hwdb for this is not necessarily
a great idea, because maintaining it will be a lot of work.

> Should desktop environments offer a couple more "advanced
> configuration" knobs for the lowest safe brightness value and the
> value-to-perceived brightness mapping to calibrate the familiar
> brightness slider? E.g. something like "click this button as soon as
> you see it on the display" for finding the lowest usable brightness,
> with defaults coming from a database.

Maybe, but that would defeat all the attempts done to make Linux
on the desktop just work.

> If the situation is as grim as you say, I would propose to drop
> "bl_brightness_0_is_min_brightness" (and
> "bl_brightness_control_method"), and document the dangers of using too
> low brightness values. Maybe also start looking for a project that
> would be appropriate for hosting such a database, just to point people
> to cooperate in a single place rather than each DE coming up with their
> own.

I agree with dropping bl_brightness_control_method, but I do
believe that bl_brightness_0_is_min_brightness will be actually
useful to have, see above.

As for starting a database for this. I gave a talk about improving the
backlight situation under Linux in 2014, nothing has happened since
then. I think we need to be realistic here and accept that we don't
have the manpower to do something like a database here.

Regards,

Hans