Re: [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
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
---
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
#include
#include
+#include
#include
#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, , >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,
_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
Re: [PATCH v8 5/8] drm/simpledrm: Add drm_panic support
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
> ---
> 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
> #include
> #include
> +#include
> #include
>
> #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, , >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,
> _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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v8 5/8] drm/simpledrm: Add drm_panic support
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
---
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
#include
#include
+#include
#include
#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, , >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,
_primary_plane_helper_funcs);
drm_plane_enable_fb_damage_clips(primary_plane);
+ drm_panic_register(primary_plane);
+ simpledrm_init_panic_buffer(primary_plane);
/* CRTC */
--
2.43.0
