Re: [PATCH v3 3/8] drm: link connectors to backlight devices

[Mario Limonciello <mario.limonciello@amd.com>]

On 4/25/26 07:41, Dmitry Baryshkov wrote:
> On Fri, Apr 24, 2026 at 05:09:48PM -0500, Mario Limonciello wrote:
>> This adds generic backlight-device support to DRM connectors. Drivers
>> that need to link backlight devices to DRM connectors can use the new
>> helpers to create a backlight property and link them at runtime.
>>
>> Also add a brightness changed notification so that sysfs and DRM connector
>> can stay in sync and handle property updates around DPMS.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/gpu/drm/Kconfig             |   1 +
>>   drivers/gpu/drm/Makefile            |   2 +
>>   drivers/gpu/drm/drm_backlight.c     | 406 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/drm_connector.c     |  12 +
>>   drivers/gpu/drm/drm_drv.c           |   8 +
>>   drivers/gpu/drm/drm_mode_config.c   |   7 +
>>   drivers/gpu/drm/drm_mode_object.c   |  66 ++++-
>>   drivers/gpu/drm/drm_sysfs.c         |  54 ++++
>>   drivers/video/backlight/backlight.c |  17 ++
>>   include/drm/drm_backlight.h         |  45 +++
>>   include/drm/drm_connector.h         |   3 +
>>   include/drm/drm_mode_config.h       |   5 +
>>   include/linux/backlight.h           |  13 +
>>   13 files changed, 637 insertions(+), 2 deletions(-)
>>   create mode 100644 drivers/gpu/drm/drm_backlight.c
>>   create mode 100644 include/drm/drm_backlight.h
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 8f5a8d3012e41..8c00f534d50b8 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -17,6 +17,7 @@ menuconfig DRM
>>   # device and dmabuf fd. Let's make sure that is available for our userspace.
>>   	select KCMP
>>   	select VIDEO
>> +	select BACKLIGHT_CLASS_DEVICE
>>   	help
>>   	  Kernel-level support for the Direct Rendering Infrastructure (DRI)
>>   	  introduced in XFree86 4.0. If you say Y here, you need to select
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index e97faabcd7830..9912306e94950 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -78,6 +78,8 @@ drm-$(CONFIG_DRM_CLIENT) += \
>>   	drm_client_event.o \
>>   	drm_client_modeset.o \
>>   	drm_client_sysrq.o
>> +drm-y += drm_backlight.o
>> +drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
>>   drm-$(CONFIG_COMPAT) += drm_ioc32.o
>>   drm-$(CONFIG_DRM_PANEL) += drm_panel.o
>>   drm-$(CONFIG_OF) += drm_of.o
>> diff --git a/drivers/gpu/drm/drm_backlight.c b/drivers/gpu/drm/drm_backlight.c
>> new file mode 100644
>> index 0000000000000..cbc6a855ed332
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_backlight.c
>> @@ -0,0 +1,406 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * DRM Backlight Helpers
>> + * Copyright (c) 2014 David Herrmann
>> + * Copyright (c) 2026 Advanced Micro Devices, Inc.
>> + */
>> +
>> +#include <linux/backlight.h>
>> +#include <linux/fs.h>
>> +#include <linux/list.h>
>> +#include <linux/math64.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/notifier.h>
>> +#include <linux/slab.h>
>> +#include <linux/spinlock.h>
>> +#include <drm/drm_backlight.h>
>> +#include <drm/drm_connector.h>
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_mode_config.h>
>> +
>> +/**
>> + * DOC: Backlight Devices
>> + *
>> + * Backlight devices have always been managed as a separate subsystem,
>> + * independent of DRM. They are usually controlled via separate hardware
>> + * interfaces than the display controller, so the split works out fine.
>> + * However, backlight brightness is a property of a display, and thus a
>> + * property of a DRM connector. We already manage DPMS states via connector
>> + * properties, so it is natural to keep brightness control at the same place.
>> + *
>> + * This DRM backlight interface implements generic backlight properties on
>> + * connectors. It does not handle any hardware backends but simply forwards
>> + * the requests to an available and linked backlight device. The links between
>> + * connectors and backlight devices have to be established by DRM drivers and
>> + * can be modified by user-space via sysfs (and udev rules). The name of the
>> + * backlight device can be written to a sysfs attribute called 'backlight'.
>> + * The device is looked up and linked to the connector (replacing a possible
>> + * previous backlight device). A 'change' uevent is sent whenever a link is
>> + * modified.
>> + *
>> + * Drivers have to call drm_backlight_alloc() after allocating a connector via
>> + * drm_connector_init(). This will automatically add a backlight device to the
> 
> I'd like to point out another kind of devices which might have been left
> w/o notice here: the panels. In a plenty of cases the driver creating
> the drm_connector (frequently through drm_bridge_connector_init())
> doesn't even know if there is a panel on the other side of the chain or
> not. Likewise connector creation doesn't involve any panel-related
> operation. Some panels provide backlight controls. Others don't and rely
> on the external backlight controls (see drm_panel_of_backlight() for
> example).
> 
> Can we work towards simplifying possible support for DRM backlights to
> the setups with drm_panel in place?

But to userspace it would interact with a connector not a panel directly 
right?

So doesn't the current design work well enough for this?  When there is 
a panel backlight control it could be added; and if there isn't just do 
nothing.

> 
>> + * given connector. No hardware device is linked to the connector by default.
>> + * Drivers can set up a default device via drm_backlight_set_name(), but are
>> + * free to leave it empty. User-space will then have to set up the link.
> 
> This sounds like a definite no-go. Think about any phone or any laptop.
> Do we need to involve userspace to let the kernel know that there is a
> connection between the DRM connector and the backlight present in the
> system?
> 
> What is the usescase for having dynamic connector<->link connections?
> 

Allowing userspace to setup the link was something in the original 
series by David and Marta that I left in place.  I don't have a problem 
with dropping that concept and requiring the driver to do the linking.