[RFC][PATCH v2 0/2] drm/panic: Add a drm panic handler

[RFC][PATCH v2 0/2] drm/panic: Add a drm panic handler

[Jocelyn Falempe]

This introduces a new drm panic handler, which displays a message when a panic occurs.
So when fbcon is disabled, you can still see a kernel panic.

This is one of the missing feature, when disabling VT/fbcon in the kernel:
https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/
Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel.

This is a proof of concept, and works only with simpledrm, using a new get_scanout_buffer() api

To test it, make sure you're using the simpledrm driver, and trigger a panic:
echo c > /proc/sysrq-trigger

v2
 * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann)
 * Add the panic reason to the panic message (Nerdopolis)
 * Add an exclamation mark (Nerdopolis)
 
I didn't reuse the fbdev functions yet, that would need some fbdev refactoring, because they rely on struct fb_info, and struct vc_data (for font/console). But I still plan to at least try it for v3.

A few more though:
 1) what about gpu with multiple monitor connected ?
maybe get_scanout_buffer() could return a list of scanout buffers ?
 2) I think for some GPU drivers, there might need a flush_scanout_buffer() function, that should be called after the scanout buffer has been filled ?

Best regards,

Jocelyn Falempe (2):
  drm/panic: Add a drm panic handler
  drm/simpledrm: Add drm_panic support

 drivers/gpu/drm/Kconfig          |  11 ++
 drivers/gpu/drm/Makefile         |   1 +
 drivers/gpu/drm/drm_drv.c        |   3 +
 drivers/gpu/drm/drm_panic.c      | 270 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/tiny/simpledrm.c |  17 ++
 include/drm/drm_drv.h            |  14 ++
 include/drm/drm_panic.h          |  41 +++++
 7 files changed, 357 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_panic.c
 create mode 100644 include/drm/drm_panic.h


base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
-- 
2.41.0

[PATCH v2 1/2] drm/panic: Add a drm panic handler

[Jocelyn Falempe]

This module displays a user friendly message when a kernel panic
occurs. It currently doesn't contain any debug information,
but that can be added later.

v2
 * Use get_scanout_buffer() instead of the drm client API.
  (Thomas Zimmermann)
 * Add the panic reason to the panic message (Nerdopolis)
 * Add an exclamation mark (Nerdopolis)

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/Kconfig     |  11 ++
 drivers/gpu/drm/Makefile    |   1 +
 drivers/gpu/drm/drm_drv.c   |   3 +
 drivers/gpu/drm/drm_panic.c | 270 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_drv.h       |  14 ++
 include/drm/drm_panic.h     |  41 ++++++
 6 files changed, 340 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_panic.c
 create mode 100644 include/drm/drm_panic.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index afb3b2f5f425..98d78f865180 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -99,6 +99,17 @@ config DRM_KMS_HELPER
 	help
 	  CRTC helpers for KMS drivers.
 
+config DRM_PANIC
+	bool "Display a user-friendly message when a kernel panic occurs"
+	depends on DRM && !FRAMEBUFFER_CONSOLE
+	select FONT_SUPPORT
+	default y if DRM_SIMPLEDRM
+	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
+	  console instead of fbcon.
+	  It will only work if your graphic driver supports this feature.
+
 config DRM_DEBUG_DP_MST_TOPOLOGY_REFS
         bool "Enable refcount backtrace history in the DP MST helpers"
 	depends on STACKTRACE_SUPPORT
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 7a09a89b493b..a525dd9a2751 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -72,6 +72,7 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += \
 	drm_privacy_screen_x86.o
 drm-$(CONFIG_DRM_ACCEL) += ../../accel/drm_accel.o
 obj-$(CONFIG_DRM)	+= drm.o
+drm-$(CONFIG_DRM_PANIC) += drm_panic.o
 
 obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 12687dd9e1ac..0e0f1e258845 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -45,6 +45,7 @@
 #include <drm/drm_mode_object.h>
 #include <drm/drm_print.h>
 #include <drm/drm_privacy_screen_machine.h>
+#include <drm/drm_panic.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -1067,6 +1068,7 @@ static void drm_core_exit(void)
 	unregister_chrdev(DRM_MAJOR, "drm");
 	debugfs_remove(drm_debugfs_root);
 	drm_sysfs_destroy();
+	drm_panic_exit();
 	idr_destroy(&drm_minors_idr);
 	drm_connector_ida_destroy();
 }
