[PATCH 0/3] drm/panic: Fixes and graphical logo

[PATCH 0/3] drm/panic: Fixes and graphical logo

[Geert Uytterhoeven]

	Hi all,

If drm/panic is enabled, a user-friendly message is shown on screen when
a kernel panic occurs, together with an ASCII art penguin logo.
Of course we can do better ;-)
Hence this patch series extends drm/panic to draw the monochrome
graphical boot logo, when available, preceded by the customary fix.

This has been tested with rcar-du.

Thanks for your comments!

Geert Uytterhoeven (3):
  drm/panic: Fix off-by-one logo size checks
  drm/panic: Rename logo to logo_ascii
  drm/panic: Add support for drawing a monochrome graphical logo

 drivers/gpu/drm/drm_panic.c | 81 +++++++++++++++++++++++++++++++++----
 drivers/video/logo/Kconfig  |  2 +
 2 files changed, 75 insertions(+), 8 deletions(-)

-- 
2.34.1

Gr{oetje,eeting}s,

						Geert

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

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

[PATCH 1/3] drm/panic: Fix off-by-one logo size checks

[Geert Uytterhoeven]

Logos that are either just as wide or just as high as the display work
fine.

Fixes: bf9fb17c6672868d ("drm/panic: Add a drm panic handler")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/drm_panic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 7ece67086cecb79f..52d8a96b7dedff2c 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -444,7 +444,7 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
 		       bg_color, sb->format->cpp[0]);
 
 	if ((r_msg.x1 >= drm_rect_width(&r_logo) || r_msg.y1 >= drm_rect_height(&r_logo)) &&
-	    drm_rect_width(&r_logo) < sb->width && drm_rect_height(&r_logo) < sb->height) {
+	    drm_rect_width(&r_logo) <= sb->width && drm_rect_height(&r_logo) <= sb->height) {
 		draw_txt_rectangle(sb, font, logo, logo_lines, false, &r_logo, fg_color, bg_color);
 	}
 	draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, &r_msg, fg_color, bg_color);
-- 
2.34.1

[PATCH 2/3] drm/panic: Rename logo to logo_ascii

[Geert Uytterhoeven]

Rename variables related to the ASCII logo, to prepare for the advent of
support for graphical logos.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/drm_panic.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 52d8a96b7dedff2c..05b406a7034bb295 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -71,7 +71,7 @@ static struct drm_panic_line panic_msg[] = {
 	PANIC_LINE("Please reboot your computer."),
 };
 
-static const struct drm_panic_line logo[] = {
+static const struct drm_panic_line logo_ascii[] = {
 	PANIC_LINE("     .--.        _"),
 	PANIC_LINE("    |o_o |      | |"),
 	PANIC_LINE("    |:_/ |      | |"),
@@ -417,7 +417,7 @@ static void draw_txt_rectangle(struct drm_scanout_buffer *sb,
 static void draw_panic_static(struct drm_scanout_buffer *sb)
 {
 	size_t msg_lines = ARRAY_SIZE(panic_msg);
-	size_t logo_lines = ARRAY_SIZE(logo);
+	size_t logo_ascii_lines = ARRAY_SIZE(logo_ascii);
 	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, NULL, NULL);
@@ -430,8 +430,8 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
 	bg_color = convert_from_xrgb8888(bg_color, sb->format->format);
 
 	r_logo = DRM_RECT_INIT(0, 0,
-			       get_max_line_len(logo, logo_lines) * font->width,
-			       logo_lines * font->height);
+			       get_max_line_len(logo_ascii, logo_ascii_lines) * font->width,
+			       logo_ascii_lines * font->height);
 	r_msg = DRM_RECT_INIT(0, 0,
 			      min(get_max_line_len(panic_msg, msg_lines) * font->width, sb->width),
 			      min(msg_lines * font->height, sb->height));
@@ -445,7 +445,8 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
 
 	if ((r_msg.x1 >= drm_rect_width(&r_logo) || r_msg.y1 >= drm_rect_height(&r_logo)) &&
 	    drm_rect_width(&r_logo) <= sb->width && drm_rect_height(&r_logo) <= sb->height) {
-		draw_txt_rectangle(sb, font, logo, logo_lines, false, &r_logo, fg_color, bg_color);
+		draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines, false, &r_logo,
+				   fg_color, bg_color);
 	}
 	draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, &r_msg, fg_color, bg_color);
 }
