[PATCH 12/13] leds: backlight trigger: Replace fb events with a dedicated function call
Thomas Zimmermann
tzimmermann at suse.de
Thu Feb 13 14:34:41 UTC 2025
Hi
Am 11.02.25 um 14:57 schrieb Lee Jones:
> On Thu, 06 Feb 2025, Thomas Zimmermann wrote:
>
>> Remove support for fb events from the led backlight trigger. Provide the
>> helper ledtrig_backlight_blank() instead. Call it from fbdev to inform
>> the trigger of changes to a display's blank state.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>> drivers/leds/trigger/ledtrig-backlight.c | 31 +++++-------------------
>> drivers/video/fbdev/core/fbmem.c | 21 +++++++++-------
>> include/linux/leds.h | 6 +++++
>> 3 files changed, 24 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
>> index f9317f93483b..e3ae4adc29e3 100644
>> --- a/drivers/leds/trigger/ledtrig-backlight.c
>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>> @@ -10,7 +10,6 @@
>> #include <linux/kernel.h>
>> #include <linux/slab.h>
>> #include <linux/init.h>
>> -#include <linux/fb.h>
>> #include <linux/leds.h>
>> #include "../leds.h"
>>
>> @@ -21,7 +20,6 @@ struct bl_trig_notifier {
>> struct led_classdev *led;
>> int brightness;
>> int old_status;
>> - struct notifier_block notifier;
>> unsigned invert;
>>
>> struct list_head entry;
>> @@ -30,7 +28,7 @@ struct bl_trig_notifier {
>> static struct list_head ledtrig_backlight_list;
>> static struct mutex ledtrig_backlight_list_mutex;
>>
>> -static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
>> +static void __ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
> I'm confused. Didn't you just introduce this?
It's being renamed here; probably something to avoid.
>
>> {
>> struct led_classdev *led = n->led;
>> int new_status = !on ? BLANK : UNBLANK;
>> @@ -48,23 +46,14 @@ static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)
>> n->old_status = new_status;
>> }
>>
>> -static int fb_notifier_callback(struct notifier_block *p,
>> - unsigned long event, void *data)
>> +void ledtrig_backlight_blank(bool on)
>> {
>> - struct bl_trig_notifier *n = container_of(p,
>> - struct bl_trig_notifier, notifier);
>> - struct fb_event *fb_event = data;
>> - int *blank;
>> -
>> - /* If we aren't interested in this event, skip it immediately ... */
>> - if (event != FB_EVENT_BLANK)
>> - return 0;
>> -
>> - blank = fb_event->data;
>> + struct bl_trig_notifier *n;
>>
>> - ledtrig_backlight_blank(n, !blank[0]);
>> + guard(mutex)(&ledtrig_backlight_list_mutex);
>>
>> - return 0;
>> + list_for_each_entry(n, &ledtrig_backlight_list, entry)
>> + __ledtrig_backlight_blank(n, on);
>> }
>>
>> static ssize_t bl_trig_invert_show(struct device *dev,
>> @@ -110,8 +99,6 @@ ATTRIBUTE_GROUPS(bl_trig);
>>
>> static int bl_trig_activate(struct led_classdev *led)
>> {
>> - int ret;
>> -
>> struct bl_trig_notifier *n;
>>
>> n = kzalloc(sizeof(struct bl_trig_notifier), GFP_KERNEL);
>> @@ -122,11 +109,6 @@ static int bl_trig_activate(struct led_classdev *led)
>> n->led = led;
>> n->brightness = led->brightness;
>> n->old_status = UNBLANK;
>> - n->notifier.notifier_call = fb_notifier_callback;
>> -
>> - ret = fb_register_client(&n->notifier);
>> - if (ret)
>> - dev_err(led->dev, "unable to register backlight trigger\n");
>>
>> mutex_lock(&ledtrig_backlight_list_mutex);
>> list_add(&n->entry, &ledtrig_backlight_list);
>> @@ -143,7 +125,6 @@ static void bl_trig_deactivate(struct led_classdev *led)
>> list_del(&n->entry);
>> mutex_unlock(&ledtrig_backlight_list_mutex);
>>
>> - fb_unregister_client(&n->notifier);
>> kfree(n);
>> }
>>
>> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
>> index fb7ca143a996..92c3fe2a7aff 100644
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -16,6 +16,7 @@
>> #include <linux/fb.h>
>> #include <linux/fbcon.h>
>> #include <linux/lcd.h>
>> +#include <linux/leds.h>
>>
>> #include <video/nomodeset.h>
>>
>> @@ -373,11 +374,19 @@ static void fb_lcd_notify_blank(struct fb_info *info)
>> #endif
>> }
>>
>> +static void fb_ledtrig_backlight_notify_blank(struct fb_info *info)
>> +{
>> +#if IS_REACHABLE(CONFIG_LEDS_TRIGGER_BACKLIGHT)
> #iferry is generally discouraged in C files.
>
> Does is_defined() work for you?
I don't think so. This ifdef covers the case that fbdev is built in, but
led is a module (i.e., fbdev=y && led=m).
> /
>> + if (info->blank == FB_BLANK_UNBLANK)
>> + ledtrig_backlight_blank(true);
> If !CONFIG_LEDS_TRIGGER_BACKLIGHT(), then ledtrig_backlight_blank() is a
> noop anyway, right? So why encompass it in the #if at all?
Because of (fbdev=y && led=m) again. ledtrig_backlight_blank() would be
defined then, but not linkable from fbdev. Preferably, I'd rather leave
out the ifdef in the led header file.
Best regards
Thomas
>
>> + else
>> + ledtrig_backlight_blank(false);
>> +#endif
>> +}
>> +
>> int fb_blank(struct fb_info *info, int blank)
>> {
>> int old_blank = info->blank;
>> - struct fb_event event;
>> - int data[2];
>> int ret;
>>
>> if (!info->fbops->fb_blank)
>> @@ -386,11 +395,6 @@ int fb_blank(struct fb_info *info, int blank)
>> if (blank > FB_BLANK_POWERDOWN)
>> blank = FB_BLANK_POWERDOWN;
>>
>> - data[0] = blank;
>> - data[1] = old_blank;
>> - event.info = info;
>> - event.data = data;
>> -
>> info->blank = blank;
>>
>> ret = info->fbops->fb_blank(blank, info);
>> @@ -399,8 +403,7 @@ int fb_blank(struct fb_info *info, int blank)
>>
>> fb_bl_notify_blank(info, old_blank);
>> fb_lcd_notify_blank(info);
>> -
>> - fb_notifier_call_chain(FB_EVENT_BLANK, &event);
>> + fb_ledtrig_backlight_notify_blank(info);
>>
>> return 0;
>>
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index 98f9719c924c..8c7c41888f7d 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -640,6 +640,12 @@ static inline void ledtrig_flash_ctrl(bool on) {}
>> static inline void ledtrig_torch_ctrl(bool on) {}
>> #endif
>>
>> +#if IS_ENABLED(CONFIG_LEDS_TRIGGER_BACKLIGHT)
>> +void ledtrig_backlight_blank(bool on);
>> +#else
>> +static inline void ledtrig_backlight_blank(bool on) {}
>> +#endif
>> +
>> /*
>> * Generic LED platform data for describing LED names and default triggers.
>> */
>> --
>> 2.48.1
>>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
More information about the dri-devel
mailing list