[RFC] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display

[Hans de Goede]

Hi All,

As discussed in the "[RFC] drm/kms: control display brightness through
drm_connector properties" thread, step 1 of the plan is to 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,
2 common scenarios where this happens are:

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

2) 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


Fixing kms driver unconditionally register their "native" backlight dev
=======================================================================

The plan for fixing 1) is to add a "bool native_backlight_available"
parameter to acpi_video_get_backlight_type(), which will be set to
true when called by e.g. the i915 code to check if it should register
its native backlight-device. This way acpi_video_get_backlight_type()
will know that a native backlight-device is (will be) available even
though it has not been registered yet and then it can return
acpi_backlight_native if that is the best option.

And then the i915 code will only actually register its native
backlight when acpi_backlight_native gets returned, thus hiding it
in scenario 1.


Fixing acpi_video0 getting registered for a brief time
======================================================

ATM the acpi_video code will delay parsing the ACPI video extensions
when an i915 opregion is present and will immediately parse these
+ register an acpi_video backlight device on laptops without Intel
integrated graphics. On laptops with i915 gfx the i915 driver calls
acpi_video_register() near the end of its probe() function when things
are ready for the acpi_video code to run, avoiding scenario 2.

Where as on systems without i915 gfx acpi_video's module_init()
immediately calls acpi_video_register() and then later the ACPI 
video_detect code calls acpi_video_unregister_backlight() to hide
the acpi_video# backlight-device on systems where the native
backlight-device should be used. The plan to fix this is to add
an acpi_video_register_backlight() and to make acpi_video_register()
do all the usual ACPI video extension probing, but have it skip
the actual registering of the backlight devices and have drivers
explicitly call acpi_video_register() after they have setup their
own native backlight-device. This way acpi_video_get_backlight_type()
already will know that a native backlight-device is available
(and preferred) when acpi_video_register_backlight() runs and
the registering of the acpi_video# device will then be skipped,
removing it briefly showing up and disappearing again.

One problem with this approach is that this relies on the GPU
driver to call acpi_video_register_backlight() when it is done.
One could argue that this is actually a feature, we have had
issues with some desktops where acpi_video falsely registers
its backlight (even though there is no internal LCD panel), but
this will likely cause issues on some systems (1). So the plan is
to queue a delayed work with an 8 second (1) delay from
acpi_video_register() and have that register the backlight-device
if not done already.


Other issues
============

The above only takes native vs acpi_video backlight issues into
account, there are also a couple of other scenarios which may
lead to multiple backlight-devices getting registered:

1) Apple laptops using the apple_gmux driver
2) Nvidia laptops using the (new) nvidia-wmi-ec-backlight driver
3) drivers/platform/x86 drivers calling acpi_video_set_dmi_backlight_type()
   to override the normal acpi_video_get_backlight_type() heuristics after
   the native/acpi_video drivers have already loaded

The plan for 1) + 2) is to extend the acpi_backlight_type enum with
acpi_backlight_gmux and acpi_backlight_nvidia_wmi_ec values and to add
detection of these 2 to acpi_video_get_backlight_type().

The plan for 3) is to move the DMI quirks from drivers/platform/x86
drivers which call acpi_video_set_dmi_backlight_type() in to the
existing DMI quirk table in drivers/acpi/video_detect.c, so that they
will be available during the first/every call of
acpi_video_get_backlight_type() and then remove
acpi_video_set_dmi_backlight_type()

Regards,

Hans


Footnotes:

1) E.g. systems using the nvidia binary driver, esp. systems using
the legacy nvidia binary driver versions. I at least expect the current
nvidia binary driver versions to get updated to actually call
acpi_video_register_backlight() when necessary.

2) The 8 second delay is the timeout plymouth uses waiting for a kms
device to show up when showing the bootsplash. This is based on the
amdgpu driver taking 5 seconds to initialize on some systems +
margin.