Re: [PATCH 3/3] drm/panic: Add a kmsg dump screen

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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>





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux