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

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

 



On Mon, 11 Apr 2022 12:18:30 +0200
Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

> Hi,
> 
> On 4/8/22 17:11, Alex Deucher wrote:
> > On Fri, Apr 8, 2022 at 10:56 AM Hans de Goede <hdegoede@xxxxxxxxxx> 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?

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

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

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.

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.


Thanks,
pq

Attachment: pgpPSP1CMQAtc.pgp
Description: OpenPGP digital signature


[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