Jocelyn Falempe <jfale...@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 <jfale...@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 <javi...@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Reply via email to