[PATCH v3 0/8] Add support for a DRM backlight capability

[PATCH v3 0/8] Add support for a DRM backlight capability

[Mario Limonciello]

From: Mario Limonciello (AMD) <superm1@kernel.org>

At Display Next Hackfest 2025 we discussed the renewed need for moving
brightness control into the DRM connector properties.  I've taken the
previous efforts from David and Marta, rebased and adjusted for the
current kernel.

The legacy sysfs interface is synchronized with the DRM connector (although
the scale may be different as DRM connector property is u16).

Later after this has been adopted by enough userspace, it may make sense to
configure the legacy sysfs interface to be configurable so that only
DRM master controls backlight.

I've done a first implementation with amdgpu with eDP connectors; but
conceivably this can be extended to other connectors like DP for displays
that can be controlled via DDC as well later.

I have also used DRM review prompts to review this series and fix some bugs
which were caught with two different Claude models.  The fixes are squashed
into the patches.

Assisted-by: Claude Opus
Assisted-by: Claude Sonnet

For ease of testing; this series is also available on this branch:
https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git/log/?h=superm1/backlight-property-v3

David Rheinsberg (1):
  backlight: add kernel-internal backlight API

Mario Limonciello (6):
  drm: link connectors to backlight devices
  DRM: Add support for client and driver indicating support for
    luminance
  drm/amd/display: Pass up errors reading actual brightness
  drm/amd: Indicate driver supports luminance
  drm/amd/display: Allow backlight registration to fail
  drm/amd/display: use drm backlight

Marta Lofstedt (1):
  backlight: expose the current brightness in the new kernel API

 drivers/gpu/drm/Kconfig                       |   1 +
 drivers/gpu/drm/Makefile                      |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |   1 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  88 +++-
 drivers/gpu/drm/drm_atomic_uapi.c             |  24 ++
 drivers/gpu/drm/drm_backlight.c               | 406 ++++++++++++++++++
 drivers/gpu/drm/drm_connector.c               |  12 +
 drivers/gpu/drm/drm_drv.c                     |   8 +
 drivers/gpu/drm/drm_ioctl.c                   |  10 +
 drivers/gpu/drm/drm_mode_config.c             |   7 +
 drivers/gpu/drm/drm_mode_object.c             |  66 ++-
 drivers/gpu/drm/drm_sysfs.c                   |  54 +++
 drivers/video/backlight/backlight.c           |  83 ++++
 include/drm/drm_backlight.h                   |  45 ++
 include/drm/drm_connector.h                   |   8 +
 include/drm/drm_drv.h                         |   7 +
 include/drm/drm_file.h                        |   8 +
 include/drm/drm_mode_config.h                 |   5 +
 include/linux/backlight.h                     |  30 ++
 include/uapi/drm/drm.h                        |  10 +
 20 files changed, 852 insertions(+), 23 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_backlight.c
 create mode 100644 include/drm/drm_backlight.h

-- 
2.53.0

[PATCH v3 1/8] backlight: add kernel-internal backlight API

[Mario Limonciello]

From: David Herrmann <dh.herrmann@gmail.com>

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 <mario.limonciello@amd.com>
 - rebase
 - Use guard(mutex)

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/video/backlight/backlight.c | 60 +++++++++++++++++++++++++++++
 include/linux/backlight.h           | 16 ++++++++
 2 files changed, 76 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index ab87a5e3dbf70..c3673bee6d9cf 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -513,6 +513,66 @@ static int devm_backlight_device_match(struct device *dev, void *res,
 	return *r == data;
 }
 
