[PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
This is not a full replacement to fbcon, as it will only print the kmsg.
It will never handle user input, or a terminal because this is better done in userspace.

If you're curious on how it looks like, I've put a small demo here:
https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4

Design decisions:
  * It uses the drm_client API, so it should work on all drm drivers from the start.
  * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
    It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
  * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
 
v2:
 * Use vmap_local() api, with that change, I've tested it successfully on simpledrm, virtio-gpu, amdgpu, and nouveau.
 * Stop drawing when the drm_master is taken. This avoid wasting CPU cycle if the buffer is not visible.
 * Use deferred probe. Only do the probe the first time there is a log to draw. With this, if you boot with quiet, drm_log won't do any modeset.
 * Add color support for the timestamp prefix, like what dmesg does.
 * Add build dependency on  disabling the fbdev emulation, as they are both drm_client, and there is no way to choose which one gets the focus.

v3:
 * Remove the work thread and circular buffer, and use the new write_thread() console API.
 * Register a console for each drm driver.

v4:
 * Can be built as a module, even if that's not really useful.
 * Rebased on top of "drm: Introduce DRM client library" series from Thomas Zimmermann.
 * Add a Kconfig menu to choose between drm client.
 * Add suspend/resume callbacks.
 * Add integer scaling support.
 
v5:
 * Build drm_log in drm_client_lib module, to avoid circular dependency.
 * Export drm_draw symbols, so they can be used if drm_client_lib is built as module.
 * Change scale parameter to unsigned int (Jani Nikula)

v6:
 * Use console_stop() and console_start() in the suspend/resume callback (Petr Mladek).
 * rebase and solve conflict with "drm/panic: Add ABGR2101010 support"

v7:
 * Add a patch fix a build issue due to missing DRM_CLIENT_LIB, reported by kernel test bot.

v8:
 * Rebased after drm client moved to drivers/gpu/drm/clients/
 * Rename DRM_LOG to DRM_CLIENT_LOG (Thomas Zimmermann)
 * Drop "Always select DRM_CLIENT_LIB", and select only if DRM_CLIENT_LOG is set
 * Add an info message if no clients are initialized in drm_client_setup()

v9:
 * Rename drm_draw.h to drm_draw_internal.h (Jani Nikula)
 * Add cflags to remove the "../" when including drm internal headers (Jani Nikula)
 * Order select alphabetically in KConfig (Thomas Zimmermann)
 * Replace drm_info with drm_dbg, to be less verbose (Thomas Zimmermann)
 * Rename module parameter to drm_client_lib.active (Thomas Zimmermann)
 * Warn if drm_client_lib.active is malformated (Thomas Zimmermann)
 
Jocelyn Falempe (6):
  drm/panic: Move drawing functions to drm_draw
  drm/log: Introduce a new boot logger to draw the kmsg on the screen
  drm/log: Do not draw if drm_master is taken
  drm/log: Color the timestamp, to improve readability
  drm/log: Implement suspend/resume
  drm/log: Add integer scaling support

 drivers/gpu/drm/Kconfig                       |   5 +
 drivers/gpu/drm/Makefile                      |   1 +
 drivers/gpu/drm/clients/Kconfig               |  48 ++
 drivers/gpu/drm/clients/Makefile              |   3 +
 drivers/gpu/drm/clients/drm_client_internal.h |   6 +
 drivers/gpu/drm/clients/drm_client_setup.c    |  29 +-
 drivers/gpu/drm/clients/drm_log.c             | 420 ++++++++++++++++++
 drivers/gpu/drm/drm_draw.c                    | 233 ++++++++++
 drivers/gpu/drm/drm_draw_internal.h           |  56 +++
 drivers/gpu/drm/drm_panic.c                   | 257 +----------
 10 files changed, 820 insertions(+), 238 deletions(-)
 create mode 100644 drivers/gpu/drm/clients/drm_log.c
 create mode 100644 drivers/gpu/drm/drm_draw.c
 create mode 100644 drivers/gpu/drm/drm_draw_internal.h


base-commit: c6eabbab359c156669e10d5dec3e71e80ff09bd2
-- 
2.47.1

[PATCH v9 1/6] drm/panic: Move drawing functions to drm_draw

[Jocelyn Falempe]

Move the color conversions, blit and fill functions to drm_draw.c,
so that they can be re-used by drm_log.
drm_draw is internal to the drm subsystem, and shouldn't be used by
gpu drivers.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---

v5:
 * Export drm_draw symbols, so they can be used if drm_client_lib is built as module.

v6:
 * rebase and solve conflict with "drm/panic: Add ABGR2101010 support"

v9:
 * Rename drm_draw.h to drm_draw_internal.h (Jani Nikula)

 drivers/gpu/drm/Kconfig             |   5 +
 drivers/gpu/drm/Makefile            |   1 +
 drivers/gpu/drm/drm_draw.c          | 233 +++++++++++++++++++++++++
 drivers/gpu/drm/drm_draw_internal.h |  56 ++++++
 drivers/gpu/drm/drm_panic.c         | 257 +++-------------------------
 5 files changed, 318 insertions(+), 234 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_draw.c
 create mode 100644 drivers/gpu/drm/drm_draw_internal.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 698981a2ed51..6f1cf235073d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -102,10 +102,15 @@ config DRM_KMS_HELPER
 	help
 	  CRTC helpers for KMS drivers.
 
+config DRM_DRAW
+	bool
+	depends on DRM
+
 config DRM_PANIC
 	bool "Display a user-friendly message when a kernel panic occurs"
 	depends on DRM
 	select FONT_SUPPORT
+	select DRM_DRAW
 	help
 	  Enable a drm panic handler, which will display a user-friendly message
 	  when a kernel panic occurs. It's useful when using a user-space
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1677c1f335fb..19fb370fbc56 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -91,6 +91,7 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += \
 	drm_privacy_screen_x86.o
 drm-$(CONFIG_DRM_ACCEL) += ../../accel/drm_accel.o
 drm-$(CONFIG_DRM_PANIC) += drm_panic.o
+drm-$(CONFIG_DRM_DRAW) += drm_draw.o
 drm-$(CONFIG_DRM_PANIC_SCREEN_QR_CODE) += drm_panic_qr.o
 obj-$(CONFIG_DRM)	+= drm.o
 
diff --git a/drivers/gpu/drm/drm_draw.c b/drivers/gpu/drm/drm_draw.c
new file mode 100644
index 000000000000..cb2ad12bce57
--- /dev/null
+++ b/drivers/gpu/drm/drm_draw.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/*
+ * Copyright (c) 2023 Red Hat.
+ * Author: Jocelyn Falempe <jfalempe@redhat.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/iosys-map.h>
+#include <linux/types.h>
+
+#include <drm/drm_fourcc.h>
+
+#include "drm_draw_internal.h"
+
+/*
+ * Conversions from xrgb8888
+ */
+
+static u16 convert_xrgb8888_to_rgb565(u32 pix)
+{
+	return ((pix & 0x00F80000) >> 8) |
+	       ((pix & 0x0000FC00) >> 5) |
+	       ((pix & 0x000000F8) >> 3);
+}
+
+static u16 convert_xrgb8888_to_rgba5551(u32 pix)
+{
+	return ((pix & 0x00f80000) >> 8) |
+	       ((pix & 0x0000f800) >> 5) |
+	       ((pix & 0x000000f8) >> 2) |
+	       BIT(0); /* set alpha bit */
+}
+
+static u16 convert_xrgb8888_to_xrgb1555(u32 pix)
+{
+	return ((pix & 0x00f80000) >> 9) |
+	       ((pix & 0x0000f800) >> 6) |
+	       ((pix & 0x000000f8) >> 3);
+}
+
+static u16 convert_xrgb8888_to_argb1555(u32 pix)
+{
+	return BIT(15) | /* set alpha bit */
+	       ((pix & 0x00f80000) >> 9) |
+	       ((pix & 0x0000f800) >> 6) |
+	       ((pix & 0x000000f8) >> 3);
+}
+
+static u32 convert_xrgb8888_to_argb8888(u32 pix)
+{
+	return pix | GENMASK(31, 24); /* fill alpha bits */
+}
+
+static u32 convert_xrgb8888_to_xbgr8888(u32 pix)
+{
+	return ((pix & 0x00ff0000) >> 16) <<  0 |
+	       ((pix & 0x0000ff00) >>  8) <<  8 |
+	       ((pix & 0x000000ff) >>  0) << 16 |
+	       ((pix & 0xff000000) >> 24) << 24;
+}
+
+static u32 convert_xrgb8888_to_abgr8888(u32 pix)
+{
+	return ((pix & 0x00ff0000) >> 16) <<  0 |
+	       ((pix & 0x0000ff00) >>  8) <<  8 |
+	       ((pix & 0x000000ff) >>  0) << 16 |
+	       GENMASK(31, 24); /* fill alpha bits */
+}
+
+static u32 convert_xrgb8888_to_xrgb2101010(u32 pix)
+{
+	pix = ((pix & 0x000000FF) << 2) |
+	      ((pix & 0x0000FF00) << 4) |
+	      ((pix & 0x00FF0000) << 6);
+	return pix | ((pix >> 8) & 0x00300C03);
+}
+
+static u32 convert_xrgb8888_to_argb2101010(u32 pix)
+{
+	pix = ((pix & 0x000000FF) << 2) |
+	      ((pix & 0x0000FF00) << 4) |
+	      ((pix & 0x00FF0000) << 6);
+	return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03);
+}
+
+static u32 convert_xrgb8888_to_abgr2101010(u32 pix)
+{
+	pix = ((pix & 0x00FF0000) >> 14) |
+	      ((pix & 0x0000FF00) << 4) |
+	      ((pix & 0x000000FF) << 22);
+	return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03);
+}
+
+/**
+ * drm_draw_color_from_xrgb8888 - convert one pixel from xrgb8888 to the desired format
+ * @color: input color, in xrgb8888 format
+ * @format: output format
+ *
+ * Returns:
+ * Color in the format specified, casted to u32.
+ * Or 0 if the format is not supported.
+ */
+u32 drm_draw_color_from_xrgb8888(u32 color, u32 format)
+{
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+		return convert_xrgb8888_to_rgb565(color);
+	case DRM_FORMAT_RGBA5551:
+		return convert_xrgb8888_to_rgba5551(color);
+	case DRM_FORMAT_XRGB1555:
+		return convert_xrgb8888_to_xrgb1555(color);
+	case DRM_FORMAT_ARGB1555:
+		return convert_xrgb8888_to_argb1555(color);
+	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_XRGB8888:
+		return color;
+	case DRM_FORMAT_ARGB8888:
+		return convert_xrgb8888_to_argb8888(color);
+	case DRM_FORMAT_XBGR8888:
+		return convert_xrgb8888_to_xbgr8888(color);
+	case DRM_FORMAT_ABGR8888:
+		return convert_xrgb8888_to_abgr8888(color);
+	case DRM_FORMAT_XRGB2101010:
+		return convert_xrgb8888_to_xrgb2101010(color);
+	case DRM_FORMAT_ARGB2101010:
+		return convert_xrgb8888_to_argb2101010(color);
+	case DRM_FORMAT_ABGR2101010:
+		return convert_xrgb8888_to_abgr2101010(color);
+	default:
+		WARN_ONCE(1, "Can't convert to %p4cc\n", &format);
+		return 0;
+	}
+}
+EXPORT_SYMBOL(drm_draw_color_from_xrgb8888);
+
+/*
+ * Blit functions
+ */
+void drm_draw_blit16(struct iosys_map *dmap, unsigned int dpitch,
+		     const u8 *sbuf8, unsigned int spitch,
+		     unsigned int height, unsigned int width,
+		     unsigned int scale, u16 fg16)
+{
+	unsigned int y, x;
+
+	for (y = 0; y < height; y++)
+		for (x = 0; x < width; x++)
+			if (drm_draw_is_pixel_fg(sbuf8, spitch, x / scale, y / scale))
+				iosys_map_wr(dmap, y * dpitch + x * sizeof(u16), u16, fg16);
+}
+EXPORT_SYMBOL(drm_draw_blit16);
+
+void drm_draw_blit24(struct iosys_map *dmap, unsigned int dpitch,
+		     const u8 *sbuf8, unsigned int spitch,
+		     unsigned int height, unsigned int width,
+		     unsigned int scale, u32 fg32)
+{
+	unsigned int y, x;
+
+	for (y = 0; y < height; y++) {
+		for (x = 0; x < width; x++) {
+			u32 off = y * dpitch + x * 3;
+
+			if (drm_draw_is_pixel_fg(sbuf8, spitch, x / scale, y / scale)) {
+				/* write blue-green-red to output in little endianness */
+				iosys_map_wr(dmap, off, u8, (fg32 & 0x000000FF) >> 0);
+				iosys_map_wr(dmap, off + 1, u8, (fg32 & 0x0000FF00) >> 8);
+				iosys_map_wr(dmap, off + 2, u8, (fg32 & 0x00FF0000) >> 16);
+			}
+		}
+	}
+}
+EXPORT_SYMBOL(drm_draw_blit24);
+
+void drm_draw_blit32(struct iosys_map *dmap, unsigned int dpitch,
+		     const u8 *sbuf8, unsigned int spitch,
+		     unsigned int height, unsigned int width,
+		     unsigned int scale, u32 fg32)
+{
+	unsigned int y, x;
+
+	for (y = 0; y < height; y++)
+		for (x = 0; x < width; x++)
+			if (drm_draw_is_pixel_fg(sbuf8, spitch, x / scale, y / scale))
+				iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, fg32);
+}
+EXPORT_SYMBOL(drm_draw_blit32);
+
+/*
+ * Fill functions
+ */
+void drm_draw_fill16(struct iosys_map *dmap, unsigned int dpitch,
+		     unsigned int height, unsigned int width,
+		     u16 color)
+{
+	unsigned int y, x;
+
+	for (y = 0; y < height; y++)
+		for (x = 0; x < width; x++)
+			iosys_map_wr(dmap, y * dpitch + x * sizeof(u16), u16, color);
+}
+EXPORT_SYMBOL(drm_draw_fill16);
+
+void drm_draw_fill24(struct iosys_map *dmap, unsigned int dpitch,
+		     unsigned int height, unsigned int width,
+		     u16 color)
+{
+	unsigned int y, x;
+
+	for (y = 0; y < height; y++) {
+		for (x = 0; x < width; x++) {
+			unsigned int off = y * dpitch + x * 3;
+
+			/* write blue-green-red to output in little endianness */
+			iosys_map_wr(dmap, off, u8, (color & 0x000000FF) >> 0);
+			iosys_map_wr(dmap, off + 1, u8, (color & 0x0000FF00) >> 8);
+			iosys_map_wr(dmap, off + 2, u8, (color & 0x00FF0000) >> 16);
+		}
+	}
+}
+EXPORT_SYMBOL(drm_draw_fill24);
+
+void drm_draw_fill32(struct iosys_map *dmap, unsigned int dpitch,
+		     unsigned int height, unsigned int width,
+		     u32 color)
+{
+	unsigned int y, x;
+
+	for (y = 0; y < height; y++)
+		for (x = 0; x < width; x++)
+			iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, color);
+}
+EXPORT_SYMBOL(drm_draw_fill32);
diff --git a/drivers/gpu/drm/drm_draw_internal.h b/drivers/gpu/drm/drm_draw_internal.h
new file mode 100644
index 000000000000..f121ee7339dc
--- /dev/null
+++ b/drivers/gpu/drm/drm_draw_internal.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 or MIT */
+/*
+ * Copyright (c) 2023 Red Hat.
+ * Author: Jocelyn Falempe <jfalempe@redhat.com>
+ */
+
+#ifndef __DRM_DRAW_INTERNAL_H__
+#define __DRM_DRAW_INTERNAL_H__
+
+#include <linux/font.h>
+#include <linux/types.h>
+
+struct iosys_map;
+
+/* check if the pixel at coord x,y is 1 (foreground) or 0 (background) */
+static inline bool drm_draw_is_pixel_fg(const u8 *sbuf8, unsigned int spitch, int x, int y)
+{
+	return (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) != 0;
+}
+
+static inline const u8 *drm_draw_get_char_bitmap(const struct font_desc *font,
+						 char c, size_t font_pitch)
+{
+	return font->data + (c * font->height) * font_pitch;
+}
+
+u32 drm_draw_color_from_xrgb8888(u32 color, u32 format);
+
+void drm_draw_blit16(struct iosys_map *dmap, unsigned int dpitch,
+		     const u8 *sbuf8, unsigned int spitch,
+		     unsigned int height, unsigned int width,
+		     unsigned int scale, u16 fg16);
+
+void drm_draw_blit24(struct iosys_map *dmap, unsigned int dpitch,
+		     const u8 *sbuf8, unsigned int spitch,
+		     unsigned int height, unsigned int width,
+		     unsigned int scale, u32 fg32);
+
+void drm_draw_blit32(struct iosys_map *dmap, unsigned int dpitch,
+		     const u8 *sbuf8, unsigned int spitch,
+		     unsigned int height, unsigned int width,
+		     unsigned int scale, u32 fg32);
+
+void drm_draw_fill16(struct iosys_map *dmap, unsigned int dpitch,
+		     unsigned int height, unsigned int width,
+		     u16 color);
+
+void drm_draw_fill24(struct iosys_map *dmap, unsigned int dpitch,
+		     unsigned int height, unsigned int width,
+		     u16 color);
+
+void drm_draw_fill32(struct iosys_map *dmap, unsigned int dpitch,
+		     unsigned int height, unsigned int width,
+		     u32 color);
+
+#endif /* __DRM_DRAW_INTERNAL_H__ */
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 0a9ecc1380d2..56e7e773f92f 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -31,6 +31,7 @@
 #include <drm/drm_rect.h>
 
 #include "drm_crtc_internal.h"
