Re: [PATCH v8 02/14] backlight: add kernel-internal backlight API

[Mario Limonciello <mario.limonciello@amd.com>]

On 9/22/26 03:49, Javier Martinez Canillas wrote:
> On Tue, Sep 22, 2026 at 10:24 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> 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 <dh.herrmann@gmail.com>
>>>
> 
> [...]
> 
>>> @@ -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.
> 
> And regardless if this can be done or not, I belive that would be
> better if this is something that drivers decide to do and just return
> -EBUSY from their struct backlight_ops..update_status.
> 
> Ideally the sysfs interface should continue to be work as Thomas said
> and a write could trigger a mode set for example, but I don't know if
> that is feasible to do due locking.
> 

Well the problem ends up being that something can change brightness 
behind the compositor's back if you leave sysfs available.

The whole design here is to put the compositor in control.  For example 
if compositor wants to enforce brightness to be a certain value when 
certain content is being displayed.