Re: [PATCH] drm/panic: Report invalid or unsupported panic modes

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 27 Nov 2025, Tvrtko Ursulin <tvrtko.ursulin@igalia.com> wrote:
> Currently the user can write anything into the drm.panic_screen modparam,
> either at runtime via sysfs, or as a kernel boot time argument. Invalid
> strings will be silently accepted and ignored at use time by defaulting to
> the 'user' panic mode.
>
> Let instead add some validation in order to have immediate feedback when
> something has been mistyped, or not compiled in.
>
> For example during kernel boot:
>
>  Booting kernel: `bsod' invalid for parameter `drm.panic_screen'
>
> Or at runtime:
>
>  # echo -n bsod > /sys/module/drm/parameters/panic_screen
>  -bash: echo: write error: Invalid argument
>
> Change of behavior is that when invalid mode is attempted to be
> configured, currently the code will default to the 'user' mode, while with
> this change the code will ignore it, and default to the mode set at kernel
> build time via CONFIG_DRM_PANIC_SCREEN.
>
> While at it lets also fix the module parameter description to include all
> compiled in modes.

I've tried to add a convenient way to use enum module parameters on two
occasions [1][2] but it went nowhere. Maybe I should've pushed harder.

In a perfect world we'd use device specific parameters, here too, but in
the imperfect world we still use module parameters. And use cases like
this would be a soooo nice with that.

Want to take over and fight the fight? ;)


BR,
Jani.


[1] https://lore.kernel.org/r/20190611141701.7432-1-jani.nikula@intel.com
[2] https://lore.kernel.org/r/20220414123033.654198-1-jani.nikula@intel.com


>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Jocelyn Falempe <jfalempe@redhat.com>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> ---
>  drivers/gpu/drm/drm_panic.c | 77 ++++++++++++++++++++++++++++++-------
>  1 file changed, 63 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index d4b6ea42db0f..f42be7f1d8c2 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -39,12 +39,6 @@ MODULE_AUTHOR("Jocelyn Falempe");
>  MODULE_DESCRIPTION("DRM panic handler");
>  MODULE_LICENSE("GPL");
>  
> -static char drm_panic_screen[16] = CONFIG_DRM_PANIC_SCREEN;
> -module_param_string(panic_screen, drm_panic_screen, sizeof(drm_panic_screen), 0644);
> -MODULE_PARM_DESC(panic_screen,
> -		 "Choose what will be displayed by drm_panic, 'user' or 'kmsg' [default="
> -		 CONFIG_DRM_PANIC_SCREEN "]");
> -
>  /**
>   * DOC: overview
>   *
> @@ -813,15 +807,60 @@ static void draw_panic_static_qr_code(struct drm_scanout_buffer *sb)
>  		draw_panic_static_user(sb);
>  }
>  #else
> -static void draw_panic_static_qr_code(struct drm_scanout_buffer *sb)
> -{
> -	draw_panic_static_user(sb);
> -}
> -
>  static void drm_panic_qr_init(void) {};
>  static void drm_panic_qr_exit(void) {};
>  #endif
>  
> +enum drm_panic_type {
> +	DRM_PANIC_TYPE_KMSG,
> +	DRM_PANIC_TYPE_USER,
> +	DRM_PANIC_TYPE_QR,
> +};
> +
> +static enum drm_panic_type drm_panic_type = -1;
> +
> +static const char *drm_panic_type_map[] = {
> +	[DRM_PANIC_TYPE_KMSG] = "kmsg",
> +	[DRM_PANIC_TYPE_USER] = "user",
> +#if IS_ENABLED(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
> +	[DRM_PANIC_TYPE_QR] = "qr",
> +#endif
> +};
> +
> +static int drm_panic_type_set(const char *val, const struct kernel_param *kp)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(drm_panic_type_map); i++) {
> +		if (!strcmp(val, drm_panic_type_map[i])) {
> +			drm_panic_type = i;
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int drm_panic_type_get(char *buffer, const struct kernel_param *kp)
> +{
> +	return scnprintf(buffer, PAGE_SIZE, "%s\n",
> +			 drm_panic_type_map[drm_panic_type]);
> +}
> +
> +static const struct kernel_param_ops drm_panic_ops = {
> +	.set = drm_panic_type_set,
> +	.get = drm_panic_type_get,
> +};
> +
> +module_param_cb(panic_screen, &drm_panic_ops, NULL, 0644);
> +MODULE_PARM_DESC(panic_screen,
> +#if IS_ENABLED(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
> +		 "Choose what will be displayed by drm_panic, 'user', 'kmsg' or 'qr' [default="
> +#else
> +		 "Choose what will be displayed by drm_panic, 'user' or 'kmsg' [default="
> +#endif
> +		 CONFIG_DRM_PANIC_SCREEN "]");
> +
>  /*
>   * drm_panic_is_format_supported()
>   * @format: a fourcc color code
> @@ -838,11 +877,19 @@ static bool drm_panic_is_format_supported(const struct drm_format_info *format)
>  
>  static void draw_panic_dispatch(struct drm_scanout_buffer *sb)
>  {
> -	if (!strcmp(drm_panic_screen, "kmsg")) {
> +	switch (drm_panic_type) {
> +	case DRM_PANIC_TYPE_KMSG:
>  		draw_panic_static_kmsg(sb);
> -	} else if (!strcmp(drm_panic_screen, "qr_code")) {
> +		break;
> +
> +#if IS_ENABLED(CONFIG_DRM_PANIC_SCREEN_QR_CODE)
> +	case DRM_PANIC_TYPE_QR:
>  		draw_panic_static_qr_code(sb);
> -	} else {
> +		break;
> +#endif
> +
> +	case DRM_PANIC_TYPE_USER:
> +	default:
>  		draw_panic_static_user(sb);
>  	}
>  }
> @@ -1025,6 +1072,8 @@ void drm_panic_unregister(struct drm_device *dev)
>   */
>  void __init drm_panic_init(void)
>  {
> +	if (drm_panic_type == -1)
> +		drm_panic_type_set(CONFIG_DRM_PANIC_SCREEN, NULL);
>  	drm_panic_qr_init();
>  }

-- 
Jani Nikula, Intel