2 commits

Author	SHA1	Message	Date
Jocelyn Falempe	bf9fb17c66	drm/panic: Add a drm panic handler ... 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) v5 * Change the drawing API, use drm_fb_blit_from_r1() to draw the font. * Also add drm_fb_fill() to fill area with background color. * Add draw_pixel_xy() API for drivers that can't provide a linear buffer. * Add a flush() callback for drivers that needs to synchronize the buffer. * Add a void *private field, so drivers can pass private data to draw_pixel_xy() and flush(). v6 * Fix sparse warning for panic_msg and logo. v7 * Add select DRM_KMS_HELPER for the color conversion functions. v8 * Register directly each plane to the panic notifier (Sima) * Add raw_spinlock to properly handle concurrency (Sima) * Register plane instead of device, to avoid looping through plane list, and simplify code. * Replace get_scanout_buffer() logic with drm_panic_set_buffer() (Thomas Zimmermann) * Removed the draw_pixel_xy() API, will see later if it can be added back. v9 * Revert to using get_scanout_buffer() (Sima) * Move get_scanout_buffer() and panic_flush() to the plane helper functions (Thomas Zimmermann) * Register all planes with get_scanout_buffer() to the panic notifier * Use drm_panic_lock() to protect against race (Sima) v10 * Move blit and fill functions back in drm_panic (Thomas Zimmermann). * Simplify the text drawing functions. * Use kmsg_dumper instead of panic_notifier (Sima). v12 * Use array for map and pitch in struct drm_scanout_buffer to support multi-planar format later. (Thomas Zimmermann) * Better indent struct drm_scanout_buffer declaration. (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240409163432.352518-3-jfalempe@redhat.com Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>	2024-04-15 16:12:49 +02:00
Daniel Vetter	e2a1cda3e0	drm/panic: Add drm panic locking ... Rough sketch for the locking of drm panic printing code. The upshot of this approach is that we can pretty much entirely rely on the atomic commit flow, with the pair of raw_spin_lock/unlock providing any barriers we need, without having to create really big critical sections in code. This also avoids the need that drivers must explicitly update the panic handler state, which they might forget to do, or not do consistently, and then we blow up in the worst possible times. It is somewhat racy against a concurrent atomic update, and we might write into a buffer which the hardware will never display. But there's fundamentally no way to avoid that - if we do the panic state update explicitly after writing to the hardware, we might instead write to an old buffer that the user will barely ever see. Note that an rcu protected deference of plane->state would give us the the same guarantees, but it has the downside that we then need to protect the plane state freeing functions with call_rcu too. Which would very widely impact a lot of code and therefore doesn't seem worth the complexity compared to a raw spinlock with very tiny critical sections. Plus rcu cannot be used to protect access to peek/poke registers anyway, so we'd still need it for those cases. Peek/poke registers for vram access (or a gart pte reserved just for panic code) are also the reason I've gone with a per-device and not per-plane spinlock, since usually these things are global for the entire display. Going with per-plane locks would mean drivers for such hardware would need additional locks, which we don't want, since it deviates from the per-console takeoverlocks design. Longer term it might be useful if the panic notifiers grow a bit more structure than just the absolute bare EXPORT_SYMBOL(panic_notifier_list) - somewhat aside, why is that not EXPORT_SYMBOL_GPL ... If panic notifiers would be more like console drivers with proper register/unregister interfaces we could perhaps reuse the very fancy console lock with all it's check and takeover semantics that John Ogness is developing to fix the console_lock mess. But for the initial cut of a drm panic printing support I don't think we need that, because the critical sections are extremely small and only happen once per display refresh. So generally just 60 tiny locked sections per second, which is nothing compared to a serial console running a 115kbaud doing really slow mmio writes for each byte. So for now the raw spintrylock in drm panic notifier callback should be good enough. Another benefit of making panic notifiers more like full blown consoles (that are used in panics only) would be that we get the two stage design, where first all the safe outputs are used. And then the dangerous takeover tricks are deployed (where for display drivers we also might try to intercept any in-flight display buffer flips, which if we race and misprogram fifos and watermarks can hang the memory controller on some hw). For context the actual implementation on the drm side is by Jocelyn and this patch is meant to be combined with the overall approach in v7 (v8 is a bit less flexible, which I think is the wrong direction): https://lore.kernel.org/dri-devel/20240104160301.185915-1-jfalempe@redhat.com/ Note that the locking is very much not correct there, hence this separate rfc. Starting from v10, I (Jocelyn) have included this patch in the drm_panic series, and done the corresponding changes. v2: - fix authorship, this was all my typing - some typo oopsies - link to the drm panic work by Jocelyn for context v10: - Use spinlock_irqsave/restore (John Ogness) v11: - Use macro instead of inline functions for drm_panic_lock/unlock (John Ogness) Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jocelyn Falempe <jfalempe@redhat.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org> Cc: Lukas Wunner <lukas@wunner.de> Cc: Petr Mladek <pmladek@suse.com> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: John Ogness <john.ogness@linutronix.de> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: David Airlie <airlied@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240409163432.352518-2-jfalempe@redhat.com Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>	2024-04-15 16:11:26 +02:00