On 31/05/2024 11:32, Javier Martinez Canillas wrote:
Jocelyn Falempe <jfalempe@xxxxxxxxxx> 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 <jfalempe@xxxxxxxxxx>
---
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.
Yes, I didn't think about it. That would allow to get more debug
information from a user without rebuilding the kernel.
I'll prepare a v2 with that.
I prefer to use a drm.panic_screen=, as "format" might be confusing with
the color format ?
[...]
-/*
- * 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 <javierm@xxxxxxxxxx>
