[RFC][PATCH v5 0/6] drm/panic: Add a drm panic handler

[RFC][PATCH v5 0/6] 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 with simpledrm and mgag200, 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)
 
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)
 
 
With mgag200 support, I was able to test that the xrgb8888 to rgb565 conversion is working.

IMX/IPUV3 support is not complete, I wasn't able to have etnaviv working on my board.
But it shows that it can still work on ARM with DMA buffer in this case.

Best regards,


Jocelyn Falempe (6):
  drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
  drm/panic: Add a drm panic handler
  drm/simpledrm: Add drm_panic support
  drm/mgag200: Add drm_panic support
  drm/ast: Add drm_panic support
  drm/imx: Add drm_panic support

 drivers/gpu/drm/Kconfig                  |  22 ++
 drivers/gpu/drm/Makefile                 |   1 +
 drivers/gpu/drm/ast/ast_drv.c            |   4 +-
 drivers/gpu/drm/ast/ast_drv.h            |   3 +
 drivers/gpu/drm/ast/ast_mode.c           |  26 ++
 drivers/gpu/drm/drm_drv.c                |   8 +
 drivers/gpu/drm/drm_format_helper.c      | 421 ++++++++++++++++++-----
 drivers/gpu/drm/drm_panic.c              | 368 ++++++++++++++++++++
 drivers/gpu/drm/imx/ipuv3/imx-drm-core.c |  30 ++
 drivers/gpu/drm/mgag200/mgag200_drv.c    |  25 ++
 drivers/gpu/drm/tiny/simpledrm.c         |  15 +
 include/drm/drm_drv.h                    |  21 ++
 include/drm/drm_format_helper.h          |   9 +
 include/drm/drm_panic.h                  |  96 ++++++
 14 files changed, 966 insertions(+), 83 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_panic.c
 create mode 100644 include/drm/drm_panic.h


base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
-- 
2.41.0

[PATCH v5 1/6] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill

