From: Hans de Goede <hdegoede@redhat.com>
To: "dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
wayland <wayland-devel@lists.freedesktop.org>
Cc: Sebastian Wick <sebastian.wick@redhat.com>,
Martin Roukala <martin.roukala@mupuf.org>,
Christoph Grenz <christophg+lkml@grenz-bonn.de>,
Yusuf Khan <yusisamerican@gmail.com>
Subject: [RFC v2] drm/kms: control display brightness through drm_connector properties
Date: Fri, 9 Sep 2022 12:12:51 +0200 [thread overview]
Message-ID: <b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com> (raw)
Hi all,
Here is v2 of my "drm/kms: control display brightness through drm_connector properties" RFC:
Changes from version 1:
- Drop bl_brightness_0_is_min_brightness from list of new connector
properties.
- Clearly define that 0 is always min-brightness when setting the brightness
through the connector properties.
- Drop bl_brightness_control_method from list of new connector
properties.
- Phase 1 of the plan has been completed
As discussed already several times in the past:
https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/
https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b02c6@linux.intel.com/
The current userspace API for brightness control offered by
/sys/class/backlight devices has various issues:
1. There is no way to map the backlight device to a specific
display-output / panel (1)
2. Controlling the brightness requires root-rights requiring
desktop-environments to use suid-root helpers for this.
3. The meaning of 0 is not clearly defined, it can be either off,
or minimum brightness at which the display is still readable
(in a low light environment)
4. It's not possible to change both the gamma and the brightness in the
same KMS atomic commit. You'd want to be able to reduce brightness to
conserve power, and counter the effects of that by changing gamma to
reach a visually similar image. And you'd want to have the changes take
effect at the same time instead of reducing brightness at some frame and
change gamma at some other frame. This is pretty much impossible to do
via the sysfs interface.
As already discussed on various conference's hallway tracks
and as has been proposed on the dri-devel list once before (2),
it seems that there is consensus that the best way to to solve these
2 issues is to add support for controlling a video-output's brightness
through properties on the drm_connector.
This RFC outlines my plan to try and actually implement this,
which has 3 phases:
Phase 1: Stop registering multiple /sys/class/backlight devs for a single display
=================================================================================
On x86 there can be multiple firmware + direct-hw-access methods
for controlling the backlight and in some cases the kernel registers
multiple backlight-devices for a single internal laptop LCD panel.
A plan to fix this was posted here:
https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343158@redhat.com/
And a pull-req actually implementing this plan has been send out this week:
https://lore.kernel.org/dri-devel/261afe3d-7790-e945-adf6-a2c96c9b1eff@redhat.com/
Phase 2: Add drm_connector properties mirroring the matching backlight device
=============================================================================
The plan is to add a drm_connector helper function, which optionally takes
a pointer to the backlight device for the GPU's native backlight device,
which will then mirror the backlight settings from the backlight device
in a set of read/write brightness* properties on the connector.
This function can then be called by GPU drivers for the drm_connector for
the internal panel and it will then take care of everything. When there
is no native GPU backlight device, or when it should not be used then
(on x86) the helper will use the acpi_video_get_backlight_type() to
determine which backlight-device should be used instead and it will find
+ mirror that one.
Phase 3: Deprecate /sys/class/backlight uAPI
============================================
Once most userspace has moved over to using the new drm_connector
brightness props, a Kconfig option can be added to stop exporting
the backlight-devices under /sys/class/backlight. The plan is to
just disable the sysfs interface and keep the existing backlight-device
internal kernel abstraction as is, since some abstraction for (non GPU
native) backlight devices will be necessary regardless.
It is unsure if we will ever be able to do this. For example people using
non fully integrated desktop environments like e.g. sway often use custom
scripts binded to hotkeys to get functionality like the brightness
up/down keyboard hotkeys changing the brightness. This typically involves
e.g. the xbacklight utility.
Even if the xbacklight utility is ported to use kms with the new connector
object brightness properties then this still will not work because
changing the properties will require drm-master rights and e.g. sway will
already hold those.
The drm_connector brightness properties
=======================================
The new uAPI for this consists of 2 properties:
1. "display brightness": rw 0-int32_max property controlling the brightness setting
of the connected display. The actual maximum of this will be less then
int32_max and is given in "display brightness max".
Unlike the /sys/class/backlight/foo/brightness this brightness property
has a clear definition for the value 0. The kernel must ensure that 0
means minimum brightness (so 0 should _never_ turn the backlight off).
If necessary the kernel must enforce a minimum value by adding
an offset to the value seen in the property to ensure this behavior.
For example if necessary the driver must clamp 0-255 to 10-255, which then
becomes 0-245 on the brightness property, adding 10 internally to writes
done to the brightness property. This adding of an extra offset when
necessary must only be done on the brightness property,
the /sys/class/backlight interface should be left unchanged to not break
userspace which may rely on 0 = off on some systems.
Note amdgpu already does something like this even for /sys/class/backlight,
see the use of AMDGPU_DM_DEFAULT_MIN_BACKLIGHT in amdgpu.
Also whenever possible the kernel must ensure that the brightness range
is in perceived brightness, but this cannot always be guaranteed.
2. "display brightness max": ro 0-int32_max property giving the actual maximum
of the display's brightness setting. This will report 0 when brightness
control is not available.
The value of "display brightness max" may change at runtime, either by
a legacy drivers/platform/x86 backlight driver loading after the drm
driver has loaded; or when plugging in a monitor which allows brightness
control over DDC/CI. In both these cases the max value will change from 0
to the actual max value, indicating backlight control has become available
on this connector.
Future extensions
=================
Some hardware do adaptive brightness in hardware, rather then providing
an ALS sensor and letting userspace handle this.
One example of this is the Steam deck, which currently uses some custom
sysfs attributes to allow tweaking (and enable/disable?) the adaptive
brightness. Adding standardized uAPI for this through new
"display brightness *" properties seems like a natural extension of this
proposal.
Regards,
Hans
1) The need to be able to map the backlight device to a specific display
has become clear once more with the recent proposal to add DDCDI support:
https://lore.kernel.org/lkml/20220403230850.2986-1-yusisamerican@gmail.com/
2) https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b02c6@linux.intel.com/
Note this proposal included a method for userspace to be able to tell the
kernel if the native/acpi_video/vendor backlight device should be used,
but this has been solved in the kernel for years now:
https://www.spinics.net/lists/linux-acpi/msg50526.html
An initial implementation of this proposal is available here:
https://cgit.freedesktop.org/~mperes/linux/log/?h=backlight
next reply other threads:[~2022-09-09 10:13 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-09 10:12 Hans de Goede [this message]
2022-09-09 13:39 ` [RFC v2] drm/kms: control display brightness through drm_connector properties Simon Ser
2022-09-09 13:53 ` Pekka Paalanen
2022-09-09 14:01 ` Simon Ser
2022-09-09 14:39 ` Hans de Goede
2022-09-28 10:04 ` Jani Nikula
2022-09-28 10:57 ` Ville Syrjälä
2022-09-28 11:14 ` Ville Syrjälä
2022-09-29 18:06 ` Sebastian Wick
2022-09-30 7:39 ` Pekka Paalanen
2022-09-30 14:20 ` Sebastian Wick
2022-09-30 14:30 ` Jani Nikula
2022-09-30 14:44 ` Ville Syrjälä
2022-09-30 14:49 ` Simon Ser
2022-09-30 15:26 ` Pekka Paalanen
2022-09-30 16:17 ` Sebastian Wick
2022-10-03 8:37 ` Pekka Paalanen
2022-10-03 9:29 ` Ville Syrjälä
2022-10-03 10:16 ` Pekka Paalanen
2022-10-03 9:44 ` Hans de Goede
2022-10-03 10:32 ` Pekka Paalanen
2022-10-03 11:02 ` Hans de Goede
2022-10-03 9:02 ` Hans de Goede
2022-10-03 8:53 ` 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=b61d3eeb-6213-afac-2e70-7b9791c86d2e@redhat.com \
--to=hdegoede@redhat.com \
--cc=christophg+lkml@grenz-bonn.de \
--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).