-- 
2.34.1

[PATCH 3/3] drm/panic: Add support for drawing a monochrome graphical logo

[Geert Uytterhoeven]

Re-use the existing support for boot-up logos to draw a monochrome
graphical logo in the DRM panic handler.  When no suitable graphical
logo is available, the code falls back to the ASCII art penguin logo.

Note that all graphical boot-up logos are freed during late kernel
initialization, hence a copy must be made for later use.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/gpu/drm/drm_panic.c | 78 +++++++++++++++++++++++++++++++++----
 drivers/video/logo/Kconfig  |  2 +
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 05b406a7034bb295..f2d7484eff43f5a8 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -7,11 +7,15 @@
  */
 
 #include <linux/font.h>
+#include <linux/init.h>
 #include <linux/iosys-map.h>
 #include <linux/kdebug.h>
 #include <linux/kmsg_dump.h>
+#include <linux/linux_logo.h>
 #include <linux/list.h>
+#include <linux/math.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/types.h>
 
 #include <drm/drm_drv.h>
@@ -81,6 +85,42 @@ static const struct drm_panic_line logo_ascii[] = {
 	PANIC_LINE(" \\___)=(___/"),
 };
 
+#ifdef CONFIG_LOGO
+static const struct linux_logo *logo_mono;
+
+static int drm_panic_setup_logo(void)
+{
+	const struct linux_logo *logo = fb_find_logo(1);
+	const unsigned char *logo_data;
+	struct linux_logo *logo_dup;
+
+	if (!logo || logo->type != LINUX_LOGO_MONO)
+		return 0;
+
+	/* The logo is __init, so we must make a copy for later use */
+	logo_data = kmemdup(logo->data,
+			    size_mul(DIV_ROUND_UP(logo->width, BITS_PER_BYTE), logo->height),
+			    GFP_KERNEL);
+	if (!logo_data)
+		return -ENOMEM;
+
+	logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
+	if (!logo_dup) {
+		kfree(logo_data);
+		return -ENOMEM;
+	}
+
+	logo_dup->data = logo_data;
+	logo_mono = logo_dup;
+
+	return 0;
+}
+
+device_initcall(drm_panic_setup_logo);
+#else
+#define logo_mono	((const struct linux_logo *)NULL)
+#endif
+
 /*
  * Color conversion
  */
@@ -357,6 +397,20 @@ static void drm_panic_fill(struct iosys_map *dmap, unsigned int dpitch,
 	}
 }
 
+/*
+ * Draw a monochrome logo on a framebuffer.
+ */
+static void draw_logo_mono(struct drm_scanout_buffer *sb, const struct linux_logo *logo,
+			   struct drm_rect *clip, u32 fg_color, u32 bg_color)
+{
+	unsigned int px_width = sb->format->cpp[0];
+	struct iosys_map dst = sb->map[0];
+
+	iosys_map_incr(&dst, clip->y1 * sb->pitch[0] + clip->x1 * px_width);
+	drm_panic_blit(&dst, sb->pitch[0], logo->data, DIV_ROUND_UP(logo->width, 8),
+		       logo->height, logo->width, fg_color, bg_color, px_width);
+}
+
 static const u8 *get_char_bitmap(const struct font_desc *font, char c, size_t font_pitch)
 {
 	return font->data + (c * font->height) * font_pitch;
@@ -421,6 +475,7 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
 	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, NULL, NULL);
+	unsigned int logo_width, logo_height;
 	struct drm_rect r_logo, r_msg;
 
 	if (!font)