+#include "drm_draw_internal.h"
 
 MODULE_AUTHOR("Jocelyn Falempe");
 MODULE_DESCRIPTION("DRM panic handler");
@@ -139,181 +140,8 @@ device_initcall(drm_panic_setup_logo);
 #endif
 
 /*
- * Color conversion
+ *  Blit & Fill functions
  */
-
-static u16 convert_xrgb8888_to_rgb565(u32 pix)
-{
-	return ((pix & 0x00F80000) >> 8) |
-	       ((pix & 0x0000FC00) >> 5) |
-	       ((pix & 0x000000F8) >> 3);
-}
-
-static u16 convert_xrgb8888_to_rgba5551(u32 pix)
-{
-	return ((pix & 0x00f80000) >> 8) |
-	       ((pix & 0x0000f800) >> 5) |
-	       ((pix & 0x000000f8) >> 2) |
-	       BIT(0); /* set alpha bit */
-}
-
-static u16 convert_xrgb8888_to_xrgb1555(u32 pix)
-{
-	return ((pix & 0x00f80000) >> 9) |
-	       ((pix & 0x0000f800) >> 6) |
-	       ((pix & 0x000000f8) >> 3);
-}
-
-static u16 convert_xrgb8888_to_argb1555(u32 pix)
-{
-	return BIT(15) | /* set alpha bit */
-	       ((pix & 0x00f80000) >> 9) |
-	       ((pix & 0x0000f800) >> 6) |
-	       ((pix & 0x000000f8) >> 3);
-}
-
-static u32 convert_xrgb8888_to_argb8888(u32 pix)
-{
-	return pix | GENMASK(31, 24); /* fill alpha bits */
-}
-
-static u32 convert_xrgb8888_to_xbgr8888(u32 pix)
-{
-	return ((pix & 0x00ff0000) >> 16) <<  0 |
-	       ((pix & 0x0000ff00) >>  8) <<  8 |
-	       ((pix & 0x000000ff) >>  0) << 16 |
-	       ((pix & 0xff000000) >> 24) << 24;
-}
-
-static u32 convert_xrgb8888_to_abgr8888(u32 pix)
-{
-	return ((pix & 0x00ff0000) >> 16) <<  0 |
-	       ((pix & 0x0000ff00) >>  8) <<  8 |
-	       ((pix & 0x000000ff) >>  0) << 16 |
-	       GENMASK(31, 24); /* fill alpha bits */
-}
-
-static u32 convert_xrgb8888_to_xrgb2101010(u32 pix)
-{
-	pix = ((pix & 0x000000FF) << 2) |
-	      ((pix & 0x0000FF00) << 4) |
-	      ((pix & 0x00FF0000) << 6);
-	return pix | ((pix >> 8) & 0x00300C03);
-}
-
-static u32 convert_xrgb8888_to_argb2101010(u32 pix)
-{
-	pix = ((pix & 0x000000FF) << 2) |
-	      ((pix & 0x0000FF00) << 4) |
-	      ((pix & 0x00FF0000) << 6);
-	return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03);
-}
-
-static u32 convert_xrgb8888_to_abgr2101010(u32 pix)
-{
-	pix = ((pix & 0x00FF0000) >> 14) |
-	      ((pix & 0x0000FF00) << 4) |
-	      ((pix & 0x000000FF) << 22);
-	return GENMASK(31, 30) /* set alpha bits */ | pix | ((pix >> 8) & 0x00300C03);
-}
-
-/*
- * convert_from_xrgb8888 - convert one pixel from xrgb8888 to the desired format
- * @color: input color, in xrgb8888 format
- * @format: output format
- *
- * Returns:
- * Color in the format specified, casted to u32.
- * Or 0 if the format is not supported.
- */
-static u32 convert_from_xrgb8888(u32 color, u32 format)
-{
-	switch (format) {
-	case DRM_FORMAT_RGB565:
-		return convert_xrgb8888_to_rgb565(color);
-	case DRM_FORMAT_RGBA5551:
-		return convert_xrgb8888_to_rgba5551(color);
-	case DRM_FORMAT_XRGB1555:
-		return convert_xrgb8888_to_xrgb1555(color);
-	case DRM_FORMAT_ARGB1555:
-		return convert_xrgb8888_to_argb1555(color);
-	case DRM_FORMAT_RGB888:
-	case DRM_FORMAT_XRGB8888:
-		return color;
-	case DRM_FORMAT_ARGB8888:
-		return convert_xrgb8888_to_argb8888(color);
-	case DRM_FORMAT_XBGR8888:
-		return convert_xrgb8888_to_xbgr8888(color);
-	case DRM_FORMAT_ABGR8888:
-		return convert_xrgb8888_to_abgr8888(color);
-	case DRM_FORMAT_XRGB2101010:
-		return convert_xrgb8888_to_xrgb2101010(color);
-	case DRM_FORMAT_ARGB2101010:
-		return convert_xrgb8888_to_argb2101010(color);
-	case DRM_FORMAT_ABGR2101010:
-		return convert_xrgb8888_to_abgr2101010(color);
-	default:
-		WARN_ONCE(1, "Can't convert to %p4cc\n", &format);
-		return 0;
-	}
-}
-
-/*
- * Blit & Fill
- */
-/* check if the pixel at coord x,y is 1 (foreground) or 0 (background) */
-static bool drm_panic_is_pixel_fg(const u8 *sbuf8, unsigned int spitch, int x, int y)
-{
-	return (sbuf8[(y * spitch) + x / 8] & (0x80 >> (x % 8))) != 0;
-}
-
-static void drm_panic_blit16(struct iosys_map *dmap, unsigned int dpitch,
-			     const u8 *sbuf8, unsigned int spitch,
-			     unsigned int height, unsigned int width,
-			     unsigned int scale, u16 fg16)
-{
-	unsigned int y, x;
-
-	for (y = 0; y < height; y++)
-		for (x = 0; x < width; x++)
-			if (drm_panic_is_pixel_fg(sbuf8, spitch, x / scale, y / scale))
-				iosys_map_wr(dmap, y * dpitch + x * sizeof(u16), u16, fg16);
-}
-
-static void drm_panic_blit24(struct iosys_map *dmap, unsigned int dpitch,
-			     const u8 *sbuf8, unsigned int spitch,
-			     unsigned int height, unsigned int width,
-			     unsigned int scale, u32 fg32)
-{
-	unsigned int y, x;
-
-	for (y = 0; y < height; y++) {
-		for (x = 0; x < width; x++) {
-			u32 off = y * dpitch + x * 3;
-
-			if (drm_panic_is_pixel_fg(sbuf8, spitch, x / scale, y / scale)) {
-				/* write blue-green-red to output in little endianness */
-				iosys_map_wr(dmap, off, u8, (fg32 & 0x000000FF) >> 0);
-				iosys_map_wr(dmap, off + 1, u8, (fg32 & 0x0000FF00) >> 8);
-				iosys_map_wr(dmap, off + 2, u8, (fg32 & 0x00FF0000) >> 16);
-			}
-		}
-	}
-}
-
-static void drm_panic_blit32(struct iosys_map *dmap, unsigned int dpitch,
-			     const u8 *sbuf8, unsigned int spitch,
-			     unsigned int height, unsigned int width,
-			     unsigned int scale, u32 fg32)
-{
-	unsigned int y, x;
-
-	for (y = 0; y < height; y++)
-		for (x = 0; x < width; x++)
-			if (drm_panic_is_pixel_fg(sbuf8, spitch, x / scale, y / scale))
-				iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, fg32);
-}
-
 static void drm_panic_blit_pixel(struct drm_scanout_buffer *sb, struct drm_rect *clip,
 				 const u8 *sbuf8, unsigned int spitch, unsigned int scale,
 				 u32 fg_color)
@@ -322,7 +150,7 @@ static void drm_panic_blit_pixel(struct drm_scanout_buffer *sb, struct drm_rect
 
 	for (y = 0; y < drm_rect_height(clip); y++)
 		for (x = 0; x < drm_rect_width(clip); x++)
-			if (drm_panic_is_pixel_fg(sbuf8, spitch, x / scale, y / scale))
+			if (drm_draw_is_pixel_fg(sbuf8, spitch, x / scale, y / scale))
 				sb->set_pixel(sb, clip->x1 + x, clip->y1 + y, fg_color);
 }
 