[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.

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

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index f93a4efcee90..c29495f51684 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -20,6 +20,147 @@
 #include <drm/drm_print.h>
 #include <drm/drm_rect.h>
 
+static inline u16 drm_format_xrgb8888_to_rgb565(u32 pix)
+{
+	u16 val16;
+
+	pix = le32_to_cpu(pix);
+	val16 = ((pix & 0x00F80000) >> 8) |
+		((pix & 0x0000FC00) >> 5) |
+		((pix & 0x000000F8) >> 3);
+	return cpu_to_le16(val16);
+}
+
+static inline u16 drm_format_xrgb8888_to_rgba5551(u32 pix)
+{
+	u16 val16;
+
+	pix = le32_to_cpu(pix);
+	val16 = ((pix & 0x00f80000) >> 8) |
+		((pix & 0x0000f800) >> 5) |
+		((pix & 0x000000f8) >> 2) |
+		BIT(0); /* set alpha bit */
+	return cpu_to_le16(val16);
+}
+
+static inline u16 drm_format_xrgb8888_to_xrgb1555(u32 pix)
+{
+	u16 val16;
+
+	pix = le32_to_cpu(pix);
+	val16 = ((pix & 0x00f80000) >> 9) |
+		((pix & 0x0000f800) >> 6) |
+		((pix & 0x000000f8) >> 3);
+	return cpu_to_le16(val16);
+}
+
+static inline u16 drm_format_xrgb8888_to_argb1555(u32 pix)
+{
+	u16 val16;
+
+	pix = le32_to_cpu(pix);
+	val16 = BIT(15) | /* set alpha bit */
+		((pix & 0x00f80000) >> 9) |
+		((pix & 0x0000f800) >> 6) |
+		((pix & 0x000000f8) >> 3);
+	return cpu_to_le16(val16);
+}
+
+static inline u32 drm_format_xrgb8888_to_argb8888(u32 pix)
+{
+	u32 val32;
+
+	val32 = le32_to_cpu(pix);
+	val32 |= GENMASK(31, 24); /* fill alpha bits */
+	return cpu_to_le32(val32);
+}
+
+static inline u32 drm_format_xrgb8888_to_xbgr8888(u32 pix)
+{
+	u32 val32;
+
+	pix = le32_to_cpu(pix);
+	val32 = ((pix & 0x00ff0000) >> 16) <<  0 |
+		((pix & 0x0000ff00) >>  8) <<  8 |
+		((pix & 0x000000ff) >>  0) << 16 |
+		((pix & 0xff000000) >> 24) << 24;
+	return cpu_to_le32(val32);
+}
+
+static inline u32 drm_format_xrgb8888_to_abgr8888(u32 pix)
+{
+	u32 val32;
+
+	pix = le32_to_cpu(pix);
+	val32 = ((pix & 0x00ff0000) >> 16) <<  0 |
+		((pix & 0x0000ff00) >>  8) <<  8 |
+		((pix & 0x000000ff) >>  0) << 16 |
+		GENMASK(31, 24); /* fill alpha bits */
+	return cpu_to_le32(val32);
+}
+
+static inline u32 drm_format_xrgb8888_to_xrgb2101010(u32 pix)
+{
+	u32 val32;
+
+	pix = le32_to_cpu(pix);
+	val32 = ((pix & 0x000000FF) << 2) |
+		((pix & 0x0000FF00) << 4) |
+		((pix & 0x00FF0000) << 6);
+	pix = val32 | ((val32 >> 8) & 0x00300C03);
+	return cpu_to_le32(pix);
+}
+
+static inline u32 drm_format_xrgb8888_to_argb2101010(u32 pix)
+{
+	u32 val32;
+
+	pix = le32_to_cpu(pix);
+	val32 = ((pix & 0x000000FF) << 2) |
+		((pix & 0x0000FF00) << 4) |
+		((pix & 0x00FF0000) << 6);
+	pix = GENMASK(31, 30) | /* set alpha bits */
+	      val32 | ((val32 >> 8) & 0x00300c03);
+	return cpu_to_le32(pix);
+}
+
+/**
+ * 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.
+ */
+u32 drm_fb_convert_from_xrgb8888(u32 color, u32 format)
+{
+	switch (format) {
+	case DRM_FORMAT_RGB565:
+		return drm_format_xrgb8888_to_rgb565(color);
+	case DRM_FORMAT_RGBA5551:
+		return drm_format_xrgb8888_to_rgba5551(color);
+	case DRM_FORMAT_XRGB1555:
+		return drm_format_xrgb8888_to_xrgb1555(color);
+	case DRM_FORMAT_ARGB1555:
+		return drm_format_xrgb8888_to_argb1555(color);
+	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_XRGB8888:
+		return color;
+	case DRM_FORMAT_ARGB8888:
+		return drm_format_xrgb8888_to_argb8888(color);
+	case DRM_FORMAT_XBGR8888:
+		return drm_format_xrgb8888_to_xbgr8888(color);
+	case DRM_FORMAT_XRGB2101010:
+		return drm_format_xrgb8888_to_xrgb2101010(color);
+	case DRM_FORMAT_ARGB2101010:
+		return drm_format_xrgb8888_to_argb2101010(color);
+	default:
+		WARN_ONCE(1, "Can't convert to %p4cc\n", &format);
+		return color;
+	}
+}
+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;
@@ -272,6 +413,188 @@ 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,
+			       u16 fg16, u16 bg16)
+{
+	unsigned int l, x;
+	u16 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, 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,
+			       u32 fg32, u32 bg32)
+{
+	unsigned int l, x;
+	u32 val32;
+
+	for (l = 0; l < height; l++) {
+		for (x = 0; x < width; x++) {
+			u32 off = l * dpitch + x * 3;
+
+			val32 = (sbuf8[(l * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg32 : bg32;
+			val32 = le32_to_cpu(val32);
+
+			/* 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,
+			       u32 fg32, u32 bg32)
+{
+	unsigned int l, x;
+	u32 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, val32);
+		}
+	}
+}
+
+/**
+ * drm_fb_blit_from_r1 - convert a monochrome image to a linear framebuffer
+ * @dmap: destination iosys_map
+ * @dpich: 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, fg_color, bg_color);
+	break;
+	case 3:
+		drm_fb_r1_to_24bit(dmap, dpitch, sbuf8, spitch,
+				   height, width, fg_color, bg_color);
+	break;
+	case 4:
+		drm_fb_r1_to_32bit(dmap, dpitch, sbuf8, spitch,
+				   height, width, fg_color, 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;
+			u32 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_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
+ * @dpich: destination pitch in bytes
+ * @height: height of the rectangle, in pixels
+ * @width: width of the rectangle, in pixels
+ * @fg_color: foreground color, in destination format
+ * @bg_color: background color, in destination format
+ * @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;
@@ -325,15 +648,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]);
 	}
 }
 
@@ -401,16 +718,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]);
 }
 
 /**
@@ -452,17 +762,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]);
 }
 
 /**
@@ -504,17 +806,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]);
 }
 
 /**
@@ -606,13 +900,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]);
 }
 
 /**
@@ -654,16 +944,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,
@@ -684,16 +967,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,
@@ -714,17 +990,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]);
 }
 
 /**
@@ -766,18 +1034,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 291deb09475b..7881cd46b9cc 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -15,6 +15,7 @@ struct drm_rect;
 
 struct iosys_map;
 
+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);
 
@@ -24,6 +25,14 @@ void drm_fb_memcpy(struct iosys_map *dst, const unsigned int *dst_pitch,
 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);
+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);
-- 
2.41.0

Re: [PATCH v5 1/6] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on ffc253263a1375a65fa6c9f62a893e9767fbebfa]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20231103-225824
base:   ffc253263a1375a65fa6c9f62a893e9767fbebfa
patch link:    https://lore.kernel.org/r/20231103145526.628138-2-jfalempe%40redhat.com
patch subject: [PATCH v5 1/6] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
config: csky-randconfig-002-20231104 (https://download.01.org/0day-ci/archive/20231104/202311040208.JqcG6ZbK-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311040208.JqcG6ZbK-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/202311040208.JqcG6ZbK-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/drm_format_helper.c:491: warning: Function parameter or member 'dpitch' not described in 'drm_fb_blit_from_r1'
>> drivers/gpu/drm/drm_format_helper.c:491: warning: Excess function parameter 'dpich' description in 'drm_fb_blit_from_r1'
>> drivers/gpu/drm/drm_format_helper.c:578: warning: Function parameter or member 'dpitch' not described in 'drm_fb_fill'
>> drivers/gpu/drm/drm_format_helper.c:578: warning: Function parameter or member 'color' not described in 'drm_fb_fill'
>> drivers/gpu/drm/drm_format_helper.c:578: warning: Excess function parameter 'dpich' description in 'drm_fb_fill'
>> drivers/gpu/drm/drm_format_helper.c:578: warning: Excess function parameter 'fg_color' description in 'drm_fb_fill'
>> drivers/gpu/drm/drm_format_helper.c:578: warning: Excess function parameter 'bg_color' description in 'drm_fb_fill'


vim +491 drivers/gpu/drm/drm_format_helper.c

   470	
   471	/**
   472	 * drm_fb_blit_from_r1 - convert a monochrome image to a linear framebuffer
   473	 * @dmap: destination iosys_map
   474	 * @dpich: destination pitch in bytes
   475	 * @sbuf8: source buffer, in monochrome format, 8 pixels per byte.
   476	 * @spitch: source pitch in bytes
   477	 * @height: height of the image to copy, in pixels
   478	 * @width: width of the image to copy, in pixels
   479	 * @fg_color: foreground color, in destination format
   480	 * @bg_color: background color, in destination format
   481	 * @pixel_width: pixel width in bytes.
   482	 *
   483	 * This can be used to draw font which are monochrome images, to a framebuffer
   484	 * in other supported format.
   485	 */
   486	void drm_fb_blit_from_r1(struct iosys_map *dmap, unsigned int dpitch,
   487				 const u8 *sbuf8, unsigned int spitch,
   488				 unsigned int height, unsigned int width,
   489				 u32 fg_color, u32 bg_color,
   490				 unsigned int pixel_width)
 > 491	{
   492		switch (pixel_width) {
   493		case 2:
   494			drm_fb_r1_to_16bit(dmap, dpitch, sbuf8, spitch,
   495					   height, width, fg_color, bg_color);
   496		break;
   497		case 3:
   498			drm_fb_r1_to_24bit(dmap, dpitch, sbuf8, spitch,
   499					   height, width, fg_color, bg_color);
   500		break;
   501		case 4:
   502			drm_fb_r1_to_32bit(dmap, dpitch, sbuf8, spitch,
   503					   height, width, fg_color, bg_color);
   504		break;
   505		default:
   506			WARN_ONCE(1, "Can't blit with pixel width %d\n", pixel_width);
   507		}
   508	}
   509	EXPORT_SYMBOL(drm_fb_blit_from_r1);
   510	
   511	static void drm_fb_fill8(struct iosys_map *dmap, unsigned int dpitch,
   512				 unsigned int height, unsigned int width,
   513				 u8 color)
   514	{
   515		unsigned int l, x;
   516	
   517		for (l = 0; l < height; l++)
   518			for (x = 0; x < width; x++)
   519				iosys_map_wr(dmap, l * dpitch + x * sizeof(u8), u8, color);
   520	}
   521	
   522	static void drm_fb_fill16(struct iosys_map *dmap, unsigned int dpitch,
   523				  unsigned int height, unsigned int width,
   524				  u16 color)
   525	{
   526		unsigned int l, x;
   527	
   528		for (l = 0; l < height; l++)
   529			for (x = 0; x < width; x++)
   530				iosys_map_wr(dmap, l * dpitch + x * sizeof(u16), u16, color);
   531	}
   532	
   533	static void drm_fb_fill24(struct iosys_map *dmap, unsigned int dpitch,
   534				  unsigned int height, unsigned int width,
   535				  u32 color)
   536	{
   537		unsigned int l, x;
   538	
   539		for (l = 0; l < height; l++) {
   540			for (x = 0; x < width; x++) {
   541				unsigned int off = l * dpitch + x * 3;
   542				u32 val32 = le32_to_cpu(color);
   543	
   544				/* write blue-green-red to output in little endianness */
   545				iosys_map_wr(dmap, off, u8, (val32 & 0x000000FF) >> 0);
   546				iosys_map_wr(dmap, off + 1, u8, (val32 & 0x0000FF00) >> 8);
   547				iosys_map_wr(dmap, off + 2, u8, (val32 & 0x00FF0000) >> 16);
   548			}
   549		}
   550	}
   551	
   552	static void drm_fb_fill32(struct iosys_map *dmap, unsigned int dpitch,
   553				  unsigned int height, unsigned int width,
   554				  u32 color)
   555	{
   556		unsigned int l, x;
   557	
   558		for (l = 0; l < height; l++)
   559			for (x = 0; x < width; x++)
   560				iosys_map_wr(dmap, l * dpitch + x * sizeof(u32), u32, color);
   561	}
   562	
   563	/**
   564	 * drm_fb_fill - Fill a rectangle with a color
   565	 * @dmap: destination iosys_map, pointing to the top left corner of the rectangle
   566	 * @dpich: destination pitch in bytes
   567	 * @height: height of the rectangle, in pixels
   568	 * @width: width of the rectangle, in pixels
   569	 * @fg_color: foreground color, in destination format
   570	 * @bg_color: background color, in destination format
   571	 * @pixel_width: pixel width in bytes
   572	 *
   573	 * Fill a rectangle with a color, in a linear framebuffer.
   574	 */
   575	void drm_fb_fill(struct iosys_map *dmap, unsigned int dpitch,
   576				 unsigned int height, unsigned int width,
   577				 u32 color, unsigned int pixel_width)
 > 578	{
   579		switch (pixel_width) {
   580		case 1:
   581			drm_fb_fill8(dmap, dpitch, height, width, color);
   582		break;
   583		case 2:
   584			drm_fb_fill16(dmap, dpitch, height, width, color);
   585		break;
   586		case 3:
   587			drm_fb_fill24(dmap, dpitch, height, width, color);
   588		break;
   589		case 4:
   590			drm_fb_fill32(dmap, dpitch, height, width, color);
   591		break;
   592		default:
   593			WARN_ONCE(1, "Can't fill with pixel width %d\n", pixel_width);
   594		}
   595	}
   596	EXPORT_SYMBOL(drm_fb_fill);
   597	

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

Re: [PATCH v5 1/6] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on ffc253263a1375a65fa6c9f62a893e9767fbebfa]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20231103-225824
base:   ffc253263a1375a65fa6c9f62a893e9767fbebfa
patch link:    https://lore.kernel.org/r/20231103145526.628138-2-jfalempe%40redhat.com
patch subject: [PATCH v5 1/6] drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
config: i386-randconfig-062-20231104 (https://download.01.org/0day-ci/archive/20231104/202311041300.X5kh1Yvx-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311041300.X5kh1Yvx-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/202311041300.X5kh1Yvx-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/drm_format_helper.c:445:33: sparse: sparse: cast to restricted __le32
   drivers/gpu/drm/drm_format_helper.c:542:37: sparse: sparse: cast to restricted __le32
>> drivers/gpu/drm/drm_format_helper.c:653:65: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:653:65: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:653:65: sparse:     got restricted __le32 const [usertype]
>> drivers/gpu/drm/drm_format_helper.c:653:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] @@     got unsigned short @@
   drivers/gpu/drm/drm_format_helper.c:653:27: sparse:     expected restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:653:27: sparse:     got unsigned short
   drivers/gpu/drm/drm_format_helper.c:723:67: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:723:67: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:723:67: sparse:     got restricted __le32 const [usertype]
   drivers/gpu/drm/drm_format_helper.c:723:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] @@     got unsigned short @@
   drivers/gpu/drm/drm_format_helper.c:723:27: sparse:     expected restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:723:27: sparse:     got unsigned short
   drivers/gpu/drm/drm_format_helper.c:767:67: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:767:67: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:767:67: sparse:     got restricted __le32 const [usertype]
   drivers/gpu/drm/drm_format_helper.c:767:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] @@     got unsigned short @@
   drivers/gpu/drm/drm_format_helper.c:767:27: sparse:     expected restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:767:27: sparse:     got unsigned short
   drivers/gpu/drm/drm_format_helper.c:811:67: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:811:67: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:811:67: sparse:     got restricted __le32 const [usertype]
   drivers/gpu/drm/drm_format_helper.c:811:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le16 [usertype] @@     got unsigned short @@
   drivers/gpu/drm/drm_format_helper.c:811:27: sparse:     expected restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:811:27: sparse:     got unsigned short
   drivers/gpu/drm/drm_format_helper.c:905:67: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:905:67: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:905:67: sparse:     got restricted __le32 const [usertype]
>> drivers/gpu/drm/drm_format_helper.c:905:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 [usertype] @@     got unsigned int @@
   drivers/gpu/drm/drm_format_helper.c:905:27: sparse:     expected restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:905:27: sparse:     got unsigned int
   drivers/gpu/drm/drm_format_helper.c:949:67: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:949:67: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:949:67: sparse:     got restricted __le32 const [usertype]
   drivers/gpu/drm/drm_format_helper.c:949:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 [usertype] @@     got unsigned int @@
   drivers/gpu/drm/drm_format_helper.c:949:27: sparse:     expected restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:949:27: sparse:     got unsigned int
   drivers/gpu/drm/drm_format_helper.c:972:67: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:972:67: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:972:67: sparse:     got restricted __le32 const [usertype]
   drivers/gpu/drm/drm_format_helper.c:972:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 [usertype] @@     got unsigned int @@
   drivers/gpu/drm/drm_format_helper.c:972:27: sparse:     expected restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:972:27: sparse:     got unsigned int
   drivers/gpu/drm/drm_format_helper.c:995:70: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:995:70: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:995:70: sparse:     got restricted __le32 const [usertype]
   drivers/gpu/drm/drm_format_helper.c:995:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 [usertype] @@     got unsigned int @@
   drivers/gpu/drm/drm_format_helper.c:995:27: sparse:     expected restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:995:27: sparse:     got unsigned int
   drivers/gpu/drm/drm_format_helper.c:1039:70: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int [usertype] pix @@     got restricted __le32 const [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:1039:70: sparse:     expected unsigned int [usertype] pix
   drivers/gpu/drm/drm_format_helper.c:1039:70: sparse:     got restricted __le32 const [usertype]
   drivers/gpu/drm/drm_format_helper.c:1039:27: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 [usertype] @@     got unsigned int @@
   drivers/gpu/drm/drm_format_helper.c:1039:27: sparse:     expected restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:1039:27: sparse:     got unsigned int
   drivers/gpu/drm/drm_format_helper.c:27:15: sparse: sparse: cast to restricted __le32
>> drivers/gpu/drm/drm_format_helper.c:31:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned short @@     got restricted __le16 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:31:16: sparse:     expected unsigned short
   drivers/gpu/drm/drm_format_helper.c:31:16: sparse:     got restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:38:15: sparse: sparse: cast to restricted __le32
   drivers/gpu/drm/drm_format_helper.c:43:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned short @@     got restricted __le16 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:43:16: sparse:     expected unsigned short
   drivers/gpu/drm/drm_format_helper.c:43:16: sparse:     got restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:50:15: sparse: sparse: cast to restricted __le32
   drivers/gpu/drm/drm_format_helper.c:54:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned short @@     got restricted __le16 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:54:16: sparse:     expected unsigned short
   drivers/gpu/drm/drm_format_helper.c:54:16: sparse:     got restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:61:15: sparse: sparse: cast to restricted __le32
   drivers/gpu/drm/drm_format_helper.c:66:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned short @@     got restricted __le16 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:66:16: sparse:     expected unsigned short
   drivers/gpu/drm/drm_format_helper.c:66:16: sparse:     got restricted __le16 [usertype]
   drivers/gpu/drm/drm_format_helper.c:73:17: sparse: sparse: cast to restricted __le32
>> drivers/gpu/drm/drm_format_helper.c:75:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned int @@     got restricted __le32 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:75:16: sparse:     expected unsigned int
   drivers/gpu/drm/drm_format_helper.c:75:16: sparse:     got restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:82:15: sparse: sparse: cast to restricted __le32
   drivers/gpu/drm/drm_format_helper.c:87:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned int @@     got restricted __le32 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:87:16: sparse:     expected unsigned int
   drivers/gpu/drm/drm_format_helper.c:87:16: sparse:     got restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:106:15: sparse: sparse: cast to restricted __le32
   drivers/gpu/drm/drm_format_helper.c:111:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned int @@     got restricted __le32 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:111:16: sparse:     expected unsigned int
   drivers/gpu/drm/drm_format_helper.c:111:16: sparse:     got restricted __le32 [usertype]
   drivers/gpu/drm/drm_format_helper.c:118:15: sparse: sparse: cast to restricted __le32
   drivers/gpu/drm/drm_format_helper.c:124:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned int @@     got restricted __le32 [usertype] @@
   drivers/gpu/drm/drm_format_helper.c:124:16: sparse:     expected unsigned int
   drivers/gpu/drm/drm_format_helper.c:124:16: sparse:     got restricted __le32 [usertype]

vim +445 drivers/gpu/drm/drm_format_helper.c

   431	
   432	static void drm_fb_r1_to_24bit(struct iosys_map *dmap, unsigned int dpitch,
   433				       const u8 *sbuf8, unsigned int spitch,
   434				       unsigned int height, unsigned int width,
   435				       u32 fg32, u32 bg32)
   436	{
   437		unsigned int l, x;
   438		u32 val32;
   439	
   440		for (l = 0; l < height; l++) {
   441			for (x = 0; x < width; x++) {
   442				u32 off = l * dpitch + x * 3;
   443	
   444				val32 = (sbuf8[(l * spitch) + x / 8] & (0x80 >> (x % 8))) ? fg32 : bg32;
 > 445				val32 = le32_to_cpu(val32);
   446	
   447				/* write blue-green-red to output in little endianness */
   448				iosys_map_wr(dmap, off, u8, (val32 & 0x000000FF) >> 0);
   449				iosys_map_wr(dmap, off + 1, u8, (val32 & 0x0000FF00) >> 8);
   450				iosys_map_wr(dmap, off + 2, u8, (val32 & 0x00FF0000) >> 16);
   451			}
   452		}
   453	}
   454	

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

[PATCH v5 2/6] 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)

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().

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/Kconfig     |  22 +++
 drivers/gpu/drm/Makefile    |   1 +
 drivers/gpu/drm/drm_drv.c   |   8 +
 drivers/gpu/drm/drm_panic.c | 368 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_drv.h       |  21 ++
 include/drm/drm_panic.h     |  96 ++++++++++
 6 files changed, 516 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 3caa020391c7..9f76cc47d0f3 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -102,6 +102,28 @@ 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
+	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 215e78e79125..2c793326c893 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -73,6 +73,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 3eda026ffac6..d4230e5a472b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -43,6 +43,7 @@
 #include <drm/drm_file.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_mode_object.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_print.h>
 #include <drm/drm_privacy_screen_machine.h>
 
@@ -943,6 +944,9 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_modeset_register_all(dev);
 
+	if (driver->get_scanout_buffer)
+		drm_panic_register(dev);
+
 	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
 		 driver->name, driver->major, driver->minor,
 		 driver->patchlevel, driver->date,
@@ -986,6 +990,8 @@ void drm_dev_unregister(struct drm_device *dev)
 
 	dev->registered = false;
 
+	drm_panic_unregister(dev);
+
 	drm_client_dev_unregister(dev);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
@@ -1067,6 +1073,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 +1085,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..70afc153065b
--- /dev/null
+++ b/drivers/gpu/drm/drm_panic.c
@@ -0,0 +1,368 @@
+// 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_format_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_panic.h>
+#include <drm/drm_print.h>
+
+
+MODULE_AUTHOR("Jocelyn Falempe");
+MODULE_DESCRIPTION("DRM panic handler");
+MODULE_LICENSE("GPL");
+
+/**
+ * DOC: DRM Panic
+ *
+ * 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.
+ */
+
+/*
+ * List of active drm devices that can render a panic
+ */
+struct drm_panic_device {
+	struct list_head head;
+	struct drm_device *dev;
+};
+
+struct drm_panic_line {
+	u32 len;
+	const char *txt;
+};
+
+#define PANIC_LINE(s) {.len = sizeof(s) - 1, .txt = s}
+
+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 */
+};
+
+const struct drm_panic_line logo[] = {
+	PANIC_LINE("     .--.        _"),
+	PANIC_LINE("    |o_o |      | |"),
+	PANIC_LINE("    |:_/ |      | |"),
+	PANIC_LINE("   //   \\ \\     |_|"),
+	PANIC_LINE("  (|     | )     _"),
+	PANIC_LINE(" /'\\_   _/`\\    (_)"),
+	PANIC_LINE(" \\___)=(___/"),
+};
+
+static LIST_HEAD(drm_panic_devices);
+static DEFINE_MUTEX(drm_panic_lock);
+
+static void draw_empty_line_map(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_map(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 void draw_empty_line_px(struct drm_scanout_buffer *sb, size_t top, size_t height, u32 color)
+{
+	unsigned int x, y;
+
+	for (y = 0; y < height; y++)
+		for (x = 0; x < sb->width; x++)
+			sb->draw_pixel_xy(x, y + top, color, sb->private);
+}
+
+static void draw_txt_line_px(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)
+{
+	unsigned int x, y, i;
+	const u8 *src;
+	u32 color;
+	size_t src_stride = DIV_ROUND_UP(font->width, 8);
+	size_t end_text = msg->len * font->width + left;
+	size_t right = sb->width > end_text ? sb->width - end_text : 0;
+
+	for (y = 0; y < font->height; y++) {
+		for (x = 0; x < left; x++)
+			sb->draw_pixel_xy(x, y + top, bg_color, sb->private);
+
+		for (i = 0; i < msg->len; i++) {
+			src = font->data + (msg->txt[i] * font->height + y) * src_stride;
+			for (x = 0; x < font->width; x++) {
+				color = (src[x / 8] & (0x80 >> (x % 8))) ? fg_color : bg_color;
+				sb->draw_pixel_xy(x + left + font->width * i, y + top, color,
+						  sb->private);
+			}
+		}
+
+		for (x = 0; x < right; x++)
+			sb->draw_pixel_xy(x + end_text, y + top, bg_color, sb->private);
+	}
+}
+
+static void draw_empty_line(struct drm_scanout_buffer *sb, size_t top, size_t height, u32 color)
+{
+	if (sb->draw_pixel_xy)
+		draw_empty_line_px(sb, top, height, color);
+	else
+		draw_empty_line_map(sb, top, height, color);
+}
+
+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)
+{
+	if (sb->draw_pixel_xy)
+		draw_txt_line_px(msg, left, top, sb, fg_color, bg_color, font);
+	else
+		draw_txt_line_map(msg, left, top, sb, fg_color, bg_color, font);
+}
+
+
+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, 0x8080, 0x8080);
+
+	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);
+}
+
+static void draw_panic_device(struct drm_device *dev, const char *msg)
+{
+	struct drm_scanout_buffer sb = {0};
+
+	if (dev->driver->get_scanout_buffer(dev, &sb))
+		return;
+	draw_panic_static(&sb, msg);
+	if (sb.flush)
+		sb.flush(sb.private);
+}
+
+static int drm_panic(struct notifier_block *this, unsigned long event,
+		     void *ptr)
+{
+	struct drm_panic_device *drm_panic_device;
+
+	list_for_each_entry(drm_panic_device, &drm_panic_devices, head) {
+		draw_panic_device(drm_panic_device->dev, ptr);
+	}
+	return NOTIFY_OK;
+}
+
+static 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 drm_panic_device *new;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return;
+
+	new->dev = dev;
+	mutex_lock(&drm_panic_lock);
+	list_add_tail(&new->head, &drm_panic_devices);
+	mutex_unlock(&drm_panic_lock);
+
+	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 drm_panic_device *drm_panic_device;
+	struct drm_panic_device *found = NULL;
+
+	mutex_lock(&drm_panic_lock);
+	list_for_each_entry(drm_panic_device, &drm_panic_devices, head) {
+		if (drm_panic_device->dev == dev)
+			found = drm_panic_device;
+	}
+	if (found) {
+		list_del(&found->head);
+		kfree(found);
+		drm_info(dev, "Unregistered with drm panic\n");
+	}
+	mutex_unlock(&drm_panic_lock);
+}
+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_panic_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 9813fa759b75..30bacae32715 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;
 
 /**
@@ -392,6 +393,26 @@ 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.
+	 * The driver should do the minimum changes to provide a linear buffer, that
+	 * can be used to display the panic screen.
+	 * It is called from a panic callback, and must follow its restrictions.
+	 * (no locks, no memory allocation, no sleep, no thread/workqueue, ...)
+	 * It's a best effort mode, so it's expected that in some complex cases the
+	 * panic screen won't be displayed.
+	 * Some hardware cannot provide a linear buffer, so there is a draw_pixel_xy()
+	 * callback in the struct drm_scanout_buffer that can be used in this case.
+	 *
+	 * 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..c977ab77fba8
--- /dev/null
+++ b/include/drm/drm_panic.h
@@ -0,0 +1,96 @@
+/* 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 - DRM scanout buffer
+ *
+ * This structure holds the information necessary for drm_panic to draw the
+ * panic screen, and display it.
+ * If the driver can't provide a linear buffer, it must clear @map with
+ * iosys_map_clear() and provide a draw_pixel_xy() function.
+ */
+struct drm_scanout_buffer {
+	/**
+	 * @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;
+	/**
+	 * @private:
+	 *
+	 * In case the driver can't provide a linear buffer, this is a pointer to
+	 * some private data, that will be passed when calling @draw_pixel_xy()
+	 * and @flush()
+	 */
+	void *private;
+	/**
+	 * @draw_pixel_xy:
+	 *
+	 * In case the driver can't provide a linear buffer, this is a function
+	 * that drm_panic will call for each pixel to draw.
+	 * Color will be converted to the format specified by @format.
+	 */
+	void (*draw_pixel_xy)(unsigned int x, unsigned int y, u32 color, void *private);
+	/**
+	 * @flush:
+	 *
+	 * This function is called after the panic screen is drawn, either using
+	 * the iosys_map or the draw_pixel_xy path. In this function, the driver
+	 * can send additional commands to the hardware, to make the buffer
+	 * visible.
+	 */
+	void (*flush)(void *private);
+};
+
+#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 v5 3/6] 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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 8ea120eb8674..e5070b82703a 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>
 
