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

[Simon Ser]

On Friday, September 9th, 2022 at 12:12, Hans de Goede <hdegoede at redhat.com> wrote:

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

I replied to this here in another thread [1].

tl;dr I think it would be fine even for Sway-like compositors.

(Also note the utilities used right now are not xbacklight, but
brightnessctl/light/brillo/etc [2])

[1]: https://lore.kernel.org/dri-devel/bZJU9OkYWFyaLHVa4XUE4d5iBTPFXBRyPe1wMd_ztKh5VBMu-EDNGoUDpvwtFn_u9-JMvN8QmIZVS3pzMZM_hZTkTCA9gOBnCGXc5HFmsnc=@emersion.fr/
[2]: https://github.com/swaywm/sway/wiki#xbacklight

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

The kernel will need to ensure that a hotplug uevent is sent to
user-space at this point. Otherwise user-space has no way to figure out
that the prop has changed.

Overall this all looks very solid to me!

Simon