@@ -1078,6 +1080,7 @@ static int __init drm_core_init(void)
 	drm_connector_ida_init();
 	idr_init(&drm_minors_idr);
 	drm_memcpy_init_early();
+	drm_panic_init();
 
 	ret = drm_sysfs_init();
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
new file mode 100644
index 000000000000..32697325456f
--- /dev/null
+++ b/drivers/gpu/drm/drm_panic.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/*
+ * Copyright (c) 2023 Jocelyn Falempe <jfalempe@redhat.com>
+ * inspired by the drm_log driver from David Herrmann <dh.herrmann@gmail.com>
+ * Tux Ascii art taken from cowsay written by Tony Monroe
+ */
+
+#include <linux/font.h>
+#include <linux/iosys-map.h>
+#include <linux/kdebug.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/panic_notifier.h>
+#include <linux/types.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_panic.h>
+#include <drm/drm_print.h>
+
+
+MODULE_AUTHOR("Jocelyn Falempe");
+MODULE_DESCRIPTION("DRM PANIC");
+MODULE_LICENSE("GPL");
+
+/**
+ * DOC: DRM Panic
+ *
+ * This module displays a user friendly message on screen when a kernel panic
+ * occurs. It's useful when using a user-space console instead of fbcon.
+ * It's intended for end-user, so have minimal technical/debug information.
+ *
+ * It will display only one frame, so just clear it, and draw white pixels for
+ * the characters. Performance optimizations are low priority as the machine is
+ * already in an unusable state.
+ */
+
+/*
+ * List of active drm devices that can render a panic
+ */
+struct dpanic_drm_device {
+	struct list_head head;
+	struct drm_device *dev;
+};
+
+struct dpanic_line {
+	u32 len;
+	const char *txt;
+};
+
+#define PANIC_LINE(s) {.len = sizeof(s), .txt = s}
+
+const struct dpanic_line panic_msg[] = {
+	PANIC_LINE("KERNEL PANIC !"),
+	PANIC_LINE(""),
+	PANIC_LINE("Please reboot your computer."),
+	PANIC_LINE(""),
+};
+
+const struct dpanic_line logo[] = {
+	PANIC_LINE("     .--.        _"),
+	PANIC_LINE("    |o_o |      | |"),
+	PANIC_LINE("    |:_/ |      | |"),
+	PANIC_LINE("   //   \\ \\     |_|"),
+	PANIC_LINE("  (|     | )     _"),
+	PANIC_LINE(" /'\\_   _/`\\    (_)"),
+	PANIC_LINE(" \\___)=(___/"),
+};
+
+static LIST_HEAD(dpanic_devices);
+static DEFINE_MUTEX(dpanic_lock);
+
+#define IOSYS_WRITE8(offset, val) iosys_map_wr(screen_base, offset, u8, val)
+/*
+ * Only handle DRM_FORMAT_XRGB8888 for testing
+ */
+static inline void dpanic_draw_px(struct iosys_map *screen_base, size_t offset, u32 pixel_format,
+				  u8 r, u8 g, u8 b)
+{
+	switch (pixel_format) {
+	case DRM_FORMAT_XRGB8888:
+		IOSYS_WRITE8(offset++, b);
+		IOSYS_WRITE8(offset++, g);
+		IOSYS_WRITE8(offset++, r);
+		IOSYS_WRITE8(offset++, 0xff);
+		break;
+	default:
+		pr_err("Format not supported\n");
+		break;
+	/* TODO other format */
+	}
+}
+
+/* Draw a single character at given destination */
+static void dpanic_draw_char(char ch, size_t x, size_t y,
+			     struct drm_scanout_buffer *sb,
+			     const struct font_desc *font)
+{
+	size_t src_width, src_height, src_stride, i, dst_off;
+	const u8 *src;
+
+	src_width = font->width;
+	src_height = font->height;
+	src_stride = DIV_ROUND_UP(src_width, 8);
+
+	dst_off = x * font->width * sb->format->cpp[0] + y * font->height * sb->pitch;
+
+	src = font->data;
+	src += ch * src_height * src_stride;
+
+	while (src_height--) {
+		for (i = 0; i < src_width; ++i) {
+			/* only draw white pixels */
+			if (src[i / 8] & (0x80 >> (i % 8)))
+				dpanic_draw_px(&sb->map, dst_off + i * sb->format->cpp[0],
+					       sb->format->format, 0xff, 0xff, 0xff);
+		}
+		src += src_stride;
+		dst_off += sb->pitch;
+	}
+}
+
+static void dpanic_draw_line_centered(const struct dpanic_line *line, size_t y,
+				      struct drm_scanout_buffer *sb,
+				      const struct font_desc *font)
+{
+	size_t chars_per_line = sb->width / font->width;
+	size_t skip_left, x;
+
+	if (line->len > chars_per_line)
+		return;
+
+	skip_left = (chars_per_line - line->len) / 2;
+
+	for (x = 0; x < line->len; x++)
+		dpanic_draw_char(line->txt[x], skip_left + x, y, sb, font);
+}
+
+/*
+ * Draw the Tux logo at the upper left corner
+ */
+static void dpanic_draw_logo(struct drm_scanout_buffer *sb,
+			     const struct font_desc *font)
+{
+	size_t chars_per_line = sb->width / font->width;
+	size_t x, y;
+
+	for (y = 0; y < ARRAY_SIZE(logo); y++)
+		for (x = 0; x < logo[y].len && x < chars_per_line; x++)
+			dpanic_draw_char(logo[y].txt[x], x, y, sb, font);
+}
+
+/*
+ * Draw the panic message at the center of the screen
+ */
+static void dpanic_static_draw(struct drm_scanout_buffer *sb, const char *msg)
+{
+	size_t y, lines;
+	int skip_top;
+	size_t len = ARRAY_SIZE(panic_msg);
+	struct dpanic_line reason;
+	const struct font_desc *font = get_default_font(sb->width, sb->height, 0x8080, 0x8080);
+
+	if (!font)
+		return;
+
+	reason.len = strlen(msg);
+	reason.txt = msg;
+
+	lines = sb->height / font->height;
+	skip_top = (lines - len - 1) / 2;
+	if (skip_top < 0)
+		return;
+
+	/* clear screen */
+	iosys_map_memset(&sb->map, 0, 0, sb->height * sb->pitch);
+
+	for (y = 0; y < len; y++)
+		dpanic_draw_line_centered(&panic_msg[y], y + skip_top, sb, font);
+	dpanic_draw_line_centered(&reason, y + skip_top, sb, font);
+
+	if (skip_top >= ARRAY_SIZE(logo))
+		dpanic_draw_logo(sb, font);
+}
+
+static void drm_panic_device(struct drm_device *dev, const char *msg)
+{
+	struct drm_scanout_buffer sb;
+
+	if (dev->driver->get_scanout_buffer(dev, &sb))
+		return;
+
+	dpanic_static_draw(&sb, msg);
+}
+
+static int drm_panic(struct notifier_block *this, unsigned long event,
+		     void *ptr)
+{
+	struct dpanic_drm_device *dpanic_device;
+
+	list_for_each_entry(dpanic_device, &dpanic_devices, head) {
+		drm_panic_device(dpanic_device->dev, ptr);
+	}
+	return NOTIFY_OK;
+}
+
+struct notifier_block drm_panic_notifier = {
+	.notifier_call = drm_panic,
+};
+
+/**
+ * drm_panic_register() - Initialize DRM panic for a device
+ * @dev: the DRM device on which the panic screen will be displayed.
+ */
+void drm_panic_register(struct drm_device *dev)
+{
+	struct dpanic_drm_device *new;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return;
+
+	new->dev = dev;
+	list_add_tail(&new->head, &dpanic_devices);
+
+	drm_info(dev, "Registered with drm panic\n");
+}
+EXPORT_SYMBOL(drm_panic_register);
+
+/**
+ * drm_panic_unregister()
+ * @dev: the DRM device previously registered.
+ */
+void drm_panic_unregister(struct drm_device *dev)
+{
+	struct dpanic_drm_device *dpanic_device;
+	struct dpanic_drm_device *found = NULL;
+
+	list_for_each_entry(dpanic_device, &dpanic_devices, head) {
+		if (dpanic_device->dev == dev)
+			found = dpanic_device;
+	}
+	if (found) {
+		list_del(&found->head);
+		kfree(found);
+		drm_info(dev, "Unregistered with drm panic\n");
+	}
+}
+EXPORT_SYMBOL(drm_panic_unregister);
+
+/**
+ * drm_panic_init() - Initialize drm-panic subsystem
+ *
+ * register the panic notifier
+ */
+void drm_panic_init(void)
+{
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &drm_panic_notifier);
+}
+
+/**
+ * drm_log_exit() - Shutdown drm-panic subsystem
+ */
+void drm_panic_exit(void)
+{
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &drm_panic_notifier);
+}
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 89e2706cac56..e538c87116d3 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -43,6 +43,7 @@ struct dma_buf_attachment;
 struct drm_display_mode;
 struct drm_mode_create_dumb;
 struct drm_printer;
