[RFC PATCH 00/13] backlight, lcd, led: Remove fbdev dependencies

[RFC PATCH 00/13] backlight, lcd, led: Remove fbdev dependencies

[Thomas Zimmermann]

This series removes the last dependencies on fbdev from the backlight,
lcd and led subsystems. Each depends on fbdev events to track display
state. Make fbdev inform each subsystem via a dedicated interface
instead.

Patches 1 to 3 make fbdev track blank state for each display, so
that backlight code doesn't have to.

Patches 4 to 6 remove fbdev event handling from backlight code. Patches
7 to 9 remove fbdev event handling from lcd code and patches 10 to 12
do the same for led's backlight trigger.

The final patch removes the event constants from fbdev.

With the series applied, the three subsystems do no longer depend
on fbdev. It's also a clean up for fbdev. Fbdev used to send out a
large number of events. That mechanism has been deprecated for some
time and converted call to dedicated functions instead.

Testing is very welcome, as I don't have the hardware to really test
this series.

Thomas Zimmermann (13):
  fbdev: Rework fb_blank()
  fbdev: Track display blanking state
  fbdev: Send old blank state in FB_EVENT_BLANK
  backlight: Implement fbdev tracking with blank state from event
  backlight: Move blank-state handling into helper
  backlight: Replace fb events with a dedicated function call
  backlight: lcd: Maintain global list of lcd devices
  backlight: lcd: Move event handling into helpers
  backlight: lcd: Replace fb events with a dedicated function call
  leds: backlight trigger: Maintain global list of led backlight
    triggers
  leds: backlight trigger: Move blank-state handling into helper
  leds: backlight trigger: Replace fb events with a dedicated function
    call
  fbdev: Remove constants of unused events

 drivers/leds/trigger/ledtrig-backlight.c |  49 +++++-----
 drivers/video/backlight/backlight.c      |  93 +++++--------------
 drivers/video/backlight/lcd.c            | 109 +++++++++--------------
 drivers/video/fbdev/core/fb_backlight.c  |  12 +++
 drivers/video/fbdev/core/fb_info.c       |   1 +
 drivers/video/fbdev/core/fbmem.c         |  88 +++++++++++++++---
 drivers/video/fbdev/core/fbsysfs.c       |   8 +-
 include/linux/backlight.h                |  22 ++---
 include/linux/fb.h                       |  12 +--
 include/linux/lcd.h                      |  12 ++-
 include/linux/leds.h                     |   6 ++
 11 files changed, 204 insertions(+), 208 deletions(-)

-- 
2.48.1

[PATCH 01/13] fbdev: Rework fb_blank()

[Thomas Zimmermann]

Reimplement fb_blank() to return early on errors. No functional
changes. Prepares the helper for tracking the blanking state in
struct fb_info.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/core/fbmem.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 3c568cff2913..39e2b81473ad 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -339,11 +339,13 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 }
 EXPORT_SYMBOL(fb_set_var);
 