@@ -429,9 +484,15 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
 	fg_color = convert_from_xrgb8888(fg_color, sb->format->format);
 	bg_color = convert_from_xrgb8888(bg_color, sb->format->format);
 
-	r_logo = DRM_RECT_INIT(0, 0,
-			       get_max_line_len(logo_ascii, logo_ascii_lines) * font->width,
-			       logo_ascii_lines * font->height);
+	if (logo_mono) {
+		logo_width = logo_mono->width;
+		logo_height = logo_mono->height;
+	} else {
+		logo_width = get_max_line_len(logo_ascii, logo_ascii_lines) * font->width;
+		logo_height = logo_ascii_lines * font->height;
+	}
+
+	r_logo = DRM_RECT_INIT(0, 0, logo_width, logo_height);
 	r_msg = DRM_RECT_INIT(0, 0,
 			      min(get_max_line_len(panic_msg, msg_lines) * font->width, sb->width),
 			      min(msg_lines * font->height, sb->height));
@@ -443,10 +504,13 @@ static void draw_panic_static(struct drm_scanout_buffer *sb)
 	drm_panic_fill(&sb->map[0], sb->pitch[0], sb->height, sb->width,
 		       bg_color, sb->format->cpp[0]);
 
-	if ((r_msg.x1 >= drm_rect_width(&r_logo) || r_msg.y1 >= drm_rect_height(&r_logo)) &&
-	    drm_rect_width(&r_logo) <= sb->width && drm_rect_height(&r_logo) <= sb->height) {
-		draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines, false, &r_logo,
-				   fg_color, bg_color);
+	if ((r_msg.x1 >= logo_width || r_msg.y1 >= logo_height) &&
+	    logo_width <= sb->width && logo_height <= sb->height) {
+		if (logo_mono)
+			draw_logo_mono(sb, logo_mono, &r_logo, fg_color, bg_color);
+		else
+			draw_txt_rectangle(sb, font, logo_ascii, logo_ascii_lines, false, &r_logo,
+					   fg_color, bg_color);
 	}
 	draw_txt_rectangle(sb, font, panic_msg, msg_lines, true, &r_msg, fg_color, bg_color);
 }
diff --git a/drivers/video/logo/Kconfig b/drivers/video/logo/Kconfig
index b7d94d1dd1585a84..ce6bb753522d215d 100644
--- a/drivers/video/logo/Kconfig
+++ b/drivers/video/logo/Kconfig
@@ -8,6 +8,8 @@ menuconfig LOGO
 	depends on FB_CORE || SGI_NEWPORT_CONSOLE
 	help
 	  Enable and select frame buffer bootup logos.
+	  Monochrome logos will also be used by the DRM panic handler, if
+	  enabled.
 
 if LOGO
 
-- 
2.34.1

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Jocelyn Falempe]

On 12/06/2024 15:54, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> If drm/panic is enabled, a user-friendly message is shown on screen when
> a kernel panic occurs, together with an ASCII art penguin logo.
> Of course we can do better ;-)
> Hence this patch series extends drm/panic to draw the monochrome
> graphical boot logo, when available, preceded by the customary fix.

Thanks for your patch.

I've tested it, and it works great.

You need to rebase your series on top of drm-misc-next, because it 
conflicts with a series I pushed last week:
https://patchwork.freedesktop.org/series/134286/

This rebase should simplify the draw_logo_mono() function.

> 
> This has been tested with rcar-du.
> 
> Thanks for your comments!
> 
> Geert Uytterhoeven (3):
>    drm/panic: Fix off-by-one logo size checks
>    drm/panic: Rename logo to logo_ascii
>    drm/panic: Add support for drawing a monochrome graphical logo
> 
>   drivers/gpu/drm/drm_panic.c | 81 +++++++++++++++++++++++++++++++++----
>   drivers/video/logo/Kconfig  |  2 +
>   2 files changed, 75 insertions(+), 8 deletions(-)
> 

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Geert Uytterhoeven]