+struct drm_scanout_buffer;
 struct sg_table;
 
 /**
@@ -408,6 +409,19 @@ struct drm_driver {
 	 */
 	void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
 
+	/**
+	 * @get_scanout_buffer:
+	 *
+	 * Get the current scanout buffer, to display a panic message with drm_panic.
+	 * It is called from a panic callback, and must follow its restrictions.
+	 *
+	 * Returns:
+	 *
+	 * Zero on success, negative errno on failure.
+	 */
+	int (*get_scanout_buffer)(struct drm_device *dev,
+				  struct drm_scanout_buffer *sb);
+
 	/** @major: driver major number */
 	int major;
 	/** @minor: driver minor number */
diff --git a/include/drm/drm_panic.h b/include/drm/drm_panic.h
new file mode 100644
index 000000000000..db430b8dfbb2
--- /dev/null
+++ b/include/drm/drm_panic.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 or MIT */
+#ifndef __DRM_PANIC_H__
+#define __DRM_PANIC_H__
+
+/*
+ * Copyright (c) 2023 Jocelyn Falempe <jfalempe@redhat.com>
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/iosys-map.h>
+
+struct drm_device;
+
+struct drm_scanout_buffer {
+	const struct drm_format_info *format;
+	struct iosys_map map;
+	unsigned int pitch;
+	unsigned int width;
+	unsigned int height;
+};
+
+#ifdef CONFIG_DRM_PANIC
+
+void drm_panic_init(void);
+void drm_panic_exit(void);
+
+void drm_panic_register(struct drm_device *dev);
+void drm_panic_unregister(struct drm_device *dev);
+
+#else
+
+static inline void drm_panic_init(void) {}
+static inline void drm_panic_exit(void) {}
+
+static inline void drm_panic_register(struct drm_device *dev) {}
+static inline void drm_panic_unregister(struct drm_device *dev) {}
+
+#endif
+
+#endif /* __DRM_LOG_H__ */
-- 
2.41.0

