dri-devel Archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler
@ 2024-02-27 10:04 Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 1/8] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill Jocelyn Falempe
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, 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 with simpledrm, mgag200, ast, and imx.

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

or you can enable CONFIG_DRM_PANIC_DEBUG and echo 1 > /sys/kernel/debug/drm_panic/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)
 
v3:
 * Rework the drawing functions, to write the pixels line by line and
 to use the drm conversion helper to support other formats.
 (Thomas Zimmermann)
 
v4:
 * Fully support all simpledrm formats using drm conversion helpers
 * Rename dpanic_* to drm_panic_*, and have more coherent function name.
   (Thomas Zimmermann)
 * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann)
 * Remove the default y to DRM_PANIC config option (Thomas Zimmermann)
 * Add foreground/background color config option
 * Fix the bottom lines not painted if the framebuffer height
   is not a multiple of the font height.
 * Automatically register the driver to drm_panic, if the function
   get_scanout_buffer() exists. (Thomas Zimmermann)
 * Add mgag200 support.
 
v5:
 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font.
   (Thomas Zimmermann)
 * Also add drm_fb_fill() to fill area with background color.
 * Add draw_pixel_xy() API for drivers that can't provide a linear buffer.
 * Add a flush() callback for drivers that needs to synchronize the buffer.
 * Add a void *private field, so drivers can pass private data to
   draw_pixel_xy() and flush(). 
 * Add ast support.
 * Add experimental imx/ipuv3 support, to test on an ARM hw. (Maxime Ripard)

v6:
 * Fix sparse and __le32 warnings
 * Drop the IMX/IPUV3 experiment, it was just to show that it works also on
   ARM devices.

v7:
 * Add a check to see if the 4cc format is supported by drm_panic.
 * Add a drm/plane helper to loop over all visible primary buffer,
   simplifying the get_scanout_buffer implementations
 * Add a generic implementation for drivers that uses drm_fb_dma. (Maxime Ripard)
 * Add back the IMX/IPUV3 support, and use the generic implementation. (Maxime Ripard)

v8:
 * Directly register each plane to the panic notifier (Sima)
 * Replace get_scanout_buffer() with set_scanout_buffer() to simplify
   the locking. (Thomas Zimmermann)
 * Add a debugfs entry, to trigger the drm_panic without a real panic (Sima)
 * Fix the drm_panic Documentation, and include it in drm-kms.rst

Best regards,


Jocelyn Falempe (8):
  drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
  drm/panic: Add a drm panic handler
  drm/panic: Add debugfs entry to test without triggering panic.
  drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
  drm/simpledrm: Add drm_panic support
  drm/mgag200: Add drm_panic support
  drm/imx: Add drm_panic support
  drm/ast: Add drm_panic support

 Documentation/gpu/drm-kms.rst             |  12 +
 drivers/gpu/drm/Kconfig                   |  32 ++
 drivers/gpu/drm/Makefile                  |   1 +
 drivers/gpu/drm/ast/ast_mode.c            |   6 +
 drivers/gpu/drm/drm_fb_dma_helper.c       |  37 ++
 drivers/gpu/drm/drm_format_helper.c       | 432 +++++++++++++++++----
 drivers/gpu/drm/drm_panic.c               | 447 ++++++++++++++++++++++
 drivers/gpu/drm/drm_plane.c               |   3 +
 drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c   |   5 +
 drivers/gpu/drm/mgag200/mgag200_g200.c    |   2 +
 drivers/gpu/drm/mgag200/mgag200_g200eh.c  |   2 +
 drivers/gpu/drm/mgag200/mgag200_g200eh3.c |   2 +
 drivers/gpu/drm/mgag200/mgag200_g200er.c  |   2 +
 drivers/gpu/drm/mgag200/mgag200_g200ev.c  |   2 +
 drivers/gpu/drm/mgag200/mgag200_g200ew3.c |   2 +
 drivers/gpu/drm/mgag200/mgag200_g200se.c  |   2 +
 drivers/gpu/drm/mgag200/mgag200_g200wb.c  |   2 +
 drivers/gpu/drm/mgag200/mgag200_mode.c    |   7 +
 drivers/gpu/drm/tiny/simpledrm.c          |  17 +
 include/drm/drm_fb_dma_helper.h           |   4 +
 include/drm/drm_format_helper.h           |   9 +
 include/drm/drm_modeset_helper_vtables.h  |  11 +
 include/drm/drm_panic.h                   |  37 ++
 include/drm/drm_plane.h                   |  17 +
 24 files changed, 1012 insertions(+), 81 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_panic.c
 create mode 100644 include/drm/drm_panic.h


base-commit: bfa4437fd3938ae2e186e7664b2db65bb8775670
-- 
2.43.0


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

* [PATCH v8 1/8] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 2/8] drm/panic: Add a drm panic handler Jocelyn Falempe
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, Jocelyn Falempe

This is needed for drm_panic, to draw the font, and fill
the background color, in multiple color format.

v5:
 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font.
 * Also add drm_fb_fill() to fill area with background color.
v6:
 * fix __le32 conversion warning found by the kernel test bot

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/drm_format_helper.c | 432 ++++++++++++++++++++++------
 include/drm/drm_format_helper.h     |   9 +
 2 files changed, 360 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index b1be458ed4dd..2d9646cefc4f 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -111,6 +111,153 @@ void drm_format_conv_state_release(struct drm_format_conv_state *state)
 }
 EXPORT_SYMBOL(drm_format_conv_state_release);
 
+static __le16 drm_format_xrgb8888_to_rgb565(__le32 val32)
+{
+	u16 val16;
+	u32 pix;
+
+	pix = le32_to_cpu(val32);
+	val16 = ((pix & 0x00F80000) >> 8) |
+		((pix & 0x0000FC00) >> 5) |
+		((pix & 0x000000F8) >> 3);
+	return cpu_to_le16(val16);
+}
+
+static __le16 drm_format_xrgb8888_to_rgba5551(__le32 val32)
+{
+	u16 val16;
+	u32 pix;
+
+	pix = le32_to_cpu(val32);
+	val16 = ((pix & 0x00f80000) >> 8) |
+		((pix & 0x0000f800) >> 5) |
+		((pix & 0x000000f8) >> 2) |
+		BIT(0); /* set alpha bit */
+	return cpu_to_le16(val16);
+}
+
+static __le16 drm_format_xrgb8888_to_xrgb1555(__le32 val32)
+{
+	u16 val16;
+	u32 pix;
+
+	pix = le32_to_cpu(val32);
+	val16 = ((pix & 0x00f80000) >> 9) |
+		((pix & 0x0000f800) >> 6) |
+		((pix & 0x000000f8) >> 3);
+	return cpu_to_le16(val16);
+}
+
+static __le16 drm_format_xrgb8888_to_argb1555(__le32 val32)
+{
+	u16 val16;
+	u32 pix;
+
+	pix = le32_to_cpu(val32);
+	val16 = BIT(15) | /* set alpha bit */
+		((pix & 0x00f80000) >> 9) |
+		((pix & 0x0000f800) >> 6) |
+		((pix & 0x000000f8) >> 3);
+	return cpu_to_le16(val16);
+}
+
+static __le32 drm_format_xrgb8888_to_argb8888(__le32 pix)
+{
+	u32 val32;
+
+	val32 = le32_to_cpu(pix);
+	val32 |= GENMASK(31, 24); /* fill alpha bits */
+	return cpu_to_le32(val32);
+}
+
+static __le32 drm_format_xrgb8888_to_xbgr8888(__le32 pix)
+{
+	u32 val32;
+
+	val32 = le32_to_cpu(pix);
+	val32 = ((val32 & 0x00ff0000) >> 16) <<  0 |
+		((val32 & 0x0000ff00) >>  8) <<  8 |
+		((val32 & 0x000000ff) >>  0) << 16 |
+		((val32 & 0xff000000) >> 24) << 24;
+	return cpu_to_le32(val32);
+}
+
+static __le32 drm_format_xrgb8888_to_abgr8888(__le32 pix)
+{
+	u32 val32;
+
+	val32 = le32_to_cpu(pix);
+	val32 = ((val32 & 0x00ff0000) >> 16) <<  0 |
+		((val32 & 0x0000ff00) >>  8) <<  8 |
+		((val32 & 0x000000ff) >>  0) << 16 |
+		GENMASK(31, 24); /* fill alpha bits */
+	return cpu_to_le32(val32);
+}
+
+static __le32 drm_format_xrgb8888_to_xrgb2101010(__le32 pix)
+{
+	u32 val32;
+
+	val32 = le32_to_cpu(pix);
+	val32 = ((val32 & 0x000000FF) << 2) |
+		((val32 & 0x0000FF00) << 4) |
+		((val32 & 0x00FF0000) << 6);
+	return cpu_to_le32(val32 | ((val32 >> 8) & 0x00300C03));
+}
+
+static __le32 drm_format_xrgb8888_to_argb2101010(__le32 pix)
+{
+	u32 val32;
+
+	val32 = le32_to_cpu(pix);
+	val32 = ((val32 & 0x000000FF) << 2) |
+		((val32 & 0x0000FF00) << 4) |
+		((val32 & 0x00FF0000) << 6);
+	val32 = GENMASK(31, 30) | /* set alpha bits */
+	      val32 | ((val32 >> 8) & 0x00300c03);
+	return cpu_to_le32(val32);
+}
+
+/**
+ * drm_fb_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 unknown.
+ */
+u32 drm_fb_convert_from_xrgb8888(u32 color, u32 format)
+{
+	__le32 pix = cpu_to_le32(color);
+
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+		return le16_to_cpu(drm_format_xrgb8888_to_rgb565(pix));
+	case DRM_FORMAT_RGBA5551:
+		return le16_to_cpu(drm_format_xrgb8888_to_rgba5551(pix));
+	case DRM_FORMAT_XRGB1555:
+		return le16_to_cpu(drm_format_xrgb8888_to_xrgb1555(pix));
+	case DRM_FORMAT_ARGB1555:
+		return le16_to_cpu(drm_format_xrgb8888_to_argb1555(pix));
+	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_XRGB8888:
+		return le32_to_cpu(pix);
+	case DRM_FORMAT_ARGB8888:
+		return le32_to_cpu(drm_format_xrgb8888_to_argb8888(pix));
+	case DRM_FORMAT_XBGR8888:
+		return le32_to_cpu(drm_format_xrgb8888_to_xbgr8888(pix));
+	case DRM_FORMAT_XRGB2101010:
+		return le32_to_cpu(drm_format_xrgb8888_to_xrgb2101010(pix));
+	case DRM_FORMAT_ARGB2101010:
+		return le32_to_cpu(drm_format_xrgb8888_to_argb2101010(pix));
+	default:
+		WARN_ONCE(1, "Can't convert to %p4cc\n", &format);
+		return 0;
+	}
+}
+EXPORT_SYMBOL(drm_fb_convert_from_xrgb8888);
+
 static unsigned int clip_offset(const struct drm_rect *clip, unsigned int pitch, unsigned int cpp)
 {
 	return clip->y1 * pitch + clip->x1 * cpp;
@@ -366,6 +513,193 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
 }
 EXPORT_SYMBOL(drm_fb_swab);
 
