On 4/25/26 00:09, Mario Limonciello wrote:
From: David Herrmann <[email protected]>

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 <[email protected]>

V2: Marta Lofstedt <[email protected]>
- rebase
- minor edit for checkpatch warning

Signed-off-by: Marta Lofstedt <[email protected]>

V3: Mario Limonciello <[email protected]>
  - rebase
  - Use guard(mutex)

Signed-off-by: Mario Limonciello <[email protected]>
---
  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)
/**

Reply via email to