All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] console/fbcon: Add support for deferred console takeover
@ 2018-06-26 10:41 ` Hans de Goede
  0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2018-06-26 10:41 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: Hans de Goede, linux-fbdev, dri-devel

Currently fbcon claims fbdevs as soon as they are registered and takes over
the console as soon as the first fbdev gets registered.

This behavior is undesirable in cases where a smooth graphical bootup is
desired, in such cases we typically want the contents of the framebuffer
(typically a vendor logo) to stay in place as is.

The current solution for this problem (on embedded systems) is to not
enable fbcon.

This commit adds a new FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER config option,
which when enabled defers fbcon taking over the console from the dummy
console until the first text is displayed on the console. Together with the
"quiet" kernel commandline option, this allows fbcon to still be used
together with a smooth graphical bootup, having it take over the console as
soon as e.g. an error message is logged.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Check the whole string when checking for erases in putcs, instead of just
 the first char
-Make dummycon_blank return 1, so that a redraw gets triggered and any text
 rendered while blanked gets output so that it can trigger a deferred
 takeover if one is pending
---
 drivers/video/console/Kconfig    | 11 ++++++
 drivers/video/console/dummycon.c | 67 ++++++++++++++++++++++++++++----
 drivers/video/fbdev/core/fbcon.c | 65 +++++++++++++++++++++++++++++++
 include/linux/console.h          |  5 +++
 4 files changed, 140 insertions(+), 8 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 4110ba7d7ca9..e91edef98633 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -150,6 +150,17 @@ config FRAMEBUFFER_CONSOLE_ROTATION
          such that other users of the framebuffer will remain normally
          oriented.
 
+config FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+	bool "Framebuffer Console Deferred Takeover"
+	depends on FRAMEBUFFER_CONSOLE=y && DUMMY_CONSOLE=y
+	help
+	  If enabled this defers the framebuffer console taking over the
+	  console from the dummy console until the first text is displayed on
+	  the console. This is useful in combination with the "quiet" kernel
+	  commandline option to keep the framebuffer contents initially put up
+	  by the firmware in place, rather then replacing the contents with a
+	  black screen as soon as fbcon loads.
+
 config STI_CONSOLE
         bool "STI text console"
 	depends on PARISC && HAS_IOMEM
diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index f2eafe2ed980..45ad925ad5f8 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -26,6 +26,65 @@
 #define DUMMY_ROWS	CONFIG_DUMMY_CONSOLE_ROWS
 #endif
 
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+/* These are both protected by the console_lock */
+static RAW_NOTIFIER_HEAD(dummycon_output_nh);
+static bool dummycon_putc_called;
+
+void dummycon_register_output_notifier(struct notifier_block *nb)
+{
+	raw_notifier_chain_register(&dummycon_output_nh, nb);
+
+	if (dummycon_putc_called)
+		nb->notifier_call(nb, 0, NULL);
+}
+
+void dummycon_unregister_output_notifier(struct notifier_block *nb)
+{
+	raw_notifier_chain_unregister(&dummycon_output_nh, nb);
+}
+
+static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos)
+{
+	dummycon_putc_called = true;
+	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
+}
+
+static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
+			   int count, int ypos, int xpos)
+{
+	int i;
+
+	if (!dummycon_putc_called) {
+		/* Ignore erases */
+		for (i = 0 ; i < count; i++) {
+			if (s[i] != vc->vc_video_erase_char)
+				break;
+		}
+		if (i = count)
+			return;
+
+		dummycon_putc_called = true;
+	}
+
+	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
+}
+
+static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
+{
+	/* Redraw, so that we get putc(s) for output done while blanked */
+	return 1;
+}
+#else
+static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
+static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
+			   int count, int ypos, int xpos) { }
+static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
+{
+	return 0;
+}
+#endif
+
 static const char *dummycon_startup(void)
 {
     return "dummy device";
@@ -44,9 +103,6 @@ static void dummycon_init(struct vc_data *vc, int init)
 static void dummycon_deinit(struct vc_data *vc) { }
 static void dummycon_clear(struct vc_data *vc, int sy, int sx, int height,
 			   int width) { }
-static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
-static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
-			   int count, int ypos, int xpos) { }
 static void dummycon_cursor(struct vc_data *vc, int mode) { }
 
 static bool dummycon_scroll(struct vc_data *vc, unsigned int top,
@@ -61,11 +117,6 @@ static int dummycon_switch(struct vc_data *vc)
 	return 0;
 }
 
-static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
-{
-	return 0;
-}
-
 static int dummycon_font_set(struct vc_data *vc, struct console_font *font,
 			     unsigned int flags)
 {
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c910e74d46ff..12d947bc0f0b 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -129,6 +129,12 @@ static inline void fbcon_map_override(void)
 }
 #endif /* CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY */
 
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+static bool deferred_takeover = true;
+#else
+#define deferred_takeover false
+#endif
+
 /* font data */
 static char fontname[40];
 
@@ -499,6 +505,12 @@ static int __init fb_console_setup(char *this_opt)
 				margin_color = simple_strtoul(options, &options, 0);
 			continue;
 		}
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+		if (!strcmp(options, "nodefer")) {
+			deferred_takeover = false;
+			continue;
+		}
+#endif
 	}
 	return 1;
 }
@@ -3094,6 +3106,9 @@ static int fbcon_fb_unregistered(struct fb_info *info)
 {
 	int i, idx;
 
+	if (deferred_takeover)
+		return 0;
+
 	idx = info->node;
 	for (i = first_fb_vc; i <= last_fb_vc; i++) {
 		if (con2fb_map[i] = idx)
@@ -3131,6 +3146,14 @@ static int fbcon_fb_unregistered(struct fb_info *info)
 static void fbcon_remap_all(int idx)
 {
 	int i;
+
+	if (deferred_takeover) {
+		for (i = first_fb_vc; i <= last_fb_vc; i++)
+			con2fb_map_boot[i] = idx;
+		fbcon_map_override();
+		return;
+	}
+
 	for (i = first_fb_vc; i <= last_fb_vc; i++)
 		set_con2fb_map(i, idx, 0);
 
@@ -3180,6 +3203,11 @@ static int fbcon_fb_registered(struct fb_info *info)
 	idx = info->node;
 	fbcon_select_primary(info);
 
+	if (deferred_takeover) {
+		pr_info("fbcon: Deferring console take-over\n");
+		return 0;
+	}
+
 	if (info_idx = -1) {
 		for (i = first_fb_vc; i <= last_fb_vc; i++) {
 			if (con2fb_map_boot[i] = idx) {
@@ -3555,8 +3583,45 @@ static int fbcon_init_device(void)
 	return 0;
 }
 
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+static struct notifier_block fbcon_output_nb;
+
+/* called with console_lock held */
+static int fbcon_output_notifier(struct notifier_block *nb,
+				 unsigned long action, void *data)
+{
+	int i;
+
+	pr_info("fbcon: Taking over console\n");
+
+	dummycon_unregister_output_notifier(&fbcon_output_nb);
+	deferred_takeover = false;
+	logo_shown = FBCON_LOGO_DONTSHOW;
+
+	for (i = 0; i < FB_MAX; i++) {
+		if (registered_fb[i])
+			fbcon_fb_registered(registered_fb[i]);
+	}
+
+	return NOTIFY_OK;
+}
+
+static void fbcon_register_output_notifier(void)
+{
+	fbcon_output_nb.notifier_call = fbcon_output_notifier;
+	dummycon_register_output_notifier(&fbcon_output_nb);
+}
+#else
+static inline void fbcon_register_output_notifier(void) {}
+#endif
+
 static void fbcon_start(void)
 {
+	if (deferred_takeover) {
+		fbcon_register_output_notifier();
+		return;
+	}
+
 	if (num_registered_fb) {
 		int i;
 
diff --git a/include/linux/console.h b/include/linux/console.h
index dfd6b0e97855..f59f3dbca65c 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -21,6 +21,7 @@ struct console_font_op;
 struct console_font;
 struct module;
 struct tty_struct;
+struct notifier_block;
 
 /*
  * this is what the terminal answers to a ESC-Z or csi0c query.
@@ -220,4 +221,8 @@ static inline bool vgacon_text_force(void) { return false; }
 
 extern void console_init(void);
 
+/* For deferred console takeover */
+void dummycon_register_output_notifier(struct notifier_block *nb);
+void dummycon_unregister_output_notifier(struct notifier_block *nb);
+
 #endif /* _LINUX_CONSOLE_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2] console/fbcon: Add support for deferred console takeover
@ 2018-06-26 10:41 ` Hans de Goede
  0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2018-06-26 10:41 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: Hans de Goede, linux-fbdev, dri-devel