+static void drm_fb_r1_to_16bit(struct iosys_map *dmap, unsigned int dpitch,
+			       const u8 *sbuf8, unsigned int spitch,
+			       unsigned int height, unsigned int width,
+			       __le16 fg16, __le16 bg16)
+{
+	unsigned int l, x;
+	__le16 val16;
+
+	for (l = 0; l < height; l++) {
+		for (x = 0; x < width; x++) {
+			val16 = (sbuf8[(l * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg16 : bg16;
+			iosys_map_wr(dmap, l * dpitch + x * sizeof(u16), u16, le16_to_cpu(val16));
+		}
+	}
+}
+
+static void drm_fb_r1_to_24bit(struct iosys_map *dmap, unsigned int dpitch,
+			       const u8 *sbuf8, unsigned int spitch,
+			       unsigned int height, unsigned int width,
+			       __le32 fg32, __le32 bg32)
+{
+	unsigned int l, x;
+	__le32 color;
+	u32 val32;
+
+	for (l = 0; l < height; l++) {
+		for (x = 0; x < width; x++) {
+			u32 off = l * dpitch + x * 3;
+
+			color = (sbuf8[(l * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg32 : bg32;
+			val32 = le32_to_cpu(color);
+
+			/* write blue-green-red to output in little endianness */
+			iosys_map_wr(dmap, off, u8, (val32 & 0x000000FF) >> 0);
+			iosys_map_wr(dmap, off + 1, u8, (val32 & 0x0000FF00) >> 8);
+			iosys_map_wr(dmap, off + 2, u8, (val32 & 0x00FF0000) >> 16);
+		}
+	}
+}
+
+static void drm_fb_r1_to_32bit(struct iosys_map *dmap, unsigned int dpitch,
+			       const u8 *sbuf8, unsigned int spitch,
+			       unsigned int height, unsigned int width,
+			       __le32 fg32, __le32 bg32)
+{
+	unsigned int l, x;
+	__le32 val32;
+
+	for (l = 0; l < height; l++) {
+		for (x = 0; x < width; x++) {
+			val32 = (sbuf8[(l * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg32 : bg32;
+			iosys_map_wr(dmap, l * dpitch + x * sizeof(u32), u32, le32_to_cpu(val32));
+		}
+	}
+}
+
+/**
+ * drm_fb_blit_from_r1 - convert a monochrome image to a linear framebuffer
+ * @dmap: destination iosys_map
+ * @dpitch: destination pitch in bytes
+ * @sbuf8: source buffer, in monochrome format, 8 pixels per byte.
+ * @spitch: source pitch in bytes
+ * @height: height of the image to copy, in pixels
+ * @width: width of the image to copy, in pixels
+ * @fg_color: foreground color, in destination format
+ * @bg_color: background color, in destination format
+ * @pixel_width: pixel width in bytes.
+ *
+ * This can be used to draw font which are monochrome images, to a framebuffer
+ * in other supported format.
+ */
+void drm_fb_blit_from_r1(struct iosys_map *dmap, unsigned int dpitch,
+			 const u8 *sbuf8, unsigned int spitch,
+			 unsigned int height, unsigned int width,
+			 u32 fg_color, u32 bg_color,
+			 unsigned int pixel_width)
+{
+	switch (pixel_width) {
+	case 2:
+		drm_fb_r1_to_16bit(dmap, dpitch, sbuf8, spitch,
+				   height, width,
+				   cpu_to_le16(fg_color),
+				   cpu_to_le16(bg_color));
+	break;
+	case 3:
+		drm_fb_r1_to_24bit(dmap, dpitch, sbuf8, spitch,
+				   height, width,
+				   cpu_to_le32(fg_color),
+				   cpu_to_le32(bg_color));
+	break;
+	case 4:
+		drm_fb_r1_to_32bit(dmap, dpitch, sbuf8, spitch,
+				   height, width,
+				   cpu_to_le32(fg_color),
+				   cpu_to_le32(bg_color));
+	break;
+	default:
+		WARN_ONCE(1, "Can't blit with pixel width %d\n", pixel_width);
+	}
+}
+EXPORT_SYMBOL(drm_fb_blit_from_r1);
+
+static void drm_fb_fill8(struct iosys_map *dmap, unsigned int dpitch,
+			 unsigned int height, unsigned int width,
+			 u8 color)
+{
+	unsigned int l, x;
+
+	for (l = 0; l < height; l++)
+		for (x = 0; x < width; x++)
+			iosys_map_wr(dmap, l * dpitch + x * sizeof(u8), u8, color);
+}
+
+static void drm_fb_fill16(struct iosys_map *dmap, unsigned int dpitch,
+			  unsigned int height, unsigned int width,
+			  u16 color)
+{
+	unsigned int l, x;
+
+	for (l = 0; l < height; l++)
+		for (x = 0; x < width; x++)
+			iosys_map_wr(dmap, l * dpitch + x * sizeof(u16), u16, color);
+}
+
+static void drm_fb_fill24(struct iosys_map *dmap, unsigned int dpitch,
+			  unsigned int height, unsigned int width,
+			  u32 color)
+{
+	unsigned int l, x;
+
+	for (l = 0; l < height; l++) {
+		for (x = 0; x < width; x++) {
+			unsigned int off = l * 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_fb_fill32(struct iosys_map *dmap, unsigned int dpitch,
+			  unsigned int height, unsigned int width,
+			  u32 color)
+{
+	unsigned int l, x;
+
+	for (l = 0; l < height; l++)
+		for (x = 0; x < width; x++)
+			iosys_map_wr(dmap, l * dpitch + x * sizeof(u32), u32, color);
+}
+
+/**
+ * drm_fb_fill - Fill a rectangle with a color
+ * @dmap: destination iosys_map, pointing to the top left corner of the rectangle
+ * @dpitch: destination pitch in bytes
+ * @height: height of the rectangle, in pixels
+ * @width: width of the rectangle, in pixels
+ * @color: color to fill the rectangle.
+ * @pixel_width: pixel width in bytes
+ *
+ * Fill a rectangle with a color, in a linear framebuffer.
+ */
+void drm_fb_fill(struct iosys_map *dmap, unsigned int dpitch,
+			 unsigned int height, unsigned int width,
+			 u32 color, unsigned int pixel_width)
+{
+	switch (pixel_width) {
+	case 1:
+		drm_fb_fill8(dmap, dpitch, height, width, color);
+	break;
+	case 2:
+		drm_fb_fill16(dmap, dpitch, height, width, color);
+	break;
+	case 3:
+		drm_fb_fill24(dmap, dpitch, height, width, color);
+	break;
+	case 4:
+		drm_fb_fill32(dmap, dpitch, height, width, color);
+	break;
+	default:
+		WARN_ONCE(1, "Can't fill with pixel width %d\n", pixel_width);
+	}
+}
+EXPORT_SYMBOL(drm_fb_fill);
+
 static void drm_fb_xrgb8888_to_rgb332_line(void *dbuf, const void *sbuf, unsigned int pixels)
 {
 	u8 *dbuf8 = dbuf;
@@ -420,15 +754,9 @@ static void drm_fb_xrgb8888_to_rgb565_line(void *dbuf, const void *sbuf, unsigne
 	__le16 *dbuf16 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u16 val16;
-	u32 pix;
 
 	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		val16 = ((pix & 0x00F80000) >> 8) |
-			((pix & 0x0000FC00) >> 5) |
-			((pix & 0x000000F8) >> 3);
-		dbuf16[x] = cpu_to_le16(val16);
+		dbuf16[x] = drm_format_xrgb8888_to_rgb565(sbuf32[x]);
 	}
 }
 
@@ -498,16 +826,9 @@ static void drm_fb_xrgb8888_to_xrgb1555_line(void *dbuf, const void *sbuf, unsig
 	__le16 *dbuf16 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u16 val16;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		val16 = ((pix & 0x00f80000) >> 9) |
-			((pix & 0x0000f800) >> 6) |
-			((pix & 0x000000f8) >> 3);
-		dbuf16[x] = cpu_to_le16(val16);
-	}
+	for (x = 0; x < pixels; x++)
+		dbuf16[x] = drm_format_xrgb8888_to_xrgb1555(sbuf32[x]);
 }
 
 /**
@@ -550,17 +871,9 @@ static void drm_fb_xrgb8888_to_argb1555_line(void *dbuf, const void *sbuf, unsig
 	__le16 *dbuf16 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u16 val16;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		val16 = BIT(15) | /* set alpha bit */
-			((pix & 0x00f80000) >> 9) |
-			((pix & 0x0000f800) >> 6) |
-			((pix & 0x000000f8) >> 3);
-		dbuf16[x] = cpu_to_le16(val16);
-	}
+	for (x = 0; x < pixels; x++)
+		dbuf16[x] = drm_format_xrgb8888_to_argb1555(sbuf32[x]);
 }
 
 /**
@@ -603,17 +916,9 @@ static void drm_fb_xrgb8888_to_rgba5551_line(void *dbuf, const void *sbuf, unsig
 	__le16 *dbuf16 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u16 val16;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		val16 = ((pix & 0x00f80000) >> 8) |
-			((pix & 0x0000f800) >> 5) |
-			((pix & 0x000000f8) >> 2) |
-			BIT(0); /* set alpha bit */
-		dbuf16[x] = cpu_to_le16(val16);
-	}
+	for (x = 0; x < pixels; x++)
+		dbuf16[x] = drm_format_xrgb8888_to_rgba5551(sbuf32[x]);
 }
 
 /**
@@ -707,13 +1012,9 @@ static void drm_fb_xrgb8888_to_argb8888_line(void *dbuf, const void *sbuf, unsig
 	__le32 *dbuf32 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		pix |= GENMASK(31, 24); /* fill alpha bits */
-		dbuf32[x] = cpu_to_le32(pix);
-	}
+	for (x = 0; x < pixels; x++)
+		dbuf32[x] = drm_format_xrgb8888_to_argb8888(sbuf32[x]);
 }
 
 /**
@@ -756,16 +1057,9 @@ static void drm_fb_xrgb8888_to_abgr8888_line(void *dbuf, const void *sbuf, unsig
 	__le32 *dbuf32 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		pix = ((pix & 0x00ff0000) >> 16) <<  0 |
-		      ((pix & 0x0000ff00) >>  8) <<  8 |
-		      ((pix & 0x000000ff) >>  0) << 16 |
-		      GENMASK(31, 24); /* fill alpha bits */
-		*dbuf32++ = cpu_to_le32(pix);
-	}
+	for (x = 0; x < pixels; x++)
+		*dbuf32++ = drm_format_xrgb8888_to_abgr8888(sbuf32[x]);
 }
 
 static void drm_fb_xrgb8888_to_abgr8888(struct iosys_map *dst, const unsigned int *dst_pitch,
@@ -787,16 +1081,9 @@ static void drm_fb_xrgb8888_to_xbgr8888_line(void *dbuf, const void *sbuf, unsig
 	__le32 *dbuf32 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		pix = ((pix & 0x00ff0000) >> 16) <<  0 |
-		      ((pix & 0x0000ff00) >>  8) <<  8 |
-		      ((pix & 0x000000ff) >>  0) << 16 |
-		      ((pix & 0xff000000) >> 24) << 24;
-		*dbuf32++ = cpu_to_le32(pix);
-	}
+	for (x = 0; x < pixels; x++)
+		*dbuf32++ = drm_format_xrgb8888_to_xbgr8888(sbuf32[x]);
 }
 
 static void drm_fb_xrgb8888_to_xbgr8888(struct iosys_map *dst, const unsigned int *dst_pitch,
@@ -818,17 +1105,9 @@ static void drm_fb_xrgb8888_to_xrgb2101010_line(void *dbuf, const void *sbuf, un
 	__le32 *dbuf32 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u32 val32;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		val32 = ((pix & 0x000000FF) << 2) |
-			((pix & 0x0000FF00) << 4) |
-			((pix & 0x00FF0000) << 6);
-		pix = val32 | ((val32 >> 8) & 0x00300C03);
-		*dbuf32++ = cpu_to_le32(pix);
-	}
+	for (x = 0; x < pixels; x++)
+		*dbuf32++ = drm_format_xrgb8888_to_xrgb2101010(sbuf32[x]);
 }
 
 /**
@@ -872,18 +1151,9 @@ static void drm_fb_xrgb8888_to_argb2101010_line(void *dbuf, const void *sbuf, un
 	__le32 *dbuf32 = dbuf;
 	const __le32 *sbuf32 = sbuf;
 	unsigned int x;
-	u32 val32;
-	u32 pix;
 
-	for (x = 0; x < pixels; x++) {
-		pix = le32_to_cpu(sbuf32[x]);
-		val32 = ((pix & 0x000000ff) << 2) |
-			((pix & 0x0000ff00) << 4) |
-			((pix & 0x00ff0000) << 6);
-		pix = GENMASK(31, 30) | /* set alpha bits */
-		      val32 | ((val32 >> 8) & 0x00300c03);
-		*dbuf32++ = cpu_to_le32(pix);
-	}
+	for (x = 0; x < pixels; x++)
+		*dbuf32++ = drm_format_xrgb8888_to_argb2101010(sbuf32[x]);
 }
 
 /**
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index f13b34e0b752..f416f0bef52d 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -66,6 +66,7 @@ void *drm_format_conv_state_reserve(struct drm_format_conv_state *state,
 				    size_t new_size, gfp_t flags);
 void drm_format_conv_state_release(struct drm_format_conv_state *state);
 
+u32 drm_fb_convert_from_xrgb8888(u32 color, u32 format);
 unsigned int drm_fb_clip_offset(unsigned int pitch, const struct drm_format_info *format,
 				const struct drm_rect *clip);
 
@@ -76,6 +77,14 @@ void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
 		 const struct iosys_map *src, const struct drm_framebuffer *fb,
 		 const struct drm_rect *clip, bool cached,
 		 struct drm_format_conv_state *state);
+void drm_fb_blit_from_r1(struct iosys_map *dmap, unsigned int dpitch,
+			 const u8 *sbuf8, unsigned int spitch,
+			 unsigned int height, unsigned int width,
+			 u32 fg_color, u32 bg_color,
+			 unsigned int pixel_width);
+void drm_fb_fill(struct iosys_map *dmap, unsigned int dpitch,
+		 unsigned int height, unsigned int width,
+		 u32 color, unsigned int pixel_width);
 void drm_fb_xrgb8888_to_rgb332(struct iosys_map *dst, const unsigned int *dst_pitch,
 			       const struct iosys_map *src, const struct drm_framebuffer *fb,
 			       const struct drm_rect *clip, struct drm_format_conv_state *state);
-- 
2.43.0


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

* [PATCH v8 2/8] drm/panic: Add a drm panic handler
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 1/8] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic Jocelyn Falempe
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, 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)

v3
 * Rework the drawing functions, to write the pixels line by line and
 to use the drm conversion helper to support other formats.
 (Thomas Zimmermann)

v4
 * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann)
 * Remove the default y to DRM_PANIC config option (Thomas Zimmermann)
 * Add foreground/background color config option
 * Fix the bottom lines not painted if the framebuffer height
   is not a multiple of the font height.
 * Automatically register the device to drm_panic, if the function
   get_scanout_buffer exists. (Thomas Zimmermann)

v5
 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font.
 * Also add drm_fb_fill() to fill area with background color.
 * Add draw_pixel_xy() API for drivers that can't provide a linear buffer.
 * Add a flush() callback for drivers that needs to synchronize the buffer.
 * Add a void *private field, so drivers can pass private data to
   draw_pixel_xy() and flush().

v6
 * Fix sparse warning for panic_msg and logo.

v7
 * Add select DRM_KMS_HELPER for the color conversion functions.

v8
 * Register directly each plane to the panic notifier (Sima)
 * Add raw_spinlock to properly handle concurrency (Sima)
 * Register plane instead of device, to avoid looping through plane
   list, and simplify code.
 * Replace get_scanout_buffer() logic with drm_panic_set_buffer()
  (Thomas Zimmermann)
 * Removed the draw_pixel_xy() API, will see later if it can be added back.

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 Documentation/gpu/drm-kms.rst            |  12 +
 drivers/gpu/drm/Kconfig                  |  23 ++
 drivers/gpu/drm/Makefile                 |   1 +
 drivers/gpu/drm/drm_panic.c              | 400 +++++++++++++++++++++++
 drivers/gpu/drm/drm_plane.c              |   3 +
 include/drm/drm_modeset_helper_vtables.h |  11 +
 include/drm/drm_panic.h                  |  37 +++
 include/drm/drm_plane.h                  |  17 +
 8 files changed, 504 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_panic.c
 create mode 100644 include/drm/drm_panic.h

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 13d3627d8bc0..b64334661aeb 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -398,6 +398,18 @@ Plane Damage Tracking Functions Reference
 .. kernel-doc:: include/drm/drm_damage_helper.h
    :internal:
 
+Plane Panic Feature
+-------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_panic.c
+   :doc: overview
+
+Plane Panic Functions Reference
+-------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_panic.c
+   :export:
+
 Display Modes Function Reference
 ================================
 
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 872edb47bb53..c17d8a8f6877 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -102,6 +102,29 @@ 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 DRM_KMS_HELPER
+	select FONT_SUPPORT
+	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.
+	  To support Hi-DPI Display, you can enable bigger fonts like
+	  FONT_TER16x32
+
+config DRM_PANIC_FOREGROUND_COLOR
+	hex "Drm panic screen foreground color, in RGB"
+	depends on DRM_PANIC
+	default 0xffffff
+
+config DRM_PANIC_BACKGROUND_COLOR
+	hex "Drm panic screen background color, in RGB"
+	depends on DRM_PANIC
+	default 0x000000
+
 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 104b42df2e95..49905b7e333f 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -60,6 +60,7 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += \
 	drm_privacy_screen.o \
 	drm_privacy_screen_x86.o
 drm-$(CONFIG_DRM_ACCEL) += ../../accel/drm_accel.o
+drm-$(CONFIG_DRM_PANIC) += drm_panic.o
 obj-$(CONFIG_DRM)	+= drm.o
 
 obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += drm_panel_orientation_quirks.o
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
new file mode 100644
index 000000000000..c9f386476ef9
--- /dev/null
+++ b/drivers/gpu/drm/drm_panic.c
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/*
+ * Copyright (c) 2023 Red Hat.
+ * Author: 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/panic_notifier.h>
+#include <linux/types.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_panic.h>
+#include <drm/drm_plane.h>
+#include <drm/drm_print.h>
+
+
+MODULE_AUTHOR("Jocelyn Falempe");
+MODULE_DESCRIPTION("DRM panic handler");
+MODULE_LICENSE("GPL");
+
+/**
+ * DOC: overview
+ *
+ * To enable DRM panic for a driver, you should register the primary plane
+ * with drm_panic_register(). Then when a scanout buffer is set for this plane,
+ * call drm_panic_set_buffer(), so if a panic occurs, it will draw to this.
+ * Make sure to update it when the scanout buffer changes. Also you should call
+ * drm_panic_unset_buffer() when the plane is disabled, or when the scanout
+ * buffer is no more accessible.
+ */
+
+/*
+ * This module displays a user friendly message on screen when a kernel panic
+ * occurs. This is conflicting with fbcon, so you can only enable it when fbcon
+ * is disabled.
+ * It's intended for end-user, so have minimal technical/debug information.
+ *
+ * Implementation details:
+ *
+ * It is a panic handler, so it can't take lock, allocate memory, run tasks/irq,
+ * or attempt to sleep. It's a best effort, and it may not be able to display
+ * the message in all situations (like if the panic occurs in the middle of a
+ * modesetting).
+ * It will display only one static frame, so performance optimizations are low
+ * priority as the machine is already in an unusable state.
+ */
+
+/**
+ * struct drm_scanout_buffer - DRM scanout buffer
+ *
+ * This structure holds the information necessary for drm_panic to draw the
+ * panic screen, and display it.
+ */
+struct drm_scanout_buffer {
+	/**
+	 * @lock:
+	 *
+	 * a raw spinlock to make sure that when the panic handler is running
+	 * the data in this struct is valid.
+	 */
+	struct raw_spinlock lock;
+	/**
+	 * @format:
+	 *
+	 * drm format of the scanout buffer.
+	 */
+	const struct drm_format_info *format;
+	/**
+	 * @map:
+	 *
+	 * Virtual address of the scanout buffer, either in memory or iomem.
+	 * The scanout buffer should be in linear format, and can be directly
+	 * sent to the display hardware. Tearing is not an issue for the panic
+	 * screen.
+	 */
+	struct iosys_map map;
+	/**
+	 * @width: Width of the scanout buffer, in pixels.
+	 */
+	unsigned int width;
+	/**
+	 * @height: Height of the scanout buffer, in pixels.
+	 */
+	unsigned int height;
+	/**
+	 * @pitch: Length in bytes between the start of two consecutive lines.
+	 */
+	unsigned int pitch;
+};
+
+static inline struct drm_plane *to_drm_plane(struct notifier_block *nb)
+{
+	return container_of(nb, struct drm_plane, panic_notifier);
+}
+
+struct drm_panic_line {
+	u32 len;
+	const char *txt;
+};
+
+#define PANIC_LINE(s) {.len = sizeof(s) - 1, .txt = s}
+
+static struct drm_panic_line panic_msg[] = {
+	PANIC_LINE("KERNEL PANIC !"),
+	PANIC_LINE(""),
+	PANIC_LINE("Please reboot your computer."),
+	PANIC_LINE(""),
+	PANIC_LINE(""), /* overwritten with panic reason */
+};
+
+static const struct drm_panic_line logo[] = {
+	PANIC_LINE("     .--.        _"),
+	PANIC_LINE("    |o_o |      | |"),
+	PANIC_LINE("    |:_/ |      | |"),
+	PANIC_LINE("   //   \\ \\     |_|"),
+	PANIC_LINE("  (|     | )     _"),
+	PANIC_LINE(" /'\\_   _/`\\    (_)"),
+	PANIC_LINE(" \\___)=(___/"),
+};
+
+static void draw_empty_line(struct drm_scanout_buffer *sb, size_t top, size_t height, u32 color)
+{
+	struct iosys_map dst = sb->map;
+
+	iosys_map_incr(&dst, top * sb->pitch);
+	drm_fb_fill(&dst, sb->pitch, height, sb->width, color, sb->format->cpp[0]);
+}
+
+static void draw_txt_line(const struct drm_panic_line *msg, size_t left, size_t top,
+			      struct drm_scanout_buffer *sb, u32 fg_color, u32 bg_color,
+			      const struct font_desc *font)
+{
+	size_t i;
+	const u8 *src;
+	size_t src_stride = DIV_ROUND_UP(font->width, 8);
+	struct iosys_map dst = sb->map;
+	size_t end_text;
+	unsigned int px_width = sb->format->cpp[0];
+
+	iosys_map_incr(&dst, top * sb->pitch);
+	drm_fb_fill(&dst, sb->pitch, font->height, left, bg_color, px_width);
+	iosys_map_incr(&dst, left * px_width);
+	for (i = 0; i < msg->len; i++) {
+		src = font->data + (msg->txt[i] * font->height) * src_stride;
+		drm_fb_blit_from_r1(&dst, sb->pitch, src, src_stride, font->height, font->width,
+				    fg_color, bg_color, px_width);
+		iosys_map_incr(&dst, font->width * px_width);
+	}
+	end_text = (msg->len * font->width) + left;
+	if (sb->width > end_text)
+		drm_fb_fill(&dst, sb->pitch, font->height, sb->width - end_text,
+			    bg_color, px_width);
+}
+
+
+static size_t panic_msg_needed_lines(size_t chars_per_line)
+{
+	size_t msg_len = ARRAY_SIZE(panic_msg);
+	size_t lines = 0;
+	size_t i;
+
+	for (i = 0; i < msg_len; i++)
+		lines += panic_msg[i].len ? DIV_ROUND_UP(panic_msg[i].len, chars_per_line) : 1;
+	return lines;
+}
+
+static bool can_draw_logo(size_t chars_per_line, size_t lines, size_t msg_lines)
+{
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(logo); i++) {
+		if (logo[i].len > chars_per_line)
+			return false;
+	}
+	if (lines < msg_lines + ARRAY_SIZE(logo))
+		return false;
+	return true;
+}
+
+static size_t get_start_line(size_t lines, size_t msg_lines, bool draw_logo)
+{
+	size_t remaining;
+	size_t logo_len = ARRAY_SIZE(logo);
+
+	if (lines < msg_lines)
+		return 0;
+	remaining = lines - msg_lines;
+	if (draw_logo && remaining / 2 <= logo_len)
+		return logo_len + (remaining - logo_len) / 4;
+	return remaining / 2;
+}
+
+/*
+ * Draw the panic message at the center of the screen
+ */
+static void draw_panic_static(struct drm_scanout_buffer *sb, const char *msg)
+{
+	size_t lines, msg_lines, l, msg_start_line, remaining, msgi;
+	size_t chars_per_line;
+	bool draw_logo;
+	struct drm_panic_line panic_line;
+	size_t msg_len = ARRAY_SIZE(panic_msg);
+	size_t logo_len = ARRAY_SIZE(logo);
+	u32 fg_color = CONFIG_DRM_PANIC_FOREGROUND_COLOR;
+	u32 bg_color = CONFIG_DRM_PANIC_BACKGROUND_COLOR;
+	const struct font_desc *font = get_default_font(sb->width, sb->height,
+							0x80808080, 0x80808080);
+
+	if (!font)
+		return;
+
+	/* Set the panic reason */
+	panic_msg[msg_len - 1].len = strlen(msg);
+	panic_msg[msg_len - 1].txt = msg;
+
+	lines = sb->height / font->height;
+	chars_per_line = sb->width / font->width;
+
+	msg_lines = panic_msg_needed_lines(chars_per_line);
+	draw_logo = can_draw_logo(chars_per_line, lines, msg_lines);
+	msg_start_line = get_start_line(lines, msg_lines, draw_logo);
+
+	fg_color = drm_fb_convert_from_xrgb8888(fg_color, sb->format->format);
+	bg_color = drm_fb_convert_from_xrgb8888(bg_color, sb->format->format);
+
+	msgi = 0;
+	panic_line.len = 0;
+	for (l = 0; l < lines; l++) {
+		if (draw_logo && l < logo_len)
+			draw_txt_line(&logo[l], 0, l * font->height, sb, fg_color, bg_color, font);
+		else if (l >= msg_start_line && msgi < msg_len) {
+			if (!panic_line.len) {
+				panic_line.txt = panic_msg[msgi].txt;
+				panic_line.len = panic_msg[msgi].len;
+			}
+			if (!panic_line.len) {
+				draw_empty_line(sb, l * font->height, font->height, bg_color);
+				msgi++;
+			} else if (panic_line.len > chars_per_line) {
+				remaining = panic_line.len - chars_per_line;
+				panic_line.len = chars_per_line;
+				draw_txt_line(&panic_line, 0, l * font->height, sb, fg_color,
+					      bg_color, font);
+				panic_line.txt += chars_per_line;
+				panic_line.len = remaining;
+			} else {
+				draw_txt_line(&panic_line,
+					      font->width * (chars_per_line - panic_line.len) / 2,
+					      l * font->height, sb, fg_color, bg_color, font);
+				panic_line.len = 0;
+				msgi++;
+			}
+		} else {
+			draw_empty_line(sb, l * font->height, font->height, bg_color);
+		}
+	}
+	/* Fill the bottom of the screen, if sb->height is not a multiple of font->height */
+	if (sb->height % font->height)
+		draw_empty_line(sb, l * font->height, sb->height - l * font->height, bg_color);
+}
+
+/*
+ * drm_panic_is_format_supported()
+ * @format: a fourcc color code
+ * Returns: true if supported, false otherwise.
+ *
+ * Check if drm_panic will be able to use this color format.
+ */
+static bool drm_panic_is_format_supported(u32 format)
+{
+	return drm_fb_convert_from_xrgb8888(0xffffff, format) != 0;
+}
+
+static void draw_panic_plane(struct drm_plane *plane, const char *msg)
+{
+	struct drm_scanout_buffer *sb = plane->panic_scanout;
+
+	if (!sb)
+		return;
+
+	if (!raw_spin_trylock(&sb->lock))
+		return;
+
+	if (!iosys_map_is_null(&sb->map) && drm_panic_is_format_supported(sb->format->format)) {
+		draw_panic_static(sb, msg);
+		if (plane->helper_private->panic_flush)
+			plane->helper_private->panic_flush(plane);
+	}
+	raw_spin_unlock(&sb->lock);
+}
+
+static int drm_panic(struct notifier_block *this, unsigned long event,
+		     void *ptr)
+{
+	struct drm_plane *plane = to_drm_plane(this);
+
+	draw_panic_plane(plane, ptr);
+
+	return NOTIFY_OK;
+}
+/**
+ * drm_panic_set_buffer()
+ *
+ * @sb: The scanout_buffer to set.
+ * @fb: The current drm_framebuffer struct (only format, height, width and
+ *      pitches[0]) is used.
+ * @map: The iosys_map pointing to the current scanout buffer.
+ *
+ * Set the scanout buffer, that will be used if a panic occurs.
+ * Make sure to update it before the iosysmap is no longer valid.
+ */
+void drm_panic_set_buffer(struct drm_scanout_buffer *sb,
+			  struct drm_framebuffer *fb,
+			  struct iosys_map *map)
+{
+	if (!sb)
+		return;
+
+	raw_spin_lock(&sb->lock);
+	sb->map = *map;
+	sb->format = fb->format;
+	sb->height = fb->height;
+	sb->width = fb->width;
+	sb->pitch = fb->pitches[0];
+	raw_spin_unlock(&sb->lock);
+}
+EXPORT_SYMBOL(drm_panic_set_buffer);
+
+/**
+ * drm_panic_unset_buffer()
+ *
+ * Unset the scanout buffer before it is no longer accessible.
+ * @sb: the scanout_buffer to be cleared.
+ *
+ * After calling this function, if a panic occurs, it won't be displayed on this
+ * plane.
+ */
+void drm_panic_unset_buffer(struct drm_scanout_buffer *sb)
+{
+	if (!sb)
+		return;
+
+	raw_spin_lock(&sb->lock);
+	iosys_map_clear(&sb->map);
+	sb->format = NULL;
+	raw_spin_unlock(&sb->lock);
+}
+EXPORT_SYMBOL(drm_panic_unset_buffer);
+
+/**
+ * drm_panic_register() - Initialize DRM panic for a primary plane
+ * @plane: the plane on which the panic screen will be displayed.
+ */
+void drm_panic_register(struct drm_plane *plane)
+{
+	struct drm_scanout_buffer *sb;
+
+	sb = kzalloc(sizeof(*sb), GFP_KERNEL);
+	if (!sb)
+		return;
+
+	raw_spin_lock_init(&sb->lock);
+	plane->panic_scanout = sb;
+	plane->panic_notifier.notifier_call = drm_panic;
+	plane->panic_notifier.priority = -5;
+	if (atomic_notifier_chain_register(&panic_notifier_list,
+					   &plane->panic_notifier)) {
+		drm_warn(plane->dev, "Failed to register panic handler\n");
+		plane->panic_scanout = NULL;
+		kfree(sb);
+	} else
+		drm_info(plane->dev, "Registered with drm panic\n");
+}
+EXPORT_SYMBOL(drm_panic_register);
+
+/**
+ * drm_panic_unregister()
+ * @plane: the plane previously registered.
+ */
+void drm_panic_unregister(struct drm_plane *plane)
+{
+	if (plane->panic_scanout) {
+		atomic_notifier_chain_unregister(&panic_notifier_list,
+						 &plane->panic_notifier);
+		kfree(plane->panic_scanout);
+		plane->panic_scanout = NULL;
+	}
+}
+EXPORT_SYMBOL(drm_panic_unregister);
+
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 672c655c7a8e..aee5a9bd3a98 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -31,6 +31,7 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_managed.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_vblank.h>
 
 #include "drm_crtc_internal.h"
