Hi,
On 4/14/22 15:10, Jani Nikula wrote:
> On Thu, 07 Apr 2022, Hans de Goede wrote:
>> 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-d37d844b0...@linux.intel.com/
>>
>> The current userspace API for brightness control offered by
>> /sys/class/backlight devices has various issues, the biggest 2 being:
>>
>> 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.
>>
>> 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) i915 and nouveau unconditionally register their "native" backlight dev
>>even on devices where /sys/class/backlight/acpi_video0 must be used
>>to control the backlight, relying on userspace to prefer the "firmware"
>>acpi_video0 device over "native" devices.
>> b) amdgpu and nouveau rely on the acpi_video driver initializing before
>>them, which currently causes /sys/class/backlight/acpi_video0 to usually
>>show up and then they register their own native backlight driver after
>>which the drivers/acpi/video_detect.c code unregisters the acpi_video0
>>device. This means that userspace briefly sees 2 devices and the
>>disappearing of acpi_video0 after a brief time confuses the systemd
>>backlight level save/restore code, see e.g.:
>>https://bbs.archlinux.org/viewtopic.php?id=269920
>>
>> I already have a pretty detailed plan to tackle this, which I will
>> post in a separate RFC email. I plan to start working on this right
>> away, as it will be good to have this fixed regardless.
>>
>>
>> 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.
>>
>> An alternative to disabling the sysfs class entirely, would be
>> to allow setting it to read-only through Kconfig.
>>
>>
>> What scale to use for the drm_connector bl_brightness property?
>> ===
>>
>> The tricky part of this plan is phase 2 and then esp. defining what the
>> new brightness properties will look like and how they will work.
>>
>> The biggest challenge here is to decide on a fixed scale for the main
>> brightness property, say 0-65535, using scaling where the actual hw scale
>> is different, or if this should simply be a 1:1 mirror of the current
>> backlight interface, with the actual hw scale / brightness_max value
>> exposed as a drm_connector property.
>>
>> 1:1 advantages / 0-65535 disadvantages
>> - Userspace will likely move over to the connector-props quite slowly and
>> we can expect various userspace bits, esp. also custom user scripts, to
>> keep using the old uAPI for a long time. Using the 2 APIs are intermixed
>> is fine when using a 1:1 brightness scale mapping. But if we end up doing
>> a