@@ -354,15 +182,15 @@ static void drm_panic_blit(struct drm_scanout_buffer *sb, struct drm_rect *clip,
 
 	switch (sb->format->cpp[0]) {
 	case 2:
-		drm_panic_blit16(&map, sb->pitch[0], sbuf8, spitch,
+		drm_draw_blit16(&map, sb->pitch[0], sbuf8, spitch,
 				 drm_rect_height(clip), drm_rect_width(clip), scale, fg_color);
 	break;
 	case 3:
-		drm_panic_blit24(&map, sb->pitch[0], sbuf8, spitch,
+		drm_draw_blit24(&map, sb->pitch[0], sbuf8, spitch,
 				 drm_rect_height(clip), drm_rect_width(clip), scale, fg_color);
 	break;
 	case 4:
-		drm_panic_blit32(&map, sb->pitch[0], sbuf8, spitch,
+		drm_draw_blit32(&map, sb->pitch[0], sbuf8, spitch,
 				 drm_rect_height(clip), drm_rect_width(clip), scale, fg_color);
 	break;
 	default:
@@ -370,46 +198,6 @@ static void drm_panic_blit(struct drm_scanout_buffer *sb, struct drm_rect *clip,
 	}
 }
 
-static void drm_panic_fill16(struct iosys_map *dmap, unsigned int dpitch,
-			     unsigned int height, unsigned int width,
-			     u16 color)
-{
-	unsigned int y, x;
-
-	for (y = 0; y < height; y++)
-		for (x = 0; x < width; x++)
-			iosys_map_wr(dmap, y * dpitch + x * sizeof(u16), u16, color);
-}
-
-static void drm_panic_fill24(struct iosys_map *dmap, unsigned int dpitch,
-			     unsigned int height, unsigned int width,
-			     u32 color)
-{
-	unsigned int y, x;
-
-	for (y = 0; y < height; y++) {
-		for (x = 0; x < width; x++) {
-			unsigned int off = y * dpitch + x * 3;
-
-			/* write blue-green-red to output in little endianness */
-			iosys_map_wr(dmap, off, u8, (color & 0x000000FF) >> 0);
-			iosys_map_wr(dmap, off + 1, u8, (color & 0x0000FF00) >> 8);
-			iosys_map_wr(dmap, off + 2, u8, (color & 0x00FF0000) >> 16);
-		}
-	}
-}
-
-static void drm_panic_fill32(struct iosys_map *dmap, unsigned int dpitch,
-			     unsigned int height, unsigned int width,
-			     u32 color)
-{
-	unsigned int y, x;
-
-	for (y = 0; y < height; y++)
-		for (x = 0; x < width; x++)
-			iosys_map_wr(dmap, y * dpitch + x * sizeof(u32), u32, color);
-}
-
 static void drm_panic_fill_pixel(struct drm_scanout_buffer *sb,
 				 struct drm_rect *clip,
 				 u32 color)
@@ -442,15 +230,15 @@ static void drm_panic_fill(struct drm_scanout_buffer *sb, struct drm_rect *clip,
 
 	switch (sb->format->cpp[0]) {
 	case 2:
-		drm_panic_fill16(&map, sb->pitch[0], drm_rect_height(clip),
+		drm_draw_fill16(&map, sb->pitch[0], drm_rect_height(clip),
 				 drm_rect_width(clip), color);
 	break;
 	case 3:
-		drm_panic_fill24(&map, sb->pitch[0], drm_rect_height(clip),
+		drm_draw_fill24(&map, sb->pitch[0], drm_rect_height(clip),
 				 drm_rect_width(clip), color);
 	break;
 	case 4:
-		drm_panic_fill32(&map, sb->pitch[0], drm_rect_height(clip),
+		drm_draw_fill32(&map, sb->pitch[0], drm_rect_height(clip),
 				 drm_rect_width(clip), color);
 	break;
 	default:
@@ -458,11 +246,6 @@ static void drm_panic_fill(struct drm_scanout_buffer *sb, struct drm_rect *clip,
 	}
 }
 
-static const u8 *get_char_bitmap(const struct font_desc *font, char c, size_t font_pitch)
-{
-	return font->data + (c * font->height) * font_pitch;
-}
-
 static unsigned int get_max_line_len(const struct drm_panic_line *lines, int len)
 {
 	int i;
@@ -501,7 +284,7 @@ static void draw_txt_rectangle(struct drm_scanout_buffer *sb,
 			rec.x1 += (drm_rect_width(clip) - (line_len * font->width)) / 2;
 
 		for (j = 0; j < line_len; j++) {
-			src = get_char_bitmap(font, msg[i].txt[j], font_pitch);
+			src = drm_draw_get_char_bitmap(font, msg[i].txt[j], font_pitch);
 			rec.x2 = rec.x1 + font->width;
 			drm_panic_blit(sb, &rec, src, font_pitch, 1, color);
 			rec.x1 += font->width;
@@ -533,8 +316,10 @@ static void drm_panic_logo_draw(struct drm_scanout_buffer *sb, struct drm_rect *
 
 static void draw_panic_static_user(struct drm_scanout_buffer *sb)
 {
-	u32 fg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_FOREGROUND_COLOR, sb->format->format);
-	u32 bg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_BACKGROUND_COLOR, sb->format->format);
+	u32 fg_color = drm_draw_color_from_xrgb8888(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
+						    sb->format->format);
+	u32 bg_color = drm_draw_color_from_xrgb8888(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
+						    sb->format->format);
 	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
 	struct drm_rect r_screen, r_logo, r_msg;
 	unsigned int msg_width, msg_height;
@@ -600,8 +385,10 @@ static int draw_line_with_wrap(struct drm_scanout_buffer *sb, const struct font_
  */
 static void draw_panic_static_kmsg(struct drm_scanout_buffer *sb)
 {
-	u32 fg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_FOREGROUND_COLOR, sb->format->format);
-	u32 bg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_BACKGROUND_COLOR, sb->format->format);
+	u32 fg_color = drm_draw_color_from_xrgb8888(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
+						    sb->format->format);
+	u32 bg_color = drm_draw_color_from_xrgb8888(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
+						    sb->format->format);
 	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
 	struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
 	struct kmsg_dump_iter iter;
@@ -791,8 +578,10 @@ static int drm_panic_get_qr_code(u8 **qr_image)
  */
 static int _draw_panic_static_qr_code(struct drm_scanout_buffer *sb)
 {
-	u32 fg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_FOREGROUND_COLOR, sb->format->format);
-	u32 bg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_BACKGROUND_COLOR, sb->format->format);
+	u32 fg_color = drm_draw_color_from_xrgb8888(CONFIG_DRM_PANIC_FOREGROUND_COLOR,
+						    sb->format->format);
+	u32 bg_color = drm_draw_color_from_xrgb8888(CONFIG_DRM_PANIC_BACKGROUND_COLOR,
+						    sb->format->format);
 	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
 	struct drm_rect r_screen, r_logo, r_msg, r_qr, r_qr_canvas;
 	unsigned int max_qr_size, scale;
@@ -878,7 +667,7 @@ static bool drm_panic_is_format_supported(const struct drm_format_info *format)
 {
 	if (format->num_planes != 1)
 		return false;
-	return convert_from_xrgb8888(0xffffff, format->format) != 0;
+	return drm_draw_color_from_xrgb8888(0xffffff, format->format) != 0;
 }
 
 static void draw_panic_dispatch(struct drm_scanout_buffer *sb)
-- 
2.47.1

[PATCH v9 2/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

drm_log is a simple logger that uses the drm_client API to print the
kmsg boot log on the screen. This is not a full replacement to fbcon,
as it will only print the kmsg. It will never handle user input, or a
terminal because this is better done in userspace.

Design decisions:
 * It uses the drm_client API, so it should work on all drm drivers
   from the start.
 * It doesn't scroll the message, that way it doesn't need to redraw
   the whole screen for each new message.
   It also means it doesn't have to keep drawn messages in memory, to
   redraw them when scrolling.
 * It uses the new non-blocking console API, so it should work well
   with PREEMPT_RT.

This patch also adds a Kconfig menu to select the drm client to use.
It can be overwritten on the kernel command line with:
drm_client_lib.active=log or drm_client_lib.active=fbdev

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: John Ogness <john.ogness@linutronix.de> # console API
---

v2:
 * Use vmap_local() api, with that change, I've tested it successfully on simpledrm, virtio-gpu, amdgpu, and nouveau.
 * Stop drawing when the drm_master is taken. This avoid wasting CPU cycle if the buffer is not visible.
 * Use deferred probe. Only do the probe the first time there is a log to draw. With this, if you boot with quiet, drm_log won't do any modeset.
 * Add color support for the timestamp prefix, like what dmesg does.
 * Add build dependency on  disabling the fbdev emulation, as they are both drm_client, and there is no way to choose which one gets the focus.

v3:
 * Remove the work thread and circular buffer, and use the new write_thread() console API.
 * Register a console for each drm driver.

v4:
 * Can be built as a module, even if that's not really useful.
 * Rebased on top of "drm: Introduce DRM client library" series from Thomas Zimmermann.
 * Add a Kconfig menu to choose between drm client.
 
v5:
 * Build drm_log in drm_client_lib module, to avoid circular dependency. 

v8:
 * Rebased after drm client moved to drivers/gpu/drm/clients/
 * Rename DRM_LOG to DRM_CLIENT_LOG (Thomas Zimmermann)
 * Add an info message if no clients are initialized in drm_client_setup()
 
v9:
 * Add cflags to remove the "../" when including drm internal headers (Jani Nikula)
 * Order select alphabetically in KConfig (Thomas Zimmermann)
 * Replace drm_info with drm_dbg, to be less verbose (Thomas Zimmermann)
 * Rename module parameter to drm_client_lib.active (Thomas Zimmermann)
 * Warn if drm_client_lib.active is malformated (Thomas Zimmermann)
 
 drivers/gpu/drm/clients/Kconfig               |  48 +++
 drivers/gpu/drm/clients/Makefile              |   3 +
 drivers/gpu/drm/clients/drm_client_internal.h |   6 +
 drivers/gpu/drm/clients/drm_client_setup.c    |  29 +-
 drivers/gpu/drm/clients/drm_log.c             | 370 ++++++++++++++++++
 5 files changed, 452 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/clients/drm_log.c

diff --git a/drivers/gpu/drm/clients/Kconfig b/drivers/gpu/drm/clients/Kconfig
index 01ad3b000130..c18decc90200 100644
--- a/drivers/gpu/drm/clients/Kconfig
+++ b/drivers/gpu/drm/clients/Kconfig
@@ -12,6 +12,7 @@ config DRM_CLIENT_LIB
 config DRM_CLIENT_SELECTION
 	tristate
 	depends on DRM
+	select DRM_CLIENT_LIB if DRM_CLIENT_LOG
 	select DRM_CLIENT_LIB if DRM_FBDEV_EMULATION
 	help
 	  Drivers that support in-kernel DRM clients have to select this
@@ -70,4 +71,51 @@ config DRM_FBDEV_LEAK_PHYS_SMEM
 	  If in doubt, say "N" or spread the word to your closed source
 	  library vendor.
 
+config DRM_CLIENT_LOG
+	bool "Print the kernel boot message on the screen"
+	depends on DRM_CLIENT_SELECTION
+	select DRM_CLIENT
+	select DRM_CLIENT_SETUP
+	select DRM_DRAW
+	help
+	  This enable a drm logger, that will print the kernel messages to the
+	  screen until the userspace is ready to take over.
+
+	  If you only need logs, but no terminal, or if you prefer userspace
+	  terminal, say "Y".
+
+choice
+	prompt "Default DRM Client"
+	depends on DRM_CLIENT_SELECTION
+	default DRM_CLIENT_DEFAULT_FBDEV
+	help
+	  Selects the default drm client.
+
+	  The selection made here can be overridden by using the kernel
+	  command line 'drm_client_lib.active=fbdev' option.
+
+config DRM_CLIENT_DEFAULT_FBDEV
+	bool "fbdev"
+	depends on DRM_FBDEV_EMULATION
+	help
+	  Use fbdev emulation as default drm client. This is needed to have
+	  fbcon on top of a drm driver.
+
+config DRM_CLIENT_DEFAULT_LOG
+	bool "log"
+	depends on DRM_CLIENT_LOG
+	help
+	  Use drm log as default drm client. This will display boot logs on the
+	  screen, but doesn't implement a full terminal. For that you will need
+	  a userspace terminal using drm/kms.
+
+endchoice
+
+config DRM_CLIENT_DEFAULT
+       string
+       depends on DRM_CLIENT
+       default "fbdev" if DRM_CLIENT_DEFAULT_FBDEV
+       default "log" if DRM_CLIENT_DEFAULT_LOG
+       default ""
+
 endmenu
diff --git a/drivers/gpu/drm/clients/Makefile b/drivers/gpu/drm/clients/Makefile
index 1d004ec92e1e..c16addbc327f 100644
--- a/drivers/gpu/drm/clients/Makefile
+++ b/drivers/gpu/drm/clients/Makefile
@@ -1,5 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
+subdir-ccflags-y += -I$(src)/..
+
 drm_client_lib-y := drm_client_setup.o
+drm_client_lib-$(CONFIG_DRM_CLIENT_LOG) += drm_log.o
 drm_client_lib-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fbdev_client.o
 obj-$(CONFIG_DRM_CLIENT_LIB) += drm_client_lib.o
diff --git a/drivers/gpu/drm/clients/drm_client_internal.h b/drivers/gpu/drm/clients/drm_client_internal.h
index 23258934956a..6dc078bf6503 100644
--- a/drivers/gpu/drm/clients/drm_client_internal.h
+++ b/drivers/gpu/drm/clients/drm_client_internal.h
@@ -16,4 +16,10 @@ static inline int drm_fbdev_client_setup(struct drm_device *dev,
 }
 #endif
 
+#ifdef CONFIG_DRM_CLIENT_LOG
+void drm_log_register(struct drm_device *dev);
+#else
+static inline void drm_log_register(struct drm_device *dev) {}
+#endif
+
 #endif
diff --git a/drivers/gpu/drm/clients/drm_client_setup.c b/drivers/gpu/drm/clients/drm_client_setup.c
index 4b211a4812b5..e17265039ca8 100644
--- a/drivers/gpu/drm/clients/drm_client_setup.c
+++ b/drivers/gpu/drm/clients/drm_client_setup.c
@@ -7,6 +7,12 @@
 
 #include "drm_client_internal.h"
 
+static char drm_client_default[16] = CONFIG_DRM_CLIENT_DEFAULT;
+module_param_string(active, drm_client_default, sizeof(drm_client_default), 0444);
+MODULE_PARM_DESC(active,
+		 "Choose which drm client to start, default is"
+		 CONFIG_DRM_CLIENT_DEFAULT "]");
+
 /**
  * drm_client_setup() - Setup in-kernel DRM clients
  * @dev: DRM device
@@ -25,11 +31,26 @@
  */
 void drm_client_setup(struct drm_device *dev, const struct drm_format_info *format)
 {
-	int ret;
 
-	ret = drm_fbdev_client_setup(dev, format);
-	if (ret)
-		drm_warn(dev, "Failed to set up DRM client; error %d\n", ret);
+#ifdef CONFIG_DRM_FBDEV_EMULATION
+	if (!strcmp(drm_client_default, "fbdev")) {
+		int ret;
+
+		ret = drm_fbdev_client_setup(dev, format);
+		if (ret)
+			drm_warn(dev, "Failed to set up DRM client; error %d\n", ret);
+		return;
+	}
+#endif
+
+#ifdef CONFIG_DRM_CLIENT_LOG
+	if (!strcmp(drm_client_default, "log")) {
+		drm_log_register(dev);
+		return;
+	}
+#endif
+	if (strcmp(drm_client_default, ""))
+		drm_warn(dev, "Unknown DRM client %s\n", drm_client_default);
 }
 EXPORT_SYMBOL(drm_client_setup);
 
diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
new file mode 100644
index 000000000000..4e07bff6c864
--- /dev/null
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -0,0 +1,370 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/*
+ * Copyright (c) 2024 Red Hat.
+ * Author: Jocelyn Falempe <jfalempe@redhat.com>
+ */
+
+#include <linux/console.h>
+#include <linux/font.h>
+#include <linux/init.h>
+#include <linux/iosys-map.h>
+#include <linux/module.h>
+#include <linux/types.h>
+
+#include <drm/drm_client.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_print.h>
+
+#include "drm_client_internal.h"
+#include "drm_draw_internal.h"
+
+MODULE_AUTHOR("Jocelyn Falempe");
+MODULE_DESCRIPTION("DRM boot logger");
+MODULE_LICENSE("GPL");
+
+/**
+ * DOC: overview
+ *
+ * This is a simple graphic logger, to print the kernel message on screen, until
+ * a userspace application is able to take over.
+ * It is only for debugging purpose.
+ */
+
+struct drm_log_scanout {
+	struct drm_client_buffer *buffer;
+	const struct font_desc *font;
+	u32 rows;
+	u32 columns;
+	u32 line;
+	u32 format;
+	u32 px_width;
+	u32 front_color;
+};
+
+struct drm_log {
+	struct mutex lock;
+	struct drm_client_dev client;
+	struct console con;
+	bool probed;
+	u32 n_scanout;
+	struct drm_log_scanout *scanout;
+};
+
+static struct drm_log *client_to_drm_log(struct drm_client_dev *client)
+{
+	return container_of(client, struct drm_log, client);
+}
+
+static struct drm_log *console_to_drm_log(struct console *con)
+{
+	return container_of(con, struct drm_log, con);
+}
+
+static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
+			 const u8 *src, unsigned int src_pitch,
+			 u32 height, u32 width, u32 scale, u32 px_width, u32 color)
+{
+	switch (px_width) {
+	case 2:
+		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, scale, color);
+		break;
+	case 3:
+		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, scale, color);
+		break;
+	case 4:
+		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, scale, color);
+		break;
+	default:
+		WARN_ONCE(1, "Can't blit with pixel width %d\n", px_width);
+	}
+}
+
+static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line)
+{
+	struct drm_framebuffer *fb = scanout->buffer->fb;
+	unsigned long height = scanout->font->height;
+	struct iosys_map map;
+	struct drm_rect r = DRM_RECT_INIT(0, line * height, fb->width, height);
+
+	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
+		return;
+	iosys_map_memset(&map, r.y1 * fb->pitches[0], 0, height * fb->pitches[0]);
+	drm_client_buffer_vunmap_local(scanout->buffer);
+	drm_client_framebuffer_flush(scanout->buffer, &r);
+}
+
+static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
+			      unsigned int len)
+{
+	struct drm_framebuffer *fb = scanout->buffer->fb;
+	struct iosys_map map;
+	const struct font_desc *font = scanout->font;
+	size_t font_pitch = DIV_ROUND_UP(font->width, 8);
+	const u8 *src;
+	u32 px_width = fb->format->cpp[0];
+	struct drm_rect r = DRM_RECT_INIT(0, scanout->line * font->height,
+					  fb->width, (scanout->line + 1) * font->height);
+	u32 i;
+
+	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
+		return;
+
+	iosys_map_incr(&map, r.y1 * fb->pitches[0]);
+	for (i = 0; i < len && i < scanout->columns; i++) {
+		src = drm_draw_get_char_bitmap(font, s[i], font_pitch);
+		drm_log_blit(&map, fb->pitches[0], src, font_pitch, font->height, font->width,
+			     1, px_width, scanout->front_color);
+		iosys_map_incr(&map, font->width * px_width);
+	}
+
+	scanout->line++;
+	if (scanout->line >= scanout->rows)
+		scanout->line = 0;
+	drm_client_buffer_vunmap_local(scanout->buffer);
+	drm_client_framebuffer_flush(scanout->buffer, &r);
+}
+
+static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
+				  const char *s, unsigned int len)
+{
+	if (scanout->line == 0) {
+		drm_log_clear_line(scanout, 0);
+		drm_log_clear_line(scanout, 1);
+		drm_log_clear_line(scanout, 2);
+	} else if (scanout->line + 2 < scanout->rows)
+		drm_log_clear_line(scanout, scanout->line + 2);
+
+	drm_log_draw_line(scanout, s, len);
+}
+
+static void drm_log_draw_kmsg_record(struct drm_log_scanout *scanout,
+				     const char *s, unsigned int len)
+{
+	/* do not print the ending \n character */
+	if (s[len - 1] == '\n')
+		len--;
+
+	while (len > scanout->columns) {
+		drm_log_draw_new_line(scanout, s, scanout->columns);
+		s += scanout->columns;
+		len -= scanout->columns;
+	}
+	if (len)
+		drm_log_draw_new_line(scanout, s, len);
+}
+
+static u32 drm_log_find_usable_format(struct drm_plane *plane)
+{
+	int i;
+
+	for (i = 0; i < plane->format_count; i++)
+		if (drm_draw_color_from_xrgb8888(0xffffff, plane->format_types[i]) != 0)
+			return plane->format_types[i];
+	return DRM_FORMAT_INVALID;
+}
+
+static int drm_log_setup_modeset(struct drm_client_dev *client,
+				 struct drm_mode_set *mode_set,
+				 struct drm_log_scanout *scanout)
+{
+	struct drm_crtc *crtc = mode_set->crtc;
+	u32 width = mode_set->mode->hdisplay;
+	u32 height = mode_set->mode->vdisplay;
+	u32 format;
+
+	scanout->font = get_default_font(width, height, NULL, NULL);
+	if (!scanout->font)
+		return -ENOENT;
+
+	format = drm_log_find_usable_format(crtc->primary);
+	if (format == DRM_FORMAT_INVALID)
+		return -EINVAL;
+
+	scanout->buffer = drm_client_framebuffer_create(client, width, height, format);
+	if (IS_ERR(scanout->buffer)) {
+		drm_warn(client->dev, "drm_log can't create framebuffer %d %d %p4cc\n",
+			 width, height, &format);
+		return -ENOMEM;
+	}
+	mode_set->fb = scanout->buffer->fb;
+	scanout->rows = height / scanout->font->height;
+	scanout->columns = width / scanout->font->width;
+	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);
+	return 0;
+}
+
+static int drm_log_count_modeset(struct drm_client_dev *client)
+{
+	struct drm_mode_set *mode_set;
+	int count = 0;
+
+	mutex_lock(&client->modeset_mutex);
+	drm_client_for_each_modeset(mode_set, client)
+		count++;
+	mutex_unlock(&client->modeset_mutex);
+	return count;
+}
+
+static void drm_log_init_client(struct drm_log *dlog)
+{
+	struct drm_client_dev *client = &dlog->client;
+	struct drm_mode_set *mode_set;
+	int i, max_modeset;
+	int n_modeset = 0;
+
+	dlog->probed = true;
+
+	if (drm_client_modeset_probe(client, 0, 0))
+		return;
+
+	max_modeset = drm_log_count_modeset(client);
+	if (!max_modeset)
+		return;
+
+	dlog->scanout = kcalloc(max_modeset, sizeof(*dlog->scanout), GFP_KERNEL);
+	if (!dlog->scanout)
+		return;
+
+	mutex_lock(&client->modeset_mutex);
+	drm_client_for_each_modeset(mode_set, client) {
+		if (!mode_set->mode)
+			continue;
+		if (drm_log_setup_modeset(client, mode_set, &dlog->scanout[n_modeset]))
+			continue;
+		n_modeset++;
+	}
+	mutex_unlock(&client->modeset_mutex);
+	if (n_modeset == 0)
+		goto err_nomodeset;
+
+	if (drm_client_modeset_commit(client))
+		goto err_failed_commit;
+
+	dlog->n_scanout = n_modeset;
+	return;
+
+err_failed_commit:
+	for (i = 0; i < n_modeset; i++)
+		drm_client_framebuffer_delete(dlog->scanout[i].buffer);
+
+err_nomodeset:
+	kfree(dlog->scanout);
+	dlog->scanout = NULL;
+}
+
+static void drm_log_free_scanout(struct drm_client_dev *client)
+{
+	struct drm_log *dlog = client_to_drm_log(client);
+	int i;
+
+	if (dlog->n_scanout) {
+		for (i = 0; i < dlog->n_scanout; i++)
+			drm_client_framebuffer_delete(dlog->scanout[i].buffer);
+		dlog->n_scanout = 0;
+		kfree(dlog->scanout);
+		dlog->scanout = NULL;
+	}
+}
+
+static void drm_log_client_unregister(struct drm_client_dev *client)
+{
+	struct drm_log *dlog = client_to_drm_log(client);
+	struct drm_device *dev = client->dev;
+
+	unregister_console(&dlog->con);
+
+	mutex_lock(&dlog->lock);
+	drm_log_free_scanout(client);
+	drm_client_release(client);
+	mutex_unlock(&dlog->lock);
+	kfree(dlog);
+	drm_dbg(dev, "Unregistered with drm log\n");
+}
+
+static int drm_log_client_hotplug(struct drm_client_dev *client)
+{
+	struct drm_log *dlog = client_to_drm_log(client);
+
+	mutex_lock(&dlog->lock);
+	drm_log_free_scanout(client);
+	dlog->probed = false;
+	mutex_unlock(&dlog->lock);
+	return 0;
+}
+
+static const struct drm_client_funcs drm_log_client_funcs = {
+	.owner		= THIS_MODULE,
+	.unregister	= drm_log_client_unregister,
+	.hotplug	= drm_log_client_hotplug,
+};
+
+static void drm_log_write_thread(struct console *con, struct nbcon_write_context *wctxt)
+{
+	struct drm_log *dlog = console_to_drm_log(con);
+	int i;
+
+	if (!dlog->probed)
+		drm_log_init_client(dlog);
+
+	for (i = 0; i < dlog->n_scanout; i++)
+		drm_log_draw_kmsg_record(&dlog->scanout[i], wctxt->outbuf, wctxt->len);
+}
+
+static void drm_log_lock(struct console *con, unsigned long *flags)
+{
+	struct drm_log *dlog = console_to_drm_log(con);
+
+	mutex_lock(&dlog->lock);
+	migrate_disable();
+}
+
+static void drm_log_unlock(struct console *con, unsigned long flags)
+{
+	struct drm_log *dlog = console_to_drm_log(con);
+
+	migrate_enable();
+	mutex_unlock(&dlog->lock);
+}
+
+static void drm_log_register_console(struct console *con)
+{
+	strscpy(con->name, "drm_log");
+	con->write_thread = drm_log_write_thread;
+	con->device_lock = drm_log_lock;
+	con->device_unlock = drm_log_unlock;
+	con->flags = CON_PRINTBUFFER | CON_NBCON;
+	con->index = -1;
+
+	register_console(con);
+}
+
+/**
+ * drm_log_register() - Register a drm device to drm_log
+ * @dev: the drm device to register.
+ */
+void drm_log_register(struct drm_device *dev)
+{
+	struct drm_log *new;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		goto err_warn;
+
+	mutex_init(&new->lock);
+	if (drm_client_init(dev, &new->client, "drm_log", &drm_log_client_funcs))
+		goto err_free;
+
+	drm_client_register(&new->client);
+
+	drm_log_register_console(&new->con);
+
+	drm_dbg(dev, "Registered with drm log as %s\n", new->con.name);
+	return;
+
+err_free:
+	kfree(new);
+err_warn:
+	drm_warn(dev, "Failed to register with drm log\n");
+}
-- 
2.47.1

[PATCH v9 3/6] drm/log: Do not draw if drm_master is taken

[Jocelyn Falempe]

When userspace takes drm_master, the drm_client buffer is no more
visible, so drm_log shouldn't waste CPU cycle to draw on it.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/clients/drm_log.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index 4e07bff6c864..a933da024b1a 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -19,6 +19,7 @@
 
 #include "drm_client_internal.h"
 #include "drm_draw_internal.h"
+#include "drm_internal.h"
 
 MODULE_AUTHOR("Jocelyn Falempe");
 MODULE_DESCRIPTION("DRM boot logger");
@@ -308,8 +309,13 @@ static void drm_log_write_thread(struct console *con, struct nbcon_write_context
 	if (!dlog->probed)
 		drm_log_init_client(dlog);
 
-	for (i = 0; i < dlog->n_scanout; i++)
-		drm_log_draw_kmsg_record(&dlog->scanout[i], wctxt->outbuf, wctxt->len);
+	/* Check that we are still the master before drawing */
+	if (drm_master_internal_acquire(dlog->client.dev)) {
+		drm_master_internal_release(dlog->client.dev);
+
+		for (i = 0; i < dlog->n_scanout; i++)
+			drm_log_draw_kmsg_record(&dlog->scanout[i], wctxt->outbuf, wctxt->len);
+	}
 }
 
 static void drm_log_lock(struct console *con, unsigned long *flags)
-- 
2.47.1

[PATCH v9 4/6] drm/log: Color the timestamp, to improve readability

[Jocelyn Falempe]

Color the timesamp prefix, similar to dmesg.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/clients/drm_log.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index a933da024b1a..5a995742aec5 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -42,6 +42,7 @@ struct drm_log_scanout {
 	u32 format;
 	u32 px_width;
 	u32 front_color;
+	u32 prefix_color;
 };
 
 struct drm_log {
@@ -97,7 +98,7 @@ static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line)
 }
 
 static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
-			      unsigned int len)
+			      unsigned int len, unsigned int prefix_len)
 {
 	struct drm_framebuffer *fb = scanout->buffer->fb;
 	struct iosys_map map;
@@ -114,9 +115,10 @@ static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
 
 	iosys_map_incr(&map, r.y1 * fb->pitches[0]);
 	for (i = 0; i < len && i < scanout->columns; i++) {
+		u32 color = (i < prefix_len) ? scanout->prefix_color : scanout->front_color;
 		src = drm_draw_get_char_bitmap(font, s[i], font_pitch);
 		drm_log_blit(&map, fb->pitches[0], src, font_pitch, font->height, font->width,
-			     1, px_width, scanout->front_color);
+			     1, px_width, color);
 		iosys_map_incr(&map, font->width * px_width);
 	}
 