@@ -644,6 +645,8 @@ void drm_plane_cleanup(struct drm_plane *plane)
 	kfree(plane->modifiers);
 	drm_mode_object_unregister(dev, &plane->base);
 
+	drm_panic_unregister(plane);
+
 	BUG_ON(list_empty(&plane->head));
 
 	/* Note that the plane_list is considered to be static; should we
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 881b03e4dc28..d5643ff6a54b 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -1442,6 +1442,17 @@ struct drm_plane_helper_funcs {
 	 */
 	void (*atomic_async_update)(struct drm_plane *plane,
 				    struct drm_atomic_state *state);
+	/**
+	 * @panic_flush:
+	 *
+	 * It is used by drm_panic, and is called after the panic screen is
+	 * drawn to the scanout buffer. In this function, the driver
+	 * can send additional commands to the hardware, to make the scanout
+	 * buffer visible.
+	 * It is called from panic context, so this function should follow the
+	 * panic restrictions, and not allocate or lock.
+	 */
+	void (*panic_flush)(struct drm_plane *plane);
 };
 
 /**
diff --git a/include/drm/drm_panic.h b/include/drm/drm_panic.h
new file mode 100644
index 000000000000..c9f5c5577eaf
--- /dev/null
+++ b/include/drm/drm_panic.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 or MIT */
+#ifndef __DRM_PANIC_H__
+#define __DRM_PANIC_H__
+
+/*
+ * Copyright (c) 2023 Red Hat.
+ * Author: Jocelyn Falempe <jfalempe@redhat.com>
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/iosys-map.h>
+
+struct drm_plane;
+
+#ifdef CONFIG_DRM_PANIC
+
+void drm_panic_register(struct drm_plane *plane);
+void drm_panic_unregister(struct drm_plane *plane);
+
+void drm_panic_set_buffer(struct drm_scanout_buffer *sb,
+			  struct drm_framebuffer *fb,
+			  struct iosys_map *map);
+void drm_panic_unset_buffer(struct drm_scanout_buffer *sb);
+#else
+
+static inline void drm_panic_register(struct drm_plane *plane) {}
+static inline void drm_panic_unregister(struct drm_plane *plane) {}
+
+static inline void drm_panic_set_buffer(struct drm_scanout_buffer *sb,
+				 struct drm_framebuffer *fb,
+				 struct iosys_map *map) {}
+static inline void drm_panic_unset_buffer(struct drm_scanout_buffer *sb) {}
+
+#endif
+
+#endif /* __DRM_PANIC_H__ */
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 641fe298052d..ebe1844e5f22 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -34,6 +34,7 @@
 struct drm_crtc;
 struct drm_printer;
 struct drm_modeset_acquire_ctx;