Hi Jocelyn,

On Thu, Jun 13, 2024 at 10:38 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 12/06/2024 15:54, Geert Uytterhoeven wrote:
> > If drm/panic is enabled, a user-friendly message is shown on screen when
> > a kernel panic occurs, together with an ASCII art penguin logo.
> > Of course we can do better ;-)
> > Hence this patch series extends drm/panic to draw the monochrome
> > graphical boot logo, when available, preceded by the customary fix.
>
> Thanks for your patch.
>
> I've tested it, and it works great.

Thank you!

> You need to rebase your series on top of drm-misc-next, because it
> conflicts with a series I pushed last week:
> https://patchwork.freedesktop.org/series/134286/

I had seen that you said you had pushed this to drm-misc-next[1]
before I posted my series, but couldn't find the actual commits in
drm-misc/for-linux-next, which is still at commit dfc1209ed5a3861c
("arm/komeda: Remove all CONFIG_DEBUG_FS conditional compilations",
so I assumed you just forgot to push?
However, the latest pull request[2] does include them, while linux-next
does not.

Has the drm-misc git repo moved?

Thanks!

[1] https://lore.kernel.org/all/3649ff15-df2b-49ba-920f-c418355d79b5@redhat.com/
[2] "[PULL] drm-misc-next"
    https://lore.kernel.org/all/20240613-cicada-of-infinite-unity-0955ca@houat/

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Jocelyn Falempe]

On 13/06/2024 11:32, Geert Uytterhoeven wrote:
> Hi Jocelyn,
> 
> On Thu, Jun 13, 2024 at 10:38 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> On 12/06/2024 15:54, Geert Uytterhoeven wrote:
>>> If drm/panic is enabled, a user-friendly message is shown on screen when
>>> a kernel panic occurs, together with an ASCII art penguin logo.
>>> Of course we can do better ;-)
>>> Hence this patch series extends drm/panic to draw the monochrome
>>> graphical boot logo, when available, preceded by the customary fix.
>>
>> Thanks for your patch.
>>
>> I've tested it, and it works great.
> 
> Thank you!
> 
>> You need to rebase your series on top of drm-misc-next, because it
>> conflicts with a series I pushed last week:
>> https://patchwork.freedesktop.org/series/134286/
> 
> I had seen that you said you had pushed this to drm-misc-next[1]
> before I posted my series, but couldn't find the actual commits in
> drm-misc/for-linux-next, which is still at commit dfc1209ed5a3861c
> ("arm/komeda: Remove all CONFIG_DEBUG_FS conditional compilations",
> so I assumed you just forgot to push?
> However, the latest pull request[2] does include them, while linux-next
> does not.
> 
> Has the drm-misc git repo moved?

It moved to gitlab recently, the new url is
git@gitlab.freedesktop.org:drm/misc/kernel.git

and the drm_panic kmsg screen commit is there:

https://gitlab.freedesktop.org/drm/misc/kernel/-/commits/drm-misc-next?ref_type=heads


> 
> Thanks!
> 
> [1] https://lore.kernel.org/all/3649ff15-df2b-49ba-920f-c418355d79b5@redhat.com/
> [2] "[PULL] drm-misc-next"
>      https://lore.kernel.org/all/20240613-cicada-of-infinite-unity-0955ca@houat/
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Geert Uytterhoeven]

Hi Jocelyn,

CC sfr