+/**
+ * backlight_device_lookup - find a backlight device
+ * @name: sysname of the backlight device
+ *
+ * @return Reference to the backlight device, NULL if not found.
+ *
+ * This searches through all registered backlight devices for a device with the
+ * given device name. In case none is found, NULL is returned, otherwise a
+ * new reference to the backlight device is returned. You must drop this
+ * reference via backlight_device_unref() once done.
+ * Note that the devices might get unregistered at any time. You need to lock
+ * around this lookup and inside of your backlight-notifier if you need to know
+ * when a device gets unregistered.
+ *
+ * This function can be safely called from IRQ context.
+ */
+struct backlight_device *backlight_device_lookup(const char *name)
+{
+	struct backlight_device *bd;
+	const char *t;
+
+	guard(mutex)(&backlight_dev_list_mutex);
+	list_for_each_entry(bd, &backlight_dev_list, entry) {
+		t = dev_name(&bd->dev);
+		if (t && !strcmp(t, name)) {
+			backlight_device_ref(bd);
+			return bd;
+		}
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(backlight_device_lookup);
+
+/**
+ * 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. The value is clamped according to device bounds.
+ * A uevent notification is sent with the reason set to @reason.
+ */
+void backlight_set_brightness(struct backlight_device *bd, unsigned int value,
+			      enum backlight_update_reason reason)
+{
+	guard(mutex)(&bd->ops_lock);
+	if (bd->ops) {
+		value = clamp(value, 0U,
+			      (unsigned int)bd->props.max_brightness);
+		dev_dbg(&bd->dev, "set brightness to %u\n", value);
+		bd->props.brightness = value;
+		backlight_update_status(bd);
+	}
+	backlight_generate_event(bd, reason);
+}
+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..7e4fee65fddd9 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -429,6 +429,22 @@ static inline void backlight_notify_blank_all(struct device *display_dev,
 { }
 #endif
 
+struct backlight_device *backlight_device_lookup(const char *name);
+void 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

[PATCH v3 2/8] backlight: expose the current brightness in the new kernel API

[Mario Limonciello]

From: Marta Lofstedt <marta.lofstedt@intel.com>

Current brightness needs to be queried from drivers.

Signed-off-by: Marta Lofstedt <marta.lofstedt@intel.com>
v3:
 * Rebase
 * Add description
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/video/backlight/backlight.c | 6 ++++++
 include/linux/backlight.h           | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index c3673bee6d9cf..9bbfc16cf2d74 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -206,6 +206,12 @@ int backlight_device_set_brightness(struct backlight_device *bd,
 }
 EXPORT_SYMBOL(backlight_device_set_brightness);
 
+int backlight_device_get_brightness(struct backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+EXPORT_SYMBOL(backlight_device_get_brightness);
+
 static ssize_t brightness_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 7e4fee65fddd9..851570b39d041 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -412,6 +412,7 @@ 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,
 				    unsigned long brightness);
+extern int backlight_device_get_brightness(struct backlight_device *bd);
 
 #if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
 void backlight_notify_blank(struct backlight_device *bd,
-- 
2.43.0

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

[Mario Limonciello]

This adds generic backlight-device support to DRM connectors. Drivers
that need to link backlight devices to DRM connectors can use the new
helpers to create a backlight property and link them at runtime.

Also add a brightness changed notification so that sysfs and DRM connector
can stay in sync and handle property updates around DPMS.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/Kconfig             |   1 +
 drivers/gpu/drm/Makefile            |   2 +
 drivers/gpu/drm/drm_backlight.c     | 406 ++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_connector.c     |  12 +
 drivers/gpu/drm/drm_drv.c           |   8 +
 drivers/gpu/drm/drm_mode_config.c   |   7 +
 drivers/gpu/drm/drm_mode_object.c   |  66 ++++-
 drivers/gpu/drm/drm_sysfs.c         |  54 ++++
 drivers/video/backlight/backlight.c |  17 ++
 include/drm/drm_backlight.h         |  45 +++
 include/drm/drm_connector.h         |   3 +
 include/drm/drm_mode_config.h       |   5 +
 include/linux/backlight.h           |  13 +
 13 files changed, 637 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_backlight.c
 create mode 100644 include/drm/drm_backlight.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 8f5a8d3012e41..8c00f534d50b8 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -17,6 +17,7 @@ menuconfig DRM
 # device and dmabuf fd. Let's make sure that is available for our userspace.
 	select KCMP
 	select VIDEO
+	select BACKLIGHT_CLASS_DEVICE
 	help
 	  Kernel-level support for the Direct Rendering Infrastructure (DRI)
 	  introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e97faabcd7830..9912306e94950 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -78,6 +78,8 @@ drm-$(CONFIG_DRM_CLIENT) += \
 	drm_client_event.o \
 	drm_client_modeset.o \
 	drm_client_sysrq.o
+drm-y += drm_backlight.o
+drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_PANEL) += drm_panel.o
 drm-$(CONFIG_OF) += drm_of.o
diff --git a/drivers/gpu/drm/drm_backlight.c b/drivers/gpu/drm/drm_backlight.c
new file mode 100644
index 0000000000000..cbc6a855ed332
--- /dev/null
+++ b/drivers/gpu/drm/drm_backlight.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: MIT
+/*
+ * DRM Backlight Helpers
+ * Copyright (c) 2014 David Herrmann
+ * Copyright (c) 2026 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/backlight.h>
+#include <linux/fs.h>
+#include <linux/list.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <drm/drm_backlight.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_device.h>
+#include <drm/drm_mode_config.h>
+
+/**
+ * DOC: Backlight Devices
+ *
+ * Backlight devices have always been managed as a separate subsystem,
+ * independent of DRM. They are usually controlled via separate hardware
+ * interfaces than the display controller, so the split works out fine.
+ * However, backlight brightness is a property of a display, and thus a
+ * property of a DRM connector. We already manage DPMS states via connector
+ * properties, so it is natural to keep brightness control at the same place.
+ *
+ * This DRM backlight interface implements generic backlight properties on
+ * connectors. It does not handle any hardware backends but simply forwards
+ * the requests to an available and linked backlight device. The links between
+ * connectors and backlight devices have to be established by DRM drivers and
+ * can be modified by user-space via sysfs (and udev rules). The name of the
+ * backlight device can be written to a sysfs attribute called 'backlight'.
+ * The device is looked up and linked to the connector (replacing a possible
+ * previous backlight device). A 'change' uevent is sent whenever a link is
+ * modified.
+ *
+ * Drivers have to call drm_backlight_alloc() after allocating a connector via
+ * drm_connector_init(). This will automatically add a backlight device to the
+ * given connector. No hardware device is linked to the connector by default.
+ * Drivers can set up a default device via drm_backlight_set_name(), but are
+ * free to leave it empty. User-space will then have to set up the link.
+ */
+
+struct drm_backlight {
+	struct list_head list;
+	struct drm_connector *connector;
+	char *link_name;
+	struct backlight_device *link;
+	struct work_struct work;
+	unsigned int set_value;
+	bool changed : 1;
+};
+
+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_schedule(struct drm_backlight *b)
+{
+	lockdep_assert_held(&drm_backlight_lock);
+	if (__drm_backlight_is_registered(b))
+		schedule_work(&b->work);
+}
+
+static void __drm_backlight_worker(struct work_struct *w)
+{
+	struct drm_backlight *b = container_of(w, struct drm_backlight, work);
+	static char *ep[] = { "BACKLIGHT=1", NULL };
+	struct backlight_device *bd;
+	bool send_uevent;
+	unsigned int v;
+
+	scoped_guard(spinlock, &drm_backlight_lock) {
+		send_uevent = b->changed;
+		b->changed = false;
+		v = b->set_value;
+		bd = b->link;
+		backlight_device_ref(bd);
+	}
+
+	if (bd) {
+		backlight_set_brightness(bd, v, BACKLIGHT_UPDATE_DRM);
+		backlight_device_unref(bd);
+	}
+
+	if (send_uevent)
+		kobject_uevent_env(&b->connector->kdev->kobj, KOBJ_CHANGE, ep);
+}
+
+/* caller must hold @drm_backlight_lock */
+static void __drm_backlight_prop_changed(struct drm_backlight *b, uint64_t v)
+{
+	uint64_t max;
+
+	lockdep_assert_held(&drm_backlight_lock);
+
+	if (!b || !b->link)
+		return;
+
+	max = b->link->props.max_brightness;
+	if (v >= U16_MAX)
+		b->set_value = max;
+	else
+		b->set_value = ((uint64_t)v * max) / U16_MAX;
+	__drm_backlight_schedule(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);
+
+	if (!b->link)
+		return;
+
+	set = v;
+	max = b->link->props.max_brightness;
+	if (max < 1)
+		return;
+
+	if (set >= max)
+		set = U16_MAX;
+	else
+		set = ((uint64_t)set * U16_MAX) / 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;
+}
+
+/* caller must hold @drm_backlight_lock */
+static void __drm_backlight_link(struct drm_backlight *b,
+				 struct backlight_device *bd)
+{
+	if (bd == b->link)
+		return;
+
+	backlight_device_unref(b->link);
+	b->link = bd;
+	backlight_device_ref(b->link);
+	if (bd)
+		__drm_backlight_real_changed(b, bd->props.brightness);
+	b->changed = true;
+	__drm_backlight_schedule(b);
+}
+
+/* caller must hold @drm_backlight_lock */
+static void __drm_backlight_lookup(struct drm_backlight *b)
+{
+	struct backlight_device *bd;
+
+	if (b->link_name)
+		bd = backlight_device_lookup(b->link_name);
+	else
+		bd = NULL;
+
+	__drm_backlight_link(b, bd);
+	backlight_device_unref(bd);
+}
+
+/**
+ * drm_backlight_alloc - add backlight capability to a connector
+ * @connector: connector to add backlight to
+ *
+ * This allocates a new DRM-backlight device and links 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.
+ *
+ * The connector will not have any hardware backlight linked by default. You
+ * need to call drm_backlight_set_name() if you want to set a default
+ * backlight. User-space can overwrite those via sysfs.
+ *
+ * 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);
+	INIT_WORK(&b->work, __drm_backlight_worker);
+	b->connector = connector;
+	connector->backlight = b;
+
+	drm_object_attach_property(&connector->base,
+				   config->luminance_property, U16_MAX);
+
+	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->link_name);
+	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);
+	__drm_backlight_lookup(b);
+}
+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);
+	}
+
+	cancel_work_sync(&b->work);
+}
+EXPORT_SYMBOL(drm_backlight_unregister);
+
+/**
+ * drm_backlight_get_name - retrieve name of linked backlight device
+ * @b: DRM backlight to retrieve name of
+ * @buf: target buffer for name
+ * @max: size of the target buffer
+ *
+ * This retrieves the name of the backlight device linked to @b and writes it
+ * into @buf. If @buf is NULL or @max is 0, no name will be retrieved, but this
+ * function only tests whether a link is set.
+ * Otherwise, the name will always be written into @buf and will always be
+ * zero-terminated (truncated if too long).
+ *
+ * If no backlight device is linked to @b, this returns -ENOENT. Otherwise, the
+ * length of the written name (excluding the terminating 0 character) is
+ * returned.
+ * Note that if a device name has been set but the underlying backlight device
+ * does not exist, this will still return the linked name. -ENOENT is only
+ * returned if no device name has been set, yet (or has been cleared).
+ *
+ * Returns: On success the length of the written name, on failure a negative
+ *          error code.
+ */
+int drm_backlight_get_name(struct drm_backlight *b, char *buf, size_t max)
+{
+	int r;
+
+	guard(spinlock)(&drm_backlight_lock);
+
+	if (!b || !b->link_name)
+		return -ENOENT;
+
+	if (!buf || !max)
+		return -EINVAL;
+
+	r = strlen(b->link_name);
+
+	if (r + 1 > max)
+		r = max - 1;
+	buf[r] = 0;
+	memcpy(buf, b->link_name, r);
+
+	return r;
+}
+EXPORT_SYMBOL(drm_backlight_get_name);
+
+/**
+ * drm_backlight_set_name - Change the device link of a DRM backlight
+ * @b: DRM backlight to modify
+ * @name: name of backlight device
+ *
+ * This changes the backlight device-link on @b to the hardware device with
+ * name @name. @name is stored on the backlight device, even if no such
+ * hardware device is registered, yet. If a backlight device appears later on,
+ * it will be automatically linked to all matching DRM backlight devices. If a
+ * real hardware backlight device already exists with such a name, it is linked
+ * with immediate effect.
+ *
+ * Whenever a real hardware backlight is linked or unlinked from a DRM connector
+ * an uevent with "BACKLIGHT=1" is generated on the connector.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int drm_backlight_set_name(struct drm_backlight *b, const char *name)
+{
+	char *namecopy;
+
+	if (name && *name) {
+		namecopy = kstrdup(name, GFP_KERNEL);
+		if (!namecopy)
+			return -ENOMEM;
+	} else {
+		namecopy = NULL;
+	}
+
+	guard(spinlock)(&drm_backlight_lock);
+
+	kfree(b->link_name);
+	b->link_name = namecopy;
+	if (__drm_backlight_is_registered(b))
+		__drm_backlight_lookup(b);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_backlight_set_name);
+
+void drm_backlight_set_luminance(struct drm_backlight *b, uint64_t value)
+{
+	guard(spinlock)(&drm_backlight_lock);
+	__drm_backlight_prop_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;
+	const char *name;
+
+	guard(spinlock)(&drm_backlight_lock);
+
+	switch (event) {
+	case BACKLIGHT_REGISTERED:
+		name = dev_name(&bd->dev);
+		if (!name)
+			break;
+
+		list_for_each_entry(b, &drm_backlight_list, list)
+			if (!b->link && b->link_name &&
+			    !strcmp(name, b->link_name))
+				__drm_backlight_link(b, bd);
+
+		break;
+	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 47dc53c4a738f..d76878548728a 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;
@@ -3257,10 +3263,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->brightness_property) {
+		if (connector->backlight && connector->dpms == DRM_MODE_DPMS_ON)
+			drm_backlight_set_brightness(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 985c283cf59fa..4fd8c2590d154 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>
@@ -1228,6 +1229,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();
@@ -1251,6 +1253,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..585d80e3c1330 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, U16_MAX);
+	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 2d943a610b88b..e6f0027d70cb5 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..eb7d54d991272 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 
 #include <drm/drm_accel.h>
+#include <drm/drm_backlight.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_device.h>
 #include <drm/drm_file.h>
@@ -303,11 +304,63 @@ static ssize_t connector_id_show(struct device *device,
 	return sysfs_emit(buf, "%d\n", connector->base.id);
 }
 
+static ssize_t backlight_show(struct device *device,
+			      struct device_attribute *attr,
+			      char *buf)
+{
+	struct drm_connector *connector = to_drm_connector(device);
+	int r;
+
+	if (!connector->backlight)
+		return -EOPNOTSUPP;
+
+	r = drm_backlight_get_name(connector->backlight, buf, PAGE_SIZE);
+	if (r < 0)
+		return r;
+
+	if (r + 1 < PAGE_SIZE) {
+		buf[r++] = '\n';
+		buf[r] = 0;
+	}
+
+	return r;
+}
+
+static ssize_t backlight_store(struct device *device,
+			       struct device_attribute *attr,
+			       const char *buf, size_t size)
+{
+	struct drm_connector *connector = to_drm_connector(device);
+	const char *t;
+	char *name;
+	size_t len;
+	int r;
+
+	if (!connector->backlight)
+		return -EOPNOTSUPP;
+
+	t = strchrnul(buf, '\n');
+	len = t - buf;
+
+	name = kstrndup(buf, len, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
+	r = drm_backlight_set_name(connector->backlight, name);
+	kfree(name);
+
+	if (r < 0)
+		return r;
+
+	return size;
+}
+
 static DEVICE_ATTR_RW(status);
 static DEVICE_ATTR_RO(enabled);
 static DEVICE_ATTR_RO(dpms);
 static DEVICE_ATTR_RO(modes);
 static DEVICE_ATTR_RO(connector_id);
+static DEVICE_ATTR_RW(backlight);
 
 static struct attribute *connector_dev_attrs[] = {
 	&dev_attr_status.attr,
@@ -315,6 +368,7 @@ static struct attribute *connector_dev_attrs[] = {
 	&dev_attr_dpms.attr,
 	&dev_attr_modes.attr,
 	&dev_attr_connector_id.attr,
+	&dev_attr_backlight.attr,
 	NULL
 };
 
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 9bbfc16cf2d74..1557dfb3a3bf3 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -125,6 +125,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;
@@ -613,6 +616,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..42f00a930584b
--- /dev/null
+++ b/include/drm/drm_backlight.h
@@ -0,0 +1,45 @@
+/* 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 drm_backlight;
+struct drm_connector;
+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);
+
+int drm_backlight_get_name(struct drm_backlight *b, char *buf, size_t max);
+int drm_backlight_set_name(struct drm_backlight *b, const char *name);
+void drm_backlight_set_luminance(struct drm_backlight *b, uint64_t value);
+#endif /* __DRM_BACKLIGHT_H__ */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index f83f28cae2075..10daf088b8f1a 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 687c0ee163d25..b340dc93f8148 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 851570b39d041..f7f1e18909460 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

[PATCH v3 4/8] DRM: Add support for client and driver indicating support for luminance

[Mario Limonciello]

The legacy backlight control interface can only be disabled when both
the client and driver have agreed that the luminance can be set during
a modeset. Add capability for the client to register and for the driver
to indicate support.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/drm_connector.c   |  4 ++--
 drivers/gpu/drm/drm_ioctl.c       | 10 ++++++++++
 include/drm/drm_connector.h       |  5 +++++
 include/drm/drm_drv.h             |  7 +++++++
 include/drm/drm_file.h            |  8 ++++++++
 include/uapi/drm/drm.h            | 10 ++++++++++
 7 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 5bd5bf6661df7..de218206cef7e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -30,6 +30,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_framebuffer.h>
 #include <drm/drm_print.h>
 #include <drm/drm_drv.h>
@@ -935,6 +936,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 +1029,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);
@@ -1126,8 +1137,21 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
 	if (connector->dpms == mode)
 		goto out;
 
+	/* Handle backlight brightness coordination with DPMS state changes */
+	if (old_mode != DRM_MODE_DPMS_OFF && mode == DRM_MODE_DPMS_OFF) {
+		/* DPMS ON -> OFF: dim backlight to 0 to save power */
+		drm_backlight_set_luminance(connector->backlight, 0);
+	}
+
 	connector->dpms = mode;
 
+	/* DPMS OFF -> ON: restore brightness to property value */
+	if (old_mode == DRM_MODE_DPMS_OFF && mode == DRM_MODE_DPMS_ON &&
+	    connector->state) {
+		drm_backlight_set_luminance(connector->backlight,
+					    connector->state->luminance);
+	}
+
 	crtc = connector->state->crtc;
 	if (!crtc)
 		goto out;
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index d76878548728a..5de33fc259b26 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -3268,9 +3268,9 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
 	/* Do DPMS ourselves */
 	if (property == connector->dev->mode_config.dpms_property) {
 		ret = (*connector->funcs->dpms)(connector, (int)value);
-	} else if (property == config->brightness_property) {
+	} else if (property == config->luminance_property) {
 		if (connector->backlight && connector->dpms == DRM_MODE_DPMS_ON)
-			drm_backlight_set_brightness(connector->backlight,
+			drm_backlight_set_luminance(connector->backlight,
 						     value);
 		ret = 0;
 	} else if (connector->funcs->set_property)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ff193155129e7..b4435c2bd6091 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -28,6 +28,7 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 
+#include "drm/drm.h"
 #include <linux/export.h>
 #include <linux/nospec.h>
 #include <linux/pci.h>
@@ -380,6 +381,15 @@ 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 (!drm_core_check_feature(dev, DRIVER_CONNECTOR_LUMINANCE))
+			return -EOPNOTSUPP;
+		if (!file_priv->atomic)
+			return -EINVAL;
+		if (req->value > 1)
+			return -EINVAL;
+		file_priv->supports_luminance_control = req->value;
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 10daf088b8f1a..762a9e2ef6e30 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
+	 */
+	u16 luminance;
 };
 
 struct drm_connector_hdmi_audio_funcs {
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 42fc085f986de..a6b668cb68c5e 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -123,6 +123,13 @@ enum drm_driver_feature {
 	 */
 	DRIVER_CURSOR_HOTSPOT           = BIT(9),
 
+	/**
+	 * @DRIVER_CONNECTOR_LUMINANCE:
+	 *
+	 * Driver supports luminance control on a per connector basis.
+	 */
+	DRIVER_CONNECTOR_LUMINANCE           = BIT(10),
+
 	/* IMPORTANT: Below are all the legacy flags, add new ones above. */
 
 	/**
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/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 27cc159c1d275..b5e6d940f2816 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -921,6 +921,16 @@ struct drm_get_cap {
  */
 #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE	7
 
+/**
+ * DRM_CLIENT_CAP_LUMINANCE
+ *
+ * If set to 1, legacy sysfs interface for controlling backlight brightness will
+ * be disabled.  The client will include luminance values as part of the modeset.
+
+ * 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

[PATCH v3 5/8] drm/amd/display: Pass up errors reading actual brightness

[Mario Limonciello]

[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 <mario.limonciello@amd.com>
---
 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 1d9ceb432ec37..5b8a0dd558e76 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5302,7 +5302,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;
@@ -5316,14 +5316,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);
 }
@@ -5331,7 +5331,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])
@@ -5339,7 +5339,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

[PATCH v3 6/8] drm/amd: Indicate driver supports luminance

[Mario Limonciello]

This will allow the connector to show luminance information for
eDP panels.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index e47921e2a9af2..fa38d9f02edf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -3072,6 +3072,7 @@ static const struct drm_driver amdgpu_kms_driver = {
 	    DRIVER_ATOMIC |
 	    DRIVER_GEM |
 	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ |
+	    DRIVER_CONNECTOR_LUMINANCE |
 	    DRIVER_SYNCOBJ_TIMELINE,
 	.open = amdgpu_driver_open_kms,
 	.postclose = amdgpu_driver_postclose_kms,
-- 
2.43.0

[PATCH v3 7/8] drm/amd/display: Allow backlight registration to fail

[Mario Limonciello]

[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 <mario.limonciello@amd.com>
---
 .../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 5b8a0dd558e76..6b29e5b6b41e7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5353,7 +5353,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;
@@ -5364,15 +5364,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];
@@ -5406,22 +5407,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,
@@ -7953,7 +7958,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

[PATCH v3 8/8] drm/amd/display: use drm backlight

[Mario Limonciello]

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 <mario.limonciello@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

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 6b29e5b6b41e7..52854abc6bb60 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>
@@ -5424,9 +5425,32 @@ 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;
 	}
-	drm_dbg_driver(drm, "DM: Registered Backlight device: %s\n", bl_name);
+
+	/* Only set backlight name if DRM backlight was allocated successfully.
+	 * drm_backlight_alloc() may have failed earlier in init_helper, leaving
+	 * aconnector->base.backlight as NULL.
+	 */
+	if (aconnector->base.backlight) {
+		r = drm_backlight_set_name(aconnector->base.backlight, bl_name);
+		if (r)
+			goto error_cleanup_backlight;
+
+		drm_dbg_driver(drm, "DM: Registered Backlight device: %s\n", bl_name);
+	} else {
+		drm_warn(drm, "DM: Backlight device registration skipped - DRM backlight not allocated\n");
+	}
 
 	return 0;
+
+error_cleanup_backlight:
+	/* Clean up hardware backlight device if DRM backlight name setting failed */
+	backlight_device_unregister(dm->backlight_dev[aconnector->bl_idx]);
+	dm->backlight_dev[aconnector->bl_idx] = NULL;
+
+	/* Also clean up DRM backlight structure to avoid resource leak */
+	drm_backlight_free(&aconnector->base);
+
+	return r;
 }
 
 static int initialize_plane(struct amdgpu_display_manager *dm,
@@ -7880,6 +7904,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);
@@ -9129,6 +9157,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

Re: [PATCH v3 4/8] DRM: Add support for client and driver indicating support for luminance

[Dmitry Baryshkov]

On Fri, Apr 24, 2026 at 05:09:49PM -0500, Mario Limonciello wrote:
> The legacy backlight control interface can only be disabled when both
> the client and driver have agreed that the luminance can be set during
> a modeset. Add capability for the client to register and for the driver
> to indicate support.

Please describe, why do we need DRM_CLIENT_CAP_LUMINANCE.

> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 24 ++++++++++++++++++++++++
>  drivers/gpu/drm/drm_connector.c   |  4 ++--
>  drivers/gpu/drm/drm_ioctl.c       | 10 ++++++++++
>  include/drm/drm_connector.h       |  5 +++++
>  include/drm/drm_drv.h             |  7 +++++++
>  include/drm/drm_file.h            |  8 ++++++++
>  include/uapi/drm/drm.h            | 10 ++++++++++
>  7 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 27cc159c1d275..b5e6d940f2816 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -921,6 +921,16 @@ struct drm_get_cap {
>   */
>  #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE	7
>  
> +/**
> + * DRM_CLIENT_CAP_LUMINANCE
> + *
> + * If set to 1, legacy sysfs interface for controlling backlight brightness will
> + * be disabled.  The client will include luminance values as part of the modeset.

I don't think this is accurate (anymore?). The sysfs doesn't seem to be
disabled. Rather the DRM updates it internally. Which might mean that we
don't need an extra client cap.

> +
> + * 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
> 

-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

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

I'd like to point out another kind of devices which might have been left
w/o notice here: the panels. In a plenty of cases the driver creating
the drm_connector (frequently through drm_bridge_connector_init())
doesn't even know if there is a panel on the other side of the chain or
not. Likewise connector creation doesn't involve any panel-related
operation. Some panels provide backlight controls. Others don't and rely
on the external backlight controls (see drm_panel_of_backlight() for
example).

Can we work towards simplifying possible support for DRM backlights to
the setups with drm_panel in place?

> + * given connector. No hardware device is linked to the connector by default.
> + * Drivers can set up a default device via drm_backlight_set_name(), but are
> + * free to leave it empty. User-space will then have to set up the link.

This sounds like a definite no-go. Think about any phone or any laptop.
Do we need to involve userspace to let the kernel know that there is a
connection between the DRM connector and the backlight present in the
system?

What is the usescase for having dynamic connector<->link connections?

> + */
> +

-- 
With best wishes
Dmitry

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

[Mario Limonciello]

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

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

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

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

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

Re: [PATCH v3 4/8] DRM: Add support for client and driver indicating support for luminance

[Mario Limonciello]

On 4/25/26 07:17, Dmitry Baryshkov wrote:
> On Fri, Apr 24, 2026 at 05:09:49PM -0500, Mario Limonciello wrote:
>> The legacy backlight control interface can only be disabled when both
>> the client and driver have agreed that the luminance can be set during
>> a modeset. Add capability for the client to register and for the driver
>> to indicate support.
> 
> Please describe, why do we need DRM_CLIENT_CAP_LUMINANCE.
> 
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/gpu/drm/drm_atomic_uapi.c | 24 ++++++++++++++++++++++++
>>   drivers/gpu/drm/drm_connector.c   |  4 ++--
>>   drivers/gpu/drm/drm_ioctl.c       | 10 ++++++++++
>>   include/drm/drm_connector.h       |  5 +++++
>>   include/drm/drm_drv.h             |  7 +++++++
>>   include/drm/drm_file.h            |  8 ++++++++
>>   include/uapi/drm/drm.h            | 10 ++++++++++
>>   7 files changed, 66 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 27cc159c1d275..b5e6d940f2816 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -921,6 +921,16 @@ struct drm_get_cap {
>>    */
>>   #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE	7
>>   
>> +/**
>> + * DRM_CLIENT_CAP_LUMINANCE
>> + *
>> + * If set to 1, legacy sysfs interface for controlling backlight brightness will
>> + * be disabled.  The client will include luminance values as part of the modeset.
> 
> I don't think this is accurate (anymore?). The sysfs doesn't seem to be
> disabled. Rather the DRM updates it internally. Which might mean that we
> don't need an extra client cap.

I had code in my tree for doing it and decided to defer that (see cover 
letter) but forgot to drop the cap.

I suppose this is an area that would be good to discuss if it's worth 
getting some of it done in the same series or should be deferred.

The overally idea I have is:

1) Allow both sysfs and DRM master to do writes initially (allow 
transitioning and PoC)
2) Userspace changes to DRM master
3) Introduce client cap for it and block off sysfs writes when DRM 
master indicates it.
4) Move sysfs compat behind a kconfig
5) Drop Kconfig

The timescale between steps 3 and 5 is "many kernel cycles".

> 
>> +
>> + * 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
>>
> 

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

[Dmitry Baryshkov]

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

No, I wrote in the other email, why. 

> 
> > 
> > > + * given connector. No hardware device is linked to the connector by default.
> > > + * Drivers can set up a default device via drm_backlight_set_name(), but are
> > > + * free to leave it empty. User-space will then have to set up the link.
> > 
> > This sounds like a definite no-go. Think about any phone or any laptop.
> > Do we need to involve userspace to let the kernel know that there is a
> > connection between the DRM connector and the backlight present in the
> > system?
> > 
> > What is the usescase for having dynamic connector<->link connections?
> > 
> 
> Allowing userspace to setup the link was something in the original series by
> David and Marta that I left in place.  I don't have a problem with dropping
> that concept and requiring the driver to do the linking.

Well, I don't want to loose something actually useful. That's why I'm
trying to understand the usecase.

-- 
With best wishes
Dmitry

Re: [PATCH v3 4/8] DRM: Add support for client and driver indicating support for luminance

[Dmitry Baryshkov]

On Sat, Apr 25, 2026 at 09:45:30AM -0500, Mario Limonciello wrote:
> 
> 
> On 4/25/26 07:17, Dmitry Baryshkov wrote:
> > On Fri, Apr 24, 2026 at 05:09:49PM -0500, Mario Limonciello wrote:
> > > The legacy backlight control interface can only be disabled when both
> > > the client and driver have agreed that the luminance can be set during
> > > a modeset. Add capability for the client to register and for the driver
> > > to indicate support.
> > 
> > Please describe, why do we need DRM_CLIENT_CAP_LUMINANCE.
> > 
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > >   drivers/gpu/drm/drm_atomic_uapi.c | 24 ++++++++++++++++++++++++
> > >   drivers/gpu/drm/drm_connector.c   |  4 ++--
> > >   drivers/gpu/drm/drm_ioctl.c       | 10 ++++++++++
> > >   include/drm/drm_connector.h       |  5 +++++
> > >   include/drm/drm_drv.h             |  7 +++++++
> > >   include/drm/drm_file.h            |  8 ++++++++
> > >   include/uapi/drm/drm.h            | 10 ++++++++++
> > >   7 files changed, 66 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > > index 27cc159c1d275..b5e6d940f2816 100644
> > > --- a/include/uapi/drm/drm.h
> > > +++ b/include/uapi/drm/drm.h
> > > @@ -921,6 +921,16 @@ struct drm_get_cap {
> > >    */
> > >   #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE	7
> > > +/**
> > > + * DRM_CLIENT_CAP_LUMINANCE
> > > + *
> > > + * If set to 1, legacy sysfs interface for controlling backlight brightness will
> > > + * be disabled.  The client will include luminance values as part of the modeset.
> > 
> > I don't think this is accurate (anymore?). The sysfs doesn't seem to be
> > disabled. Rather the DRM updates it internally. Which might mean that we
> > don't need an extra client cap.
> 
> I had code in my tree for doing it and decided to defer that (see cover
> letter) but forgot to drop the cap.
> 
> I suppose this is an area that would be good to discuss if it's worth
> getting some of it done in the same series or should be deferred.
> 
> The overally idea I have is:
> 
> 1) Allow both sysfs and DRM master to do writes initially (allow
> transitioning and PoC)
> 2) Userspace changes to DRM master
> 3) Introduce client cap for it and block off sysfs writes when DRM master
> indicates it.
> 4) Move sysfs compat behind a kconfig
> 5) Drop Kconfig
> 
> The timescale between steps 3 and 5 is "many kernel cycles".

I like the idea of controlling the backlight through the connector API,
but frankly speaking, I think that the implementation is somewhat
strange. After checking the AMD driver, I'd really like to propose the
following plan (again, unless I miss something, please feel free to
point me to discussions or other patches describing the reasons):

1. Establish a way to link connectors to the backlight devices. For the
   userspace this might be visible as a symlink in sysfs, for the kernel
   drivers it should be drm_connector::backlight link to struct
   backlight_device.

   Three major cases. Maybe four.
   - Native backlight in the "big" drivers: amdgpu, i915 nouveau,
     radeon, gma500, gud, solomon.

   - ACPI backlight (here the kernel would need to find the mapping. See
     ACPI DSDT standard and USB-C mapping code for the inspiration)

   - backlight supported by the drm_panel

   - backlight provided by the separate device and identified via the
     link in OF.

   present around the time the drm_connector is created, maybe right
   after the connector is created (AMD does it in late_register, which
   sounds matching this description).

   I can help you with the "embedded" systems. Keeping the word in
   quotes as, for example, I'm currently writing from the Snapdragon
   laptop (X13s), which uses drm_panel and all the stuff.

2. Find a good way to turn the backlight on and off. I see that you have
   hooked it into the drm_atomic_connector_commit_dpms(), but the
   problem is that it is only called when userspace toggles the DPMS
   property. There are other ways to turn the output off and on.

   I _think_ the correct place would be
   drm_atomic_helper_commit_encoder_bridge_enable(). and corresponding
   dsiable() function. This is the place where the output chain gets
   enabled or disabled, no matter why is it happening.

3. Implement the client property. I don't think there is a need for the
   client capability or anything comples. The major design decision here
   should be if the property is always from 0 to 65535 or whether it
   takes backlight's max_brightness into the account and registers only
   relevant values. I don't have a strong opinion here.

   There is no need to store the brightness level, calls to
   backlight_disable() / backlight_enable() (used in the previous
   step) should do the trick.


-- 
With best wishes
Dmitry

Re: [PATCH v3 0/8] Add support for a DRM backlight capability

[Louis Chauvet]

On 4/25/26 00:09, Mario Limonciello wrote:
> From: Mario Limonciello (AMD) <superm1@kernel.org>
> 
> At Display Next Hackfest 2025 we discussed the renewed need for moving
> brightness control into the DRM connector properties.  I've taken the
> previous efforts from David and Marta, rebased and adjusted for the
> current kernel.
> 
> The legacy sysfs interface is synchronized with the DRM connector (although
> the scale may be different as DRM connector property is u16).
> 
> Later after this has been adopted by enough userspace, it may make sense to
> configure the legacy sysfs interface to be configurable so that only
> DRM master controls backlight.
> 
> I've done a first implementation with amdgpu with eDP connectors; but
> conceivably this can be extended to other connectors like DP for displays
> that can be controlled via DDC as well later.
> 
> I have also used DRM review prompts to review this series and fix some bugs
> which were caught with two different Claude models.  The fixes are squashed
> into the patches.
> 
> Assisted-by: Claude Opus
> Assisted-by: Claude Sonnet
> 
> For ease of testing; this series is also available on this branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git/log/?h=superm1/backlight-property-v3

Hello,

thanks for this work, I am very interested in this progress so I can 
help you to test / implement more features.

I think you forgot to include the revert of "backlight: Remove notifier" 
in your series, it can't be applied without it.

I will take a look and see if I can create a VKMS implementation of your 
work.

Thanks for this work,

> David Rheinsberg (1):
>    backlight: add kernel-internal backlight API
> 
> Mario Limonciello (6):
>    drm: link connectors to backlight devices
>    DRM: Add support for client and driver indicating support for
>      luminance
>    drm/amd/display: Pass up errors reading actual brightness
>    drm/amd: Indicate driver supports luminance
>    drm/amd/display: Allow backlight registration to fail
>    drm/amd/display: use drm backlight
> 
> Marta Lofstedt (1):
>    backlight: expose the current brightness in the new kernel API
> 
>   drivers/gpu/drm/Kconfig                       |   1 +
>   drivers/gpu/drm/Makefile                      |   2 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |   1 +
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  88 +++-
>   drivers/gpu/drm/drm_atomic_uapi.c             |  24 ++
>   drivers/gpu/drm/drm_backlight.c               | 406 ++++++++++++++++++
>   drivers/gpu/drm/drm_connector.c               |  12 +
>   drivers/gpu/drm/drm_drv.c                     |   8 +
>   drivers/gpu/drm/drm_ioctl.c                   |  10 +
>   drivers/gpu/drm/drm_mode_config.c             |   7 +
>   drivers/gpu/drm/drm_mode_object.c             |  66 ++-
>   drivers/gpu/drm/drm_sysfs.c                   |  54 +++
>   drivers/video/backlight/backlight.c           |  83 ++++
>   include/drm/drm_backlight.h                   |  45 ++
>   include/drm/drm_connector.h                   |   8 +
>   include/drm/drm_drv.h                         |   7 +
>   include/drm/drm_file.h                        |   8 +
>   include/drm/drm_mode_config.h                 |   5 +
>   include/linux/backlight.h                     |  30 ++
>   include/uapi/drm/drm.h                        |  10 +
>   20 files changed, 852 insertions(+), 23 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_backlight.c
>   create mode 100644 include/drm/drm_backlight.h
> 

Re: [PATCH v3 1/8] backlight: add kernel-internal backlight API

[Louis Chauvet]

On 4/25/26 00:09, Mario Limonciello wrote:
> From: David Herrmann <dh.herrmann@gmail.com>
> 
> 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 <mario.limonciello@amd.com>
>   - rebase
>   - Use guard(mutex)
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/video/backlight/backlight.c | 60 +++++++++++++++++++++++++++++
>   include/linux/backlight.h           | 16 ++++++++
>   2 files changed, 76 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index ab87a5e3dbf70..c3673bee6d9cf 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -513,6 +513,66 @@ static int devm_backlight_device_match(struct device *dev, void *res,
>   	return *r == data;
>   }
>   
> +/**
> + * backlight_device_lookup - find a backlight device
> + * @name: sysname of the backlight device
> + *
> + * @return Reference to the backlight device, NULL if not found.
> + *
> + * This searches through all registered backlight devices for a device with the
> + * given device name. In case none is found, NULL is returned, otherwise a
> + * new reference to the backlight device is returned. You must drop this
> + * reference via backlight_device_unref() once done.
> + * Note that the devices might get unregistered at any time. You need to lock
> + * around this lookup and inside of your backlight-notifier if you need to know
> + * when a device gets unregistered.
> + *
> + * This function can be safely called from IRQ context.
> + */
> +struct backlight_device *backlight_device_lookup(const char *name)
> +{
> +	struct backlight_device *bd;
> +	const char *t;
> +
> +	guard(mutex)(&backlight_dev_list_mutex);
> +	list_for_each_entry(bd, &backlight_dev_list, entry) {
> +		t = dev_name(&bd->dev);
> +		if (t && !strcmp(t, name)) {
> +			backlight_device_ref(bd);
> +			return bd;
> +		}
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(backlight_device_lookup);
> 

Hello,

I think this function can be repalced with backlight_device_get_by_name.

> +/**
> + * 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. The value is clamped according to device bounds.
> + * A uevent notification is sent with the reason set to @reason.
> + */
> +void backlight_set_brightness(struct backlight_device *bd, unsigned int value,
> +			      enum backlight_update_reason reason)
> +{
> +	guard(mutex)(&bd->ops_lock);
> +	if (bd->ops) {
> +		value = clamp(value, 0U,
> +			      (unsigned int)bd->props.max_brightness);

Why did you use a clamping here? I think it is better to return error 
instead.

> +		dev_dbg(&bd->dev, "set brightness to %u\n", value);
> +		bd->props.brightness = value;
> +		backlight_update_status(bd);
> +	}
> +	backlight_generate_event(bd, reason);
> +}
> +EXPORT_SYMBOL_GPL(backlight_set_brightness);
> 

I think this could be nice to update backlight_device_set_brightness to 
avoid code duplication:

int backlight_device_set_brightness(...) {
	return backlight_set_brightness(..., BACKLIGHT_UPDATE_SYSFS);
}

>   /**
>    * 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..7e4fee65fddd9 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -429,6 +429,22 @@ static inline void backlight_notify_blank_all(struct device *display_dev,
>   { }
>   #endif
>   
> +struct backlight_device *backlight_device_lookup(const char *name);
> +void 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);
> +}
> +
Most of the kernel use _put and _get functions, I think it could be nice 
to keep the same naming.

Thanks,
Louis Chauvet

>   #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
>   
>   /**

Re: [PATCH v3 2/8] backlight: expose the current brightness in the new kernel API

[Louis Chauvet]

On 4/25/26 00:09, Mario Limonciello wrote:
> From: Marta Lofstedt <marta.lofstedt@intel.com>
> 
> Current brightness needs to be queried from drivers.
> 
> Signed-off-by: Marta Lofstedt <marta.lofstedt@intel.com>
> v3:
>   * Rebase
>   * Add description
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/video/backlight/backlight.c | 6 ++++++
>   include/linux/backlight.h           | 1 +
>   2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index c3673bee6d9cf..9bbfc16cf2d74 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -206,6 +206,12 @@ int backlight_device_set_brightness(struct backlight_device *bd,
>   }
>   EXPORT_SYMBOL(backlight_device_set_brightness);
>   
> +int backlight_device_get_brightness(struct backlight_device *bd)
> +{
> +	return bd->props.brightness;

Hello,

The function backlight_get_brightness already exists and is already 
public, maybe you can use the existing function?

Thanks,
Louis Chauvet

> +}
> +EXPORT_SYMBOL(backlight_device_get_brightness);
> +
>   static ssize_t brightness_store(struct device *dev,
>   		struct device_attribute *attr, const char *buf, size_t count)
>   {
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 7e4fee65fddd9..851570b39d041 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -412,6 +412,7 @@ 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,
>   				    unsigned long brightness);
> +extern int backlight_device_get_brightness(struct backlight_device *bd);
>   
>   #if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>   void backlight_notify_blank(struct backlight_device *bd,

Re: [PATCH v3 2/8] backlight: expose the current brightness in the new kernel API

[Mario Limonciello]

On 5/4/26 08:55, Louis Chauvet wrote:
> [You don't often get email from louis.chauvet@bootlin.com. Learn why 
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 4/25/26 00:09, Mario Limonciello wrote:
>> From: Marta Lofstedt <marta.lofstedt@intel.com>
>>
>> Current brightness needs to be queried from drivers.
>>
>> Signed-off-by: Marta Lofstedt <marta.lofstedt@intel.com>
>> v3:
>>   * Rebase
>>   * Add description
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/video/backlight/backlight.c | 6 ++++++
>>   include/linux/backlight.h           | 1 +
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/ 
>> backlight/backlight.c
>> index c3673bee6d9cf..9bbfc16cf2d74 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -206,6 +206,12 @@ int backlight_device_set_brightness(struct 
>> backlight_device *bd,
>>   }
>>   EXPORT_SYMBOL(backlight_device_set_brightness);
>>
>> +int backlight_device_get_brightness(struct backlight_device *bd)
>> +{
>> +     return bd->props.brightness;
> 
> Hello,
> 
> The function backlight_get_brightness already exists and is already
> public, maybe you can use the existing function?
> 

Thanks for the suggestion.  I'll have a try with this.

> 
>> +}
>> +EXPORT_SYMBOL(backlight_device_get_brightness);
>> +
>>   static ssize_t brightness_store(struct device *dev,
>>               struct device_attribute *attr, const char *buf, size_t 
>> count)
>>   {
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index 7e4fee65fddd9..851570b39d041 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -412,6 +412,7 @@ 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,
>>                                   unsigned long brightness);
>> +extern int backlight_device_get_brightness(struct backlight_device *bd);
>>
>>   #if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>   void backlight_notify_blank(struct backlight_device *bd,
> 

Re: [PATCH v3 1/8] backlight: add kernel-internal backlight API

[Mario Limonciello]

On 5/4/26 08:55, Louis Chauvet wrote:
> [You don't often get email from louis.chauvet@bootlin.com. Learn why 
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 4/25/26 00:09, Mario Limonciello wrote:
>> From: David Herrmann <dh.herrmann@gmail.com>
>>
>> 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 <mario.limonciello@amd.com>
>>   - rebase
>>   - Use guard(mutex)
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/video/backlight/backlight.c | 60 +++++++++++++++++++++++++++++
>>   include/linux/backlight.h           | 16 ++++++++
>>   2 files changed, 76 insertions(+)
>>
>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/ 
>> backlight/backlight.c
>> index ab87a5e3dbf70..c3673bee6d9cf 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -513,6 +513,66 @@ static int devm_backlight_device_match(struct 
>> device *dev, void *res,
>>       return *r == data;
>>   }
>>
>> +/**
>> + * backlight_device_lookup - find a backlight device
>> + * @name: sysname of the backlight device
>> + *
>> + * @return Reference to the backlight device, NULL if not found.
>> + *
>> + * This searches through all registered backlight devices for a 
>> device with the
>> + * given device name. In case none is found, NULL is returned, 
>> otherwise a
>> + * new reference to the backlight device is returned. You must drop this
>> + * reference via backlight_device_unref() once done.
>> + * Note that the devices might get unregistered at any time. You need 
>> to lock
>> + * around this lookup and inside of your backlight-notifier if you 
>> need to know
>> + * when a device gets unregistered.
>> + *
>> + * This function can be safely called from IRQ context.
>> + */
>> +struct backlight_device *backlight_device_lookup(const char *name)
>> +{
>> +     struct backlight_device *bd;
>> +     const char *t;
>> +
>> +     guard(mutex)(&backlight_dev_list_mutex);
>> +     list_for_each_entry(bd, &backlight_dev_list, entry) {
>> +             t = dev_name(&bd->dev);
>> +             if (t && !strcmp(t, name)) {
>> +                     backlight_device_ref(bd);
>> +                     return bd;
>> +             }
>> +     }
>> +
>> +     return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(backlight_device_lookup);
>>
> 
> Hello,
> 
> I think this function can be repalced with backlight_device_get_by_name.

Yes; good call.

> 
>> +/**
>> + * 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. The value is clamped according to device bounds.
>> + * A uevent notification is sent with the reason set to @reason.
>> + */
>> +void backlight_set_brightness(struct backlight_device *bd, unsigned 
>> int value,
>> +                           enum backlight_update_reason reason)
>> +{
>> +     guard(mutex)(&bd->ops_lock);
>> +     if (bd->ops) {
>> +             value = clamp(value, 0U,
>> +                           (unsigned int)bd->props.max_brightness);
> 
> Why did you use a clamping here? I think it is better to return error
> instead.
> 

This is called from a work queue.  This is the call path:

__drm_backlight_worker().
-> __drm_backlight_schedule()
->-> __drm_backlight_prop_changed()
->->-> drm_backlight_set_luminance().

So - I suppose that actually what you are suggesting is to plumb an 
error all the way from the work queue up to all the callers.  That might 
for a change to make things synchronous that weren't 'intended' to be 
synchronous.

Maybe a better solution is to try to look at the max brightness 
'directly' in drm_backlight_set_luminance() and then reject it before 
going down the work queue path.

Thoughts?

>> +             dev_dbg(&bd->dev, "set brightness to %u\n", value);
>> +             bd->props.brightness = value;
>> +             backlight_update_status(bd);
>> +     }
>> +     backlight_generate_event(bd, reason);
>> +}
>> +EXPORT_SYMBOL_GPL(backlight_set_brightness);
>>
> 
> I think this could be nice to update backlight_device_set_brightness to
> avoid code duplication:
> 
> int backlight_device_set_brightness(...) {
>         return backlight_set_brightness(..., BACKLIGHT_UPDATE_SYSFS);
> }

OK.

> 
>>   /**
>>    * 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..7e4fee65fddd9 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -429,6 +429,22 @@ static inline void 
>> backlight_notify_blank_all(struct device *display_dev,
>>   { }
>>   #endif
>>
>> +struct backlight_device *backlight_device_lookup(const char *name);
>> +void 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);
>> +}
>> +
> Most of the kernel use _put and _get functions, I think it could be nice
> to keep the same naming.

OK.

Re: [PATCH v3 0/8] Add support for a DRM backlight capability

[Mario Limonciello]

On 5/4/26 08:55, Louis Chauvet wrote:
> [You don't often get email from louis.chauvet@bootlin.com. Learn why 
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 4/25/26 00:09, Mario Limonciello wrote:
>> From: Mario Limonciello (AMD) <superm1@kernel.org>
>>
>> At Display Next Hackfest 2025 we discussed the renewed need for moving
>> brightness control into the DRM connector properties.  I've taken the
>> previous efforts from David and Marta, rebased and adjusted for the
>> current kernel.
>>
>> The legacy sysfs interface is synchronized with the DRM connector 
>> (although
>> the scale may be different as DRM connector property is u16).
>>
>> Later after this has been adopted by enough userspace, it may make 
>> sense to
>> configure the legacy sysfs interface to be configurable so that only
>> DRM master controls backlight.
>>
>> I've done a first implementation with amdgpu with eDP connectors; but
>> conceivably this can be extended to other connectors like DP for displays
>> that can be controlled via DDC as well later.
>>
>> I have also used DRM review prompts to review this series and fix some 
>> bugs
>> which were caught with two different Claude models.  The fixes are 
>> squashed
>> into the patches.
>>
>> Assisted-by: Claude Opus
>> Assisted-by: Claude Sonnet
>>
>> For ease of testing; this series is also available on this branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git/ 
>> log/?h=superm1/backlight-property-v3
> 
> Hello,
> 
> thanks for this work, I am very interested in this progress so I can
> help you to test / implement more features.

Thanks!  I have a few other series I'm juggling in other subsystems, but 
will try to get another spin this cycle.

> 
> I think you forgot to include the revert of "backlight: Remove notifier"
> in your series, it can't be applied without it.
> 

Yes - thanks for catching this.  It's on my tree at 
https://git.kernel.org/pub/scm/linux/kernel/git/superm1/linux.git/log/?h=superm1/backlight-property-v3 
but I forgot it when I built the series to get that one.

> I will take a look and see if I can create a VKMS implementation of your
> work.

Cool!  Please take a look at the IGT patches I did too if you didn't see 
them.  This should hopefully work once you glue it to VKMS.

https://lore.kernel.org/dri-devel/20260424221021.167179-1-mario.limonciello@amd.com/

> 
> Thanks for this work,
> 
>> David Rheinsberg (1):
>>    backlight: add kernel-internal backlight API
>>
>> Mario Limonciello (6):
>>    drm: link connectors to backlight devices
>>    DRM: Add support for client and driver indicating support for
>>      luminance
>>    drm/amd/display: Pass up errors reading actual brightness
>>    drm/amd: Indicate driver supports luminance
>>    drm/amd/display: Allow backlight registration to fail
>>    drm/amd/display: use drm backlight
>>
>> Marta Lofstedt (1):
>>    backlight: expose the current brightness in the new kernel API
>>
>>   drivers/gpu/drm/Kconfig                       |   1 +
>>   drivers/gpu/drm/Makefile                      |   2 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |   1 +
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  88 +++-
>>   drivers/gpu/drm/drm_atomic_uapi.c             |  24 ++
>>   drivers/gpu/drm/drm_backlight.c               | 406 ++++++++++++++++++
>>   drivers/gpu/drm/drm_connector.c               |  12 +
>>   drivers/gpu/drm/drm_drv.c                     |   8 +
>>   drivers/gpu/drm/drm_ioctl.c                   |  10 +
>>   drivers/gpu/drm/drm_mode_config.c             |   7 +
>>   drivers/gpu/drm/drm_mode_object.c             |  66 ++-
>>   drivers/gpu/drm/drm_sysfs.c                   |  54 +++
>>   drivers/video/backlight/backlight.c           |  83 ++++
>>   include/drm/drm_backlight.h                   |  45 ++
>>   include/drm/drm_connector.h                   |   8 +
>>   include/drm/drm_drv.h                         |   7 +
>>   include/drm/drm_file.h                        |   8 +
>>   include/drm/drm_mode_config.h                 |   5 +
>>   include/linux/backlight.h                     |  30 ++
>>   include/uapi/drm/drm.h                        |  10 +
>>   20 files changed, 852 insertions(+), 23 deletions(-)
>>   create mode 100644 drivers/gpu/drm/drm_backlight.c
>>   create mode 100644 include/drm/drm_backlight.h
>>
> 

Re: [PATCH v3 1/8] backlight: add kernel-internal backlight API

[Louis Chauvet]

On 5/4/26 19:55, Mario Limonciello wrote:
> 
> 
> On 5/4/26 08:55, Louis Chauvet wrote:
>> [You don't often get email from louis.chauvet@bootlin.com. Learn why 
>> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> On 4/25/26 00:09, Mario Limonciello wrote:
>>> From: David Herrmann <dh.herrmann@gmail.com>
>>>
>>> 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 <mario.limonciello@amd.com>
>>>   - rebase
>>>   - Use guard(mutex)
>>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>>   drivers/video/backlight/backlight.c | 60 +++++++++++++++++++++++++++++
>>>   include/linux/backlight.h           | 16 ++++++++
>>>   2 files changed, 76 insertions(+)
>>>
>>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/ 
>>> backlight/backlight.c
>>> index ab87a5e3dbf70..c3673bee6d9cf 100644
>>> --- a/drivers/video/backlight/backlight.c
>>> +++ b/drivers/video/backlight/backlight.c
>>> @@ -513,6 +513,66 @@ static int devm_backlight_device_match(struct 
>>> device *dev, void *res,
>>>       return *r == data;
>>>   }
>>>
>>> +/**
>>> + * backlight_device_lookup - find a backlight device
>>> + * @name: sysname of the backlight device
>>> + *
>>> + * @return Reference to the backlight device, NULL if not found.
>>> + *
>>> + * This searches through all registered backlight devices for a 
>>> device with the
>>> + * given device name. In case none is found, NULL is returned, 
>>> otherwise a
>>> + * new reference to the backlight device is returned. You must drop 
>>> this
>>> + * reference via backlight_device_unref() once done.
>>> + * Note that the devices might get unregistered at any time. You 
>>> need to lock
>>> + * around this lookup and inside of your backlight-notifier if you 
>>> need to know
>>> + * when a device gets unregistered.
>>> + *
>>> + * This function can be safely called from IRQ context.
>>> + */
>>> +struct backlight_device *backlight_device_lookup(const char *name)
>>> +{
>>> +     struct backlight_device *bd;
>>> +     const char *t;
>>> +
>>> +     guard(mutex)(&backlight_dev_list_mutex);
>>> +     list_for_each_entry(bd, &backlight_dev_list, entry) {
>>> +             t = dev_name(&bd->dev);
>>> +             if (t && !strcmp(t, name)) {
>>> +                     backlight_device_ref(bd);
>>> +                     return bd;
>>> +             }
>>> +     }
>>> +
>>> +     return NULL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(backlight_device_lookup);
>>>
>>
>> Hello,
>>
>> I think this function can be repalced with backlight_device_get_by_name.
> 
> Yes; good call.
> 
>>
>>> +/**
>>> + * 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. The value is clamped according to device bounds.
>>> + * A uevent notification is sent with the reason set to @reason.
>>> + */
>>> +void backlight_set_brightness(struct backlight_device *bd, unsigned 
>>> int value,
>>> +                           enum backlight_update_reason reason)
>>> +{
>>> +     guard(mutex)(&bd->ops_lock);
>>> +     if (bd->ops) {
>>> +             value = clamp(value, 0U,
>>> +                           (unsigned int)bd->props.max_brightness);
>>
>> Why did you use a clamping here? I think it is better to return error
>> instead.
>>
> 
> This is called from a work queue.  This is the call path:
> 
> __drm_backlight_worker().
> -> __drm_backlight_schedule()
> ->-> __drm_backlight_prop_changed()
> ->->-> drm_backlight_set_luminance().
> 
> So - I suppose that actually what you are suggesting is to plumb an 
> error all the way from the work queue up to all the callers.  That might 
> for a change to make things synchronous that weren't 'intended' to be 
> synchronous.

You are right, this is not a good idea to make this synchronous.

After looking a bit more at the code, I think you can simply not clamp 
at all and use a warn. In all cases:
- drm should never ask for a value outside 0..U16_MAX
- the conversion from 0..U16_MAX to 0..bd->props.max_brightness should 
never be bigger than bd->props.max_brightness

So if you have to clamp, it means there is a bug somewhere in the process.

I imagine something like:

b_set_brightness(value)
	if value > max:
		return -EINVAL
	[...]

__drm_backlight_worker()
	r = b_set_brightness(new_value)
	if r == -EINVAL: # This should never be the case, new_value should be 
clamped to 0..max_value before
		WARN("Invalid backlight value requested")
		# maybe add a fallback to at least have a coherent on/off state and 
avoid complete black screens
		if new_value==0: b_set_brightness(0)
		else: b_set_brightness(max_value)

__drm_backlight_prop_changed(u16 requested_value)
	new_value = requested_value * max_value / U16_MAX
	schedule_work(new_value)


> Maybe a better solution is to try to look at the max brightness 
> 'directly' in drm_backlight_set_luminance() and then reject it before 
> going down the work queue path.
>
> Thoughts?

That also a solution, but I think it is better to have a clear interface 
for drm_backlight_set_luminance and the backlight property: 
drm_backlight_set_luminance must get a 0..U16_MAX value, any other value 
is invalid.

But I have a question for this: is the scale of luminance linear or not? 
I think this information should be given to userspace.
And if lunimance is not linear, I don't think you can easily scale 
between 0..U16MAX to 0..max_value.

Same thing for backlight=0, IIRC sometimes the backlight driver switch 
off the light, sometimes it is just the minimum luminosity.

I think that can be a topic for discussion at drm hackfest, but it may 
also require more precise information / specification from the backlight 
subsystem / to the userspace interface.


>>> +             dev_dbg(&bd->dev, "set brightness to %u\n", value);
>>> +             bd->props.brightness = value;
>>> +             backlight_update_status(bd);
>>> +     }
>>> +     backlight_generate_event(bd, reason);
>>> +}
>>> +EXPORT_SYMBOL_GPL(backlight_set_brightness);
>>>
>>
>> I think this could be nice to update backlight_device_set_brightness to
>> avoid code duplication:
>>
>> int backlight_device_set_brightness(...) {
>>         return backlight_set_brightness(..., BACKLIGHT_UPDATE_SYSFS);
>> }
> 
> OK.

(note: this is only valid if you don't clamp in 
backlight_set_brightness, current code return -EINVAL if it is outside 
the range)

>>
>>>   /**
>>>    * 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..7e4fee65fddd9 100644
>>> --- a/include/linux/backlight.h
>>> +++ b/include/linux/backlight.h
>>> @@ -429,6 +429,22 @@ static inline void 
>>> backlight_notify_blank_all(struct device *display_dev,
>>>   { }
>>>   #endif
>>>
>>> +struct backlight_device *backlight_device_lookup(const char *name);
>>> +void 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);
>>> +}
>>> +
>> Most of the kernel use _put and _get functions, I think it could be nice
>> to keep the same naming.
> 
> OK.
> 

Re: [PATCH v3 4/8] DRM: Add support for client and driver indicating support for luminance

[Louis Chauvet]

On 4/25/26 00:09, Mario Limonciello wrote:
> The legacy backlight control interface can only be disabled when both
> the client and driver have agreed that the luminance can be set during
> a modeset. Add capability for the client to register and for the driver
> to indicate support.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/gpu/drm/drm_atomic_uapi.c | 24 ++++++++++++++++++++++++
>   drivers/gpu/drm/drm_connector.c   |  4 ++--
>   drivers/gpu/drm/drm_ioctl.c       | 10 ++++++++++
>   include/drm/drm_connector.h       |  5 +++++
>   include/drm/drm_drv.h             |  7 +++++++
>   include/drm/drm_file.h            |  8 ++++++++
>   include/uapi/drm/drm.h            | 10 ++++++++++
>   7 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 5bd5bf6661df7..de218206cef7e 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -30,6 +30,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_framebuffer.h>
>   #include <drm/drm_print.h>
>   #include <drm/drm_drv.h>
> @@ -935,6 +936,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 +1029,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;

I think this is a bad idea to have two "source of truth" for the 
luminance value. Is there a reason to not get the backlight value 
directly from the backlight driver?

>   	} else if (connector->funcs->atomic_get_property) {
>   		return connector->funcs->atomic_get_property(connector,
>   				state, property, val);
> @@ -1126,8 +1137,21 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
>   	if (connector->dpms == mode)
>   		goto out;
>   
> +	/* Handle backlight brightness coordination with DPMS state changes */
> +	if (old_mode != DRM_MODE_DPMS_OFF && mode == DRM_MODE_DPMS_OFF) {
> +		/* DPMS ON -> OFF: dim backlight to 0 to save power */
> +		drm_backlight_set_luminance(connector->backlight, 0);
> +	}
> +
>   	connector->dpms = mode;
>   
> +	/* DPMS OFF -> ON: restore brightness to property value */
> +	if (old_mode == DRM_MODE_DPMS_OFF && mode == DRM_MODE_DPMS_ON &&
> +	    connector->state) {
> +		drm_backlight_set_luminance(connector->backlight,
> +					    connector->state->luminance);
> +	}
> +

According to the documentation, DPMS is a deprecated property and most 
of the work should be handled using the crtc->active property.

I think this will not change the backlight of other connector using the 
same CRTC (CRTC outputs on connector1 and connector2, userspace change 
DPMS only for connector1, for me it makes sense to change connector1 and 
connector2).

After looking a bit more, I think you need to keep this code, but:
- add something to also disable backlight of other connector using the 
same crtc
- handle the backlight when CRTC is disconnected from the connector (i.e 
when you set CRTC=0 on a connector)

>   	crtc = connector->state->crtc;
>   	if (!crtc)
>   		goto out;
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index d76878548728a..5de33fc259b26 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -3268,9 +3268,9 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>   	/* Do DPMS ourselves */
>   	if (property == connector->dev->mode_config.dpms_property) {
>   		ret = (*connector->funcs->dpms)(connector, (int)value);
> -	} else if (property == config->brightness_property) {
> +	} else if (property == config->luminance_property) {

Can you directly name it luminance in the previous patch?

>   		if (connector->backlight && connector->dpms == DRM_MODE_DPMS_ON)
> -			drm_backlight_set_brightness(connector->backlight,
> +			drm_backlight_set_luminance(connector->backlight,
>   						     value);
>   		ret = 0;
>   	} else if (connector->funcs->set_property)
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index ff193155129e7..b4435c2bd6091 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -28,6 +28,7 @@
>    * OTHER DEALINGS IN THE SOFTWARE.
>    */
>   
> +#include "drm/drm.h"
>   #include <linux/export.h>
>   #include <linux/nospec.h>
>   #include <linux/pci.h>
> @@ -380,6 +381,15 @@ 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 (!drm_core_check_feature(dev, DRIVER_CONNECTOR_LUMINANCE))
> +			return -EOPNOTSUPP;
> +		if (!file_priv->atomic)
> +			return -EINVAL;
> +		if (req->value > 1)
> +			return -EINVAL;
> +		file_priv->supports_luminance_control = req->value;
> +		break;
>   	default:
>   		return -EINVAL;
>   	}
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 10daf088b8f1a..762a9e2ef6e30 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
> +	 */
> +	u16 luminance;
>   };
>   
>   struct drm_connector_hdmi_audio_funcs {
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 42fc085f986de..a6b668cb68c5e 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -123,6 +123,13 @@ enum drm_driver_feature {
>   	 */
>   	DRIVER_CURSOR_HOTSPOT           = BIT(9),
>   
> +	/**
> +	 * @DRIVER_CONNECTOR_LUMINANCE:
> +	 *
> +	 * Driver supports luminance control on a per connector basis.
> +	 */
> +	DRIVER_CONNECTOR_LUMINANCE           = BIT(10),
> +
>   	/* IMPORTANT: Below are all the legacy flags, add new ones above. */
>   
>   	/**
> 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/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 27cc159c1d275..b5e6d940f2816 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -921,6 +921,16 @@ struct drm_get_cap {
>    */
>   #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE	7
>   
> +/**
> + * DRM_CLIENT_CAP_LUMINANCE
> + *
> + * If set to 1, legacy sysfs interface for controlling backlight brightness will
> + * be disabled.  The client will include luminance values as part of the modeset.
> +
> + * 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;