Hi Mario,
On 24-Jun-26 18:57, Mario Limonciello wrote:
> From: "Mario Limonciello (AMD)" <[email protected]>
>
> The legacy backlight control interface can only be disabled when both
> the client and driver has connector support that the luminance can be
> set during a modeset. Add capability for the client to register.
>
> When a luminance-aware client sets DRM_CLIENT_CAP_LUMINANCE, each
> DRM-connected backlight on the device is marked as taken over. Writes
> to the legacy /sys/class/backlight/<dev>/brightness attribute then
> return -EBUSY until the last luminance-aware client clears the cap or
> closes its DRM file. The takeover follows the active backlight_device
> when drm_backlight_link() retargets the link.
>
> Tested-by: Dmitry Baryshkov <[email protected]> # SM8150-HDK
> Signed-off-by: Mario Limonciello (AMD) <[email protected]>
> ---
<snip>
> diff --git a/drivers/gpu/drm/drm_backlight.c b/drivers/gpu/drm/drm_backlight.c
> index b1ec470be86ca..2cddf209d5088 100644
> --- a/drivers/gpu/drm/drm_backlight.c
> +++ b/drivers/gpu/drm/drm_backlight.c
> @@ -71,6 +71,7 @@ static bool __drm_backlight_is_registered(struct
> drm_backlight *b)
> /* caller must hold @drm_backlight_lock */
> static void __drm_backlight_real_changed(struct drm_backlight *b, uint64_t v)
> {
> + struct drm_connector *connector = b->connector;
> unsigned int max, set;
>
> lockdep_assert_held(&drm_backlight_lock);
> @@ -85,6 +86,15 @@ static void __drm_backlight_real_changed(struct
> drm_backlight *b, uint64_t v)
> set = v;
> if (set >= max)
> set = max;
> +
> + /* Update the atomic state directly.
> + * For atomic drivers, the luminance value is stored in
> + * connector->state->luminance, not in the legacy property array.
> + * We update it unconditionally to reflect the hardware state,
> + * regardless of DPMS.
> + */
> + if (connector->state)
> + connector->state->luminance = set;
> }
>
> /**
> @@ -100,18 +110,22 @@ static void __drm_backlight_update_prop_range(struct
> drm_backlight *b)
> struct drm_device *dev = b->connector->dev;
> struct drm_property *prop = dev->mode_config.luminance_property;
> unsigned int max = 0;
> + bool can_disable = false;
>
> lockdep_assert_held(&drm_backlight_lock);
>
> - if (b->link && b->link->props.max_brightness > 0)
> + if (b->link && b->link->props.max_brightness > 0) {
> max = b->link->props.max_brightness;
> + can_disable = b->link->props.can_disable;
> + }
>
> /* Update property range to match hardware capabilities.
> * Range of 0-0 indicates no backing device.
> - * Range of 1-max for normal operation (0 reserved for display off).
> + * Range of 1-max for normal operation.
> + * Range of 0-max means that the display would turn off at 0
> */
> if (prop->values[1] != max) {
> - prop->values[0] = max ? 1 : 0;
> + prop->values[0] = max ? (can_disable ? 0 : 1) : 0;
> prop->values[1] = max;
> }
So here we now get:
range 0 - 0 for max = 0, aka no backlight control support
range 0 - max for max != 0, can_disable = true
range 1 - max for max != 0, can_disable = false
<snip>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 128d431f0d6b0..70afe3d579f38 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1512,6 +1512,57 @@
> EXPORT_SYMBOL(drm_hdmi_connector_get_output_format_name);
> * Summarizing: Only set "DPMS" when the connector is known to be enabled,
> * assume that a successful SETCONFIG call also sets "DPMS" to on, and
> * never read back the value of "DPMS" because it can be incorrect.
> + * LUMINANCE:
> + * Atomic property for controlling the backlight brightness level of the
> + * connector's display. This property provides unified access to the
> display
> + * backlight, replacing the legacy sysfs interface for brightness control.
> + *
> + * The property value is an unsigned integer representing the brightness
> level.
> + * The valid range is dynamically determined by the capabilities of the
> + * connected backlight hardware and is exposed through the property's
> minimum
> + * and maximum values:
> + *
> + * - Range 0-0: No backlight device is available for this connector.
> + * - Range 1-N: Normal operation. Values from 1 to N (max_brightness) are
> + * valid brightness levels, where 1 is the minimum visible brightness and
> + * N is the maximum brightness the hardware supports.
But here range 0 - N is not documented.
Good that this documents that "1 is the minimum visible brightness" but I think
some drivers may need to be adjusted for that. Last time I checked the amdgpu
driver would take a certain fixed minimum percentage as lowest brightness when
the firmware provided a 0 value for minimum pwm duty-cycle, so amdgpu has this
covered. But the i915 driver happily goes down to 0% duty when the VBT does
not give a non 0 minimum.
This is why e.g. GNOME adds its own minimum percentage and never writes 0 to
the classic sysfs interface. So assuming this all holds, this means that
supporting "1 is the minimum visible brightness" will require some fixes on
the i915 driver (which would be good to have) and possibly other drivers
need fixing as well.
> + * - Value 0: Special value to turn off the display backlight completely.
> + * This value is accepted even when the normal range starts at 1.
> + *> + * The range may change during runtime if a new backlight device
> is linked
> + * or unlinked. The kernel will send a change uevent when this occurs.
> + *
> + * Setting LUMINANCE to 0 turns off the backlight, which may turn off the
> + * display completely depending on the hardware.
Ok, so 0 is DPMS off, right ? maybe mention that explicitly because
I think that has certain implications like possible no longer getting
vsync events / framebuffer swap completions which userspace might
depend on. Where as a classic set pwm duty to 0 for backlight does
result in vsync events to continue. "turn off the display" is a bit
ambigous IMHO, e.g. is it just the panel which gets turned off or
the entire dsiplay pipeline?
Might also just me not being familiar enough with the kms API...
Regards,
Hans
> Setting it to any value
> + * from 1 to N adjusts the brightness accordingly. Reading this property
> + * returns the current brightness level that was last set (or the
> hardware's
> + * current state for drivers that support reading actual brightness).
> + *
> + * For atomic drivers, the luminance value is stored in
> + * &drm_connector_state.luminance. The actual hardware update only occurs
> + * when the connector is active (DPMS is ON). When DPMS transitions to OFF,
> + * the kernel automatically sets luminance to 0 to turn off the backlight.
> + * When DPMS transitions back to ON, the kernel restores the previously
> + * set luminance value.
> + *
> + * This property is only available on connectors that have an associated
> + * backlight device. The property is created by calling
> drm_backlight_alloc()
> + * during connector initialization.
> + *
> + * Client Capability:
> + * User-space must set the DRM_CLIENT_CAP_LUMINANCE client
> capability
> + * to 1 before using this property. When this capability is
> enabled,
> + * the legacy sysfs backlight interface is inhibited to prevent
> + * conflicts between multiple clients trying to control the same
> + * backlight. This ensures that only luminance-aware clients
> control
> + * the backlight through the DRM atomic interface.
> + *
> + * Legacy clients that do not set this capability will not see the
> + * LUMINANCE property and should continue using the sysfs interface
> + * (if available).
> + *
> + * Note: This property can be set through MODE_ATOMIC ioctl as part of the
> + * atomic state.
> * panel_type:
> * Immutable enum property to indicate the type of connected panel.
> * Possible values are "unknown" (default) and "OLED".
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index ec820686b3021..4d2520de7614c 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -41,6 +41,7 @@
> #include <linux/slab.h>
> #include <linux/vga_switcheroo.h>
>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_client_event.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> @@ -252,6 +253,10 @@ void drm_file_free(struct drm_file *file)
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> drm_fb_release(file);
> drm_property_destroy_user_blobs(dev, file);
> + if (file->supports_luminance_control) {
> + drm_backlight_uninhibit_legacy_all(dev);
> + file->supports_luminance_control = false;
> + }
> }
>
> if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index ff193155129e7..fdae36b13300a 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -28,12 +28,14 @@
> * OTHER DEALINGS IN THE SOFTWARE.
> */
>
> +#include "drm/drm.h"
> #include <linux/export.h>
> #include <linux/nospec.h>
> #include <linux/pci.h>
> #include <linux/uaccess.h>
>
> #include <drm/drm_auth.h>
> +#include <drm/drm_backlight.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> @@ -380,6 +382,19 @@ drm_setclientcap(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
> return -EINVAL;
> file_priv->plane_color_pipeline = req->value;
> break;
> + case DRM_CLIENT_CAP_LUMINANCE:
> + if (!file_priv->atomic)
> + return -EINVAL;
> + if (req->value > 1)
> + return -EINVAL;
> + if (req->value == file_priv->supports_luminance_control)
> + break;
> + if (req->value)
> + drm_backlight_inhibit_legacy_all(dev);
> + else
> + drm_backlight_uninhibit_legacy_all(dev);
> + file_priv->supports_luminance_control = req->value;
> + break;
> default:
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index f38f2c5437e68..4475896c963b4 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -952,6 +952,12 @@ bool drm_property_change_valid_get(struct drm_property
> *property,
> *ref = NULL;
>
> if (drm_property_type_is(property, DRM_MODE_PROP_RANGE)) {
> + /* Special case for luminance property: allow 0 to turn off
> display
> + * even when the normal range starts at 1.
> + */
> + if (property == property->dev->mode_config.luminance_property &&
> + value == 0 && property->values[1] > 0)
> + return true;
> if (value < property->values[0] || value > property->values[1])
> return false;
> return true;
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index 13954c2220b7e..40cfc2296445c 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -217,6 +217,13 @@ static ssize_t brightness_store(struct device *dev,
> struct backlight_device *bd = to_backlight_device(dev);
> unsigned long brightness;
>
> + /* A luminance-aware DRM client has taken over this backlight; the
> + * legacy sysfs interface is disabled until the last such client
> + * goes away.
> + */
> + if (atomic_read(&bd->drm_takeover) > 0)
> + return -EBUSY;
> +
> rc = kstrtoul(buf, 0, &brightness);
> if (rc)
> return rc;
> diff --git a/include/drm/drm_backlight.h b/include/drm/drm_backlight.h
> index e0e09e38f7c06..2af48be3aa372 100644
> --- a/include/drm/drm_backlight.h
> +++ b/include/drm/drm_backlight.h
> @@ -43,9 +43,9 @@ void drm_backlight_unregister(struct drm_backlight *b);
>
> void drm_backlight_link(struct drm_backlight *b, struct backlight_device
> *bd);
> struct backlight_device *drm_backlight_get_device(struct drm_backlight *b);
> -void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int
> value);
> void drm_backlight_inhibit_legacy(struct drm_backlight *b);
> void drm_backlight_uninhibit_legacy(struct drm_backlight *b);
> void drm_backlight_inhibit_legacy_all(struct drm_device *dev);
> void drm_backlight_uninhibit_legacy_all(struct drm_device *dev);
> +void drm_backlight_set_luminance(struct drm_backlight *b, unsigned int
> value);
> #endif /* __DRM_BACKLIGHT_H__ */
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index d78ac1068d12e..5e32b6063d228 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1209,6 +1209,11 @@ struct drm_connector_state {
> * @drm_atomic_helper_connector_hdmi_check().
> */
> struct drm_connector_hdmi_state hdmi;
> +
> + /**
> + * @luminance: Luminance for the connector
> + */
> + unsigned int luminance;
> };
>
> struct drm_connector_hdmi_audio_funcs {
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 6ee70ad65e1fd..0bb1e53f36bec 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -248,6 +248,14 @@ struct drm_file {
> */
> bool supports_virtualized_cursor_plane;
>
> + /**
> + * @supports_luminance_control:
> + *
> + * This client is capable of setting the luminance for connectors.
> + *
> + */
> + bool supports_luminance_control;
> +
> /**
> * @master:
> *
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 26a7281d179c1..b03737ee8dacd 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -197,6 +197,13 @@ struct backlight_properties {
> */
> int brightness;
>
> + /**
> + * @can_disable: Minimum brightness would turn off the panel.
> + *
> + * Setting minimum brightness turns off the panel.
> + */
> + bool can_disable;
> +
> /**
> * @max_brightness: The maximum brightness value.
> *
> @@ -314,6 +321,14 @@ struct backlight_device {
> * @use_count: The number of unblanked displays.
> */
> int use_count;
> +
> + /**
> + * @drm_takeover: Number of luminance-aware DRM clients that have
> + * taken over brightness control of this device. When non-zero,
> + * writes to the legacy sysfs ``brightness`` attribute return
> + * ``-EBUSY``. Managed by the DRM backlight helpers.
> + */
> + atomic_t drm_takeover;
> };
>
> /* Forward declaration for backlight_update_status */
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index bc7ef7684099b..a3141d46d7d66 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -903,6 +903,28 @@ struct drm_get_cap {
> */
> #define DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE 7
>
> +/**
> + * DRM_CLIENT_CAP_LUMINANCE
> + *
> + * If set to 1, the client declares support for the LUMINANCE connector
> property
> + * and will control backlight brightness through the DRM atomic interface.
> This
> + * enables the kernel to expose the LUMINANCE property on connectors that
> have
> + * an associated backlight device.
> + *
> + * When this capability is enabled:
> + * - The LUMINANCE property becomes visible on supported connectors
> + * - Legacy sysfs writes to /sys/class/backlight/{*}/brightness will return
> + * -EBUSY to prevent conflicts with DRM-based brightness control
> + * - The client should include luminance values as part of atomic commits
> + * - Brightness changes are synchronized with display power state (DPMS)
> + *
> + * The LUMINANCE property accepts values from 0 to max_brightness, where 0
> turns
> + * off the backlight, and 1 to max_brightness control the brightness level.
> + *
> + * This capability is supported starting in kernel 7.2.
> + */
> +#define DRM_CLIENT_CAP_LUMINANCE 8
> +
> /* DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */
> struct drm_set_client_cap {
> __u64 capability;
