* [PATCH v6 01/10] Revert "backlight: Remove notifier"
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-26 11:41 ` Thomas Zimmermann
2026-06-24 16:57 ` [PATCH v6 02/10] backlight: add kernel-internal backlight API Mario Limonciello
` (9 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Simon Ser
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
This reverts commit 5461f3fd74a89757f95f351eb0bc26aafc2a2e91.
The backlight notifier support is needed in order to add backlight
control support into DRM connectors.
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Simon Ser <contact@emersion.fr>
---
drivers/video/backlight/backlight.c | 42 +++++++++++++++++++++++++++++
include/linux/backlight.h | 20 ++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index a22d0bbb6e639..ff2c2084c73a4 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -65,6 +65,7 @@
static struct list_head backlight_dev_list;
static struct mutex backlight_dev_list_mutex;
+static struct blocking_notifier_head backlight_notifier;
static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = "raw",
@@ -415,6 +416,9 @@ struct backlight_device *backlight_device_register(const char *name,
list_add(&new_bd->entry, &backlight_dev_list);
mutex_unlock(&backlight_dev_list_mutex);
+ blocking_notifier_call_chain(&backlight_notifier,
+ BACKLIGHT_REGISTERED, new_bd);
+
return new_bd;
}
EXPORT_SYMBOL(backlight_device_register);
@@ -484,6 +488,9 @@ void backlight_device_unregister(struct backlight_device *bd)
mutex_unlock(&pmac_backlight_mutex);
#endif
+ blocking_notifier_call_chain(&backlight_notifier,
+ BACKLIGHT_UNREGISTERED, bd);
+
mutex_lock(&bd->ops_lock);
bd->ops = NULL;
mutex_unlock(&bd->ops_lock);
@@ -507,6 +514,40 @@ static int devm_backlight_device_match(struct device *dev, void *res,
return *r == data;
}
+/**
+ * backlight_register_notifier - get notified of backlight (un)registration
+ * @nb: notifier block with the notifier to call on backlight (un)registration
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ *
+ * RETURNS:
+ *
+ * 0 on success, otherwise a negative error code
+ */
+int backlight_register_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_register_notifier);
+
+/**
+ * backlight_unregister_notifier - unregister a backlight notifier
+ * @nb: notifier block to unregister
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ *
+ * RETURNS:
+ *
+ * 0 on success, otherwise a negative error code
+ */
+int backlight_unregister_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_unregister_notifier);
+
/**
* devm_backlight_device_register - register a new backlight device
* @dev: the device to register
@@ -674,6 +715,7 @@ static int __init backlight_class_init(void)
INIT_LIST_HEAD(&backlight_dev_list);
mutex_init(&backlight_dev_list_mutex);
+ BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
return 0;
}
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index f29a9ef1052e7..d905173c7f73c 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -64,6 +64,24 @@ enum backlight_type {
BACKLIGHT_TYPE_MAX,
};
+/**
+ * enum backlight_notification - the type of notification
+ *
+ * The notifications that is used for notification sent to the receiver
+ * that registered notifications using backlight_register_notifier().
+ */
+enum backlight_notification {
+ /**
+ * @BACKLIGHT_REGISTERED: The backlight device is registered.
+ */
+ BACKLIGHT_REGISTERED,
+
+ /**
+ * @BACKLIGHT_UNREGISTERED: The backlight revice is unregistered.
+ */
+ BACKLIGHT_UNREGISTERED,
+};
+
/** enum backlight_scale - the type of scale used for brightness values
*
* The type of scale used for brightness values.
@@ -388,6 +406,8 @@ void devm_backlight_device_unregister(struct device *dev,
struct backlight_device *bd);
void backlight_force_update(struct backlight_device *bd,
enum backlight_update_reason reason);
+int backlight_register_notifier(struct notifier_block *nb);
+int backlight_unregister_notifier(struct notifier_block *nb);
struct backlight_device *backlight_device_get_by_name(const char *name);
struct backlight_device *backlight_device_get_by_type(enum backlight_type type);
int backlight_device_set_brightness(struct backlight_device *bd,
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v6 01/10] Revert "backlight: Remove notifier"
2026-06-24 16:57 ` [PATCH v6 01/10] Revert "backlight: Remove notifier" Mario Limonciello
@ 2026-06-26 11:41 ` Thomas Zimmermann
0 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2026-06-26 11:41 UTC (permalink / raw)
To: Mario Limonciello, dri-devel, harry.wentland, Simona Vetter,
Alex Deucher, Maarten Lankhorst, Maxime Ripard, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Simon Ser
Hi
Am 24.06.26 um 18:57 schrieb Mario Limonciello:
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> This reverts commit 5461f3fd74a89757f95f351eb0bc26aafc2a2e91.
> The backlight notifier support is needed in order to add backlight
> control support into DRM connectors.
>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Tested-by: Simon Ser <contact@emersion.fr>
I don't like this, but I don't have a better idea for now. Therefore
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
until a better idea shows up.
> ---
> drivers/video/backlight/backlight.c | 42 +++++++++++++++++++++++++++++
> include/linux/backlight.h | 20 ++++++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index a22d0bbb6e639..ff2c2084c73a4 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -65,6 +65,7 @@
>
> static struct list_head backlight_dev_list;
> static struct mutex backlight_dev_list_mutex;
> +static struct blocking_notifier_head backlight_notifier;
>
> static const char *const backlight_types[] = {
> [BACKLIGHT_RAW] = "raw",
> @@ -415,6 +416,9 @@ struct backlight_device *backlight_device_register(const char *name,
> list_add(&new_bd->entry, &backlight_dev_list);
> mutex_unlock(&backlight_dev_list_mutex);
>
> + blocking_notifier_call_chain(&backlight_notifier,
> + BACKLIGHT_REGISTERED, new_bd);
> +
> return new_bd;
> }
> EXPORT_SYMBOL(backlight_device_register);
> @@ -484,6 +488,9 @@ void backlight_device_unregister(struct backlight_device *bd)
> mutex_unlock(&pmac_backlight_mutex);
> #endif
>
> + blocking_notifier_call_chain(&backlight_notifier,
> + BACKLIGHT_UNREGISTERED, bd);
> +
> mutex_lock(&bd->ops_lock);
> bd->ops = NULL;
> mutex_unlock(&bd->ops_lock);
> @@ -507,6 +514,40 @@ static int devm_backlight_device_match(struct device *dev, void *res,
> return *r == data;
> }
>
> +/**
> + * backlight_register_notifier - get notified of backlight (un)registration
> + * @nb: notifier block with the notifier to call on backlight (un)registration
> + *
> + * Register a notifier to get notified when backlight devices get registered
> + * or unregistered.
> + *
> + * RETURNS:
> + *
> + * 0 on success, otherwise a negative error code
> + */
> +int backlight_register_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&backlight_notifier, nb);
> +}
> +EXPORT_SYMBOL(backlight_register_notifier);
> +
> +/**
> + * backlight_unregister_notifier - unregister a backlight notifier
> + * @nb: notifier block to unregister
> + *
> + * Register a notifier to get notified when backlight devices get registered
> + * or unregistered.
> + *
> + * RETURNS:
> + *
> + * 0 on success, otherwise a negative error code
> + */
> +int backlight_unregister_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&backlight_notifier, nb);
> +}
> +EXPORT_SYMBOL(backlight_unregister_notifier);
> +
> /**
> * devm_backlight_device_register - register a new backlight device
> * @dev: the device to register
> @@ -674,6 +715,7 @@ static int __init backlight_class_init(void)
>
> INIT_LIST_HEAD(&backlight_dev_list);
> mutex_init(&backlight_dev_list_mutex);
> + BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
>
> return 0;
> }
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index f29a9ef1052e7..d905173c7f73c 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -64,6 +64,24 @@ enum backlight_type {
> BACKLIGHT_TYPE_MAX,
> };
>
> +/**
> + * enum backlight_notification - the type of notification
> + *
> + * The notifications that is used for notification sent to the receiver
> + * that registered notifications using backlight_register_notifier().
> + */
> +enum backlight_notification {
> + /**
> + * @BACKLIGHT_REGISTERED: The backlight device is registered.
> + */
> + BACKLIGHT_REGISTERED,
> +
> + /**
> + * @BACKLIGHT_UNREGISTERED: The backlight revice is unregistered.
> + */
> + BACKLIGHT_UNREGISTERED,
> +};
> +
> /** enum backlight_scale - the type of scale used for brightness values
> *
> * The type of scale used for brightness values.
> @@ -388,6 +406,8 @@ void devm_backlight_device_unregister(struct device *dev,
> struct backlight_device *bd);
> void backlight_force_update(struct backlight_device *bd,
> enum backlight_update_reason reason);
> +int backlight_register_notifier(struct notifier_block *nb);
> +int backlight_unregister_notifier(struct notifier_block *nb);
> struct backlight_device *backlight_device_get_by_name(const char *name);
> struct backlight_device *backlight_device_get_by_type(enum backlight_type type);
> int backlight_device_set_brightness(struct backlight_device *bd,
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 02/10] backlight: add kernel-internal backlight API
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 01/10] Revert "backlight: Remove notifier" Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 03/10] drm: link connectors to backlight devices Mario Limonciello
` (8 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), David Herrmann, Marta Lofstedt
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
So far backlights have only been controlled via sysfs. However, sysfs is
not a proper user-space API for runtime modifications, and never was
intended to provide such. The DRM drivers are now prepared to provide
such a backlight link so user-space can control backlight via DRM
connector properties. This allows us to employ the same access-management
we use for mode-setting.
This patch adds few kernel-internal backlight helpers so we can modify
backlights from within DRM.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
V2: Marta Lofstedt <marta.lofstedt@intel.com>
- rebase
- minor edit for checkpatch warning
Signed-off-by: Marta Lofstedt <marta.lofstedt@intel.com>
V3: Mario Limonciello (AMD) <superm1@kernel.org>
- rebase
- Use guard(mutex)
V4: Mario Limonciello (AMD) <superm1@kernel.org>
- Adjust return type for backlight_set_brightness() to return errors
- Stop clamping in backlight_set_brightness()
- Drop backlight_device_lookup()
V5: Mario Limonciello (AMD) <superm1@kernel.org>
- Drop unnecessary dynamic debug message as backlight_generate_event()
sends a netlink event.
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/video/backlight/backlight.c | 31 +++++++++++++++++++++++++++++
include/linux/backlight.h | 15 ++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index ff2c2084c73a4..cd1a161ae7bc6 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -514,6 +514,37 @@ static int devm_backlight_device_match(struct device *dev, void *res,
return *r == data;
}
+/**
+ * backlight_set_brightness - set brightness on a backlight device
+ * @bd: backlight device to operate on
+ * @value: brightness value to set on the device
+ * @reason: backlight-change reason to use for notifications
+ *
+ * This is the in-kernel API equivalent of writing into the 'brightness' sysfs
+ * file. It calls into the underlying backlight driver to change the brightness
+ * value.
+ * A uevent notification is sent with the reason set to @reason.
+ * Return: 0 if successfully notified, -EINVAL for invalid values
+ */
+int backlight_set_brightness(struct backlight_device *bd, unsigned int value,
+ enum backlight_update_reason reason)
+{
+ int rc = 0;
+
+ guard(mutex)(&bd->ops_lock);
+ if (bd->ops) {
+ if (value > bd->props.max_brightness)
+ return -EINVAL;
+ bd->props.brightness = value;
+ rc = backlight_update_status(bd);
+ }
+ if (rc == 0)
+ backlight_generate_event(bd, reason);
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(backlight_set_brightness);
+
/**
* backlight_register_notifier - get notified of backlight (un)registration
* @nb: notifier block with the notifier to call on backlight (un)registration
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index d905173c7f73c..204eea9256fd7 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -429,6 +429,21 @@ static inline void backlight_notify_blank_all(struct device *display_dev,
{ }
#endif
+int backlight_set_brightness(struct backlight_device *bd, unsigned int value,
+ enum backlight_update_reason reason);
+
+static inline void backlight_device_ref(struct backlight_device *bd)
+{
+ if (bd)
+ get_device(&bd->dev);
+}
+
+static inline void backlight_device_unref(struct backlight_device *bd)
+{
+ if (bd)
+ put_device(&bd->dev);
+}
+
#define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
/**
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v6 03/10] drm: link connectors to backlight devices
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 01/10] Revert "backlight: Remove notifier" Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 02/10] backlight: add kernel-internal backlight API Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-26 7:34 ` Maxime Ripard
2026-06-26 11:49 ` Thomas Zimmermann
2026-06-24 16:57 ` [PATCH v6 04/10] DRM: Add support for client indicating support for luminance Mario Limonciello
` (7 subsequent siblings)
10 siblings, 2 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
This will show which connector in sysfs matches which backlight.
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # SM8150-HDK
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_backlight.c | 371 ++++++++++++++++++++++++++++
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 | 28 ++-
drivers/video/backlight/backlight.c | 17 ++
include/drm/drm_backlight.h | 51 ++++
include/drm/drm_connector.h | 3 +
include/drm/drm_mode_config.h | 5 +
include/linux/backlight.h | 13 +
13 files changed, 578 insertions(+), 5 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 323422861e8f6..d6035bbbdc83f 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..bf980a2ac1e6b 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -78,6 +78,7 @@ drm-$(CONFIG_DRM_CLIENT) += \
drm_client_event.o \
drm_client_modeset.o \
drm_client_sysrq.o
+drm-y += drm_backlight.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..b1ec470be86ca
--- /dev/null
+++ b/drivers/gpu/drm/drm_backlight.c
@@ -0,0 +1,371 @@
+// 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 a linked backlight device. The links between connectors and
+ * backlight devices are established by DRM drivers; user-space cannot create
+ * or modify these links. A 'change' uevent is sent whenever the brightness is
+ * updated.
+ *
+ * Drivers have to call drm_backlight_alloc() after allocating a connector via
+ * drm_connector_init(). This will automatically add a backlight device to the
+ * given connector. Drivers must then link a hardware backlight by calling
+ * drm_backlight_link() with the registered backlight_device. If no link is
+ * established, the DRM backlight property reports an empty range and
+ * brightness changes are no-ops.
+ */
+
+struct drm_backlight {
+ struct list_head list;
+ struct drm_connector *connector;
+ struct backlight_device *link;
+ /*
+ * Number of luminance-aware DRM clients that have taken over this
+ * connector's backlight. While > 0, legacy sysfs writes to the
+ * linked backlight_device return -EBUSY. Protected by
+ * drm_backlight_lock.
+ */
+ unsigned int luminance_clients;
+};
+
+static LIST_HEAD(drm_backlight_list);
+static DEFINE_SPINLOCK(drm_backlight_lock);
+
+/* caller must hold @drm_backlight_lock */
+static bool __drm_backlight_is_registered(struct drm_backlight *b)
+{
+ lockdep_assert_held(&drm_backlight_lock);
+ /* a device is live if it is linked to @drm_backlight_list */
+ return !list_empty(&b->list);
+}
+
+/* caller must hold @drm_backlight_lock */
+static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
+{
+ unsigned int max, set;
+
+ lockdep_assert_held(&drm_backlight_lock);
+
+ if (!b->link)
+ return;
+
+ max = b->link->props.max_brightness;
+ if (max < 1)
+ return;
+
+ set = v;
+ if (set >= max)
+ set = max;
+}
+
+/**
+ * __drm_backlight_update_prop_range - update the luminance property range
+ * @b: backlight device
+ *
+ * Updates the luminance property range based on the linked backlight device's
+ * max_brightness. If no device is linked, sets range to 0-0 to indicate
+ * unavailability.
+ */
+static void __drm_backlight_update_prop_range(struct drm_backlight *b)
+{
+ struct drm_device *dev = b->connector->dev;
+ struct drm_property *prop = dev->mode_config.luminance_property;
+ unsigned int max = 0;
+
+ lockdep_assert_held(&drm_backlight_lock);
+
+ if (b->link && b->link->props.max_brightness > 0)
+ max = b->link->props.max_brightness;
+
+ /* Update property range to match hardware capabilities.
+ * Range of 0-0 indicates no backing device.
+ * Range of 1-max for normal operation (0 reserved for display off).
+ */
+ if (prop->values[1] != max) {
+ prop->values[0] = max ? 1 : 0;
+ prop->values[1] = max;
+ }
+}
+
+/* caller must hold @drm_backlight_lock */
+static bool __drm_backlight_link(struct drm_backlight *b,
+ struct backlight_device *bd)
+{
+ if (bd == b->link)
+ return false;
+
+ backlight_device_unref(b->link);
+ b->link = bd;
+ backlight_device_ref(b->link);
+ if (bd)
+ __drm_backlight_real_changed(b, bd->props.brightness);
+ __drm_backlight_update_prop_range(b);
+
+ return true;
+}
+
+/**
+ * drm_backlight_alloc - add backlight capability to a connector
+ * @connector: connector to add backlight to
+ *
+ * This allocates a new DRM-backlight device and attaches it to @connector.
+ * This *must* be called before registering the connector. The backlight
+ * device will be automatically registered in sync with the connector. It will
+ * also get removed once the connector is removed.
+ *
+ * No hardware backlight is linked by default. Drivers must call
+ * drm_backlight_link() to associate a registered backlight_device with the
+ * connector. User-space cannot create or modify this link.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int drm_backlight_alloc(struct drm_connector *connector)
+{
+ struct drm_mode_config *config = &connector->dev->mode_config;
+ struct drm_backlight *b;
+
+ b = kzalloc_obj(*b, GFP_KERNEL);
+ if (!b)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&b->list);
+ b->connector = connector;
+ connector->backlight = b;
+
+ drm_object_attach_property(&connector->base,
+ config->luminance_property, 0);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_backlight_alloc);
+
+void drm_backlight_free(struct drm_connector *connector)
+{
+ struct drm_backlight *b = connector->backlight;
+
+ if (!b)
+ return;
+
+ WARN_ON(__drm_backlight_is_registered(b));
+ WARN_ON(b->link);
+
+ kfree(b);
+ connector->backlight = NULL;
+}
+EXPORT_SYMBOL(drm_backlight_free);
+
+void drm_backlight_register(struct drm_backlight *b)
+{
+ if (!b)
+ return;
+
+ WARN_ON(__drm_backlight_is_registered(b));
+
+ guard(spinlock)(&drm_backlight_lock);
+ list_add(&b->list, &drm_backlight_list);
+}
+EXPORT_SYMBOL(drm_backlight_register);
+
+void drm_backlight_unregister(struct drm_backlight *b)
+{
+ if (!b)
+ return;
+
+ WARN_ON(!__drm_backlight_is_registered(b));
+
+ scoped_guard(spinlock, &drm_backlight_lock) {
+ list_del_init(&b->list);
+ __drm_backlight_link(b, NULL);
+ }
+}
+EXPORT_SYMBOL(drm_backlight_unregister);
+
+/**
+ * drm_backlight_link - link a backlight device to a DRM backlight
+ * @b: DRM backlight to modify
+ * @bd: backlight device to link, or NULL to unlink
+ *
+ * Establish the link between a DRM connector's backlight property and a
+ * registered backlight_device. Drivers must call this with the
+ * backlight_device they registered for the connector. Passing NULL unlinks
+ * any previously linked device.
+ *
+ * The caller is responsible for ensuring @bd remains valid until either it
+ * is unlinked via drm_backlight_link(b, NULL) or the connector is
+ * unregistered.
+ *
+ * Whenever a hardware backlight is linked or unlinked, a uevent with
+ * "BACKLIGHT=1" is generated on the connector.
+ */
+void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd)
+{
+ if (!b)
+ return;
+
+ guard(spinlock)(&drm_backlight_lock);
+ __drm_backlight_link(b, bd);
+}
+EXPORT_SYMBOL(drm_backlight_link);
+
+/**
+ * drm_backlight_get_device - get the backlight_device linked to a DRM backlight
+ * @b: DRM backlight
+ *
+ * Returns the &backlight_device linked to @b, or NULL if no device is linked
+ * or @b is NULL. The caller must hold the appropriate lock to prevent the
+ * link from changing while the pointer is in use.
+ */
+struct backlight_device *drm_backlight_get_device(struct drm_backlight *b)
+{
+ if (!b)
+ return NULL;
+
+ guard(spinlock)(&drm_backlight_lock);
+ return b->link;
+}
+EXPORT_SYMBOL(drm_backlight_get_device);
+
+/**
+ * drm_backlight_inhibit_legacy - disable legacy sysfs control of the linked bd
+ * @b: DRM backlight to inhibit
+ *
+ * Record that one more luminance-aware DRM client has taken over @b. While
+ * any clients are recorded, writes to the linked backlight_device's legacy
+ * ``brightness`` sysfs attribute return ``-EBUSY``. The takeover follows
+ * @b->link if the link changes.
+ *
+ * Calls must be balanced with drm_backlight_uninhibit_legacy().
+ */
+void drm_backlight_inhibit_legacy(struct drm_backlight *b)
+{
+ if (!b)
+ return;
+}
+EXPORT_SYMBOL(drm_backlight_inhibit_legacy);
+
+/**
+ * drm_backlight_uninhibit_legacy - re-enable legacy sysfs control
+ * @b: DRM backlight to uninhibit
+ *
+ * Balances a previous drm_backlight_inhibit_legacy() call. When the last
+ * luminance-aware client goes away, legacy sysfs writes are allowed again.
+ */
+void drm_backlight_uninhibit_legacy(struct drm_backlight *b)
+{
+ if (!b)
+ return;
+}
+EXPORT_SYMBOL(drm_backlight_uninhibit_legacy);
+
+/**
+ * drm_backlight_inhibit_legacy_all - inhibit legacy sysfs on every connector
+ * @dev: DRM device whose connectors should be inhibited
+ *
+ * Walks all connectors on @dev and calls drm_backlight_inhibit_legacy() on
+ * each connector that has a DRM backlight attached. Used when a client
+ * declares it is luminance-aware via DRM_CLIENT_CAP_LUMINANCE.
+ */
+void drm_backlight_inhibit_legacy_all(struct drm_device *dev)
+{
+ struct drm_connector_list_iter iter;
+ struct drm_connector *connector;
+
+ drm_connector_list_iter_begin(dev, &iter);
+ drm_for_each_connector_iter(connector, &iter)
+ drm_backlight_inhibit_legacy(connector->backlight);
+ drm_connector_list_iter_end(&iter);
+}
+EXPORT_SYMBOL(drm_backlight_inhibit_legacy_all);
+
+/**
+ * drm_backlight_uninhibit_legacy_all - reverse drm_backlight_inhibit_legacy_all()
+ * @dev: DRM device whose connectors should be uninhibited
+ */
+void drm_backlight_uninhibit_legacy_all(struct drm_device *dev)
+{
+ struct drm_connector_list_iter iter;
+ struct drm_connector *connector;
+
+ drm_connector_list_iter_begin(dev, &iter);
+ drm_for_each_connector_iter(connector, &iter)
+ drm_backlight_uninhibit_legacy(connector->backlight);
+ drm_connector_list_iter_end(&iter);
+}
+EXPORT_SYMBOL(drm_backlight_uninhibit_legacy_all);
+
+void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value)
+{
+ guard(spinlock)(&drm_backlight_lock);
+ __drm_backlight_real_changed(b, value);
+}
+EXPORT_SYMBOL(drm_backlight_set_luminance);
+
+static int drm_backlight_notify(struct notifier_block *self,
+ unsigned long event, void *data)
+{
+ struct backlight_device *bd = data;
+ struct drm_backlight *b;
+
+ guard(spinlock)(&drm_backlight_lock);
+
+ switch (event) {
+ case BACKLIGHT_UNREGISTERED:
+ list_for_each_entry(b, &drm_backlight_list, list)
+ if (b->link == bd)
+ __drm_backlight_link(b, NULL);
+
+ break;
+ case BACKLIGHT_BRIGHTNESS_CHANGED:
+ /* Update DRM property value when hardware backlight changes */
+ list_for_each_entry(b, &drm_backlight_list, list)
+ if (b->link == bd)
+ __drm_backlight_real_changed(b, bd->props.brightness);
+
+ break;
+ }
+
+ return 0;
+}
+
+static struct notifier_block drm_backlight_notifier = {
+ .notifier_call = drm_backlight_notify,
+};
+
+int drm_backlight_init(void)
+{
+ return backlight_register_notifier(&drm_backlight_notifier);
+}
+
+void drm_backlight_exit(void)
+{
+ backlight_unregister_notifier(&drm_backlight_notifier);
+}
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 3fa4d2082cd72..128d431f0d6b0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -21,6 +21,7 @@
*/
#include <drm/drm_auth.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_connector.h>
#include <drm/drm_drv.h>
#include <drm/drm_edid.h>
@@ -760,6 +761,7 @@ void drm_connector_cleanup(struct drm_connector *connector)
struct drm_device *dev = connector->dev;
struct drm_display_mode *mode, *t;
+ drm_backlight_free(connector);
/* The connector should have been removed from userspace long before
* it is finally destroyed.
*/
@@ -845,6 +847,8 @@ int drm_connector_register(struct drm_connector *connector)
if (connector->registration_state != DRM_CONNECTOR_INITIALIZING)
goto unlock;
+ drm_backlight_register(connector->backlight);
+
ret = drm_sysfs_connector_add(connector);
if (ret)
goto unlock;
@@ -931,6 +935,8 @@ EXPORT_SYMBOL(drm_connector_dynamic_register);
void drm_connector_unregister(struct drm_connector *connector)
{
mutex_lock(&connector->mutex);
+ drm_backlight_unregister(connector->backlight);
+
if (connector->registration_state != DRM_CONNECTOR_REGISTERED) {
mutex_unlock(&connector->mutex);
return;
@@ -3252,10 +3258,16 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
{
int ret = -EINVAL;
struct drm_connector *connector = obj_to_connector(obj);
+ struct drm_mode_config *config = &connector->dev->mode_config;
/* Do DPMS ourselves */
if (property == connector->dev->mode_config.dpms_property) {
ret = (*connector->funcs->dpms)(connector, (int)value);
+ } else if (property == config->luminance_property) {
+ if (connector->backlight && connector->dpms == DRM_MODE_DPMS_ON)
+ drm_backlight_set_luminance(connector->backlight,
+ value);
+ ret = 0;
} else if (connector->funcs->set_property)
ret = connector->funcs->set_property(connector, property, value);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 675675480da49..5bf402197867a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -42,6 +42,7 @@
#include <linux/xarray.h>
#include <drm/drm_accel.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_bridge.h>
#include <drm/drm_cache.h>
#include <drm/drm_client_event.h>
@@ -1230,6 +1231,7 @@ static void drm_core_exit(void)
drm_privacy_screen_lookup_exit();
drm_panic_exit();
accel_core_exit();
+ drm_backlight_exit();
unregister_chrdev(DRM_MAJOR, "drm");
drm_debugfs_remove_root();
drm_sysfs_destroy();
@@ -1253,6 +1255,12 @@ static int __init drm_core_init(void)
drm_debugfs_init_root();
drm_debugfs_bridge_params();
+ ret = drm_backlight_init();
+ if (ret < 0) {
+ DRM_ERROR("Cannot initialize backlight interface\n");
+ goto error;
+ }
+
ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
if (ret < 0)
goto error;
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 66f7dc37b5970..25c5d29694624 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -32,6 +32,7 @@
#include <drm/drm_print.h>
#include <drm/drm_colorop.h>
#include <linux/dma-resv.h>
+#include <drm/drm_backlight.h>
#include "drm_crtc_internal.h"
#include "drm_internal.h"
@@ -407,6 +408,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
return -ENOMEM;
dev->mode_config.size_hints_property = prop;
+ prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+ "LUMINANCE", 0, 0);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.luminance_property = prop;
+
return 0;
}
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 21fc9deda4371..a50d33af95e8c 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -30,6 +30,7 @@
#include <drm/drm_mode_object.h>
#include <drm/drm_plane.h>
#include <drm/drm_print.h>
+#include <drm/drm_backlight.h>
#include "drm_crtc_internal.h"
@@ -287,11 +288,72 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
{
int i;
- WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
- !(property->flags & DRM_MODE_PROP_IMMUTABLE));
+ /* Exempt dpms and luminance properties from the atomic warning, as these
+ * have special interdependencies managed internally in this function
+ */
+ if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
+ struct drm_connector *connector = obj_to_connector(obj);
+ struct drm_mode_config *config = &connector->dev->mode_config;
+
+ if (property != config->dpms_property &&
+ property != config->luminance_property) {
+ WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
+ !(property->flags & DRM_MODE_PROP_IMMUTABLE));
+ }
+ } else {
+ WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
+ !(property->flags & DRM_MODE_PROP_IMMUTABLE));
+ }
for (i = 0; i < obj->properties->count; i++) {
+ /* If properties depends on each other
+ * this is where to resolve that issue
+ */
if (obj->properties->properties[i] == property) {
+ /* Connector-specific property interdependencies */
+ if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
+ struct drm_connector *connector = obj_to_connector(obj);
+ struct drm_property *dpms_property =
+ connector->dev->mode_config.dpms_property;
+ struct drm_property *luminance_property =
+ connector->dev->mode_config.luminance_property;
+
+ if (property == dpms_property) {
+ /* When DPMS goes from ON -> OFF,
+ * set the brightness to the minimum possible
+ * to save power.
+ */
+ if (obj->properties->values[i] !=
+ DRM_MODE_DPMS_OFF &&
+ val == DRM_MODE_DPMS_OFF)
+ drm_backlight_set_luminance(
+ connector->backlight, 0);
+ /* When DPMS OFF -> ON, reset the brightness
+ * to the original level
+ */
+ else if (obj->properties->values[i] ==
+ DRM_MODE_DPMS_OFF &&
+ val != DRM_MODE_DPMS_OFF) {
+ uint64_t value;
+
+ drm_object_property_get_value(
+ obj, luminance_property,
+ &value);
+ drm_backlight_set_luminance(
+ connector->backlight, value);
+ }
+ } else if (property == luminance_property) {
+ /* Always update the property value to remember
+ * the user's desired brightness, but only update
+ * hardware when DPMS is ON.
+ */
+ obj->properties->values[i] = val;
+ if (connector->dpms == DRM_MODE_DPMS_ON)
+ drm_backlight_set_luminance(
+ connector->backlight, val);
+ return 0;
+ }
+ }
obj->properties->values[i] = val;
return 0;
}
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ef4e923a87284..8609e1660f210 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -11,6 +11,7 @@
*/
#include <linux/acpi.h>
+#include <linux/backlight.h>
#include <linux/component.h>
#include <linux/device.h>
#include <linux/err.h>
@@ -27,6 +28,7 @@
#include <drm/drm_device.h>
#include <drm/drm_file.h>
#include <drm/drm_modes.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_print.h>
#include <drm/drm_property.h>
#include <drm/drm_sysfs.h>
@@ -391,15 +393,35 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
int drm_sysfs_connector_add_late(struct drm_connector *connector)
{
- if (connector->ddc)
- return sysfs_create_link(&connector->kdev->kobj,
- &connector->ddc->dev.kobj, "ddc");
+ if (connector->ddc) {
+ int ret = sysfs_create_link(&connector->kdev->kobj,
+ &connector->ddc->dev.kobj, "ddc");
+ if (ret)
+ return ret;
+ }
+
+ if (connector->backlight) {
+ struct backlight_device *bd = drm_backlight_get_device(connector->backlight);
+
+ if (bd) {
+ int ret = sysfs_create_link(&connector->kdev->kobj,
+ &bd->dev.kobj, "backlight");
+ if (ret) {
+ if (connector->ddc)
+ sysfs_remove_link(&connector->kdev->kobj, "ddc");
+ return ret;
+ }
+ }
+ }
return 0;
}
void drm_sysfs_connector_remove_early(struct drm_connector *connector)
{
+ if (connector->backlight && drm_backlight_get_device(connector->backlight))
+ sysfs_remove_link(&connector->kdev->kobj, "backlight");
+
if (connector->ddc)
sysfs_remove_link(&connector->kdev->kobj, "ddc");
}
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index cd1a161ae7bc6..13954c2220b7e 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -126,6 +126,9 @@ static void backlight_generate_event(struct backlight_device *bd,
case BACKLIGHT_UPDATE_HOTKEY:
envp[0] = "SOURCE=hotkey";
break;
+ case BACKLIGHT_UPDATE_DRM:
+ envp[0] = "SOURCE=drm";
+ break;
default:
envp[0] = "SOURCE=unknown";
break;
@@ -579,6 +582,20 @@ int backlight_unregister_notifier(struct notifier_block *nb)
}
EXPORT_SYMBOL(backlight_unregister_notifier);
+/**
+ * backlight_notify_brightness - notify brightness change to listeners
+ * @bd: backlight device that changed
+ *
+ * Notify registered listeners that the backlight brightness has changed.
+ * This is called automatically after successful brightness updates.
+ */
+void backlight_notify_brightness(struct backlight_device *bd)
+{
+ blocking_notifier_call_chain(&backlight_notifier,
+ BACKLIGHT_BRIGHTNESS_CHANGED, bd);
+}
+EXPORT_SYMBOL(backlight_notify_brightness);
+
/**
* devm_backlight_device_register - register a new backlight device
* @dev: the device to register
diff --git a/include/drm/drm_backlight.h b/include/drm/drm_backlight.h
new file mode 100644
index 0000000000000..e0e09e38f7c06
--- /dev/null
+++ b/include/drm/drm_backlight.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: MIT */
+#ifndef __DRM_BACKLIGHT_H__
+#define __DRM_BACKLIGHT_H__
+
+/*
+ * Copyright (c) 2014 David Herrmann <dh.herrmann at gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+
+struct backlight_device;
+struct drm_backlight;
+struct drm_connector;
+struct drm_device;
+struct drm_mode_object;
+
+int drm_backlight_init(void);
+void drm_backlight_exit(void);
+
+int drm_backlight_alloc(struct drm_connector *connector);
+void drm_backlight_free(struct drm_connector *connector);
+void drm_backlight_register(struct drm_backlight *b);
+void drm_backlight_unregister(struct drm_backlight *b);
+
+void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd);
+struct backlight_device *drm_backlight_get_device(struct drm_backlight *b);
+void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
+void drm_backlight_inhibit_legacy(struct drm_backlight *b);
+void drm_backlight_uninhibit_legacy(struct drm_backlight *b);
+void drm_backlight_inhibit_legacy_all(struct drm_device *dev);
+void drm_backlight_uninhibit_legacy_all(struct drm_device *dev);
+#endif /* __DRM_BACKLIGHT_H__ */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5ad62c207d009..d78ac1068d12e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2410,6 +2410,9 @@ struct drm_connector {
* @cec: CEC-related data.
*/
struct drm_connector_cec cec;
+
+ /* backlight link */
+ struct drm_backlight *backlight;
};
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index e584652ddf676..b6d88319d5d0f 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -852,6 +852,11 @@ struct drm_mode_config {
* the position of the output on the host's screen.
*/
struct drm_property *suggested_y_property;
+ /**
+ * @luminance_property: Default connector property to control the
+ * connector's backlight luminance.
+ */
+ struct drm_property *luminance_property;
/**
* @non_desktop_property: Optional connector property with a hint
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 204eea9256fd7..26a7281d179c1 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -29,6 +29,7 @@ enum backlight_update_reason {
* @BACKLIGHT_UPDATE_SYSFS: The backlight was updated using sysfs.
*/
BACKLIGHT_UPDATE_SYSFS,
+ BACKLIGHT_UPDATE_DRM,
};
/**
@@ -80,6 +81,11 @@ enum backlight_notification {
* @BACKLIGHT_UNREGISTERED: The backlight revice is unregistered.
*/
BACKLIGHT_UNREGISTERED,
+
+ /**
+ * @BACKLIGHT_BRIGHTNESS_CHANGED: The backlight brightness has changed.
+ */
+ BACKLIGHT_BRIGHTNESS_CHANGED,
};
/** enum backlight_scale - the type of scale used for brightness values
@@ -310,6 +316,9 @@ struct backlight_device {
int use_count;
};
+/* Forward declaration for backlight_update_status */
+void backlight_notify_brightness(struct backlight_device *bd);
+
/**
* backlight_update_status - force an update of the backlight device status
* @bd: the backlight device
@@ -323,6 +332,10 @@ static inline int backlight_update_status(struct backlight_device *bd)
ret = bd->ops->update_status(bd);
mutex_unlock(&bd->update_lock);
+ /* Notify DRM and other listeners that brightness changed */
+ if (ret == 0)
+ backlight_notify_brightness(bd);
+
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v6 03/10] drm: link connectors to backlight devices
2026-06-24 16:57 ` [PATCH v6 03/10] drm: link connectors to backlight devices Mario Limonciello
@ 2026-06-26 7:34 ` Maxime Ripard
2026-06-26 21:40 ` Mario Limonciello
2026-06-26 11:49 ` Thomas Zimmermann
1 sibling, 1 reply; 19+ messages in thread
From: Maxime Ripard @ 2026-06-26 7:34 UTC (permalink / raw)
To: Mario Limonciello
Cc: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Xaver Hugl,
amd-gfx, open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov
[-- Attachment #1: Type: text/plain, Size: 30269 bytes --]
On Wed, Jun 24, 2026 at 09:57:43AM -0700, Mario Limonciello wrote:
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> This will show which connector in sysfs matches which backlight.
>
> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # SM8150-HDK
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_backlight.c | 371 ++++++++++++++++++++++++++++
> 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 | 28 ++-
> drivers/video/backlight/backlight.c | 17 ++
> include/drm/drm_backlight.h | 51 ++++
> include/drm/drm_connector.h | 3 +
> include/drm/drm_mode_config.h | 5 +
> include/linux/backlight.h | 13 +
> 13 files changed, 578 insertions(+), 5 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 323422861e8f6..d6035bbbdc83f 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..bf980a2ac1e6b 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -78,6 +78,7 @@ drm-$(CONFIG_DRM_CLIENT) += \
> drm_client_event.o \
> drm_client_modeset.o \
> drm_client_sysrq.o
> +drm-y += drm_backlight.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..b1ec470be86ca
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_backlight.c
> @@ -0,0 +1,371 @@
> +// 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 a linked backlight device. The links between connectors and
> + * backlight devices are established by DRM drivers; user-space cannot create
> + * or modify these links. A 'change' uevent is sent whenever the brightness is
> + * updated.
I think we should explain why, because if it's a property and comes from
the userspace, then why would luminance be special and trigger a uevent
when it's updated, unlike any other property?
> + * Drivers have to call drm_backlight_alloc() after allocating a connector via
> + * drm_connector_init(). This will automatically add a backlight device to the
> + * given connector. Drivers must then link a hardware backlight by calling
> + * drm_backlight_link() with the registered backlight_device. If no link is
> + * established, the DRM backlight property reports an empty range and
> + * brightness changes are no-ops.
> + */
It's not clear to me why we need to dynamically allocate them at all. If
we're using the backlight subsystem we'll already have a handle to it.
If we don't and want to implement something like DDC/CI, then it just
becomes a hassle. Why not treat it like i2c, add a backlight field to
drm_connector, and create the link at registration, add a new hook to
set luminance, and then provide helpers to either use the backlight API,
or anything else if the driver wants to.
> +struct drm_backlight {
> + struct list_head list;
> + struct drm_connector *connector;
> + struct backlight_device *link;
> + /*
> + * Number of luminance-aware DRM clients that have taken over this
> + * connector's backlight. While > 0, legacy sysfs writes to the
> + * linked backlight_device return -EBUSY. Protected by
> + * drm_backlight_lock.
> + */
> + unsigned int luminance_clients;
> +};
> +
> +static LIST_HEAD(drm_backlight_list);
> +static DEFINE_SPINLOCK(drm_backlight_lock);
> +
> +/* caller must hold @drm_backlight_lock */
> +static bool __drm_backlight_is_registered(struct drm_backlight *b)
> +{
> + lockdep_assert_held(&drm_backlight_lock);
> + /* a device is live if it is linked to @drm_backlight_list */
> + return !list_empty(&b->list);
> +}
> +
> +/* caller must hold @drm_backlight_lock */
> +static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
> +{
> + unsigned int max, set;
> +
> + lockdep_assert_held(&drm_backlight_lock);
> +
> + if (!b->link)
> + return;
> +
> + max = b->link->props.max_brightness;
> + if (max < 1)
> + return;
> +
> + set = v;
> + if (set >= max)
> + set = max;
> +}
> +
> +/**
> + * __drm_backlight_update_prop_range - update the luminance property range
> + * @b: backlight device
> + *
> + * Updates the luminance property range based on the linked backlight device's
> + * max_brightness. If no device is linked, sets range to 0-0 to indicate
> + * unavailability.
> + */
> +static void __drm_backlight_update_prop_range(struct drm_backlight *b)
> +{
> + struct drm_device *dev = b->connector->dev;
> + struct drm_property *prop = dev->mode_config.luminance_property;
> + unsigned int max = 0;
> +
> + lockdep_assert_held(&drm_backlight_lock);
> +
> + if (b->link && b->link->props.max_brightness > 0)
> + max = b->link->props.max_brightness;
> +
> + /* Update property range to match hardware capabilities.
> + * Range of 0-0 indicates no backing device.
> + * Range of 1-max for normal operation (0 reserved for display off).
> + */
> + if (prop->values[1] != max) {
> + prop->values[0] = max ? 1 : 0;
> + prop->values[1] = max;
> + }
> +}
> +
> +/* caller must hold @drm_backlight_lock */
> +static bool __drm_backlight_link(struct drm_backlight *b,
> + struct backlight_device *bd)
> +{
> + if (bd == b->link)
> + return false;
> +
> + backlight_device_unref(b->link);
> + b->link = bd;
> + backlight_device_ref(b->link);
> + if (bd)
> + __drm_backlight_real_changed(b, bd->props.brightness);
> + __drm_backlight_update_prop_range(b);
> +
> + return true;
> +}
> +
> +/**
> + * drm_backlight_alloc - add backlight capability to a connector
> + * @connector: connector to add backlight to
> + *
> + * This allocates a new DRM-backlight device and attaches it to @connector.
> + * This *must* be called before registering the connector. The backlight
> + * device will be automatically registered in sync with the connector. It will
> + * also get removed once the connector is removed.
> + *
> + * No hardware backlight is linked by default. Drivers must call
> + * drm_backlight_link() to associate a registered backlight_device with the
> + * connector. User-space cannot create or modify this link.
> + *
> + * Returns: 0 on success, negative error code on failure.
> + */
> +int drm_backlight_alloc(struct drm_connector *connector)
> +{
> + struct drm_mode_config *config = &connector->dev->mode_config;
> + struct drm_backlight *b;
> +
> + b = kzalloc_obj(*b, GFP_KERNEL);
> + if (!b)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&b->list);
> + b->connector = connector;
> + connector->backlight = b;
> +
> + drm_object_attach_property(&connector->base,
> + config->luminance_property, 0);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_backlight_alloc);
> +
> +void drm_backlight_free(struct drm_connector *connector)
> +{
> + struct drm_backlight *b = connector->backlight;
> +
> + if (!b)
> + return;
> +
> + WARN_ON(__drm_backlight_is_registered(b));
> + WARN_ON(b->link);
> +
> + kfree(b);
> + connector->backlight = NULL;
> +}
> +EXPORT_SYMBOL(drm_backlight_free);
> +
> +void drm_backlight_register(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +
> + WARN_ON(__drm_backlight_is_registered(b));
> +
> + guard(spinlock)(&drm_backlight_lock);
> + list_add(&b->list, &drm_backlight_list);
> +}
> +EXPORT_SYMBOL(drm_backlight_register);
> +
> +void drm_backlight_unregister(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +
> + WARN_ON(!__drm_backlight_is_registered(b));
> +
> + scoped_guard(spinlock, &drm_backlight_lock) {
> + list_del_init(&b->list);
> + __drm_backlight_link(b, NULL);
> + }
> +}
> +EXPORT_SYMBOL(drm_backlight_unregister);
> +
> +/**
> + * drm_backlight_link - link a backlight device to a DRM backlight
> + * @b: DRM backlight to modify
> + * @bd: backlight device to link, or NULL to unlink
> + *
> + * Establish the link between a DRM connector's backlight property and a
> + * registered backlight_device. Drivers must call this with the
> + * backlight_device they registered for the connector. Passing NULL unlinks
> + * any previously linked device.
> + *
> + * The caller is responsible for ensuring @bd remains valid until either it
> + * is unlinked via drm_backlight_link(b, NULL) or the connector is
> + * unregistered.
> + *
> + * Whenever a hardware backlight is linked or unlinked, a uevent with
> + * "BACKLIGHT=1" is generated on the connector.
> + */
> +void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd)
> +{
> + if (!b)
> + return;
> +
> + guard(spinlock)(&drm_backlight_lock);
> + __drm_backlight_link(b, bd);
> +}
> +EXPORT_SYMBOL(drm_backlight_link);
> +
> +/**
> + * drm_backlight_get_device - get the backlight_device linked to a DRM backlight
> + * @b: DRM backlight
> + *
> + * Returns the &backlight_device linked to @b, or NULL if no device is linked
> + * or @b is NULL. The caller must hold the appropriate lock to prevent the
> + * link from changing while the pointer is in use.
> + */
> +struct backlight_device *drm_backlight_get_device(struct drm_backlight *b)
> +{
> + if (!b)
> + return NULL;
> +
> + guard(spinlock)(&drm_backlight_lock);
> + return b->link;
> +}
> +EXPORT_SYMBOL(drm_backlight_get_device);
> +
> +/**
> + * drm_backlight_inhibit_legacy - disable legacy sysfs control of the linked bd
> + * @b: DRM backlight to inhibit
> + *
> + * Record that one more luminance-aware DRM client has taken over @b. While
> + * any clients are recorded, writes to the linked backlight_device's legacy
> + * ``brightness`` sysfs attribute return ``-EBUSY``. The takeover follows
> + * @b->link if the link changes.
> + *
> + * Calls must be balanced with drm_backlight_uninhibit_legacy().
> + */
> +void drm_backlight_inhibit_legacy(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +}
> +EXPORT_SYMBOL(drm_backlight_inhibit_legacy);
> +
> +/**
> + * drm_backlight_uninhibit_legacy - re-enable legacy sysfs control
> + * @b: DRM backlight to uninhibit
> + *
> + * Balances a previous drm_backlight_inhibit_legacy() call. When the last
> + * luminance-aware client goes away, legacy sysfs writes are allowed again.
> + */
> +void drm_backlight_uninhibit_legacy(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +}
> +EXPORT_SYMBOL(drm_backlight_uninhibit_legacy);
> +
> +/**
> + * drm_backlight_inhibit_legacy_all - inhibit legacy sysfs on every connector
> + * @dev: DRM device whose connectors should be inhibited
> + *
> + * Walks all connectors on @dev and calls drm_backlight_inhibit_legacy() on
> + * each connector that has a DRM backlight attached. Used when a client
> + * declares it is luminance-aware via DRM_CLIENT_CAP_LUMINANCE.
> + */
> +void drm_backlight_inhibit_legacy_all(struct drm_device *dev)
> +{
> + struct drm_connector_list_iter iter;
> + struct drm_connector *connector;
> +
> + drm_connector_list_iter_begin(dev, &iter);
> + drm_for_each_connector_iter(connector, &iter)
> + drm_backlight_inhibit_legacy(connector->backlight);
> + drm_connector_list_iter_end(&iter);
> +}
> +EXPORT_SYMBOL(drm_backlight_inhibit_legacy_all);
> +
> +/**
> + * drm_backlight_uninhibit_legacy_all - reverse drm_backlight_inhibit_legacy_all()
> + * @dev: DRM device whose connectors should be uninhibited
> + */
> +void drm_backlight_uninhibit_legacy_all(struct drm_device *dev)
> +{
> + struct drm_connector_list_iter iter;
> + struct drm_connector *connector;
> +
> + drm_connector_list_iter_begin(dev, &iter);
> + drm_for_each_connector_iter(connector, &iter)
> + drm_backlight_uninhibit_legacy(connector->backlight);
> + drm_connector_list_iter_end(&iter);
> +}
> +EXPORT_SYMBOL(drm_backlight_uninhibit_legacy_all);
> +
> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value)
> +{
> + guard(spinlock)(&drm_backlight_lock);
> + __drm_backlight_real_changed(b, value);
> +}
> +EXPORT_SYMBOL(drm_backlight_set_luminance);
> +
> +static int drm_backlight_notify(struct notifier_block *self,
> + unsigned long event, void *data)
> +{
> + struct backlight_device *bd = data;
> + struct drm_backlight *b;
> +
> + guard(spinlock)(&drm_backlight_lock);
> +
> + switch (event) {
> + case BACKLIGHT_UNREGISTERED:
> + list_for_each_entry(b, &drm_backlight_list, list)
> + if (b->link == bd)
> + __drm_backlight_link(b, NULL);
> +
> + break;
> + case BACKLIGHT_BRIGHTNESS_CHANGED:
> + /* Update DRM property value when hardware backlight changes */
> + list_for_each_entry(b, &drm_backlight_list, list)
> + if (b->link == bd)
> + __drm_backlight_real_changed(b, bd->props.brightness);
> +
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static struct notifier_block drm_backlight_notifier = {
> + .notifier_call = drm_backlight_notify,
> +};
> +
> +int drm_backlight_init(void)
> +{
> + return backlight_register_notifier(&drm_backlight_notifier);
> +}
> +
> +void drm_backlight_exit(void)
> +{
> + backlight_unregister_notifier(&drm_backlight_notifier);
> +}
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 3fa4d2082cd72..128d431f0d6b0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -21,6 +21,7 @@
> */
>
> #include <drm/drm_auth.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_connector.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_edid.h>
> @@ -760,6 +761,7 @@ void drm_connector_cleanup(struct drm_connector *connector)
> struct drm_device *dev = connector->dev;
> struct drm_display_mode *mode, *t;
>
> + drm_backlight_free(connector);
> /* The connector should have been removed from userspace long before
> * it is finally destroyed.
> */
> @@ -845,6 +847,8 @@ int drm_connector_register(struct drm_connector *connector)
> if (connector->registration_state != DRM_CONNECTOR_INITIALIZING)
> goto unlock;
>
> + drm_backlight_register(connector->backlight);
> +
> ret = drm_sysfs_connector_add(connector);
> if (ret)
> goto unlock;
> @@ -931,6 +935,8 @@ EXPORT_SYMBOL(drm_connector_dynamic_register);
> void drm_connector_unregister(struct drm_connector *connector)
> {
> mutex_lock(&connector->mutex);
> + drm_backlight_unregister(connector->backlight);
> +
> if (connector->registration_state != DRM_CONNECTOR_REGISTERED) {
> mutex_unlock(&connector->mutex);
> return;
> @@ -3252,10 +3258,16 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
> {
> int ret = -EINVAL;
> struct drm_connector *connector = obj_to_connector(obj);
> + struct drm_mode_config *config = &connector->dev->mode_config;
>
> /* Do DPMS ourselves */
> if (property == connector->dev->mode_config.dpms_property) {
> ret = (*connector->funcs->dpms)(connector, (int)value);
> + } else if (property == config->luminance_property) {
> + if (connector->backlight && connector->dpms == DRM_MODE_DPMS_ON)
> + drm_backlight_set_luminance(connector->backlight,
> + value);
> + ret = 0;
> } else if (connector->funcs->set_property)
> ret = connector->funcs->set_property(connector, property, value);
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 675675480da49..5bf402197867a 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -42,6 +42,7 @@
> #include <linux/xarray.h>
>
> #include <drm/drm_accel.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_bridge.h>
> #include <drm/drm_cache.h>
> #include <drm/drm_client_event.h>
> @@ -1230,6 +1231,7 @@ static void drm_core_exit(void)
> drm_privacy_screen_lookup_exit();
> drm_panic_exit();
> accel_core_exit();
> + drm_backlight_exit();
> unregister_chrdev(DRM_MAJOR, "drm");
> drm_debugfs_remove_root();
> drm_sysfs_destroy();
> @@ -1253,6 +1255,12 @@ static int __init drm_core_init(void)
> drm_debugfs_init_root();
> drm_debugfs_bridge_params();
>
> + ret = drm_backlight_init();
> + if (ret < 0) {
> + DRM_ERROR("Cannot initialize backlight interface\n");
> + goto error;
> + }
> +
> ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
> if (ret < 0)
> goto error;
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 66f7dc37b5970..25c5d29694624 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -32,6 +32,7 @@
> #include <drm/drm_print.h>
> #include <drm/drm_colorop.h>
> #include <linux/dma-resv.h>
> +#include <drm/drm_backlight.h>
>
> #include "drm_crtc_internal.h"
> #include "drm_internal.h"
> @@ -407,6 +408,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.size_hints_property = prop;
>
> + prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
> + "LUMINANCE", 0, 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.luminance_property = prop;
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 21fc9deda4371..a50d33af95e8c 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -30,6 +30,7 @@
> #include <drm/drm_mode_object.h>
> #include <drm/drm_plane.h>
> #include <drm/drm_print.h>
> +#include <drm/drm_backlight.h>
>
> #include "drm_crtc_internal.h"
>
> @@ -287,11 +288,72 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
> {
> int i;
>
> - WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
> - !(property->flags & DRM_MODE_PROP_IMMUTABLE));
drm_object_property_set_value() is explicitly documented to be for
legacy properties only. This warning is here for a reason, you should
update the atomic path.
> + /* Exempt dpms and luminance properties from the atomic warning, as these
> + * have special interdependencies managed internally in this function
> + */
> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> + struct drm_connector *connector = obj_to_connector(obj);
> + struct drm_mode_config *config = &connector->dev->mode_config;
> +
> + if (property != config->dpms_property &&
> + property != config->luminance_property) {
> + WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
> + !(property->flags & DRM_MODE_PROP_IMMUTABLE));
> + }
> + } else {
> + WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
> + !(property->flags & DRM_MODE_PROP_IMMUTABLE));
> + }
>
> for (i = 0; i < obj->properties->count; i++) {
> + /* If properties depends on each other
> + * this is where to resolve that issue
> + */
This is not the right commit style:
https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
> if (obj->properties->properties[i] == property) {
> + /* Connector-specific property interdependencies */
> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> + struct drm_connector *connector = obj_to_connector(obj);
> + struct drm_property *dpms_property =
> + connector->dev->mode_config.dpms_property;
> + struct drm_property *luminance_property =
> + connector->dev->mode_config.luminance_property;
> +
> + if (property == dpms_property) {
> + /* When DPMS goes from ON -> OFF,
> + * set the brightness to the minimum possible
> + * to save power.
> + */
> + if (obj->properties->values[i] !=
> + DRM_MODE_DPMS_OFF &&
> + val == DRM_MODE_DPMS_OFF)
> + drm_backlight_set_luminance(
> + connector->backlight, 0);
> + /* When DPMS OFF -> ON, reset the brightness
> + * to the original level
> + */
> + else if (obj->properties->values[i] ==
> + DRM_MODE_DPMS_OFF &&
> + val != DRM_MODE_DPMS_OFF) {
> + uint64_t value;
> +
> + drm_object_property_get_value(
> + obj, luminance_property,
> + &value);
> + drm_backlight_set_luminance(
> + connector->backlight, value);
> + }
> + } else if (property == luminance_property) {
> + /* Always update the property value to remember
> + * the user's desired brightness, but only update
> + * hardware when DPMS is ON.
> + */
> + obj->properties->values[i] = val;
> + if (connector->dpms == DRM_MODE_DPMS_ON)
> + drm_backlight_set_luminance(
> + connector->backlight, val);
> + return 0;
> + }
> + }
And this all shouldn't be plugged into the function directly, but a helper ideally.
> obj->properties->values[i] = val;
> return 0;
> }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ef4e923a87284..8609e1660f210 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -11,6 +11,7 @@
> */
>
> #include <linux/acpi.h>
> +#include <linux/backlight.h>
> #include <linux/component.h>
> #include <linux/device.h>
> #include <linux/err.h>
> @@ -27,6 +28,7 @@
> #include <drm/drm_device.h>
> #include <drm/drm_file.h>
> #include <drm/drm_modes.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_print.h>
> #include <drm/drm_property.h>
> #include <drm/drm_sysfs.h>
> @@ -391,15 +393,35 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>
> int drm_sysfs_connector_add_late(struct drm_connector *connector)
> {
> - if (connector->ddc)
> - return sysfs_create_link(&connector->kdev->kobj,
> - &connector->ddc->dev.kobj, "ddc");
> + if (connector->ddc) {
> + int ret = sysfs_create_link(&connector->kdev->kobj,
> + &connector->ddc->dev.kobj, "ddc");
> + if (ret)
> + return ret;
> + }
> +
> + if (connector->backlight) {
> + struct backlight_device *bd = drm_backlight_get_device(connector->backlight);
> +
> + if (bd) {
> + int ret = sysfs_create_link(&connector->kdev->kobj,
> + &bd->dev.kobj, "backlight");
> + if (ret) {
> + if (connector->ddc)
> + sysfs_remove_link(&connector->kdev->kobj, "ddc");
> + return ret;
> + }
> + }
> + }
>
> return 0;
> }
>
> void drm_sysfs_connector_remove_early(struct drm_connector *connector)
> {
> + if (connector->backlight && drm_backlight_get_device(connector->backlight))
> + sysfs_remove_link(&connector->kdev->kobj, "backlight");
> +
> if (connector->ddc)
> sysfs_remove_link(&connector->kdev->kobj, "ddc");
> }
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index cd1a161ae7bc6..13954c2220b7e 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -126,6 +126,9 @@ static void backlight_generate_event(struct backlight_device *bd,
> case BACKLIGHT_UPDATE_HOTKEY:
> envp[0] = "SOURCE=hotkey";
> break;
> + case BACKLIGHT_UPDATE_DRM:
> + envp[0] = "SOURCE=drm";
> + break;
> default:
> envp[0] = "SOURCE=unknown";
> break;
> @@ -579,6 +582,20 @@ int backlight_unregister_notifier(struct notifier_block *nb)
> }
> EXPORT_SYMBOL(backlight_unregister_notifier);
>
> +/**
> + * backlight_notify_brightness - notify brightness change to listeners
> + * @bd: backlight device that changed
> + *
> + * Notify registered listeners that the backlight brightness has changed.
> + * This is called automatically after successful brightness updates.
> + */
> +void backlight_notify_brightness(struct backlight_device *bd)
> +{
> + blocking_notifier_call_chain(&backlight_notifier,
> + BACKLIGHT_BRIGHTNESS_CHANGED, bd);
> +}
> +EXPORT_SYMBOL(backlight_notify_brightness);
> +
This should be in separate commits.
> /**
> * devm_backlight_device_register - register a new backlight device
> * @dev: the device to register
> diff --git a/include/drm/drm_backlight.h b/include/drm/drm_backlight.h
> new file mode 100644
> index 0000000000000..e0e09e38f7c06
> --- /dev/null
> +++ b/include/drm/drm_backlight.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: MIT */
> +#ifndef __DRM_BACKLIGHT_H__
> +#define __DRM_BACKLIGHT_H__
> +
> +/*
> + * Copyright (c) 2014 David Herrmann <dh.herrmann at gmail.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +
> +struct backlight_device;
> +struct drm_backlight;
> +struct drm_connector;
> +struct drm_device;
> +struct drm_mode_object;
> +
> +int drm_backlight_init(void);
> +void drm_backlight_exit(void);
> +
> +int drm_backlight_alloc(struct drm_connector *connector);
> +void drm_backlight_free(struct drm_connector *connector);
> +void drm_backlight_register(struct drm_backlight *b);
> +void drm_backlight_unregister(struct drm_backlight *b);
> +
> +void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd);
> +struct backlight_device *drm_backlight_get_device(struct drm_backlight *b);
> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
> +void drm_backlight_inhibit_legacy(struct drm_backlight *b);
> +void drm_backlight_uninhibit_legacy(struct drm_backlight *b);
> +void drm_backlight_inhibit_legacy_all(struct drm_device *dev);
> +void drm_backlight_uninhibit_legacy_all(struct drm_device *dev);
> +#endif /* __DRM_BACKLIGHT_H__ */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 5ad62c207d009..d78ac1068d12e 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2410,6 +2410,9 @@ struct drm_connector {
> * @cec: CEC-related data.
> */
> struct drm_connector_cec cec;
> +
> + /* backlight link */
> + struct drm_backlight *backlight;
> };
>
> #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index e584652ddf676..b6d88319d5d0f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -852,6 +852,11 @@ struct drm_mode_config {
> * the position of the output on the host's screen.
> */
> struct drm_property *suggested_y_property;
> + /**
> + * @luminance_property: Default connector property to control the
> + * connector's backlight luminance.
> + */
> + struct drm_property *luminance_property;
>
> /**
> * @non_desktop_property: Optional connector property with a hint
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 204eea9256fd7..26a7281d179c1 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -29,6 +29,7 @@ enum backlight_update_reason {
> * @BACKLIGHT_UPDATE_SYSFS: The backlight was updated using sysfs.
> */
> BACKLIGHT_UPDATE_SYSFS,
> + BACKLIGHT_UPDATE_DRM,
Doc?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v6 03/10] drm: link connectors to backlight devices
2026-06-26 7:34 ` Maxime Ripard
@ 2026-06-26 21:40 ` Mario Limonciello
2026-07-07 11:53 ` Maxime Ripard
0 siblings, 1 reply; 19+ messages in thread
From: Mario Limonciello @ 2026-06-26 21:40 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Xaver Hugl,
amd-gfx, open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov
On 6/26/26 02:34, Maxime Ripard wrote:
> On Wed, Jun 24, 2026 at 09:57:43AM -0700, Mario Limonciello wrote:
>> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>>
>> This will show which connector in sysfs matches which backlight.
>>
>> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # SM8150-HDK
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> drivers/gpu/drm/Kconfig | 1 +
>> drivers/gpu/drm/Makefile | 1 +
>> drivers/gpu/drm/drm_backlight.c | 371 ++++++++++++++++++++++++++++
>> 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 | 28 ++-
>> drivers/video/backlight/backlight.c | 17 ++
>> include/drm/drm_backlight.h | 51 ++++
>> include/drm/drm_connector.h | 3 +
>> include/drm/drm_mode_config.h | 5 +
>> include/linux/backlight.h | 13 +
>> 13 files changed, 578 insertions(+), 5 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 323422861e8f6..d6035bbbdc83f 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..bf980a2ac1e6b 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -78,6 +78,7 @@ drm-$(CONFIG_DRM_CLIENT) += \
>> drm_client_event.o \
>> drm_client_modeset.o \
>> drm_client_sysrq.o
>> +drm-y += drm_backlight.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..b1ec470be86ca
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_backlight.c
>> @@ -0,0 +1,371 @@
>> +// 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 a linked backlight device. The links between connectors and
>> + * backlight devices are established by DRM drivers; user-space cannot create
>> + * or modify these links. A 'change' uevent is sent whenever the brightness is
>> + * updated.
>
> I think we should explain why, because if it's a property and comes from
> the userspace, then why would luminance be special and trigger a uevent
> when it's updated, unlike any other property?
At one point there was a concept of userspace could still change it from
sysfs, but now that's changed. I think you're right and this can be
dropped.
>
>> + * Drivers have to call drm_backlight_alloc() after allocating a connector via
>> + * drm_connector_init(). This will automatically add a backlight device to the
>> + * given connector. Drivers must then link a hardware backlight by calling
>> + * drm_backlight_link() with the registered backlight_device. If no link is
>> + * established, the DRM backlight property reports an empty range and
>> + * brightness changes are no-ops.
>> + */
>
> It's not clear to me why we need to dynamically allocate them at all. If
> we're using the backlight subsystem we'll already have a handle to it.
> If we don't and want to implement something like DDC/CI, then it just
> becomes a hassle. Why not treat it like i2c, add a backlight field to
> drm_connector, and create the link at registration, add a new hook to
> set luminance, and then provide helpers to either use the backlight API,
> or anything else if the driver wants to.
I did envision that later on we can have displays with DDC use this
infrastructure as well. But I didn't want to hold up the series
implementing that.
>
>> +struct drm_backlight {
>> + struct list_head list;
>> + struct drm_connector *connector;
>> + struct backlight_device *link;
>> + /*
>> + * Number of luminance-aware DRM clients that have taken over this
>> + * connector's backlight. While > 0, legacy sysfs writes to the
>> + * linked backlight_device return -EBUSY. Protected by
>> + * drm_backlight_lock.
>> + */
>> + unsigned int luminance_clients;
>> +};
>> +
>> +static LIST_HEAD(drm_backlight_list);
>> +static DEFINE_SPINLOCK(drm_backlight_lock);
>> +
>> +/* caller must hold @drm_backlight_lock */
>> +static bool __drm_backlight_is_registered(struct drm_backlight *b)
>> +{
>> + lockdep_assert_held(&drm_backlight_lock);
>> + /* a device is live if it is linked to @drm_backlight_list */
>> + return !list_empty(&b->list);
>> +}
>> +
>> +/* caller must hold @drm_backlight_lock */
>> +static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
>> +{
>> + unsigned int max, set;
>> +
>> + lockdep_assert_held(&drm_backlight_lock);
>> +
>> + if (!b->link)
>> + return;
>> +
>> + max = b->link->props.max_brightness;
>> + if (max < 1)
>> + return;
>> +
>> + set = v;
>> + if (set >= max)
>> + set = max;
>> +}
>> +
>> +/**
>> + * __drm_backlight_update_prop_range - update the luminance property range
>> + * @b: backlight device
>> + *
>> + * Updates the luminance property range based on the linked backlight device's
>> + * max_brightness. If no device is linked, sets range to 0-0 to indicate
>> + * unavailability.
>> + */
>> +static void __drm_backlight_update_prop_range(struct drm_backlight *b)
>> +{
>> + struct drm_device *dev = b->connector->dev;
>> + struct drm_property *prop = dev->mode_config.luminance_property;
>> + unsigned int max = 0;
>> +
>> + lockdep_assert_held(&drm_backlight_lock);
>> +
>> + if (b->link && b->link->props.max_brightness > 0)
>> + max = b->link->props.max_brightness;
>> +
>> + /* Update property range to match hardware capabilities.
>> + * Range of 0-0 indicates no backing device.
>> + * Range of 1-max for normal operation (0 reserved for display off).
>> + */
>> + if (prop->values[1] != max) {
>> + prop->values[0] = max ? 1 : 0;
>> + prop->values[1] = max;
>> + }
>> +}
>> +
>> +/* caller must hold @drm_backlight_lock */
>> +static bool __drm_backlight_link(struct drm_backlight *b,
>> + struct backlight_device *bd)
>> +{
>> + if (bd == b->link)
>> + return false;
>> +
>> + backlight_device_unref(b->link);
>> + b->link = bd;
>> + backlight_device_ref(b->link);
>> + if (bd)
>> + __drm_backlight_real_changed(b, bd->props.brightness);
>> + __drm_backlight_update_prop_range(b);
>> +
>> + return true;
>> +}
>> +
>> +/**
>> + * drm_backlight_alloc - add backlight capability to a connector
>> + * @connector: connector to add backlight to
>> + *
>> + * This allocates a new DRM-backlight device and attaches it to @connector.
>> + * This *must* be called before registering the connector. The backlight
>> + * device will be automatically registered in sync with the connector. It will
>> + * also get removed once the connector is removed.
>> + *
>> + * No hardware backlight is linked by default. Drivers must call
>> + * drm_backlight_link() to associate a registered backlight_device with the
>> + * connector. User-space cannot create or modify this link.
>> + *
>> + * Returns: 0 on success, negative error code on failure.
>> + */
>> +int drm_backlight_alloc(struct drm_connector *connector)
>> +{
>> + struct drm_mode_config *config = &connector->dev->mode_config;
>> + struct drm_backlight *b;
>> +
>> + b = kzalloc_obj(*b, GFP_KERNEL);
>> + if (!b)
>> + return -ENOMEM;
>> +
>> + INIT_LIST_HEAD(&b->list);
>> + b->connector = connector;
>> + connector->backlight = b;
>> +
>> + drm_object_attach_property(&connector->base,
>> + config->luminance_property, 0);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(drm_backlight_alloc);
>> +
>> +void drm_backlight_free(struct drm_connector *connector)
>> +{
>> + struct drm_backlight *b = connector->backlight;
>> +
>> + if (!b)
>> + return;
>> +
>> + WARN_ON(__drm_backlight_is_registered(b));
>> + WARN_ON(b->link);
>> +
>> + kfree(b);
>> + connector->backlight = NULL;
>> +}
>> +EXPORT_SYMBOL(drm_backlight_free);
>> +
>> +void drm_backlight_register(struct drm_backlight *b)
>> +{
>> + if (!b)
>> + return;
>> +
>> + WARN_ON(__drm_backlight_is_registered(b));
>> +
>> + guard(spinlock)(&drm_backlight_lock);
>> + list_add(&b->list, &drm_backlight_list);
>> +}
>> +EXPORT_SYMBOL(drm_backlight_register);
>> +
>> +void drm_backlight_unregister(struct drm_backlight *b)
>> +{
>> + if (!b)
>> + return;
>> +
>> + WARN_ON(!__drm_backlight_is_registered(b));
>> +
>> + scoped_guard(spinlock, &drm_backlight_lock) {
>> + list_del_init(&b->list);
>> + __drm_backlight_link(b, NULL);
>> + }
>> +}
>> +EXPORT_SYMBOL(drm_backlight_unregister);
>> +
>> +/**
>> + * drm_backlight_link - link a backlight device to a DRM backlight
>> + * @b: DRM backlight to modify
>> + * @bd: backlight device to link, or NULL to unlink
>> + *
>> + * Establish the link between a DRM connector's backlight property and a
>> + * registered backlight_device. Drivers must call this with the
>> + * backlight_device they registered for the connector. Passing NULL unlinks
>> + * any previously linked device.
>> + *
>> + * The caller is responsible for ensuring @bd remains valid until either it
>> + * is unlinked via drm_backlight_link(b, NULL) or the connector is
>> + * unregistered.
>> + *
>> + * Whenever a hardware backlight is linked or unlinked, a uevent with
>> + * "BACKLIGHT=1" is generated on the connector.
>> + */
>> +void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd)
>> +{
>> + if (!b)
>> + return;
>> +
>> + guard(spinlock)(&drm_backlight_lock);
>> + __drm_backlight_link(b, bd);
>> +}
>> +EXPORT_SYMBOL(drm_backlight_link);
>> +
>> +/**
>> + * drm_backlight_get_device - get the backlight_device linked to a DRM backlight
>> + * @b: DRM backlight
>> + *
>> + * Returns the &backlight_device linked to @b, or NULL if no device is linked
>> + * or @b is NULL. The caller must hold the appropriate lock to prevent the
>> + * link from changing while the pointer is in use.
>> + */
>> +struct backlight_device *drm_backlight_get_device(struct drm_backlight *b)
>> +{
>> + if (!b)
>> + return NULL;
>> +
>> + guard(spinlock)(&drm_backlight_lock);
>> + return b->link;
>> +}
>> +EXPORT_SYMBOL(drm_backlight_get_device);
>> +
>> +/**
>> + * drm_backlight_inhibit_legacy - disable legacy sysfs control of the linked bd
>> + * @b: DRM backlight to inhibit
>> + *
>> + * Record that one more luminance-aware DRM client has taken over @b. While
>> + * any clients are recorded, writes to the linked backlight_device's legacy
>> + * ``brightness`` sysfs attribute return ``-EBUSY``. The takeover follows
>> + * @b->link if the link changes.
>> + *
>> + * Calls must be balanced with drm_backlight_uninhibit_legacy().
>> + */
>> +void drm_backlight_inhibit_legacy(struct drm_backlight *b)
>> +{
>> + if (!b)
>> + return;
>> +}
>> +EXPORT_SYMBOL(drm_backlight_inhibit_legacy);
>> +
>> +/**
>> + * drm_backlight_uninhibit_legacy - re-enable legacy sysfs control
>> + * @b: DRM backlight to uninhibit
>> + *
>> + * Balances a previous drm_backlight_inhibit_legacy() call. When the last
>> + * luminance-aware client goes away, legacy sysfs writes are allowed again.
>> + */
>> +void drm_backlight_uninhibit_legacy(struct drm_backlight *b)
>> +{
>> + if (!b)
>> + return;
>> +}
>> +EXPORT_SYMBOL(drm_backlight_uninhibit_legacy);
>> +
>> +/**
>> + * drm_backlight_inhibit_legacy_all - inhibit legacy sysfs on every connector
>> + * @dev: DRM device whose connectors should be inhibited
>> + *
>> + * Walks all connectors on @dev and calls drm_backlight_inhibit_legacy() on
>> + * each connector that has a DRM backlight attached. Used when a client
>> + * declares it is luminance-aware via DRM_CLIENT_CAP_LUMINANCE.
>> + */
>> +void drm_backlight_inhibit_legacy_all(struct drm_device *dev)
>> +{
>> + struct drm_connector_list_iter iter;
>> + struct drm_connector *connector;
>> +
>> + drm_connector_list_iter_begin(dev, &iter);
>> + drm_for_each_connector_iter(connector, &iter)
>> + drm_backlight_inhibit_legacy(connector->backlight);
>> + drm_connector_list_iter_end(&iter);
>> +}
>> +EXPORT_SYMBOL(drm_backlight_inhibit_legacy_all);
>> +
>> +/**
>> + * drm_backlight_uninhibit_legacy_all - reverse drm_backlight_inhibit_legacy_all()
>> + * @dev: DRM device whose connectors should be uninhibited
>> + */
>> +void drm_backlight_uninhibit_legacy_all(struct drm_device *dev)
>> +{
>> + struct drm_connector_list_iter iter;
>> + struct drm_connector *connector;
>> +
>> + drm_connector_list_iter_begin(dev, &iter);
>> + drm_for_each_connector_iter(connector, &iter)
>> + drm_backlight_uninhibit_legacy(connector->backlight);
>> + drm_connector_list_iter_end(&iter);
>> +}
>> +EXPORT_SYMBOL(drm_backlight_uninhibit_legacy_all);
>> +
>> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value)
>> +{
>> + guard(spinlock)(&drm_backlight_lock);
>> + __drm_backlight_real_changed(b, value);
>> +}
>> +EXPORT_SYMBOL(drm_backlight_set_luminance);
>> +
>> +static int drm_backlight_notify(struct notifier_block *self,
>> + unsigned long event, void *data)
>> +{
>> + struct backlight_device *bd = data;
>> + struct drm_backlight *b;
>> +
>> + guard(spinlock)(&drm_backlight_lock);
>> +
>> + switch (event) {
>> + case BACKLIGHT_UNREGISTERED:
>> + list_for_each_entry(b, &drm_backlight_list, list)
>> + if (b->link == bd)
>> + __drm_backlight_link(b, NULL);
>> +
>> + break;
>> + case BACKLIGHT_BRIGHTNESS_CHANGED:
>> + /* Update DRM property value when hardware backlight changes */
>> + list_for_each_entry(b, &drm_backlight_list, list)
>> + if (b->link == bd)
>> + __drm_backlight_real_changed(b, bd->props.brightness);
>> +
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static struct notifier_block drm_backlight_notifier = {
>> + .notifier_call = drm_backlight_notify,
>> +};
>> +
>> +int drm_backlight_init(void)
>> +{
>> + return backlight_register_notifier(&drm_backlight_notifier);
>> +}
>> +
>> +void drm_backlight_exit(void)
>> +{
>> + backlight_unregister_notifier(&drm_backlight_notifier);
>> +}
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 3fa4d2082cd72..128d431f0d6b0 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -21,6 +21,7 @@
>> */
>>
>> #include <drm/drm_auth.h>
>> +#include <drm/drm_backlight.h>
>> #include <drm/drm_connector.h>
>> #include <drm/drm_drv.h>
>> #include <drm/drm_edid.h>
>> @@ -760,6 +761,7 @@ void drm_connector_cleanup(struct drm_connector *connector)
>> struct drm_device *dev = connector->dev;
>> struct drm_display_mode *mode, *t;
>>
>> + drm_backlight_free(connector);
>> /* The connector should have been removed from userspace long before
>> * it is finally destroyed.
>> */
>> @@ -845,6 +847,8 @@ int drm_connector_register(struct drm_connector *connector)
>> if (connector->registration_state != DRM_CONNECTOR_INITIALIZING)
>> goto unlock;
>>
>> + drm_backlight_register(connector->backlight);
>> +
>> ret = drm_sysfs_connector_add(connector);
>> if (ret)
>> goto unlock;
>> @@ -931,6 +935,8 @@ EXPORT_SYMBOL(drm_connector_dynamic_register);
>> void drm_connector_unregister(struct drm_connector *connector)
>> {
>> mutex_lock(&connector->mutex);
>> + drm_backlight_unregister(connector->backlight);
>> +
>> if (connector->registration_state != DRM_CONNECTOR_REGISTERED) {
>> mutex_unlock(&connector->mutex);
>> return;
>> @@ -3252,10 +3258,16 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>> {
>> int ret = -EINVAL;
>> struct drm_connector *connector = obj_to_connector(obj);
>> + struct drm_mode_config *config = &connector->dev->mode_config;
>>
>> /* Do DPMS ourselves */
>> if (property == connector->dev->mode_config.dpms_property) {
>> ret = (*connector->funcs->dpms)(connector, (int)value);
>> + } else if (property == config->luminance_property) {
>> + if (connector->backlight && connector->dpms == DRM_MODE_DPMS_ON)
>> + drm_backlight_set_luminance(connector->backlight,
>> + value);
>> + ret = 0;
>> } else if (connector->funcs->set_property)
>> ret = connector->funcs->set_property(connector, property, value);
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 675675480da49..5bf402197867a 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -42,6 +42,7 @@
>> #include <linux/xarray.h>
>>
>> #include <drm/drm_accel.h>
>> +#include <drm/drm_backlight.h>
>> #include <drm/drm_bridge.h>
>> #include <drm/drm_cache.h>
>> #include <drm/drm_client_event.h>
>> @@ -1230,6 +1231,7 @@ static void drm_core_exit(void)
>> drm_privacy_screen_lookup_exit();
>> drm_panic_exit();
>> accel_core_exit();
>> + drm_backlight_exit();
>> unregister_chrdev(DRM_MAJOR, "drm");
>> drm_debugfs_remove_root();
>> drm_sysfs_destroy();
>> @@ -1253,6 +1255,12 @@ static int __init drm_core_init(void)
>> drm_debugfs_init_root();
>> drm_debugfs_bridge_params();
>>
>> + ret = drm_backlight_init();
>> + if (ret < 0) {
>> + DRM_ERROR("Cannot initialize backlight interface\n");
>> + goto error;
>> + }
>> +
>> ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
>> if (ret < 0)
>> goto error;
>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>> index 66f7dc37b5970..25c5d29694624 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -32,6 +32,7 @@
>> #include <drm/drm_print.h>
>> #include <drm/drm_colorop.h>
>> #include <linux/dma-resv.h>
>> +#include <drm/drm_backlight.h>
>>
>> #include "drm_crtc_internal.h"
>> #include "drm_internal.h"
>> @@ -407,6 +408,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>> return -ENOMEM;
>> dev->mode_config.size_hints_property = prop;
>>
>> + prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
>> + "LUMINANCE", 0, 0);
>> + if (!prop)
>> + return -ENOMEM;
>> + dev->mode_config.luminance_property = prop;
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
>> index 21fc9deda4371..a50d33af95e8c 100644
>> --- a/drivers/gpu/drm/drm_mode_object.c
>> +++ b/drivers/gpu/drm/drm_mode_object.c
>> @@ -30,6 +30,7 @@
>> #include <drm/drm_mode_object.h>
>> #include <drm/drm_plane.h>
>> #include <drm/drm_print.h>
>> +#include <drm/drm_backlight.h>
>>
>> #include "drm_crtc_internal.h"
>>
>> @@ -287,11 +288,72 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
>> {
>> int i;
>>
>> - WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
>> - !(property->flags & DRM_MODE_PROP_IMMUTABLE));
>
> drm_object_property_set_value() is explicitly documented to be for
> legacy properties only. This warning is here for a reason, you should
> update the atomic path.
I seem to remember having a problem earlier on. I'll try again.
>
>> + /* Exempt dpms and luminance properties from the atomic warning, as these
>> + * have special interdependencies managed internally in this function
>> + */
>> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
>> + struct drm_connector *connector = obj_to_connector(obj);
>> + struct drm_mode_config *config = &connector->dev->mode_config;
>> +
>> + if (property != config->dpms_property &&
>> + property != config->luminance_property) {
>> + WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
>> + !(property->flags & DRM_MODE_PROP_IMMUTABLE));
>> + }
>> + } else {
>> + WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
>> + !(property->flags & DRM_MODE_PROP_IMMUTABLE));
>> + }
>>
>> for (i = 0; i < obj->properties->count; i++) {
>> + /* If properties depends on each other
>> + * this is where to resolve that issue
>> + */
>
> This is not the right commit style:
> https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
>
>> if (obj->properties->properties[i] == property) {
>> + /* Connector-specific property interdependencies */
>> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
>> + struct drm_connector *connector = obj_to_connector(obj);
>> + struct drm_property *dpms_property =
>> + connector->dev->mode_config.dpms_property;
>> + struct drm_property *luminance_property =
>> + connector->dev->mode_config.luminance_property;
>> +
>> + if (property == dpms_property) {
>> + /* When DPMS goes from ON -> OFF,
>> + * set the brightness to the minimum possible
>> + * to save power.
>> + */
>> + if (obj->properties->values[i] !=
>> + DRM_MODE_DPMS_OFF &&
>> + val == DRM_MODE_DPMS_OFF)
>> + drm_backlight_set_luminance(
>> + connector->backlight, 0);
>> + /* When DPMS OFF -> ON, reset the brightness
>> + * to the original level
>> + */
>> + else if (obj->properties->values[i] ==
>> + DRM_MODE_DPMS_OFF &&
>> + val != DRM_MODE_DPMS_OFF) {
>> + uint64_t value;
>> +
>> + drm_object_property_get_value(
>> + obj, luminance_property,
>> + &value);
>> + drm_backlight_set_luminance(
>> + connector->backlight, value);
>> + }
>> + } else if (property == luminance_property) {
>> + /* Always update the property value to remember
>> + * the user's desired brightness, but only update
>> + * hardware when DPMS is ON.
>> + */
>> + obj->properties->values[i] = val;
>> + if (connector->dpms == DRM_MODE_DPMS_ON)
>> + drm_backlight_set_luminance(
>> + connector->backlight, val);
>> + return 0;
>> + }
>> + }
>
> And this all shouldn't be plugged into the function directly, but a helper ideally.
>
>> obj->properties->values[i] = val;
>> return 0;
>> }
>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> index ef4e923a87284..8609e1660f210 100644
>> --- a/drivers/gpu/drm/drm_sysfs.c
>> +++ b/drivers/gpu/drm/drm_sysfs.c
>> @@ -11,6 +11,7 @@
>> */
>>
>> #include <linux/acpi.h>
>> +#include <linux/backlight.h>
>> #include <linux/component.h>
>> #include <linux/device.h>
>> #include <linux/err.h>
>> @@ -27,6 +28,7 @@
>> #include <drm/drm_device.h>
>> #include <drm/drm_file.h>
>> #include <drm/drm_modes.h>
>> +#include <drm/drm_backlight.h>
>> #include <drm/drm_print.h>
>> #include <drm/drm_property.h>
>> #include <drm/drm_sysfs.h>
>> @@ -391,15 +393,35 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>>
>> int drm_sysfs_connector_add_late(struct drm_connector *connector)
>> {
>> - if (connector->ddc)
>> - return sysfs_create_link(&connector->kdev->kobj,
>> - &connector->ddc->dev.kobj, "ddc");
>> + if (connector->ddc) {
>> + int ret = sysfs_create_link(&connector->kdev->kobj,
>> + &connector->ddc->dev.kobj, "ddc");
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + if (connector->backlight) {
>> + struct backlight_device *bd = drm_backlight_get_device(connector->backlight);
>> +
>> + if (bd) {
>> + int ret = sysfs_create_link(&connector->kdev->kobj,
>> + &bd->dev.kobj, "backlight");
>> + if (ret) {
>> + if (connector->ddc)
>> + sysfs_remove_link(&connector->kdev->kobj, "ddc");
>> + return ret;
>> + }
>> + }
>> + }
>>
>> return 0;
>> }
>>
>> void drm_sysfs_connector_remove_early(struct drm_connector *connector)
>> {
>> + if (connector->backlight && drm_backlight_get_device(connector->backlight))
>> + sysfs_remove_link(&connector->kdev->kobj, "backlight");
>> +
>> if (connector->ddc)
>> sysfs_remove_link(&connector->kdev->kobj, "ddc");
>> }
>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
>> index cd1a161ae7bc6..13954c2220b7e 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -126,6 +126,9 @@ static void backlight_generate_event(struct backlight_device *bd,
>> case BACKLIGHT_UPDATE_HOTKEY:
>> envp[0] = "SOURCE=hotkey";
>> break;
>> + case BACKLIGHT_UPDATE_DRM:
>> + envp[0] = "SOURCE=drm";
>> + break;
>> default:
>> envp[0] = "SOURCE=unknown";
>> break;
>> @@ -579,6 +582,20 @@ int backlight_unregister_notifier(struct notifier_block *nb)
>> }
>> EXPORT_SYMBOL(backlight_unregister_notifier);
>>
>> +/**
>> + * backlight_notify_brightness - notify brightness change to listeners
>> + * @bd: backlight device that changed
>> + *
>> + * Notify registered listeners that the backlight brightness has changed.
>> + * This is called automatically after successful brightness updates.
>> + */
>> +void backlight_notify_brightness(struct backlight_device *bd)
>> +{
>> + blocking_notifier_call_chain(&backlight_notifier,
>> + BACKLIGHT_BRIGHTNESS_CHANGED, bd);
>> +}
>> +EXPORT_SYMBOL(backlight_notify_brightness);
>> +
>
> This should be in separate commits.
>
>> /**
>> * devm_backlight_device_register - register a new backlight device
>> * @dev: the device to register
>> diff --git a/include/drm/drm_backlight.h b/include/drm/drm_backlight.h
>> new file mode 100644
>> index 0000000000000..e0e09e38f7c06
>> --- /dev/null
>> +++ b/include/drm/drm_backlight.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: MIT */
>> +#ifndef __DRM_BACKLIGHT_H__
>> +#define __DRM_BACKLIGHT_H__
>> +
>> +/*
>> + * Copyright (c) 2014 David Herrmann <dh.herrmann at gmail.com>
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +
>> +struct backlight_device;
>> +struct drm_backlight;
>> +struct drm_connector;
>> +struct drm_device;
>> +struct drm_mode_object;
>> +
>> +int drm_backlight_init(void);
>> +void drm_backlight_exit(void);
>> +
>> +int drm_backlight_alloc(struct drm_connector *connector);
>> +void drm_backlight_free(struct drm_connector *connector);
>> +void drm_backlight_register(struct drm_backlight *b);
>> +void drm_backlight_unregister(struct drm_backlight *b);
>> +
>> +void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd);
>> +struct backlight_device *drm_backlight_get_device(struct drm_backlight *b);
>> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
>> +void drm_backlight_inhibit_legacy(struct drm_backlight *b);
>> +void drm_backlight_uninhibit_legacy(struct drm_backlight *b);
>> +void drm_backlight_inhibit_legacy_all(struct drm_device *dev);
>> +void drm_backlight_uninhibit_legacy_all(struct drm_device *dev);
>> +#endif /* __DRM_BACKLIGHT_H__ */
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 5ad62c207d009..d78ac1068d12e 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -2410,6 +2410,9 @@ struct drm_connector {
>> * @cec: CEC-related data.
>> */
>> struct drm_connector_cec cec;
>> +
>> + /* backlight link */
>> + struct drm_backlight *backlight;
>> };
>>
>> #define obj_to_connector(x) container_of(x, struct drm_connector, base)
>> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
>> index e584652ddf676..b6d88319d5d0f 100644
>> --- a/include/drm/drm_mode_config.h
>> +++ b/include/drm/drm_mode_config.h
>> @@ -852,6 +852,11 @@ struct drm_mode_config {
>> * the position of the output on the host's screen.
>> */
>> struct drm_property *suggested_y_property;
>> + /**
>> + * @luminance_property: Default connector property to control the
>> + * connector's backlight luminance.
>> + */
>> + struct drm_property *luminance_property;
>>
>> /**
>> * @non_desktop_property: Optional connector property with a hint
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index 204eea9256fd7..26a7281d179c1 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -29,6 +29,7 @@ enum backlight_update_reason {
>> * @BACKLIGHT_UPDATE_SYSFS: The backlight was updated using sysfs.
>> */
>> BACKLIGHT_UPDATE_SYSFS,
>> + BACKLIGHT_UPDATE_DRM,
>
> Doc?
>
> Maxime
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v6 03/10] drm: link connectors to backlight devices
2026-06-26 21:40 ` Mario Limonciello
@ 2026-07-07 11:53 ` Maxime Ripard
0 siblings, 0 replies; 19+ messages in thread
From: Maxime Ripard @ 2026-07-07 11:53 UTC (permalink / raw)
To: Mario Limonciello
Cc: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Xaver Hugl,
amd-gfx, open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov
[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]
On Fri, Jun 26, 2026 at 04:40:56PM -0500, Mario Limonciello wrote:
> > > + * Drivers have to call drm_backlight_alloc() after allocating a connector via
> > > + * drm_connector_init(). This will automatically add a backlight device to the
> > > + * given connector. Drivers must then link a hardware backlight by calling
> > > + * drm_backlight_link() with the registered backlight_device. If no link is
> > > + * established, the DRM backlight property reports an empty range and
> > > + * brightness changes are no-ops.
> > > + */
> >
> > It's not clear to me why we need to dynamically allocate them at all. If
> > we're using the backlight subsystem we'll already have a handle to it.
> > If we don't and want to implement something like DDC/CI, then it just
> > becomes a hassle. Why not treat it like i2c, add a backlight field to
> > drm_connector, and create the link at registration, add a new hook to
> > set luminance, and then provide helpers to either use the backlight API,
> > or anything else if the driver wants to.
>
> I did envision that later on we can have displays with DDC use this
> infrastructure as well. But I didn't want to hold up the series
> implementing that.
Oh absolutely. My point wasn't that you should support it right away,
but rather that the architecture you work on would be extensible enough
to accomodate for them later on.
I'm exagerating, but for example having an architecture built around
having a backlight device doesn't work, but having one where you have a
set of helpers built around the backlight API does, because then we can
easily create new helpers to deal with DDC/CI, MIPI-DCS, etc. without
reworking the core part.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 03/10] drm: link connectors to backlight devices
2026-06-24 16:57 ` [PATCH v6 03/10] drm: link connectors to backlight devices Mario Limonciello
2026-06-26 7:34 ` Maxime Ripard
@ 2026-06-26 11:49 ` Thomas Zimmermann
1 sibling, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2026-06-26 11:49 UTC (permalink / raw)
To: Mario Limonciello, dri-devel, harry.wentland, Simona Vetter,
Alex Deucher, Maarten Lankhorst, Maxime Ripard, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov
Hi
Am 24.06.26 um 18:57 schrieb Mario Limonciello:
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> This will show which connector in sysfs matches which backlight.
>
> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # SM8150-HDK
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_backlight.c | 371 ++++++++++++++++++++++++++++
> 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 | 28 ++-
> drivers/video/backlight/backlight.c | 17 ++
> include/drm/drm_backlight.h | 51 ++++
> include/drm/drm_connector.h | 3 +
> include/drm/drm_mode_config.h | 5 +
> include/linux/backlight.h | 13 +
> 13 files changed, 578 insertions(+), 5 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 323422861e8f6..d6035bbbdc83f 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
Nope, sorry. I've work a long time over many kernel releases to get
these out. And there are still many of them left.
BACKLIGHT_CLASS_DEVICE is a user-configurable option. Whatever you build
needs to use 'depends on' or preferably make it optional. Mixing select
and depends on can result in dependency problems.
Another issue is that nowadays, DRM is usually part of the kernel binary
instead of a module. The last thing we want is to pull in yet another
subsystem into the binary.
Best regards
Thomas
> 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..bf980a2ac1e6b 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -78,6 +78,7 @@ drm-$(CONFIG_DRM_CLIENT) += \
> drm_client_event.o \
> drm_client_modeset.o \
> drm_client_sysrq.o
> +drm-y += drm_backlight.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..b1ec470be86ca
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_backlight.c
> @@ -0,0 +1,371 @@
> +// 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 a linked backlight device. The links between connectors and
> + * backlight devices are established by DRM drivers; user-space cannot create
> + * or modify these links. A 'change' uevent is sent whenever the brightness is
> + * updated.
> + *
> + * Drivers have to call drm_backlight_alloc() after allocating a connector via
> + * drm_connector_init(). This will automatically add a backlight device to the
> + * given connector. Drivers must then link a hardware backlight by calling
> + * drm_backlight_link() with the registered backlight_device. If no link is
> + * established, the DRM backlight property reports an empty range and
> + * brightness changes are no-ops.
> + */
> +
> +struct drm_backlight {
> + struct list_head list;
> + struct drm_connector *connector;
> + struct backlight_device *link;
> + /*
> + * Number of luminance-aware DRM clients that have taken over this
> + * connector's backlight. While > 0, legacy sysfs writes to the
> + * linked backlight_device return -EBUSY. Protected by
> + * drm_backlight_lock.
> + */
> + unsigned int luminance_clients;
> +};
> +
> +static LIST_HEAD(drm_backlight_list);
> +static DEFINE_SPINLOCK(drm_backlight_lock);
> +
> +/* caller must hold @drm_backlight_lock */
> +static bool __drm_backlight_is_registered(struct drm_backlight *b)
> +{
> + lockdep_assert_held(&drm_backlight_lock);
> + /* a device is live if it is linked to @drm_backlight_list */
> + return !list_empty(&b->list);
> +}
> +
> +/* caller must hold @drm_backlight_lock */
> +static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
> +{
> + unsigned int max, set;
> +
> + lockdep_assert_held(&drm_backlight_lock);
> +
> + if (!b->link)
> + return;
> +
> + max = b->link->props.max_brightness;
> + if (max < 1)
> + return;
> +
> + set = v;
> + if (set >= max)
> + set = max;
> +}
> +
> +/**
> + * __drm_backlight_update_prop_range - update the luminance property range
> + * @b: backlight device
> + *
> + * Updates the luminance property range based on the linked backlight device's
> + * max_brightness. If no device is linked, sets range to 0-0 to indicate
> + * unavailability.
> + */
> +static void __drm_backlight_update_prop_range(struct drm_backlight *b)
> +{
> + struct drm_device *dev = b->connector->dev;
> + struct drm_property *prop = dev->mode_config.luminance_property;
> + unsigned int max = 0;
> +
> + lockdep_assert_held(&drm_backlight_lock);
> +
> + if (b->link && b->link->props.max_brightness > 0)
> + max = b->link->props.max_brightness;
> +
> + /* Update property range to match hardware capabilities.
> + * Range of 0-0 indicates no backing device.
> + * Range of 1-max for normal operation (0 reserved for display off).
> + */
> + if (prop->values[1] != max) {
> + prop->values[0] = max ? 1 : 0;
> + prop->values[1] = max;
> + }
> +}
> +
> +/* caller must hold @drm_backlight_lock */
> +static bool __drm_backlight_link(struct drm_backlight *b,
> + struct backlight_device *bd)
> +{
> + if (bd == b->link)
> + return false;
> +
> + backlight_device_unref(b->link);
> + b->link = bd;
> + backlight_device_ref(b->link);
> + if (bd)
> + __drm_backlight_real_changed(b, bd->props.brightness);
> + __drm_backlight_update_prop_range(b);
> +
> + return true;
> +}
> +
> +/**
> + * drm_backlight_alloc - add backlight capability to a connector
> + * @connector: connector to add backlight to
> + *
> + * This allocates a new DRM-backlight device and attaches it to @connector.
> + * This *must* be called before registering the connector. The backlight
> + * device will be automatically registered in sync with the connector. It will
> + * also get removed once the connector is removed.
> + *
> + * No hardware backlight is linked by default. Drivers must call
> + * drm_backlight_link() to associate a registered backlight_device with the
> + * connector. User-space cannot create or modify this link.
> + *
> + * Returns: 0 on success, negative error code on failure.
> + */
> +int drm_backlight_alloc(struct drm_connector *connector)
> +{
> + struct drm_mode_config *config = &connector->dev->mode_config;
> + struct drm_backlight *b;
> +
> + b = kzalloc_obj(*b, GFP_KERNEL);
> + if (!b)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&b->list);
> + b->connector = connector;
> + connector->backlight = b;
> +
> + drm_object_attach_property(&connector->base,
> + config->luminance_property, 0);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_backlight_alloc);
> +
> +void drm_backlight_free(struct drm_connector *connector)
> +{
> + struct drm_backlight *b = connector->backlight;
> +
> + if (!b)
> + return;
> +
> + WARN_ON(__drm_backlight_is_registered(b));
> + WARN_ON(b->link);
> +
> + kfree(b);
> + connector->backlight = NULL;
> +}
> +EXPORT_SYMBOL(drm_backlight_free);
> +
> +void drm_backlight_register(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +
> + WARN_ON(__drm_backlight_is_registered(b));
> +
> + guard(spinlock)(&drm_backlight_lock);
> + list_add(&b->list, &drm_backlight_list);
> +}
> +EXPORT_SYMBOL(drm_backlight_register);
> +
> +void drm_backlight_unregister(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +
> + WARN_ON(!__drm_backlight_is_registered(b));
> +
> + scoped_guard(spinlock, &drm_backlight_lock) {
> + list_del_init(&b->list);
> + __drm_backlight_link(b, NULL);
> + }
> +}
> +EXPORT_SYMBOL(drm_backlight_unregister);
> +
> +/**
> + * drm_backlight_link - link a backlight device to a DRM backlight
> + * @b: DRM backlight to modify
> + * @bd: backlight device to link, or NULL to unlink
> + *
> + * Establish the link between a DRM connector's backlight property and a
> + * registered backlight_device. Drivers must call this with the
> + * backlight_device they registered for the connector. Passing NULL unlinks
> + * any previously linked device.
> + *
> + * The caller is responsible for ensuring @bd remains valid until either it
> + * is unlinked via drm_backlight_link(b, NULL) or the connector is
> + * unregistered.
> + *
> + * Whenever a hardware backlight is linked or unlinked, a uevent with
> + * "BACKLIGHT=1" is generated on the connector.
> + */
> +void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd)
> +{
> + if (!b)
> + return;
> +
> + guard(spinlock)(&drm_backlight_lock);
> + __drm_backlight_link(b, bd);
> +}
> +EXPORT_SYMBOL(drm_backlight_link);
> +
> +/**
> + * drm_backlight_get_device - get the backlight_device linked to a DRM backlight
> + * @b: DRM backlight
> + *
> + * Returns the &backlight_device linked to @b, or NULL if no device is linked
> + * or @b is NULL. The caller must hold the appropriate lock to prevent the
> + * link from changing while the pointer is in use.
> + */
> +struct backlight_device *drm_backlight_get_device(struct drm_backlight *b)
> +{
> + if (!b)
> + return NULL;
> +
> + guard(spinlock)(&drm_backlight_lock);
> + return b->link;
> +}
> +EXPORT_SYMBOL(drm_backlight_get_device);
> +
> +/**
> + * drm_backlight_inhibit_legacy - disable legacy sysfs control of the linked bd
> + * @b: DRM backlight to inhibit
> + *
> + * Record that one more luminance-aware DRM client has taken over @b. While
> + * any clients are recorded, writes to the linked backlight_device's legacy
> + * ``brightness`` sysfs attribute return ``-EBUSY``. The takeover follows
> + * @b->link if the link changes.
> + *
> + * Calls must be balanced with drm_backlight_uninhibit_legacy().
> + */
> +void drm_backlight_inhibit_legacy(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +}
> +EXPORT_SYMBOL(drm_backlight_inhibit_legacy);
> +
> +/**
> + * drm_backlight_uninhibit_legacy - re-enable legacy sysfs control
> + * @b: DRM backlight to uninhibit
> + *
> + * Balances a previous drm_backlight_inhibit_legacy() call. When the last
> + * luminance-aware client goes away, legacy sysfs writes are allowed again.
> + */
> +void drm_backlight_uninhibit_legacy(struct drm_backlight *b)
> +{
> + if (!b)
> + return;
> +}
> +EXPORT_SYMBOL(drm_backlight_uninhibit_legacy);
> +
> +/**
> + * drm_backlight_inhibit_legacy_all - inhibit legacy sysfs on every connector
> + * @dev: DRM device whose connectors should be inhibited
> + *
> + * Walks all connectors on @dev and calls drm_backlight_inhibit_legacy() on
> + * each connector that has a DRM backlight attached. Used when a client
> + * declares it is luminance-aware via DRM_CLIENT_CAP_LUMINANCE.
> + */
> +void drm_backlight_inhibit_legacy_all(struct drm_device *dev)
> +{
> + struct drm_connector_list_iter iter;
> + struct drm_connector *connector;
> +
> + drm_connector_list_iter_begin(dev, &iter);
> + drm_for_each_connector_iter(connector, &iter)
> + drm_backlight_inhibit_legacy(connector->backlight);
> + drm_connector_list_iter_end(&iter);
> +}
> +EXPORT_SYMBOL(drm_backlight_inhibit_legacy_all);
> +
> +/**
> + * drm_backlight_uninhibit_legacy_all - reverse drm_backlight_inhibit_legacy_all()
> + * @dev: DRM device whose connectors should be uninhibited
> + */
> +void drm_backlight_uninhibit_legacy_all(struct drm_device *dev)
> +{
> + struct drm_connector_list_iter iter;
> + struct drm_connector *connector;
> +
> + drm_connector_list_iter_begin(dev, &iter);
> + drm_for_each_connector_iter(connector, &iter)
> + drm_backlight_uninhibit_legacy(connector->backlight);
> + drm_connector_list_iter_end(&iter);
> +}
> +EXPORT_SYMBOL(drm_backlight_uninhibit_legacy_all);
> +
> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value)
> +{
> + guard(spinlock)(&drm_backlight_lock);
> + __drm_backlight_real_changed(b, value);
> +}
> +EXPORT_SYMBOL(drm_backlight_set_luminance);
> +
> +static int drm_backlight_notify(struct notifier_block *self,
> + unsigned long event, void *data)
> +{
> + struct backlight_device *bd = data;
> + struct drm_backlight *b;
> +
> + guard(spinlock)(&drm_backlight_lock);
> +
> + switch (event) {
> + case BACKLIGHT_UNREGISTERED:
> + list_for_each_entry(b, &drm_backlight_list, list)
> + if (b->link == bd)
> + __drm_backlight_link(b, NULL);
> +
> + break;
> + case BACKLIGHT_BRIGHTNESS_CHANGED:
> + /* Update DRM property value when hardware backlight changes */
> + list_for_each_entry(b, &drm_backlight_list, list)
> + if (b->link == bd)
> + __drm_backlight_real_changed(b, bd->props.brightness);
> +
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static struct notifier_block drm_backlight_notifier = {
> + .notifier_call = drm_backlight_notify,
> +};
> +
> +int drm_backlight_init(void)
> +{
> + return backlight_register_notifier(&drm_backlight_notifier);
> +}
> +
> +void drm_backlight_exit(void)
> +{
> + backlight_unregister_notifier(&drm_backlight_notifier);
> +}
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 3fa4d2082cd72..128d431f0d6b0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -21,6 +21,7 @@
> */
>
> #include <drm/drm_auth.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_connector.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_edid.h>
> @@ -760,6 +761,7 @@ void drm_connector_cleanup(struct drm_connector *connector)
> struct drm_device *dev = connector->dev;
> struct drm_display_mode *mode, *t;
>
> + drm_backlight_free(connector);
> /* The connector should have been removed from userspace long before
> * it is finally destroyed.
> */
> @@ -845,6 +847,8 @@ int drm_connector_register(struct drm_connector *connector)
> if (connector->registration_state != DRM_CONNECTOR_INITIALIZING)
> goto unlock;
>
> + drm_backlight_register(connector->backlight);
> +
> ret = drm_sysfs_connector_add(connector);
> if (ret)
> goto unlock;
> @@ -931,6 +935,8 @@ EXPORT_SYMBOL(drm_connector_dynamic_register);
> void drm_connector_unregister(struct drm_connector *connector)
> {
> mutex_lock(&connector->mutex);
> + drm_backlight_unregister(connector->backlight);
> +
> if (connector->registration_state != DRM_CONNECTOR_REGISTERED) {
> mutex_unlock(&connector->mutex);
> return;
> @@ -3252,10 +3258,16 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
> {
> int ret = -EINVAL;
> struct drm_connector *connector = obj_to_connector(obj);
> + struct drm_mode_config *config = &connector->dev->mode_config;
>
> /* Do DPMS ourselves */
> if (property == connector->dev->mode_config.dpms_property) {
> ret = (*connector->funcs->dpms)(connector, (int)value);
> + } else if (property == config->luminance_property) {
> + if (connector->backlight && connector->dpms == DRM_MODE_DPMS_ON)
> + drm_backlight_set_luminance(connector->backlight,
> + value);
> + ret = 0;
> } else if (connector->funcs->set_property)
> ret = connector->funcs->set_property(connector, property, value);
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 675675480da49..5bf402197867a 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -42,6 +42,7 @@
> #include <linux/xarray.h>
>
> #include <drm/drm_accel.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_bridge.h>
> #include <drm/drm_cache.h>
> #include <drm/drm_client_event.h>
> @@ -1230,6 +1231,7 @@ static void drm_core_exit(void)
> drm_privacy_screen_lookup_exit();
> drm_panic_exit();
> accel_core_exit();
> + drm_backlight_exit();
> unregister_chrdev(DRM_MAJOR, "drm");
> drm_debugfs_remove_root();
> drm_sysfs_destroy();
> @@ -1253,6 +1255,12 @@ static int __init drm_core_init(void)
> drm_debugfs_init_root();
> drm_debugfs_bridge_params();
>
> + ret = drm_backlight_init();
> + if (ret < 0) {
> + DRM_ERROR("Cannot initialize backlight interface\n");
> + goto error;
> + }
> +
> ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
> if (ret < 0)
> goto error;
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 66f7dc37b5970..25c5d29694624 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -32,6 +32,7 @@
> #include <drm/drm_print.h>
> #include <drm/drm_colorop.h>
> #include <linux/dma-resv.h>
> +#include <drm/drm_backlight.h>
>
> #include "drm_crtc_internal.h"
> #include "drm_internal.h"
> @@ -407,6 +408,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.size_hints_property = prop;
>
> + prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
> + "LUMINANCE", 0, 0);
> + if (!prop)
> + return -ENOMEM;
> + dev->mode_config.luminance_property = prop;
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 21fc9deda4371..a50d33af95e8c 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -30,6 +30,7 @@
> #include <drm/drm_mode_object.h>
> #include <drm/drm_plane.h>
> #include <drm/drm_print.h>
> +#include <drm/drm_backlight.h>
>
> #include "drm_crtc_internal.h"
>
> @@ -287,11 +288,72 @@ int drm_object_property_set_value(struct drm_mode_object *obj,
> {
> int i;
>
> - WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
> - !(property->flags & DRM_MODE_PROP_IMMUTABLE));
> + /* Exempt dpms and luminance properties from the atomic warning, as these
> + * have special interdependencies managed internally in this function
> + */
> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> + struct drm_connector *connector = obj_to_connector(obj);
> + struct drm_mode_config *config = &connector->dev->mode_config;
> +
> + if (property != config->dpms_property &&
> + property != config->luminance_property) {
> + WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
> + !(property->flags & DRM_MODE_PROP_IMMUTABLE));
> + }
> + } else {
> + WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
> + !(property->flags & DRM_MODE_PROP_IMMUTABLE));
> + }
>
> for (i = 0; i < obj->properties->count; i++) {
> + /* If properties depends on each other
> + * this is where to resolve that issue
> + */
> if (obj->properties->properties[i] == property) {
> + /* Connector-specific property interdependencies */
> + if (obj->type == DRM_MODE_OBJECT_CONNECTOR) {
> + struct drm_connector *connector = obj_to_connector(obj);
> + struct drm_property *dpms_property =
> + connector->dev->mode_config.dpms_property;
> + struct drm_property *luminance_property =
> + connector->dev->mode_config.luminance_property;
> +
> + if (property == dpms_property) {
> + /* When DPMS goes from ON -> OFF,
> + * set the brightness to the minimum possible
> + * to save power.
> + */
> + if (obj->properties->values[i] !=
> + DRM_MODE_DPMS_OFF &&
> + val == DRM_MODE_DPMS_OFF)
> + drm_backlight_set_luminance(
> + connector->backlight, 0);
> + /* When DPMS OFF -> ON, reset the brightness
> + * to the original level
> + */
> + else if (obj->properties->values[i] ==
> + DRM_MODE_DPMS_OFF &&
> + val != DRM_MODE_DPMS_OFF) {
> + uint64_t value;
> +
> + drm_object_property_get_value(
> + obj, luminance_property,
> + &value);
> + drm_backlight_set_luminance(
> + connector->backlight, value);
> + }
> + } else if (property == luminance_property) {
> + /* Always update the property value to remember
> + * the user's desired brightness, but only update
> + * hardware when DPMS is ON.
> + */
> + obj->properties->values[i] = val;
> + if (connector->dpms == DRM_MODE_DPMS_ON)
> + drm_backlight_set_luminance(
> + connector->backlight, val);
> + return 0;
> + }
> + }
> obj->properties->values[i] = val;
> return 0;
> }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ef4e923a87284..8609e1660f210 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -11,6 +11,7 @@
> */
>
> #include <linux/acpi.h>
> +#include <linux/backlight.h>
> #include <linux/component.h>
> #include <linux/device.h>
> #include <linux/err.h>
> @@ -27,6 +28,7 @@
> #include <drm/drm_device.h>
> #include <drm/drm_file.h>
> #include <drm/drm_modes.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_print.h>
> #include <drm/drm_property.h>
> #include <drm/drm_sysfs.h>
> @@ -391,15 +393,35 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>
> int drm_sysfs_connector_add_late(struct drm_connector *connector)
> {
> - if (connector->ddc)
> - return sysfs_create_link(&connector->kdev->kobj,
> - &connector->ddc->dev.kobj, "ddc");
> + if (connector->ddc) {
> + int ret = sysfs_create_link(&connector->kdev->kobj,
> + &connector->ddc->dev.kobj, "ddc");
> + if (ret)
> + return ret;
> + }
> +
> + if (connector->backlight) {
> + struct backlight_device *bd = drm_backlight_get_device(connector->backlight);
> +
> + if (bd) {
> + int ret = sysfs_create_link(&connector->kdev->kobj,
> + &bd->dev.kobj, "backlight");
> + if (ret) {
> + if (connector->ddc)
> + sysfs_remove_link(&connector->kdev->kobj, "ddc");
> + return ret;
> + }
> + }
> + }
>
> return 0;
> }
>
> void drm_sysfs_connector_remove_early(struct drm_connector *connector)
> {
> + if (connector->backlight && drm_backlight_get_device(connector->backlight))
> + sysfs_remove_link(&connector->kdev->kobj, "backlight");
> +
> if (connector->ddc)
> sysfs_remove_link(&connector->kdev->kobj, "ddc");
> }
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index cd1a161ae7bc6..13954c2220b7e 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -126,6 +126,9 @@ static void backlight_generate_event(struct backlight_device *bd,
> case BACKLIGHT_UPDATE_HOTKEY:
> envp[0] = "SOURCE=hotkey";
> break;
> + case BACKLIGHT_UPDATE_DRM:
> + envp[0] = "SOURCE=drm";
> + break;
> default:
> envp[0] = "SOURCE=unknown";
> break;
> @@ -579,6 +582,20 @@ int backlight_unregister_notifier(struct notifier_block *nb)
> }
> EXPORT_SYMBOL(backlight_unregister_notifier);
>
> +/**
> + * backlight_notify_brightness - notify brightness change to listeners
> + * @bd: backlight device that changed
> + *
> + * Notify registered listeners that the backlight brightness has changed.
> + * This is called automatically after successful brightness updates.
> + */
> +void backlight_notify_brightness(struct backlight_device *bd)
> +{
> + blocking_notifier_call_chain(&backlight_notifier,
> + BACKLIGHT_BRIGHTNESS_CHANGED, bd);
> +}
> +EXPORT_SYMBOL(backlight_notify_brightness);
> +
> /**
> * devm_backlight_device_register - register a new backlight device
> * @dev: the device to register
> diff --git a/include/drm/drm_backlight.h b/include/drm/drm_backlight.h
> new file mode 100644
> index 0000000000000..e0e09e38f7c06
> --- /dev/null
> +++ b/include/drm/drm_backlight.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: MIT */
> +#ifndef __DRM_BACKLIGHT_H__
> +#define __DRM_BACKLIGHT_H__
> +
> +/*
> + * Copyright (c) 2014 David Herrmann <dh.herrmann at gmail.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +
> +struct backlight_device;
> +struct drm_backlight;
> +struct drm_connector;
> +struct drm_device;
> +struct drm_mode_object;
> +
> +int drm_backlight_init(void);
> +void drm_backlight_exit(void);
> +
> +int drm_backlight_alloc(struct drm_connector *connector);
> +void drm_backlight_free(struct drm_connector *connector);
> +void drm_backlight_register(struct drm_backlight *b);
> +void drm_backlight_unregister(struct drm_backlight *b);
> +
> +void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd);
> +struct backlight_device *drm_backlight_get_device(struct drm_backlight *b);
> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
> +void drm_backlight_inhibit_legacy(struct drm_backlight *b);
> +void drm_backlight_uninhibit_legacy(struct drm_backlight *b);
> +void drm_backlight_inhibit_legacy_all(struct drm_device *dev);
> +void drm_backlight_uninhibit_legacy_all(struct drm_device *dev);
> +#endif /* __DRM_BACKLIGHT_H__ */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 5ad62c207d009..d78ac1068d12e 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2410,6 +2410,9 @@ struct drm_connector {
> * @cec: CEC-related data.
> */
> struct drm_connector_cec cec;
> +
> + /* backlight link */
> + struct drm_backlight *backlight;
> };
>
> #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index e584652ddf676..b6d88319d5d0f 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -852,6 +852,11 @@ struct drm_mode_config {
> * the position of the output on the host's screen.
> */
> struct drm_property *suggested_y_property;
> + /**
> + * @luminance_property: Default connector property to control the
> + * connector's backlight luminance.
> + */
> + struct drm_property *luminance_property;
>
> /**
> * @non_desktop_property: Optional connector property with a hint
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 204eea9256fd7..26a7281d179c1 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -29,6 +29,7 @@ enum backlight_update_reason {
> * @BACKLIGHT_UPDATE_SYSFS: The backlight was updated using sysfs.
> */
> BACKLIGHT_UPDATE_SYSFS,
> + BACKLIGHT_UPDATE_DRM,
> };
>
> /**
> @@ -80,6 +81,11 @@ enum backlight_notification {
> * @BACKLIGHT_UNREGISTERED: The backlight revice is unregistered.
> */
> BACKLIGHT_UNREGISTERED,
> +
> + /**
> + * @BACKLIGHT_BRIGHTNESS_CHANGED: The backlight brightness has changed.
> + */
> + BACKLIGHT_BRIGHTNESS_CHANGED,
> };
>
> /** enum backlight_scale - the type of scale used for brightness values
> @@ -310,6 +316,9 @@ struct backlight_device {
> int use_count;
> };
>
> +/* Forward declaration for backlight_update_status */
> +void backlight_notify_brightness(struct backlight_device *bd);
> +
> /**
> * backlight_update_status - force an update of the backlight device status
> * @bd: the backlight device
> @@ -323,6 +332,10 @@ static inline int backlight_update_status(struct backlight_device *bd)
> ret = bd->ops->update_status(bd);
> mutex_unlock(&bd->update_lock);
>
> + /* Notify DRM and other listeners that brightness changed */
> + if (ret == 0)
> + backlight_notify_brightness(bd);
> +
> return ret;
> }
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 04/10] DRM: Add support for client indicating support for luminance
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (2 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 03/10] drm: link connectors to backlight devices Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-07-21 18:31 ` Hans de Goede
2026-06-24 16:57 ` [PATCH v6 05/10] drm/amd/display: Pass up errors reading actual brightness Mario Limonciello
` (6 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
The legacy backlight control interface can only be disabled when both
the client and driver has connector support that the luminance can be
set during a modeset. Add capability for the client to register.
When a luminance-aware client sets DRM_CLIENT_CAP_LUMINANCE, each
DRM-connected backlight on the device is marked as taken over. Writes
to the legacy /sys/class/backlight/<dev>/brightness attribute then
return -EBUSY until the last luminance-aware client clears the cap or
closes its DRM file. The takeover follows the active backlight_device
when drm_backlight_link() retargets the link.
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # SM8150-HDK
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/gpu/drm/drm_atomic_helper.c | 7 +++
drivers/gpu/drm/drm_atomic_uapi.c | 59 +++++++++++++++++++-
drivers/gpu/drm/drm_backlight.c | 86 +++++++++++++++++++++++++++--
drivers/gpu/drm/drm_connector.c | 51 +++++++++++++++++
drivers/gpu/drm/drm_file.c | 5 ++
drivers/gpu/drm/drm_ioctl.c | 15 +++++
drivers/gpu/drm/drm_property.c | 6 ++
drivers/video/backlight/backlight.c | 7 +++
include/drm/drm_backlight.h | 2 +-
include/drm/drm_connector.h | 5 ++
include/drm/drm_file.h | 8 +++
include/linux/backlight.h | 15 +++++
include/uapi/drm/drm.h | 22 ++++++++
13 files changed, 280 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 51f39edc31edc..17492ff6c5579 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -32,6 +32,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_atomic_uapi.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_blend.h>
#include <drm/drm_bridge.h>
#include <drm/drm_colorop.h>
@@ -1224,6 +1225,8 @@ drm_atomic_helper_commit_encoder_bridge_disable(struct drm_device *dev,
* it away), so we won't call disable hooks twice.
*/
bridge = drm_bridge_chain_get_first_bridge(encoder);
+ if (connector->backlight)
+ drm_backlight_set_luminance(connector->backlight, 0);
drm_atomic_bridge_chain_disable(bridge, state);
drm_bridge_put(bridge);
@@ -1738,6 +1741,10 @@ drm_atomic_helper_commit_encoder_bridge_enable(struct drm_device *dev, struct dr
drm_atomic_bridge_chain_enable(bridge, state);
drm_bridge_put(bridge);
+
+ if (connector->backlight && connector->state)
+ drm_backlight_set_luminance(connector->backlight,
+ connector->state->luminance);
}
}
EXPORT_SYMBOL(drm_atomic_helper_commit_encoder_bridge_enable);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 6441b55cc2740..ebf434bbd963b 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -30,6 +30,8 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_atomic_uapi.h>
+#include <drm/drm_backlight.h>
+#include <drm/drm_connector.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_print.h>
#include <drm/drm_drv.h>
@@ -935,6 +937,14 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
state->privacy_screen_sw_state = val;
} else if (property == connector->broadcast_rgb_property) {
state->hdmi.broadcast_rgb = val;
+ } else if (property == config->luminance_property) {
+ state->luminance = val;
+ /* Update hardware backlight only when DPMS is ON.
+ * Property value is always updated to remember the user's
+ * desired brightness.
+ */
+ if (connector->dpms == DRM_MODE_DPMS_ON)
+ drm_backlight_set_luminance(connector->backlight, val);
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -1020,6 +1030,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
*val = state->privacy_screen_sw_state;
} else if (property == connector->broadcast_rgb_property) {
*val = state->hdmi.broadcast_rgb;
+ } else if (property == config->luminance_property) {
+ *val = state->luminance;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
@@ -1104,6 +1116,31 @@ static struct drm_pending_vblank_event *create_vblank_event(
return e;
}
+static void drm_atomic_connector_set_backlight(struct drm_connector *connector,
+ unsigned int luminance)
+{
+ if (!connector->backlight)
+ return;
+
+ drm_backlight_set_luminance(connector->backlight, luminance);
+}
+
+static void drm_atomic_crtc_set_backlight(struct drm_crtc *crtc, bool active)
+{
+ struct drm_connector_list_iter conn_iter;
+ struct drm_connector *connector;
+
+ drm_connector_list_iter_begin(crtc->dev, &conn_iter);
+ drm_for_each_connector_iter(connector, &conn_iter) {
+ if (!connector->state || connector->state->crtc != crtc)
+ continue;
+
+ drm_atomic_connector_set_backlight(connector,
+ active ? connector->state->luminance : 0);
+ }
+ drm_connector_list_iter_end(&conn_iter);
+}
+
int drm_atomic_connector_commit_dpms(struct drm_atomic_commit *state,
struct drm_connector *connector,
int mode)
@@ -1126,9 +1163,29 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_commit *state,
if (connector->dpms == mode)
goto out;
+ crtc = connector->state ? connector->state->crtc : NULL;
+
+ /* Handle backlight brightness coordination with DPMS state changes */
+ if (old_mode != DRM_MODE_DPMS_OFF && mode == DRM_MODE_DPMS_OFF) {
+ /* DPMS ON -> OFF: dim all connectors driven by this CRTC. */
+ if (crtc)
+ drm_atomic_crtc_set_backlight(crtc, false);
+ else
+ drm_atomic_connector_set_backlight(connector, 0);
+ }
+
connector->dpms = mode;
- crtc = connector->state->crtc;
+ /* DPMS OFF -> ON: restore brightness to property value */
+ if (old_mode == DRM_MODE_DPMS_OFF && mode == DRM_MODE_DPMS_ON &&
+ connector->state) {
+ if (crtc)
+ drm_atomic_crtc_set_backlight(crtc, true);
+ else
+ drm_atomic_connector_set_backlight(connector,
+ connector->state->luminance);
+ }
+
if (!crtc)
goto out;
ret = drm_atomic_add_affected_connectors(state, crtc);
diff --git a/drivers/gpu/drm/drm_backlight.c b/drivers/gpu/drm/drm_backlight.c
index b1ec470be86ca..2cddf209d5088 100644
--- a/drivers/gpu/drm/drm_backlight.c
+++ b/drivers/gpu/drm/drm_backlight.c
@@ -71,6 +71,7 @@ static bool __drm_backlight_is_registered(struct drm_backlight *b)
/* caller must hold @drm_backlight_lock */
static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
{
+ struct drm_connector *connector = b->connector;
unsigned int max, set;
lockdep_assert_held(&drm_backlight_lock);
@@ -85,6 +86,15 @@ static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
set = v;
if (set >= max)
set = max;
+
+ /* Update the atomic state directly.
+ * For atomic drivers, the luminance value is stored in
+ * connector->state->luminance, not in the legacy property array.
+ * We update it unconditionally to reflect the hardware state,
+ * regardless of DPMS.
+ */
+ if (connector->state)
+ connector->state->luminance = set;
}
/**
@@ -100,18 +110,22 @@ static void __drm_backlight_update_prop_range(struct drm_backlight *b)
struct drm_device *dev = b->connector->dev;
struct drm_property *prop = dev->mode_config.luminance_property;
unsigned int max = 0;
+ bool can_disable = false;
lockdep_assert_held(&drm_backlight_lock);
- if (b->link && b->link->props.max_brightness > 0)
+ if (b->link && b->link->props.max_brightness > 0) {
max = b->link->props.max_brightness;
+ can_disable = b->link->props.can_disable;
+ }
/* Update property range to match hardware capabilities.
* Range of 0-0 indicates no backing device.
- * Range of 1-max for normal operation (0 reserved for display off).
+ * Range of 1-max for normal operation.
+ * Range of 0-max means that the display would turn off at 0
*/
if (prop->values[1] != max) {
- prop->values[0] = max ? 1 : 0;
+ prop->values[0] = max ? (can_disable ? 0 : 1) : 0;
prop->values[1] = max;
}
}
@@ -123,6 +137,16 @@ static bool __drm_backlight_link(struct drm_backlight *b,
if (bd == b->link)
return false;
+ /* Transfer any DRM legacy-sysfs takeover from the old link to the
+ * new one so the inhibit follows the active backlight_device.
+ */
+ if (b->luminance_clients) {
+ if (b->link)
+ atomic_sub(b->luminance_clients, &b->link->drm_takeover);
+ if (bd)
+ atomic_add(b->luminance_clients, &bd->drm_takeover);
+ }
+
backlight_device_unref(b->link);
b->link = bd;
backlight_device_ref(b->link);
@@ -177,6 +201,7 @@ void drm_backlight_free(struct drm_connector *connector)
WARN_ON(__drm_backlight_is_registered(b));
WARN_ON(b->link);
+ WARN_ON(b->luminance_clients);
kfree(b);
connector->backlight = NULL;
@@ -228,11 +253,18 @@ EXPORT_SYMBOL(drm_backlight_unregister);
*/
void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd)
{
+ static const char * const ep[] = { "BACKLIGHT=1", NULL };
+ bool send_uevent = false;
+
if (!b)
return;
guard(spinlock)(&drm_backlight_lock);
- __drm_backlight_link(b, bd);
+ send_uevent = __drm_backlight_link(b, bd);
+
+ if (send_uevent)
+ kobject_uevent_env(&b->connector->kdev->kobj, KOBJ_CHANGE,
+ (char **)ep);
}
EXPORT_SYMBOL(drm_backlight_link);
@@ -269,6 +301,11 @@ void drm_backlight_inhibit_legacy(struct drm_backlight *b)
{
if (!b)
return;
+
+ guard(spinlock)(&drm_backlight_lock);
+ b->luminance_clients++;
+ if (b->link)
+ atomic_inc(&b->link->drm_takeover);
}
EXPORT_SYMBOL(drm_backlight_inhibit_legacy);
@@ -283,6 +320,13 @@ void drm_backlight_uninhibit_legacy(struct drm_backlight *b)
{
if (!b)
return;
+
+ guard(spinlock)(&drm_backlight_lock);
+ if (WARN_ON(b->luminance_clients == 0))
+ return;
+ b->luminance_clients--;
+ if (b->link)
+ atomic_dec(&b->link->drm_takeover);
}
EXPORT_SYMBOL(drm_backlight_uninhibit_legacy);
@@ -324,8 +368,38 @@ EXPORT_SYMBOL(drm_backlight_uninhibit_legacy_all);
void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value)
{
- guard(spinlock)(&drm_backlight_lock);
- __drm_backlight_real_changed(b, value);
+ struct backlight_device *bd = NULL;
+ unsigned int set = 0;
+ unsigned long flags;
+ unsigned int max = 0;
+
+ spin_lock_irqsave(&drm_backlight_lock, flags);
+ if (b && b->link) {
+ struct backlight_device *link = b->link;
+
+ max = b->link->props.max_brightness;
+
+ if (max == 0)
+ goto out;
+
+ set = min(value, max);
+ if (set == link->props.brightness)
+ goto out;
+
+ bd = link;
+ backlight_device_ref(bd);
+ }
+out:
+ spin_unlock_irqrestore(&drm_backlight_lock, flags);
+
+ if (bd) {
+ int rc = backlight_set_brightness(bd, set, BACKLIGHT_UPDATE_DRM);
+
+ WARN_ON(rc);
+ if (rc)
+ backlight_set_brightness(bd, max, BACKLIGHT_UPDATE_DRM);
+ backlight_device_unref(bd);
+ }
}
EXPORT_SYMBOL(drm_backlight_set_luminance);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 128d431f0d6b0..70afe3d579f38 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1512,6 +1512,57 @@ EXPORT_SYMBOL(drm_hdmi_connector_get_output_format_name);
* Summarizing: Only set "DPMS" when the connector is known to be enabled,
* assume that a successful SETCONFIG call also sets "DPMS" to on, and
* never read back the value of "DPMS" because it can be incorrect.
+ * LUMINANCE:
+ * Atomic property for controlling the backlight brightness level of the
+ * connector's display. This property provides unified access to the display
+ * backlight, replacing the legacy sysfs interface for brightness control.
+ *
+ * The property value is an unsigned integer representing the brightness level.
+ * The valid range is dynamically determined by the capabilities of the
+ * connected backlight hardware and is exposed through the property's minimum
+ * and maximum values:
+ *
+ * - Range 0-0: No backlight device is available for this connector.
+ * - Range 1-N: Normal operation. Values from 1 to N (max_brightness) are
+ * valid brightness levels, where 1 is the minimum visible brightness and
+ * N is the maximum brightness the hardware supports.
+ * - Value 0: Special value to turn off the display backlight completely.
+ * This value is accepted even when the normal range starts at 1.
+ *
+ * The range may change during runtime if a new backlight device is linked
+ * or unlinked. The kernel will send a change uevent when this occurs.
+ *
+ * Setting LUMINANCE to 0 turns off the backlight, which may turn off the
+ * display completely depending on the hardware. Setting it to any value
+ * from 1 to N adjusts the brightness accordingly. Reading this property
+ * returns the current brightness level that was last set (or the hardware's
+ * current state for drivers that support reading actual brightness).
+ *
+ * For atomic drivers, the luminance value is stored in
+ * &drm_connector_state.luminance. The actual hardware update only occurs
+ * when the connector is active (DPMS is ON). When DPMS transitions to OFF,
+ * the kernel automatically sets luminance to 0 to turn off the backlight.
+ * When DPMS transitions back to ON, the kernel restores the previously
+ * set luminance value.
+ *
+ * This property is only available on connectors that have an associated
+ * backlight device. The property is created by calling drm_backlight_alloc()
+ * during connector initialization.
+ *
+ * Client Capability:
+ * User-space must set the DRM_CLIENT_CAP_LUMINANCE client capability
+ * to 1 before using this property. When this capability is enabled,
+ * the legacy sysfs backlight interface is inhibited to prevent
+ * conflicts between multiple clients trying to control the same
+ * backlight. This ensures that only luminance-aware clients control
+ * the backlight through the DRM atomic interface.
+ *
+ * Legacy clients that do not set this capability will not see the
+ * LUMINANCE property and should continue using the sysfs interface
+ * (if available).
+ *
+ * Note: This property can be set through MODE_ATOMIC ioctl as part of the
+ * atomic state.
* panel_type:
* Immutable enum property to indicate the type of connected panel.
* Possible values are "unknown" (default) and "OLED".
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index ec820686b3021..4d2520de7614c 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -41,6 +41,7 @@
#include <linux/slab.h>
#include <linux/vga_switcheroo.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_client_event.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
@@ -252,6 +253,10 @@ void drm_file_free(struct drm_file *file)
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
drm_fb_release(file);
drm_property_destroy_user_blobs(dev, file);
+ if (file->supports_luminance_control) {
+ drm_backlight_uninhibit_legacy_all(dev);
+ file->supports_luminance_control = false;
+ }
}
if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ff193155129e7..fdae36b13300a 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -28,12 +28,14 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/
+#include "drm/drm.h"
#include <linux/export.h>
#include <linux/nospec.h>
#include <linux/pci.h>
#include <linux/uaccess.h>
#include <drm/drm_auth.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_crtc.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
@@ -380,6 +382,19 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
return -EINVAL;
file_priv->plane_color_pipeline = req->value;
break;
+ case DRM_CLIENT_CAP_LUMINANCE:
+ if (!file_priv->atomic)
+ return -EINVAL;
+ if (req->value > 1)
+ return -EINVAL;
+ if (req->value == file_priv->supports_luminance_control)
+ break;
+ if (req->value)
+ drm_backlight_inhibit_legacy_all(dev);
+ else
+ drm_backlight_uninhibit_legacy_all(dev);
+ file_priv->supports_luminance_control = req->value;
+ break;
default:
return -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index f38f2c5437e68..4475896c963b4 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -952,6 +952,12 @@ bool drm_property_change_valid_get(struct drm_property *property,
*ref = NULL;
if (drm_property_type_is(property, DRM_MODE_PROP_RANGE)) {
+ /* Special case for luminance property: allow 0 to turn off display
+ * even when the normal range starts at 1.
+ */
+ if (property == property->dev->mode_config.luminance_property &&
+ value == 0 && property->values[1] > 0)
+ return true;
if (value < property->values[0] || value > property->values[1])
return false;
return true;
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 13954c2220b7e..40cfc2296445c 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -217,6 +217,13 @@ static ssize_t brightness_store(struct device *dev,
struct backlight_device *bd = to_backlight_device(dev);
unsigned long brightness;
+ /* A luminance-aware DRM client has taken over this backlight; the
+ * legacy sysfs interface is disabled until the last such client
+ * goes away.
+ */
+ if (atomic_read(&bd->drm_takeover) > 0)
+ return -EBUSY;
+
rc = kstrtoul(buf, 0, &brightness);
if (rc)
return rc;
diff --git a/include/drm/drm_backlight.h b/include/drm/drm_backlight.h
index e0e09e38f7c06..2af48be3aa372 100644
--- a/include/drm/drm_backlight.h
+++ b/include/drm/drm_backlight.h
@@ -43,9 +43,9 @@ void drm_backlight_unregister(struct drm_backlight *b);
void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd);
struct backlight_device *drm_backlight_get_device(struct drm_backlight *b);
-void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
void drm_backlight_inhibit_legacy(struct drm_backlight *b);
void drm_backlight_uninhibit_legacy(struct drm_backlight *b);
void drm_backlight_inhibit_legacy_all(struct drm_device *dev);
void drm_backlight_uninhibit_legacy_all(struct drm_device *dev);
+void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
#endif /* __DRM_BACKLIGHT_H__ */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index d78ac1068d12e..5e32b6063d228 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1209,6 +1209,11 @@ struct drm_connector_state {
* @drm_atomic_helper_connector_hdmi_check().
*/
struct drm_connector_hdmi_state hdmi;
+
+ /**
+ * @luminance: Luminance for the connector
+ */
+ unsigned int luminance;
};
struct drm_connector_hdmi_audio_funcs {
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 6ee70ad65e1fd..0bb1e53f36bec 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -248,6 +248,14 @@ struct drm_file {
*/
bool supports_virtualized_cursor_plane;
+ /**
+ * @supports_luminance_control:
+ *
+ * This client is capable of setting the luminance for connectors.
+ *
+ */
+ bool supports_luminance_control;
+
/**
* @master:
*
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 26a7281d179c1..b03737ee8dacd 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -197,6 +197,13 @@ struct backlight_properties {
*/
int brightness;
+ /**
+ * @can_disable: Minimum brightness would turn off the panel.
+ *
+ * Setting minimum brightness turns off the panel.
+ */
+ bool can_disable;
+
/**
* @max_brightness: The maximum brightness value.
*
@@ -314,6 +321,14 @@ struct backlight_device {
* @use_count: The number of unblanked displays.
*/
int use_count;
+
+ /**
+ * @drm_takeover: Number of luminance-aware DRM clients that have
+ * taken over brightness control of this device. When non-zero,
+ * writes to the legacy sysfs ``brightness`` attribute return
+ * ``-EBUSY``. Managed by the DRM backlight helpers.
+ */
+ atomic_t drm_takeover;
};
/* Forward declaration for backlight_update_status */
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index bc7ef7684099b..a3141d46d7d66 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -903,6 +903,28 @@ struct drm_get_cap {
*/
#define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE 7
+/**
+ * DRM_CLIENT_CAP_LUMINANCE
+ *
+ * If set to 1, the client declares support for the LUMINANCE connector property
+ * and will control backlight brightness through the DRM atomic interface. This
+ * enables the kernel to expose the LUMINANCE property on connectors that have
+ * an associated backlight device.
+ *
+ * When this capability is enabled:
+ * - The LUMINANCE property becomes visible on supported connectors
+ * - Legacy sysfs writes to /sys/class/backlight/{*}/brightness will return
+ * -EBUSY to prevent conflicts with DRM-based brightness control
+ * - The client should include luminance values as part of atomic commits
+ * - Brightness changes are synchronized with display power state (DPMS)
+ *
+ * The LUMINANCE property accepts values from 0 to max_brightness, where 0 turns
+ * off the backlight, and 1 to max_brightness control the brightness level.
+ *
+ * This capability is supported starting in kernel 7.2.
+ */
+#define DRM_CLIENT_CAP_LUMINANCE 8
+
/* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
struct drm_set_client_cap {
__u64 capability;
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v6 04/10] DRM: Add support for client indicating support for luminance
2026-06-24 16:57 ` [PATCH v6 04/10] DRM: Add support for client indicating support for luminance Mario Limonciello
@ 2026-07-21 18:31 ` Hans de Goede
0 siblings, 0 replies; 19+ messages in thread
From: Hans de Goede @ 2026-07-21 18:31 UTC (permalink / raw)
To: Mario Limonciello, dri-devel, harry.wentland, Simona Vetter,
Alex Deucher, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov
Hi Mario,
On 24-Jun-26 18:57, Mario Limonciello wrote:
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> The legacy backlight control interface can only be disabled when both
> the client and driver has connector support that the luminance can be
> set during a modeset. Add capability for the client to register.
>
> When a luminance-aware client sets DRM_CLIENT_CAP_LUMINANCE, each
> DRM-connected backlight on the device is marked as taken over. Writes
> to the legacy /sys/class/backlight/<dev>/brightness attribute then
> return -EBUSY until the last luminance-aware client clears the cap or
> closes its DRM file. The takeover follows the active backlight_device
> when drm_backlight_link() retargets the link.
>
> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # SM8150-HDK
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
<snip>
> diff --git a/drivers/gpu/drm/drm_backlight.c b/drivers/gpu/drm/drm_backlight.c
> index b1ec470be86ca..2cddf209d5088 100644
> --- a/drivers/gpu/drm/drm_backlight.c
> +++ b/drivers/gpu/drm/drm_backlight.c
> @@ -71,6 +71,7 @@ static bool __drm_backlight_is_registered(struct drm_backlight *b)
> /* caller must hold @drm_backlight_lock */
> static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
> {
> + struct drm_connector *connector = b->connector;
> unsigned int max, set;
>
> lockdep_assert_held(&drm_backlight_lock);
> @@ -85,6 +86,15 @@ static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
> set = v;
> if (set >= max)
> set = max;
> +
> + /* Update the atomic state directly.
> + * For atomic drivers, the luminance value is stored in
> + * connector->state->luminance, not in the legacy property array.
> + * We update it unconditionally to reflect the hardware state,
> + * regardless of DPMS.
> + */
> + if (connector->state)
> + connector->state->luminance = set;
> }
>
> /**
> @@ -100,18 +110,22 @@ static void __drm_backlight_update_prop_range(struct drm_backlight *b)
> struct drm_device *dev = b->connector->dev;
> struct drm_property *prop = dev->mode_config.luminance_property;
> unsigned int max = 0;
> + bool can_disable = false;
>
> lockdep_assert_held(&drm_backlight_lock);
>
> - if (b->link && b->link->props.max_brightness > 0)
> + if (b->link && b->link->props.max_brightness > 0) {
> max = b->link->props.max_brightness;
> + can_disable = b->link->props.can_disable;
> + }
>
> /* Update property range to match hardware capabilities.
> * Range of 0-0 indicates no backing device.
> - * Range of 1-max for normal operation (0 reserved for display off).
> + * Range of 1-max for normal operation.
> + * Range of 0-max means that the display would turn off at 0
> */
> if (prop->values[1] != max) {
> - prop->values[0] = max ? 1 : 0;
> + prop->values[0] = max ? (can_disable ? 0 : 1) : 0;
> prop->values[1] = max;
> }
So here we now get:
range 0 - 0 for max = 0, aka no backlight control support
range 0 - max for max != 0, can_disable = true
range 1 - max for max != 0, can_disable = false
<snip>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 128d431f0d6b0..70afe3d579f38 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1512,6 +1512,57 @@ EXPORT_SYMBOL(drm_hdmi_connector_get_output_format_name);
> * Summarizing: Only set "DPMS" when the connector is known to be enabled,
> * assume that a successful SETCONFIG call also sets "DPMS" to on, and
> * never read back the value of "DPMS" because it can be incorrect.
> + * LUMINANCE:
> + * Atomic property for controlling the backlight brightness level of the
> + * connector's display. This property provides unified access to the display
> + * backlight, replacing the legacy sysfs interface for brightness control.
> + *
> + * The property value is an unsigned integer representing the brightness level.
> + * The valid range is dynamically determined by the capabilities of the
> + * connected backlight hardware and is exposed through the property's minimum
> + * and maximum values:
> + *
> + * - Range 0-0: No backlight device is available for this connector.
> + * - Range 1-N: Normal operation. Values from 1 to N (max_brightness) are
> + * valid brightness levels, where 1 is the minimum visible brightness and
> + * N is the maximum brightness the hardware supports.
But here range 0 - N is not documented.
Good that this documents that "1 is the minimum visible brightness" but I think
some drivers may need to be adjusted for that. Last time I checked the amdgpu
driver would take a certain fixed minimum percentage as lowest brightness when
the firmware provided a 0 value for minimum pwm duty-cycle, so amdgpu has this
covered. But the i915 driver happily goes down to 0% duty when the VBT does
not give a non 0 minimum.
This is why e.g. GNOME adds its own minimum percentage and never writes 0 to
the classic sysfs interface. So assuming this all holds, this means that
supporting "1 is the minimum visible brightness" will require some fixes on
the i915 driver (which would be good to have) and possibly other drivers
need fixing as well.
> + * - Value 0: Special value to turn off the display backlight completely.
> + * This value is accepted even when the normal range starts at 1.
> + *> + * The range may change during runtime if a new backlight device is linked
> + * or unlinked. The kernel will send a change uevent when this occurs.
> + *
> + * Setting LUMINANCE to 0 turns off the backlight, which may turn off the
> + * display completely depending on the hardware.
Ok, so 0 is DPMS off, right ? maybe mention that explicitly because
I think that has certain implications like possible no longer getting
vsync events / framebuffer swap completions which userspace might
depend on. Where as a classic set pwm duty to 0 for backlight does
result in vsync events to continue. "turn off the display" is a bit
ambigous IMHO, e.g. is it just the panel which gets turned off or
the entire dsiplay pipeline?
Might also just me not being familiar enough with the kms API...
Regards,
Hans
> Setting it to any value
> + * from 1 to N adjusts the brightness accordingly. Reading this property
> + * returns the current brightness level that was last set (or the hardware's
> + * current state for drivers that support reading actual brightness).
> + *
> + * For atomic drivers, the luminance value is stored in
> + * &drm_connector_state.luminance. The actual hardware update only occurs
> + * when the connector is active (DPMS is ON). When DPMS transitions to OFF,
> + * the kernel automatically sets luminance to 0 to turn off the backlight.
> + * When DPMS transitions back to ON, the kernel restores the previously
> + * set luminance value.
> + *
> + * This property is only available on connectors that have an associated
> + * backlight device. The property is created by calling drm_backlight_alloc()
> + * during connector initialization.
> + *
> + * Client Capability:
> + * User-space must set the DRM_CLIENT_CAP_LUMINANCE client capability
> + * to 1 before using this property. When this capability is enabled,
> + * the legacy sysfs backlight interface is inhibited to prevent
> + * conflicts between multiple clients trying to control the same
> + * backlight. This ensures that only luminance-aware clients control
> + * the backlight through the DRM atomic interface.
> + *
> + * Legacy clients that do not set this capability will not see the
> + * LUMINANCE property and should continue using the sysfs interface
> + * (if available).
> + *
> + * Note: This property can be set through MODE_ATOMIC ioctl as part of the
> + * atomic state.
> * panel_type:
> * Immutable enum property to indicate the type of connected panel.
> * Possible values are "unknown" (default) and "OLED".
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index ec820686b3021..4d2520de7614c 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -41,6 +41,7 @@
> #include <linux/slab.h>
> #include <linux/vga_switcheroo.h>
>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_client_event.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> @@ -252,6 +253,10 @@ void drm_file_free(struct drm_file *file)
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> drm_fb_release(file);
> drm_property_destroy_user_blobs(dev, file);
> + if (file->supports_luminance_control) {
> + drm_backlight_uninhibit_legacy_all(dev);
> + file->supports_luminance_control = false;
> + }
> }
>
> if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index ff193155129e7..fdae36b13300a 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -28,12 +28,14 @@
> * OTHER DEALINGS IN THE SOFTWARE.
> */
>
> +#include "drm/drm.h"
> #include <linux/export.h>
> #include <linux/nospec.h>
> #include <linux/pci.h>
> #include <linux/uaccess.h>
>
> #include <drm/drm_auth.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> @@ -380,6 +382,19 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
> return -EINVAL;
> file_priv->plane_color_pipeline = req->value;
> break;
> + case DRM_CLIENT_CAP_LUMINANCE:
> + if (!file_priv->atomic)
> + return -EINVAL;
> + if (req->value > 1)
> + return -EINVAL;
> + if (req->value == file_priv->supports_luminance_control)
> + break;
> + if (req->value)
> + drm_backlight_inhibit_legacy_all(dev);
> + else
> + drm_backlight_uninhibit_legacy_all(dev);
> + file_priv->supports_luminance_control = req->value;
> + break;
> default:
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index f38f2c5437e68..4475896c963b4 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -952,6 +952,12 @@ bool drm_property_change_valid_get(struct drm_property *property,
> *ref = NULL;
>
> if (drm_property_type_is(property, DRM_MODE_PROP_RANGE)) {
> + /* Special case for luminance property: allow 0 to turn off display
> + * even when the normal range starts at 1.
> + */
> + if (property == property->dev->mode_config.luminance_property &&
> + value == 0 && property->values[1] > 0)
> + return true;
> if (value < property->values[0] || value > property->values[1])
> return false;
> return true;
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 13954c2220b7e..40cfc2296445c 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -217,6 +217,13 @@ static ssize_t brightness_store(struct device *dev,
> struct backlight_device *bd = to_backlight_device(dev);
> unsigned long brightness;
>
> + /* A luminance-aware DRM client has taken over this backlight; the
> + * legacy sysfs interface is disabled until the last such client
> + * goes away.
> + */
> + if (atomic_read(&bd->drm_takeover) > 0)
> + return -EBUSY;
> +
> rc = kstrtoul(buf, 0, &brightness);
> if (rc)
> return rc;
> diff --git a/include/drm/drm_backlight.h b/include/drm/drm_backlight.h
> index e0e09e38f7c06..2af48be3aa372 100644
> --- a/include/drm/drm_backlight.h
> +++ b/include/drm/drm_backlight.h
> @@ -43,9 +43,9 @@ void drm_backlight_unregister(struct drm_backlight *b);
>
> void drm_backlight_link(struct drm_backlight *b, struct backlight_device *bd);
> struct backlight_device *drm_backlight_get_device(struct drm_backlight *b);
> -void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
> void drm_backlight_inhibit_legacy(struct drm_backlight *b);
> void drm_backlight_uninhibit_legacy(struct drm_backlight *b);
> void drm_backlight_inhibit_legacy_all(struct drm_device *dev);
> void drm_backlight_uninhibit_legacy_all(struct drm_device *dev);
> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int value);
> #endif /* __DRM_BACKLIGHT_H__ */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index d78ac1068d12e..5e32b6063d228 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1209,6 +1209,11 @@ struct drm_connector_state {
> * @drm_atomic_helper_connector_hdmi_check().
> */
> struct drm_connector_hdmi_state hdmi;
> +
> + /**
> + * @luminance: Luminance for the connector
> + */
> + unsigned int luminance;
> };
>
> struct drm_connector_hdmi_audio_funcs {
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 6ee70ad65e1fd..0bb1e53f36bec 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -248,6 +248,14 @@ struct drm_file {
> */
> bool supports_virtualized_cursor_plane;
>
> + /**
> + * @supports_luminance_control:
> + *
> + * This client is capable of setting the luminance for connectors.
> + *
> + */
> + bool supports_luminance_control;
> +
> /**
> * @master:
> *
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 26a7281d179c1..b03737ee8dacd 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -197,6 +197,13 @@ struct backlight_properties {
> */
> int brightness;
>
> + /**
> + * @can_disable: Minimum brightness would turn off the panel.
> + *
> + * Setting minimum brightness turns off the panel.
> + */
> + bool can_disable;
> +
> /**
> * @max_brightness: The maximum brightness value.
> *
> @@ -314,6 +321,14 @@ struct backlight_device {
> * @use_count: The number of unblanked displays.
> */
> int use_count;
> +
> + /**
> + * @drm_takeover: Number of luminance-aware DRM clients that have
> + * taken over brightness control of this device. When non-zero,
> + * writes to the legacy sysfs ``brightness`` attribute return
> + * ``-EBUSY``. Managed by the DRM backlight helpers.
> + */
> + atomic_t drm_takeover;
> };
>
> /* Forward declaration for backlight_update_status */
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index bc7ef7684099b..a3141d46d7d66 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -903,6 +903,28 @@ struct drm_get_cap {
> */
> #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE 7
>
> +/**
> + * DRM_CLIENT_CAP_LUMINANCE
> + *
> + * If set to 1, the client declares support for the LUMINANCE connector property
> + * and will control backlight brightness through the DRM atomic interface. This
> + * enables the kernel to expose the LUMINANCE property on connectors that have
> + * an associated backlight device.
> + *
> + * When this capability is enabled:
> + * - The LUMINANCE property becomes visible on supported connectors
> + * - Legacy sysfs writes to /sys/class/backlight/{*}/brightness will return
> + * -EBUSY to prevent conflicts with DRM-based brightness control
> + * - The client should include luminance values as part of atomic commits
> + * - Brightness changes are synchronized with display power state (DPMS)
> + *
> + * The LUMINANCE property accepts values from 0 to max_brightness, where 0 turns
> + * off the backlight, and 1 to max_brightness control the brightness level.
> + *
> + * This capability is supported starting in kernel 7.2.
> + */
> +#define DRM_CLIENT_CAP_LUMINANCE 8
> +
> /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> struct drm_set_client_cap {
> __u64 capability;
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 05/10] drm/amd/display: Pass up errors reading actual brightness
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (3 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 04/10] DRM: Add support for client indicating support for luminance Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 06/10] drm/amd/display: Allow backlight registration to fail Mario Limonciello
` (5 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Simon Ser
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
[Why]
If the DC API fails to return actual brightness when backlight control
API requests it, then the wrong value may be returned.
[How]
Change return type of amdgpu_dm_backlight_get_level() to an integer
and pass an error code up to the caller.
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Simon Ser <contact@emersion.fr>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index eb5696b5daeb7..9b3e2fc6cef5d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5492,7 +5492,7 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
return 0;
}
-static u32 amdgpu_dm_backlight_get_level(struct amdgpu_display_manager *dm,
+static int amdgpu_dm_backlight_get_level(struct amdgpu_display_manager *dm,
int bl_idx)
{
int ret;
@@ -5506,14 +5506,14 @@ static u32 amdgpu_dm_backlight_get_level(struct amdgpu_display_manager *dm,
u32 avg, peak;
if (!dc_link_get_backlight_level_nits(link, &avg, &peak))
- return dm->brightness[bl_idx];
+ return -EINVAL;
return convert_brightness_to_user(&caps, avg);
}
ret = dc_link_get_backlight_level(link);
if (ret == DC_ERROR_UNEXPECTED)
- return dm->brightness[bl_idx];
+ return -EINVAL;
return convert_brightness_to_user(&caps, ret);
}
@@ -5521,7 +5521,7 @@ static u32 amdgpu_dm_backlight_get_level(struct amdgpu_display_manager *dm,
static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
{
struct amdgpu_display_manager *dm = bl_get_data(bd);
- int i;
+ int i, ret;
for (i = 0; i < dm->num_of_edps; i++) {
if (bd == dm->backlight_dev[i])
@@ -5529,7 +5529,12 @@ static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
}
if (i >= AMDGPU_DM_MAX_NUM_EDP)
i = 0;
- return amdgpu_dm_backlight_get_level(dm, i);
+
+ ret = amdgpu_dm_backlight_get_level(dm, i);
+ if (ret < 0)
+ return dm->brightness[i];
+
+ return ret;
}
static const struct backlight_ops amdgpu_dm_backlight_ops = {
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v6 06/10] drm/amd/display: Allow backlight registration to fail
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (4 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 05/10] drm/amd/display: Pass up errors reading actual brightness Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 07/10] drm/amd/display: use drm backlight Mario Limonciello
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Simon Ser
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
[Why]
If backlight registration fails then it's ignored. This could hide
a fatal problem to a user.
[How]
Pass error codes up from backlight registration failures.
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Simon Ser <contact@emersion.fr>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 +++++++++++--------
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9b3e2fc6cef5d..2f87940aaea42 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5543,7 +5543,7 @@ static const struct backlight_ops amdgpu_dm_backlight_ops = {
.update_status = amdgpu_dm_backlight_update_status,
};
-static void
+static int
amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
{
struct drm_device *drm = aconnector->base.dev;
@@ -5554,15 +5554,16 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
int min, max;
int real_brightness;
int init_brightness;
+ int r;
if (aconnector->bl_idx == -1)
- return;
+ return 0;
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;
+ return 0;
}
caps = &dm->backlight_caps[aconnector->bl_idx];
@@ -5596,22 +5597,26 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
dm->brightness[aconnector->bl_idx] = props.brightness;
if (IS_ERR(dm->backlight_dev[aconnector->bl_idx])) {
- drm_err(drm, "DM: Backlight registration failed!\n");
+ r = PTR_ERR(dm->backlight_dev[aconnector->bl_idx]);
+ drm_err(drm, "DM: Backlight registration failed: %d\n", r);
dm->backlight_dev[aconnector->bl_idx] = NULL;
- } else {
- /*
- * dm->brightness[x] can be inconsistent just after startup until
- * ops.get_brightness is called.
- */
- real_brightness =
- amdgpu_dm_backlight_ops.get_brightness(dm->backlight_dev[aconnector->bl_idx]);
+ return r;
+ }
- if (real_brightness != init_brightness) {
- dm->actual_brightness[aconnector->bl_idx] = real_brightness;
- dm->brightness[aconnector->bl_idx] = real_brightness;
- }
- drm_dbg_driver(drm, "DM: Registered Backlight device: %s\n", bl_name);
+ /*
+ * dm->brightness[x] can be inconsistent just after startup until
+ * ops.get_brightness is called.
+ */
+ real_brightness =
+ amdgpu_dm_backlight_ops.get_brightness(dm->backlight_dev[aconnector->bl_idx]);
+
+ if (real_brightness != init_brightness) {
+ dm->actual_brightness[aconnector->bl_idx] = real_brightness;
+ dm->brightness[aconnector->bl_idx] = real_brightness;
}
+ drm_dbg_driver(drm, "DM: Registered Backlight device: %s\n", bl_name);
+
+ return 0;
}
static int initialize_plane(struct amdgpu_display_manager *dm,
@@ -8225,7 +8230,9 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)
return r;
}
- amdgpu_dm_register_backlight_device(amdgpu_dm_connector);
+ r = amdgpu_dm_register_backlight_device(amdgpu_dm_connector);
+ if (r)
+ return r;
if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||
(connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v6 07/10] drm/amd/display: use drm backlight
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (5 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 06/10] drm/amd/display: Allow backlight registration to fail Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 08/10] drm/amd/display: Drop brightness caching in amdgpu_dm Mario Limonciello
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Simon Ser
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
Convert AMD display driver to use the new DRM backlight infrastructure.
This allows brightness control via DRM connector properties.
Includes fixes for backlight resource lifecycle management.
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Simon Ser <contact@emersion.fr>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2f87940aaea42..9653cfd99c6ad 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -86,6 +86,7 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_uapi.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_blend.h>
#include <drm/drm_fixed.h>
#include <drm/drm_fourcc.h>
@@ -5614,6 +5615,15 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
dm->actual_brightness[aconnector->bl_idx] = real_brightness;
dm->brightness[aconnector->bl_idx] = real_brightness;
}
+
+ /* Link the registered backlight device to the DRM connector. If
+ * drm_backlight_alloc() failed earlier in init_helper,
+ * aconnector->base.backlight is NULL and drm_backlight_link() is a
+ * no-op.
+ */
+ drm_backlight_link(aconnector->base.backlight,
+ dm->backlight_dev[aconnector->bl_idx]);
+
drm_dbg_driver(drm, "DM: Registered Backlight device: %s\n", bl_name);
return 0;
@@ -8140,6 +8150,7 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
}
if (aconnector->bl_idx != -1) {
+ drm_backlight_link(aconnector->base.backlight, NULL);
backlight_device_unregister(dm->backlight_dev[aconnector->bl_idx]);
dm->backlight_dev[aconnector->bl_idx] = NULL;
}
@@ -8152,6 +8163,10 @@ static void amdgpu_dm_connector_destroy(struct drm_connector *connector)
aconnector->dc_sink = NULL;
drm_dp_cec_unregister_connector(&aconnector->dm_dp_aux.aux);
+ /* DRM core will handle drm_backlight cleanup via:
+ * drm_connector_unregister() -> drm_backlight_unregister()
+ * drm_connector_cleanup() -> drm_backlight_free()
+ */
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
kfree(aconnector->dm_dp_aux.aux.name);
@@ -9402,6 +9417,12 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
aconnector->base.state->max_bpc = 16;
aconnector->base.state->max_requested_bpc = aconnector->base.state->max_bpc;
+ if (connector_type == DRM_MODE_CONNECTOR_eDP) {
+ int r = drm_backlight_alloc(&aconnector->base);
+
+ if (r)
+ drm_err(dm->ddev, "Failed to allocate backlight: %d\n", r);
+ }
if (connector_type == DRM_MODE_CONNECTOR_HDMIA) {
/* Content Type is currently only implemented for HDMI. */
drm_connector_attach_content_type_property(&aconnector->base);
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v6 08/10] drm/amd/display: Drop brightness caching in amdgpu_dm
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (6 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 07/10] drm/amd/display: use drm backlight Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 09/10] drm/bridge: auto-link panel backlight in bridge connector Mario Limonciello
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD)
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
[Why]
Brightness caching isn't necessary when the core is changing brightness
as part of the connector state.
[How]
Drop all brightness and actual brightness caching
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 72 +++----------------
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 13 ----
2 files changed, 9 insertions(+), 76 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9653cfd99c6ad..43b22ab9180e8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3798,12 +3798,6 @@ static int dm_resume(struct amdgpu_ip_block *ip_block)
mutex_unlock(&dm->dc_lock);
- /* set the backlight after a reset */
- for (i = 0; i < dm->num_of_edps; i++) {
- if (dm->backlight_dev[i])
- amdgpu_dm_backlight_set_level(dm, i, dm->brightness[i]);
- }
-
return 0;
}
/* Recreate dc_state - DC invalidates it when setting power state to S3. */
@@ -5388,7 +5382,7 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
struct amdgpu_dm_backlight_caps *caps;
struct dc_link *link;
u32 brightness = 0;
- bool rc = false, reallow_idle = false;
+ bool reallow_idle = false;
struct drm_connector *connector;
struct dc_stream_state *stream;
unsigned int min, max;
@@ -5399,10 +5393,8 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
if (aconnector->bl_idx != bl_idx)
continue;
- /* if connector is off, save the brightness for next time it's on */
+ /* if connector is off, DRM core will restore it next time it's on */
if (!aconnector->base.encoder) {
- dm->brightness[bl_idx] = user_brightness;
- dm->actual_brightness[bl_idx] = 0;
return;
}
}
@@ -5410,11 +5402,10 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
amdgpu_dm_update_backlight_caps(dm, bl_idx);
caps = &dm->backlight_caps[bl_idx];
- dm->brightness[bl_idx] = user_brightness;
/* update scratch register */
if (bl_idx == 0)
- amdgpu_atombios_scratch_regs_set_backlight_level(dm->adev, dm->brightness[bl_idx]);
- brightness = convert_brightness_from_user(caps, dm->brightness[bl_idx]);
+ amdgpu_atombios_scratch_regs_set_backlight_level(dm->adev, user_brightness);
+ brightness = convert_brightness_from_user(caps, user_brightness);
link = (struct dc_link *)dm->backlight_link[bl_idx];
/* Apply brightness quirk */
@@ -5440,14 +5431,14 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
}
if (caps->aux_support) {
- rc = mod_power_set_backlight_nits(dm->power_module, stream, brightness,
+ mod_power_set_backlight_nits(dm->power_module, stream, brightness,
AUX_BL_DEFAULT_TRANSITION_TIME_MS, false, true);
} else {
/* power module uses millipercent */
get_brightness_range(caps, &min, &max);
brightness = DIV_ROUND_CLOSEST(brightness * 100, (max - min)) * 1000;
- rc = mod_power_set_backlight_percent(dm->power_module, stream,
- brightness, 0, false);
+ mod_power_set_backlight_percent(dm->power_module, stream,
+ brightness, 0, false);
}
/*
@@ -5472,9 +5463,6 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm,
dc_allow_idle_optimizations(dm->dc, true);
mutex_unlock(&dm->dc_lock);
-
- if (rc)
- dm->actual_brightness[bl_idx] = user_brightness;
}
static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
@@ -5522,7 +5510,7 @@ static int amdgpu_dm_backlight_get_level(struct amdgpu_display_manager *dm,
static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
{
struct amdgpu_display_manager *dm = bl_get_data(bd);
- int i, ret;
+ int i;
for (i = 0; i < dm->num_of_edps; i++) {
if (bd == dm->backlight_dev[i])
@@ -5531,11 +5519,7 @@ static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
if (i >= AMDGPU_DM_MAX_NUM_EDP)
i = 0;
- ret = amdgpu_dm_backlight_get_level(dm, i);
- if (ret < 0)
- return dm->brightness[i];
-
- return ret;
+ return amdgpu_dm_backlight_get_level(dm, i);
}
static const struct backlight_ops amdgpu_dm_backlight_ops = {
@@ -5553,8 +5537,6 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
struct amdgpu_dm_backlight_caps *caps;
char bl_name[16];
int min, max;
- int real_brightness;
- int init_brightness;
int r;
if (aconnector->bl_idx == -1)
@@ -5580,8 +5562,6 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
} else
props.brightness = props.max_brightness = MAX_BACKLIGHT_LEVEL;
- init_brightness = props.brightness;
-
if (caps->data_points && !(amdgpu_dc_debug_mask & DC_DISABLE_CUSTOM_BRIGHTNESS_CURVE)) {
drm_info(drm, "Using custom brightness curve\n");
props.scale = BACKLIGHT_SCALE_NON_LINEAR;
@@ -5595,7 +5575,6 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
dm->backlight_dev[aconnector->bl_idx] =
backlight_device_register(bl_name, aconnector->base.kdev, dm,
&amdgpu_dm_backlight_ops, &props);
- dm->brightness[aconnector->bl_idx] = props.brightness;
if (IS_ERR(dm->backlight_dev[aconnector->bl_idx])) {
r = PTR_ERR(dm->backlight_dev[aconnector->bl_idx]);
@@ -5604,18 +5583,6 @@ amdgpu_dm_register_backlight_device(struct amdgpu_dm_connector *aconnector)
return r;
}
- /*
- * dm->brightness[x] can be inconsistent just after startup until
- * ops.get_brightness is called.
- */
- real_brightness =
- amdgpu_dm_backlight_ops.get_brightness(dm->backlight_dev[aconnector->bl_idx]);
-
- if (real_brightness != init_brightness) {
- dm->actual_brightness[aconnector->bl_idx] = real_brightness;
- dm->brightness[aconnector->bl_idx] = real_brightness;
- }
-
/* Link the registered backlight device to the DRM connector. If
* drm_backlight_alloc() failed earlier in init_helper,
* aconnector->base.backlight is NULL and drm_backlight_link() is a
@@ -10894,7 +10861,6 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_commit *state,
bool mode_set_reset_required = false;
u32 i;
struct dc_commit_streams_params params = {dc_state->streams, dc_state->stream_count};
- bool set_backlight_level = false;
/* Disable writeback */
for_each_old_connector_in_state(state, connector, old_con_state, i) {
@@ -11016,7 +10982,6 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_commit *state,
acrtc->hw_mode = new_crtc_state->mode;
crtc->hwmode = new_crtc_state->mode;
mode_set_reset_required = true;
- set_backlight_level = true;
} else if (modereset_required(new_crtc_state)) {
drm_dbg_atomic(dev,
"Atomic commit: RESET. crtc id %d:[%p]\n",
@@ -11085,18 +11050,6 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_commit *state,
}
}
- /* During boot up and resume the DC layer will reset the panel brightness
- * to fix a flicker issue.
- * It will cause the dm->actual_brightness is not the current panel brightness
- * level. (the dm->brightness is the correct panel level)
- * So we set the backlight level with dm->brightness value after set mode
- */
- if (set_backlight_level) {
- for (i = 0; i < dm->num_of_edps; i++) {
- if (dm->backlight_dev[i])
- amdgpu_dm_backlight_set_level(dm, i, dm->brightness[i]);
- }
- }
}
static void dm_set_writeback(struct amdgpu_display_manager *dm,
@@ -11604,13 +11557,6 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_commit *state)
/* Update audio instances for each connector. */
amdgpu_dm_commit_audio(dev, state);
- /* restore the backlight level */
- for (i = 0; i < dm->num_of_edps; i++) {
- if (dm->backlight_dev[i] &&
- (dm->actual_brightness[i] != dm->brightness[i]))
- amdgpu_dm_backlight_set_level(dm, i, dm->brightness[i]);
- }
-
/*
* send vblank event on all events not handled in flip and
* mark consumed event for drm_atomic_helper_commit_hw_done
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index dd199e0b79226..1aeb223256fa7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -645,19 +645,6 @@ struct amdgpu_display_manager {
struct completion dmub_aux_transfer_done;
struct workqueue_struct *delayed_hpd_wq;
- /**
- * @brightness:
- *
- * cached backlight values.
- */
- u32 brightness[AMDGPU_DM_MAX_NUM_EDP];
- /**
- * @actual_brightness:
- *
- * last successfully applied backlight values.
- */
- u32 actual_brightness[AMDGPU_DM_MAX_NUM_EDP];
-
/**
* @aux_hpd_discon_quirk:
*
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v6 09/10] drm/bridge: auto-link panel backlight in bridge connector
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (7 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 08/10] drm/amd/display: Drop brightness caching in amdgpu_dm Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-06-24 16:57 ` [PATCH v6 10/10] drm/i915/display: use drm backlight Mario Limonciello
2026-07-21 17:50 ` [PATCH v6 00/10] Add support for a DRM backlight capability Hans de Goede
10 siblings, 0 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD), Dmitry Baryshkov, Simon Ser
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
The link should allow clients to change luminance property.
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> # SM8150-HDK
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Simon Ser <contact@emersion.fr>
---
drivers/gpu/drm/bridge/panel.c | 15 +++++++++++++++
drivers/gpu/drm/display/drm_bridge_connector.c | 15 ++++++++++++++-
include/drm/drm_bridge.h | 1 +
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 4978ec98a0828..2fa15278faf87 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -233,6 +233,21 @@ bool drm_bridge_is_panel(const struct drm_bridge *bridge)
}
EXPORT_SYMBOL(drm_bridge_is_panel);
+/**
+ * drm_panel_bridge_to_panel - get the drm_panel wrapped by a panel bridge
+ * @bridge: the panel bridge
+ *
+ * Returns the &drm_panel wrapped by @bridge, or NULL if @bridge is not a
+ * panel bridge.
+ */
+struct drm_panel *drm_panel_bridge_to_panel(struct drm_bridge *bridge)
+{
+ if (!drm_bridge_is_panel(bridge))
+ return NULL;
+ return drm_bridge_to_panel_bridge(bridge)->panel;
+}
+EXPORT_SYMBOL(drm_panel_bridge_to_panel);
+
/**
* drm_panel_bridge_add - Creates a &drm_bridge and &drm_connector that
* just calls the appropriate functions from &drm_panel.
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 649969fca1413..23a3802294625 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -11,7 +11,9 @@
#include <linux/slab.h>
#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_bridge.h>
+#include <drm/drm_panel.h>
#include <drm/drm_bridge_connector.h>
#include <drm/drm_connector.h>
#include <drm/drm_device.h>
@@ -1051,9 +1053,20 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
connector->polled = DRM_CONNECTOR_POLL_CONNECT
| DRM_CONNECTOR_POLL_DISCONNECT;
- if (panel_bridge)
+ if (panel_bridge) {
+ struct drm_panel *panel;
+
drm_panel_bridge_set_orientation(connector, panel_bridge);
+ panel = drm_panel_bridge_to_panel(panel_bridge);
+ if (panel && panel->backlight) {
+ ret = drm_backlight_alloc(connector);
+ if (!ret)
+ drm_backlight_link(connector->backlight,
+ panel->backlight);
+ }
+ }
+
if (support_hdcp && IS_REACHABLE(CONFIG_DRM_DISPLAY_HELPER) &&
IS_ENABLED(CONFIG_DRM_DISPLAY_HDCP_HELPER))
drm_connector_attach_content_protection_property(connector, true);
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4ba3a5deef9a6..6311ca726b45f 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1586,6 +1586,7 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
#ifdef CONFIG_DRM_PANEL_BRIDGE
bool drm_bridge_is_panel(const struct drm_bridge *bridge);
+struct drm_panel *drm_panel_bridge_to_panel(struct drm_bridge *bridge);
struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
struct drm_bridge *drm_panel_bridge_add_typed(struct drm_panel *panel,
u32 connector_type);
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v6 10/10] drm/i915/display: use drm backlight
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (8 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 09/10] drm/bridge: auto-link panel backlight in bridge connector Mario Limonciello
@ 2026-06-24 16:57 ` Mario Limonciello
2026-07-21 17:50 ` [PATCH v6 00/10] Add support for a DRM backlight capability Hans de Goede
10 siblings, 0 replies; 19+ messages in thread
From: Mario Limonciello @ 2026-06-24 16:57 UTC (permalink / raw)
To: dri-devel, harry.wentland, Simona Vetter, Alex Deucher,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello (AMD)
From: "Mario Limonciello (AMD)" <superm1@kernel.org>
Convert Intel display drivers (i915 and xe) to use the new DRM backlight
infrastructure. This allows brightness control via DRM connector
properties.
Allocate DRM backlight for eDP connectors during initialization, link the
backlight device to the connector during registration, and unlink during
unregistration.
Assisted-by: Sonnet:4
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
drivers/gpu/drm/i915/display/intel_backlight.c | 4 ++++
drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++++
2 files changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index b128896cb1c2d..4543c2a8f3568 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -9,6 +9,7 @@
#include <linux/string_helpers.h>
#include <acpi/video.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_file.h>
#include <drm/drm_print.h>
#include <drm/intel/pci_config.h>
@@ -1004,6 +1005,8 @@ int intel_backlight_device_register(struct intel_connector *connector)
"[CONNECTOR:%d:%s] backlight device %s registered\n",
connector->base.base.id, connector->base.name, name);
+ drm_backlight_link(connector->base.backlight, bd);
+
out:
kfree(name);
@@ -1015,6 +1018,7 @@ void intel_backlight_device_unregister(struct intel_connector *connector)
struct intel_panel *panel = &connector->panel;
if (panel->backlight.device) {
+ drm_backlight_link(connector->base.backlight, NULL);
backlight_device_unregister(panel->backlight.device);
panel->backlight.device = NULL;
}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0ce0c09835f6d..cd35cce329a55 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -44,6 +44,7 @@
#include <drm/display/drm_dsc_helper.h>
#include <drm/display/drm_hdmi_helper.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_backlight.h>
#include <drm/drm_crtc.h>
#include <drm/drm_edid.h>
#include <drm/drm_fixed.h>
@@ -7421,6 +7422,13 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
type, &intel_dp->aux.ddc);
drm_connector_helper_add(&connector->base, &intel_dp_connector_helper_funcs);
+ if (type == DRM_MODE_CONNECTOR_eDP) {
+ int r = drm_backlight_alloc(&connector->base);
+
+ if (r)
+ drm_err(display->drm, "Failed to allocate backlight: %d\n", r);
+ }
+
if (!HAS_GMCH(display) && DISPLAY_VER(display) < 12)
connector->base.interlace_allowed = true;
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v6 00/10] Add support for a DRM backlight capability
2026-06-24 16:57 [PATCH v6 00/10] Add support for a DRM backlight capability Mario Limonciello
` (9 preceding siblings ...)
2026-06-24 16:57 ` [PATCH v6 10/10] drm/i915/display: use drm backlight Mario Limonciello
@ 2026-07-21 17:50 ` Hans de Goede
2026-07-21 18:19 ` Hans de Goede
10 siblings, 1 reply; 19+ messages in thread
From: Hans de Goede @ 2026-07-21 17:50 UTC (permalink / raw)
To: Mario Limonciello, dri-devel, harry.wentland, Simona Vetter,
Alex Deucher, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello
Hi Mario,
On 24-Jun-26 18:57, Mario Limonciello wrote:
> From: Mario Limonciello (AMD) <superm1@kernel.org>
>
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v6 00/10] Add support for a DRM backlight capability
2026-07-21 17:50 ` [PATCH v6 00/10] Add support for a DRM backlight capability Hans de Goede
@ 2026-07-21 18:19 ` Hans de Goede
0 siblings, 0 replies; 19+ messages in thread
From: Hans de Goede @ 2026-07-21 18:19 UTC (permalink / raw)
To: Mario Limonciello, dri-devel, harry.wentland, Simona Vetter,
Alex Deucher, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie
Cc: Xaver Hugl, amd-gfx,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
open list:INTEL DRM DISPLAY FOR XE AND I915 DRIVERS,
Mario Limonciello
Hi,
On 21-Jul-26 19:50, Hans de Goede wrote:
> Hi Mario,
>
> On 24-Jun-26 18:57, Mario Limonciello wrote:
>> From: Mario Limonciello (AMD) <superm1@kernel.org>
>>
>> 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
p.s.
i. For non native backlights set-brightness calls should
probably be deferred to a workqueue because these often
fw calls can be quite slow.
ii. For the next version of this series please Cc me.
>
>
>
>
>
>
>>
>> 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
>
^ permalink raw reply [flat|nested] 19+ messages in thread