@@ -842,6 +843,19 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
 	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 +871,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,
 };
 
 /*
-- 
2.41.0

[PATCH v5 4/6] drm/mgag200: Add drm_panic support

[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)

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

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index abddf37f0ea1..7bfcc66485db 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -12,10 +12,12 @@
 #include <drm/drm_aperture.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fbdev_generic.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_file.h>
 #include <drm/drm_ioctl.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_module.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_pciids.h>
 
 #include "mgag200_drv.h"
@@ -83,6 +85,28 @@ resource_size_t mgag200_probe_vram(void __iomem *mem, resource_size_t size)
 	return offset - 65536;
 }
 
+static int mgag200_get_scanout_buffer(struct drm_device *dev,
+				      struct drm_scanout_buffer *sb)
+{
+	struct drm_plane *plane;
+	struct mga_device *mdev = to_mga_device(dev);
+	struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(mdev->vram);
+
+	/* find the primary and visible plane */
+	drm_for_each_plane(plane, dev) {
+		if (!plane->state || !plane->state->visible || !plane->state->fb ||
+		    plane->type != DRM_PLANE_TYPE_PRIMARY)
+			continue;
+		sb->format = plane->state->fb->format;
+		sb->width = plane->state->fb->width;
+		sb->height = plane->state->fb->height;
+		sb->pitch = plane->state->fb->pitches[0];
+		sb->map = map;
+		return 0;
+	}
+	return -ENODEV;
+}
+
 /*
  * DRM driver
  */
