On 07/10/2023 16:30, Noralf Trønnes wrote:
On 10/3/23 16:22, Jocelyn Falempe wrote:
Add support for the drm_panic module, which displays a message to
the screen when a kernel panic occurs.
Signed-off-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
---
drivers/gpu/drm/mgag200/mgag200_drv.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 976f0ab2006b..229d9c116b42 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -12,10 +12,12 @@
#include <drm/drm_aperture.h>
#include <drm/drm_drv.h>
#include <drm/drm_fbdev_generic.h>
+#include <drm/drm_framebuffer.h>
#include <drm/drm_file.h>
#include <drm/drm_ioctl.h>
#include <drm/drm_managed.h>
#include <drm/drm_module.h>
+#include <drm/drm_panic.h>
#include <drm/drm_pciids.h>
#include "mgag200_drv.h"
@@ -83,6 +85,27 @@ resource_size_t mgag200_probe_vram(void __iomem *mem, resource_size_t size)
return offset - 65536;
}
+static int mgag200_get_scanout_buffer(struct drm_device *dev,
+ struct drm_scanout_buffer *sb)
+{
+ struct drm_plane *plane;
+ struct mga_device *mdev = to_mga_device(dev);
+ struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(mdev->vram);
+
+ /* mgag200 has only one plane */
+ drm_for_each_plane(plane, dev) {
In my first 2016 attempt on a panic handler I was told to trylock crtc
and plane and skip if it wasn't possible:
- We need locking. One of the biggest problems with the old oops handling
was that it was very good at trampling over driver state, causing more
(unrelated) oopses in kms code and making sure the original oops was no
longer visible. I think the shared code must take care of all the
locking needs to avoid fragile code in drivers. ww_mutex_trylock on the
drm_crtc and drm_plane should be enough (we need both for drivers where
planes can be reassigned between drivers).
drm_panic register a panic handler, so it won't get called on oopses.
So when the panic handler is called, no other task can run in parallel,
and no drm code will run after it.
Also for the Matrox driver, it's always safe to write to the VRAM. The
only risk is that if you're in the middle of a modeset, you may get
garbage output.
There might still be a race condition for fb->format, width, height and
pitches, and looping through the plane list.
I also need to check if crtc and plane locks are not taken when the
driver is copying the damage region to the VRAM, otherwise the
probability to actually see the panic will be very low.
I also need to wake up the monitor, if it's currently in DPMS off.
--
Jocelyn
See [1] for a list of other things to consider.
Noralf.
[1]
https://lore.kernel.org/dri-devel/20160810091529.GQ6232@phenom.ffwll.local/
+ if (!plane->state || !plane->state->fb)
+ return -ENODEV;
+ sb->format = plane->state->fb->format;
+ sb->width = plane->state->fb->width;
+ sb->height = plane->state->fb->height;
+ sb->pitch = plane->state->fb->pitches[0];
+ sb->map = map;
+ return 0;
+ }
+ return -ENODEV;
+}
+
/*
* DRM driver
*/
@@ -98,6 +121,7 @@ static const struct drm_driver mgag200_driver = {
.major = DRIVER_MAJOR,
.minor = DRIVER_MINOR,
.patchlevel = DRIVER_PATCHLEVEL,
+ .get_scanout_buffer = mgag200_get_scanout_buffer,
DRM_GEM_SHMEM_DRIVER_OPS,
};
