dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Sebastian Wick <sebastian.wick@redhat.com>,
	Martin Roukala <martin.roukala@mupuf.org>,
	Christoph Grenz <christophg+lkml@grenz-bonn.de>,
	wayland <wayland-devel@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Yusuf Khan <yusisamerican@gmail.com>
Subject: Re: [RFC] drm/kms: control display brightness through drm_connector properties
Date: Wed, 13 Apr 2022 11:44:40 +0200	[thread overview]
Message-ID: <3b467446-95de-3773-eb4b-8090eab27daf@redhat.com> (raw)
In-Reply-To: <YlaKnRY/aON+EUOx@phenom.ffwll.local>

Hi,

On 4/13/22 10:32, Daniel Vetter wrote:
> On Fri, Apr 08, 2022 at 12:26:24PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 4/8/22 12:16, Simon Ser wrote:
>>> Would it be an option to only support the KMS prop for Good devices,
>>> and continue using the suboptimal existing sysfs API for Bad devices?
>>>
>>> (I'm just throwing ideas around to see what sticks, feel free to ignore.)
>>
>> Currently suid-root or pkexec helpers are used to deal with the
>> /sys/class/backlight requires root rights issue. I really want to
>> be able to disable these helpers at build time in e.g. GNOME once
>> the new properties are supported in GNOME.  So that distros with
>> a new enough kernel can reduce their attack surface this way.
> 
> Yeah but otoh perpetuating a bad interface forever isn't a great idea
> either. I think the pragmatic plan here is
> - Implement this properly on good devices, i.e. anything new.
> - Do some runtime disabling in the pkexec helpers if they detect a modern
>   system (we should be able to put a proper symlink into the drm sysfs
>   connector directories, to make this easy to detect). It's not as great
>   as doing this at compile time, but it's a step.
> - Figure out a solution for the old crap. We can't really change anything
>   with the load ordering for existing systems, so if the hacked-up compat
>   libbacklight-backlight isn't an option then I guess we need some quirk
>   list/extracted code which makes i915/nouveau/radeon driver load fail
>   until the right backlight shows up. And that needs to be behind a
>   Kconfig to avoid breaking existing systems.
> 
> Inflicting hotplug complications on all userspace (including uevent
> handling for this hotpluggable backlight and everything) just because
> fixing the old crap systems is work is imo really not a good idea. Much
> better if we get to the correct future step-by-step.

This assumes that we only use the new brightness properties for laptop
internal LCD panels.

But what about controlling the brightness of external monitors through
DDC/CI? If we do that we need hotplug support for this regardless since
external monitors can be hotplugged.

As I mentioned in other parts of the thread one of the reasons why
I've started looking into this again is because of people being
interested in this (1).

And even just taking internal LCD panels into account, there are
hybrid GPU laptops where the backlight is directly controlled by
the GPU (native type backlight driver) connected to the panel(2),
if we runtime switch the GPU attached to the panel there, then we
will get an actual hotplug of the LCD connector and I'm not sure if
we can always detect the maximum value of the brightness on the GPU
which is not connected to the panel at boot. So in this case we
need userspace to support re-reading the brightness max at
a hotplug event regardless.

So in all in all I feel that supporting hotplug events is
unavoidable here.

Regards,

Hans


1) Including attempting to do this through the old /sys/class/backlight
interface which IMHO would be quite bad to do:
https://lore.kernel.org/lkml/20220403230850.2986-1-yusisamerican@gmail.com/

2) E.g. gnome-settings-daemon has special code to detect which native
backlight interface to use if there are 2 native backlight devices on
a single laptop, see:
https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/blob/master/plugins/power/gsd-backlight.c#L95





  parent reply	other threads:[~2022-04-13  9:44 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 [this message]
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
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=3b467446-95de-3773-eb4b-8090eab27daf@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=christophg+lkml@grenz-bonn.de \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --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).