dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Pekka Paalanen <ppaalanen@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Sebastian Wick <sebastian.wick@redhat.com>,
	Martin Roukala <martin.roukala@mupuf.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	wayland <wayland-devel@lists.freedesktop.org>,
	Christoph Grenz <christophg+lkml@grenz-bonn.de>,
	Yusuf Khan <yusisamerican@gmail.com>
Subject: Re: [RFC] drm/kms: control display brightness through drm_connector properties
Date: Mon, 11 Apr 2022 14:34:16 +0300	[thread overview]
Message-ID: <20220411143416.04a65b5d@eldfell> (raw)
In-Reply-To: <a42f03bf-bf85-b08e-fa4f-e36a226922bc@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3679 bytes --]

On Mon, 11 Apr 2022 12:18:30 +0200
Hans de Goede <hdegoede@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@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?

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2022-04-11 11:34 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07 15:38 [RFC] drm/kms: control display brightness through drm_connector properties Hans de Goede
2022-04-07 16:51 ` Simon Ser
2022-04-07 17:43   ` Hans de Goede
2022-04-07 21:05     ` Alex Deucher
2022-04-08  8:07       ` Daniel Vetter
2022-04-08  9:58         ` Hans de Goede
2022-04-08 10:09           ` Hans de Goede
2022-04-08 10:16             ` Simon Ser
2022-04-08 10:26               ` Hans de Goede
2022-04-13  8:32                 ` Daniel Vetter
2022-04-13  8:38                   ` Simon Ser
2022-04-13  9:44                   ` Hans de Goede
2022-04-08 10:23           ` Hans de Goede
2022-04-08 14:08         ` Alex Deucher
2022-04-08 14:55           ` Hans de Goede
2022-04-08 15:11             ` Alex Deucher
2022-04-11 10:18               ` Hans de Goede
2022-04-11 11:34                 ` Pekka Paalanen [this message]
2022-04-11 11:50                   ` Hans de Goede
2022-04-11 13:11                     ` Mikhail Gusarov
2022-04-11 14:11                 ` Alex Deucher
2022-04-14 10:24                   ` Jani Nikula
2022-04-27 14:03                     ` Daniel Vetter
2022-04-27 14:23                       ` Jani Nikula
2022-04-27 14:26                         ` Daniel Vetter
2022-04-29  8:55                           ` Hans de Goede
2022-04-29  8:59                             ` Simon Ser
2022-04-29  9:06                               ` Pekka Paalanen
2022-04-29  9:49                                 ` Lattannavar, Sameer
2022-04-08  8:22     ` Simon Ser
2022-04-08 15:00       ` Hans de Goede
2022-04-11 10:35       ` Hans de Goede
2022-04-07 18:58 ` Carsten Haitzler
2022-04-11 10:27   ` Hans de Goede
2022-04-11 11:14     ` Carsten Haitzler
2022-04-14 13:10 ` Jani Nikula
2022-05-18 12:59   ` Hans de Goede
2022-05-18 14:23     ` Jani Nikula
2022-05-31 10:40       ` Hans de Goede
2022-05-18 14:40     ` Ville Syrjälä
2022-08-24  2:18 ` Yusuf Khan
2022-08-25  8:27   ` Hans de Goede
2022-08-25 21:40     ` Yusuf Khan
2022-08-28  8:08       ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220411143416.04a65b5d@eldfell \
    --to=ppaalanen@gmail.com \
    --cc=christophg+lkml@grenz-bonn.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=martin.roukala@mupuf.org \
    --cc=sebastian.wick@redhat.com \
    --cc=wayland-devel@lists.freedesktop.org \
    --cc=yusisamerican@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).