+struct drm_scanout_buffer;
 
 enum drm_scaling_filter {
 	DRM_SCALING_FILTER_DEFAULT,
@@ -779,6 +780,22 @@ struct drm_plane {
 	 * @hotspot_y_property: property to set mouse hotspot y offset.
 	 */
 	struct drm_property *hotspot_y_property;
+
+	/**
+	 * @panic_notifier: Used to register a panic notifier for this plane
+	 */
+	struct notifier_block panic_notifier;
+
+	/**
+	 * @panic_scanout:
+	 *
+	 * Optional Panic scanout data, it is allocated when calling
+	 * drm_panic_register() for this plane.
+	 * This will be used by drm panic when a panic occurs.
+	 * Don't access it directly, only use drm_panic_set_buffer() and
+	 * drm_panic_unset_buffer() if there is no scanout buffer available.
+	 */
+	struct drm_scanout_buffer *panic_scanout;
 };
 
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
-- 
2.43.0


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

* [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 1/8] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 2/8] drm/panic: Add a drm panic handler Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  2024-02-29 11:21   ` Daniel Vetter
  2024-02-29 19:08   ` kernel test robot
  2024-02-27 10:04 ` [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic Jocelyn Falempe
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, Jocelyn Falempe

Add a debugfs file, so you can test drm_panic without freezing
your machine. This is unsafe, and should be enabled only for
developer or tester.

to display the drm_panic screen, just run:
echo 1 > /sys/kernel/debug/drm_panic/trigger

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/Kconfig     |  9 +++++++
 drivers/gpu/drm/drm_panic.c | 47 +++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index c17d8a8f6877..8dcea29f595c 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR
 	depends on DRM_PANIC
 	default 0x000000
 
+config DRM_PANIC_DEBUG
+	bool "Add a debug fs entry to trigger drm_panic"
+	depends on DRM_PANIC && DEBUG_FS
+	help
+	  Add drm_panic/trigger in the kernel debugfs, to force the panic
+	  handler to write the panic message to the scanout buffer. This is
+	  unsafe and should not be enabled on a production build.
+	  If in doubt, say "N".
+
 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/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index c9f386476ef9..c5d3f725c5f5 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane)
 }
 EXPORT_SYMBOL(drm_panic_unregister);
 
+
+/*
+ * DEBUG, This is currently unsafe.
+ * Also it will call all panic_notifier, since there is no way to filter and
+ * only call the drm_panic notifier.
+ */
+#ifdef CONFIG_DRM_PANIC_DEBUG
+#include <linux/debugfs.h>
+
+static struct dentry *debug_dir;
+static struct dentry *debug_trigger;
+
+static ssize_t dbgfs_trigger_write(struct file *file, const char __user *user_buf,
+				   size_t count, loff_t *ppos)
+{
+	bool run;
+
+	if (kstrtobool_from_user(user_buf, count, &run) == 0 && run)
+		atomic_notifier_call_chain(&panic_notifier_list, 0, "Test drm panic from debugfs");
+	return count;
+}
+
+static const struct file_operations dbg_drm_panic_ops = {
+	.owner = THIS_MODULE,
+	.write = dbgfs_trigger_write,
+};
+
+static int __init debugfs_start(void)
+{
+	debug_dir = debugfs_create_dir("drm_panic", NULL);
+
+	if (IS_ERR(debug_dir))
+		return PTR_ERR(debug_dir);
+	debug_trigger = debugfs_create_file("trigger", 0200, debug_dir,
+					    NULL, &dbg_drm_panic_ops);
+	return 0;
+}
+
+static void __exit debugfs_end(void)
+{
+	debugfs_remove_recursive(debug_dir);
+}
+
+module_init(debugfs_start);
+module_exit(debugfs_end);
+
+#endif
-- 
2.43.0


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

* [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
                   ` (2 preceding siblings ...)
  2024-02-27 10:04 ` [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  2024-02-29 15:12   ` kernel test robot
  2024-03-01  2:13   ` kernel test robot
  2024-02-27 10:04 ` [PATCH v8 5/8] drm/simpledrm: Add drm_panic support Jocelyn Falempe
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, Jocelyn Falempe