@@ -98,6 +122,7 @@ static const struct drm_driver mgag200_driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
+	.get_scanout_buffer = mgag200_get_scanout_buffer,
 	DRM_GEM_SHMEM_DRIVER_OPS,
 };
 
-- 
2.41.0

[PATCH v5 5/6] drm/ast: Add drm_panic support

[Jocelyn Falempe]

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

Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/ast/ast_drv.c  |  4 ++--
 drivers/gpu/drm/ast/ast_drv.h  |  3 +++
 drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index e1224ef4ad83..da60d2e237f5 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -62,8 +62,8 @@ static const struct drm_driver ast_driver = {
 	.major = DRIVER_MAJOR,
 	.minor = DRIVER_MINOR,
 	.patchlevel = DRIVER_PATCHLEVEL,
-
-	DRM_GEM_SHMEM_DRIVER_OPS
+	.get_scanout_buffer = ast_get_scanout_buffer,
+	DRM_GEM_SHMEM_DRIVER_OPS,
 };
 
 /*
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 848a9f1403e8..b9c62d18e14e 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -504,6 +504,9 @@ int ast_mode_config_init(struct ast_device *ast);
 #define ASTDP_1366x768_60		0x1E
 #define ASTDP_1152x864_75		0x1F
 
+int ast_get_scanout_buffer(struct drm_device *dev,
+			   struct drm_scanout_buffer *sb);
+
 int ast_mm_init(struct ast_device *ast);
 
 /* ast post */
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 32f04ec6c386..3653e65a135b 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>
 