@@ -128,7 +130,7 @@ static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
 }
 
 static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
-				  const char *s, unsigned int len)
+				  const char *s, unsigned int len, unsigned int prefix_len)
 {
 	if (scanout->line == 0) {
 		drm_log_clear_line(scanout, 0);
@@ -137,23 +139,35 @@ static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
 	} else if (scanout->line + 2 < scanout->rows)
 		drm_log_clear_line(scanout, scanout->line + 2);
 
-	drm_log_draw_line(scanout, s, len);
+	drm_log_draw_line(scanout, s, len, prefix_len);
 }
 
+/*
+ * Depends on print_time() in printk.c
+ * Timestamp is written with "[%5lu.%06lu]"
+ */
+#define TS_PREFIX_LEN 13
+
 static void drm_log_draw_kmsg_record(struct drm_log_scanout *scanout,
 				     const char *s, unsigned int len)
 {
+	u32 prefix_len = 0;
+
+	if (len > TS_PREFIX_LEN && s[0] == '[' && s[6] == '.' && s[TS_PREFIX_LEN] == ']')
+		prefix_len = TS_PREFIX_LEN + 1;
+
 	/* do not print the ending \n character */
 	if (s[len - 1] == '\n')
 		len--;
 
 	while (len > scanout->columns) {
-		drm_log_draw_new_line(scanout, s, scanout->columns);
+		drm_log_draw_new_line(scanout, s, scanout->columns, prefix_len);
 		s += scanout->columns;
 		len -= scanout->columns;
+		prefix_len = 0;
 	}
 	if (len)
-		drm_log_draw_new_line(scanout, s, len);
+		drm_log_draw_new_line(scanout, s, len, prefix_len);
 }
 
 static u32 drm_log_find_usable_format(struct drm_plane *plane)
@@ -193,6 +207,7 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
 	scanout->rows = height / scanout->font->height;
 	scanout->columns = width / scanout->font->width;
 	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);
+	scanout->prefix_color = drm_draw_color_from_xrgb8888(0x4e9a06, format);
 	return 0;
 }
 
-- 
2.47.1

[PATCH v9 5/6] drm/log: Implement suspend/resume

[Jocelyn Falempe]

Normally the console is already suspended when the graphic driver
suspend callback is called, but if the parameter no_console_suspend
is set, it might still be active.
So call console_stop()/console_start() in the suspend/resume
callbacks, to make sure it won't try to write to the framebuffer
while the graphic driver is suspended.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: John Ogness <john.ogness@linutronix.de>
Acked-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---


v6:
 * Use console_stop() and console_start() in the suspend/resume callback (Petr Mladek).

 drivers/gpu/drm/clients/drm_log.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index 5a995742aec5..d5f9c679f2c0 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -310,10 +310,30 @@ static int drm_log_client_hotplug(struct drm_client_dev *client)
 	return 0;
 }
 
+static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_lock)
+{
+	struct drm_log *dlog = client_to_drm_log(client);
+
+	console_stop(&dlog->con);
+
+	return 0;
+}
+
+static int drm_log_client_resume(struct drm_client_dev *client, bool _console_lock)
+{
+	struct drm_log *dlog = client_to_drm_log(client);
+
+	console_start(&dlog->con);
+
+	return 0;
+}
+
 static const struct drm_client_funcs drm_log_client_funcs = {
 	.owner		= THIS_MODULE,
 	.unregister	= drm_log_client_unregister,
 	.hotplug	= drm_log_client_hotplug,
+	.suspend	= drm_log_client_suspend,
+	.resume		= drm_log_client_resume,
 };
 
 static void drm_log_write_thread(struct console *con, struct nbcon_write_context *wctxt)
-- 
2.47.1

[PATCH v9 6/6] drm/log: Add integer scaling support

[Jocelyn Falempe]

Add a module parameter, to increase the font size for HiDPI screen.
Even with CONFIG_FONT_TER16x32, it can still be a bit small to read.
In this case, adding drm_log.scale=2 to your kernel command line will
double the character size.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---

v5:
 * Change scale parameter to unsigned int (Jani Nikula)

 drivers/gpu/drm/clients/drm_log.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index d5f9c679f2c0..379850c83e51 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -25,6 +25,10 @@ MODULE_AUTHOR("Jocelyn Falempe");
 MODULE_DESCRIPTION("DRM boot logger");
 MODULE_LICENSE("GPL");
 
