Re: [PATCH v6 00/10] Add support for a DRM backlight capability

Hi Mario,

On 24-Jun-26 18:57, Mario Limonciello wrote:
> From: Mario Limonciello (AMD) <[email protected]>
> 
> At Display Next Hackfest 2026 we reviewed progress moving brightness
> control into the DRM connector properties.
> 
> There is a range LUMINANCE property that will default to 0->0.
> Once a driver attaches a backlight it will be updated to 1->max.
> If the panel supports the minimum backlight turning off the display
> the range can later be updated to 0->max instead of 1->max.
> 
> The legacy sysfs interface is synchronized with the DRM connector.
> When a compositor using this feature is loaded, sysfs writes are disabled
> to prevent legacy tools from going out of sync with the compositor.
> 
> This has an implementation initially for amdgpu, i915, and Xe with eDP
> connectors.  It can be extended to other connectors like DP for displays
> that can be controlled via DDC as well later.

First of all thank you very much for working on this.

I'm a bit late to the party, since I only became aware of this
patch-set recently through:
https://blog.sebastianwick.net/posts/display-next-hackfest-2026/

Overall this looks pretty good to me.

My main remark is related to non acpi_backlight=native backlight
control.

When I first started working on implementing backlight control
as a DRM connector property I never got further than a whole
bunch of prep working making sure that all backlight drivers
(and all drm/kms driver backlight code) were using / honoring
acpi_video_get_backlight_type() resp. its special
acpi_video_backlight_use_native() variant.

This prep work was meant to allow the drm connector prop code
to deal with the case where e.g. the backlight code from
drivers/acpi/acpi_video.c (acpi_backlight=video) should be used as
is the case on many older laptops.

The prep work basically made sure that all backlight hw/fw
interface selection handling (on x86) goes through
drivers/acpi/video_detect.c . To have a single source of
truth of which backlight control method should be used.

Looking at e.g. "[PATCH v6 07/10] drm/amd/display: use drm
backlight" then this is partly honored since 
amdgpu_dm_register_backlight_device() starts with:

        if (!acpi_video_backlight_use_native()) {
                drm_info(drm, "Skipping amdgpu DM backlight registration\n");
                /* Try registering an ACPI video backlight device instead. */
                acpi_video_register_backlight();
                return;
        }

and the added drm_backlight_link() call happens later and thus
gets skipped if the acpi_video_backlight_use_native() check fails,
but any non native backlight devices never get linked.

What IMHO needs to happen (perhaps in a follow-up series) is:

If the acpi_video_backlight_use_native() call returns false then
replace the acpi_video_register_backlight() call with a new:

drm_backlight_handle_non_native_bl(internal_panel_drm_connector, pci_dev);

function which does the following:

1. Call acpi_video_get_backlight_type() and cache the type it returns
and on acpi_backlight_none bail from the function.

2. Add a new acpi_video_register_single_backlight(), which takes
a pci_dev pointer as argument and then only registers the acpi_video
backlight matching that PCI device, see the parent handling in
acpi_video_dev_register_backlight().

This new function then returns the registered backlight on success
or NULL (or maybe an ERR_PTR()?).

3. If the type is acpi_backlight_video call
acpi_video_register_single_backlight() and link the returned
backlight instead of the native one.

4. For other types register a backlight notifier which listens for
new backlight drivers to show up and then checks those against
the cached type. And on a successful match, link the new
backlight device to the connector.

This is for drivers/platform/x86 backlight drivers, like
dell-laptop on old Dell Latitudes which will typically not
be in the initramfs, while the GPU driver will be in
the initramfs.

5. After registering the notifier, check if a backlight device
of the expected type has not already been registered beforehand
and if yes, link it and remove the notifier.

Stating the obvious: 5. needs to be done last to avoid a race
between checking for the backlight device already being there
and a new backlight getting registered.

I specifically have retained a bunch of old laptops which use
the acpi_backlight_vendor and acpi_backlight_native methods to
test this. I even have one with 2 GPUs which IIRC both
work with acpi_backlight=video and I can switch which one
drivers the panel in the BIOS.

Regards,

Hans






> 
> The following compositors have implemented matching support:
>  * Kwin: https://invent.kde.org/plasma/kwin/-/merge_requests/9298
>  * Mutter: https://gitlab.gnome.org/swick/mutter/-/commits/wip/kms-luminance-prop
>  * Wlroots: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/5384
> 
> ---
> v5->v6:
>  * Rebase on drm-next
>  * Drop patch 7/11 (Move backlight tracing out of the dc lock)
> 
> Mario Limonciello (AMD) (10):
>   Revert "backlight: Remove notifier"
>   backlight: add kernel-internal backlight API
>   drm: link connectors to backlight devices
>   DRM: Add support for client indicating support for luminance
>   drm/amd/display: Pass up errors reading actual brightness
>   drm/amd/display: Allow backlight registration to fail
>   drm/amd/display: use drm backlight
>   drm/amd/display: Drop brightness caching in amdgpu_dm
>   drm/bridge: auto-link panel backlight in bridge connector
>   drm/i915/display: use drm backlight
> 
>  drivers/gpu/drm/Kconfig                       |   1 +
>  drivers/gpu/drm/Makefile                      |   1 +
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 111 ++---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  13 -
>  drivers/gpu/drm/bridge/panel.c                |  15 +
>  .../gpu/drm/display/drm_bridge_connector.c    |  15 +-
>  drivers/gpu/drm/drm_atomic_helper.c           |   7 +
>  drivers/gpu/drm/drm_atomic_uapi.c             |  59 ++-
>  drivers/gpu/drm/drm_backlight.c               | 445 ++++++++++++++++++
>  drivers/gpu/drm/drm_connector.c               |  63 +++
>  drivers/gpu/drm/drm_drv.c                     |   8 +
>  drivers/gpu/drm/drm_file.c                    |   5 +
>  drivers/gpu/drm/drm_ioctl.c                   |  15 +
>  drivers/gpu/drm/drm_mode_config.c             |   7 +
>  drivers/gpu/drm/drm_mode_object.c             |  66 ++-
>  drivers/gpu/drm/drm_property.c                |   6 +
>  drivers/gpu/drm/drm_sysfs.c                   |  28 +-
>  .../gpu/drm/i915/display/intel_backlight.c    |   4 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |   8 +
>  drivers/video/backlight/backlight.c           |  97 ++++
>  include/drm/drm_backlight.h                   |  51 ++
>  include/drm/drm_bridge.h                      |   1 +
>  include/drm/drm_connector.h                   |   8 +
>  include/drm/drm_file.h                        |   8 +
>  include/drm/drm_mode_config.h                 |   5 +
>  include/linux/backlight.h                     |  63 +++
>  include/uapi/drm/drm.h                        |  22 +
>  27 files changed, 1046 insertions(+), 86 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_backlight.c
>  create mode 100644 include/drm/drm_backlight.h
> 
> 
> base-commit: 0e8233409d4f6def051dd42a432c6815bb780d78