* [PATCH v2 10/11] leds: backlight trigger: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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.
Fbdev maintains a list of all installed notifiers. Instead of the fbdev
notifiers, maintain an internal list of led backlight triggers.
v2:
- maintain global list of led backlight triggers (Lee)
- avoid IS_REACHABLE() in source file (Lee)
- notify on changes to blank state instead of display state
- use lock guards
- initialize led list and list mutex
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/leds/trigger/ledtrig-backlight.c | 41 +++++++++---------------
drivers/video/fbdev/core/fbmem.c | 19 +++++------
include/linux/leds.h | 6 ++++
3 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
index 8e66d55a6c82..2d351de81927 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,10 +20,14 @@ struct bl_trig_notifier {
struct led_classdev *led;
int brightness;
int old_status;
- struct notifier_block notifier;
unsigned invert;
+
+ struct list_head entry;
};
+static DEFINE_MUTEX(ledtrig_backlight_list_mutex);
+static LIST_HEAD(ledtrig_backlight_list);
+
static void ledtrig_backlight_notify_blank(struct bl_trig_notifier *n, int new_status)
{
struct led_classdev *led = n->led;
@@ -42,25 +45,15 @@ static void ledtrig_backlight_notify_blank(struct bl_trig_notifier *n, int new_s
n->old_status = new_status;
}
-static int fb_notifier_callback(struct notifier_block *p,
- unsigned long event, void *data)
+void ledtrig_backlight_blank(bool blank)
{
- struct bl_trig_notifier *n = container_of(p,
- struct bl_trig_notifier, notifier);
- 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;
+ struct bl_trig_notifier *n;
+ int new_status = blank ? BLANK : UNBLANK;
- ledtrig_backlight_notify_blank(n, new_status);
+ guard(mutex)(&ledtrig_backlight_list_mutex);
- return 0;
+ list_for_each_entry(n, &ledtrig_backlight_list, entry)
+ ledtrig_backlight_notify_blank(n, new_status);
}
static ssize_t bl_trig_invert_show(struct device *dev,
@@ -106,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);
@@ -118,11 +109,9 @@ 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");
+ guard(mutex)(&ledtrig_backlight_list_mutex);
+ list_add(&n->entry, &ledtrig_backlight_list);
return 0;
}
@@ -131,7 +120,9 @@ static void bl_trig_deactivate(struct led_classdev *led)
{
struct bl_trig_notifier *n = led_get_trigger_data(led);
- fb_unregister_client(&n->notifier);
+ guard(mutex)(&ledtrig_backlight_list_mutex);
+ list_del(&n->entry);
+
kfree(n);
}
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 001662c606d7..f089f72ec75a 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>
@@ -369,11 +370,17 @@ static void fb_lcd_notify_blank(struct fb_info *info)
lcd_notify_blank_all(info->device, power);
}
+static void fb_ledtrig_backlight_notify_blank(struct fb_info *info)
+{
+ if (info->blank == FB_BLANK_UNBLANK)
+ ledtrig_backlight_blank(false);
+ else
+ ledtrig_backlight_blank(true);
+}
+
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)
@@ -382,11 +389,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);
@@ -395,8 +397,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..b3f0aa081064 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_REACHABLE(CONFIG_LEDS_TRIGGER_BACKLIGHT)
+void ledtrig_backlight_blank(bool blank);
+#else
+static inline void ledtrig_backlight_blank(bool blank) {}
+#endif
+
/*
* Generic LED platform data for describing LED names and default triggers.
*/
--
2.48.1
^ permalink raw reply related
* [PATCH v2 09/11] leds: backlight trigger: Move blank-state handling into helper
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
Move the handling of blank-state updates into a separate helper,
so that is can be called without the fbdev event. No functional
changes.
v2:
- rename helper to avoid renaming in a later patch (Lee)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/leds/trigger/ledtrig-backlight.c | 30 ++++++++++++++----------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
index 487577d22cfc..8e66d55a6c82 100644
--- a/drivers/leds/trigger/ledtrig-backlight.c
+++ b/drivers/leds/trigger/ledtrig-backlight.c
@@ -25,12 +25,28 @@ struct bl_trig_notifier {
unsigned invert;
};
+static void ledtrig_backlight_notify_blank(struct bl_trig_notifier *n, int new_status)
+{
+ struct led_classdev *led = n->led;
+
+ 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;
@@ -42,17 +58,7 @@ static int fb_notifier_callback(struct notifier_block *p,
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_notify_blank(n, new_status);
return 0;
}
--
2.48.1
^ permalink raw reply related
* [PATCH v2 07/11] backlight: lcd: Move event handling into helpers
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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 3267acf8dc5b..f57ff8bcc2fa 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -18,6 +18,32 @@
#include <linux/fb.h>
#include <linux/slab.h>
+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)
@@ -50,25 +76,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
^ permalink raw reply related
* [PATCH v2 05/11] backlight: Move blank-state handling into helper
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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
^ permalink raw reply related
* [PATCH v2 04/11] backlight: Implement fbdev tracking with blank state from event
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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
^ permalink raw reply related
* [PATCH v2 11/11] fbdev: Remove constants of unused events
From: Thomas Zimmermann @ 2025-02-26 9:32 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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
^ permalink raw reply related
* [PATCH v2 03/11] fbdev: Send old blank state in FB_EVENT_BLANK
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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 optimal as it ties backlight code to fbdev. A
subsystem should 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 5d1529d300b7..9650b641d8e8 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 = ␣
+ event.data = data;
info->blank = blank;
--
2.48.1
^ permalink raw reply related
* [PATCH v2 08/11] backlight: lcd: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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.
Fbdev maintains a list of all installed notifiers. Instead of fbdev
notifiers, maintain an internal list of lcd devices.
v2:
- maintain global list of lcd devices
- avoid IS_REACHABLE() in source file
- use lock guards
- initialize lcd list and list mutex
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/video/backlight/lcd.c | 97 ++++++++------------------------
drivers/video/fbdev/core/fbmem.c | 39 +++++++++++--
include/linux/lcd.h | 21 ++++++-
3 files changed, 78 insertions(+), 79 deletions(-)
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index f57ff8bcc2fa..b59531d7f4d6 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -15,9 +15,11 @@
#include <linux/notifier.h>
#include <linux/ctype.h>
#include <linux/err.h>
-#include <linux/fb.h>
#include <linux/slab.h>
+static DEFINE_MUTEX(lcd_dev_list_mutex);
+static LIST_HEAD(lcd_dev_list);
+
static void lcd_notify_blank(struct lcd_device *ld, struct device *display_dev,
int power)
{
@@ -31,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)
{
@@ -44,76 +57,17 @@ 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)
+void lcd_notify_mode_change_all(struct device *display_dev,
+ unsigned int width, unsigned int height)
{
- 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;
- }
-}
+ struct lcd_device *ld;
-/* 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);
- }
+ guard(mutex)(&lcd_dev_list_mutex);
- return 0;
+ list_for_each_entry(ld, &lcd_dev_list, entry)
+ lcd_notify_mode_change(ld, display_dev, width, height);
}
-static int lcd_register_fb(struct lcd_device *ld)
-{
- memset(&ld->fb_notif, 0, sizeof(ld->fb_notif));
- ld->fb_notif.notifier_call = fb_notifier_callback;
- return fb_register_client(&ld->fb_notif);
-}
-
-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;
-}
-
-static inline void lcd_unregister_fb(struct lcd_device *ld)
-{
-}
-#endif /* CONFIG_FB */
-
static ssize_t lcd_power_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -263,11 +217,8 @@ 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);
- }
+ guard(mutex)(&lcd_dev_list_mutex);
+ list_add(&new_ld->entry, &lcd_dev_list);
return new_ld;
}
@@ -284,10 +235,12 @@ void lcd_device_unregister(struct lcd_device *ld)
if (!ld)
return;
+ guard(mutex)(&lcd_dev_list_mutex);
+ list_del(&ld->entry);
+
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 c931f270ac34..001662c606d7 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,12 @@ 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)
+{
+ lcd_notify_mode_change_all(info->device, mode->xres, mode->yres);
+}
+
int
fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
{
@@ -227,7 +234,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 +337,38 @@ 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)
+{
+ 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);
+}
+
int fb_blank(struct fb_info *info, int blank)
{
int old_blank = info->blank;
@@ -364,6 +394,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 c3ccdff4519a..d4fa03722b72 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,11 @@ 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
+ */
+ struct list_head entry;
struct device dev;
};
@@ -125,6 +127,19 @@ extern void lcd_device_unregister(struct lcd_device *ld);
extern void devm_lcd_device_unregister(struct device *dev,
struct lcd_device *ld);
+#if IS_REACHABLE(CONFIG_LCD_CLASS_DEVICE)
+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);
+#else
+static inline void lcd_notify_blank_all(struct device *display_dev, int power)
+{}
+
+static inline void lcd_notify_mode_change_all(struct device *display_dev,
+ unsigned int width, unsigned int height)
+{}
+#endif
+
#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
^ permalink raw reply related
* [PATCH v2 01/11] fbdev: Rework fb_blank()
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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 = ␣
- 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
^ permalink raw reply related
* [PATCH v2 06/11] backlight: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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 9650b641d8e8..c931f270ac34 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
^ permalink raw reply related
* [PATCH v2 00/11] backlight, lcd, led: Remove fbdev dependencies
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
This series removes the remaining 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 and 8 remove fbdev event handling from lcd code and patches 9 and 10
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 test this
series.
Thomas Zimmermann (11):
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: Move event handling into helpers
backlight: lcd: Replace fb events with a dedicated function call
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 | 47 +++++-----
drivers/video/backlight/backlight.c | 93 +++++---------------
drivers/video/backlight/lcd.c | 107 +++++++++--------------
drivers/video/fbdev/core/fb_backlight.c | 12 +++
drivers/video/fbdev/core/fb_info.c | 1 +
drivers/video/fbdev/core/fbmem.c | 82 ++++++++++++++---
drivers/video/fbdev/core/fbsysfs.c | 8 +-
include/linux/backlight.h | 22 ++---
include/linux/fb.h | 12 +--
include/linux/lcd.h | 21 ++++-
include/linux/leds.h | 6 ++
11 files changed, 203 insertions(+), 208 deletions(-)
--
2.48.1
^ permalink raw reply
* [PATCH v2 02/11] fbdev: Track display blanking state
From: Thomas Zimmermann @ 2025-02-26 9:31 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
In-Reply-To: <20250226093456.147402-1-tzimmermann@suse.de>
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..5d1529d300b7 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 = ␣
+ 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 is 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
^ permalink raw reply related
* Re: [PATCH 1/3] dummycon: only build module if there are users
From: Thomas Zimmermann @ 2025-02-26 8:16 UTC (permalink / raw)
To: Arnd Bergmann, Arnd Bergmann, Greg Kroah-Hartman, Helge Deller
Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <a2c0e681-2cdf-4dc9-82fc-be35f54ff795@app.fastmail.com>
Hi
Am 26.02.25 um 08:55 schrieb Arnd Bergmann:
> On Wed, Feb 26, 2025, at 08:48, Thomas Zimmermann wrote:
>> Am 25.02.25 um 17:44 schrieb Arnd Bergmann:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> Dummycon is used as a fallback conswitchp for vgacon and fbcon
>>> in the VT code, and there are no references to it if all three
>>> are disabled, so just leave it out of the kernel image for
>>> configurations without those.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>> drivers/video/console/Kconfig | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
>>> index bc31db6ef7d2..1c4263c164ce 100644
>>> --- a/drivers/video/console/Kconfig
>>> +++ b/drivers/video/console/Kconfig
>>> @@ -47,8 +47,7 @@ config SGI_NEWPORT_CONSOLE
>>> card of your Indy. Most people say Y here.
>>>
>>> config DUMMY_CONSOLE
>>> - bool
>>> - default y
>>> + def_bool VT || VGA_CONSOLE || FRAMEBUFFER_CONSOLE
>> What about MDA_CONSOLE and STI_CONSOLE. Don't they require this as fallback?
>>
> MDA_CONSOLE clearly does not, because that is only the second
> console when VGA_CONSOLE is the main one.
>
> For sti_console, I don't see how it would do use it: when CONFIG_VT
> is enabled, the line above turns on DUMMY_CONSOLE, but without
> CONFIG_VT there seems to be no reference to it after
> 58a5c67aadde ("parisc/sticon: Always register sticon console
> driver"). I also see that CONFIG_STI_CONSOLE is a 'bool' symbol,
> so there is no dynamic loading/unloading of the driver.
Thanks.
Here's another general question. vgacon and fbcon only seem usable with
CONFIG_VT=y. Wouldn't it make sense to have them depend on CONFIG_VT=y?
dummycon could then be implemented as part of the vt code, maybe even
become a vt-internal thing. The console code is complex, so I'm probably
missing something here?
Best regards
Thomas
>
> Arnd
--
--
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)
^ permalink raw reply
* Re: [PATCH 3/3] mdacon: rework dependency list
From: Thomas Zimmermann @ 2025-02-26 8:08 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Helge Deller, Roland Kletzing,
Andrew Morton
Cc: Arnd Bergmann, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20250225164436.56654-3-arnd@kernel.org>
Am 25.02.25 um 17:44 schrieb Arnd Bergmann:
> From: Arnd Bergmann <arnd@arndb.de>
>
> mdacon has roughly the same dependencies as vgacon but expresses them
> as a negative list instead of a positive list, with the only practical
> difference being PowerPC/CHRP, which uses vga16fb instead of vgacon.
>
> The CONFIG_MDA_CONSOLE description advises to only turn it on when vgacon
> is also used because MDA/Hercules-only systems should be using vgacon
> instead, so just change the list to enforce that directly for simplicity.
>
> The probing was broken from 2002 to 2008, this improves on the fix
> that was added then: If vgacon is a loadable module, then mdacon
> cannot be built-in now, and the list of systems that support vgacon
> is carried over.
>
> Fixes: 0b9cf3aa6b1e ("mdacon messing up default vc's - set default to vc13-16 again")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> I have no idea when the last time was that someone actually tried using
> dualhead vgacon/mdacon with two ISA cards, or if it still works. We may
> be better off removing the driver altogether, but I don't see anything
> immediately wrong it with it.
> ---
> drivers/video/console/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index ea4863919eb9..12f54480f57f 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -24,7 +24,7 @@ config VGA_CONSOLE
> Say Y.
>
> config MDA_CONSOLE
> - depends on !M68K && !PARISC && ISA
> + depends on VGA_CONSOLE && ISA
> tristate "MDA text console (dual-headed)"
> help
> Say Y here if you have an old MDA or monochrome Hercules graphics
--
--
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)
^ permalink raw reply
* Re: [PATCH 1/3] dummycon: only build module if there are users
From: Arnd Bergmann @ 2025-02-26 7:55 UTC (permalink / raw)
To: Thomas Zimmermann, Arnd Bergmann, Greg Kroah-Hartman,
Helge Deller
Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <4d047af3-fd30-4fa4-aa3d-c0359856d750@suse.de>
On Wed, Feb 26, 2025, at 08:48, Thomas Zimmermann wrote:
> Am 25.02.25 um 17:44 schrieb Arnd Bergmann:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> Dummycon is used as a fallback conswitchp for vgacon and fbcon
>> in the VT code, and there are no references to it if all three
>> are disabled, so just leave it out of the kernel image for
>> configurations without those.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> drivers/video/console/Kconfig | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
>> index bc31db6ef7d2..1c4263c164ce 100644
>> --- a/drivers/video/console/Kconfig
>> +++ b/drivers/video/console/Kconfig
>> @@ -47,8 +47,7 @@ config SGI_NEWPORT_CONSOLE
>> card of your Indy. Most people say Y here.
>>
>> config DUMMY_CONSOLE
>> - bool
>> - default y
>> + def_bool VT || VGA_CONSOLE || FRAMEBUFFER_CONSOLE
>
> What about MDA_CONSOLE and STI_CONSOLE. Don't they require this as fallback?
>
MDA_CONSOLE clearly does not, because that is only the second
console when VGA_CONSOLE is the main one.
For sti_console, I don't see how it would do use it: when CONFIG_VT
is enabled, the line above turns on DUMMY_CONSOLE, but without
CONFIG_VT there seems to be no reference to it after
58a5c67aadde ("parisc/sticon: Always register sticon console
driver"). I also see that CONFIG_STI_CONSOLE is a 'bool' symbol,
so there is no dynamic loading/unloading of the driver.
Arnd
^ permalink raw reply
* Re: [PATCH 2/3] dummycon: fix default rows/cols
From: Thomas Zimmermann @ 2025-02-26 7:54 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Helge Deller, Arnd Bergmann,
Javier Martinez Canillas
Cc: kernel test robot, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20250225164436.56654-2-arnd@kernel.org>
Am 25.02.25 um 17:44 schrieb Arnd Bergmann:
> From: Arnd Bergmann <arnd@arndb.de>
>
> dummycon fails to build on ARM/footbridge when the VGA console is
> disabled, since I got the dependencies slightly wrong in a previous
> patch:
>
> drivers/video/console/dummycon.c: In function 'dummycon_init':
> drivers/video/console/dummycon.c:27:25: error: 'CONFIG_DUMMY_CONSOLE_COLUMNS' undeclared (first use in this function); did you mean 'CONFIG_DUMMY_CONSOLE'?
> 27 | #define DUMMY_COLUMNS CONFIG_DUMMY_CONSOLE_COLUMNS
> drivers/video/console/dummycon.c:28:25: error: 'CONFIG_DUMMY_CONSOLE_ROWS' undeclared (first use in this function); did you mean 'CONFIG_DUMMY_CONSOLE'?
> 28 | #define DUMMY_ROWS CONFIG_DUMMY_CONSOLE_ROWS
>
> This only showed up after many thousand randconfig builds on Arm, and
> doesn't matter in practice, but should still be fixed. Address it by
> using the default row/columns on footbridge after all in that corner
> case.
>
> Fixes: 4293b0925149 ("dummycon: limit Arm console size hack to footbridge")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202409151512.LML1slol-lkp@intel.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/video/console/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index 1c4263c164ce..ea4863919eb9 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -51,7 +51,7 @@ config DUMMY_CONSOLE
>
> config DUMMY_CONSOLE_COLUMNS
> int "Initial number of console screen columns"
> - depends on DUMMY_CONSOLE && !ARCH_FOOTBRIDGE
> + depends on DUMMY_CONSOLE && !(ARCH_FOOTBRIDGE && VGA_CONSOLE)
> default 160 if PARISC
> default 80
> help
> @@ -61,7 +61,7 @@ config DUMMY_CONSOLE_COLUMNS
>
> config DUMMY_CONSOLE_ROWS
> int "Initial number of console screen rows"
> - depends on DUMMY_CONSOLE && !ARCH_FOOTBRIDGE
> + depends on DUMMY_CONSOLE && !(ARCH_FOOTBRIDGE && VGA_CONSOLE)
> default 64 if PARISC
> default 30 if ARM
> default 25
--
--
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)
^ permalink raw reply
* Re: [PATCH 1/3] dummycon: only build module if there are users
From: Thomas Zimmermann @ 2025-02-26 7:53 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Helge Deller
Cc: Arnd Bergmann, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <4d047af3-fd30-4fa4-aa3d-c0359856d750@suse.de>
Am 26.02.25 um 08:48 schrieb Thomas Zimmermann:
> Hi Arnd
>
> Am 25.02.25 um 17:44 schrieb Arnd Bergmann:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> Dummycon is used as a fallback conswitchp for vgacon and fbcon
>> in the VT code, and there are no references to it if all three
>> are disabled, so just leave it out of the kernel image for
>> configurations without those.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>> drivers/video/console/Kconfig | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/console/Kconfig
>> b/drivers/video/console/Kconfig
>> index bc31db6ef7d2..1c4263c164ce 100644
>> --- a/drivers/video/console/Kconfig
>> +++ b/drivers/video/console/Kconfig
>> @@ -47,8 +47,7 @@ config SGI_NEWPORT_CONSOLE
>> card of your Indy. Most people say Y here.
>> config DUMMY_CONSOLE
>> - bool
>> - default y
>> + def_bool VT || VGA_CONSOLE || FRAMEBUFFER_CONSOLE
>
> What about MDA_CONSOLE and STI_CONSOLE. Don't they require this as
> fallback?
I did some grepping in the kernel sources and found the answer to this
question. It's still surprising that the other consoles don't use
dummycon. They are never unloaded, it seems.
Best regards
Thomas
>
> Best regards
> Thomas
>
>> config DUMMY_CONSOLE_COLUMNS
>> int "Initial number of console screen columns"
>
--
--
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)
^ permalink raw reply
* Re: [PATCH 1/3] dummycon: only build module if there are users
From: Thomas Zimmermann @ 2025-02-26 7:48 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Helge Deller
Cc: Arnd Bergmann, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20250225164436.56654-1-arnd@kernel.org>
Hi Arnd
Am 25.02.25 um 17:44 schrieb Arnd Bergmann:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Dummycon is used as a fallback conswitchp for vgacon and fbcon
> in the VT code, and there are no references to it if all three
> are disabled, so just leave it out of the kernel image for
> configurations without those.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/video/console/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index bc31db6ef7d2..1c4263c164ce 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -47,8 +47,7 @@ config SGI_NEWPORT_CONSOLE
> card of your Indy. Most people say Y here.
>
> config DUMMY_CONSOLE
> - bool
> - default y
> + def_bool VT || VGA_CONSOLE || FRAMEBUFFER_CONSOLE
What about MDA_CONSOLE and STI_CONSOLE. Don't they require this as fallback?
Best regards
Thomas
>
> config DUMMY_CONSOLE_COLUMNS
> int "Initial number of console screen columns"
--
--
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)
^ permalink raw reply
* Re: [PATCH v2] fbdev: hyperv_fb: Allow graceful removal of framebuffer
From: Saurabh Singh Sengar @ 2025-02-26 3:58 UTC (permalink / raw)
To: Michael Kelley
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, deller@gmx.de, akpm@linux-foundation.org,
linux-hyperv@vger.kernel.org, linux-fbdev@vger.kernel.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
ssengar@microsoft.com
In-Reply-To: <SN6PR02MB41574CD095A292D20AD6C84ED4C32@SN6PR02MB4157.namprd02.prod.outlook.com>
On Tue, Feb 25, 2025 at 04:46:12PM +0000, Michael Kelley wrote:
> From: Saurabh Sengar <ssengar@linux.microsoft.com>
> >
> > When a Hyper-V framebuffer device is unbind, hyperv_fb driver tries to
> > release the framebuffer forcefully. If this framebuffer is in use it
> > produce the following WARN and hence this framebuffer is never released.
> >
> > [ 44.111220] WARNING: CPU: 35 PID: 1882 at drivers/video/fbdev/core/fb_info.c:70
> > framebuffer_release+0x2c/0x40
> > < snip >
> > [ 44.111289] Call Trace:
> > [ 44.111290] <TASK>
> > [ 44.111291] ? show_regs+0x6c/0x80
> > [ 44.111295] ? __warn+0x8d/0x150
> > [ 44.111298] ? framebuffer_release+0x2c/0x40
> > [ 44.111300] ? report_bug+0x182/0x1b0
> > [ 44.111303] ? handle_bug+0x6e/0xb0
> > [ 44.111306] ? exc_invalid_op+0x18/0x80
> > [ 44.111308] ? asm_exc_invalid_op+0x1b/0x20
> > [ 44.111311] ? framebuffer_release+0x2c/0x40
> > [ 44.111313] ? hvfb_remove+0x86/0xa0 [hyperv_fb]
> > [ 44.111315] vmbus_remove+0x24/0x40 [hv_vmbus]
> > [ 44.111323] device_remove+0x40/0x80
> > [ 44.111325] device_release_driver_internal+0x20b/0x270
> > [ 44.111327] ? bus_find_device+0xb3/0xf0
> >
> > Fix this by moving the release of framebuffer and assosiated memory
> > to fb_ops.fb_destroy function, so that framebuffer framework handles
> > it gracefully.
> >
> > While we fix this, also replace manual registrations/unregistration of
> > framebuffer with devm_register_framebuffer.
> >
> > Fixes: 68a2d20b79b1 ("drivers/video: add Hyper-V Synthetic Video Frame Buffer
> > Driver")
> > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > ---
> > V2 : Move hvfb_putmem into destroy()
> >
> > drivers/video/fbdev/hyperv_fb.c | 28 ++++++++++++++++++++++------
> > 1 file changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> > index 363e4ccfcdb7..89ee49f1c3dc 100644
> > --- a/drivers/video/fbdev/hyperv_fb.c
> > +++ b/drivers/video/fbdev/hyperv_fb.c
> > @@ -282,6 +282,8 @@ static uint screen_depth;
> > static uint screen_fb_size;
> > static uint dio_fb_size; /* FB size for deferred IO */
> >
> > +static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info);
> > +
> > /* Send message to Hyper-V host */
> > static inline int synthvid_send(struct hv_device *hdev,
> > struct synthvid_msg *msg)
> > @@ -862,6 +864,24 @@ static void hvfb_ops_damage_area(struct fb_info *info, u32 x,
> > u32 y, u32 width,
> > hvfb_ondemand_refresh_throttle(par, x, y, width, height);
> > }
> >
> > +/*
> > + * fb_ops.fb_destroy is called by the last put_fb_info() call at the end
> > + * of unregister_framebuffer() or fb_release(). Do any cleanup related to
> > + * framebuffer here.
> > + */
> > +static void hvfb_destroy(struct fb_info *info)
> > +{
> > + struct hv_device *hdev;
> > + struct device *dev;
> > + void *driver_data = (void *)info;
> > +
> > + dev = container_of(driver_data, struct device, driver_data);
>
> I don't think the above is right. The struct fb_info was allocated
> with kzalloc() in framebuffer_alloc(). You would use container_of()
> if fb_info was embedded in a struct device, but that's not the case
> here. The driver_data field within the struct device is a pointer to the
> fb_info, but that doesn't help find the struct device.
Thanks for catching this. I should have been more careful.
>
> What does help is that info->device (not to be confused with info->dev,
> which is a different struct device) points to the struct device that
> you need here. That "device" field is set in framebuffer_alloc().
> So that's an easy fix.
Right, thanks.
>
> > + hdev = container_of(dev, struct hv_device, device);
> > +
> > + hvfb_putmem(hdev, info);
>
> Another observation: The interface to hvfb_putmem() is more
> complicated than it needs to be. The hdev argument could be
> dropped because it is used only to get the device pointer,
> which is already stored in info->device. hvfb_release_phymem()
> would also need to be updated to take a struct device instead of
> struct hv_device. But if you made those changes, then the
> container_of() to get the hdev wouldn't be needed either.
Make sense.
>
> A similar simplification could be applied to hvfb_getmem().
>
> Maybe do two patches -- the first to simplify the interfaces,
> and the second to do this patch but with reduced
> complexity because of the simpler interfaces.
Agree, let me do it in V3.
- Saurabh
>
> Michael
>
<snip>
^ permalink raw reply
* RE: [PATCH v2] fbdev: hyperv_fb: Allow graceful removal of framebuffer
From: Michael Kelley @ 2025-02-25 16:46 UTC (permalink / raw)
To: Saurabh Sengar, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, deller@gmx.de,
akpm@linux-foundation.org, linux-hyperv@vger.kernel.org,
linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Cc: ssengar@microsoft.com
In-Reply-To: <1740473808-9754-1-git-send-email-ssengar@linux.microsoft.com>
From: Saurabh Sengar <ssengar@linux.microsoft.com>
>
> When a Hyper-V framebuffer device is unbind, hyperv_fb driver tries to
> release the framebuffer forcefully. If this framebuffer is in use it
> produce the following WARN and hence this framebuffer is never released.
>
> [ 44.111220] WARNING: CPU: 35 PID: 1882 at drivers/video/fbdev/core/fb_info.c:70
> framebuffer_release+0x2c/0x40
> < snip >
> [ 44.111289] Call Trace:
> [ 44.111290] <TASK>
> [ 44.111291] ? show_regs+0x6c/0x80
> [ 44.111295] ? __warn+0x8d/0x150
> [ 44.111298] ? framebuffer_release+0x2c/0x40
> [ 44.111300] ? report_bug+0x182/0x1b0
> [ 44.111303] ? handle_bug+0x6e/0xb0
> [ 44.111306] ? exc_invalid_op+0x18/0x80
> [ 44.111308] ? asm_exc_invalid_op+0x1b/0x20
> [ 44.111311] ? framebuffer_release+0x2c/0x40
> [ 44.111313] ? hvfb_remove+0x86/0xa0 [hyperv_fb]
> [ 44.111315] vmbus_remove+0x24/0x40 [hv_vmbus]
> [ 44.111323] device_remove+0x40/0x80
> [ 44.111325] device_release_driver_internal+0x20b/0x270
> [ 44.111327] ? bus_find_device+0xb3/0xf0
>
> Fix this by moving the release of framebuffer and assosiated memory
> to fb_ops.fb_destroy function, so that framebuffer framework handles
> it gracefully.
>
> While we fix this, also replace manual registrations/unregistration of
> framebuffer with devm_register_framebuffer.
>
> Fixes: 68a2d20b79b1 ("drivers/video: add Hyper-V Synthetic Video Frame Buffer
> Driver")
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> V2 : Move hvfb_putmem into destroy()
>
> drivers/video/fbdev/hyperv_fb.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 363e4ccfcdb7..89ee49f1c3dc 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -282,6 +282,8 @@ static uint screen_depth;
> static uint screen_fb_size;
> static uint dio_fb_size; /* FB size for deferred IO */
>
> +static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info);
> +
> /* Send message to Hyper-V host */
> static inline int synthvid_send(struct hv_device *hdev,
> struct synthvid_msg *msg)
> @@ -862,6 +864,24 @@ static void hvfb_ops_damage_area(struct fb_info *info, u32 x,
> u32 y, u32 width,
> hvfb_ondemand_refresh_throttle(par, x, y, width, height);
> }
>
> +/*
> + * fb_ops.fb_destroy is called by the last put_fb_info() call at the end
> + * of unregister_framebuffer() or fb_release(). Do any cleanup related to
> + * framebuffer here.
> + */
> +static void hvfb_destroy(struct fb_info *info)
> +{
> + struct hv_device *hdev;
> + struct device *dev;
> + void *driver_data = (void *)info;
> +
> + dev = container_of(driver_data, struct device, driver_data);
I don't think the above is right. The struct fb_info was allocated
with kzalloc() in framebuffer_alloc(). You would use container_of()
if fb_info was embedded in a struct device, but that's not the case
here. The driver_data field within the struct device is a pointer to the
fb_info, but that doesn't help find the struct device.
What does help is that info->device (not to be confused with info->dev,
which is a different struct device) points to the struct device that
you need here. That "device" field is set in framebuffer_alloc().
So that's an easy fix.
> + hdev = container_of(dev, struct hv_device, device);
> +
> + hvfb_putmem(hdev, info);
Another observation: The interface to hvfb_putmem() is more
complicated than it needs to be. The hdev argument could be
dropped because it is used only to get the device pointer,
which is already stored in info->device. hvfb_release_phymem()
would also need to be updated to take a struct device instead of
struct hv_device. But if you made those changes, then the
container_of() to get the hdev wouldn't be needed either.
A similar simplification could be applied to hvfb_getmem().
Maybe do two patches -- the first to simplify the interfaces,
and the second to do this patch but with reduced
complexity because of the simpler interfaces.
Michael
> + framebuffer_release(info);
> +}
> +
> /*
> * TODO: GEN1 codepaths allocate from system or DMA-able memory. Fix the
> * driver to use the _SYSMEM_ or _DMAMEM_ helpers in these cases.
> @@ -877,6 +897,7 @@ static const struct fb_ops hvfb_ops = {
> .fb_set_par = hvfb_set_par,
> .fb_setcolreg = hvfb_setcolreg,
> .fb_blank = hvfb_blank,
> + .fb_destroy = hvfb_destroy,
> };
>
> /* Get options from kernel paramenter "video=" */
> @@ -1172,7 +1193,7 @@ static int hvfb_probe(struct hv_device *hdev,
> if (ret)
> goto error;
>
> - ret = register_framebuffer(info);
> + ret = devm_register_framebuffer(&hdev->device, info);
> if (ret) {
> pr_err("Unable to register framebuffer\n");
> goto error;
> @@ -1220,14 +1241,9 @@ static void hvfb_remove(struct hv_device *hdev)
>
> fb_deferred_io_cleanup(info);
>
> - unregister_framebuffer(info);
> cancel_delayed_work_sync(&par->dwork);
>
> vmbus_close(hdev->channel);
> - hv_set_drvdata(hdev, NULL);
> -
> - hvfb_putmem(hdev, info);
> - framebuffer_release(info);
> }
>
> static int hvfb_suspend(struct hv_device *hdev)
> --
> 2.43.0
^ permalink raw reply
* [PATCH 3/3] mdacon: rework dependency list
From: Arnd Bergmann @ 2025-02-25 16:44 UTC (permalink / raw)
To: Greg Kroah-Hartman, Helge Deller, Roland Kletzing, Andrew Morton
Cc: Arnd Bergmann, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20250225164436.56654-1-arnd@kernel.org>
From: Arnd Bergmann <arnd@arndb.de>
mdacon has roughly the same dependencies as vgacon but expresses them
as a negative list instead of a positive list, with the only practical
difference being PowerPC/CHRP, which uses vga16fb instead of vgacon.
The CONFIG_MDA_CONSOLE description advises to only turn it on when vgacon
is also used because MDA/Hercules-only systems should be using vgacon
instead, so just change the list to enforce that directly for simplicity.
The probing was broken from 2002 to 2008, this improves on the fix
that was added then: If vgacon is a loadable module, then mdacon
cannot be built-in now, and the list of systems that support vgacon
is carried over.
Fixes: 0b9cf3aa6b1e ("mdacon messing up default vc's - set default to vc13-16 again")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I have no idea when the last time was that someone actually tried using
dualhead vgacon/mdacon with two ISA cards, or if it still works. We may
be better off removing the driver altogether, but I don't see anything
immediately wrong it with it.
---
drivers/video/console/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index ea4863919eb9..12f54480f57f 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -24,7 +24,7 @@ config VGA_CONSOLE
Say Y.
config MDA_CONSOLE
- depends on !M68K && !PARISC && ISA
+ depends on VGA_CONSOLE && ISA
tristate "MDA text console (dual-headed)"
help
Say Y here if you have an old MDA or monochrome Hercules graphics
--
2.39.5
^ permalink raw reply related
* [PATCH 2/3] dummycon: fix default rows/cols
From: Arnd Bergmann @ 2025-02-25 16:44 UTC (permalink / raw)
To: Greg Kroah-Hartman, Helge Deller, Thomas Zimmermann,
Arnd Bergmann, Javier Martinez Canillas
Cc: kernel test robot, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20250225164436.56654-1-arnd@kernel.org>
From: Arnd Bergmann <arnd@arndb.de>
dummycon fails to build on ARM/footbridge when the VGA console is
disabled, since I got the dependencies slightly wrong in a previous
patch:
drivers/video/console/dummycon.c: In function 'dummycon_init':
drivers/video/console/dummycon.c:27:25: error: 'CONFIG_DUMMY_CONSOLE_COLUMNS' undeclared (first use in this function); did you mean 'CONFIG_DUMMY_CONSOLE'?
27 | #define DUMMY_COLUMNS CONFIG_DUMMY_CONSOLE_COLUMNS
drivers/video/console/dummycon.c:28:25: error: 'CONFIG_DUMMY_CONSOLE_ROWS' undeclared (first use in this function); did you mean 'CONFIG_DUMMY_CONSOLE'?
28 | #define DUMMY_ROWS CONFIG_DUMMY_CONSOLE_ROWS
This only showed up after many thousand randconfig builds on Arm, and
doesn't matter in practice, but should still be fixed. Address it by
using the default row/columns on footbridge after all in that corner
case.
Fixes: 4293b0925149 ("dummycon: limit Arm console size hack to footbridge")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202409151512.LML1slol-lkp@intel.com/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/video/console/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 1c4263c164ce..ea4863919eb9 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -51,7 +51,7 @@ config DUMMY_CONSOLE
config DUMMY_CONSOLE_COLUMNS
int "Initial number of console screen columns"
- depends on DUMMY_CONSOLE && !ARCH_FOOTBRIDGE
+ depends on DUMMY_CONSOLE && !(ARCH_FOOTBRIDGE && VGA_CONSOLE)
default 160 if PARISC
default 80
help
@@ -61,7 +61,7 @@ config DUMMY_CONSOLE_COLUMNS
config DUMMY_CONSOLE_ROWS
int "Initial number of console screen rows"
- depends on DUMMY_CONSOLE && !ARCH_FOOTBRIDGE
+ depends on DUMMY_CONSOLE && !(ARCH_FOOTBRIDGE && VGA_CONSOLE)
default 64 if PARISC
default 30 if ARM
default 25
--
2.39.5
^ permalink raw reply related
* [PATCH 1/3] dummycon: only build module if there are users
From: Arnd Bergmann @ 2025-02-25 16:44 UTC (permalink / raw)
To: Greg Kroah-Hartman, Helge Deller
Cc: Arnd Bergmann, linux-fbdev, dri-devel, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
Dummycon is used as a fallback conswitchp for vgacon and fbcon
in the VT code, and there are no references to it if all three
are disabled, so just leave it out of the kernel image for
configurations without those.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/video/console/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index bc31db6ef7d2..1c4263c164ce 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -47,8 +47,7 @@ config SGI_NEWPORT_CONSOLE
card of your Indy. Most people say Y here.
config DUMMY_CONSOLE
- bool
- default y
+ def_bool VT || VGA_CONSOLE || FRAMEBUFFER_CONSOLE
config DUMMY_CONSOLE_COLUMNS
int "Initial number of console screen columns"
--
2.39.5
^ permalink raw reply related
* Re: [syzbot] [fbdev?] KASAN: slab-out-of-bounds Read in fbcon_prepare_logo
From: syzbot @ 2025-02-25 11:43 UTC (permalink / raw)
To: deller, dri-devel, linux-fbdev, linux-kernel, simona,
syzkaller-bugs
In-Reply-To: <67bd6bef.050a0220.bbfd1.009d.GAE@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: d082ecbc71e9 Linux 6.14-rc4
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14f56db0580000
kernel config: https://syzkaller.appspot.com/x/.config?x=b1635bf4c5557b92
dashboard link: https://syzkaller.appspot.com/bug?extid=0c815b25cdb3678e7083
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=172e77f8580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/323a5d590eec/disk-d082ecbc.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/f7c4b6e33fd9/vmlinux-d082ecbc.xz
kernel image: https://storage.googleapis.com/syzbot-assets/c518bbd55334/bzImage-d082ecbc.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+0c815b25cdb3678e7083@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: slab-out-of-bounds in scr_memcpyw include/linux/vt_buffer.h:38 [inline]
BUG: KASAN: slab-out-of-bounds in fbcon_prepare_logo+0xa15/0xc80 drivers/video/fbdev/core/fbcon.c:614
Read of size 256 at addr ffff888032edef60 by task syz.2.2428/8600
CPU: 0 UID: 0 PID: 8600 Comm: syz.2.2428 Not tainted 6.14.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2025
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:408 [inline]
print_report+0xc3/0x670 mm/kasan/report.c:521
kasan_report+0xd9/0x110 mm/kasan/report.c:634
check_region_inline mm/kasan/generic.c:183 [inline]
kasan_check_range+0xef/0x1a0 mm/kasan/generic.c:189
__asan_memcpy+0x23/0x60 mm/kasan/shadow.c:105
scr_memcpyw include/linux/vt_buffer.h:38 [inline]
fbcon_prepare_logo+0xa15/0xc80 drivers/video/fbdev/core/fbcon.c:614
fbcon_init+0xd41/0x1890 drivers/video/fbdev/core/fbcon.c:1146
visual_init+0x31d/0x620 drivers/tty/vt/vt.c:1011
do_bind_con_driver.isra.0+0x57a/0xbf0 drivers/tty/vt/vt.c:3831
vt_bind drivers/tty/vt/vt.c:3987 [inline]
store_bind+0x61d/0x760 drivers/tty/vt/vt.c:4059
dev_attr_store+0x55/0x80 drivers/base/core.c:2439
sysfs_kf_write+0x117/0x170 fs/sysfs/file.c:139
kernfs_fop_write_iter+0x33d/0x500 fs/kernfs/file.c:334
new_sync_write fs/read_write.c:586 [inline]
vfs_write+0x5ae/0x1150 fs/read_write.c:679
ksys_write+0x12b/0x250 fs/read_write.c:731
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fb44418d169
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe52f027e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007fb4443a5fa0 RCX: 00007fb44418d169
RDX: 0000000000000002 RSI: 0000000000000000 RDI: 0000000000000003
RBP: 00007fb44420e2a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fb4443a5fa0 R14: 00007fb4443a5fa0 R15: 0000000000000003
</TASK>
The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x32edc
head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0xfff00000000040(head|node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000000040 0000000000000000 dead000000000122 0000000000000000
raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
head: 00fff00000000040 0000000000000000 dead000000000122 0000000000000000
head: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
head: 00fff00000000002 ffffea0000cbb701 ffffffffffffffff 0000000000000000
head: 0000000000000004 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 2, migratetype Unmovable, gfp_mask 0x140dc0(GFP_USER|__GFP_COMP|__GFP_ZERO), pid 8600, tgid 8600 (syz.2.2428), ts 463971995066, free_ts 463963903452
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x181/0x1b0 mm/page_alloc.c:1551
prep_new_page mm/page_alloc.c:1559 [inline]
get_page_from_freelist+0xfce/0x2f80 mm/page_alloc.c:3477
__alloc_frozen_pages_noprof+0x221/0x2470 mm/page_alloc.c:4739
__alloc_pages_noprof+0xb/0x1b0 mm/page_alloc.c:4773
__alloc_pages_node_noprof include/linux/gfp.h:265 [inline]
alloc_pages_node_noprof include/linux/gfp.h:292 [inline]
___kmalloc_large_node+0x84/0x1b0 mm/slub.c:4239
__kmalloc_large_node_noprof+0x1c/0x70 mm/slub.c:4266
__do_kmalloc_node mm/slub.c:4282 [inline]
__kmalloc_noprof.cold+0xc/0x61 mm/slub.c:4306
kmalloc_noprof include/linux/slab.h:905 [inline]
kzalloc_noprof include/linux/slab.h:1037 [inline]
vc_do_resize+0x1e3/0x10f0 drivers/tty/vt/vt.c:1174
vc_resize include/linux/vt_kern.h:49 [inline]
fbcon_init+0xd1d/0x1890 drivers/video/fbdev/core/fbcon.c:1143
visual_init+0x31d/0x620 drivers/tty/vt/vt.c:1011
do_bind_con_driver.isra.0+0x57a/0xbf0 drivers/tty/vt/vt.c:3831
vt_bind drivers/tty/vt/vt.c:3987 [inline]
store_bind+0x61d/0x760 drivers/tty/vt/vt.c:4059
dev_attr_store+0x55/0x80 drivers/base/core.c:2439
sysfs_kf_write+0x117/0x170 fs/sysfs/file.c:139
kernfs_fop_write_iter+0x33d/0x500 fs/kernfs/file.c:334
new_sync_write fs/read_write.c:586 [inline]
vfs_write+0x5ae/0x1150 fs/read_write.c:679
page last free pid 8600 tgid 8600 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1127 [inline]
free_frozen_pages+0x6db/0xfb0 mm/page_alloc.c:2660
__folio_put+0x32a/0x450 mm/swap.c:112
vc_do_resize+0xe31/0x10f0 drivers/tty/vt/vt.c:1194
vc_resize include/linux/vt_kern.h:49 [inline]
fbcon_startup+0x406/0xb70 drivers/video/fbdev/core/fbcon.c:997
do_bind_con_driver.isra.0+0x207/0xbf0 drivers/tty/vt/vt.c:3794
vt_bind drivers/tty/vt/vt.c:3987 [inline]
store_bind+0x61d/0x760 drivers/tty/vt/vt.c:4059
dev_attr_store+0x55/0x80 drivers/base/core.c:2439
sysfs_kf_write+0x117/0x170 fs/sysfs/file.c:139
kernfs_fop_write_iter+0x33d/0x500 fs/kernfs/file.c:334
new_sync_write fs/read_write.c:586 [inline]
vfs_write+0x5ae/0x1150 fs/read_write.c:679
ksys_write+0x12b/0x250 fs/read_write.c:731
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Memory state around the buggy address:
ffff888032edef00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff888032edef80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff888032edf000: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
^
ffff888032edf080: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
ffff888032edf100: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
==================================================================
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
^ permalink raw reply
* [PATCH v2] fbdev: hyperv_fb: Allow graceful removal of framebuffer
From: Saurabh Sengar @ 2025-02-25 8:56 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, deller, akpm, linux-hyperv,
linux-fbdev, dri-devel, linux-kernel
Cc: ssengar, mhklinux
When a Hyper-V framebuffer device is unbind, hyperv_fb driver tries to
release the framebuffer forcefully. If this framebuffer is in use it
produce the following WARN and hence this framebuffer is never released.
[ 44.111220] WARNING: CPU: 35 PID: 1882 at drivers/video/fbdev/core/fb_info.c:70 framebuffer_release+0x2c/0x40
< snip >
[ 44.111289] Call Trace:
[ 44.111290] <TASK>
[ 44.111291] ? show_regs+0x6c/0x80
[ 44.111295] ? __warn+0x8d/0x150
[ 44.111298] ? framebuffer_release+0x2c/0x40
[ 44.111300] ? report_bug+0x182/0x1b0
[ 44.111303] ? handle_bug+0x6e/0xb0
[ 44.111306] ? exc_invalid_op+0x18/0x80
[ 44.111308] ? asm_exc_invalid_op+0x1b/0x20
[ 44.111311] ? framebuffer_release+0x2c/0x40
[ 44.111313] ? hvfb_remove+0x86/0xa0 [hyperv_fb]
[ 44.111315] vmbus_remove+0x24/0x40 [hv_vmbus]
[ 44.111323] device_remove+0x40/0x80
[ 44.111325] device_release_driver_internal+0x20b/0x270
[ 44.111327] ? bus_find_device+0xb3/0xf0
Fix this by moving the release of framebuffer and assosiated memory
to fb_ops.fb_destroy function, so that framebuffer framework handles
it gracefully.
While we fix this, also replace manual registrations/unregistration of
framebuffer with devm_register_framebuffer.
Fixes: 68a2d20b79b1 ("drivers/video: add Hyper-V Synthetic Video Frame Buffer Driver")
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
V2 : Move hvfb_putmem into destroy()
drivers/video/fbdev/hyperv_fb.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 363e4ccfcdb7..89ee49f1c3dc 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -282,6 +282,8 @@ static uint screen_depth;
static uint screen_fb_size;
static uint dio_fb_size; /* FB size for deferred IO */
+static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info);
+
/* Send message to Hyper-V host */
static inline int synthvid_send(struct hv_device *hdev,
struct synthvid_msg *msg)
@@ -862,6 +864,24 @@ static void hvfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width,
hvfb_ondemand_refresh_throttle(par, x, y, width, height);
}
+/*
+ * fb_ops.fb_destroy is called by the last put_fb_info() call at the end
+ * of unregister_framebuffer() or fb_release(). Do any cleanup related to
+ * framebuffer here.
+ */
+static void hvfb_destroy(struct fb_info *info)
+{
+ struct hv_device *hdev;
+ struct device *dev;
+ void *driver_data = (void *)info;
+
+ dev = container_of(driver_data, struct device, driver_data);
+ hdev = container_of(dev, struct hv_device, device);
+
+ hvfb_putmem(hdev, info);
+ framebuffer_release(info);
+}
+
/*
* TODO: GEN1 codepaths allocate from system or DMA-able memory. Fix the
* driver to use the _SYSMEM_ or _DMAMEM_ helpers in these cases.
@@ -877,6 +897,7 @@ static const struct fb_ops hvfb_ops = {
.fb_set_par = hvfb_set_par,
.fb_setcolreg = hvfb_setcolreg,
.fb_blank = hvfb_blank,
+ .fb_destroy = hvfb_destroy,
};
/* Get options from kernel paramenter "video=" */
@@ -1172,7 +1193,7 @@ static int hvfb_probe(struct hv_device *hdev,
if (ret)
goto error;
- ret = register_framebuffer(info);
+ ret = devm_register_framebuffer(&hdev->device, info);
if (ret) {
pr_err("Unable to register framebuffer\n");
goto error;
@@ -1220,14 +1241,9 @@ static void hvfb_remove(struct hv_device *hdev)
fb_deferred_io_cleanup(info);
- unregister_framebuffer(info);
cancel_delayed_work_sync(&par->dwork);
vmbus_close(hdev->channel);
- hv_set_drvdata(hdev, NULL);
-
- hvfb_putmem(hdev, info);
- framebuffer_release(info);
}
static int hvfb_suspend(struct hv_device *hdev)
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox