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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] backlight_notify_brightness() is called inside backlight_update_status() while bd->ops_lock is held, creating an AB-BA deadlock risk with DRM listeners.
- [Medium] Hardware/hotkey-driven brightness changes reported via backlight_force_update() bypass backlight_notify_brightness(), causing in-kernel listeners to lose sync with the actual state.
--

commit 56d45c464432e9b84ad73dff6ec5f6ddda9af544
Author: Mario Limonciello <mario.limonciello@amd.com>

backlight: add kernel-internal backlight API

This commit adds internal backlight helpers allowing DRM drivers to provide
an interface for user-space to control the backlight via DRM connector
properties. It introduces a brightness-changed notification and a per-device
takeover count to inhibit legacy sysfs writes while a luminance-aware DRM
client is in control.

> 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
[ ... ]
> @@ -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);

[Severity: Medium]
Will in-kernel listeners lose sync with the actual hardware backlight state
when brightness is changed via a hardware hotkey or ACPI event?

When physical hotkey presses trigger platform drivers to invoke
backlight_force_update():

drivers/video/backlight/backlight.c:backlight_force_update() {
	...
	mutex_unlock(&bd->ops_lock);
	backlight_generate_event(bd, reason);
}

It emits a user-space uevent via backlight_generate_event(), but it doesn't
call the newly added backlight_notify_brightness() helper. Should it also
notify in-kernel DRM connector luminance properties here?

> 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
[ ... ]
> @@ -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);

[Severity: High]
Does calling backlight_notify_brightness() here create an AB-BA deadlock risk
with DRM listeners?

The caller of backlight_update_status() often holds bd->ops_lock (for example,
in backlight_set_brightness() or legacy sysfs writes). This means the blocking
notifier chain is fired synchronously under ops_lock.

If a DRM listener acquires a DRM lock during the notification callback, it
establishes an ops_lock -> drm_lock dependency. 

Conversely, if a DRM driver calls backlight_set_brightness() while holding
DRM locks, it will attempt to acquire ops_lock, resulting in a drm_lock ->
ops_lock dependency.

Should the notification be deferred until after ops_lock is released, just
as backlight_generate_event() is handled?

> +
>  	return ret;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908044035.62093-1-mario.limonciello@amd.com?part=2