On Thu, Jun 13, 2024 at 11:41 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 13/06/2024 11:32, Geert Uytterhoeven wrote:
> > On Thu, Jun 13, 2024 at 10:38 AM Jocelyn Falempe <jfalempe@redhat.com> wrote:
> >> On 12/06/2024 15:54, Geert Uytterhoeven wrote:
> >>> If drm/panic is enabled, a user-friendly message is shown on screen when
> >>> a kernel panic occurs, together with an ASCII art penguin logo.
> >>> Of course we can do better ;-)
> >>> Hence this patch series extends drm/panic to draw the monochrome
> >>> graphical boot logo, when available, preceded by the customary fix.
> >>
> >> Thanks for your patch.
> >>
> >> I've tested it, and it works great.
> >
> > Thank you!
> >
> >> You need to rebase your series on top of drm-misc-next, because it
> >> conflicts with a series I pushed last week:
> >> https://patchwork.freedesktop.org/series/134286/
> >
> > I had seen that you said you had pushed this to drm-misc-next[1]
> > before I posted my series, but couldn't find the actual commits in
> > drm-misc/for-linux-next, which is still at commit dfc1209ed5a3861c
> > ("arm/komeda: Remove all CONFIG_DEBUG_FS conditional compilations",
> > so I assumed you just forgot to push?
> > However, the latest pull request[2] does include them, while linux-next
> > does not.
> >
> > Has the drm-misc git repo moved?
>
> It moved to gitlab recently, the new url is
> git@gitlab.freedesktop.org:drm/misc/kernel.git

Time to tell Stephen...

> and the drm_panic kmsg screen commit is there:
>
> https://gitlab.freedesktop.org/drm/misc/kernel/-/commits/drm-misc-next?ref_type=heads

Thanks!

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Stephen Rothwell]

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

Hi Geert,

On Thu, 13 Jun 2024 11:48:15 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> > > Has the drm-misc git repo moved?  
> >
> > It moved to gitlab recently, the new url is
> > git@gitlab.freedesktop.org:drm/misc/kernel.git  
> 
> Time to tell Stephen...

linux-next has been using that URL for some time.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Geert Uytterhoeven]

Hi Stephen and Maxime,

On Fri, Jun 14, 2024 at 12:00 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> On Thu, 13 Jun 2024 11:48:15 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > Has the drm-misc git repo moved?
> > >
> > > It moved to gitlab recently, the new url is
> > > git@gitlab.freedesktop.org:drm/misc/kernel.git
> >
> > Time to tell Stephen...
>
> linux-next has been using that URL for some time.

OK.

Looks like the top commit of last drm-misc PR [1] is part of the
drm-misc-next branch. but not of the for-linux-next branch, while
Stephen pulls the latter.
Is that a drm-misc or a linux-next issue?

Thanks!

[1] a13aaf157467e694a3824d81304106b58d4c20d6

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Maxime Ripard]

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

Hi,

On Fri, Jun 14, 2024 at 08:58:26AM GMT, Geert Uytterhoeven wrote:
> Hi Stephen and Maxime,
> 
> On Fri, Jun 14, 2024 at 12:00 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > On Thu, 13 Jun 2024 11:48:15 +0200 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > Has the drm-misc git repo moved?
> > > >
> > > > It moved to gitlab recently, the new url is
> > > > git@gitlab.freedesktop.org:drm/misc/kernel.git
> > >
> > > Time to tell Stephen...
> >
> > linux-next has been using that URL for some time.
> 
> OK.
> 
> Looks like the top commit of last drm-misc PR [1] is part of the
> drm-misc-next branch. but not of the for-linux-next branch, while
> Stephen pulls the latter.
> Is that a drm-misc or a linux-next issue?

This was a drm-misc issue, it should now be solved

Maxime

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

Re: [PATCH 0/3] drm/panic: Fixes and graphical logo

[Geert Uytterhoeven]

Hi Maxime,

On Fri, Jun 14, 2024 at 10:35 AM Maxime Ripard <mripard@kernel.org> wrote:
> On Fri, Jun 14, 2024 at 08:58:26AM GMT, Geert Uytterhoeven wrote:
> > Looks like the top commit of last drm-misc PR [1] is part of the
> > drm-misc-next branch. but not of the for-linux-next branch, while
> > Stephen pulls the latter.
> > Is that a drm-misc or a linux-next issue?
>
> This was a drm-misc issue, it should now be solved

Thanks, confirmed!
(and updated my .git/config for next renesas-drivers release again ;-)

Gr{oetje,eeting}s,

                        Geert

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

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