[PATCH v2 3/3] drm/panic: Add a kmsg panic screen

[Javier Martinez Canillas]

Jocelyn Falempe <jfalempe at redhat.com> writes:

> Add a kmsg option, which will display the last lines of kmsg,
> and should be similar to fbcon.
> Add a drm.panic_screen module parameter, so you can choose between
> the different panic screens available.
> two options currently, but more will be added later:
>  * "user": a short message telling the user to reboot the machine.
>  * "kmsg": fill the screen with the last lines of kmsg.
>
> You can even change it at runtime by writing to
> /sys/module/drm/parameters/panic_screen
>

Great!

> v2:
>  * use module parameter instead of Kconfig choice
>    (Javier Martinez Canillas)
>
> Signed-off-by: Jocelyn Falempe <jfalempe at redhat.com>
> ---
>  drivers/gpu/drm/Kconfig     |  11 ++++
>  drivers/gpu/drm/drm_panic.c | 108 ++++++++++++++++++++++++++++++++----
>  2 files changed, 109 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 9703429de6b9..944815cee080 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -137,6 +137,17 @@ config DRM_PANIC_DEBUG
>  	  This is unsafe and should not be enabled on a production build.
>  	  If in doubt, say "N".
>  
> +config DRM_PANIC_SCREEN
> +	string "Panic screen formater"
> +	default "user"
> +	depends on DRM_PANIC
> +	help
> +	  This option enable to choose what will be displayed when a kernel
> +	  panic occurs. You can choose between "user", a short message telling
> +	  the user to reboot the system, or "kmsg" which will display the last
> +	  lines of kmsg.

Maybe I would mention here that this is only about the default, but that
can be changed using the "drm.panic_screen=" kernel cmdline parameter or
writting to the /sys/module/drm/parameters/panic_screen sysfs entry.

[...]

> +static void draw_panic_static_kmsg(struct drm_scanout_buffer *sb)
> +{
> +	u32 fg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_FOREGROUND_COLOR, sb->format->format);
> +	u32 bg_color = convert_from_xrgb8888(CONFIG_DRM_PANIC_BACKGROUND_COLOR, sb->format->format);
> +	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);

Dan reported that get_default_font() can return NULL....

> +	struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
> +	struct kmsg_dump_iter iter;
> +	char kmsg_buf[512];
> +	size_t kmsg_len;
> +	struct drm_panic_line line;
> +	int yoffset = sb->height - font->height - (sb->height % font->height) / 2;
> +
> +	if (!font)
> +		return;
> +

... so you have to calculate yoffset after checking if the font is not NULL.

with that fixed:

Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat