Hi Mario

Am 08.09.26 um 06:40 schrieb Mario Limonciello:
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 a few kernel-internal backlight helpers so we can modify
backlights from within DRM, a brightness-changed notification, and a
per-device takeover count so that legacy sysfs writes can be inhibited
(-EBUSY) while a luminance-aware DRM client is in control.

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 (AMD) <[email protected]>
  - rebase
  - Use guard(mutex)

V4: Mario Limonciello (AMD) <[email protected]>
  - Adjust return type for backlight_set_brightness() to return errors
  - Stop clamping in backlight_set_brightness()
  - Drop backlight_device_lookup()

Signed-off-by: Mario Limonciello (AMD) <[email protected]>
---
  drivers/video/backlight/backlight.c | 66 +++++++++++++++++++++++++++++
  include/linux/backlight.h           | 45 ++++++++++++++++++++
  2 files changed, 111 insertions(+)

diff --git a/drivers/video/backlight/backlight.c 
b/drivers/video/backlight/backlight.c
index 4401f6294ccc8..1c700c1e77c3f 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -126,6 +126,9 @@ static void backlight_generate_event(struct 
backlight_device *bd,
        case BACKLIGHT_UPDATE_HOTKEY:
                envp[0] = "SOURCE=hotkey";
                break;
+       case BACKLIGHT_UPDATE_DRM:
+               envp[0] = "SOURCE=drm";
+               break;
        default:
                envp[0] = "SOURCE=unknown";
                break;
@@ -150,6 +153,13 @@ static ssize_t bl_power_store(struct device *dev, struct 
device_attribute *attr,
        struct backlight_device *bd = to_backlight_device(dev);
        unsigned long power, old_power;
+ /* A luminance-aware DRM client has taken over this backlight; the
+        * legacy sysfs interface is disabled until the last such client
+        * goes away.
+        */
+       if (atomic_read(&bd->drm_takeover) > 0)
+               return -EBUSY;
+
        rc = kstrtoul(buf, 0, &power);
        if (rc)
                return rc;
@@ -214,6 +224,13 @@ static ssize_t brightness_store(struct device *dev,
        struct backlight_device *bd = to_backlight_device(dev);
        unsigned long brightness;
+ /* A luminance-aware DRM client has taken over this backlight; the
+        * legacy sysfs interface is disabled until the last such client
+        * goes away.
+        */
+       if (atomic_read(&bd->drm_takeover) > 0)
+               return -EBUSY;

Can you really do that?  AFAIU sysfs is now a de-facto uapi for backlights.


+
        rc = kstrtoul(buf, 0, &brightness);
        if (rc)
                return rc;
@@ -514,6 +531,41 @@ static int devm_backlight_device_match(struct device *dev, 
void *res,
        return *r == data;
  }
+/**
+ * backlight_set_brightness - set brightness on a backlight device
+ * @bd: backlight device to operate on
+ * @value: brightness value to set on the device
+ * @reason: backlight-change reason to use for notifications
+ *
+ * This is the in-kernel API equivalent of writing into the 'brightness' sysfs
+ * file. It calls into the underlying backlight driver to change the brightness
+ * value.
+ * A uevent notification is sent with the reason set to @reason.
+ * Return: 0 if successfully notified, -EINVAL for invalid values, -ENXIO if 
the
+ * device has no ops (e.g. it is being unregistered)
+ */
+int backlight_set_brightness(struct backlight_device *bd, unsigned int value,
+                             enum backlight_update_reason reason)
+{
+       int rc = -ENXIO;
+
+       scoped_guard(mutex, &bd->ops_lock) {
+               if (bd->ops) {
+                       if (value > bd->props.max_brightness)
+                               return -EINVAL;
+
+                       dev_dbg(&bd->dev, "set brightness to %u\n", value);
+                       bd->props.brightness = value;
+                       rc = backlight_update_status(bd);
+               }
+       }
+       if (rc == 0)
+               backlight_generate_event(bd, reason);
+
+       return rc;
+}
+EXPORT_SYMBOL_GPL(backlight_set_brightness);
+
  /**
   * backlight_register_notifier - get notified of backlight (un)registration
   * @nb: notifier block with the notifier to call on backlight (un)registration
@@ -548,6 +600,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);

With the sysfs interfaces disabled, who is going to trigger this code except DRM?

AFAICT when the DRM connector updates the backlight status, backlight runs this notifier only to call back into DRM to read back the value. But the luminance property should already store that same value.

Best regards
Thomas

+
  /**
   * devm_backlight_device_register - register a new backlight device
   * @dev: the device to register
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 015183d129f96..fac1acbc698a1 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -31,6 +31,12 @@ enum backlight_update_reason {
         * @BACKLIGHT_UPDATE_SYSFS: The backlight was updated using sysfs.
         */
        BACKLIGHT_UPDATE_SYSFS,
+
+       /**
+        * @BACKLIGHT_UPDATE_DRM: The backlight was updated from DRM, i.e. 
through
+        * a connector LUMINANCE property rather than the legacy sysfs 
interface.
+        */
+       BACKLIGHT_UPDATE_DRM,
  };
/**
@@ -82,6 +88,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,8 +321,23 @@ struct backlight_device {
         * @use_count: The number of unblanked displays.
         */
        int use_count;
+
+       /**
+        * @drm_takeover: Number of luminance-aware DRM clients that have
+        * taken over brightness control of this device. When non-zero,
+        * writes to the legacy sysfs ``brightness`` attribute return
+        * ``-EBUSY``. Managed by the DRM backlight helpers.
+        */
+       atomic_t drm_takeover;
  };
+/* Forward declaration for backlight_update_status */
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
+void backlight_notify_brightness(struct backlight_device *bd);
+#else
+static inline void backlight_notify_brightness(struct backlight_device *bd) {}
+#endif
+
  /**
   * backlight_update_status - force an update of the backlight device status
   * @bd: the backlight device
@@ -325,6 +351,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;
  }
@@ -431,6 +461,21 @@ static inline void backlight_notify_blank_all(struct device *display_dev,
  { }
  #endif
+int backlight_set_brightness(struct backlight_device *bd, unsigned int value,
+                             enum backlight_update_reason reason);
+
+static inline void backlight_device_ref(struct backlight_device *bd)
+{
+       if (bd)
+               get_device(&bd->dev);
+}
+
+static inline void backlight_device_unref(struct backlight_device *bd)
+{
+       if (bd)
+               put_device(&bd->dev);
+}
+
  #define to_backlight_device(obj) container_of(obj, struct backlight_device, 
dev)
/**

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)


Reply via email to