Currently fbcon claims fbdevs as soon as they are registered and takes over
the console as soon as the first fbdev gets registered.

This behavior is undesirable in cases where a smooth graphical bootup is
desired, in such cases we typically want the contents of the framebuffer
(typically a vendor logo) to stay in place as is.

The current solution for this problem (on embedded systems) is to not
enable fbcon.

This commit adds a new FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER config option,
which when enabled defers fbcon taking over the console from the dummy
console until the first text is displayed on the console. Together with the
"quiet" kernel commandline option, this allows fbcon to still be used
together with a smooth graphical bootup, having it take over the console as
soon as e.g. an error message is logged.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Check the whole string when checking for erases in putcs, instead of just
 the first char
-Make dummycon_blank return 1, so that a redraw gets triggered and any text
 rendered while blanked gets output so that it can trigger a deferred
 takeover if one is pending
---
 drivers/video/console/Kconfig    | 11 ++++++
 drivers/video/console/dummycon.c | 67 ++++++++++++++++++++++++++++----
 drivers/video/fbdev/core/fbcon.c | 65 +++++++++++++++++++++++++++++++
 include/linux/console.h          |  5 +++
 4 files changed, 140 insertions(+), 8 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 4110ba7d7ca9..e91edef98633 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -150,6 +150,17 @@ config FRAMEBUFFER_CONSOLE_ROTATION
          such that other users of the framebuffer will remain normally
          oriented.
 
+config FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+	bool "Framebuffer Console Deferred Takeover"
+	depends on FRAMEBUFFER_CONSOLE=y && DUMMY_CONSOLE=y
+	help
+	  If enabled this defers the framebuffer console taking over the
+	  console from the dummy console until the first text is displayed on
+	  the console. This is useful in combination with the "quiet" kernel
+	  commandline option to keep the framebuffer contents initially put up
+	  by the firmware in place, rather then replacing the contents with a
+	  black screen as soon as fbcon loads.
+
 config STI_CONSOLE
         bool "STI text console"
 	depends on PARISC && HAS_IOMEM
diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index f2eafe2ed980..45ad925ad5f8 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -26,6 +26,65 @@
 #define DUMMY_ROWS	CONFIG_DUMMY_CONSOLE_ROWS
 #endif
 
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+/* These are both protected by the console_lock */
+static RAW_NOTIFIER_HEAD(dummycon_output_nh);
+static bool dummycon_putc_called;
+
+void dummycon_register_output_notifier(struct notifier_block *nb)
+{
+	raw_notifier_chain_register(&dummycon_output_nh, nb);
+
+	if (dummycon_putc_called)
+		nb->notifier_call(nb, 0, NULL);
+}
+
+void dummycon_unregister_output_notifier(struct notifier_block *nb)
+{
+	raw_notifier_chain_unregister(&dummycon_output_nh, nb);
+}
+
+static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos)
+{
+	dummycon_putc_called = true;
+	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
+}
+
+static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
+			   int count, int ypos, int xpos)
+{
+	int i;
+
+	if (!dummycon_putc_called) {
+		/* Ignore erases */
+		for (i = 0 ; i < count; i++) {
+			if (s[i] != vc->vc_video_erase_char)
+				break;
+		}
+		if (i == count)
+			return;
+
+		dummycon_putc_called = true;
+	}
+
+	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
+}
+
+static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
+{
+	/* Redraw, so that we get putc(s) for output done while blanked */
+	return 1;
+}
+#else
+static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
+static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
+			   int count, int ypos, int xpos) { }
+static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
+{
+	return 0;
+}
+#endif
+
 static const char *dummycon_startup(void)
 {
     return "dummy device";
@@ -44,9 +103,6 @@ static void dummycon_init(struct vc_data *vc, int init)
 static void dummycon_deinit(struct vc_data *vc) { }
 static void dummycon_clear(struct vc_data *vc, int sy, int sx, int height,
 			   int width) { }
-static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
-static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
-			   int count, int ypos, int xpos) { }
 static void dummycon_cursor(struct vc_data *vc, int mode) { }
 
 static bool dummycon_scroll(struct vc_data *vc, unsigned int top,
@@ -61,11 +117,6 @@ static int dummycon_switch(struct vc_data *vc)
 	return 0;
 }
 
-static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
-{
-	return 0;
-}
-
 static int dummycon_font_set(struct vc_data *vc, struct console_font *font,
 			     unsigned int flags)
 {
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c910e74d46ff..12d947bc0f0b 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -129,6 +129,12 @@ static inline void fbcon_map_override(void)
 }
 #endif /* CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY */
 
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+static bool deferred_takeover = true;
+#else
+#define deferred_takeover false
+#endif
+
 /* font data */
 static char fontname[40];
 
@@ -499,6 +505,12 @@ static int __init fb_console_setup(char *this_opt)
 				margin_color = simple_strtoul(options, &options, 0);
 			continue;
 		}
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+		if (!strcmp(options, "nodefer")) {
+			deferred_takeover = false;
+			continue;
+		}
+#endif
 	}
 	return 1;
 }