-int
-fb_blank(struct fb_info *info, int blank)
+int fb_blank(struct fb_info *info, int blank)
 {
 	struct fb_event event;
-	int ret = -EINVAL;
+	int ret;
+
+	if (!info->fbops->fb_blank)
+		return -EINVAL;
 
 	if (blank > FB_BLANK_POWERDOWN)
 		blank = FB_BLANK_POWERDOWN;
@@ -351,13 +353,13 @@ fb_blank(struct fb_info *info, int blank)
 	event.info = info;
 	event.data = &blank;
 
-	if (info->fbops->fb_blank)
-		ret = info->fbops->fb_blank(blank, info);
+	ret = info->fbops->fb_blank(blank, info);
+	if (ret)
+		return ret;
 
-	if (!ret)
-		fb_notifier_call_chain(FB_EVENT_BLANK, &event);
+	fb_notifier_call_chain(FB_EVENT_BLANK, &event);
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(fb_blank);
 
-- 
2.48.1

[PATCH 02/13] fbdev: Track display blanking state

[Thomas Zimmermann]

Store the display's blank status in struct fb_info.blank and track
it in fb_blank(). As an extra, the status is now available from the
sysfs blank attribute.

Support for blanking is optional. Therefore framebuffer_alloc()
initializes the state to FB_BLANK_UNBLANK (i.e., the display is
on). If the fb_blank callback has been set, register_framebuffer()
sets the state to FB_BLANK_POWERDOWN. On the first modeset, the
call to fb_blank() will update it to _UNBLANK. This is important,
as listeners to FB_EVENT_BLANK will now see the display being
switched on.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/core/fb_info.c |  1 +
 drivers/video/fbdev/core/fbmem.c   | 17 ++++++++++++++++-
 drivers/video/fbdev/core/fbsysfs.c |  8 ++++----
 include/linux/fb.h                 |  2 ++
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/core/fb_info.c b/drivers/video/fbdev/core/fb_info.c
index 4847ebe50d7d..52f9bd2c5417 100644
--- a/drivers/video/fbdev/core/fb_info.c
+++ b/drivers/video/fbdev/core/fb_info.c
@@ -42,6 +42,7 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
 
 	info->device = dev;
 	info->fbcon_rotate_hint = -1;
+	info->blank = FB_BLANK_UNBLANK;
 
 #if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 	mutex_init(&info->bl_curve_mutex);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 39e2b81473ad..f34a80c7fc3a 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -341,6 +341,7 @@ EXPORT_SYMBOL(fb_set_var);
 
 int fb_blank(struct fb_info *info, int blank)
 {
+	int old_blank = info->blank;
 	struct fb_event event;
 	int ret;
 
@@ -353,13 +354,19 @@ int fb_blank(struct fb_info *info, int blank)
 	event.info = info;
 	event.data = &blank;
 
+	info->blank = blank;
+
 	ret = info->fbops->fb_blank(blank, info);
 	if (ret)
-		return ret;
+		goto err;
 
 	fb_notifier_call_chain(FB_EVENT_BLANK, &event);
 
 	return 0;
+
+err:
+	info->blank = old_blank;
+	return ret;
 }
 EXPORT_SYMBOL(fb_blank);
 
@@ -408,6 +415,14 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	mutex_init(&fb_info->lock);
 	mutex_init(&fb_info->mm_lock);
 
+	/*
+	 * With an fb_blank callback present, we assume that the
+	 * display blank, so that fb_blank() enables it on the first
+	 * modeset.
+	 */
+	if (fb_info->fbops->fb_blank)
+		fb_info->blank = FB_BLANK_POWERDOWN;
+
 	fb_device_create(fb_info);
 
 	if (fb_info->pixmap.addr == NULL) {
diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index 1b3c9958ef5c..e337660bce46 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -242,11 +242,11 @@ static ssize_t store_blank(struct device *device,
 	return count;
 }
 
-static ssize_t show_blank(struct device *device,
-			  struct device_attribute *attr, char *buf)
+static ssize_t show_blank(struct device *device, struct device_attribute *attr, char *buf)
 {
-//	struct fb_info *fb_info = dev_get_drvdata(device);
-	return 0;
+	struct fb_info *fb_info = dev_get_drvdata(device);
+
+	return sysfs_emit(buf, "%d\n", fb_info->blank);
 }
 
 static ssize_t store_console(struct device *device,
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 5ba187e08cf7..f41d3334ac23 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -471,6 +471,8 @@ struct fb_info {
 	struct list_head modelist;      /* mode list */
 	struct fb_videomode *mode;	/* current mode */
 
+	int blank; /* current blanking; see FB_BLANK_ constants */
+
 #if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 	/* assigned backlight device */
 	/* set before framebuffer registration,
-- 
2.48.1

[PATCH 03/13] fbdev: Send old blank state in FB_EVENT_BLANK

[Thomas Zimmermann]

The event FB_EVENT_BLANK sends the new blank state in the event's
data field. Also send the old state. It's an additional field in the
data array; existing receivers won't notice the difference.

The backlight subsystem currently tracks blank state per display per
backlight. That is not optional as it ties backlight code to fbdev. A
subsystem should also not track internal state of another subsystem.
With both, new and old, blank state in FB_EVENT_BLANK, the backlight
code will not require its own state tracker any longer.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/core/fbmem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index f34a80c7fc3a..997f0bfcdbb6 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -343,6 +343,7 @@ 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)
@@ -351,8 +352,10 @@ 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 = &blank;
+	event.data = data;
 
 	info->blank = blank;
 
-- 
2.48.1

[PATCH 04/13] backlight: Implement fbdev tracking with blank state from event

[Thomas Zimmermann]

Look at the blank state provided by FB_EVENT_BLANK to determine
whether to enable or disable a backlight. Remove the tracking fields
from struct backlight_device.

Tracking requires three variables, fb_on, prev_fb_on and the
backlight's use_count. If fb_on is true, the display has been
unblanked. The backlight needs to be enabled if the display was
blanked before (i.e., prev_fb_on is false) or if use_count is still
at 0. If fb_on is false, the display has been blanked. In this case,
the backlight has to be disabled was unblanked before and the
backlight's use_count is greater than 0.

This change removes fbdev state tracking from blacklight. All the
backlight requires it its own use counter and information about
changes to the display. Removing fbdev internals makes  backlight
drivers easier to integrate into other display drivers, such as DRM.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/backlight/backlight.c | 14 +++++++-------
 include/linux/backlight.h           | 10 +---------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index f699e5827ccb..bb01f57c4683 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -98,9 +98,9 @@ static int fb_notifier_callback(struct notifier_block *self,
 	struct backlight_device *bd;
 	struct fb_event *evdata = data;
 	struct fb_info *info = evdata->info;
+	const int *fb_blank = evdata->data;
 	struct backlight_device *fb_bd = fb_bl_device(info);
-	int node = info->node;
-	int fb_blank = 0;
+	bool fb_on, prev_fb_on;
 
 	/* If we aren't interested in this event, skip it immediately ... */
 	if (event != FB_EVENT_BLANK)
@@ -116,15 +116,15 @@ static int fb_notifier_callback(struct notifier_block *self,
 	if (fb_bd && fb_bd != bd)
 		goto out;
 
-	fb_blank = *(int *)evdata->data;
-	if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
-		bd->fb_bl_on[node] = true;
+	fb_on = fb_blank[0] == FB_BLANK_UNBLANK;
+	prev_fb_on = fb_blank[1] == FB_BLANK_UNBLANK;
+
+	if (fb_on && (!prev_fb_on || !bd->use_count)) {
 		if (!bd->use_count++) {
 			bd->props.state &= ~BL_CORE_FBBLANK;
 			backlight_update_status(bd);
 		}
-	} else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) {
-		bd->fb_bl_on[node] = false;
+	} else if (!fb_on && prev_fb_on && bd->use_count) {
 		if (!(--bd->use_count)) {
 			bd->props.state |= BL_CORE_FBBLANK;
 			backlight_update_status(bd);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index f5652e5a9060..03723a5478f8 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -294,15 +294,7 @@ struct backlight_device {
 	struct device dev;
 
 	/**
-	 * @fb_bl_on: The state of individual fbdev's.
-	 *
-	 * Multiple fbdev's may share one backlight device. The fb_bl_on
-	 * records the state of the individual fbdev.
-	 */
-	bool fb_bl_on[FB_MAX];
-
-	/**
-	 * @use_count: The number of uses of fb_bl_on.
+	 * @use_count: The number of unblanked displays.
 	 */
 	int use_count;
 };
-- 
2.48.1

[PATCH 05/13] backlight: Move blank-state handling into helper

[Thomas Zimmermann]

Move the handling of blank-state updates into a separate helper,
so that is can be called without the fbdev event. No functional
changes.

As a minor improvement over the original code, the update replaces
manual locking with a guard.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/backlight/backlight.c | 46 +++++++++++++++++------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bb01f57c4683..1c43f579396f 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -80,6 +80,30 @@ static const char *const backlight_scale_types[] = {
 
 #if defined(CONFIG_FB_CORE) || (defined(CONFIG_FB_CORE_MODULE) && \
 				defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
+static void backlight_notify_blank(struct backlight_device *bd,
+				   struct device *display_dev,
+				   bool fb_on, bool prev_fb_on)
+{
+	guard(mutex)(&bd->ops_lock);
+
+	if (!bd->ops)
+		return;
+	if (bd->ops->controls_device && !bd->ops->controls_device(bd, display_dev))
+		return;
+
+	if (fb_on && (!prev_fb_on || !bd->use_count)) {
+		if (!bd->use_count++) {
+			bd->props.state &= ~BL_CORE_FBBLANK;
+			backlight_update_status(bd);
+		}
+	} else if (!fb_on && prev_fb_on && bd->use_count) {
+		if (!(--bd->use_count)) {
+			bd->props.state |= BL_CORE_FBBLANK;
+			backlight_update_status(bd);
+		}
+	}
+}
+
 /*
  * fb_notifier_callback
  *
@@ -107,31 +131,15 @@ static int fb_notifier_callback(struct notifier_block *self,
 		return 0;
 
 	bd = container_of(self, struct backlight_device, fb_notif);
-	mutex_lock(&bd->ops_lock);
 
-	if (!bd->ops)
-		goto out;
-	if (bd->ops->controls_device && !bd->ops->controls_device(bd, info->device))
-		goto out;
 	if (fb_bd && fb_bd != bd)
-		goto out;
+		return 0;
 
 	fb_on = fb_blank[0] == FB_BLANK_UNBLANK;
 	prev_fb_on = fb_blank[1] == FB_BLANK_UNBLANK;
 
-	if (fb_on && (!prev_fb_on || !bd->use_count)) {
-		if (!bd->use_count++) {
-			bd->props.state &= ~BL_CORE_FBBLANK;
-			backlight_update_status(bd);
-		}
-	} else if (!fb_on && prev_fb_on && bd->use_count) {
-		if (!(--bd->use_count)) {
-			bd->props.state |= BL_CORE_FBBLANK;
-			backlight_update_status(bd);
-		}
-	}
-out:
-	mutex_unlock(&bd->ops_lock);
+	backlight_notify_blank(bd, info->device, fb_on, prev_fb_on);
+
 	return 0;
 }
 
-- 
2.48.1

[PATCH 06/13] backlight: Replace fb events with a dedicated function call

[Thomas Zimmermann]

Remove support for fb events from backlight subsystem. Provide the
helper backlight_notify_blank_all() instead. Also export the existing
helper backlight_notify_blank() to update a single backlight device.

In fbdev, call either helper to inform the backlight subsystem of
changes to a display's blank state. If the framebuffer device has a
specific backlight, only update this one; otherwise update all.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/backlight/backlight.c     | 85 ++++---------------------
 drivers/video/fbdev/core/fb_backlight.c | 12 ++++
 drivers/video/fbdev/core/fbmem.c        |  2 +
 include/linux/backlight.h               | 12 ++--
 include/linux/fb.h                      |  4 ++
 5 files changed, 36 insertions(+), 79 deletions(-)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 1c43f579396f..9dc93c5e480b 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -15,7 +15,6 @@
 #include <linux/notifier.h>
 #include <linux/ctype.h>
 #include <linux/err.h>
-#include <linux/fb.h>
 #include <linux/slab.h>
 
 #ifdef CONFIG_PMAC_BACKLIGHT
@@ -57,10 +56,10 @@
  * a hot-key to adjust backlight, the driver must notify the backlight
  * core that brightness has changed using backlight_force_update().
  *
- * The backlight driver core receives notifications from fbdev and
- * if the event is FB_EVENT_BLANK and if the value of blank, from the
- * FBIOBLANK ioctrl, results in a change in the backlight state the
- * update_status() operation is called.
+ * Display drives can control the backlight device's status using
+ * backlight_notify_blank() and backlight_notify_blank_all(). If this
+ * results in a change in the backlight state the functions call the
+ * update_status() operation.
  */
 
 static struct list_head backlight_dev_list;
@@ -78,11 +77,8 @@ static const char *const backlight_scale_types[] = {
 	[BACKLIGHT_SCALE_NON_LINEAR]	= "non-linear",
 };
 
-#if defined(CONFIG_FB_CORE) || (defined(CONFIG_FB_CORE_MODULE) && \
-				defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
-static void backlight_notify_blank(struct backlight_device *bd,
-				   struct device *display_dev,
-				   bool fb_on, bool prev_fb_on)
+void backlight_notify_blank(struct backlight_device *bd, struct device *display_dev,
+			    bool fb_on, bool prev_fb_on)
 {
 	guard(mutex)(&bd->ops_lock);
 
@@ -103,68 +99,18 @@ static void backlight_notify_blank(struct backlight_device *bd,
 		}
 	}
 }
+EXPORT_SYMBOL(backlight_notify_blank);
 
-/*
- * fb_notifier_callback
- *
- * This callback gets called when something important happens inside a
- * framebuffer driver. The backlight core only cares about FB_BLANK_UNBLANK
- * which is reported to the driver using backlight_update_status()
- * as a state change.
- *
- * There may be several fbdev's connected to the backlight device,
- * in which case they are kept track of. A state change is only reported
- * if there is a change in backlight for the specified fbdev.
- */
-static int fb_notifier_callback(struct notifier_block *self,
-				unsigned long event, void *data)
+void backlight_notify_blank_all(struct device *display_dev, bool fb_on, bool prev_fb_on)
 {
 	struct backlight_device *bd;
-	struct fb_event *evdata = data;
-	struct fb_info *info = evdata->info;
-	const int *fb_blank = evdata->data;
-	struct backlight_device *fb_bd = fb_bl_device(info);
-	bool fb_on, prev_fb_on;
-
-	/* If we aren't interested in this event, skip it immediately ... */
-	if (event != FB_EVENT_BLANK)
-		return 0;
-
-	bd = container_of(self, struct backlight_device, fb_notif);
-
-	if (fb_bd && fb_bd != bd)
-		return 0;
-
-	fb_on = fb_blank[0] == FB_BLANK_UNBLANK;
-	prev_fb_on = fb_blank[1] == FB_BLANK_UNBLANK;
-
-	backlight_notify_blank(bd, info->device, fb_on, prev_fb_on);
-
-	return 0;
-}
-
-static int backlight_register_fb(struct backlight_device *bd)
-{
-	memset(&bd->fb_notif, 0, sizeof(bd->fb_notif));
-	bd->fb_notif.notifier_call = fb_notifier_callback;
 
-	return fb_register_client(&bd->fb_notif);
-}
+	guard(mutex)(&backlight_dev_list_mutex);
 
-static void backlight_unregister_fb(struct backlight_device *bd)
-{
-	fb_unregister_client(&bd->fb_notif);
-}
-#else
-static inline int backlight_register_fb(struct backlight_device *bd)
-{
-	return 0;
+	list_for_each_entry(bd, &backlight_dev_list, entry)
+		backlight_notify_blank(bd, display_dev, fb_on, prev_fb_on);
 }
-
-static inline void backlight_unregister_fb(struct backlight_device *bd)
-{
-}
-#endif /* CONFIG_FB_CORE */
+EXPORT_SYMBOL(backlight_notify_blank_all);
 
 static void backlight_generate_event(struct backlight_device *bd,
 				     enum backlight_update_reason reason)
@@ -455,12 +401,6 @@ struct backlight_device *backlight_device_register(const char *name,
 		return ERR_PTR(rc);
 	}
 
-	rc = backlight_register_fb(new_bd);
-	if (rc) {
-		device_unregister(&new_bd->dev);
-		return ERR_PTR(rc);
-	}
-
 	new_bd->ops = ops;
 
 #ifdef CONFIG_PMAC_BACKLIGHT
@@ -547,7 +487,6 @@ void backlight_device_unregister(struct backlight_device *bd)
 	bd->ops = NULL;
 	mutex_unlock(&bd->ops_lock);
 
-	backlight_unregister_fb(bd);
 	device_unregister(&bd->dev);
 }
 EXPORT_SYMBOL(backlight_device_unregister);
diff --git a/drivers/video/fbdev/core/fb_backlight.c b/drivers/video/fbdev/core/fb_backlight.c
index 6fdaa9f81be9..dbed9696f4c5 100644
--- a/drivers/video/fbdev/core/fb_backlight.c
+++ b/drivers/video/fbdev/core/fb_backlight.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include <linux/backlight.h>
 #include <linux/export.h>
 #include <linux/fb.h>
 #include <linux/mutex.h>
@@ -36,4 +37,15 @@ struct backlight_device *fb_bl_device(struct fb_info *info)
 	return info->bl_dev;
 }
 EXPORT_SYMBOL(fb_bl_device);
+
+void fb_bl_notify_blank(struct fb_info *info, int old_blank)
+{
+	bool on = info->blank == FB_BLANK_UNBLANK;
+	bool prev_on = old_blank == FB_BLANK_UNBLANK;
+
+	if (info->bl_dev)
+		backlight_notify_blank(info->bl_dev, info->device, on, prev_on);
+	else
+		backlight_notify_blank_all(info->device, on, prev_on);
+}
 #endif
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 997f0bfcdbb6..993fbee602e0 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -363,6 +363,8 @@ int fb_blank(struct fb_info *info, int blank)
 	if (ret)
 		goto err;
 
+	fb_bl_notify_blank(info, old_blank);
+
 	fb_notifier_call_chain(FB_EVENT_BLANK, &event);
 
 	return 0;
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 03723a5478f8..7ed99cfd030f 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -12,7 +12,6 @@
 #include <linux/device.h>
 #include <linux/fb.h>
 #include <linux/mutex.h>
-#include <linux/notifier.h>
 #include <linux/types.h>
 
 /**
@@ -278,11 +277,6 @@ struct backlight_device {
 	 */
 	const struct backlight_ops *ops;
 
-	/**
-	 * @fb_notif: The framebuffer notifier block
-	 */
-	struct notifier_block fb_notif;
-
 	/**
 	 * @entry: List entry of all registered backlight devices
 	 */
@@ -400,6 +394,12 @@ struct backlight_device *backlight_device_get_by_type(enum backlight_type type);
 int backlight_device_set_brightness(struct backlight_device *bd,
 				    unsigned long brightness);
 
+void backlight_notify_blank(struct backlight_device *bd,
+			    struct device *display_dev,
+			    bool fb_on, bool prev_fb_on);
+void backlight_notify_blank_all(struct device *display_dev,
+				bool fb_on, bool prev_fb_on);
+
 #define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
 
 /**
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f41d3334ac23..bebf279c8bc6 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -757,11 +757,15 @@ extern void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max)
 
 #if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 struct backlight_device *fb_bl_device(struct fb_info *info);
+void fb_bl_notify_blank(struct fb_info *info, int old_blank);
 #else
 static inline struct backlight_device *fb_bl_device(struct fb_info *info)
 {
 	return NULL;
 }
+
+void fb_bl_notify_blank(struct fb_info *info, int old_blank)
+{ }
 #endif
 
 static inline struct lcd_device *fb_lcd_device(struct fb_info *info)
-- 
2.48.1

[PATCH 07/13] backlight: lcd: Maintain global list of lcd devices

[Thomas Zimmermann]

Maintain a list of lcd devices. This will replace the fbdev notifiers
that all lcd devices currently subscribe to.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/backlight/lcd.c | 11 +++++++++++
 include/linux/lcd.h           |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 3267acf8dc5b..95a371b3e8a4 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -18,6 +18,9 @@
 #include <linux/fb.h>
 #include <linux/slab.h>
 
+static struct list_head lcd_dev_list;
+static struct mutex lcd_dev_list_mutex;
+
 #if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
 			   defined(CONFIG_LCD_CLASS_DEVICE_MODULE))
 static int to_lcd_power(int fb_blank)
@@ -251,6 +254,10 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
 		return ERR_PTR(rc);
 	}
 
+	mutex_lock(&lcd_dev_list_mutex);
+	list_add(&new_ld->entry, &lcd_dev_list);
+	mutex_unlock(&lcd_dev_list_mutex);
+
 	return new_ld;
 }
 EXPORT_SYMBOL(lcd_device_register);
@@ -266,6 +273,10 @@ void lcd_device_unregister(struct lcd_device *ld)
 	if (!ld)
 		return;
 
+	mutex_lock(&lcd_dev_list_mutex);
+	list_del(&ld->entry);
+	mutex_unlock(&lcd_dev_list_mutex);
+
 	mutex_lock(&ld->ops_lock);
 	ld->ops = NULL;
 	mutex_unlock(&ld->ops_lock);
diff --git a/include/linux/lcd.h b/include/linux/lcd.h
index c3ccdff4519a..195b95edb13f 100644
--- a/include/linux/lcd.h
+++ b/include/linux/lcd.h
@@ -82,6 +82,11 @@ struct lcd_device {
 	/* The framebuffer notifier block */
 	struct notifier_block fb_notif;
 
+	/**
+	 * @entry: List entry of all registered lcd devices
+	 */
+	struct list_head entry;
+
 	struct device dev;
 };
 
-- 
2.48.1

[PATCH 08/13] backlight: lcd: Move event handling into helpers

[Thomas Zimmermann]

Move the handling of display updates to separate helper functions.
There is code for handling fbdev blank events and fbdev mode changes.
The code currently runs from fbdev event notifiers, which will be
replaced.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/backlight/lcd.c | 38 ++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 95a371b3e8a4..b2ee25060d66 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -21,6 +21,32 @@
 static struct list_head lcd_dev_list;
 static struct mutex lcd_dev_list_mutex;
 
+static void lcd_notify_blank(struct lcd_device *ld, struct device *display_dev,
+			     int power)
+{
+	guard(mutex)(&ld->ops_lock);
+
+	if (!ld->ops || !ld->ops->set_power)
+		return;
+	if (ld->ops->controls_device && !ld->ops->controls_device(ld, display_dev))
+		return;
+
+	ld->ops->set_power(ld, power);
+}
+
+static void lcd_notify_mode_change(struct lcd_device *ld, struct device *display_dev,
+				   unsigned int width, unsigned int height)
+{
+	guard(mutex)(&ld->ops_lock);
+
+	if (!ld->ops || !ld->ops->set_mode)
+		return;
+	if (ld->ops->controls_device && !ld->ops->controls_device(ld, display_dev))
+		return;
+
+	ld->ops->set_mode(ld, width, height);
+}
+
 #if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
 			   defined(CONFIG_LCD_CLASS_DEVICE_MODULE))
 static int to_lcd_power(int fb_blank)
@@ -53,25 +79,17 @@ static int fb_notifier_callback(struct notifier_block *self,
 	struct fb_info *info = evdata->info;
 	struct lcd_device *fb_lcd = fb_lcd_device(info);
 
-	guard(mutex)(&ld->ops_lock);
-
-	if (!ld->ops)
-		return 0;
-	if (ld->ops->controls_device && !ld->ops->controls_device(ld, info->device))
-		return 0;
 	if (fb_lcd && fb_lcd != ld)
 		return 0;
 
 	if (event == FB_EVENT_BLANK) {
 		int power = to_lcd_power(*(int *)evdata->data);
 
-		if (ld->ops->set_power)
-			ld->ops->set_power(ld, power);
+		lcd_notify_blank(ld, info->device, power);
 	} else {
 		const struct fb_videomode *videomode = evdata->data;
 
-		if (ld->ops->set_mode)
-			ld->ops->set_mode(ld, videomode->xres, videomode->yres);
+		lcd_notify_mode_change(ld, info->device, videomode->xres, videomode->yres);
 	}
 
 	return 0;
-- 
2.48.1

[PATCH 09/13] backlight: lcd: Replace fb events with a dedicated function call

[Thomas Zimmermann]

Remove support for fb events from the lcd subsystem. Provide the
helper lcd_notify_blank_all() instead. In fbdev, call
lcd_notify_blank_all() to inform the lcd subsystem of changes
to a display's blank state.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/backlight/lcd.c    | 90 ++++++--------------------------
 drivers/video/fbdev/core/fbmem.c | 43 +++++++++++++--
 include/linux/lcd.h              |  7 +--
 3 files changed, 60 insertions(+), 80 deletions(-)

diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index b2ee25060d66..67daa3757211 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -15,7 +15,6 @@
 #include <linux/notifier.h>
 #include <linux/ctype.h>
 #include <linux/err.h>
-#include <linux/fb.h>
 #include <linux/slab.h>
 
 static struct list_head lcd_dev_list;
@@ -34,6 +33,17 @@ static void lcd_notify_blank(struct lcd_device *ld, struct device *display_dev,
 	ld->ops->set_power(ld, power);
 }
 
+void lcd_notify_blank_all(struct device *display_dev, int power)
+{
+	struct lcd_device *ld;
+
+	guard(mutex)(&lcd_dev_list_mutex);
+
+	list_for_each_entry(ld, &lcd_dev_list, entry)
+		lcd_notify_blank(ld, display_dev, power);
+}
+EXPORT_SYMBOL(lcd_notify_blank_all);
+
 static void lcd_notify_mode_change(struct lcd_device *ld, struct device *display_dev,
 				   unsigned int width, unsigned int height)
 {
@@ -47,75 +57,16 @@ static void lcd_notify_mode_change(struct lcd_device *ld, struct device *display
 	ld->ops->set_mode(ld, width, height);
 }
 
-#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
-			   defined(CONFIG_LCD_CLASS_DEVICE_MODULE))
-static int to_lcd_power(int fb_blank)
-{
-	switch (fb_blank) {
-	case FB_BLANK_UNBLANK:
-		return LCD_POWER_ON;
-	/* deprecated; TODO: should become 'off' */
-	case FB_BLANK_NORMAL:
-		return LCD_POWER_REDUCED;
-	case FB_BLANK_VSYNC_SUSPEND:
-		return LCD_POWER_REDUCED_VSYNC_SUSPEND;
-	/* 'off' */
-	case FB_BLANK_HSYNC_SUSPEND:
-	case FB_BLANK_POWERDOWN:
-	default:
-		return LCD_POWER_OFF;
-	}
-}
-
-/* This callback gets called when something important happens inside a
- * framebuffer driver. We're looking if that important event is blanking,
- * and if it is, we're switching lcd power as well ...
- */
-static int fb_notifier_callback(struct notifier_block *self,
-				 unsigned long event, void *data)
-{
-	struct lcd_device *ld = container_of(self, struct lcd_device, fb_notif);
-	struct fb_event *evdata = data;
-	struct fb_info *info = evdata->info;
-	struct lcd_device *fb_lcd = fb_lcd_device(info);
-
-	if (fb_lcd && fb_lcd != ld)
-		return 0;
-
-	if (event == FB_EVENT_BLANK) {
-		int power = to_lcd_power(*(int *)evdata->data);
-
-		lcd_notify_blank(ld, info->device, power);
-	} else {
-		const struct fb_videomode *videomode = evdata->data;
-
-		lcd_notify_mode_change(ld, info->device, videomode->xres, videomode->yres);
-	}
-
-	return 0;
-}
-
-static int lcd_register_fb(struct lcd_device *ld)
+void lcd_notify_mode_change_all(struct device *display_dev,
+				unsigned int width, unsigned int height)
 {
-	memset(&ld->fb_notif, 0, sizeof(ld->fb_notif));
-	ld->fb_notif.notifier_call = fb_notifier_callback;
-	return fb_register_client(&ld->fb_notif);
-}
+	struct lcd_device *ld;
 
-static void lcd_unregister_fb(struct lcd_device *ld)
-{
-	fb_unregister_client(&ld->fb_notif);
-}
-#else
-static int lcd_register_fb(struct lcd_device *ld)
-{
-	return 0;
-}
+	guard(mutex)(&lcd_dev_list_mutex);
 
-static inline void lcd_unregister_fb(struct lcd_device *ld)
-{
+	list_for_each_entry(ld, &lcd_dev_list, entry)
+		lcd_notify_mode_change(ld, display_dev, width, height);
 }
-#endif /* CONFIG_FB */
 
 static ssize_t lcd_power_show(struct device *dev, struct device_attribute *attr,
 		char *buf)
@@ -266,12 +217,6 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
 		return ERR_PTR(rc);
 	}
 
-	rc = lcd_register_fb(new_ld);
-	if (rc) {
-		device_unregister(&new_ld->dev);
-		return ERR_PTR(rc);
-	}
-
 	mutex_lock(&lcd_dev_list_mutex);
 	list_add(&new_ld->entry, &lcd_dev_list);
 	mutex_unlock(&lcd_dev_list_mutex);
@@ -298,7 +243,6 @@ void lcd_device_unregister(struct lcd_device *ld)
 	mutex_lock(&ld->ops_lock);
 	ld->ops = NULL;
 	mutex_unlock(&ld->ops_lock);
-	lcd_unregister_fb(ld);
 
 	device_unregister(&ld->dev);
 }
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 993fbee602e0..fb7ca143a996 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -15,6 +15,7 @@
 #include <linux/export.h>
 #include <linux/fb.h>
 #include <linux/fbcon.h>
+#include <linux/lcd.h>
 
 #include <video/nomodeset.h>
 
@@ -220,6 +221,14 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
 	return err;
 }
 
