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

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

[Tvrtko Ursulin]

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.

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();
 }
 
-- 
2.51.1

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

[Javier Martinez Canillas]

Tvrtko Ursulin <tvrtko.ursulin@igalia.com> writes:

Hello Tvrtko,

> 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.
>
> 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(-)
>

I agree that the behaviour is better after your change. The code looks
good to me.

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

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

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

[Jocelyn Falempe]

On 27/11/2025 10:03, Tvrtko Ursulin 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.

Thanks, I agree that's better than the current implementation.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.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();
>   }
>   

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

[Jani Nikula]

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

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

[Tvrtko Ursulin]

On 27/11/2025 09:52, Jocelyn Falempe wrote:
> On 27/11/2025 10:03, Tvrtko Ursulin 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.
> 
> Thanks, I agree that's better than the current implementation.
> 
> Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

Thank you both, I have pushed it to drm-misc-next.

Btw I briefly thought about how we could change kconfig away from free 
form text to a list of choices but I did not have time to think about 
kconfig backward compatibility.

Regards,

Tvrtko
>> 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();
>>   }
> 

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

[Tvrtko Ursulin]

On 27/11/2025 12:23, Jani Nikula wrote:
> 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? ;)

You are not selling it very well. :))

If we can find enough users where new infra would consolidate them it 
might be easier to sell. Anything else you know of apart from 
drm.panic_screen?

Maybe drm_client_lib.active looks as a candidate? But we would need 
something outside DRM ideally too.

Regards,

Tvrtko

> 
> [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();
>>   }
> 

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

[Jocelyn Falempe]

On 29/11/2025 11:50, Tvrtko Ursulin wrote:
> 
> On 27/11/2025 12:23, Jani Nikula wrote:
>> 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? ;)
> 
> You are not selling it very well. :))
> 
> If we can find enough users where new infra would consolidate them it 
> might be easier to sell. Anything else you know of apart from 
> drm.panic_screen?
> 
> Maybe drm_client_lib.active looks as a candidate? But we would need 
> something outside DRM ideally too.

Maybe the zswap compressor can be a good candidate:
https://elixir.bootlin.com/linux/v6.17.9/source/mm/zswap.c#L105
https://elixir.bootlin.com/linux/v6.17.9/source/mm/Kconfig#L117

I've taken this as an example to write CONFIG_DRM_PANIC_SCREEN.

Regards,

-- 

Jocelyn

> 
> Regards,
> 
> Tvrtko
> 
>>
>> [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();
>>>   }
>>
>