| Message ID | 20240227100459.194478-6-jfalempe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panic: Add a drm panic handler | expand |
On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote: > Add support for the drm_panic module, which displays a user-friendly > message to the screen when a kernel panic occurs. > > v8: > * Replace get_scanout_buffer() with drm_panic_set_buffer() > (Thomas Zimmermann) > > Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> > --- > drivers/gpu/drm/tiny/simpledrm.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c > index 7ce1c4617675..a2190995354a 100644 > --- a/drivers/gpu/drm/tiny/simpledrm.c > +++ b/drivers/gpu/drm/tiny/simpledrm.c > @@ -25,6 +25,7 @@ > #include <drm/drm_gem_shmem_helper.h> > #include <drm/drm_managed.h> > #include <drm/drm_modeset_helper_vtables.h> > +#include <drm/drm_panic.h> > #include <drm/drm_probe_helper.h> > > #define DRIVER_NAME "simpledrm" > @@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > +static void simpledrm_init_panic_buffer(struct drm_plane *plane) > +{ > + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); > + struct drm_framebuffer fb; > + > + /* Fake framebuffer struct for drm_panic_set_buffer */ > + fb.width = sdev->mode.hdisplay; > + fb.height = sdev->mode.vdisplay; > + fb.format = sdev->format; > + fb.pitches[0] = sdev->pitch; > + > + drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base); > +} > + > static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = { > .fb_create = drm_gem_fb_create_with_dirty, > .atomic_check = drm_atomic_helper_check, > @@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv, > return ERR_PTR(ret); > drm_plane_helper_add(primary_plane, &simpledrm_primary_plane_helper_funcs); > drm_plane_enable_fb_damage_clips(primary_plane); > + drm_panic_register(primary_plane); Just a quick comment on this: This does not work, the driver is not ready to handle panic calls at this stage. Instead we need to automatically register all planes that support panic handling in drm_dev_register(), and we need to remove them all again in drm_dev_unregister(). Outside of these functions it is not safe to call into driver code. At that point it might be simpler to only register one panic notifier per drm_device, and push the loop into the panic handler again. Cheers, Sima > + simpledrm_init_panic_buffer(primary_plane); > > /* CRTC */ > > -- > 2.43.0 >
On 29/02/2024 12:17, Daniel Vetter wrote: > On Tue, Feb 27, 2024 at 11:04:16AM +0100, Jocelyn Falempe wrote: >> Add support for the drm_panic module, which displays a user-friendly >> message to the screen when a kernel panic occurs. >> >> v8: >> * Replace get_scanout_buffer() with drm_panic_set_buffer() >> (Thomas Zimmermann) >> >> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> >> --- >> drivers/gpu/drm/tiny/simpledrm.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c >> index 7ce1c4617675..a2190995354a 100644 >> --- a/drivers/gpu/drm/tiny/simpledrm.c >> +++ b/drivers/gpu/drm/tiny/simpledrm.c >> @@ -25,6 +25,7 @@ >> #include <drm/drm_gem_shmem_helper.h> >> #include <drm/drm_managed.h> >> #include <drm/drm_modeset_helper_vtables.h> >> +#include <drm/drm_panic.h> >> #include <drm/drm_probe_helper.h> >> >> #define DRIVER_NAME "simpledrm" >> @@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = { >> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, >> }; >> >> +static void simpledrm_init_panic_buffer(struct drm_plane *plane) >> +{ >> + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); >> + struct drm_framebuffer fb; >> + >> + /* Fake framebuffer struct for drm_panic_set_buffer */ >> + fb.width = sdev->mode.hdisplay; >> + fb.height = sdev->mode.vdisplay; >> + fb.format = sdev->format; >> + fb.pitches[0] = sdev->pitch; >> + >> + drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base); >> +} >> + >> static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = { >> .fb_create = drm_gem_fb_create_with_dirty, >> .atomic_check = drm_atomic_helper_check, >> @@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv, >> return ERR_PTR(ret); >> drm_plane_helper_add(primary_plane, &simpledrm_primary_plane_helper_funcs); >> drm_plane_enable_fb_damage_clips(primary_plane); >> + drm_panic_register(primary_plane); > > Just a quick comment on this: > > This does not work, the driver is not ready to handle panic calls at this > stage. Instead we need to automatically register all planes that support > panic handling in drm_dev_register(), and we need to remove them all again > in drm_dev_unregister(). Outside of these functions it is not safe to call > into driver code. If you register the primary plane and didn't call drm_panic_set_buffer() yet, the panic handler will not do anything, so it should be safe. But if we revert to using the get_scanout_buffer(), this makes sense. > > At that point it might be simpler to only register one panic notifier per > drm_device, and push the loop into the panic handler again. > > Cheers, Sima > >> + simpledrm_init_panic_buffer(primary_plane); >> >> /* CRTC */ >> >> -- >> 2.43.0 >> >
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 7ce1c4617675..a2190995354a 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -25,6 +25,7 @@ #include <drm/drm_gem_shmem_helper.h> #include <drm/drm_managed.h> #include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_panic.h> #include <drm/drm_probe_helper.h> #define DRIVER_NAME "simpledrm" @@ -735,6 +736,20 @@ static const struct drm_connector_funcs simpledrm_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; +static void simpledrm_init_panic_buffer(struct drm_plane *plane) +{ + struct simpledrm_device *sdev = simpledrm_device_of_dev(plane->dev); + struct drm_framebuffer fb; + + /* Fake framebuffer struct for drm_panic_set_buffer */ + fb.width = sdev->mode.hdisplay; + fb.height = sdev->mode.vdisplay; + fb.format = sdev->format; + fb.pitches[0] = sdev->pitch; + + drm_panic_set_buffer(plane->panic_scanout, &fb, &sdev->screen_base); +} + static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = { .fb_create = drm_gem_fb_create_with_dirty, .atomic_check = drm_atomic_helper_check, @@ -945,6 +960,8 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv, return ERR_PTR(ret); drm_plane_helper_add(primary_plane, &simpledrm_primary_plane_helper_funcs); drm_plane_enable_fb_damage_clips(primary_plane); + drm_panic_register(primary_plane); + simpledrm_init_panic_buffer(primary_plane); /* CRTC */
Add support for the drm_panic module, which displays a user-friendly message to the screen when a kernel panic occurs. v8: * Replace get_scanout_buffer() with drm_panic_set_buffer() (Thomas Zimmermann) Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com> --- drivers/gpu/drm/tiny/simpledrm.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)