+static unsigned int scale = 1;
+module_param(scale, uint, 0444);
+MODULE_PARM_DESC(scale, "Integer scaling factor for drm_log, default is 1");
+
 /**
  * DOC: overview
  *
@@ -38,6 +42,8 @@ struct drm_log_scanout {
 	const struct font_desc *font;
 	u32 rows;
 	u32 columns;
+	u32 scaled_font_h;
+	u32 scaled_font_w;
 	u32 line;
 	u32 format;
 	u32 px_width;
@@ -66,7 +72,7 @@ static struct drm_log *console_to_drm_log(struct console *con)
 
 static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
 			 const u8 *src, unsigned int src_pitch,
-			 u32 height, u32 width, u32 scale, u32 px_width, u32 color)
+			 u32 height, u32 width, u32 px_width, u32 color)
 {
 	switch (px_width) {
 	case 2:
@@ -86,7 +92,7 @@ static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
 static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line)
 {
 	struct drm_framebuffer *fb = scanout->buffer->fb;
-	unsigned long height = scanout->font->height;
+	unsigned long height = scanout->scaled_font_h;
 	struct iosys_map map;
 	struct drm_rect r = DRM_RECT_INIT(0, line * height, fb->width, height);
 
@@ -106,8 +112,8 @@ static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
 	size_t font_pitch = DIV_ROUND_UP(font->width, 8);
 	const u8 *src;
 	u32 px_width = fb->format->cpp[0];
-	struct drm_rect r = DRM_RECT_INIT(0, scanout->line * font->height,
-					  fb->width, (scanout->line + 1) * font->height);
+	struct drm_rect r = DRM_RECT_INIT(0, scanout->line * scanout->scaled_font_h,
+					  fb->width, (scanout->line + 1) * scanout->scaled_font_h);
 	u32 i;
 
 	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
@@ -117,9 +123,10 @@ static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
 	for (i = 0; i < len && i < scanout->columns; i++) {
 		u32 color = (i < prefix_len) ? scanout->prefix_color : scanout->front_color;
 		src = drm_draw_get_char_bitmap(font, s[i], font_pitch);
-		drm_log_blit(&map, fb->pitches[0], src, font_pitch, font->height, font->width,
-			     1, px_width, color);
-		iosys_map_incr(&map, font->width * px_width);
+		drm_log_blit(&map, fb->pitches[0], src, font_pitch,
+			     scanout->scaled_font_h, scanout->scaled_font_w,
+			     px_width, color);
+		iosys_map_incr(&map, scanout->scaled_font_w * px_width);
 	}
 
 	scanout->line++;
@@ -204,8 +211,10 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
 		return -ENOMEM;
 	}
 	mode_set->fb = scanout->buffer->fb;
-	scanout->rows = height / scanout->font->height;
-	scanout->columns = width / scanout->font->width;
+	scanout->scaled_font_h = scanout->font->height * scale;
+	scanout->scaled_font_w = scanout->font->width * scale;
+	scanout->rows = height / scanout->scaled_font_h;
+	scanout->columns = width / scanout->scaled_font_w;
 	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);
 	scanout->prefix_color = drm_draw_color_from_xrgb8888(0x4e9a06, format);
 	return 0;
-- 
2.47.1

Re: [PATCH v9 2/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Thomas Zimmermann]

Am 04.12.24 um 16:45 schrieb Jocelyn Falempe:
> drm_log is a simple logger that uses the drm_client API to print the
> kmsg boot log on the screen. This is not a full replacement to fbcon,
> as it will only print the kmsg. It will never handle user input, or a
> terminal because this is better done in userspace.
>
> Design decisions:
>   * It uses the drm_client API, so it should work on all drm drivers
>     from the start.
>   * It doesn't scroll the message, that way it doesn't need to redraw
>     the whole screen for each new message.
>     It also means it doesn't have to keep drawn messages in memory, to
>     redraw them when scrolling.
>   * It uses the new non-blocking console API, so it should work well
>     with PREEMPT_RT.
>
> This patch also adds a Kconfig menu to select the drm client to use.
> It can be overwritten on the kernel command line with:
> drm_client_lib.active=log or drm_client_lib.active=fbdev
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Reviewed-by: John Ogness <john.ogness@linutronix.de> # console API

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>
> v2:
>   * Use vmap_local() api, with that change, I've tested it successfully on simpledrm, virtio-gpu, amdgpu, and nouveau.
>   * Stop drawing when the drm_master is taken. This avoid wasting CPU cycle if the buffer is not visible.
>   * Use deferred probe. Only do the probe the first time there is a log to draw. With this, if you boot with quiet, drm_log won't do any modeset.
>   * Add color support for the timestamp prefix, like what dmesg does.
>   * Add build dependency on  disabling the fbdev emulation, as they are both drm_client, and there is no way to choose which one gets the focus.
>
> v3:
>   * Remove the work thread and circular buffer, and use the new write_thread() console API.
>   * Register a console for each drm driver.
>
> v4:
>   * Can be built as a module, even if that's not really useful.
>   * Rebased on top of "drm: Introduce DRM client library" series from Thomas Zimmermann.
>   * Add a Kconfig menu to choose between drm client.
>   
> v5:
>   * Build drm_log in drm_client_lib module, to avoid circular dependency.
>
> v8:
>   * Rebased after drm client moved to drivers/gpu/drm/clients/
>   * Rename DRM_LOG to DRM_CLIENT_LOG (Thomas Zimmermann)
>   * Add an info message if no clients are initialized in drm_client_setup()
>   
> v9:
>   * Add cflags to remove the "../" when including drm internal headers (Jani Nikula)
>   * Order select alphabetically in KConfig (Thomas Zimmermann)
>   * Replace drm_info with drm_dbg, to be less verbose (Thomas Zimmermann)
>   * Rename module parameter to drm_client_lib.active (Thomas Zimmermann)
>   * Warn if drm_client_lib.active is malformated (Thomas Zimmermann)
>   
>   drivers/gpu/drm/clients/Kconfig               |  48 +++
>   drivers/gpu/drm/clients/Makefile              |   3 +
>   drivers/gpu/drm/clients/drm_client_internal.h |   6 +
>   drivers/gpu/drm/clients/drm_client_setup.c    |  29 +-
>   drivers/gpu/drm/clients/drm_log.c             | 370 ++++++++++++++++++
>   5 files changed, 452 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/gpu/drm/clients/drm_log.c
>
> diff --git a/drivers/gpu/drm/clients/Kconfig b/drivers/gpu/drm/clients/Kconfig
> index 01ad3b000130..c18decc90200 100644
> --- a/drivers/gpu/drm/clients/Kconfig
> +++ b/drivers/gpu/drm/clients/Kconfig
> @@ -12,6 +12,7 @@ config DRM_CLIENT_LIB
>   config DRM_CLIENT_SELECTION
>   	tristate
>   	depends on DRM
> +	select DRM_CLIENT_LIB if DRM_CLIENT_LOG
>   	select DRM_CLIENT_LIB if DRM_FBDEV_EMULATION
>   	help
>   	  Drivers that support in-kernel DRM clients have to select this
> @@ -70,4 +71,51 @@ config DRM_FBDEV_LEAK_PHYS_SMEM
>   	  If in doubt, say "N" or spread the word to your closed source
>   	  library vendor.
>   
> +config DRM_CLIENT_LOG
> +	bool "Print the kernel boot message on the screen"
> +	depends on DRM_CLIENT_SELECTION
> +	select DRM_CLIENT
> +	select DRM_CLIENT_SETUP
> +	select DRM_DRAW
> +	help
> +	  This enable a drm logger, that will print the kernel messages to the
> +	  screen until the userspace is ready to take over.
> +
> +	  If you only need logs, but no terminal, or if you prefer userspace
> +	  terminal, say "Y".
> +
> +choice
> +	prompt "Default DRM Client"
> +	depends on DRM_CLIENT_SELECTION
> +	default DRM_CLIENT_DEFAULT_FBDEV
> +	help
> +	  Selects the default drm client.
> +
> +	  The selection made here can be overridden by using the kernel
> +	  command line 'drm_client_lib.active=fbdev' option.
> +
> +config DRM_CLIENT_DEFAULT_FBDEV
> +	bool "fbdev"
> +	depends on DRM_FBDEV_EMULATION
> +	help
> +	  Use fbdev emulation as default drm client. This is needed to have
> +	  fbcon on top of a drm driver.
> +
> +config DRM_CLIENT_DEFAULT_LOG
> +	bool "log"
> +	depends on DRM_CLIENT_LOG
> +	help
> +	  Use drm log as default drm client. This will display boot logs on the
> +	  screen, but doesn't implement a full terminal. For that you will need
> +	  a userspace terminal using drm/kms.
> +
> +endchoice
> +
> +config DRM_CLIENT_DEFAULT
> +       string
> +       depends on DRM_CLIENT
> +       default "fbdev" if DRM_CLIENT_DEFAULT_FBDEV
> +       default "log" if DRM_CLIENT_DEFAULT_LOG
> +       default ""
> +
>   endmenu
> diff --git a/drivers/gpu/drm/clients/Makefile b/drivers/gpu/drm/clients/Makefile
> index 1d004ec92e1e..c16addbc327f 100644
> --- a/drivers/gpu/drm/clients/Makefile
> +++ b/drivers/gpu/drm/clients/Makefile
> @@ -1,5 +1,8 @@
>   # SPDX-License-Identifier: GPL-2.0
>   
> +subdir-ccflags-y += -I$(src)/..
> +
>   drm_client_lib-y := drm_client_setup.o
> +drm_client_lib-$(CONFIG_DRM_CLIENT_LOG) += drm_log.o
>   drm_client_lib-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fbdev_client.o
>   obj-$(CONFIG_DRM_CLIENT_LIB) += drm_client_lib.o
> diff --git a/drivers/gpu/drm/clients/drm_client_internal.h b/drivers/gpu/drm/clients/drm_client_internal.h
> index 23258934956a..6dc078bf6503 100644
> --- a/drivers/gpu/drm/clients/drm_client_internal.h
> +++ b/drivers/gpu/drm/clients/drm_client_internal.h
> @@ -16,4 +16,10 @@ static inline int drm_fbdev_client_setup(struct drm_device *dev,
>   }
>   #endif
>   
> +#ifdef CONFIG_DRM_CLIENT_LOG
> +void drm_log_register(struct drm_device *dev);
> +#else
> +static inline void drm_log_register(struct drm_device *dev) {}
> +#endif
> +
>   #endif
> diff --git a/drivers/gpu/drm/clients/drm_client_setup.c b/drivers/gpu/drm/clients/drm_client_setup.c
> index 4b211a4812b5..e17265039ca8 100644
> --- a/drivers/gpu/drm/clients/drm_client_setup.c
> +++ b/drivers/gpu/drm/clients/drm_client_setup.c
> @@ -7,6 +7,12 @@
>   
>   #include "drm_client_internal.h"
>   
> +static char drm_client_default[16] = CONFIG_DRM_CLIENT_DEFAULT;
> +module_param_string(active, drm_client_default, sizeof(drm_client_default), 0444);
> +MODULE_PARM_DESC(active,
> +		 "Choose which drm client to start, default is"
> +		 CONFIG_DRM_CLIENT_DEFAULT "]");
> +
>   /**
>    * drm_client_setup() - Setup in-kernel DRM clients
>    * @dev: DRM device
> @@ -25,11 +31,26 @@
>    */
>   void drm_client_setup(struct drm_device *dev, const struct drm_format_info *format)
>   {
> -	int ret;
>   
> -	ret = drm_fbdev_client_setup(dev, format);
> -	if (ret)
> -		drm_warn(dev, "Failed to set up DRM client; error %d\n", ret);
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> +	if (!strcmp(drm_client_default, "fbdev")) {
> +		int ret;
> +
> +		ret = drm_fbdev_client_setup(dev, format);
> +		if (ret)
> +			drm_warn(dev, "Failed to set up DRM client; error %d\n", ret);
> +		return;
> +	}
> +#endif
> +
> +#ifdef CONFIG_DRM_CLIENT_LOG
> +	if (!strcmp(drm_client_default, "log")) {
> +		drm_log_register(dev);
> +		return;
> +	}
> +#endif
> +	if (strcmp(drm_client_default, ""))
> +		drm_warn(dev, "Unknown DRM client %s\n", drm_client_default);
>   }
>   EXPORT_SYMBOL(drm_client_setup);
>   
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> new file mode 100644
> index 000000000000..4e07bff6c864
> --- /dev/null
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -0,0 +1,370 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/*
> + * Copyright (c) 2024 Red Hat.
> + * Author: Jocelyn Falempe <jfalempe@redhat.com>
> + */
> +
> +#include <linux/console.h>
> +#include <linux/font.h>
> +#include <linux/init.h>
> +#include <linux/iosys-map.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +
> +#include <drm/drm_client.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_print.h>
> +
> +#include "drm_client_internal.h"
> +#include "drm_draw_internal.h"
> +
> +MODULE_AUTHOR("Jocelyn Falempe");
> +MODULE_DESCRIPTION("DRM boot logger");
> +MODULE_LICENSE("GPL");
> +
> +/**
> + * DOC: overview
> + *
> + * This is a simple graphic logger, to print the kernel message on screen, until
> + * a userspace application is able to take over.
> + * It is only for debugging purpose.
> + */
> +
> +struct drm_log_scanout {
> +	struct drm_client_buffer *buffer;
> +	const struct font_desc *font;
> +	u32 rows;
> +	u32 columns;
> +	u32 line;
> +	u32 format;
> +	u32 px_width;
> +	u32 front_color;
> +};
> +
> +struct drm_log {
> +	struct mutex lock;
> +	struct drm_client_dev client;
> +	struct console con;
> +	bool probed;
> +	u32 n_scanout;
> +	struct drm_log_scanout *scanout;
> +};
> +
> +static struct drm_log *client_to_drm_log(struct drm_client_dev *client)
> +{
> +	return container_of(client, struct drm_log, client);
> +}
> +
> +static struct drm_log *console_to_drm_log(struct console *con)
> +{
> +	return container_of(con, struct drm_log, con);
> +}
> +
> +static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
> +			 const u8 *src, unsigned int src_pitch,
> +			 u32 height, u32 width, u32 scale, u32 px_width, u32 color)
> +{
> +	switch (px_width) {
> +	case 2:
> +		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		break;
> +	case 3:
> +		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		break;
> +	case 4:
> +		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		break;
> +	default:
> +		WARN_ONCE(1, "Can't blit with pixel width %d\n", px_width);
> +	}
> +}
> +
> +static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line)
> +{
> +	struct drm_framebuffer *fb = scanout->buffer->fb;
> +	unsigned long height = scanout->font->height;
> +	struct iosys_map map;
> +	struct drm_rect r = DRM_RECT_INIT(0, line * height, fb->width, height);
> +
> +	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
> +		return;
> +	iosys_map_memset(&map, r.y1 * fb->pitches[0], 0, height * fb->pitches[0]);
> +	drm_client_buffer_vunmap_local(scanout->buffer);
> +	drm_client_framebuffer_flush(scanout->buffer, &r);
> +}
> +
> +static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
> +			      unsigned int len)
> +{
> +	struct drm_framebuffer *fb = scanout->buffer->fb;
> +	struct iosys_map map;
> +	const struct font_desc *font = scanout->font;
> +	size_t font_pitch = DIV_ROUND_UP(font->width, 8);
> +	const u8 *src;
> +	u32 px_width = fb->format->cpp[0];
> +	struct drm_rect r = DRM_RECT_INIT(0, scanout->line * font->height,
> +					  fb->width, (scanout->line + 1) * font->height);
> +	u32 i;
> +
> +	if (drm_client_buffer_vmap_local(scanout->buffer, &map))
> +		return;
> +
> +	iosys_map_incr(&map, r.y1 * fb->pitches[0]);
> +	for (i = 0; i < len && i < scanout->columns; i++) {
> +		src = drm_draw_get_char_bitmap(font, s[i], font_pitch);
> +		drm_log_blit(&map, fb->pitches[0], src, font_pitch, font->height, font->width,
> +			     1, px_width, scanout->front_color);
> +		iosys_map_incr(&map, font->width * px_width);
> +	}
> +
> +	scanout->line++;
> +	if (scanout->line >= scanout->rows)
> +		scanout->line = 0;
> +	drm_client_buffer_vunmap_local(scanout->buffer);
> +	drm_client_framebuffer_flush(scanout->buffer, &r);
> +}
> +
> +static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
> +				  const char *s, unsigned int len)
> +{
> +	if (scanout->line == 0) {
> +		drm_log_clear_line(scanout, 0);
> +		drm_log_clear_line(scanout, 1);
> +		drm_log_clear_line(scanout, 2);
> +	} else if (scanout->line + 2 < scanout->rows)
> +		drm_log_clear_line(scanout, scanout->line + 2);
> +
> +	drm_log_draw_line(scanout, s, len);
> +}
> +
> +static void drm_log_draw_kmsg_record(struct drm_log_scanout *scanout,
> +				     const char *s, unsigned int len)
> +{
> +	/* do not print the ending \n character */
> +	if (s[len - 1] == '\n')
> +		len--;
> +
> +	while (len > scanout->columns) {
> +		drm_log_draw_new_line(scanout, s, scanout->columns);
> +		s += scanout->columns;
> +		len -= scanout->columns;
> +	}
> +	if (len)
> +		drm_log_draw_new_line(scanout, s, len);
> +}
> +
> +static u32 drm_log_find_usable_format(struct drm_plane *plane)
> +{
> +	int i;
> +
> +	for (i = 0; i < plane->format_count; i++)
> +		if (drm_draw_color_from_xrgb8888(0xffffff, plane->format_types[i]) != 0)
> +			return plane->format_types[i];
> +	return DRM_FORMAT_INVALID;
> +}
> +
> +static int drm_log_setup_modeset(struct drm_client_dev *client,
> +				 struct drm_mode_set *mode_set,
> +				 struct drm_log_scanout *scanout)
> +{
> +	struct drm_crtc *crtc = mode_set->crtc;
> +	u32 width = mode_set->mode->hdisplay;
> +	u32 height = mode_set->mode->vdisplay;
> +	u32 format;
> +
> +	scanout->font = get_default_font(width, height, NULL, NULL);
> +	if (!scanout->font)
> +		return -ENOENT;
> +
> +	format = drm_log_find_usable_format(crtc->primary);
> +	if (format == DRM_FORMAT_INVALID)
> +		return -EINVAL;
> +
> +	scanout->buffer = drm_client_framebuffer_create(client, width, height, format);
> +	if (IS_ERR(scanout->buffer)) {
> +		drm_warn(client->dev, "drm_log can't create framebuffer %d %d %p4cc\n",
> +			 width, height, &format);
> +		return -ENOMEM;
> +	}
> +	mode_set->fb = scanout->buffer->fb;
> +	scanout->rows = height / scanout->font->height;
> +	scanout->columns = width / scanout->font->width;
> +	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);
> +	return 0;
> +}
> +
> +static int drm_log_count_modeset(struct drm_client_dev *client)
> +{
> +	struct drm_mode_set *mode_set;
> +	int count = 0;
> +
> +	mutex_lock(&client->modeset_mutex);
> +	drm_client_for_each_modeset(mode_set, client)
> +		count++;
> +	mutex_unlock(&client->modeset_mutex);
> +	return count;
> +}
> +
> +static void drm_log_init_client(struct drm_log *dlog)
> +{
> +	struct drm_client_dev *client = &dlog->client;
> +	struct drm_mode_set *mode_set;
> +	int i, max_modeset;
> +	int n_modeset = 0;
> +
> +	dlog->probed = true;
> +
> +	if (drm_client_modeset_probe(client, 0, 0))
> +		return;
> +
> +	max_modeset = drm_log_count_modeset(client);
> +	if (!max_modeset)
> +		return;
> +
> +	dlog->scanout = kcalloc(max_modeset, sizeof(*dlog->scanout), GFP_KERNEL);
> +	if (!dlog->scanout)
> +		return;
> +
> +	mutex_lock(&client->modeset_mutex);
> +	drm_client_for_each_modeset(mode_set, client) {
> +		if (!mode_set->mode)
> +			continue;
> +		if (drm_log_setup_modeset(client, mode_set, &dlog->scanout[n_modeset]))
> +			continue;
> +		n_modeset++;
> +	}
> +	mutex_unlock(&client->modeset_mutex);
> +	if (n_modeset == 0)
> +		goto err_nomodeset;
> +
> +	if (drm_client_modeset_commit(client))
> +		goto err_failed_commit;
> +
> +	dlog->n_scanout = n_modeset;
> +	return;
> +
> +err_failed_commit:
> +	for (i = 0; i < n_modeset; i++)
> +		drm_client_framebuffer_delete(dlog->scanout[i].buffer);
> +
> +err_nomodeset:
> +	kfree(dlog->scanout);
> +	dlog->scanout = NULL;
> +}
> +
> +static void drm_log_free_scanout(struct drm_client_dev *client)
> +{
> +	struct drm_log *dlog = client_to_drm_log(client);
> +	int i;
> +
> +	if (dlog->n_scanout) {
> +		for (i = 0; i < dlog->n_scanout; i++)
> +			drm_client_framebuffer_delete(dlog->scanout[i].buffer);
> +		dlog->n_scanout = 0;
> +		kfree(dlog->scanout);
> +		dlog->scanout = NULL;
> +	}
> +}
> +
> +static void drm_log_client_unregister(struct drm_client_dev *client)
> +{
> +	struct drm_log *dlog = client_to_drm_log(client);
> +	struct drm_device *dev = client->dev;
> +
> +	unregister_console(&dlog->con);
> +
> +	mutex_lock(&dlog->lock);
> +	drm_log_free_scanout(client);
> +	drm_client_release(client);
> +	mutex_unlock(&dlog->lock);
> +	kfree(dlog);
> +	drm_dbg(dev, "Unregistered with drm log\n");
> +}
> +
> +static int drm_log_client_hotplug(struct drm_client_dev *client)
> +{
> +	struct drm_log *dlog = client_to_drm_log(client);
> +
> +	mutex_lock(&dlog->lock);
> +	drm_log_free_scanout(client);
> +	dlog->probed = false;
> +	mutex_unlock(&dlog->lock);
> +	return 0;
> +}
> +
> +static const struct drm_client_funcs drm_log_client_funcs = {
> +	.owner		= THIS_MODULE,
> +	.unregister	= drm_log_client_unregister,
> +	.hotplug	= drm_log_client_hotplug,
> +};
> +
> +static void drm_log_write_thread(struct console *con, struct nbcon_write_context *wctxt)
> +{
> +	struct drm_log *dlog = console_to_drm_log(con);
> +	int i;
> +
> +	if (!dlog->probed)
> +		drm_log_init_client(dlog);
> +
> +	for (i = 0; i < dlog->n_scanout; i++)
> +		drm_log_draw_kmsg_record(&dlog->scanout[i], wctxt->outbuf, wctxt->len);
> +}
> +
> +static void drm_log_lock(struct console *con, unsigned long *flags)
> +{
> +	struct drm_log *dlog = console_to_drm_log(con);
> +
> +	mutex_lock(&dlog->lock);
> +	migrate_disable();
> +}
> +
> +static void drm_log_unlock(struct console *con, unsigned long flags)
> +{
> +	struct drm_log *dlog = console_to_drm_log(con);
> +
> +	migrate_enable();
> +	mutex_unlock(&dlog->lock);
> +}
> +
> +static void drm_log_register_console(struct console *con)
> +{
> +	strscpy(con->name, "drm_log");
> +	con->write_thread = drm_log_write_thread;
> +	con->device_lock = drm_log_lock;
> +	con->device_unlock = drm_log_unlock;
> +	con->flags = CON_PRINTBUFFER | CON_NBCON;
> +	con->index = -1;
> +
> +	register_console(con);
> +}
> +
> +/**
> + * drm_log_register() - Register a drm device to drm_log
> + * @dev: the drm device to register.
> + */
> +void drm_log_register(struct drm_device *dev)
> +{
> +	struct drm_log *new;
> +
> +	new = kzalloc(sizeof(*new), GFP_KERNEL);
> +	if (!new)
> +		goto err_warn;
> +
> +	mutex_init(&new->lock);
> +	if (drm_client_init(dev, &new->client, "drm_log", &drm_log_client_funcs))
> +		goto err_free;
> +
> +	drm_client_register(&new->client);
> +
> +	drm_log_register_console(&new->con);
> +
> +	drm_dbg(dev, "Registered with drm log as %s\n", new->con.name);
> +	return;
> +
> +err_free:
> +	kfree(new);
> +err_warn:
> +	drm_warn(dev, "Failed to register with drm log\n");
> +}

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

On 04/12/2024 16:44, Jocelyn Falempe wrote:
> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
> This is not a full replacement to fbcon, as it will only print the kmsg.
> It will never handle user input, or a terminal because this is better done in userspace.
> 
> If you're curious on how it looks like, I've put a small demo here:
> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4

I just pushed it to drm-misc-next.

Thanks all for your reviews and comments.

Best regards,

-- 

Jocelyn
> 
> Design decisions:
>    * It uses the drm_client API, so it should work on all drm drivers from the start.
>    * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
>      It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
>    * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
>   
> v2:
>   * Use vmap_local() api, with that change, I've tested it successfully on simpledrm, virtio-gpu, amdgpu, and nouveau.
>   * Stop drawing when the drm_master is taken. This avoid wasting CPU cycle if the buffer is not visible.
>   * Use deferred probe. Only do the probe the first time there is a log to draw. With this, if you boot with quiet, drm_log won't do any modeset.
>   * Add color support for the timestamp prefix, like what dmesg does.
>   * Add build dependency on  disabling the fbdev emulation, as they are both drm_client, and there is no way to choose which one gets the focus.
> 
> v3:
>   * Remove the work thread and circular buffer, and use the new write_thread() console API.
>   * Register a console for each drm driver.
> 
> v4:
>   * Can be built as a module, even if that's not really useful.
>   * Rebased on top of "drm: Introduce DRM client library" series from Thomas Zimmermann.
>   * Add a Kconfig menu to choose between drm client.
>   * Add suspend/resume callbacks.
>   * Add integer scaling support.
>   
> v5:
>   * Build drm_log in drm_client_lib module, to avoid circular dependency.
>   * Export drm_draw symbols, so they can be used if drm_client_lib is built as module.
>   * Change scale parameter to unsigned int (Jani Nikula)
> 
> v6:
>   * Use console_stop() and console_start() in the suspend/resume callback (Petr Mladek).
>   * rebase and solve conflict with "drm/panic: Add ABGR2101010 support"
> 
> v7:
>   * Add a patch fix a build issue due to missing DRM_CLIENT_LIB, reported by kernel test bot.
> 
> v8:
>   * Rebased after drm client moved to drivers/gpu/drm/clients/
>   * Rename DRM_LOG to DRM_CLIENT_LOG (Thomas Zimmermann)
>   * Drop "Always select DRM_CLIENT_LIB", and select only if DRM_CLIENT_LOG is set
>   * Add an info message if no clients are initialized in drm_client_setup()
> 
> v9:
>   * Rename drm_draw.h to drm_draw_internal.h (Jani Nikula)
>   * Add cflags to remove the "../" when including drm internal headers (Jani Nikula)
>   * Order select alphabetically in KConfig (Thomas Zimmermann)
>   * Replace drm_info with drm_dbg, to be less verbose (Thomas Zimmermann)
>   * Rename module parameter to drm_client_lib.active (Thomas Zimmermann)
>   * Warn if drm_client_lib.active is malformated (Thomas Zimmermann)
>   
> Jocelyn Falempe (6):
>    drm/panic: Move drawing functions to drm_draw
>    drm/log: Introduce a new boot logger to draw the kmsg on the screen
>    drm/log: Do not draw if drm_master is taken
>    drm/log: Color the timestamp, to improve readability
>    drm/log: Implement suspend/resume
>    drm/log: Add integer scaling support
> 
>   drivers/gpu/drm/Kconfig                       |   5 +
>   drivers/gpu/drm/Makefile                      |   1 +
>   drivers/gpu/drm/clients/Kconfig               |  48 ++
>   drivers/gpu/drm/clients/Makefile              |   3 +
>   drivers/gpu/drm/clients/drm_client_internal.h |   6 +
>   drivers/gpu/drm/clients/drm_client_setup.c    |  29 +-
>   drivers/gpu/drm/clients/drm_log.c             | 420 ++++++++++++++++++
>   drivers/gpu/drm/drm_draw.c                    | 233 ++++++++++
>   drivers/gpu/drm/drm_draw_internal.h           |  56 +++
>   drivers/gpu/drm/drm_panic.c                   | 257 +----------
>   10 files changed, 820 insertions(+), 238 deletions(-)
>   create mode 100644 drivers/gpu/drm/clients/drm_log.c
>   create mode 100644 drivers/gpu/drm/drm_draw.c
>   create mode 100644 drivers/gpu/drm/drm_draw_internal.h
> 
> 
> base-commit: c6eabbab359c156669e10d5dec3e71e80ff09bd2