[PATCH v2 2/2] drm/simpledrm: Add drm_panic support

[Jocelyn Falempe]

Add support for the drm_panic module, which displays a user-friendly
message to the screen when a kernel panic occurs.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/tiny/simpledrm.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 25e11ef11c4c..f0454b58ead3 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -23,6 +23,7 @@
 #include <drm/drm_gem_shmem_helper.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_probe_helper.h>
 
@@ -838,10 +839,24 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 		return ERR_PTR(ret);
 
 	drm_mode_config_reset(dev);
+	drm_panic_register(dev);
 
 	return sdev;
 }
 
+static int simpledrm_get_scanout_buffer(struct drm_device *dev,
+					struct drm_scanout_buffer *sb)
+{
+	struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
+
+	sb->width = sdev->mode.hdisplay;
+	sb->height = sdev->mode.vdisplay;
+	sb->pitch = sdev->pitch;
+	sb->format = sdev->format;
+	sb->map = sdev->screen_base;
+	return 0;
+}
+
 /*
  * DRM driver
  */
@@ -857,6 +872,7 @@ static struct drm_driver simpledrm_driver = {
 	.minor			= DRIVER_MINOR,
 	.driver_features	= DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
 	.fops			= &simpledrm_fops,
+	.get_scanout_buffer	= simpledrm_get_scanout_buffer,
 };
 
 /*
@@ -894,6 +910,7 @@ static int simpledrm_remove(struct platform_device *pdev)
 	struct drm_device *dev = &sdev->dev;
 
 	drm_dev_unplug(dev);
+	drm_panic_unregister(dev);
 
 	return 0;
 }
-- 
2.41.0

Re: [RFC][PATCH v2 0/2] drm/panic: Add a drm panic handler

[nerdopolis]

On Friday, September 15, 2023 4:28:20 AM EDT Jocelyn Falempe wrote:
> This introduces a new drm panic handler, which displays a message when a panic occurs.
> So when fbcon is disabled, you can still see a kernel panic.
> 
> This is one of the missing feature, when disabling VT/fbcon in the kernel:
> https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/
> Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel.
> 
> This is a proof of concept, and works only with simpledrm, using a new get_scanout_buffer() api
> 
> To test it, make sure you're using the simpledrm driver, and trigger a panic:
> echo c > /proc/sysrq-trigger
> 
This seems to work pretty good! With this one, I don't need to have Weston (or another display server) running for it to work this time.
The panic reason works, which is pretty sweet.

FYI: I do get a hunk that fails to apply in simpledrm_remove in drivers/gpu/drm/tiny/simpledrm.c
Seems to be a change in a recentish commit
https://github.com/torvalds/linux/commit/84e6da7ad5537826343636b846530ec2167d4a19

Thanks!
> v2
>  * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann)
>  * Add the panic reason to the panic message (Nerdopolis)
>  * Add an exclamation mark (Nerdopolis)
>  
> I didn't reuse the fbdev functions yet, that would need some fbdev refactoring, because they rely on struct fb_info, and struct vc_data (for font/console). But I still plan to at least try it for v3.
> 
> A few more though:
>  1) what about gpu with multiple monitor connected ?
> maybe get_scanout_buffer() could return a list of scanout buffers ?
>  2) I think for some GPU drivers, there might need a flush_scanout_buffer() function, that should be called after the scanout buffer has been filled ?
> 
> Best regards,
> 
> Jocelyn Falempe (2):
>   drm/panic: Add a drm panic handler
>   drm/simpledrm: Add drm_panic support
> 
>  drivers/gpu/drm/Kconfig          |  11 ++
>  drivers/gpu/drm/Makefile         |   1 +
>  drivers/gpu/drm/drm_drv.c        |   3 +
>  drivers/gpu/drm/drm_panic.c      | 270 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tiny/simpledrm.c |  17 ++
>  include/drm/drm_drv.h            |  14 ++
>  include/drm/drm_panic.h          |  41 +++++
>  7 files changed, 357 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_panic.c
>  create mode 100644 include/drm/drm_panic.h
> 
> 
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> 

Re: [RFC][PATCH v2 0/2] drm/panic: Add a drm panic handler

[Jocelyn Falempe]

On 16/09/2023 15:09, nerdopolis wrote:
> On Friday, September 15, 2023 4:28:20 AM EDT Jocelyn Falempe wrote:
>> This introduces a new drm panic handler, which displays a message when a panic occurs.
>> So when fbcon is disabled, you can still see a kernel panic.
>>
>> This is one of the missing feature, when disabling VT/fbcon in the kernel:
>> https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/
>> Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel.
>>
>> This is a proof of concept, and works only with simpledrm, using a new get_scanout_buffer() api
>>
>> To test it, make sure you're using the simpledrm driver, and trigger a panic:
>> echo c > /proc/sysrq-trigger
>>
> This seems to work pretty good! With this one, I don't need to have Weston (or another display server) running for it to work this time.
> The panic reason works, which is pretty sweet.

Thanks for testing, that's really appreciated.
> 
> FYI: I do get a hunk that fails to apply in simpledrm_remove in drivers/gpu/drm/tiny/simpledrm.c
> Seems to be a change in a recentish commit
> https://github.com/torvalds/linux/commit/84e6da7ad5537826343636b846530ec2167d4a19

Thanks for the head-up, when doing this RFC, I'm based on latest 
released version v6.5, to avoid having to rebase too often.
When it's closer to merging, I will rebase to drm-misc-next.

Best regards,

-- 

Jocelyn

Re: [RFC][PATCH v2 0/2] drm/panic: Add a drm panic handler

[Noralf Trønnes]

Hi,

On 9/15/23 10:28, Jocelyn Falempe wrote:
> This introduces a new drm panic handler, which displays a message when a panic occurs.
> So when fbcon is disabled, you can still see a kernel panic.
> 
> This is one of the missing feature, when disabling VT/fbcon in the kernel:
> https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/
> Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel.
> 
> This is a proof of concept, and works only with simpledrm, using a new get_scanout_buffer() api
> 

There's a panic handling entry in Documentation/gpu/todo.rst pointing to
some work done in this area.

Noralf.

> To test it, make sure you're using the simpledrm driver, and trigger a panic:
> echo c > /proc/sysrq-trigger
> 
> v2
>  * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann)
>  * Add the panic reason to the panic message (Nerdopolis)
>  * Add an exclamation mark (Nerdopolis)
>  
> I didn't reuse the fbdev functions yet, that would need some fbdev refactoring, because they rely on struct fb_info, and struct vc_data (for font/console). But I still plan to at least try it for v3.
> 
> A few more though:
>  1) what about gpu with multiple monitor connected ?
> maybe get_scanout_buffer() could return a list of scanout buffers ?
>  2) I think for some GPU drivers, there might need a flush_scanout_buffer() function, that should be called after the scanout buffer has been filled ?
> 
> Best regards,
> 
> Jocelyn Falempe (2):
>   drm/panic: Add a drm panic handler
>   drm/simpledrm: Add drm_panic support
> 
>  drivers/gpu/drm/Kconfig          |  11 ++
>  drivers/gpu/drm/Makefile         |   1 +
>  drivers/gpu/drm/drm_drv.c        |   3 +
>  drivers/gpu/drm/drm_panic.c      | 270 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tiny/simpledrm.c |  17 ++
>  include/drm/drm_drv.h            |  14 ++
>  include/drm/drm_panic.h          |  41 +++++
>  7 files changed, 357 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_panic.c
>  create mode 100644 include/drm/drm_panic.h
> 
> 
> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c

Re: [RFC][PATCH v2 0/2] drm/panic: Add a drm panic handler

[Jocelyn Falempe]

On 19/09/2023 01:19, Noralf Trønnes wrote:
> Hi,
> 
> On 9/15/23 10:28, Jocelyn Falempe wrote:
>> This introduces a new drm panic handler, which displays a message when a panic occurs.
>> So when fbcon is disabled, you can still see a kernel panic.
>>
>> This is one of the missing feature, when disabling VT/fbcon in the kernel:
>> https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/
>> Fbcon can be replaced by a userspace kms console, but the panic screen must be done in the kernel.
>>
>> This is a proof of concept, and works only with simpledrm, using a new get_scanout_buffer() api
>>
> 
> There's a panic handling entry in Documentation/gpu/todo.rst pointing to
> some work done in this area.

Thanks a lot for this pointer, I wasn't aware of this previous drm panic 
attempt. I hope this one will go a bit further ;)
Also I wasn't sure about what is allowed or not in the panic handler, 
and this doc makes things really clear.

Regarding the fbcon interactions, I prevent this at build time, I rely 
on userspace consoles, and hope they will progressively replace fbcon.

My real question is how did I missed that ? My google-fu failed me this 
time.

Best regards,

-- 

Jocelyn




> 
> Noralf.
> 
>> To test it, make sure you're using the simpledrm driver, and trigger a panic:
>> echo c > /proc/sysrq-trigger
>>
>> v2
>>   * Use get_scanout_buffer() instead of the drm client API. (Thomas Zimmermann)
>>   * Add the panic reason to the panic message (Nerdopolis)
>>   * Add an exclamation mark (Nerdopolis)
>>   
>> I didn't reuse the fbdev functions yet, that would need some fbdev refactoring, because they rely on struct fb_info, and struct vc_data (for font/console). But I still plan to at least try it for v3.
>>
>> A few more though:
>>   1) what about gpu with multiple monitor connected ?
>> maybe get_scanout_buffer() could return a list of scanout buffers ?
>>   2) I think for some GPU drivers, there might need a flush_scanout_buffer() function, that should be called after the scanout buffer has been filled ?
>>
>> Best regards,
>>
>> Jocelyn Falempe (2):
>>    drm/panic: Add a drm panic handler
>>    drm/simpledrm: Add drm_panic support
>>
>>   drivers/gpu/drm/Kconfig          |  11 ++
>>   drivers/gpu/drm/Makefile         |   1 +
>>   drivers/gpu/drm/drm_drv.c        |   3 +
>>   drivers/gpu/drm/drm_panic.c      | 270 +++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/tiny/simpledrm.c |  17 ++
>>   include/drm/drm_drv.h            |  14 ++
>>   include/drm/drm_panic.h          |  41 +++++
>>   7 files changed, 357 insertions(+)
>>   create mode 100644 drivers/gpu/drm/drm_panic.c
>>   create mode 100644 include/drm/drm_panic.h
>>
>>
>> base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
>