@@ -3094,6 +3106,9 @@ static int fbcon_fb_unregistered(struct fb_info *info)
 {
 	int i, idx;
 
+	if (deferred_takeover)
+		return 0;
+
 	idx = info->node;
 	for (i = first_fb_vc; i <= last_fb_vc; i++) {
 		if (con2fb_map[i] == idx)
@@ -3131,6 +3146,14 @@ static int fbcon_fb_unregistered(struct fb_info *info)
 static void fbcon_remap_all(int idx)
 {
 	int i;
+
+	if (deferred_takeover) {
+		for (i = first_fb_vc; i <= last_fb_vc; i++)
+			con2fb_map_boot[i] = idx;
+		fbcon_map_override();
+		return;
+	}
+
 	for (i = first_fb_vc; i <= last_fb_vc; i++)
 		set_con2fb_map(i, idx, 0);
 
@@ -3180,6 +3203,11 @@ static int fbcon_fb_registered(struct fb_info *info)
 	idx = info->node;
 	fbcon_select_primary(info);
 
+	if (deferred_takeover) {
+		pr_info("fbcon: Deferring console take-over\n");
+		return 0;
+	}
+
 	if (info_idx == -1) {
 		for (i = first_fb_vc; i <= last_fb_vc; i++) {
 			if (con2fb_map_boot[i] == idx) {
@@ -3555,8 +3583,45 @@ static int fbcon_init_device(void)
 	return 0;
 }
 
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+static struct notifier_block fbcon_output_nb;
+
+/* called with console_lock held */
+static int fbcon_output_notifier(struct notifier_block *nb,
+				 unsigned long action, void *data)
+{
+	int i;
+
+	pr_info("fbcon: Taking over console\n");
+
+	dummycon_unregister_output_notifier(&fbcon_output_nb);
+	deferred_takeover = false;
+	logo_shown = FBCON_LOGO_DONTSHOW;
+
+	for (i = 0; i < FB_MAX; i++) {
+		if (registered_fb[i])
+			fbcon_fb_registered(registered_fb[i]);
+	}
+
+	return NOTIFY_OK;
+}
+
+static void fbcon_register_output_notifier(void)
+{
+	fbcon_output_nb.notifier_call = fbcon_output_notifier;
+	dummycon_register_output_notifier(&fbcon_output_nb);
+}
+#else
+static inline void fbcon_register_output_notifier(void) {}
+#endif
+
 static void fbcon_start(void)
 {
+	if (deferred_takeover) {
+		fbcon_register_output_notifier();
+		return;
+	}
+
 	if (num_registered_fb) {
 		int i;
 
diff --git a/include/linux/console.h b/include/linux/console.h
index dfd6b0e97855..f59f3dbca65c 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -21,6 +21,7 @@ struct console_font_op;
 struct console_font;
 struct module;
 struct tty_struct;
+struct notifier_block;
 
 /*
  * this is what the terminal answers to a ESC-Z or csi0c query.
@@ -220,4 +221,8 @@ static inline bool vgacon_text_force(void) { return false; }
 
 extern void console_init(void);
 
+/* For deferred console takeover */
+void dummycon_register_output_notifier(struct notifier_block *nb);
+void dummycon_unregister_output_notifier(struct notifier_block *nb);
+
 #endif /* _LINUX_CONSOLE_H */
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
  2018-06-26 10:41 ` Hans de Goede
@ 2018-06-26 12:01   ` Daniel Vetter
  -1 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2018-06-26 12:01 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-fbdev, dri-devel, Bartlomiej Zolnierkiewicz

On Tue, Jun 26, 2018 at 12:41:03PM +0200, Hans de Goede wrote:
> Currently fbcon claims fbdevs as soon as they are registered and takes over
> the console as soon as the first fbdev gets registered.
> 
> This behavior is undesirable in cases where a smooth graphical bootup is
> desired, in such cases we typically want the contents of the framebuffer
> (typically a vendor logo) to stay in place as is.
> 
> The current solution for this problem (on embedded systems) is to not
> enable fbcon.
> 
> This commit adds a new FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER config option,
> which when enabled defers fbcon taking over the console from the dummy
> console until the first text is displayed on the console. Together with the
> "quiet" kernel commandline option, this allows fbcon to still be used
> together with a smooth graphical bootup, having it take over the console as
> soon as e.g. an error message is logged.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Check the whole string when checking for erases in putcs, instead of just
>  the first char
> -Make dummycon_blank return 1, so that a redraw gets triggered and any text
>  rendered while blanked gets output so that it can trigger a deferred
>  takeover if one is pending
> ---
>  drivers/video/console/Kconfig    | 11 ++++++
>  drivers/video/console/dummycon.c | 67 ++++++++++++++++++++++++++++----
>  drivers/video/fbdev/core/fbcon.c | 65 +++++++++++++++++++++++++++++++
>  include/linux/console.h          |  5 +++
>  4 files changed, 140 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index 4110ba7d7ca9..e91edef98633 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -150,6 +150,17 @@ config FRAMEBUFFER_CONSOLE_ROTATION
>           such that other users of the framebuffer will remain normally
>           oriented.
>  
> +config FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +	bool "Framebuffer Console Deferred Takeover"
> +	depends on FRAMEBUFFER_CONSOLE=y && DUMMY_CONSOLE=y
> +	help
> +	  If enabled this defers the framebuffer console taking over the
> +	  console from the dummy console until the first text is displayed on
> +	  the console. This is useful in combination with the "quiet" kernel
> +	  commandline option to keep the framebuffer contents initially put up
> +	  by the firmware in place, rather then replacing the contents with a
> +	  black screen as soon as fbcon loads.
> +
>  config STI_CONSOLE
>          bool "STI text console"
>  	depends on PARISC && HAS_IOMEM
> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
> index f2eafe2ed980..45ad925ad5f8 100644
> --- a/drivers/video/console/dummycon.c
> +++ b/drivers/video/console/dummycon.c
> @@ -26,6 +26,65 @@
>  #define DUMMY_ROWS	CONFIG_DUMMY_CONSOLE_ROWS
>  #endif
>  
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +/* These are both protected by the console_lock */
> +static RAW_NOTIFIER_HEAD(dummycon_output_nh);
> +static bool dummycon_putc_called;
> +
> +void dummycon_register_output_notifier(struct notifier_block *nb)
> +{
> +	raw_notifier_chain_register(&dummycon_output_nh, nb);
> +
> +	if (dummycon_putc_called)
> +		nb->notifier_call(nb, 0, NULL);
> +}
> +
> +void dummycon_unregister_output_notifier(struct notifier_block *nb)
> +{
> +	raw_notifier_chain_unregister(&dummycon_output_nh, nb);
> +}
> +
> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos)
> +{
> +	dummycon_putc_called = true;
> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
> +}
> +
> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
> +			   int count, int ypos, int xpos)
> +{
> +	int i;
> +
> +	if (!dummycon_putc_called) {
> +		/* Ignore erases */
> +		for (i = 0 ; i < count; i++) {
> +			if (s[i] != vc->vc_video_erase_char)
> +				break;
> +		}
> +		if (i = count)
> +			return;
> +
> +		dummycon_putc_called = true;
> +	}
> +
> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
> +}
> +
> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
> +{
> +	/* Redraw, so that we get putc(s) for output done while blanked */
> +	return 1;
> +}
> +#else
> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
> +			   int count, int ypos, int xpos) { }
> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static const char *dummycon_startup(void)
>  {
>      return "dummy device";
> @@ -44,9 +103,6 @@ static void dummycon_init(struct vc_data *vc, int init)
>  static void dummycon_deinit(struct vc_data *vc) { }
>  static void dummycon_clear(struct vc_data *vc, int sy, int sx, int height,
>  			   int width) { }
> -static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
> -static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
> -			   int count, int ypos, int xpos) { }
>  static void dummycon_cursor(struct vc_data *vc, int mode) { }
>  
>  static bool dummycon_scroll(struct vc_data *vc, unsigned int top,
> @@ -61,11 +117,6 @@ static int dummycon_switch(struct vc_data *vc)
>  	return 0;
>  }
>  
> -static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
> -{
> -	return 0;
> -}
> -
>  static int dummycon_font_set(struct vc_data *vc, struct console_font *font,
>  			     unsigned int flags)
>  {
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index c910e74d46ff..12d947bc0f0b 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -129,6 +129,12 @@ static inline void fbcon_map_override(void)
>  }
>  #endif /* CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY */
>  
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +static bool deferred_takeover = true;
> +#else
> +#define deferred_takeover false
> +#endif
> +
>  /* font data */
>  static char fontname[40];
>  
> @@ -499,6 +505,12 @@ static int __init fb_console_setup(char *this_opt)
>  				margin_color = simple_strtoul(options, &options, 0);
>  			continue;
>  		}
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +		if (!strcmp(options, "nodefer")) {
> +			deferred_takeover = false;
> +			continue;
> +		}
> +#endif
>  	}
>  	return 1;
>  }
> @@ -3094,6 +3106,9 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>  {
>  	int i, idx;
>  
> +	if (deferred_takeover)
> +		return 0;
> +
>  	idx = info->node;
>  	for (i = first_fb_vc; i <= last_fb_vc; i++) {
>  		if (con2fb_map[i] = idx)
> @@ -3131,6 +3146,14 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>  static void fbcon_remap_all(int idx)
>  {
>  	int i;
> +
> +	if (deferred_takeover) {
> +		for (i = first_fb_vc; i <= last_fb_vc; i++)
> +			con2fb_map_boot[i] = idx;
> +		fbcon_map_override();
> +		return;
> +	}
> +
>  	for (i = first_fb_vc; i <= last_fb_vc; i++)
>  		set_con2fb_map(i, idx, 0);
>  
> @@ -3180,6 +3203,11 @@ static int fbcon_fb_registered(struct fb_info *info)
>  	idx = info->node;
>  	fbcon_select_primary(info);
>  
> +	if (deferred_takeover) {
> +		pr_info("fbcon: Deferring console take-over\n");
> +		return 0;
> +	}
> +
>  	if (info_idx = -1) {
>  		for (i = first_fb_vc; i <= last_fb_vc; i++) {
>  			if (con2fb_map_boot[i] = idx) {
> @@ -3555,8 +3583,45 @@ static int fbcon_init_device(void)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +static struct notifier_block fbcon_output_nb;
> +
> +/* called with console_lock held */
> +static int fbcon_output_notifier(struct notifier_block *nb,
> +				 unsigned long action, void *data)
> +{
> +	int i;
> +
> +	pr_info("fbcon: Taking over console\n");
> +
> +	dummycon_unregister_output_notifier(&fbcon_output_nb);
> +	deferred_takeover = false;
> +	logo_shown = FBCON_LOGO_DONTSHOW;
> +
> +	for (i = 0; i < FB_MAX; i++) {
> +		if (registered_fb[i])
> +			fbcon_fb_registered(registered_fb[i]);
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static void fbcon_register_output_notifier(void)
> +{
> +	fbcon_output_nb.notifier_call = fbcon_output_notifier;
> +	dummycon_register_output_notifier(&fbcon_output_nb);
> +}
> +#else
> +static inline void fbcon_register_output_notifier(void) {}
> +#endif
> +
>  static void fbcon_start(void)
>  {
> +	if (deferred_takeover) {
> +		fbcon_register_output_notifier();

Nothing seems to unregister the notifier in fbcon_exit. Not likely in your
use-case, but let's make sure this is correct.

Another request: Please create a console_lock_assert_held() in printk.c
(or export the existing is_console_locked to make WARN_CONSOLE_UNLOCKED
work properly in modules), and then sprinkle those into
fbcon_fb_(un)registered.  fbcon and console subsystem locking is a major
headache, I want to at least annotate stuff for when I have to review
things again.

On that note: Notifiers are a real pain, but since there's only a BKL for
everything console related the usual locking inversions between
registering and callbacks can't happen. If dummycon and fbcon would each
have their own locks, the simple notifier wouldn't work.

With the above addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +		return;
> +	}
> +
>  	if (num_registered_fb) {
>  		int i;
>  
> diff --git a/include/linux/console.h b/include/linux/console.h
> index dfd6b0e97855..f59f3dbca65c 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -21,6 +21,7 @@ struct console_font_op;
>  struct console_font;
>  struct module;
>  struct tty_struct;
> +struct notifier_block;
>  
>  /*
>   * this is what the terminal answers to a ESC-Z or csi0c query.
> @@ -220,4 +221,8 @@ static inline bool vgacon_text_force(void) { return false; }
>  
>  extern void console_init(void);
>  
> +/* For deferred console takeover */
> +void dummycon_register_output_notifier(struct notifier_block *nb);
> +void dummycon_unregister_output_notifier(struct notifier_block *nb);
> +
>  #endif /* _LINUX_CONSOLE_H */
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
@ 2018-06-26 12:01   ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2018-06-26 12:01 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-fbdev, dri-devel, Bartlomiej Zolnierkiewicz

On Tue, Jun 26, 2018 at 12:41:03PM +0200, Hans de Goede wrote:
> Currently fbcon claims fbdevs as soon as they are registered and takes over
> the console as soon as the first fbdev gets registered.
> 
> This behavior is undesirable in cases where a smooth graphical bootup is
> desired, in such cases we typically want the contents of the framebuffer
> (typically a vendor logo) to stay in place as is.
> 
> The current solution for this problem (on embedded systems) is to not
> enable fbcon.
> 
> This commit adds a new FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER config option,
> which when enabled defers fbcon taking over the console from the dummy
> console until the first text is displayed on the console. Together with the
> "quiet" kernel commandline option, this allows fbcon to still be used
> together with a smooth graphical bootup, having it take over the console as
> soon as e.g. an error message is logged.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Check the whole string when checking for erases in putcs, instead of just
>  the first char
> -Make dummycon_blank return 1, so that a redraw gets triggered and any text
>  rendered while blanked gets output so that it can trigger a deferred
>  takeover if one is pending
> ---
>  drivers/video/console/Kconfig    | 11 ++++++
>  drivers/video/console/dummycon.c | 67 ++++++++++++++++++++++++++++----
>  drivers/video/fbdev/core/fbcon.c | 65 +++++++++++++++++++++++++++++++
>  include/linux/console.h          |  5 +++
>  4 files changed, 140 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index 4110ba7d7ca9..e91edef98633 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -150,6 +150,17 @@ config FRAMEBUFFER_CONSOLE_ROTATION
>           such that other users of the framebuffer will remain normally
>           oriented.
>  
> +config FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +	bool "Framebuffer Console Deferred Takeover"
> +	depends on FRAMEBUFFER_CONSOLE=y && DUMMY_CONSOLE=y
> +	help
> +	  If enabled this defers the framebuffer console taking over the
> +	  console from the dummy console until the first text is displayed on
> +	  the console. This is useful in combination with the "quiet" kernel
> +	  commandline option to keep the framebuffer contents initially put up
> +	  by the firmware in place, rather then replacing the contents with a
> +	  black screen as soon as fbcon loads.
> +
>  config STI_CONSOLE
>          bool "STI text console"
>  	depends on PARISC && HAS_IOMEM
> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
> index f2eafe2ed980..45ad925ad5f8 100644
> --- a/drivers/video/console/dummycon.c
> +++ b/drivers/video/console/dummycon.c
> @@ -26,6 +26,65 @@
>  #define DUMMY_ROWS	CONFIG_DUMMY_CONSOLE_ROWS
>  #endif
>  
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +/* These are both protected by the console_lock */
> +static RAW_NOTIFIER_HEAD(dummycon_output_nh);
> +static bool dummycon_putc_called;
> +
> +void dummycon_register_output_notifier(struct notifier_block *nb)
> +{
> +	raw_notifier_chain_register(&dummycon_output_nh, nb);
> +
> +	if (dummycon_putc_called)
> +		nb->notifier_call(nb, 0, NULL);
> +}
> +
> +void dummycon_unregister_output_notifier(struct notifier_block *nb)
> +{
> +	raw_notifier_chain_unregister(&dummycon_output_nh, nb);
> +}
> +
> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos)
> +{
> +	dummycon_putc_called = true;
> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
> +}
> +
> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
> +			   int count, int ypos, int xpos)
> +{
> +	int i;
> +
> +	if (!dummycon_putc_called) {
> +		/* Ignore erases */
> +		for (i = 0 ; i < count; i++) {
> +			if (s[i] != vc->vc_video_erase_char)
> +				break;
> +		}
> +		if (i == count)
> +			return;
> +
> +		dummycon_putc_called = true;
> +	}
> +
> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
> +}
> +
> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
> +{
> +	/* Redraw, so that we get putc(s) for output done while blanked */
> +	return 1;
> +}
> +#else
> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
> +			   int count, int ypos, int xpos) { }
> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static const char *dummycon_startup(void)
>  {
>      return "dummy device";
> @@ -44,9 +103,6 @@ static void dummycon_init(struct vc_data *vc, int init)
>  static void dummycon_deinit(struct vc_data *vc) { }
>  static void dummycon_clear(struct vc_data *vc, int sy, int sx, int height,
>  			   int width) { }
> -static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
> -static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
> -			   int count, int ypos, int xpos) { }
>  static void dummycon_cursor(struct vc_data *vc, int mode) { }
>  
>  static bool dummycon_scroll(struct vc_data *vc, unsigned int top,
> @@ -61,11 +117,6 @@ static int dummycon_switch(struct vc_data *vc)
>  	return 0;
>  }
>  
> -static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
> -{
> -	return 0;
> -}
> -
>  static int dummycon_font_set(struct vc_data *vc, struct console_font *font,
>  			     unsigned int flags)
>  {
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index c910e74d46ff..12d947bc0f0b 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -129,6 +129,12 @@ static inline void fbcon_map_override(void)
>  }
>  #endif /* CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY */
>  
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +static bool deferred_takeover = true;
> +#else
> +#define deferred_takeover false
> +#endif
> +
>  /* font data */
>  static char fontname[40];
>  
> @@ -499,6 +505,12 @@ static int __init fb_console_setup(char *this_opt)
>  				margin_color = simple_strtoul(options, &options, 0);
>  			continue;
>  		}
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +		if (!strcmp(options, "nodefer")) {
> +			deferred_takeover = false;
> +			continue;
> +		}
> +#endif
>  	}
>  	return 1;
>  }
> @@ -3094,6 +3106,9 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>  {
>  	int i, idx;
>  
> +	if (deferred_takeover)
> +		return 0;
> +
>  	idx = info->node;
>  	for (i = first_fb_vc; i <= last_fb_vc; i++) {
>  		if (con2fb_map[i] == idx)
> @@ -3131,6 +3146,14 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>  static void fbcon_remap_all(int idx)
>  {
>  	int i;
> +
> +	if (deferred_takeover) {
> +		for (i = first_fb_vc; i <= last_fb_vc; i++)
> +			con2fb_map_boot[i] = idx;
> +		fbcon_map_override();
> +		return;
> +	}
> +
>  	for (i = first_fb_vc; i <= last_fb_vc; i++)
>  		set_con2fb_map(i, idx, 0);
>  
> @@ -3180,6 +3203,11 @@ static int fbcon_fb_registered(struct fb_info *info)
>  	idx = info->node;
>  	fbcon_select_primary(info);
>  
> +	if (deferred_takeover) {
> +		pr_info("fbcon: Deferring console take-over\n");
> +		return 0;
> +	}
> +
>  	if (info_idx == -1) {
>  		for (i = first_fb_vc; i <= last_fb_vc; i++) {
>  			if (con2fb_map_boot[i] == idx) {
> @@ -3555,8 +3583,45 @@ static int fbcon_init_device(void)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +static struct notifier_block fbcon_output_nb;
> +
> +/* called with console_lock held */
> +static int fbcon_output_notifier(struct notifier_block *nb,
> +				 unsigned long action, void *data)
> +{
> +	int i;
> +
> +	pr_info("fbcon: Taking over console\n");
> +
> +	dummycon_unregister_output_notifier(&fbcon_output_nb);
> +	deferred_takeover = false;
> +	logo_shown = FBCON_LOGO_DONTSHOW;
> +
> +	for (i = 0; i < FB_MAX; i++) {
> +		if (registered_fb[i])
> +			fbcon_fb_registered(registered_fb[i]);
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static void fbcon_register_output_notifier(void)
> +{
> +	fbcon_output_nb.notifier_call = fbcon_output_notifier;
> +	dummycon_register_output_notifier(&fbcon_output_nb);
> +}
> +#else
> +static inline void fbcon_register_output_notifier(void) {}
> +#endif
> +
>  static void fbcon_start(void)
>  {
> +	if (deferred_takeover) {
> +		fbcon_register_output_notifier();

Nothing seems to unregister the notifier in fbcon_exit. Not likely in your
use-case, but let's make sure this is correct.

Another request: Please create a console_lock_assert_held() in printk.c
(or export the existing is_console_locked to make WARN_CONSOLE_UNLOCKED
work properly in modules), and then sprinkle those into
fbcon_fb_(un)registered.  fbcon and console subsystem locking is a major
headache, I want to at least annotate stuff for when I have to review
things again.

On that note: Notifiers are a real pain, but since there's only a BKL for
everything console related the usual locking inversions between
registering and callbacks can't happen. If dummycon and fbcon would each
have their own locks, the simple notifier wouldn't work.

With the above addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +		return;
> +	}
> +
>  	if (num_registered_fb) {
>  		int i;
>  
> diff --git a/include/linux/console.h b/include/linux/console.h
> index dfd6b0e97855..f59f3dbca65c 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -21,6 +21,7 @@ struct console_font_op;
>  struct console_font;
>  struct module;
>  struct tty_struct;
> +struct notifier_block;
>  
>  /*
>   * this is what the terminal answers to a ESC-Z or csi0c query.
> @@ -220,4 +221,8 @@ static inline bool vgacon_text_force(void) { return false; }
>  
>  extern void console_init(void);
>  
> +/* For deferred console takeover */
> +void dummycon_register_output_notifier(struct notifier_block *nb);
> +void dummycon_unregister_output_notifier(struct notifier_block *nb);
> +
>  #endif /* _LINUX_CONSOLE_H */
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
  2018-06-26 10:41 ` Hans de Goede
@ 2018-06-26 13:11   ` Emil Velikov
  -1 siblings, 0 replies; 10+ messages in thread
From: Emil Velikov @ 2018-06-26 13:11 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-fbdev, ML dri-devel, Bartlomiej Zolnierkiewicz

Hi Hans,

On 26 June 2018 at 11:41, Hans de Goede <hdegoede@redhat.com> wrote:

> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +               if (!strcmp(options, "nodefer")) {
> +                       deferred_takeover = false;
> +                       continue;
> +               }
> +#endif
Can we please have some documentation about "nodefer".
I may have missed something, but it seems like the only instance of it
is in this hunk.

If you prefer, the documentation can be a separate commit.

Thanks
Emil

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
@ 2018-06-26 13:11   ` Emil Velikov
  0 siblings, 0 replies; 10+ messages in thread
From: Emil Velikov @ 2018-06-26 13:11 UTC (permalink / raw)
  To: Hans de Goede; +Cc: linux-fbdev, ML dri-devel, Bartlomiej Zolnierkiewicz

Hi Hans,

On 26 June 2018 at 11:41, Hans de Goede <hdegoede@redhat.com> wrote:

> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> +               if (!strcmp(options, "nodefer")) {
> +                       deferred_takeover = false;
> +                       continue;
> +               }
> +#endif
Can we please have some documentation about "nodefer".
I may have missed something, but it seems like the only instance of it
is in this hunk.

If you prefer, the documentation can be a separate commit.

Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
  2018-06-26 13:11   ` Emil Velikov
@ 2018-06-26 13:42     ` Hans de Goede
  -1 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2018-06-26 13:42 UTC (permalink / raw)
  To: Emil Velikov; +Cc: linux-fbdev, ML dri-devel, Bartlomiej Zolnierkiewicz

Hi,

On 26-06-18 15:11, Emil Velikov wrote:
> Hi Hans,
> 
> On 26 June 2018 at 11:41, Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +               if (!strcmp(options, "nodefer")) {
>> +                       deferred_takeover = false;
>> +                       continue;
>> +               }
>> +#endif
> Can we please have some documentation about "nodefer".
> I may have missed something, but it seems like the only instance of it
> is in this hunk.
> 
> If you prefer, the documentation can be a separate commit.

Good point, fixed for v3 (and kept in the same commit).

Regards,

Hans

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
@ 2018-06-26 13:42     ` Hans de Goede
  0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2018-06-26 13:42 UTC (permalink / raw)
  To: Emil Velikov; +Cc: linux-fbdev, ML dri-devel, Bartlomiej Zolnierkiewicz

Hi,

On 26-06-18 15:11, Emil Velikov wrote:
> Hi Hans,
> 
> On 26 June 2018 at 11:41, Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +               if (!strcmp(options, "nodefer")) {
>> +                       deferred_takeover = false;
>> +                       continue;
>> +               }
>> +#endif
> Can we please have some documentation about "nodefer".
> I may have missed something, but it seems like the only instance of it
> is in this hunk.
> 
> If you prefer, the documentation can be a separate commit.

Good point, fixed for v3 (and kept in the same commit).

Regards,

Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
  2018-06-26 12:01   ` Daniel Vetter
@ 2018-06-26 13:44     ` Hans de Goede
  -1 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2018-06-26 13:44 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-fbdev, dri-devel, Bartlomiej Zolnierkiewicz

Hi,

On 26-06-18 14:01, Daniel Vetter wrote:
> On Tue, Jun 26, 2018 at 12:41:03PM +0200, Hans de Goede wrote:
>> Currently fbcon claims fbdevs as soon as they are registered and takes over
>> the console as soon as the first fbdev gets registered.
>>
>> This behavior is undesirable in cases where a smooth graphical bootup is
>> desired, in such cases we typically want the contents of the framebuffer
>> (typically a vendor logo) to stay in place as is.
>>
>> The current solution for this problem (on embedded systems) is to not
>> enable fbcon.
>>
>> This commit adds a new FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER config option,
>> which when enabled defers fbcon taking over the console from the dummy
>> console until the first text is displayed on the console. Together with the
>> "quiet" kernel commandline option, this allows fbcon to still be used
>> together with a smooth graphical bootup, having it take over the console as
>> soon as e.g. an error message is logged.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Check the whole string when checking for erases in putcs, instead of just
>>   the first char
>> -Make dummycon_blank return 1, so that a redraw gets triggered and any text
>>   rendered while blanked gets output so that it can trigger a deferred
>>   takeover if one is pending
>> ---
>>   drivers/video/console/Kconfig    | 11 ++++++
>>   drivers/video/console/dummycon.c | 67 ++++++++++++++++++++++++++++----
>>   drivers/video/fbdev/core/fbcon.c | 65 +++++++++++++++++++++++++++++++
>>   include/linux/console.h          |  5 +++
>>   4 files changed, 140 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
>> index 4110ba7d7ca9..e91edef98633 100644
>> --- a/drivers/video/console/Kconfig
>> +++ b/drivers/video/console/Kconfig
>> @@ -150,6 +150,17 @@ config FRAMEBUFFER_CONSOLE_ROTATION
>>            such that other users of the framebuffer will remain normally
>>            oriented.
>>   
>> +config FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +	bool "Framebuffer Console Deferred Takeover"
>> +	depends on FRAMEBUFFER_CONSOLE=y && DUMMY_CONSOLE=y
>> +	help
>> +	  If enabled this defers the framebuffer console taking over the
>> +	  console from the dummy console until the first text is displayed on
>> +	  the console. This is useful in combination with the "quiet" kernel
>> +	  commandline option to keep the framebuffer contents initially put up
>> +	  by the firmware in place, rather then replacing the contents with a
>> +	  black screen as soon as fbcon loads.
>> +
>>   config STI_CONSOLE
>>           bool "STI text console"
>>   	depends on PARISC && HAS_IOMEM
>> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
>> index f2eafe2ed980..45ad925ad5f8 100644
>> --- a/drivers/video/console/dummycon.c
>> +++ b/drivers/video/console/dummycon.c
>> @@ -26,6 +26,65 @@
>>   #define DUMMY_ROWS	CONFIG_DUMMY_CONSOLE_ROWS
>>   #endif
>>   
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +/* These are both protected by the console_lock */
>> +static RAW_NOTIFIER_HEAD(dummycon_output_nh);
>> +static bool dummycon_putc_called;
>> +
>> +void dummycon_register_output_notifier(struct notifier_block *nb)
>> +{
>> +	raw_notifier_chain_register(&dummycon_output_nh, nb);
>> +
>> +	if (dummycon_putc_called)
>> +		nb->notifier_call(nb, 0, NULL);
>> +}
>> +
>> +void dummycon_unregister_output_notifier(struct notifier_block *nb)
>> +{
>> +	raw_notifier_chain_unregister(&dummycon_output_nh, nb);
>> +}
>> +
>> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos)
>> +{
>> +	dummycon_putc_called = true;
>> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
>> +}
>> +
>> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
>> +			   int count, int ypos, int xpos)
>> +{
>> +	int i;
>> +
>> +	if (!dummycon_putc_called) {
>> +		/* Ignore erases */
>> +		for (i = 0 ; i < count; i++) {
>> +			if (s[i] != vc->vc_video_erase_char)
>> +				break;
>> +		}
>> +		if (i = count)
>> +			return;
>> +
>> +		dummycon_putc_called = true;
>> +	}
>> +
>> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
>> +}
>> +
>> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
>> +{
>> +	/* Redraw, so that we get putc(s) for output done while blanked */
>> +	return 1;
>> +}
>> +#else
>> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
>> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
>> +			   int count, int ypos, int xpos) { }
>> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +
>>   static const char *dummycon_startup(void)
>>   {
>>       return "dummy device";
>> @@ -44,9 +103,6 @@ static void dummycon_init(struct vc_data *vc, int init)
>>   static void dummycon_deinit(struct vc_data *vc) { }
>>   static void dummycon_clear(struct vc_data *vc, int sy, int sx, int height,
>>   			   int width) { }
>> -static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
>> -static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
>> -			   int count, int ypos, int xpos) { }
>>   static void dummycon_cursor(struct vc_data *vc, int mode) { }
>>   
>>   static bool dummycon_scroll(struct vc_data *vc, unsigned int top,
>> @@ -61,11 +117,6 @@ static int dummycon_switch(struct vc_data *vc)
>>   	return 0;
>>   }
>>   
>> -static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
>> -{
>> -	return 0;
>> -}
>> -
>>   static int dummycon_font_set(struct vc_data *vc, struct console_font *font,
>>   			     unsigned int flags)
>>   {
>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>> index c910e74d46ff..12d947bc0f0b 100644
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -129,6 +129,12 @@ static inline void fbcon_map_override(void)
>>   }
>>   #endif /* CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY */
>>   
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +static bool deferred_takeover = true;
>> +#else
>> +#define deferred_takeover false
>> +#endif
>> +
>>   /* font data */
>>   static char fontname[40];
>>   
>> @@ -499,6 +505,12 @@ static int __init fb_console_setup(char *this_opt)
>>   				margin_color = simple_strtoul(options, &options, 0);
>>   			continue;
>>   		}
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +		if (!strcmp(options, "nodefer")) {
>> +			deferred_takeover = false;
>> +			continue;
>> +		}
>> +#endif
>>   	}
>>   	return 1;
>>   }
>> @@ -3094,6 +3106,9 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>>   {
>>   	int i, idx;
>>   
>> +	if (deferred_takeover)
>> +		return 0;
>> +
>>   	idx = info->node;
>>   	for (i = first_fb_vc; i <= last_fb_vc; i++) {
>>   		if (con2fb_map[i] = idx)
>> @@ -3131,6 +3146,14 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>>   static void fbcon_remap_all(int idx)
>>   {
>>   	int i;
>> +
>> +	if (deferred_takeover) {
>> +		for (i = first_fb_vc; i <= last_fb_vc; i++)
>> +			con2fb_map_boot[i] = idx;
>> +		fbcon_map_override();
>> +		return;
>> +	}
>> +
>>   	for (i = first_fb_vc; i <= last_fb_vc; i++)
>>   		set_con2fb_map(i, idx, 0);
>>   
>> @@ -3180,6 +3203,11 @@ static int fbcon_fb_registered(struct fb_info *info)
>>   	idx = info->node;
>>   	fbcon_select_primary(info);
>>   
>> +	if (deferred_takeover) {
>> +		pr_info("fbcon: Deferring console take-over\n");
>> +		return 0;
>> +	}
>> +
>>   	if (info_idx = -1) {
>>   		for (i = first_fb_vc; i <= last_fb_vc; i++) {
>>   			if (con2fb_map_boot[i] = idx) {
>> @@ -3555,8 +3583,45 @@ static int fbcon_init_device(void)
>>   	return 0;
>>   }
>>   
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +static struct notifier_block fbcon_output_nb;
>> +
>> +/* called with console_lock held */
>> +static int fbcon_output_notifier(struct notifier_block *nb,
>> +				 unsigned long action, void *data)
>> +{
>> +	int i;
>> +
>> +	pr_info("fbcon: Taking over console\n");
>> +
>> +	dummycon_unregister_output_notifier(&fbcon_output_nb);
>> +	deferred_takeover = false;
>> +	logo_shown = FBCON_LOGO_DONTSHOW;
>> +
>> +	for (i = 0; i < FB_MAX; i++) {
>> +		if (registered_fb[i])
>> +			fbcon_fb_registered(registered_fb[i]);
>> +	}
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static void fbcon_register_output_notifier(void)
>> +{
>> +	fbcon_output_nb.notifier_call = fbcon_output_notifier;
>> +	dummycon_register_output_notifier(&fbcon_output_nb);
>> +}
>> +#else
>> +static inline void fbcon_register_output_notifier(void) {}
>> +#endif
>> +
>>   static void fbcon_start(void)
>>   {
>> +	if (deferred_takeover) {
>> +		fbcon_register_output_notifier();
> 
> Nothing seems to unregister the notifier in fbcon_exit. Not likely in your
> use-case, but let's make sure this is correct.
> 
> Another request: Please create a console_lock_assert_held() in printk.c
> (or export the existing is_console_locked to make WARN_CONSOLE_UNLOCKED
> work properly in modules),

I've exported the existing is_console_locked() for v3 of my patchset
(in a separate prep. patch).

> and then sprinkle those into
> fbcon_fb_(un)registered.

Also done for v3, also in a separate prep. patch.

> fbcon and console subsystem locking is a major
> headache, I want to at least annotate stuff for when I have to review
> things again.

Ack.

> On that note: Notifiers are a real pain, but since there's only a BKL for
> everything console related the usual locking inversions between
> registering and callbacks can't happen. If dummycon and fbcon would each
> have their own locks, the simple notifier wouldn't work.
> 
> With the above addressed:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks, added for v3 which I will send out after some testing.

Regards,

Hans


> 
>> +		return;
>> +	}
>> +
>>   	if (num_registered_fb) {
>>   		int i;
>>   
>> diff --git a/include/linux/console.h b/include/linux/console.h
>> index dfd6b0e97855..f59f3dbca65c 100644
>> --- a/include/linux/console.h
>> +++ b/include/linux/console.h
>> @@ -21,6 +21,7 @@ struct console_font_op;
>>   struct console_font;
>>   struct module;
>>   struct tty_struct;
>> +struct notifier_block;
>>   
>>   /*
>>    * this is what the terminal answers to a ESC-Z or csi0c query.
>> @@ -220,4 +221,8 @@ static inline bool vgacon_text_force(void) { return false; }
>>   
>>   extern void console_init(void);
>>   
>> +/* For deferred console takeover */
>> +void dummycon_register_output_notifier(struct notifier_block *nb);
>> +void dummycon_unregister_output_notifier(struct notifier_block *nb);
>> +
>>   #endif /* _LINUX_CONSOLE_H */
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] console/fbcon: Add support for deferred console takeover
@ 2018-06-26 13:44     ` Hans de Goede
  0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2018-06-26 13:44 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-fbdev, dri-devel, Bartlomiej Zolnierkiewicz

Hi,

On 26-06-18 14:01, Daniel Vetter wrote:
> On Tue, Jun 26, 2018 at 12:41:03PM +0200, Hans de Goede wrote:
>> Currently fbcon claims fbdevs as soon as they are registered and takes over
>> the console as soon as the first fbdev gets registered.
>>
>> This behavior is undesirable in cases where a smooth graphical bootup is
>> desired, in such cases we typically want the contents of the framebuffer
>> (typically a vendor logo) to stay in place as is.
>>
>> The current solution for this problem (on embedded systems) is to not
>> enable fbcon.
>>
>> This commit adds a new FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER config option,
>> which when enabled defers fbcon taking over the console from the dummy
>> console until the first text is displayed on the console. Together with the
>> "quiet" kernel commandline option, this allows fbcon to still be used
>> together with a smooth graphical bootup, having it take over the console as
>> soon as e.g. an error message is logged.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Check the whole string when checking for erases in putcs, instead of just
>>   the first char
>> -Make dummycon_blank return 1, so that a redraw gets triggered and any text
>>   rendered while blanked gets output so that it can trigger a deferred
>>   takeover if one is pending
>> ---
>>   drivers/video/console/Kconfig    | 11 ++++++
>>   drivers/video/console/dummycon.c | 67 ++++++++++++++++++++++++++++----
>>   drivers/video/fbdev/core/fbcon.c | 65 +++++++++++++++++++++++++++++++
>>   include/linux/console.h          |  5 +++
>>   4 files changed, 140 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
>> index 4110ba7d7ca9..e91edef98633 100644
>> --- a/drivers/video/console/Kconfig
>> +++ b/drivers/video/console/Kconfig
>> @@ -150,6 +150,17 @@ config FRAMEBUFFER_CONSOLE_ROTATION
>>            such that other users of the framebuffer will remain normally
>>            oriented.
>>   
>> +config FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +	bool "Framebuffer Console Deferred Takeover"
>> +	depends on FRAMEBUFFER_CONSOLE=y && DUMMY_CONSOLE=y
>> +	help
>> +	  If enabled this defers the framebuffer console taking over the
>> +	  console from the dummy console until the first text is displayed on
>> +	  the console. This is useful in combination with the "quiet" kernel
>> +	  commandline option to keep the framebuffer contents initially put up
>> +	  by the firmware in place, rather then replacing the contents with a
>> +	  black screen as soon as fbcon loads.
>> +
>>   config STI_CONSOLE
>>           bool "STI text console"
>>   	depends on PARISC && HAS_IOMEM
>> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
>> index f2eafe2ed980..45ad925ad5f8 100644
>> --- a/drivers/video/console/dummycon.c
>> +++ b/drivers/video/console/dummycon.c
>> @@ -26,6 +26,65 @@
>>   #define DUMMY_ROWS	CONFIG_DUMMY_CONSOLE_ROWS
>>   #endif
>>   
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +/* These are both protected by the console_lock */
>> +static RAW_NOTIFIER_HEAD(dummycon_output_nh);
>> +static bool dummycon_putc_called;
>> +
>> +void dummycon_register_output_notifier(struct notifier_block *nb)
>> +{
>> +	raw_notifier_chain_register(&dummycon_output_nh, nb);
>> +
>> +	if (dummycon_putc_called)
>> +		nb->notifier_call(nb, 0, NULL);
>> +}
>> +
>> +void dummycon_unregister_output_notifier(struct notifier_block *nb)
>> +{
>> +	raw_notifier_chain_unregister(&dummycon_output_nh, nb);
>> +}
>> +
>> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos)
>> +{
>> +	dummycon_putc_called = true;
>> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
>> +}
>> +
>> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
>> +			   int count, int ypos, int xpos)
>> +{
>> +	int i;
>> +
>> +	if (!dummycon_putc_called) {
>> +		/* Ignore erases */
>> +		for (i = 0 ; i < count; i++) {
>> +			if (s[i] != vc->vc_video_erase_char)
>> +				break;
>> +		}
>> +		if (i == count)
>> +			return;
>> +
>> +		dummycon_putc_called = true;
>> +	}
>> +
>> +	raw_notifier_call_chain(&dummycon_output_nh, 0, NULL);
>> +}
>> +
>> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
>> +{
>> +	/* Redraw, so that we get putc(s) for output done while blanked */
>> +	return 1;
>> +}
>> +#else
>> +static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
>> +static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
>> +			   int count, int ypos, int xpos) { }
>> +static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +
>>   static const char *dummycon_startup(void)
>>   {
>>       return "dummy device";
>> @@ -44,9 +103,6 @@ static void dummycon_init(struct vc_data *vc, int init)
>>   static void dummycon_deinit(struct vc_data *vc) { }
>>   static void dummycon_clear(struct vc_data *vc, int sy, int sx, int height,
>>   			   int width) { }
>> -static void dummycon_putc(struct vc_data *vc, int c, int ypos, int xpos) { }
>> -static void dummycon_putcs(struct vc_data *vc, const unsigned short *s,
>> -			   int count, int ypos, int xpos) { }
>>   static void dummycon_cursor(struct vc_data *vc, int mode) { }
>>   
>>   static bool dummycon_scroll(struct vc_data *vc, unsigned int top,
>> @@ -61,11 +117,6 @@ static int dummycon_switch(struct vc_data *vc)
>>   	return 0;
>>   }
>>   
>> -static int dummycon_blank(struct vc_data *vc, int blank, int mode_switch)
>> -{
>> -	return 0;
>> -}
>> -
>>   static int dummycon_font_set(struct vc_data *vc, struct console_font *font,
>>   			     unsigned int flags)
>>   {
>> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
>> index c910e74d46ff..12d947bc0f0b 100644
>> --- a/drivers/video/fbdev/core/fbcon.c
>> +++ b/drivers/video/fbdev/core/fbcon.c
>> @@ -129,6 +129,12 @@ static inline void fbcon_map_override(void)
>>   }
>>   #endif /* CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY */
>>   
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +static bool deferred_takeover = true;
>> +#else
>> +#define deferred_takeover false
>> +#endif
>> +
>>   /* font data */
>>   static char fontname[40];
>>   
>> @@ -499,6 +505,12 @@ static int __init fb_console_setup(char *this_opt)
>>   				margin_color = simple_strtoul(options, &options, 0);
>>   			continue;
>>   		}
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +		if (!strcmp(options, "nodefer")) {
>> +			deferred_takeover = false;
>> +			continue;
>> +		}
>> +#endif
>>   	}
>>   	return 1;
>>   }
>> @@ -3094,6 +3106,9 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>>   {
>>   	int i, idx;
>>   
>> +	if (deferred_takeover)
>> +		return 0;
>> +
>>   	idx = info->node;
>>   	for (i = first_fb_vc; i <= last_fb_vc; i++) {
>>   		if (con2fb_map[i] == idx)
>> @@ -3131,6 +3146,14 @@ static int fbcon_fb_unregistered(struct fb_info *info)
>>   static void fbcon_remap_all(int idx)
>>   {
>>   	int i;
>> +
>> +	if (deferred_takeover) {
>> +		for (i = first_fb_vc; i <= last_fb_vc; i++)
>> +			con2fb_map_boot[i] = idx;
>> +		fbcon_map_override();
>> +		return;
>> +	}
>> +
>>   	for (i = first_fb_vc; i <= last_fb_vc; i++)
>>   		set_con2fb_map(i, idx, 0);
>>   
>> @@ -3180,6 +3203,11 @@ static int fbcon_fb_registered(struct fb_info *info)
>>   	idx = info->node;
>>   	fbcon_select_primary(info);
>>   
>> +	if (deferred_takeover) {
>> +		pr_info("fbcon: Deferring console take-over\n");
>> +		return 0;
>> +	}
>> +
>>   	if (info_idx == -1) {
>>   		for (i = first_fb_vc; i <= last_fb_vc; i++) {
>>   			if (con2fb_map_boot[i] == idx) {
>> @@ -3555,8 +3583,45 @@ static int fbcon_init_device(void)
>>   	return 0;
>>   }
>>   
>> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
>> +static struct notifier_block fbcon_output_nb;
>> +
>> +/* called with console_lock held */
>> +static int fbcon_output_notifier(struct notifier_block *nb,
>> +				 unsigned long action, void *data)
>> +{
>> +	int i;
>> +
>> +	pr_info("fbcon: Taking over console\n");
>> +
>> +	dummycon_unregister_output_notifier(&fbcon_output_nb);
>> +	deferred_takeover = false;
>> +	logo_shown = FBCON_LOGO_DONTSHOW;
>> +
>> +	for (i = 0; i < FB_MAX; i++) {
>> +		if (registered_fb[i])
>> +			fbcon_fb_registered(registered_fb[i]);
>> +	}
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>> +static void fbcon_register_output_notifier(void)
>> +{
>> +	fbcon_output_nb.notifier_call = fbcon_output_notifier;
>> +	dummycon_register_output_notifier(&fbcon_output_nb);
>> +}
>> +#else
>> +static inline void fbcon_register_output_notifier(void) {}
>> +#endif
>> +
>>   static void fbcon_start(void)
>>   {
>> +	if (deferred_takeover) {
>> +		fbcon_register_output_notifier();
> 
> Nothing seems to unregister the notifier in fbcon_exit. Not likely in your
> use-case, but let's make sure this is correct.
> 
> Another request: Please create a console_lock_assert_held() in printk.c
> (or export the existing is_console_locked to make WARN_CONSOLE_UNLOCKED
> work properly in modules),

I've exported the existing is_console_locked() for v3 of my patchset
(in a separate prep. patch).

> and then sprinkle those into
> fbcon_fb_(un)registered.

Also done for v3, also in a separate prep. patch.

> fbcon and console subsystem locking is a major
> headache, I want to at least annotate stuff for when I have to review
> things again.

Ack.

> On that note: Notifiers are a real pain, but since there's only a BKL for
> everything console related the usual locking inversions between
> registering and callbacks can't happen. If dummycon and fbcon would each
> have their own locks, the simple notifier wouldn't work.
> 
> With the above addressed:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks, added for v3 which I will send out after some testing.

Regards,

Hans


> 
>> +		return;
>> +	}
>> +
>>   	if (num_registered_fb) {
>>   		int i;
>>   
>> diff --git a/include/linux/console.h b/include/linux/console.h
>> index dfd6b0e97855..f59f3dbca65c 100644
>> --- a/include/linux/console.h
>> +++ b/include/linux/console.h
>> @@ -21,6 +21,7 @@ struct console_font_op;
>>   struct console_font;
>>   struct module;
>>   struct tty_struct;
>> +struct notifier_block;
>>   
>>   /*
>>    * this is what the terminal answers to a ESC-Z or csi0c query.
>> @@ -220,4 +221,8 @@ static inline bool vgacon_text_force(void) { return false; }
>>   
>>   extern void console_init(void);
>>   
>> +/* For deferred console takeover */
>> +void dummycon_register_output_notifier(struct notifier_block *nb);
>> +void dummycon_unregister_output_notifier(struct notifier_block *nb);
>> +
>>   #endif /* _LINUX_CONSOLE_H */
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-06-26 13:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-26 10:41 [PATCH v2] console/fbcon: Add support for deferred console takeover Hans de Goede
2018-06-26 10:41 ` Hans de Goede
2018-06-26 12:01 ` Daniel Vetter
2018-06-26 12:01   ` Daniel Vetter
2018-06-26 13:44   ` Hans de Goede
2018-06-26 13:44     ` Hans de Goede
2018-06-26 13:11 ` Emil Velikov
2018-06-26 13:11   ` Emil Velikov
2018-06-26 13:42   ` Hans de Goede
2018-06-26 13:42     ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.