Re: [PATCH v4 2/4] drm/panic: Add a drm panic handler

[Jocelyn Falempe <jfalempe@redhat.com>]

On 07/10/2023 14:38, Noralf Trønnes wrote:
> 
> 
> On 10/3/23 16:22, Jocelyn Falempe wrote:
>> This module displays a user friendly message when a kernel panic
>> occurs. It currently doesn't contain any debug information,
>> but that can be added later.
>>
>> v2
>>   * Use get_scanout_buffer() instead of the drm client API.
>>    (Thomas Zimmermann)
>>   * Add the panic reason to the panic message (Nerdopolis)
>>   * Add an exclamation mark (Nerdopolis)
>>
>> v3
>>   * Rework the drawing functions, to write the pixels line by line and
>>   to use the drm conversion helper to support other formats.
>>   (Thomas Zimmermann)
>>
>> v4
>>   * Use drm_fb_r1_to_32bit for fonts (Thomas Zimmermann)
>>   * Remove the default y to DRM_PANIC config option (Thomas Zimmermann)
>>   * Add foreground/background color config option
>>   * Fix the bottom lines not painted if the framebuffer height
>>     is not a multiple of the font height.
>>   * Automatically register the device to drm_panic, if the function
>>     get_scanout_buffer exists. (Thomas Zimmermann)
>>
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> ---
>>   drivers/gpu/drm/Kconfig     |  22 ++
>>   drivers/gpu/drm/Makefile    |   1 +
>>   drivers/gpu/drm/drm_drv.c   |   8 +
>>   drivers/gpu/drm/drm_panic.c | 413 ++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_drv.h       |  14 ++
>>   include/drm/drm_panic.h     |  41 ++++
>>   6 files changed, 499 insertions(+)
>>   create mode 100644 drivers/gpu/drm/drm_panic.c
>>   create mode 100644 include/drm/drm_panic.h
>>
> 
>> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
>> new file mode 100644
> 
>> +static void draw_panic_device(struct drm_device *dev, const char *msg)
>> +{
>> +	struct drm_scanout_buffer sb;
>> +
>> +	if (dev->driver->get_scanout_buffer(dev, &sb))
>> +		return;
>> +	if (!drm_panic_line_buf)
>> +		return;
>> +
> 
> Unless something has changed since 2019 we need to make sure fbcon
> hasn't already printed the panic to avoid jumbled output. See [1] for
> more info.

I think DRM_PANIC and fbcon are incompatible, so in Kconfig I prevent 
them to be built together:

config DRM_PANIC
   bool "Display a user-friendly message when a kernel panic occurs"
   depends on DRM && !FRAMEBUFFER_CONSOLE

So DRM_PANIC should be used with a userspace console, either kmscon, or 
some lightweight terminal emulator/wayland compositor, like cage/foot.

As fbcon has lost scrolling support, it's time to switch to something 
better. [1]

-- 

Jocelyn

[1]
https://www.reddit.com/r/linux/comments/10eccv9/config_vtn_in_2023/

> 
> Noralf.
> 
> [1]
> https://lore.kernel.org/dri-devel/20190312095320.GX2665@phenom.ffwll.local/
> 
>> +	/* to avoid buffer overflow on drm_panic_line_buf */
>> +	if (sb.width > DRM_PANIC_MAX_WIDTH)
>> +		sb.width = DRM_PANIC_MAX_WIDTH;
>> +
>> +	draw_panic_static(&sb, msg);
>> +}
>