On Friday, September 9th, 2022 at 12:12, Hans de Goede <hdegoede@xxxxxxxxxx> 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