+static void fb_lcd_notify_mode_change(struct fb_info *info,
+				      struct fb_videomode *mode)
+{
+#if IS_REACHABLE(CONFIG_LCD_CLASS_DEVICE)
+	lcd_notify_mode_change_all(info->device, mode->xres, mode->yres);
+#endif
+}
+
 int
 fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 {
@@ -227,7 +236,6 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 	u32 activate;
 	struct fb_var_screeninfo old_var;
 	struct fb_videomode mode;
-	struct fb_event event;
 	u32 unused;
 
 	if (var->activate & FB_ACTIVATE_INV_MODE) {
@@ -331,14 +339,40 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 	if (ret)
 		return ret;
 
-	event.info = info;
-	event.data = &mode;
-	fb_notifier_call_chain(FB_EVENT_MODE_CHANGE, &event);
+	fb_lcd_notify_mode_change(info, &mode);
 
 	return 0;
 }
 EXPORT_SYMBOL(fb_set_var);
 
+static void fb_lcd_notify_blank(struct fb_info *info)
+{
+#if IS_REACHABLE(CONFIG_LCD_CLASS_DEVICE)
+	int power;
+
+	switch (info->blank) {
+	case FB_BLANK_UNBLANK:
+		power = LCD_POWER_ON;
+		break;
+	/* deprecated; TODO: should become 'off' */
+	case FB_BLANK_NORMAL:
+		power = LCD_POWER_REDUCED;
+		break;
+	case FB_BLANK_VSYNC_SUSPEND:
+		power = LCD_POWER_REDUCED_VSYNC_SUSPEND;
+		break;
+	/* 'off' */
+	case FB_BLANK_HSYNC_SUSPEND:
+	case FB_BLANK_POWERDOWN:
+	default:
+		power = LCD_POWER_OFF;
+		break;
+	}
+
+	lcd_notify_blank_all(info->device, power);
+#endif
+}
+
 int fb_blank(struct fb_info *info, int blank)
 {
 	int old_blank = info->blank;
@@ -364,6 +398,7 @@ int fb_blank(struct fb_info *info, int blank)
 		goto err;
 
 	fb_bl_notify_blank(info, old_blank);
+	fb_lcd_notify_blank(info);
 
 	fb_notifier_call_chain(FB_EVENT_BLANK, &event);
 
diff --git a/include/linux/lcd.h b/include/linux/lcd.h
index 195b95edb13f..cabcc193a818 100644
--- a/include/linux/lcd.h
+++ b/include/linux/lcd.h
@@ -11,7 +11,6 @@
 
 #include <linux/device.h>
 #include <linux/mutex.h>
-#include <linux/notifier.h>
 
 #define LCD_POWER_ON			(0)
 #define LCD_POWER_REDUCED		(1) // deprecated; don't use in new code
@@ -79,8 +78,6 @@ struct lcd_device {
 	const struct lcd_ops *ops;
 	/* Serialise access to set_power method */
 	struct mutex update_lock;
-	/* The framebuffer notifier block */
-	struct notifier_block fb_notif;
 
 	/**
 	 * @entry: List entry of all registered lcd devices
@@ -130,6 +127,10 @@ extern void lcd_device_unregister(struct lcd_device *ld);
 extern void devm_lcd_device_unregister(struct device *dev,
 	struct lcd_device *ld);
 
+void lcd_notify_blank_all(struct device *display_dev, int power);
+void lcd_notify_mode_change_all(struct device *display_dev,
+				unsigned int width, unsigned int height);
+
 #define to_lcd_device(obj) container_of(obj, struct lcd_device, dev)
 
 static inline void * lcd_get_data(struct lcd_device *ld_dev)
-- 
2.48.1

[PATCH 10/13] leds: backlight trigger: Maintain global list of led backlight triggers

[Thomas Zimmermann]

Maintain a list of led backlight triggers. This will replace the
fbdev notifiers that all backlight triggers currently subscribe to.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/leds/trigger/ledtrig-backlight.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
index 487577d22cfc..c1c1aa60cf07 100644
--- a/drivers/leds/trigger/ledtrig-backlight.c
+++ b/drivers/leds/trigger/ledtrig-backlight.c
@@ -23,8 +23,13 @@ struct bl_trig_notifier {
 	int old_status;
 	struct notifier_block notifier;
 	unsigned invert;
+
+	struct list_head entry;
 };
 
+static struct list_head ledtrig_backlight_list;
+static struct mutex ledtrig_backlight_list_mutex;
+
 static int fb_notifier_callback(struct notifier_block *p,
 				unsigned long event, void *data)
 {
@@ -118,6 +123,10 @@ static int bl_trig_activate(struct led_classdev *led)
 	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);
+	mutex_unlock(&ledtrig_backlight_list_mutex);
+
 	return 0;
 }
 
@@ -125,6 +134,10 @@ static void bl_trig_deactivate(struct led_classdev *led)
 {
 	struct bl_trig_notifier *n = led_get_trigger_data(led);
 
+	mutex_lock(&ledtrig_backlight_list_mutex);
+	list_del(&n->entry);
+	mutex_unlock(&ledtrig_backlight_list_mutex);
+
 	fb_unregister_client(&n->notifier);
 	kfree(n);
 }
-- 
2.48.1

Re: [PATCH 10/13] leds: backlight trigger: Maintain global list of led backlight triggers

[Lee Jones]

On Thu, 06 Feb 2025, Thomas Zimmermann wrote:

> Maintain a list of led backlight triggers. This will replace the
> fbdev notifiers that all backlight triggers currently subscribe to.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/leds/trigger/ledtrig-backlight.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
> index 487577d22cfc..c1c1aa60cf07 100644
> --- a/drivers/leds/trigger/ledtrig-backlight.c
> +++ b/drivers/leds/trigger/ledtrig-backlight.c
> @@ -23,8 +23,13 @@ struct bl_trig_notifier {
>  	int old_status;
>  	struct notifier_block notifier;
>  	unsigned invert;
> +
> +	struct list_head entry;

You don't appear to be doing anything with the list here.

It would be better if you introduced the list when it's first utilised.

>  };
>  
> +static struct list_head ledtrig_backlight_list;
> +static struct mutex ledtrig_backlight_list_mutex;
> +
>  static int fb_notifier_callback(struct notifier_block *p,
>  				unsigned long event, void *data)
>  {
> @@ -118,6 +123,10 @@ static int bl_trig_activate(struct led_classdev *led)
>  	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);
> +	mutex_unlock(&ledtrig_backlight_list_mutex);
> +
>  	return 0;
>  }
>  
> @@ -125,6 +134,10 @@ static void bl_trig_deactivate(struct led_classdev *led)
>  {
>  	struct bl_trig_notifier *n = led_get_trigger_data(led);
>  
> +	mutex_lock(&ledtrig_backlight_list_mutex);
> +	list_del(&n->entry);
> +	mutex_unlock(&ledtrig_backlight_list_mutex);
> +
>  	fb_unregister_client(&n->notifier);
>  	kfree(n);
>  }
> -- 
> 2.48.1
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH 10/13] leds: backlight trigger: Maintain global list of led backlight triggers

[Thomas Zimmermann]

Hi

Am 11.02.25 um 15:00 schrieb Lee Jones:
> On Thu, 06 Feb 2025, Thomas Zimmermann wrote:
>
>> Maintain a list of led backlight triggers. This will replace the
>> fbdev notifiers that all backlight triggers currently subscribe to.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/leds/trigger/ledtrig-backlight.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
>> index 487577d22cfc..c1c1aa60cf07 100644
>> --- a/drivers/leds/trigger/ledtrig-backlight.c
>> +++ b/drivers/leds/trigger/ledtrig-backlight.c
>> @@ -23,8 +23,13 @@ struct bl_trig_notifier {
>>   	int old_status;
>>   	struct notifier_block notifier;
>>   	unsigned invert;
>> +
>> +	struct list_head entry;
> You don't appear to be doing anything with the list here.
>
> It would be better if you introduced the list when it's first utilised.

That's patch 12. I'll merge them.

Best regards
Thomas

>
>>   };
>>   
>> +static struct list_head ledtrig_backlight_list;
>> +static struct mutex ledtrig_backlight_list_mutex;
>> +
>>   static int fb_notifier_callback(struct notifier_block *p,
>>   				unsigned long event, void *data)
>>   {
>> @@ -118,6 +123,10 @@ static int bl_trig_activate(struct led_classdev *led)
>>   	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);
>> +	mutex_unlock(&ledtrig_backlight_list_mutex);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -125,6 +134,10 @@ static void bl_trig_deactivate(struct led_classdev *led)
>>   {
>>   	struct bl_trig_notifier *n = led_get_trigger_data(led);
>>   
>> +	mutex_lock(&ledtrig_backlight_list_mutex);
>> +	list_del(&n->entry);
>> +	mutex_unlock(&ledtrig_backlight_list_mutex);
>> +
>>   	fb_unregister_client(&n->notifier);
>>   	kfree(n);
>>   }
>> -- 
>> 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)

[PATCH 11/13] leds: backlight trigger: Move blank-state handling into helper

[Thomas Zimmermann]

Move the handling of blank-state updates into a separate helper,
so that is can be called without the fbdev event. No functional
changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/leds/trigger/ledtrig-backlight.c | 33 ++++++++++++++----------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
index c1c1aa60cf07..f9317f93483b 100644
--- a/drivers/leds/trigger/ledtrig-backlight.c
+++ b/drivers/leds/trigger/ledtrig-backlight.c
@@ -30,34 +30,39 @@ 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)
+{
+	struct led_classdev *led = n->led;
+	int new_status = !on ? BLANK : UNBLANK;
+
+	if (new_status == n->old_status)
+		return;
+
+	if ((n->old_status == UNBLANK) ^ n->invert) {
+		n->brightness = led->brightness;
+		led_set_brightness_nosleep(led, LED_OFF);
+	} else {
+		led_set_brightness_nosleep(led, n->brightness);
+	}
+
+	n->old_status = new_status;
+}
+
 static int fb_notifier_callback(struct notifier_block *p,
 				unsigned long event, void *data)
 {
 	struct bl_trig_notifier *n = container_of(p,
 					struct bl_trig_notifier, notifier);
-	struct led_classdev *led = n->led;
 	struct fb_event *fb_event = data;
 	int *blank;
-	int new_status;
 
 	/* If we aren't interested in this event, skip it immediately ... */
 	if (event != FB_EVENT_BLANK)
 		return 0;
 
 	blank = fb_event->data;
-	new_status = *blank ? BLANK : UNBLANK;
 
-	if (new_status == n->old_status)
-		return 0;
-
-	if ((n->old_status == UNBLANK) ^ n->invert) {
-		n->brightness = led->brightness;
-		led_set_brightness_nosleep(led, LED_OFF);
-	} else {
-		led_set_brightness_nosleep(led, n->brightness);
-	}
-
-	n->old_status = new_status;
+	ledtrig_backlight_blank(n, !blank[0]);
 
 	return 0;
 }
-- 
2.48.1

[PATCH 12/13] leds: backlight trigger: Replace fb events with a dedicated function call

[Thomas Zimmermann]

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@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)
 {
 	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)
+	if (info->blank == FB_BLANK_UNBLANK)
+		ledtrig_backlight_blank(true);
+	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

Re: [PATCH 12/13] leds: backlight trigger: Replace fb events with a dedicated function call

[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@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?

>  {
>  	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?
/
> +	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?

> +	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
> 

-- 
/fb_ledtrig_backlight_notify_blankLee Jones [李琼斯]

Re: [PATCH 12/13] leds: backlight trigger: Replace fb events with a dedicated function call

[Thomas Zimmermann]

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@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)

Re: [PATCH 12/13] leds: backlight trigger: Replace fb events with a dedicated function call

[Lee Jones]

On Thu, 13 Feb 2025, Thomas Zimmermann wrote:

> 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@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.

#ifdefs are not generally welcome in C-files.  Please rework it.

-- 
Lee Jones [李琼斯]

[PATCH 13/13] fbdev: Remove constants of unused events

[Thomas Zimmermann]

The constants FB_EVENT_MODE_CHANGE and FB_EVENT_BLANK are unused.
Remove them from the header file.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 include/linux/fb.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index bebf279c8bc6..c272c0fc074d 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -129,18 +129,12 @@ struct fb_cursor_user {
  * Register/unregister for framebuffer events
  */
 
-/*	The resolution of the passed in fb_info about to change */
-#define FB_EVENT_MODE_CHANGE		0x01
-
 #ifdef CONFIG_GUMSTIX_AM200EPD
 /* only used by mach-pxa/am200epd.c */
 #define FB_EVENT_FB_REGISTERED          0x05
 #define FB_EVENT_FB_UNREGISTERED        0x06
 #endif
 
-/*      A display blank is requested       */
-#define FB_EVENT_BLANK                  0x09
-
 struct fb_event {
 	struct fb_info *info;
 	void *data;
-- 
2.48.1