@@ -1930,3 +1931,28 @@ int ast_mode_config_init(struct ast_device *ast)
 
 	return 0;
 }
+
+int ast_get_scanout_buffer(struct drm_device *dev,
+			   struct drm_scanout_buffer *sb)
+{
+	struct drm_plane *plane;
+	struct ast_plane *ast_plane;
+	struct iosys_map map;
+
+	drm_for_each_plane(plane, dev) {
+		if (!plane->state || !plane->state->visible || !plane->state->fb ||
+		    plane->type != DRM_PLANE_TYPE_PRIMARY)
+			continue;
+		ast_plane = to_ast_plane(plane);
+		if (!ast_plane->vaddr)
+			continue;
+
+		sb->format = plane->state->fb->format;
+		sb->width = plane->state->fb->width;
+		sb->height = plane->state->fb->height;
+		sb->pitch = plane->state->fb->pitches[0];
+		iosys_map_set_vaddr_iomem(&sb->map, ast_plane->vaddr);
+		return 0;
+	}
+	return -ENODEV;
+}
-- 
2.41.0

Re: [PATCH v5 5/6] drm/ast: Add drm_panic support

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on ffc253263a1375a65fa6c9f62a893e9767fbebfa]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20231103-225824
base:   ffc253263a1375a65fa6c9f62a893e9767fbebfa
patch link:    https://lore.kernel.org/r/20231103145526.628138-6-jfalempe%40redhat.com
patch subject: [PATCH v5 5/6] drm/ast: Add drm_panic support
config: csky-randconfig-001-20231104 (https://download.01.org/0day-ci/archive/20231104/202311040239.qKjutDxj-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311040239.qKjutDxj-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/202311040239.qKjutDxj-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/ast/ast_i2c.c:27:
>> drivers/gpu/drm/ast/ast_drv.h:508:35: warning: 'struct drm_scanout_buffer' declared inside parameter list will not be visible outside of this definition or declaration
     508 |                            struct drm_scanout_buffer *sb);
         |                                   ^~~~~~~~~~~~~~~~~~