This was initialy done for imx6, but should work on most drivers
using drm_fb_dma_helper.

v8:
 * Replace get_scanout_buffer() logic with drm_panic_set_buffer()
   (Thomas Zimmermann)

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/drm_fb_dma_helper.c | 37 +++++++++++++++++++++++++++++
 include/drm/drm_fb_dma_helper.h     |  4 ++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c b/drivers/gpu/drm/drm_fb_dma_helper.c
index 3b535ad1b07c..31ba71644e2b 100644
--- a/drivers/gpu/drm/drm_fb_dma_helper.c
+++ b/drivers/gpu/drm/drm_fb_dma_helper.c
@@ -15,6 +15,7 @@
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_dma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_plane.h>
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
@@ -148,3 +149,39 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm,
 	}
 }
 EXPORT_SYMBOL_GPL(drm_fb_dma_sync_non_coherent);
+
+#if defined(CONFIG_DRM_PANIC)
+/**
+ * drm_panic_gem_set_scanout_buffer - helper around drm_panic_set_buffer()
+ *
+ * @plane: primary plane registered to drm_panic
+ * @fb: framebuffer attached to the plane state
+ *
+ * Update plane->panic_scanout with the new framebuffer.
+ */
+void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane,
+				      struct drm_framebuffer *fb)
+{
+	struct drm_gem_dma_object *dma_obj;
+	struct iosys_map map;
+
+	if (!plane->panic_scanout)
+		return;
+
+	if (fb->modifier == DRM_FORMAT_MOD_LINEAR) {
+		dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+		if (dma_obj && dma_obj->vaddr) {
+			iosys_map_set_vaddr(&map, dma_obj->vaddr);
+			drm_panic_set_buffer(plane->panic_scanout, fb, &map);
+			return;
+		}
+	}
+	drm_panic_unset_buffer(plane->panic_scanout);
+}
+#else
+void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane,
+				      struct drm_framebuffer *fb)
+{
+}
+#endif
+EXPORT_SYMBOL(drm_panic_gem_set_scanout_buffer);
diff --git a/include/drm/drm_fb_dma_helper.h b/include/drm/drm_fb_dma_helper.h
index d5e036c57801..9f9ec11343cd 100644
--- a/include/drm/drm_fb_dma_helper.h
+++ b/include/drm/drm_fb_dma_helper.h
@@ -7,6 +7,7 @@
 struct drm_device;
 struct drm_framebuffer;
 struct drm_plane_state;