Re: [v9,1/6] drm/panic: Move drawing functions to drm_draw

[Kees Bakker]

Op 04-12-2024 om 16:45 schreef Jocelyn Falempe:
> Move the color conversions, blit and fill functions to drm_draw.c,
> so that they can be re-used by drm_log.
> drm_draw is internal to the drm subsystem, and shouldn't be used by
> gpu drivers.
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>
> v5:
>   * Export drm_draw symbols, so they can be used if drm_client_lib is built as module.
>
> v6:
>   * rebase and solve conflict with "drm/panic: Add ABGR2101010 support"
>
> v9:
>   * Rename drm_draw.h to drm_draw_internal.h (Jani Nikula)
>
>   drivers/gpu/drm/Kconfig             |   5 +
>   drivers/gpu/drm/Makefile            |   1 +
>   drivers/gpu/drm/drm_draw.c          | 233 +++++++++++++++++++++++++
>   drivers/gpu/drm/drm_draw_internal.h |  56 ++++++
>   drivers/gpu/drm/drm_panic.c         | 257 +++-------------------------
>   5 files changed, 318 insertions(+), 234 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_draw.c
>   create mode 100644 drivers/gpu/drm/drm_draw_internal.h
>
> [...]
> diff --git a/drivers/gpu/drm/drm_draw.c b/drivers/gpu/drm/drm_draw.c
> new file mode 100644
> index 000000000000..cb2ad12bce57
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_draw.c
> @@ -0,0 +1,233 @@
> +[...]
> +void drm_draw_fill24(struct iosys_map *dmap, unsigned int dpitch,
> +		     unsigned int height, unsigned int width,
> +		     u16 color)
> +{
> +	unsigned int y, x;
> +
> +	for (y = 0; y < height; y++) {
> +		for (x = 0; x < width; x++) {
> +			unsigned int off = y * dpitch + x * 3;
> +
> +			/* write blue-green-red to output in little endianness */
> +			iosys_map_wr(dmap, off, u8, (color & 0x000000FF) >> 0);
> +			iosys_map_wr(dmap, off + 1, u8, (color & 0x0000FF00) >> 8);
> +			iosys_map_wr(dmap, off + 2, u8, (color & 0x00FF0000) >> 16);
> +		}
> +	}
> +}
>
u16 is not wide enough for a 24bit color
-- 
Kees

Re: [PATCH v9 2/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Dan Carpenter]

This patch breaks "make oldconfig" for me.  It just gets into an endless
loop of:

  Default DRM Client
  choice[1-0?]: 0
  Default DRM Client
  choice[1-0?]: 0
  Default DRM Client
  choice[1-0?]: 0
  Default DRM Client
  choice[1-0?]: 0
  ...

I don't have to type anything, it just spams that forever.  It's weird
that it's 1-0 instead of 0-1.  Does that means something?  I don't know
much about Kconfig.

I'm using this arm64 randconfig as a base.  I type "make oldconfig" and
press enter until it gets to "Default DRM Client" and then it starts
scrolling endlessly.
https://download.01.org/0day-ci/archive/20241212/202412121555.Fp663tyH-lkp@intel.com/config

regards,
dan carpenter

Re: [v9,1/6] drm/panic: Move drawing functions to drm_draw

[Jocelyn Falempe]

On 11/12/2024 22:34, Kees Bakker wrote:
> Op 04-12-2024 om 16:45 schreef Jocelyn Falempe:
>> Move the color conversions, blit and fill functions to drm_draw.c,
>> so that they can be re-used by drm_log.
>> drm_draw is internal to the drm subsystem, and shouldn't be used by
>> gpu drivers.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>
>> v5:
>>   * Export drm_draw symbols, so they can be used if drm_client_lib is 
>> built as module.
>>
>> v6:
>>   * rebase and solve conflict with "drm/panic: Add ABGR2101010 support"
>>
>> v9:
>>   * Rename drm_draw.h to drm_draw_internal.h (Jani Nikula)
>>
>>   drivers/gpu/drm/Kconfig             |   5 +
>>   drivers/gpu/drm/Makefile            |   1 +
>>   drivers/gpu/drm/drm_draw.c          | 233 +++++++++++++++++++++++++
>>   drivers/gpu/drm/drm_draw_internal.h |  56 ++++++
>>   drivers/gpu/drm/drm_panic.c         | 257 +++-------------------------
>>   5 files changed, 318 insertions(+), 234 deletions(-)
>>   create mode 100644 drivers/gpu/drm/drm_draw.c
>>   create mode 100644 drivers/gpu/drm/drm_draw_internal.h
>>
>> [...]
>> diff --git a/drivers/gpu/drm/drm_draw.c b/drivers/gpu/drm/drm_draw.c
>> new file mode 100644
>> index 000000000000..cb2ad12bce57
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_draw.c
>> @@ -0,0 +1,233 @@
>> +[...]
>> +void drm_draw_fill24(struct iosys_map *dmap, unsigned int dpitch,
>> +             unsigned int height, unsigned int width,
>> +             u16 color)
>> +{
>> +    unsigned int y, x;
>> +
>> +    for (y = 0; y < height; y++) {
>> +        for (x = 0; x < width; x++) {
>> +            unsigned int off = y * dpitch + x * 3;
>> +
>> +            /* write blue-green-red to output in little endianness */
>> +            iosys_map_wr(dmap, off, u8, (color & 0x000000FF) >> 0);
>> +            iosys_map_wr(dmap, off + 1, u8, (color & 0x0000FF00) >> 8);
>> +            iosys_map_wr(dmap, off + 2, u8, (color & 0x00FF0000) >> 16);
>> +        }
>> +    }
>> +}
>>
> u16 is not wide enough for a 24bit color

Good catch, I will send a fix when I get some time.

Best regards,

-- 

Jocelyn

Re: [PATCH v9 2/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

On 12/12/2024 08:41, Dan Carpenter wrote:
> This patch breaks "make oldconfig" for me.  It just gets into an endless
> loop of:
> 
>    Default DRM Client
>    choice[1-0?]: 0
>    Default DRM Client
>    choice[1-0?]: 0
>    Default DRM Client
>    choice[1-0?]: 0
>    Default DRM Client
>    choice[1-0?]: 0
>    ...
> 
> I don't have to type anything, it just spams that forever.  It's weird
> that it's 1-0 instead of 0-1.  Does that means something?  I don't know
> much about Kconfig.

I can reproduce it with your provided config.

It looks like it happens if DRM_CLIENT_SELECTION is enabled, but none of 
the client is.
The attached patch should fix it, can you try it ?

I will submit that shortly.

Thanks for reporting it.

Best regards,

-- 

Jocelyn


> 
> I'm using this arm64 randconfig as a base.  I type "make oldconfig" and
> press enter until it gets to "Default DRM Client" and then it starts
> scrolling endlessly.
> https://download.01.org/0day-ci/archive/20241212/202412121555.Fp663tyH-lkp@intel.com/config
> 
> regards,
> dan carpenter
> 

[-- Attachment #2: 0001-Fix-endless-Kconfig-loop.patch --]
[-- Type: text/x-patch, Size: 773 bytes --]

From e4d197debd2c199c9f2d8e35e41e36c2836926b9 Mon Sep 17 00:00:00 2001
From: Jocelyn Falempe <jfalempe@redhat.com>
Date: Thu, 12 Dec 2024 09:43:50 +0100
Subject: [PATCH] Fix endless Kconfig loop

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/clients/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/clients/Kconfig b/drivers/gpu/drm/clients/Kconfig
index c18decc90200..82a7d4e584dd 100644
--- a/drivers/gpu/drm/clients/Kconfig
+++ b/drivers/gpu/drm/clients/Kconfig
@@ -87,6 +87,7 @@ config DRM_CLIENT_LOG
 choice
 	prompt "Default DRM Client"
 	depends on DRM_CLIENT_SELECTION
+	depends on DRM_FBDEV_EMULATION || DRM_CLIENT_LOG
 	default DRM_CLIENT_DEFAULT_FBDEV
 	help
 	  Selects the default drm client.
-- 
2.47.1

Re: [PATCH v9 2/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Dan Carpenter]

On Thu, Dec 12, 2024 at 09:49:19AM +0100, Jocelyn Falempe wrote:
> On 12/12/2024 08:41, Dan Carpenter wrote:
> > This patch breaks "make oldconfig" for me.  It just gets into an endless
> > loop of:
> > 
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    ...
> > 
> > I don't have to type anything, it just spams that forever.  It's weird
> > that it's 1-0 instead of 0-1.  Does that means something?  I don't know
> > much about Kconfig.
> 
> I can reproduce it with your provided config.
> 
> It looks like it happens if DRM_CLIENT_SELECTION is enabled, but none of the
> client is.
> The attached patch should fix it, can you try it ?
> 

Thanks!

Tested-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

Re: [PATCH v9 2/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Simona Vetter]

On Thu, Dec 12, 2024 at 09:49:19AM +0100, Jocelyn Falempe wrote:
> On 12/12/2024 08:41, Dan Carpenter wrote:
> > This patch breaks "make oldconfig" for me.  It just gets into an endless
> > loop of:
> > 
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    Default DRM Client
> >    choice[1-0?]: 0
> >    ...
> > 
> > I don't have to type anything, it just spams that forever.  It's weird
> > that it's 1-0 instead of 0-1.  Does that means something?  I don't know
> > much about Kconfig.
> 
> I can reproduce it with your provided config.
> 
> It looks like it happens if DRM_CLIENT_SELECTION is enabled, but none of the
> client is.
> The attached patch should fix it, can you try it ?
> 
> I will submit that shortly.
> 
> Thanks for reporting it.
> 
> Best regards,
> 
> -- 
> 
> Jocelyn
> 
> 
> > 
> > I'm using this arm64 randconfig as a base.  I type "make oldconfig" and
> > press enter until it gets to "Default DRM Client" and then it starts
> > scrolling endlessly.
> > https://download.01.org/0day-ci/archive/20241212/202412121555.Fp663tyH-lkp@intel.com/config
> > 
> > regards,
> > dan carpenter
> > 

> From e4d197debd2c199c9f2d8e35e41e36c2836926b9 Mon Sep 17 00:00:00 2001
> From: Jocelyn Falempe <jfalempe@redhat.com>
> Date: Thu, 12 Dec 2024 09:43:50 +0100
> Subject: [PATCH] Fix endless Kconfig loop
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>

Since I haven't seen the patch anywhere else I'll drop my ack here.

Acked-by: Simona Vetter <simona.vetter@ffwll.ch>

Please push directly to drm-misc-next so the breakage doesn't linger.
-Sima
> ---
>  drivers/gpu/drm/clients/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/clients/Kconfig b/drivers/gpu/drm/clients/Kconfig
> index c18decc90200..82a7d4e584dd 100644
> --- a/drivers/gpu/drm/clients/Kconfig
> +++ b/drivers/gpu/drm/clients/Kconfig
> @@ -87,6 +87,7 @@ config DRM_CLIENT_LOG
>  choice
>  	prompt "Default DRM Client"
>  	depends on DRM_CLIENT_SELECTION
> +	depends on DRM_FBDEV_EMULATION || DRM_CLIENT_LOG
>  	default DRM_CLIENT_DEFAULT_FBDEV
>  	help
>  	  Selects the default drm client.
> -- 
> 2.47.1
> 


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

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Geert Uytterhoeven]

Hi Jocelyn,

On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
> This is not a full replacement to fbcon, as it will only print the kmsg.
> It will never handle user input, or a terminal because this is better done in userspace.
>
> If you're curious on how it looks like, I've put a small demo here:
> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
>
> Design decisions:
>   * It uses the drm_client API, so it should work on all drm drivers from the start.
>   * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
>     It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
>   * It uses the new non-blocking console API, so it should work well with PREEMPT_RT

I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
Unfortunately I don't see any kernel messages, and my monitor complains
about no signal. Does this require special support from the driver?

    CONFIG_DRM_CLIENT=y
    CONFIG_DRM_CLIENT_LIB=y
    CONFIG_DRM_CLIENT_SELECTION=y
    CONFIG_DRM_CLIENT_SETUP=y
    CONFIG_DRM_CLIENT_LOG=y
    # CONFIG_DRM_CLIENT_DEFAULT_FBDEV is not set
    CONFIG_DRM_CLIENT_DEFAULT_LOG=y
    CONFIG_DRM_CLIENT_DEFAULT="log"

Switching to fbdev gives a working display, as before:

    CONFIG_DRM_CLIENT_DEFAULT_FBDEV=y
    # CONFIG_DRM_CLIENT_DEFAULT_LOG is not set
    CONFIG_DRM_CLIENT_DEFAULT="fbdev"

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

On 17/12/2024 15:19, Geert Uytterhoeven wrote:
> Hi Jocelyn,
> 
> On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
>> This is not a full replacement to fbcon, as it will only print the kmsg.
>> It will never handle user input, or a terminal because this is better done in userspace.
>>
>> If you're curious on how it looks like, I've put a small demo here:
>> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
>>
>> Design decisions:
>>    * It uses the drm_client API, so it should work on all drm drivers from the start.
>>    * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
>>      It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
>>    * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
> 
> I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
> Unfortunately I don't see any kernel messages, and my monitor complains
> about no signal. Does this require special support from the driver?

It doesn't require a special support from the driver. But as it is the 
first drm client other than fbdev emulation, I'm not surprised it's 
broken on some driver.
I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6 
(Qualcomm SDM845/freedreno), without requiring driver changes.

Do you have a serial console on this device, to check if there is 
something in kmsg?

Best regards,

-- 

Jocelyn

> 
>      CONFIG_DRM_CLIENT=y
>      CONFIG_DRM_CLIENT_LIB=y
>      CONFIG_DRM_CLIENT_SELECTION=y
>      CONFIG_DRM_CLIENT_SETUP=y
>      CONFIG_DRM_CLIENT_LOG=y
>      # CONFIG_DRM_CLIENT_DEFAULT_FBDEV is not set
>      CONFIG_DRM_CLIENT_DEFAULT_LOG=y
>      CONFIG_DRM_CLIENT_DEFAULT="log"
> 
> Switching to fbdev gives a working display, as before:
> 
>      CONFIG_DRM_CLIENT_DEFAULT_FBDEV=y
>      # CONFIG_DRM_CLIENT_DEFAULT_LOG is not set
>      CONFIG_DRM_CLIENT_DEFAULT="fbdev"
> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Geert Uytterhoeven]

Hi Jocelyn.

On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 17/12/2024 15:19, Geert Uytterhoeven wrote:
> > On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> >> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
> >> This is not a full replacement to fbcon, as it will only print the kmsg.
> >> It will never handle user input, or a terminal because this is better done in userspace.
> >>
> >> If you're curious on how it looks like, I've put a small demo here:
> >> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
> >>
> >> Design decisions:
> >>    * It uses the drm_client API, so it should work on all drm drivers from the start.
> >>    * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
> >>      It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
> >>    * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
> >
> > I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
> > Unfortunately I don't see any kernel messages, and my monitor complains
> > about no signal. Does this require special support from the driver?
>
> It doesn't require a special support from the driver. But as it is the
> first drm client other than fbdev emulation, I'm not surprised it's
> broken on some driver.
> I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
> (Qualcomm SDM845/freedreno), without requiring driver changes.
>
> Do you have a serial console on this device, to check if there is
> something in kmsg?