--
   drivers/gpu/drm/ast/ast_mode.c: In function 'ast_get_scanout_buffer':
>> drivers/gpu/drm/ast/ast_mode.c:1940:26: warning: unused variable 'map' [-Wunused-variable]
    1940 |         struct iosys_map map;
         |                          ^~~


vim +508 drivers/gpu/drm/ast/ast_drv.h

   506	
   507	int ast_get_scanout_buffer(struct drm_device *dev,
 > 508				   struct drm_scanout_buffer *sb);
   509	

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

Re: [PATCH v5 5/6] drm/ast: Add drm_panic support

[kernel test robot]

Hi Jocelyn,

kernel test robot noticed the following build warnings:

[auto build test WARNING on ffc253263a1375a65fa6c9f62a893e9767fbebfa]

url:    https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/drm-format-helper-Add-drm_fb_blit_from_r1-and-drm_fb_fill/20231103-225824
base:   ffc253263a1375a65fa6c9f62a893e9767fbebfa
patch link:    https://lore.kernel.org/r/20231103145526.628138-6-jfalempe%40redhat.com
patch subject: [PATCH v5 5/6] drm/ast: Add drm_panic support
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231104/202311041116.Madu45VI-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311041116.Madu45VI-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/202311041116.Madu45VI-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/ast/ast_i2c.c:27:
>> drivers/gpu/drm/ast/ast_drv.h:508:14: warning: declaration of 'struct drm_scanout_buffer' will not be visible outside of this function [-Wvisibility]
                              struct drm_scanout_buffer *sb);
                                     ^
   1 warning generated.


vim +508 drivers/gpu/drm/ast/ast_drv.h

   506	
   507	int ast_get_scanout_buffer(struct drm_device *dev,
 > 508				   struct drm_scanout_buffer *sb);
   509	

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

[PATCH v5 6/6] drm/imx: Add drm_panic support

[Jocelyn Falempe]

Proof of concept to add drm_panic support on an arm based GPU.
I've tested it with X11/llvmpipe, because I wasn't able to have
3d rendering with etnaviv on my imx6 board.

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

diff --git a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
index 4a866ac60fff..db24b4976c61 100644
--- a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
@@ -10,6 +10,7 @@
 #include <linux/dma-buf.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/iosys-map.h>
 
 #include <video/imx-ipu-v3.h>
 
@@ -17,9 +18,12 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fbdev_dma.h>
+#include <drm/drm_fb_dma_helper.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_dma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_managed.h>
+#include <drm/drm_panic.h>
 #include <drm/drm_of.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_vblank.h>
@@ -160,6 +164,31 @@ static int imx_drm_dumb_create(struct drm_file *file_priv,
 	return ret;
 }
 
+static int imx_drm_get_scanout_buffer(struct drm_device *dev,
+				      struct drm_scanout_buffer *sb)
+{
+	struct drm_plane *plane;
+	struct drm_gem_dma_object *dma_obj;
+
+	drm_for_each_plane(plane, dev) {
+		if (!plane->state || !plane->state->fb || !plane->state->visible ||
+		    plane->type != DRM_PLANE_TYPE_PRIMARY)
+			continue;
+
+		dma_obj = drm_fb_dma_get_gem_obj(plane->state->fb, 0);
+		if (!dma_obj->vaddr)
+			continue;
+
+		iosys_map_set_vaddr(&sb->map, dma_obj->vaddr);
+		sb->format = plane->state->fb->format;
+		sb->height = plane->state->fb->height;
+		sb->width = plane->state->fb->width;
+		sb->pitch = plane->state->fb->pitches[0];
+		return 0;
+	}
+	return -ENODEV;
+}
+
 static const struct drm_driver imx_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
 	DRM_GEM_DMA_DRIVER_OPS_WITH_DUMB_CREATE(imx_drm_dumb_create),
@@ -172,6 +201,7 @@ static const struct drm_driver imx_drm_driver = {
 	.major			= 1,
 	.minor			= 0,
 	.patchlevel		= 0,
+	.get_scanout_buffer	= imx_drm_get_scanout_buffer,
 };
 
 static int compare_of(struct device *dev, void *data)
-- 
2.41.0

Re: [PATCH v5 6/6] drm/imx: Add drm_panic support

[Maxime Ripard]

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

Hi,

On Fri, Nov 03, 2023 at 03:53:30PM +0100, Jocelyn Falempe wrote:
> Proof of concept to add drm_panic support on an arm based GPU.
> I've tested it with X11/llvmpipe, because I wasn't able to have
> 3d rendering with etnaviv on my imx6 board.
> 
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>

Like I said in the v6, this shouldn't be dropped because it also kind of
documents and shows what we are expecting from a "real" driver.