+struct drm_scanout_buffer;
 
 struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb,
 	unsigned int plane);
@@ -19,5 +20,8 @@ void drm_fb_dma_sync_non_coherent(struct drm_device *drm,
 				  struct drm_plane_state *old_state,
 				  struct drm_plane_state *state);
 
+void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane,
+				     struct drm_framebuffer *fb);
+
 #endif
 
-- 
2.43.0


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

* [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
                   ` (3 preceding siblings ...)
  2024-02-27 10:04 ` [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  2024-02-29 11:17   ` Daniel Vetter
  2024-02-27 10:04 ` [PATCH v8 6/8] drm/mgag200: " Jocelyn Falempe
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, Jocelyn Falempe

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

v8:
 * Replace get_scanout_buffer() with drm_panic_set_buffer()
   (Thomas Zimmermann)

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 7ce1c4617675..a2190995354a 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -25,6 +25,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_probe_helper.h>
 
 #define DRIVER_NAME	"simpledrm"
@@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
+static void simpledrm_init_panic_buffer(struct drm_plane *plane)
+{
+	struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev);
+	struct drm_framebuffer fb;
+
+	/* Fake framebuffer struct for drm_panic_set_buffer */
+	fb.width = sdev->mode.hdisplay;
+	fb.height = sdev->mode.vdisplay;
+	fb.format = sdev->format;
+	fb.pitches[0] = sdev->pitch;
+
+	drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base);
+}
+
 static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create_with_dirty,
 	.atomic_check = drm_atomic_helper_check,
@@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 		return ERR_PTR(ret);
 	drm_plane_helper_add(primary_plane, &simpledrm_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
+	simpledrm_init_panic_buffer(primary_plane);
 
 	/* CRTC */
 
-- 
2.43.0


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

* [PATCH v8 6/8] drm/mgag200: Add drm_panic support
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
                   ` (4 preceding siblings ...)
  2024-02-27 10:04 ` [PATCH v8 5/8] drm/simpledrm: Add drm_panic support Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 7/8] drm/imx: " Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 8/8] drm/ast: " Jocelyn Falempe
  7 siblings, 0 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, Jocelyn Falempe

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

v5:
 * Also check that the plane is visible and primary. (Thomas Zimmermann)

v7:
 * use drm_for_each_primary_visible_plane()

v8:
 * Replace get_scanout_buffer() logic with drm_panic_set_buffer()
   (Thomas Zimmermann)

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/mgag200/mgag200_g200.c    | 2 ++
 drivers/gpu/drm/mgag200/mgag200_g200eh.c  | 2 ++
 drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 2 ++
 drivers/gpu/drm/mgag200/mgag200_g200er.c  | 2 ++
 drivers/gpu/drm/mgag200/mgag200_g200ev.c  | 2 ++
 drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 2 ++
 drivers/gpu/drm/mgag200/mgag200_g200se.c  | 2 ++
 drivers/gpu/drm/mgag200/mgag200_g200wb.c  | 2 ++
 drivers/gpu/drm/mgag200/mgag200_mode.c    | 7 +++++++
 9 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/mgag200/mgag200_g200.c b/drivers/gpu/drm/mgag200/mgag200_g200.c
index bf5d7fe525a3..1af71785733a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200.c
@@ -7,6 +7,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -217,6 +218,7 @@ static int mgag200_g200_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh.c b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
index fad62453a91d..759cff8480f7 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200eh.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
@@ -7,6 +7,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -216,6 +217,7 @@ static int mgag200_g200eh_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200eh_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200eh_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
index 0f7d8112cd49..753b3292a384 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
@@ -6,6 +6,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -120,6 +121,7 @@ static int mgag200_g200eh3_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200eh3_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200eh3_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
index 8d4538b71047..3dd6120bf9bb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
@@ -7,6 +7,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -259,6 +260,7 @@ static int mgag200_g200er_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200er_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200er_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
index 56e6f986bff3..28476a93c9ba 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
@@ -7,6 +7,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -260,6 +261,7 @@ static int mgag200_g200ev_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200ev_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200ev_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
index 170934414d7d..f7c17bc52afc 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
@@ -6,6 +6,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -129,6 +130,7 @@ static int mgag200_g200ew3_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200ew3_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200ew3_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
index ff2b3c6622e7..3ca1f5c1ca30 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
@@ -7,6 +7,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -391,6 +392,7 @@ static int mgag200_g200se_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200se_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200se_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200wb.c b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
index 9baa727ac6f9..7d735f6b1fc7 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200wb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
@@ -7,6 +7,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 
 #include "mgag200_drv.h"
@@ -263,6 +264,7 @@ static int mgag200_g200wb_pipeline_init(struct mga_device *mdev)
 	}
 	drm_plane_helper_add(primary_plane, &mgag200_g200wb_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
 					&mgag200_g200wb_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index e17cb4c5f774..73e548143983 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -21,6 +21,7 @@
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_print.h>
 
 #include "mgag200_drv.h"
@@ -510,6 +511,12 @@ void mgag200_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	struct drm_atomic_helper_damage_iter iter;
 	struct drm_rect damage;
 
+	if (old_plane_state->fb != plane_state->fb) {
+		struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(mdev->vram);
+
+		drm_panic_set_buffer(plane->panic_scanout, plane->state->fb, &map);
+	}
+
 	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
 	drm_atomic_for_each_plane_damage(&iter, &damage) {
 		mgag200_handle_damage(mdev, shadow_plane_state->data, fb, &damage);
-- 
2.43.0


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

* [PATCH v8 7/8] drm/imx: Add drm_panic support
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
                   ` (5 preceding siblings ...)
  2024-02-27 10:04 ` [PATCH v8 6/8] drm/mgag200: " Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  2024-02-27 10:04 ` [PATCH v8 8/8] drm/ast: " Jocelyn Falempe
  7 siblings, 0 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, Jocelyn Falempe

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

v7:
 * use drm_panic_gem_get_scanout_buffer() helper

v8:
 * Replace get_scanout_buffer() logic with drm_panic_set_buffer()

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

diff --git a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
index dade8b59feae..e820858732ec 100644
--- a/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3/ipuv3-plane.c
@@ -14,6 +14,7 @@
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_dma_helper.h>
 #include <drm/drm_managed.h>
+#include <drm/drm_panic.h>
 
 #include <video/imx-ipu-v3.h>
 
@@ -765,6 +766,8 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 	ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
 	ipu_idmac_lock_enable(ipu_plane->ipu_ch, num_bursts);
 	ipu_plane_enable(ipu_plane);
+
+	drm_panic_gem_set_scanout_buffer(plane, fb);
 }
 
 static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = {
@@ -942,6 +945,8 @@ struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
 			  zpos ? "overlay" : "primary", &ret);
 		return ERR_PTR(ret);
 	}
+	if (type == DRM_PLANE_TYPE_PRIMARY)
+		drm_panic_register(&ipu_plane->base);
 
 	return ipu_plane;
 }
-- 
2.43.0


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

* [PATCH v8 8/8] drm/ast: Add drm_panic support
  2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
                   ` (6 preceding siblings ...)
  2024-02-27 10:04 ` [PATCH v8 7/8] drm/imx: " Jocelyn Falempe
@ 2024-02-27 10:04 ` Jocelyn Falempe
  7 siblings, 0 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-27 10:04 UTC (permalink / raw
  To: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf
  Cc: gpiccoli, Jocelyn Falempe

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

v7
 * Use drm_for_each_primary_visible_plane()

v8:
 * Replace get_scanout_buffer() logic with drm_panic_set_buffer()
   (Thomas Zimmermann)

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

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index a718646a66b8..3d6d4c71bc34 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -43,6 +43,7 @@
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_gem_shmem_helper.h>
 #include <drm/drm_managed.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
 
@@ -656,9 +657,13 @@ static void ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
 		struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 		struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state);
 		struct ast_vbios_mode_info *vbios_mode_info = &ast_crtc_state->vbios_mode_info;
+		struct iosys_map map;
 
 		ast_set_color_reg(ast, fb->format);
 		ast_set_vbios_color_reg(ast, fb->format, vbios_mode_info);
+
+		iosys_map_set_vaddr_iomem(&map, ast_plane->vaddr);
+		drm_panic_set_buffer(plane->panic_scanout, fb, &map);
 	}
 
 	drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
@@ -736,6 +741,7 @@ static int ast_primary_plane_init(struct ast_device *ast)
 	}
 	drm_plane_helper_add(primary_plane, &ast_primary_plane_helper_funcs);
 	drm_plane_enable_fb_damage_clips(primary_plane);
+	drm_panic_register(primary_plane);
 
 	return 0;
 }
-- 
2.43.0


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

* Re: [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
  2024-02-27 10:04 ` [PATCH v8 5/8] drm/simpledrm: Add drm_panic support Jocelyn Falempe