Nothing interesting to see. Compared to the fbdev client:

     rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
     [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
     rcar-du feb00000.display: [drm] Device feb00000.display probed
    -Console: switching to colour frame buffer device 240x67
    -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device

I did verify (by adding my own debug prints) that the code does
get to the success case in drm_log_register().
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

On 17/12/2024 15:54, Geert Uytterhoeven wrote:
> Hi Jocelyn.
> 
> On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> On 17/12/2024 15:19, Geert Uytterhoeven wrote:
>>> On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
>>>> This is not a full replacement to fbcon, as it will only print the kmsg.
>>>> It will never handle user input, or a terminal because this is better done in userspace.
>>>>
>>>> If you're curious on how it looks like, I've put a small demo here:
>>>> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
>>>>
>>>> Design decisions:
>>>>     * It uses the drm_client API, so it should work on all drm drivers from the start.
>>>>     * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
>>>>       It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
>>>>     * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
>>>
>>> I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
>>> Unfortunately I don't see any kernel messages, and my monitor complains
>>> about no signal. Does this require special support from the driver?
>>
>> It doesn't require a special support from the driver. But as it is the
>> first drm client other than fbdev emulation, I'm not surprised it's
>> broken on some driver.
>> I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
>> (Qualcomm SDM845/freedreno), without requiring driver changes.
>>
>> Do you have a serial console on this device, to check if there is
>> something in kmsg?
> 
> Nothing interesting to see. Compared to the fbdev client:
> 
>       rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
>       [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
>       rcar-du feb00000.display: [drm] Device feb00000.display probed
>      -Console: switching to colour frame buffer device 240x67
>      -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
> 
> I did verify (by adding my own debug prints) that the code does
> get to the success case in drm_log_register().
> Thanks!

Maybe you need to add console=drm_log to your kernel command line, so 
the kernel will actually use this console.

> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Geert Uytterhoeven]

Hi Jocelyn,

On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 17/12/2024 15:54, Geert Uytterhoeven wrote:
> > On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> >> On 17/12/2024 15:19, Geert Uytterhoeven wrote:
> >>> On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> >>>> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
> >>>> This is not a full replacement to fbcon, as it will only print the kmsg.
> >>>> It will never handle user input, or a terminal because this is better done in userspace.
> >>>>
> >>>> If you're curious on how it looks like, I've put a small demo here:
> >>>> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
> >>>>
> >>>> Design decisions:
> >>>>     * It uses the drm_client API, so it should work on all drm drivers from the start.
> >>>>     * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
> >>>>       It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
> >>>>     * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
> >>>
> >>> I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
> >>> Unfortunately I don't see any kernel messages, and my monitor complains
> >>> about no signal. Does this require special support from the driver?
> >>
> >> It doesn't require a special support from the driver. But as it is the
> >> first drm client other than fbdev emulation, I'm not surprised it's
> >> broken on some driver.
> >> I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
> >> (Qualcomm SDM845/freedreno), without requiring driver changes.
> >>
> >> Do you have a serial console on this device, to check if there is
> >> something in kmsg?
> >
> > Nothing interesting to see. Compared to the fbdev client:
> >
> >       rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
> >       [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
> >       rcar-du feb00000.display: [drm] Device feb00000.display probed
> >      -Console: switching to colour frame buffer device 240x67
> >      -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
> >
> > I did verify (by adding my own debug prints) that the code does
> > get to the success case in drm_log_register().
> > Thanks!
>
> Maybe you need to add console=drm_log to your kernel command line, so
> the kernel will actually use this console.

Thanks, that does the trick!

Note that I do not need to specify any console= kernel command line
parameter for the fbdev console.

With

    CONFIG_VT_CONSOLE=y
    CONFIG_DRM_CLIENT_DEFAULT_FBDEV=y

I see all console messages on both the emulated fbdev console and on
the serial console by default.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

On 18/12/2024 12:00, Geert Uytterhoeven wrote:
> Hi Jocelyn,
> 
> On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> On 17/12/2024 15:54, Geert Uytterhoeven wrote:
>>> On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>> On 17/12/2024 15:19, Geert Uytterhoeven wrote:
>>>>> On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>>> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
>>>>>> This is not a full replacement to fbcon, as it will only print the kmsg.
>>>>>> It will never handle user input, or a terminal because this is better done in userspace.
>>>>>>
>>>>>> If you're curious on how it looks like, I've put a small demo here:
>>>>>> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
>>>>>>
>>>>>> Design decisions:
>>>>>>      * It uses the drm_client API, so it should work on all drm drivers from the start.
>>>>>>      * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
>>>>>>        It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
>>>>>>      * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
>>>>>
>>>>> I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
>>>>> Unfortunately I don't see any kernel messages, and my monitor complains
>>>>> about no signal. Does this require special support from the driver?
>>>>
>>>> It doesn't require a special support from the driver. But as it is the
>>>> first drm client other than fbdev emulation, I'm not surprised it's
>>>> broken on some driver.
>>>> I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
>>>> (Qualcomm SDM845/freedreno), without requiring driver changes.
>>>>
>>>> Do you have a serial console on this device, to check if there is
>>>> something in kmsg?
>>>
>>> Nothing interesting to see. Compared to the fbdev client:
>>>
>>>        rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
>>>        [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
>>>        rcar-du feb00000.display: [drm] Device feb00000.display probed
>>>       -Console: switching to colour frame buffer device 240x67
>>>       -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
>>>
>>> I did verify (by adding my own debug prints) that the code does
>>> get to the success case in drm_log_register().
>>> Thanks!
>>
>> Maybe you need to add console=drm_log to your kernel command line, so
>> the kernel will actually use this console.
> 
> Thanks, that does the trick!
> 
> Note that I do not need to specify any console= kernel command line
> parameter for the fbdev console.

Yes, the fbcon console is tty0, which is hardcoded for historical reason.
Some architectures use add_preferred_console() to enable specific 
consoles, I'm not sure it's allowed to use that from the 
drm_log_register() code.

I will still send a patch to add update the Kconfig help for drm_log, as 
this command line argument is required to have it working.

Best regards,

-- 

Jocelyn

> 
> With
> 
>      CONFIG_VT_CONSOLE=y
>      CONFIG_DRM_CLIENT_DEFAULT_FBDEV=y
> 
> I see all console messages on both the emulated fbdev console and on
> the serial console by default.
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

Re: [PATCH v9 2/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Markus Elfring]

…
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -0,0 +1,370 @@
…
> +static int drm_log_count_modeset(struct drm_client_dev *client)
> +{
> +	struct drm_mode_set *mode_set;
> +	int count = 0;
> +
> +	mutex_lock(&client->modeset_mutex);
> +	drm_client_for_each_modeset(mode_set, client)
> +		count++;
> +	mutex_unlock(&client->modeset_mutex);
> +	return count;
> +}
…

Under which circumstances would you become interested to apply a statement
like “guard(mutex)(&client->modeset_mutex);”?
https://elixir.bootlin.com/linux/v6.13-rc3/source/include/linux/mutex.h#L201

Regards,
Markus

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Petr Mladek]

On Wed 2024-12-18 12:41:39, Jocelyn Falempe wrote:
> On 18/12/2024 12:00, Geert Uytterhoeven wrote:
> > Hi Jocelyn,
> > 
> > On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > On 17/12/2024 15:54, Geert Uytterhoeven wrote:
> > > > On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > > On 17/12/2024 15:19, Geert Uytterhoeven wrote:
> > > > > > On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > > > > drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
> > > > > > > This is not a full replacement to fbcon, as it will only print the kmsg.
> > > > > > > It will never handle user input, or a terminal because this is better done in userspace.
> > > > > > > 
> > > > > > > If you're curious on how it looks like, I've put a small demo here:
> > > > > > > https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
> > > > > > > 
> > > > > > > Design decisions:
> > > > > > >      * It uses the drm_client API, so it should work on all drm drivers from the start.
> > > > > > >      * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
> > > > > > >        It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
> > > > > > >      * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
> > > > > > 
> > > > > > I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
> > > > > > Unfortunately I don't see any kernel messages, and my monitor complains
> > > > > > about no signal. Does this require special support from the driver?
> > > > > 
> > > > > It doesn't require a special support from the driver. But as it is the
> > > > > first drm client other than fbdev emulation, I'm not surprised it's
> > > > > broken on some driver.
> > > > > I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
> > > > > (Qualcomm SDM845/freedreno), without requiring driver changes.
> > > > > 
> > > > > Do you have a serial console on this device, to check if there is
> > > > > something in kmsg?
> > > > 
> > > > Nothing interesting to see. Compared to the fbdev client:
> > > > 
> > > >        rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
> > > >        [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
> > > >        rcar-du feb00000.display: [drm] Device feb00000.display probed
> > > >       -Console: switching to colour frame buffer device 240x67
> > > >       -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
> > > > 
> > > > I did verify (by adding my own debug prints) that the code does
> > > > get to the success case in drm_log_register().
> > > > Thanks!
> > > 
> > > Maybe you need to add console=drm_log to your kernel command line, so
> > > the kernel will actually use this console.
> > 
> > Thanks, that does the trick!
> > 
> > Note that I do not need to specify any console= kernel command line
> > parameter for the fbdev console.
> 
> Yes, the fbcon console is tty0, which is hardcoded for historical reason.
> Some architectures use add_preferred_console() to enable specific consoles,
> I'm not sure it's allowed to use that from the drm_log_register() code.

add_preferred_console() is used when the console should get enabled
intentionally. I would split the intentions into two categories:

  + requested by the end-user on the command line, see
       __add_preferred_console(..., true) in console_setup()

  + enabled by default by a hardware definition (manufacture), see
       add_preferred_console() in:

	+ of_console_check(): generic solution via device tree
	+ acpi_parse_spcr(): generic solution via SPCR table
	+ *: hardcoded in few more HW-specific drivers

add_preferred_console() causes the console will always get enabled
when it is successfully initialized.

So, should the "drm_log" console get always enabled?


> I will still send a patch to add update the Kconfig help for drm_log, as
> this command line argument is required to have it working.

I guess that the drm_log consoles should get enabled only when
requested by the user => documenting the command line parameter
is the right solution here.

Best Regards,
Petr

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Geert Uytterhoeven]

Hi Petr,

On Wed, Dec 18, 2024 at 3:18 PM Petr Mladek <pmladek@suse.com> wrote:
> On Wed 2024-12-18 12:41:39, Jocelyn Falempe wrote:
> > On 18/12/2024 12:00, Geert Uytterhoeven wrote:
> > > On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > On 17/12/2024 15:54, Geert Uytterhoeven wrote:
> > > > > On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > > > On 17/12/2024 15:19, Geert Uytterhoeven wrote:
> > > > > > > On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > > > > > drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
> > > > > > > > This is not a full replacement to fbcon, as it will only print the kmsg.
> > > > > > > > It will never handle user input, or a terminal because this is better done in userspace.
> > > > > > > >
> > > > > > > > If you're curious on how it looks like, I've put a small demo here:
> > > > > > > > https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
> > > > > > > >
> > > > > > > > Design decisions:
> > > > > > > >      * It uses the drm_client API, so it should work on all drm drivers from the start.
> > > > > > > >      * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
> > > > > > > >        It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
> > > > > > > >      * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
> > > > > > >
> > > > > > > I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
> > > > > > > Unfortunately I don't see any kernel messages, and my monitor complains
> > > > > > > about no signal. Does this require special support from the driver?
> > > > > >
> > > > > > It doesn't require a special support from the driver. But as it is the
> > > > > > first drm client other than fbdev emulation, I'm not surprised it's
> > > > > > broken on some driver.
> > > > > > I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
> > > > > > (Qualcomm SDM845/freedreno), without requiring driver changes.
> > > > > >
> > > > > > Do you have a serial console on this device, to check if there is
> > > > > > something in kmsg?
> > > > >
> > > > > Nothing interesting to see. Compared to the fbdev client:
> > > > >
> > > > >        rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
> > > > >        [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
> > > > >        rcar-du feb00000.display: [drm] Device feb00000.display probed
> > > > >       -Console: switching to colour frame buffer device 240x67
> > > > >       -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
> > > > >
> > > > > I did verify (by adding my own debug prints) that the code does
> > > > > get to the success case in drm_log_register().
> > > > > Thanks!
> > > >
> > > > Maybe you need to add console=drm_log to your kernel command line, so
> > > > the kernel will actually use this console.
> > >
> > > Thanks, that does the trick!
> > >
> > > Note that I do not need to specify any console= kernel command line
> > > parameter for the fbdev console.
> >
> > Yes, the fbcon console is tty0, which is hardcoded for historical reason.
> > Some architectures use add_preferred_console() to enable specific consoles,
> > I'm not sure it's allowed to use that from the drm_log_register() code.
>
> add_preferred_console() is used when the console should get enabled
> intentionally. I would split the intentions into two categories:
>
>   + requested by the end-user on the command line, see
>        __add_preferred_console(..., true) in console_setup()
>
>   + enabled by default by a hardware definition (manufacture), see
>        add_preferred_console() in:
>
>         + of_console_check(): generic solution via device tree
>         + acpi_parse_spcr(): generic solution via SPCR table
>         + *: hardcoded in few more HW-specific drivers
>
> add_preferred_console() causes the console will always get enabled
> when it is successfully initialized.
>
> So, should the "drm_log" console get always enabled?

Good question!

> > I will still send a patch to add update the Kconfig help for drm_log, as
> > this command line argument is required to have it working.
>
> I guess that the drm_log consoles should get enabled only when
> requested by the user => documenting the command line parameter
> is the right solution here.

There are actually two ways to request it:
  1. Through CONFIG_DRM_CLIENT_DEFAULT, at kernel configuration
     time,
  2. Through drm_client_lib.active=log, at kernel boot or module load time.

Currently both options need console=drm_log on top of that, which
sounds like overkill?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

On 18/12/2024 15:18, Petr Mladek wrote:
> On Wed 2024-12-18 12:41:39, Jocelyn Falempe wrote:
>> On 18/12/2024 12:00, Geert Uytterhoeven wrote:
>>> Hi Jocelyn,
>>>
>>> On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>> On 17/12/2024 15:54, Geert Uytterhoeven wrote:
>>>>> On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>>> On 17/12/2024 15:19, Geert Uytterhoeven wrote:
>>>>>>> On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>>>>> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
>>>>>>>> This is not a full replacement to fbcon, as it will only print the kmsg.
>>>>>>>> It will never handle user input, or a terminal because this is better done in userspace.
>>>>>>>>
>>>>>>>> If you're curious on how it looks like, I've put a small demo here:
>>>>>>>> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
>>>>>>>>
>>>>>>>> Design decisions:
>>>>>>>>       * It uses the drm_client API, so it should work on all drm drivers from the start.
>>>>>>>>       * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
>>>>>>>>         It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
>>>>>>>>       * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
>>>>>>>
>>>>>>> I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
>>>>>>> Unfortunately I don't see any kernel messages, and my monitor complains
>>>>>>> about no signal. Does this require special support from the driver?
>>>>>>
>>>>>> It doesn't require a special support from the driver. But as it is the
>>>>>> first drm client other than fbdev emulation, I'm not surprised it's
>>>>>> broken on some driver.
>>>>>> I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
>>>>>> (Qualcomm SDM845/freedreno), without requiring driver changes.
>>>>>>
>>>>>> Do you have a serial console on this device, to check if there is
>>>>>> something in kmsg?
>>>>>
>>>>> Nothing interesting to see. Compared to the fbdev client:
>>>>>
>>>>>         rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
>>>>>         [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
>>>>>         rcar-du feb00000.display: [drm] Device feb00000.display probed
>>>>>        -Console: switching to colour frame buffer device 240x67
>>>>>        -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
>>>>>
>>>>> I did verify (by adding my own debug prints) that the code does
>>>>> get to the success case in drm_log_register().
>>>>> Thanks!
>>>>
>>>> Maybe you need to add console=drm_log to your kernel command line, so
>>>> the kernel will actually use this console.
>>>
>>> Thanks, that does the trick!
>>>
>>> Note that I do not need to specify any console= kernel command line
>>> parameter for the fbdev console.
>>
>> Yes, the fbcon console is tty0, which is hardcoded for historical reason.
>> Some architectures use add_preferred_console() to enable specific consoles,
>> I'm not sure it's allowed to use that from the drm_log_register() code.
> 
> add_preferred_console() is used when the console should get enabled
> intentionally. I would split the intentions into two categories:
> 
>    + requested by the end-user on the command line, see
>         __add_preferred_console(..., true) in console_setup()
> 
>    + enabled by default by a hardware definition (manufacture), see
>         add_preferred_console() in:
> 
> 	+ of_console_check(): generic solution via device tree
> 	+ acpi_parse_spcr(): generic solution via SPCR table
> 	+ *: hardcoded in few more HW-specific drivers
> 
> add_preferred_console() causes the console will always get enabled
> when it is successfully initialized.
> 
> So, should the "drm_log" console get always enabled?

drm_log is a replacement for fbcon, which is always enabled, so I think 
it should also be always enabled. Otherwise you won't get any console as 
fbcon is no more available.
drm_log doesn't really fit in the architecture/hardware model, because 
drm drivers are available for a wide range of architecture, and a GPU 
can do either fbdev/fbcon or drm_log.

> 
> 
>> I will still send a patch to add update the Kconfig help for drm_log, as
>> this command line argument is required to have it working.
> 
> I guess that the drm_log consoles should get enabled only when
> requested by the user => documenting the command line parameter
> is the right solution here.

Most embedded linux specify the console on the command line, but for 
laptop/desktop distributions, it's not the case as fbcon is the default 
since the beginning.

I see a few options here:
1) Use add_preferred_console("drm_log") if DRM_CLIENT_LOG is enabled for 
x86_64 and maybe arm64, so at least the main users are covered.
2) Have a DRM_CLIENT_LOG_PREFERRED_CONSOLE config, so that it's easier 
to setup than changing the kernel command line.
3) Use the kernel command line.

Best Regards,

-- 

Jocelyn

> 
> Best Regards,
> Petr
> 

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Geert Uytterhoeven]

Hi Jocelyn,

On Wed, Dec 18, 2024 at 3:58 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 18/12/2024 15:18, Petr Mladek wrote:
> > On Wed 2024-12-18 12:41:39, Jocelyn Falempe wrote:
> >> On 18/12/2024 12:00, Geert Uytterhoeven wrote:
> >>> On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> >>>> Maybe you need to add console=drm_log to your kernel command line, so
> >>>> the kernel will actually use this console.
> >>>
> >>> Thanks, that does the trick!
> >>>
> >>> Note that I do not need to specify any console= kernel command line
> >>> parameter for the fbdev console.
> >>
> >> Yes, the fbcon console is tty0, which is hardcoded for historical reason.
> >> Some architectures use add_preferred_console() to enable specific consoles,
> >> I'm not sure it's allowed to use that from the drm_log_register() code.
> >
> > add_preferred_console() is used when the console should get enabled
> > intentionally. I would split the intentions into two categories:
> >
> >    + requested by the end-user on the command line, see
> >         __add_preferred_console(..., true) in console_setup()
> >
> >    + enabled by default by a hardware definition (manufacture), see
> >         add_preferred_console() in:
> >
> >       + of_console_check(): generic solution via device tree
> >       + acpi_parse_spcr(): generic solution via SPCR table
> >       + *: hardcoded in few more HW-specific drivers
> >
> > add_preferred_console() causes the console will always get enabled
> > when it is successfully initialized.
> >
> > So, should the "drm_log" console get always enabled?
>
> drm_log is a replacement for fbcon, which is always enabled, so I think
> it should also be always enabled. Otherwise you won't get any console as
> fbcon is no more available.
> drm_log doesn't really fit in the architecture/hardware model, because
> drm drivers are available for a wide range of architecture, and a GPU
> can do either fbdev/fbcon or drm_log.
>
> >> I will still send a patch to add update the Kconfig help for drm_log, as
> >> this command line argument is required to have it working.
> >
> > I guess that the drm_log consoles should get enabled only when
> > requested by the user => documenting the command line parameter
> > is the right solution here.
>
> Most embedded linux specify the console on the command line, but for
> laptop/desktop distributions, it's not the case as fbcon is the default
> since the beginning.