> ---
>  drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 30 ++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> index 4a866ac60fff..db24b4976c61 100644
> --- a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> +++ b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> @@ -10,6 +10,7 @@
>  #include <linux/dma-buf.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
> +#include <linux/iosys-map.h>
>  
>  #include <video/imx-ipu-v3.h>
>  
> @@ -17,9 +18,12 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fbdev_dma.h>
> +#include <drm/drm_fb_dma_helper.h>
> +#include <drm/drm_framebuffer.h>
>  #include <drm/drm_gem_dma_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_managed.h>
> +#include <drm/drm_panic.h>
>  #include <drm/drm_of.h>
>  #include <drm/drm_probe_helper.h>
>  #include <drm/drm_vblank.h>
> @@ -160,6 +164,31 @@ static int imx_drm_dumb_create(struct drm_file *file_priv,
>  	return ret;
>  }
>  
> +static int imx_drm_get_scanout_buffer(struct drm_device *dev,
> +				      struct drm_scanout_buffer *sb)
> +{
> +	struct drm_plane *plane;
> +	struct drm_gem_dma_object *dma_obj;
> +
> +	drm_for_each_plane(plane, dev) {
> +		if (!plane->state || !plane->state->fb || !plane->state->visible ||
> +		    plane->type != DRM_PLANE_TYPE_PRIMARY)
> +			continue;
> +
> +		dma_obj = drm_fb_dma_get_gem_obj(plane->state->fb, 0);
> +		if (!dma_obj->vaddr)
> +			continue;
> +
> +		iosys_map_set_vaddr(&sb->map, dma_obj->vaddr);
> +		sb->format = plane->state->fb->format;

Planes can be using a framebuffer in one of the numerous YUV format the
driver advertises.

> +		sb->height = plane->state->fb->height;
> +		sb->width = plane->state->fb->width;
> +		sb->pitch = plane->state->fb->pitches[0];

And your code assumes that the buffer will be large enough for an RGB
buffer, which probably isn't the case for a single-planar YUV format,
and certainly not the case for a multi-planar one.

When the driver gives back its current framebuffer, the code should check:

  * If the buffer backed by an actual buffer (and not a dma-buf handle)
  * If the buffer is mappable by the CPU
  * If the buffer is in a format that the panic code can handle
  * If the buffer uses a linear modifier

Failing that, your code cannot work at the moment. We need to be clear
about that and "gracefully" handle things instead of going forward and
writing pixels to places we might not be able to write to.

Which kind of makes me think, why do we need to involve the driver at
all there?

If in the panic code, we're going over all enabled CRTCs, finding the
primary plane currently setup for them and getting the drm_framebuffer
assigned to them, it should be enough to get us all the informations we
need, right?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 6/6] drm/imx: Add drm_panic support

[Maxime Ripard]

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

On Thu, Dec 14, 2023 at 02:48:21PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Nov 03, 2023 at 03:53:30PM +0100, Jocelyn Falempe wrote:
> > Proof of concept to add drm_panic support on an arm based GPU.
> > I've tested it with X11/llvmpipe, because I wasn't able to have
> > 3d rendering with etnaviv on my imx6 board.
> > 
> > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Like I said in the v6, this shouldn't be dropped because it also kind of
> documents and shows what we are expecting from a "real" driver.
> 
> > ---
> >  drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 30 ++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> > index 4a866ac60fff..db24b4976c61 100644
> > --- a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> > +++ b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/dma-buf.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/iosys-map.h>
> >  
> >  #include <video/imx-ipu-v3.h>
> >  
> > @@ -17,9 +18,12 @@
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_drv.h>
> >  #include <drm/drm_fbdev_dma.h>
> > +#include <drm/drm_fb_dma_helper.h>
> > +#include <drm/drm_framebuffer.h>
> >  #include <drm/drm_gem_dma_helper.h>
> >  #include <drm/drm_gem_framebuffer_helper.h>
> >  #include <drm/drm_managed.h>
> > +#include <drm/drm_panic.h>
> >  #include <drm/drm_of.h>
> >  #include <drm/drm_probe_helper.h>
> >  #include <drm/drm_vblank.h>
> > @@ -160,6 +164,31 @@ static int imx_drm_dumb_create(struct drm_file *file_priv,
> >  	return ret;
> >  }
> >  
> > +static int imx_drm_get_scanout_buffer(struct drm_device *dev,
> > +				      struct drm_scanout_buffer *sb)
> > +{
> > +	struct drm_plane *plane;
> > +	struct drm_gem_dma_object *dma_obj;
> > +
> > +	drm_for_each_plane(plane, dev) {
> > +		if (!plane->state || !plane->state->fb || !plane->state->visible ||
> > +		    plane->type != DRM_PLANE_TYPE_PRIMARY)
> > +			continue;
> > +
> > +		dma_obj = drm_fb_dma_get_gem_obj(plane->state->fb, 0);
> > +		if (!dma_obj->vaddr)
> > +			continue;
> > +
> > +		iosys_map_set_vaddr(&sb->map, dma_obj->vaddr);
> > +		sb->format = plane->state->fb->format;
> 
> Planes can be using a framebuffer in one of the numerous YUV format the
> driver advertises.
> 
> > +		sb->height = plane->state->fb->height;
> > +		sb->width = plane->state->fb->width;
> > +		sb->pitch = plane->state->fb->pitches[0];
> 
> And your code assumes that the buffer will be large enough for an RGB
> buffer, which probably isn't the case for a single-planar YUV format,
> and certainly not the case for a multi-planar one.
> 
> When the driver gives back its current framebuffer, the code should check:

Oh, and also, you need to keep an eye on the solid fill support:

https://lore.kernel.org/all/20231027-solid-fill-v7-0-780188bfa7b2@quicinc.com/

Because with that, a plane might not have a framebuffer anymore.

>   * If the buffer backed by an actual buffer (and not a dma-buf handle)
>   * If the buffer is mappable by the CPU
>   * If the buffer is in a format that the panic code can handle
>   * If the buffer uses a linear modifier
> 
> Failing that, your code cannot work at the moment. We need to be clear
> about that and "gracefully" handle things instead of going forward and
> writing pixels to places we might not be able to write to.
> 
> Which kind of makes me think, why do we need to involve the driver at
> all there?
> 
> If in the panic code, we're going over all enabled CRTCs, finding the
> primary plane currently setup for them and getting the drm_framebuffer
> assigned to them, it should be enough to get us all the informations we
> need, right?
> 
> Maxime



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 6/6] drm/imx: Add drm_panic support

[Jocelyn Falempe]

On 14/12/2023 14:48, Maxime Ripard wrote:
> Hi,
> 
> On Fri, Nov 03, 2023 at 03:53:30PM +0100, Jocelyn Falempe wrote:
>> Proof of concept to add drm_panic support on an arm based GPU.
>> I've tested it with X11/llvmpipe, because I wasn't able to have
>> 3d rendering with etnaviv on my imx6 board.
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> Like I said in the v6, this shouldn't be dropped because it also kind of
> documents and shows what we are expecting from a "real" driver.

Ok, I can bring it back in v7.

> 
>> ---
>>   drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 30 ++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
>> index 4a866ac60fff..db24b4976c61 100644
>> --- a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
>> +++ b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/dma-buf.h>
>>   #include <linux/module.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/iosys-map.h>
>>   
>>   #include <video/imx-ipu-v3.h>
>>   
>> @@ -17,9 +18,12 @@
>>   #include <drm/drm_atomic_helper.h>
>>   #include <drm/drm_drv.h>
>>   #include <drm/drm_fbdev_dma.h>
>> +#include <drm/drm_fb_dma_helper.h>
>> +#include <drm/drm_framebuffer.h>
>>   #include <drm/drm_gem_dma_helper.h>
>>   #include <drm/drm_gem_framebuffer_helper.h>
>>   #include <drm/drm_managed.h>
>> +#include <drm/drm_panic.h>
>>   #include <drm/drm_of.h>
>>   #include <drm/drm_probe_helper.h>
>>   #include <drm/drm_vblank.h>
>> @@ -160,6 +164,31 @@ static int imx_drm_dumb_create(struct drm_file *file_priv,
>>   	return ret;
>>   }
>>   
>> +static int imx_drm_get_scanout_buffer(struct drm_device *dev,
>> +				      struct drm_scanout_buffer *sb)
>> +{
>> +	struct drm_plane *plane;
>> +	struct drm_gem_dma_object *dma_obj;
>> +
>> +	drm_for_each_plane(plane, dev) {
>> +		if (!plane->state || !plane->state->fb || !plane->state->visible ||
>> +		    plane->type != DRM_PLANE_TYPE_PRIMARY)
>> +			continue;
>> +
>> +		dma_obj = drm_fb_dma_get_gem_obj(plane->state->fb, 0);
>> +		if (!dma_obj->vaddr)
>> +			continue;
>> +
>> +		iosys_map_set_vaddr(&sb->map, dma_obj->vaddr);
>> +		sb->format = plane->state->fb->format;
> 
> Planes can be using a framebuffer in one of the numerous YUV format the
> driver advertises.
> 
>> +		sb->height = plane->state->fb->height;
>> +		sb->width = plane->state->fb->width;
>> +		sb->pitch = plane->state->fb->pitches[0];
> 
> And your code assumes that the buffer will be large enough for an RGB
> buffer, which probably isn't the case for a single-planar YUV format,
> and certainly not the case for a multi-planar one.

Yes, this code is a bit hacky, and on my test setup the framebuffer was 
in RGB, so I didn't handle other formats.
Also it should be possible to add YUV format later, but I would like to 
have a minimal drm_panic merged, before adding more features.
> 
> When the driver gives back its current framebuffer, the code should check:
> 
>    * If the buffer backed by an actual buffer (and not a dma-buf handle)

Regarding the struct drm_framebuffer, I'm not sure how do you 
differentiate an actual buffer from a dma-buf handle ?

>    * If the buffer is mappable by the CPU

If "dma_obj->vaddr" is not null, then it's already mapped by the CPU right ?
>    * If the buffer is in a format that the panic code can handle
>    * If the buffer uses a linear modifier

Yes, that must be checked too.

> 
> Failing that, your code cannot work at the moment. We need to be clear
> about that and "gracefully" handle things instead of going forward and
> writing pixels to places we might not be able to write to.
> 
> Which kind of makes me think, why do we need to involve the driver at
> all there?
> 
> If in the panic code, we're going over all enabled CRTCs, finding the
> primary plane currently setup for them and getting the drm_framebuffer
> assigned to them, it should be enough to get us all the informations we
> need, right?

Yes, I think I can do a generic implementation for the drivers using the 
drm_gem_dma helper like imx6.
But for simpledrm, ast, or mgag200, I need to retrieve the VRAM address, 
because it's not in the drm_framebuffer struct, so they won't be able to 
use this generic implementation.

> 
> Maxime

Thanks for the review,

-- 

Jocelyn

Re: [PATCH v5 6/6] drm/imx: Add drm_panic support

[Maxime Ripard]

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

On Thu, Dec 14, 2023 at 04:03:04PM +0100, Jocelyn Falempe wrote:
> > > +static int imx_drm_get_scanout_buffer(struct drm_device *dev,
> > > +				      struct drm_scanout_buffer *sb)
> > > +{
> > > +	struct drm_plane *plane;
> > > +	struct drm_gem_dma_object *dma_obj;
> > > +
> > > +	drm_for_each_plane(plane, dev) {
> > > +		if (!plane->state || !plane->state->fb || !plane->state->visible ||
> > > +		    plane->type != DRM_PLANE_TYPE_PRIMARY)
> > > +			continue;
> > > +
> > > +		dma_obj = drm_fb_dma_get_gem_obj(plane->state->fb, 0);
> > > +		if (!dma_obj->vaddr)
> > > +			continue;
> > > +
> > > +		iosys_map_set_vaddr(&sb->map, dma_obj->vaddr);
> > > +		sb->format = plane->state->fb->format;
> > 
> > Planes can be using a framebuffer in one of the numerous YUV format the
> > driver advertises.
> > 
> > > +		sb->height = plane->state->fb->height;
> > > +		sb->width = plane->state->fb->width;
> > > +		sb->pitch = plane->state->fb->pitches[0];
> > 
> > And your code assumes that the buffer will be large enough for an RGB
> > buffer, which probably isn't the case for a single-planar YUV format,
> > and certainly not the case for a multi-planar one.
> 
> Yes, this code is a bit hacky, and on my test setup the framebuffer was in
> RGB, so I didn't handle other formats.
> Also it should be possible to add YUV format later, but I would like to have
> a minimal drm_panic merged, before adding more features.

Sure. Having a minimal panic code is reasonable, but we should properly
handle not supporting them still :)

There's cases where, with the current architecture anyway, you just
won't be able to print a panic message and that's fine. The important
part is not crashing the kernel further and being as loud as we can that
we couldn't print a panic message on the screen because of the setup.

> > When the driver gives back its current framebuffer, the code should check:
> > 
> >    * If the buffer backed by an actual buffer (and not a dma-buf handle)
> 
> Regarding the struct drm_framebuffer, I'm not sure how do you differentiate
> an actual buffer from a dma-buf handle ?

Its backing drm_gem_object should be set and have the field import_attach set

> >    * If the buffer is mappable by the CPU
> 
> If "dma_obj->vaddr" is not null, then it's already mapped by the CPU right ?

I'm not sure. drm_gem_dma_create will only ever create CPU-mappable
buffers, but drm_gem_dma_prime_import_sg_table won't for example.

> >    * If the buffer is in a format that the panic code can handle
> >    * If the buffer uses a linear modifier
> 
> Yes, that must be checked too.
> 
> > 
> > Failing that, your code cannot work at the moment. We need to be clear
> > about that and "gracefully" handle things instead of going forward and
> > writing pixels to places we might not be able to write to.
> > 
> > Which kind of makes me think, why do we need to involve the driver at
> > all there?
> > 
> > If in the panic code, we're going over all enabled CRTCs, finding the
> > primary plane currently setup for them and getting the drm_framebuffer
> > assigned to them, it should be enough to get us all the informations we
> > need, right?
> 
> Yes, I think I can do a generic implementation for the drivers using the
> drm_gem_dma helper like imx6.
> But for simpledrm, ast, or mgag200, I need to retrieve the VRAM address,
> because it's not in the drm_framebuffer struct, so they won't be able to use
> this generic implementation.

Sure :)

I guess we could have a CRTC function then that by default will just
return the current primary plane framebuffer (or could be a plane
function?), and if it's not there just grabs the one from the current
active state.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC][PATCH v5 0/6] drm/panic: Add a drm panic handler

[nerdopolis]

On Friday, November 3, 2023 10:53:24 AM EST 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 with simpledrm and mgag200, 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)
>  
> 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)
>  
>  
> With mgag200 support, I was able to test that the xrgb8888 to rgb565 conversion is working.
> 
I am unable to test with that hardware, but I am able to test with simpledrm, and it works pretty good
> IMX/IPUV3 support is not complete, I wasn't able to have etnaviv working on my board.
> But it shows that it can still work on ARM with DMA buffer in this case.
> 
> Best regards,
> 
> 
> Jocelyn Falempe (6):
>   drm/format-helper: Add drm_fb_blit_from_r1 and drm_fb_fill
>   drm/panic: Add a drm panic handler
>   drm/simpledrm: Add drm_panic support
>   drm/mgag200: Add drm_panic support
>   drm/ast: Add drm_panic support
>   drm/imx: Add drm_panic support
> 
>  drivers/gpu/drm/Kconfig                  |  22 ++
>  drivers/gpu/drm/Makefile                 |   1 +
>  drivers/gpu/drm/ast/ast_drv.c            |   4 +-
>  drivers/gpu/drm/ast/ast_drv.h            |   3 +
>  drivers/gpu/drm/ast/ast_mode.c           |  26 ++
>  drivers/gpu/drm/drm_drv.c                |   8 +
>  drivers/gpu/drm/drm_format_helper.c      | 421 ++++++++++++++++++-----
>  drivers/gpu/drm/drm_panic.c              | 368 ++++++++++++++++++++
>  drivers/gpu/drm/imx/ipuv3/imx-drm-core.c |  30 ++
>  drivers/gpu/drm/mgag200/mgag200_drv.c    |  25 ++
>  drivers/gpu/drm/tiny/simpledrm.c         |  15 +
>  include/drm/drm_drv.h                    |  21 ++
>  include/drm/drm_format_helper.h          |   9 +
>  include/drm/drm_panic.h                  |  96 ++++++
>  14 files changed, 966 insertions(+), 83 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_panic.c
>  create mode 100644 include/drm/drm_panic.h
> 
> 
> base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
>