@ 2024-02-29 11:17   ` Daniel Vetter
  2024-02-29 14:02     ` Jocelyn Falempe
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2024-02-29 11:17 UTC (permalink / raw
  To: Jocelyn Falempe
  Cc: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf, gpiccoli

On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote:
> Add support for the drm_panic module, which displays a user-friendly
> message to the screen when a kernel panic occurs.
> 
> v8:
>  * Replace get_scanout_buffer() with drm_panic_set_buffer()
>    (Thomas Zimmermann)
> 
> 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 7ce1c4617675..a2190995354a 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -25,6 +25,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_probe_helper.h>
>  
>  #define DRIVER_NAME	"simpledrm"
> @@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = {
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
> +static void simpledrm_init_panic_buffer(struct drm_plane *plane)
> +{
> +	struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev);
> +	struct drm_framebuffer fb;
> +
> +	/* Fake framebuffer struct for drm_panic_set_buffer */
> +	fb.width = sdev->mode.hdisplay;
> +	fb.height = sdev->mode.vdisplay;
> +	fb.format = sdev->format;
> +	fb.pitches[0] = sdev->pitch;
> +
> +	drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base);
> +}
> +
>  static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = {
>  	.fb_create = drm_gem_fb_create_with_dirty,
>  	.atomic_check = drm_atomic_helper_check,
> @@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>  		return ERR_PTR(ret);
>  	drm_plane_helper_add(primary_plane, &simpledrm_primary_plane_helper_funcs);
>  	drm_plane_enable_fb_damage_clips(primary_plane);
> +	drm_panic_register(primary_plane);

Just a quick comment on this:

This does not work, the driver is not ready to handle panic calls at this
stage. Instead we need to automatically register all planes that support
panic handling in drm_dev_register(), and we need to remove them all again
in drm_dev_unregister(). Outside of these functions it is not safe to call
into driver code.

At that point it might be simpler to only register one panic notifier per
drm_device, and push the loop into the panic handler again.

Cheers, Sima

> +	simpledrm_init_panic_buffer(primary_plane);
>  
>  	/* CRTC */
>  
> -- 
> 2.43.0
> 

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

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

* Re: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
  2024-02-27 10:04 ` [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic Jocelyn Falempe
@ 2024-02-29 11:21   ` Daniel Vetter
  2024-02-29 13:56     ` Jocelyn Falempe
  2024-02-29 19:08   ` kernel test robot
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2024-02-29 11:21 UTC (permalink / raw
  To: Jocelyn Falempe
  Cc: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	daniel, javierm, bluescreen_avenger, noralf, gpiccoli

On Tue, Feb 27, 2024 at 11:04:14AM +0100, Jocelyn Falempe wrote:
> Add a debugfs file, so you can test drm_panic without freezing
> your machine. This is unsafe, and should be enabled only for
> developer or tester.
> 
> to display the drm_panic screen, just run:
> echo 1 > /sys/kernel/debug/drm_panic/trigger
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>  drivers/gpu/drm/Kconfig     |  9 +++++++
>  drivers/gpu/drm/drm_panic.c | 47 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index c17d8a8f6877..8dcea29f595c 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR
>  	depends on DRM_PANIC
>  	default 0x000000
>  
> +config DRM_PANIC_DEBUG
> +	bool "Add a debug fs entry to trigger drm_panic"
> +	depends on DRM_PANIC && DEBUG_FS
> +	help
> +	  Add drm_panic/trigger in the kernel debugfs, to force the panic
> +	  handler to write the panic message to the scanout buffer. This is
> +	  unsafe and should not be enabled on a production build.
> +	  If in doubt, say "N".
> +
>  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/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index c9f386476ef9..c5d3f725c5f5 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane)
>  }
>  EXPORT_SYMBOL(drm_panic_unregister);
>  
> +
> +/*
> + * DEBUG, This is currently unsafe.
> + * Also it will call all panic_notifier, since there is no way to filter and
> + * only call the drm_panic notifier.
> + */
> +#ifdef CONFIG_DRM_PANIC_DEBUG
> +#include <linux/debugfs.h>
> +
> +static struct dentry *debug_dir;
> +static struct dentry *debug_trigger;
> +
> +static ssize_t dbgfs_trigger_write(struct file *file, const char __user *user_buf,
> +				   size_t count, loff_t *ppos)
> +{
> +	bool run;
> +
> +	if (kstrtobool_from_user(user_buf, count, &run) == 0 && run)
> +		atomic_notifier_call_chain(&panic_notifier_list, 0, "Test drm panic from debugfs");

Since this is just the general panic notifier it feels very misplaced in
the drm subsystem. I think moving that code into the core panic code makes
a lot more sense, then we'd also have all the right people on Cc: to
figure out how we can best recreate the correct calling context (like nmi
context or whatever) for best case simulation of panic code. John Ogness
definitely needs to see this and ack, wherever we put it.
-Sima

> +	return count;
> +}
> +
> +static const struct file_operations dbg_drm_panic_ops = {
> +	.owner = THIS_MODULE,
> +	.write = dbgfs_trigger_write,
> +};
> +
> +static int __init debugfs_start(void)
> +{
> +	debug_dir = debugfs_create_dir("drm_panic", NULL);
> +
> +	if (IS_ERR(debug_dir))
> +		return PTR_ERR(debug_dir);
> +	debug_trigger = debugfs_create_file("trigger", 0200, debug_dir,
> +					    NULL, &dbg_drm_panic_ops);
> +	return 0;
> +}
> +
> +static void __exit debugfs_end(void)
> +{
> +	debugfs_remove_recursive(debug_dir);
> +}
> +
> +module_init(debugfs_start);
> +module_exit(debugfs_end);
> +
> +#endif
> -- 
> 2.43.0
> 

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

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

* Re: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
  2024-02-29 11:21   ` Daniel Vetter
@ 2024-02-29 13:56     ` Jocelyn Falempe
  0 siblings, 0 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-29 13:56 UTC (permalink / raw
  To: Daniel Vetter
  Cc: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	javierm, bluescreen_avenger, noralf, gpiccoli



On 29/02/2024 12:21, Daniel Vetter wrote:
> On Tue, Feb 27, 2024 at 11:04:14AM +0100, Jocelyn Falempe wrote:
>> Add a debugfs file, so you can test drm_panic without freezing
>> your machine. This is unsafe, and should be enabled only for
>> developer or tester.
>>
>> to display the drm_panic screen, just run:
>> echo 1 > /sys/kernel/debug/drm_panic/trigger
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> ---
>>   drivers/gpu/drm/Kconfig     |  9 +++++++
>>   drivers/gpu/drm/drm_panic.c | 47 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index c17d8a8f6877..8dcea29f595c 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -125,6 +125,15 @@ config DRM_PANIC_BACKGROUND_COLOR
>>   	depends on DRM_PANIC
>>   	default 0x000000
>>   
>> +config DRM_PANIC_DEBUG
>> +	bool "Add a debug fs entry to trigger drm_panic"
>> +	depends on DRM_PANIC && DEBUG_FS
>> +	help
>> +	  Add drm_panic/trigger in the kernel debugfs, to force the panic
>> +	  handler to write the panic message to the scanout buffer. This is
>> +	  unsafe and should not be enabled on a production build.
>> +	  If in doubt, say "N".
>> +
>>   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/drm_panic.c b/drivers/gpu/drm/drm_panic.c
>> index c9f386476ef9..c5d3f725c5f5 100644
>> --- a/drivers/gpu/drm/drm_panic.c
>> +++ b/drivers/gpu/drm/drm_panic.c
>> @@ -398,3 +398,50 @@ void drm_panic_unregister(struct drm_plane *plane)
>>   }
>>   EXPORT_SYMBOL(drm_panic_unregister);
>>   
>> +
>> +/*
>> + * DEBUG, This is currently unsafe.
>> + * Also it will call all panic_notifier, since there is no way to filter and
>> + * only call the drm_panic notifier.
>> + */
>> +#ifdef CONFIG_DRM_PANIC_DEBUG
>> +#include <linux/debugfs.h>
>> +
>> +static struct dentry *debug_dir;
>> +static struct dentry *debug_trigger;
>> +
>> +static ssize_t dbgfs_trigger_write(struct file *file, const char __user *user_buf,
>> +				   size_t count, loff_t *ppos)
>> +{
>> +	bool run;
>> +
>> +	if (kstrtobool_from_user(user_buf, count, &run) == 0 && run)
>> +		atomic_notifier_call_chain(&panic_notifier_list, 0, "Test drm panic from debugfs");
> 
> Since this is just the general panic notifier it feels very misplaced in
> the drm subsystem. I think moving that code into the core panic code makes
> a lot more sense, then we'd also have all the right people on Cc: to
> figure out how we can best recreate the correct calling context (like nmi
> context or whatever) for best case simulation of panic code. John Ogness
> definitely needs to see this and ack, wherever we put it.

I'm not sure it makes sense to test all panic notifiers at once.

So maybe I can write an atomic_notifier_call_chain_with_filter(), and 
filter on the callback address, so it will only call the drm_panic 
handlers ?

-- 

Jocelyn

> -Sima
> 
>> +	return count;
>> +}
>> +
>> +static const struct file_operations dbg_drm_panic_ops = {
>> +	.owner = THIS_MODULE,
>> +	.write = dbgfs_trigger_write,
>> +};
>> +
>> +static int __init debugfs_start(void)
>> +{
>> +	debug_dir = debugfs_create_dir("drm_panic", NULL);
>> +
>> +	if (IS_ERR(debug_dir))
>> +		return PTR_ERR(debug_dir);
>> +	debug_trigger = debugfs_create_file("trigger", 0200, debug_dir,
>> +					    NULL, &dbg_drm_panic_ops);
>> +	return 0;
>> +}
>> +
>> +static void __exit debugfs_end(void)
>> +{
>> +	debugfs_remove_recursive(debug_dir);
>> +}
>> +
>> +module_init(debugfs_start);
>> +module_exit(debugfs_end);
>> +
>> +#endif
>> -- 
>> 2.43.0
>>
> 


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

