Jocelyn Falempe <[email protected]> writes:

> Add a kmsg dump option, which will display the last lines of kmsg,
> and should be similar to fbcon.
> Add a Kconfig choice for the panic screen, so that the user can
> choose between this new kmsg dump, or the userfriendly option.
>
> Signed-off-by: Jocelyn Falempe <[email protected]>
> ---
>  drivers/gpu/drm/Kconfig     |  21 +++++
>  drivers/gpu/drm/drm_panic.c | 151 +++++++++++++++++++++++++++---------
>  2 files changed, 136 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 9703429de6b9..78d401b55102 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -137,6 +137,27 @@ config DRM_PANIC_DEBUG
>         This is unsafe and should not be enabled on a production build.
>         If in doubt, say "N".
>  
> +choice
> +     prompt "Panic screen formater"
> +     default DRM_PANIC_SCREEN_USERFRIENDLY
> +     depends on DRM_PANIC
> +     help
> +       This option enable to choose what will be displayed when a kernel
> +       panic occurs.
> +
> +     config DRM_PANIC_SCREEN_USERFRIENDLY
> +             bool "Default user friendly message"
> +             help
> +               Only a short message telling the user to reboot the system.
> +
> +     config DRM_PANIC_SCREEN_KMSG
> +             bool "Display the last lines of kmsg"
> +             help
> +               Display kmsg last lines on panic.
> +               Enable if you are a kernel developer, and want to debug a
> +               kernel panic.
> +endchoice

Why having it as a compile time option and not a runtime option ? AFAICT
this could be a drm.panic_format= kernel command line parameter instead.

[...]
  
> -/*
> - * Draw the panic message at the center of the screen
> - */
> +#if defined(CONFIG_DRM_PANIC_SCREEN_USERFRIENDLY)
> +

And that could avoid ifdefery in the C file.

[...]

> +#elif defined(CONFIG_DRM_PANIC_SCREEN_KMSG)
> +
> +#include <linux/kmsg_dump.h>
> +#include <linux/printk.h>
> +

I would add these even if guarded by DRM_PANIC_SCREEN_KMSG, to the
start of the C file where are the other headers include directives.

The patch looks good to me though, so if you prefer to keep it as a
build time option:

Reviewed-by: Javier Martinez Canillas <[email protected]>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Reply via email to