Note that on embedded systems with DT, the console is auto-selected
via chosen/stdout-path. No explicit console= needed.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Petr Mladek]

On Wed 2024-12-18 15:58:46, Jocelyn Falempe wrote:
> On 18/12/2024 15:18, Petr Mladek wrote:
> > On Wed 2024-12-18 12:41:39, Jocelyn Falempe wrote:
> > > On 18/12/2024 12:00, Geert Uytterhoeven wrote:
> > > > Hi Jocelyn,
> > > > 
> > > > On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > > On 17/12/2024 15:54, Geert Uytterhoeven wrote:
> > > > > > On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > > > > On 17/12/2024 15:19, Geert Uytterhoeven wrote:
> > > > > > > > On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > > > > > > > > drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
> > > > > > > > > This is not a full replacement to fbcon, as it will only print the kmsg.
> > > > > > > > > It will never handle user input, or a terminal because this is better done in userspace.
> > > > > > > > > 
> > > > > > > > > If you're curious on how it looks like, I've put a small demo here:
> > > > > > > > > https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
> > > > > > > > > 
> > > > > > > > > Design decisions:
> > > > > > > > >       * It uses the drm_client API, so it should work on all drm drivers from the start.
> > > > > > > > >       * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
> > > > > > > > >         It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
> > > > > > > > >       * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
> > > > > > > > 
> > > > > > > > I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
> > > > > > > > Unfortunately I don't see any kernel messages, and my monitor complains
> > > > > > > > about no signal. Does this require special support from the driver?
> > > > > > > 
> > > > > > > It doesn't require a special support from the driver. But as it is the
> > > > > > > first drm client other than fbdev emulation, I'm not surprised it's
> > > > > > > broken on some driver.
> > > > > > > I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
> > > > > > > (Qualcomm SDM845/freedreno), without requiring driver changes.
> > > > > > > 
> > > > > > > Do you have a serial console on this device, to check if there is
> > > > > > > something in kmsg?
> > > > > > 
> > > > > > Nothing interesting to see. Compared to the fbdev client:
> > > > > > 
> > > > > >         rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
> > > > > >         [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
> > > > > >         rcar-du feb00000.display: [drm] Device feb00000.display probed
> > > > > >        -Console: switching to colour frame buffer device 240x67
> > > > > >        -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
> > > > > > 
> > > > > > I did verify (by adding my own debug prints) that the code does
> > > > > > get to the success case in drm_log_register().
> > > > > > Thanks!
> > > > > 
> > > > > Maybe you need to add console=drm_log to your kernel command line, so
> > > > > the kernel will actually use this console.
> > > > 
> > > > Thanks, that does the trick!
> > > > 
> > > > Note that I do not need to specify any console= kernel command line
> > > > parameter for the fbdev console.
> > > 
> > > Yes, the fbcon console is tty0, which is hardcoded for historical reason.
> > > Some architectures use add_preferred_console() to enable specific consoles,
> > > I'm not sure it's allowed to use that from the drm_log_register() code.
> > 
> > add_preferred_console() is used when the console should get enabled
> > intentionally. I would split the intentions into two categories:
> > 
> >    + requested by the end-user on the command line, see
> >         __add_preferred_console(..., true) in console_setup()
> > 
> >    + enabled by default by a hardware definition (manufacture), see
> >         add_preferred_console() in:
> > 
> > 	+ of_console_check(): generic solution via device tree
> > 	+ acpi_parse_spcr(): generic solution via SPCR table
> > 	+ *: hardcoded in few more HW-specific drivers
> > 
> > add_preferred_console() causes the console will always get enabled
> > when it is successfully initialized.
> > 
> > So, should the "drm_log" console get always enabled?
> 
> drm_log is a replacement for fbcon, which is always enabled, so I think it
> should also be always enabled. Otherwise you won't get any console as fbcon
> is no more available.

IMHO, it is not true that "fbcon" is always enabled. For example,
if you mention "console=ttyS0,115200" on the command line then only
the serial console is enabled.

Honestly, I do not understand completely the connection between the
drm drivers and the console subsystem.

My understanding is that fbcon is registered via the very old and
generic VT (Virtual Terminal) layer, see do_fbcon_takeover().

As a result, it gets registered/enabled by default when there is
no other console defined on the command line or an
add_preferred_console() call.

It is this code:

void register_console(struct console *newcon)
{
[...]
	/*
	 * See if we want to enable this console driver by default.
	 *
	 * Nope when a console is preferred by the command line, device
	 * tree, or SPCR.
	 *
	 * The first real console with tty binding (driver) wins. More
	 * consoles might get enabled before the right one is found.
	 *
	 * Note that a console with tty binding will have CON_CONSDEV
	 * flag set and will be first in the list.
	 */
	if (preferred_console < 0) {
		if (hlist_empty(&console_list) || !console_first()->device ||
		    console_first()->flags & CON_BOOT) {
			try_enable_default_console(newcon);
		}
	}
[...]
}

Note that try_enable_default_console(newcon) fails only when
newcon->setup() fails. It means that the given console is almost
always enabled when this function is called.

The important thing here might be the check of "!console_first()->device".
It causes, that register_console() stop calling
try_enable_default_console(newcon) when there is at least one console
with the tty binding registered.

IMHO, this is the reason why "fbcon" is enabled by default. It is
the first registered driver with the tty binding.

Also this might be the reason why "drm_log" is _not_ enabled by
default. I guess that drm_log_register() is called too late.
I guess that there already is enabled another console driver with
tty binding at that time, most likely the serial console.

Could you please confirm this?
What console is enabled when console=drm_log is not used, please?


> drm_log doesn't really fit in the architecture/hardware model, because drm
> drivers are available for a wide range of architecture, and a GPU can do
> either fbdev/fbcon or drm_log.
> > 
> > > I will still send a patch to add update the Kconfig help for drm_log, as
> > > this command line argument is required to have it working.
> > 
> > I guess that the drm_log consoles should get enabled only when
> > requested by the user => documenting the command line parameter
> > is the right solution here.
> 
> Most embedded linux specify the console on the command line, but for
> laptop/desktop distributions, it's not the case as fbcon is the default
> since the beginning.
> 
> I see a few options here:
> 1) Use add_preferred_console("drm_log") if DRM_CLIENT_LOG is enabled for
> x86_64 and maybe arm64, so at least the main users are covered.
> 2) Have a DRM_CLIENT_LOG_PREFERRED_CONSOLE config, so that it's easier to
> setup than changing the kernel command line.

I guess that you plan to call add_preferred_console() in this case.
It is not the same as "fbcon" now:

1. It would cause that "drm_log" will always be enabled. It won't be
   possible to override it on the command line.

2. It will set "preferred_console". As a result,
   try_enable_default_console(newcon) will never be called.
   => there won't be any console when "drm_log" initialization
   fails and drm_log_register() is not called from some reason.

> 3) Use the kernel command line.


An ideal solution would be to allow calling drm_log_register() so
that it can be registered as the first console driver by
try_enable_default_console().

If this is not possible than we should think hard about it.
The console registration rules are already very complicated.
And we should think twice before adding any other twist
in the logic.

Best Regards,
Petr

Re: [PATCH v9 0/6] drm/log: Introduce a new boot logger to draw the kmsg on the screen

[Jocelyn Falempe]

On 18/12/2024 17:18, Petr Mladek wrote:
> On Wed 2024-12-18 15:58:46, Jocelyn Falempe wrote:
>> On 18/12/2024 15:18, Petr Mladek wrote:
>>> On Wed 2024-12-18 12:41:39, Jocelyn Falempe wrote:
>>>> On 18/12/2024 12:00, Geert Uytterhoeven wrote:
>>>>> Hi Jocelyn,
>>>>>
>>>>> On Wed, Dec 18, 2024 at 11:14 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>>> On 17/12/2024 15:54, Geert Uytterhoeven wrote:
>>>>>>> On Tue, Dec 17, 2024 at 3:46 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>>>>> On 17/12/2024 15:19, Geert Uytterhoeven wrote:
>>>>>>>>> On Wed, Dec 4, 2024 at 6:41 PM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>>>>>>> drm_log is a simple logger that uses the drm_client API to print the kmsg boot log on the screen.
>>>>>>>>>> This is not a full replacement to fbcon, as it will only print the kmsg.
>>>>>>>>>> It will never handle user input, or a terminal because this is better done in userspace.
>>>>>>>>>>
>>>>>>>>>> If you're curious on how it looks like, I've put a small demo here:
>>>>>>>>>> https://people.redhat.com/jfalempe/drm_log/drm_log_draft_boot_v2.mp4
>>>>>>>>>>
>>>>>>>>>> Design decisions:
>>>>>>>>>>        * It uses the drm_client API, so it should work on all drm drivers from the start.
>>>>>>>>>>        * It doesn't scroll the message, that way it doesn't need to redraw the whole screen for each new message.
>>>>>>>>>>          It also means it doesn't have to keep drawn messages in memory, to redraw them when scrolling.
>>>>>>>>>>        * It uses the new non-blocking console API, so it should work well with PREEMPT_RT
>>>>>>>>>
>>>>>>>>> I gave this a try on Koelsch (R-Car M2-W), using rcar-du.
>>>>>>>>> Unfortunately I don't see any kernel messages, and my monitor complains
>>>>>>>>> about no signal. Does this require special support from the driver?
>>>>>>>>
>>>>>>>> It doesn't require a special support from the driver. But as it is the
>>>>>>>> first drm client other than fbdev emulation, I'm not surprised it's
>>>>>>>> broken on some driver.
>>>>>>>> I know it works on virtio-gpu, nouveau, amdgpu, and even on a OnePlus 6
>>>>>>>> (Qualcomm SDM845/freedreno), without requiring driver changes.
>>>>>>>>
>>>>>>>> Do you have a serial console on this device, to check if there is
>>>>>>>> something in kmsg?
>>>>>>>
>>>>>>> Nothing interesting to see. Compared to the fbdev client:
>>>>>>>
>>>>>>>          rcar-du feb00000.display: [drm] Registered 2 planes with drm panic
>>>>>>>          [drm] Initialized rcar-du 1.0.0 for feb00000.display on minor 0
>>>>>>>          rcar-du feb00000.display: [drm] Device feb00000.display probed
>>>>>>>         -Console: switching to colour frame buffer device 240x67
>>>>>>>         -rcar-du feb00000.display: [drm] fb0: rcar-dudrmfb frame buffer device
>>>>>>>
>>>>>>> I did verify (by adding my own debug prints) that the code does
>>>>>>> get to the success case in drm_log_register().
>>>>>>> Thanks!
>>>>>>
>>>>>> Maybe you need to add console=drm_log to your kernel command line, so
>>>>>> the kernel will actually use this console.
>>>>>
>>>>> Thanks, that does the trick!
>>>>>
>>>>> Note that I do not need to specify any console= kernel command line
>>>>> parameter for the fbdev console.
>>>>
>>>> Yes, the fbcon console is tty0, which is hardcoded for historical reason.
>>>> Some architectures use add_preferred_console() to enable specific consoles,
>>>> I'm not sure it's allowed to use that from the drm_log_register() code.
>>>
>>> add_preferred_console() is used when the console should get enabled
>>> intentionally. I would split the intentions into two categories:
>>>
>>>     + requested by the end-user on the command line, see
>>>          __add_preferred_console(..., true) in console_setup()
>>>
>>>     + enabled by default by a hardware definition (manufacture), see
>>>          add_preferred_console() in:
>>>
>>> 	+ of_console_check(): generic solution via device tree
>>> 	+ acpi_parse_spcr(): generic solution via SPCR table
>>> 	+ *: hardcoded in few more HW-specific drivers
>>>
>>> add_preferred_console() causes the console will always get enabled
>>> when it is successfully initialized.
>>>
>>> So, should the "drm_log" console get always enabled?
>>
>> drm_log is a replacement for fbcon, which is always enabled, so I think it
>> should also be always enabled. Otherwise you won't get any console as fbcon
>> is no more available.
> 
> IMHO, it is not true that "fbcon" is always enabled. For example,
> if you mention "console=ttyS0,115200" on the command line then only
> the serial console is enabled.
> 
> Honestly, I do not understand completely the connection between the
> drm drivers and the console subsystem.
> 
> My understanding is that fbcon is registered via the very old and
> generic VT (Virtual Terminal) layer, see do_fbcon_takeover().
> 
> As a result, it gets registered/enabled by default when there is
> no other console defined on the command line or an
> add_preferred_console() call.
> 
> It is this code:
> 
> void register_console(struct console *newcon)
> {
> [...]
> 	/*
> 	 * See if we want to enable this console driver by default.
> 	 *
> 	 * Nope when a console is preferred by the command line, device
> 	 * tree, or SPCR.
> 	 *
> 	 * The first real console with tty binding (driver) wins. More
> 	 * consoles might get enabled before the right one is found.
> 	 *
> 	 * Note that a console with tty binding will have CON_CONSDEV
> 	 * flag set and will be first in the list.
> 	 */
> 	if (preferred_console < 0) {
> 		if (hlist_empty(&console_list) || !console_first()->device ||
> 		    console_first()->flags & CON_BOOT) {
> 			try_enable_default_console(newcon);
> 		}
> 	}
> [...]
> }
> 
> Note that try_enable_default_console(newcon) fails only when
> newcon->setup() fails. It means that the given console is almost
> always enabled when this function is called.
> 
> The important thing here might be the check of "!console_first()->device".
> It causes, that register_console() stop calling
> try_enable_default_console(newcon) when there is at least one console
> with the tty binding registered.
> 
> IMHO, this is the reason why "fbcon" is enabled by default. It is
> the first registered driver with the tty binding.
> 
> Also this might be the reason why "drm_log" is _not_ enabled by
> default. I guess that drm_log_register() is called too late.
> I guess that there already is enabled another console driver with
> tty binding at that time, most likely the serial console.
> 
> Could you please confirm this?
> What console is enabled when console=drm_log is not used, please?

For fbcon, it's the VT_CONSOLE config option that enable the console.
It is run very early in the boot, and the console is registered even if 
there are no framebuffer device or fbdev emulation.

so if I have VT_CONSOLE=y, then tty0 is the default console, even if 
there are no drivers to write the content somewhere.

[    0.103324] Console: colour dummy device 80x25

If I disable VT_CONSOLE, then the default console is ttyS0, even if in 
most laptop/desktop, the serial line is no more accessible.

So in order to mimic the fbcon behavior, drm_log should register a dummy 
console very early, without knowing if some hardware would be able to 
actually display something later.


> 
> 
>> drm_log doesn't really fit in the architecture/hardware model, because drm
>> drivers are available for a wide range of architecture, and a GPU can do
>> either fbdev/fbcon or drm_log.
>>>
>>>> I will still send a patch to add update the Kconfig help for drm_log, as
>>>> this command line argument is required to have it working.
>>>
>>> I guess that the drm_log consoles should get enabled only when
>>> requested by the user => documenting the command line parameter
>>> is the right solution here.
>>
>> Most embedded linux specify the console on the command line, but for
>> laptop/desktop distributions, it's not the case as fbcon is the default
>> since the beginning.
>>
>> I see a few options here:
>> 1) Use add_preferred_console("drm_log") if DRM_CLIENT_LOG is enabled for
>> x86_64 and maybe arm64, so at least the main users are covered.
>> 2) Have a DRM_CLIENT_LOG_PREFERRED_CONSOLE config, so that it's easier to
>> setup than changing the kernel command line.
> 
> I guess that you plan to call add_preferred_console() in this case.
> It is not the same as "fbcon" now:
> 
> 1. It would cause that "drm_log" will always be enabled. It won't be
>     possible to override it on the command line.

You can disable drm_log from the command line, by passing 
"drm_client.active=", in this case no drm_log console would be registered.
> 
> 2. It will set "preferred_console". As a result,
>     try_enable_default_console(newcon) will never be called.
>     => there won't be any console when "drm_log" initialization
>     fails and drm_log_register() is not called from some reason.
> 
I don't think it's a problem. It's the same currently with fbcon.
tty0 is the default console with dummy output. if fbdev initialization 
fails, then the only console available is the dummy console, which does 
nothing at all.
So at least there won't be any regression when switching from fbcon to 
drm_log.

>> 3) Use the kernel command line.
> 
> 
> An ideal solution would be to allow calling drm_log_register() so
> that it can be registered as the first console driver by
> try_enable_default_console().
> 
> If this is not possible than we should think hard about it.
> The console registration rules are already very complicated.
> And we should think twice before adding any other twist
> in the logic.

Yes, I agree, we shouldn't make it worse than what it is already.

I will experiment with add_preferred_console() or having an early dummy 
drm_log registration, and see what works best.

> 
> Best Regards,
> Petr
>