* Re: [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
  2024-02-29 11:17   ` Daniel Vetter
@ 2024-02-29 14:02     ` Jocelyn Falempe
  0 siblings, 0 replies; 16+ messages in thread
From: Jocelyn Falempe @ 2024-02-29 14:02 UTC (permalink / raw
  To: Daniel Vetter
  Cc: dri-devel, tzimmermann, airlied, maarten.lankhorst, mripard,
	javierm, bluescreen_avenger, noralf, gpiccoli



On 29/02/2024 12:17, Daniel Vetter wrote:
> On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote:
>> Add support for the drm_panic module, which displays a user-friendly
>> message to the screen when a kernel panic occurs.
>>
>> v8:
>>   * Replace get_scanout_buffer() with drm_panic_set_buffer()
>>     (Thomas Zimmermann)
>>
>> 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 7ce1c4617675..a2190995354a 100644
>> --- a/drivers/gpu/drm/tiny/simpledrm.c
>> +++ b/drivers/gpu/drm/tiny/simpledrm.c
>> @@ -25,6 +25,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_probe_helper.h>
>>   
>>   #define DRIVER_NAME	"simpledrm"
>> @@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = {
>>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>   };
>>   
>> +static void simpledrm_init_panic_buffer(struct drm_plane *plane)
>> +{
>> +	struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev);
>> +	struct drm_framebuffer fb;
>> +
>> +	/* Fake framebuffer struct for drm_panic_set_buffer */
>> +	fb.width = sdev->mode.hdisplay;
>> +	fb.height = sdev->mode.vdisplay;
>> +	fb.format = sdev->format;
>> +	fb.pitches[0] = sdev->pitch;
>> +
>> +	drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base);
>> +}
>> +
>>   static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = {
>>   	.fb_create = drm_gem_fb_create_with_dirty,
>>   	.atomic_check = drm_atomic_helper_check,
>> @@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>>   		return ERR_PTR(ret);
>>   	drm_plane_helper_add(primary_plane, &simpledrm_primary_plane_helper_funcs);
>>   	drm_plane_enable_fb_damage_clips(primary_plane);
>> +	drm_panic_register(primary_plane);
> 
> Just a quick comment on this:
> 
> This does not work, the driver is not ready to handle panic calls at this
> stage. Instead we need to automatically register all planes that support
> panic handling in drm_dev_register(), and we need to remove them all again
> in drm_dev_unregister(). Outside of these functions it is not safe to call
> into driver code.

If you register the primary plane and didn't call drm_panic_set_buffer() 
yet, the panic handler will not do anything, so it should be safe.

But if we revert to using the get_scanout_buffer(), this makes sense.
> 
> At that point it might be simpler to only register one panic notifier per
> drm_device, and push the loop into the panic handler again.
> 
> Cheers, Sima
> 
>> +	simpledrm_init_panic_buffer(primary_plane);
>>   
>>   	/* CRTC */
>>   
>> -- 
>> 2.43.0
>>
> 


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

* Re: [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
  2024-02-27 10:04 ` [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic Jocelyn Falempe
@ 2024-02-29 15:12   ` kernel test robot
  2024-03-01  2:13   ` kernel test robot
  1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2024-02-29 15:12 UTC (permalink / raw
  To: Jocelyn Falempe, dri-devel, tzimmermann, airlied,
	maarten.lankhorst, mripard, daniel, javierm, bluescreen_avenger,
	noralf
  Cc: llvm, oe-kbuild-all, gpiccoli, Jocelyn Falempe

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bfa4437fd3938ae2e186e7664b2db65bb8775670]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20240227-185901
base:   bfa4437fd3938ae2e186e7664b2db65bb8775670
patch link:    https://lore.kernel.org/r/20240227100459.194478-5-jfalempe%40redhat.com
patch subject: [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240229/202402292348.4l5K5ZmN-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240229/202402292348.4l5K5ZmN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402292348.4l5K5ZmN-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/pl111/pl111_display.c:18:
>> include/drm/drm_fb_dma_helper.h:23:46: warning: declaration of 'struct drm_plane' will not be visible outside of this function [-Wvisibility]
   void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane,
                                                ^
   1 warning generated.


vim +23 include/drm/drm_fb_dma_helper.h

    11	
    12	struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb,
    13		unsigned int plane);
    14	
    15	dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
    16					   struct drm_plane_state *state,
    17					   unsigned int plane);
    18	
    19	void drm_fb_dma_sync_non_coherent(struct drm_device *drm,
    20					  struct drm_plane_state *old_state,
    21					  struct drm_plane_state *state);
    22	
  > 23	void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane,
    24					     struct drm_framebuffer *fb);
    25	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
  2024-02-27 10:04 ` [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic Jocelyn Falempe
  2024-02-29 11:21   ` Daniel Vetter
@ 2024-02-29 19:08   ` kernel test robot
  1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2024-02-29 19:08 UTC (permalink / raw
  To: Jocelyn Falempe, dri-devel, tzimmermann, airlied,
	maarten.lankhorst, mripard, daniel, javierm, bluescreen_avenger,
	noralf
  Cc: oe-kbuild-all, gpiccoli, Jocelyn Falempe

Hi Jocelyn,

kernel test robot noticed the following build errors:

[auto build test ERROR on bfa4437fd3938ae2e186e7664b2db65bb8775670]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20240227-185901
base:   bfa4437fd3938ae2e186e7664b2db65bb8775670
patch link:    https://lore.kernel.org/r/20240227100459.194478-4-jfalempe%40redhat.com
patch subject: [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic.
config: m68k-randconfig-r052-20240229 (https://download.01.org/0day-ci/archive/20240301/202403010210.gfty1nKq-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240301/202403010210.gfty1nKq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403010210.gfty1nKq-lkp@intel.com/

All errors (new ones prefixed by >>):

   m68k-linux-ld: drivers/gpu/drm/drm_panic.o: in function `debugfs_start':
>> drm_panic.c:(.init.text+0x0): multiple definition of `init_module'; drivers/gpu/drm/drm_drv.o:drm_drv.c:(.init.text+0x0): first defined here
   m68k-linux-ld: drivers/gpu/drm/drm_panic.o: in function `debugfs_end':
>> drm_panic.c:(.exit.text+0x0): multiple definition of `cleanup_module'; drivers/gpu/drm/drm_drv.o:drm_drv.c:(.text+0x714): first defined here

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
  2024-02-27 10:04 ` [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic Jocelyn Falempe
  2024-02-29 15:12   ` kernel test robot
@ 2024-03-01  2:13   ` kernel test robot
  1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2024-03-01  2:13 UTC (permalink / raw
  To: Jocelyn Falempe, dri-devel, tzimmermann, airlied,
	maarten.lankhorst, mripard, daniel, javierm, bluescreen_avenger,
	noralf
  Cc: oe-kbuild-all, gpiccoli, Jocelyn Falempe

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bfa4437fd3938ae2e186e7664b2db65bb8775670]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20240227-185901
base:   bfa4437fd3938ae2e186e7664b2db65bb8775670
patch link:    https://lore.kernel.org/r/20240227100459.194478-5-jfalempe%40redhat.com
patch subject: [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240301/202403010934.Yop7HCSQ-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240301/202403010934.Yop7HCSQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403010934.Yop7HCSQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/pl111/pl111_display.c:18:
>> include/drm/drm_fb_dma_helper.h:23:46: warning: 'struct drm_plane' declared inside parameter list will not be visible outside of this definition or declaration
      23 | void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane,
         |                                              ^~~~~~~~~


vim +23 include/drm/drm_fb_dma_helper.h

    11	
    12	struct drm_gem_dma_object *drm_fb_dma_get_gem_obj(struct drm_framebuffer *fb,
    13		unsigned int plane);
    14	
    15	dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
    16					   struct drm_plane_state *state,
    17					   unsigned int plane);
    18	
    19	void drm_fb_dma_sync_non_coherent(struct drm_device *drm,
    20					  struct drm_plane_state *old_state,
    21					  struct drm_plane_state *state);
    22	
  > 23	void drm_panic_gem_set_scanout_buffer(struct drm_plane *plane,
    24					     struct drm_framebuffer *fb);
    25	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

end of thread, other threads:[~2024-03-01  2:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-27 10:04 [RFC][PATCH v8 0/8] drm/panic: Add a drm panic handler Jocelyn Falempe
2024-02-27 10:04 ` [PATCH v8 1/8] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill Jocelyn Falempe
2024-02-27 10:04 ` [PATCH v8 2/8] drm/panic: Add a drm panic handler Jocelyn Falempe
2024-02-27 10:04 ` [PATCH v8 3/8] drm/panic: Add debugfs entry to test without triggering panic Jocelyn Falempe
2024-02-29 11:21   ` Daniel Vetter
2024-02-29 13:56     ` Jocelyn Falempe
2024-02-29 19:08   ` kernel test robot
2024-02-27 10:04 ` [PATCH v8 4/8] drm/fb_dma: Add generic set_scanout_buffer() for drm_panic Jocelyn Falempe
2024-02-29 15:12   ` kernel test robot
2024-03-01  2:13   ` kernel test robot
2024-02-27 10:04 ` [PATCH v8 5/8] drm/simpledrm: Add drm_panic support Jocelyn Falempe
2024-02-29 11:17   ` Daniel Vetter
2024-02-29 14:02     ` Jocelyn Falempe
2024-02-27 10:04 ` [PATCH v8 6/8] drm/mgag200: " Jocelyn Falempe
2024-02-27 10:04 ` [PATCH v8 7/8] drm/imx: " Jocelyn Falempe
2024-02-27 10:04 ` [PATCH v8 8/8] drm/ast: " Jocelyn Falempe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).