On Thu, 27 Nov 2025, Tvrtko Ursulin <[email protected]> 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/[email protected] [2] https://lore.kernel.org/r/[email protected] > > Signed-off-by: Tvrtko Ursulin <[email protected]> > Cc: Jocelyn Falempe <[email protected]> > Cc: Javier Martinez Canillas <[email protected]> > --- > 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
