Re: [PATCH] drm/ssd130x: Add drm_panic support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 21/06/2024 00:22, Javier Martinez Canillas wrote:
Add support for the drm_panic infrastructure, which allows to display
a user friendly message on the screen when a Linux kernel panic occurs.

The display controller doesn't scanout the framebuffer, but instead the
pixels are sent to the device using a transport bus. For this reason, a
.panic_flush handler is needed to flush the panic image to the display.

Thanks for this patch, that's really cool that drm_panic can work on this device too.


Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
---

  drivers/gpu/drm/solomon/ssd130x.c | 64 +++++++++++++++++++++++++++++++
  1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 6f51bcf774e2..0bac97bd39b9 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -32,6 +32,7 @@
  #include <drm/drm_managed.h>
  #include <drm/drm_modes.h>
  #include <drm/drm_rect.h>
+#include <drm/drm_panic.h>
  #include <drm/drm_probe_helper.h>
#include "ssd130x.h"
@@ -1386,6 +1387,63 @@ static void ssd133x_primary_plane_atomic_disable(struct drm_plane *plane,
  	drm_dev_exit(idx);
  }
+static int ssd130x_primary_plane_helper_get_scanout_buffer(struct drm_plane *plane,
+							   struct drm_scanout_buffer *sb)
+{
+	struct drm_plane_state *plane_state = plane->state;
+	struct drm_shadow_plane_state *shadow_plane_state;
+
+	if (!plane_state || !plane_state->fb || !plane_state->crtc)
+		return -EINVAL;
+
+	shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+
+	sb->format = plane->state->fb->format;
+	sb->width = plane->state->fb->width;
+	sb->height = plane->state->fb->height;
+	sb->pitch[0] = plane->state->fb->pitches[0];
+	sb->map[0] = shadow_plane_state->data[0];
+
+	return 0;
+}
+
+static void ssd130x_primary_plane_helper_panic_flush(struct drm_plane *plane)
+{
+	struct drm_plane_state *plane_state = plane->state;
+	struct ssd130x_plane_state *ssd130x_plane_state = to_ssd130x_plane_state(plane_state);
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+	struct drm_crtc *crtc = plane_state->crtc;
+	struct ssd130x_crtc_state *ssd130x_crtc_state = to_ssd130x_crtc_state(crtc->state);
+
+	ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &plane_state->dst,
+			     ssd130x_plane_state->buffer, ssd130x_crtc_state->data_array,
+			     &shadow_plane_state->fmtcnv_state);

ssd130x_fb_blit_rect() will call regmap->write(), which involve mutex and might sleep. And if the mutex is taken when the panic occurs, it might deadlock the panic handling. One solution would be to configure the regmap with config->fast_io and config->use_raw_spinlock, and check that the lock is available with try_lock(map->raw_spin_lock) But that means it will waste cpu cycle with busy waiting for normal operation, which is not good.

So for this particular device, I think it's ok, because it's unlikely you'll run kdump or other kernel panic handlers. But I would like to know what others think about it, and if it's acceptable or not.

--

Jocelyn



+}
+
+static void ssd132x_primary_plane_helper_panic_flush(struct drm_plane *plane)
+{
+	struct drm_plane_state *plane_state = plane->state;
+	struct ssd130x_plane_state *ssd130x_plane_state = to_ssd130x_plane_state(plane_state);
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+	struct drm_crtc *crtc = plane_state->crtc;
+	struct ssd130x_crtc_state *ssd130x_crtc_state = to_ssd130x_crtc_state(crtc->state);
+
+	ssd132x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &plane_state->dst,
+			     ssd130x_plane_state->buffer, ssd130x_crtc_state->data_array,
+			     &shadow_plane_state->fmtcnv_state);
+}
+
+static void ssd133x_primary_plane_helper_panic_flush(struct drm_plane *plane)
+{
+	struct drm_plane_state *plane_state = plane->state;
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
+	struct drm_crtc *crtc = plane_state->crtc;
+	struct ssd130x_crtc_state *ssd130x_crtc_state = to_ssd130x_crtc_state(crtc->state);
+
+	ssd133x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &plane_state->dst,
+			     ssd130x_crtc_state->data_array, &shadow_plane_state->fmtcnv_state);
+}
+
  /* Called during init to allocate the plane's atomic state. */
  static void ssd130x_primary_plane_reset(struct drm_plane *plane)
  {
@@ -1442,18 +1500,24 @@ static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs[]
  		.atomic_check = ssd130x_primary_plane_atomic_check,
  		.atomic_update = ssd130x_primary_plane_atomic_update,
  		.atomic_disable = ssd130x_primary_plane_atomic_disable,
+		.get_scanout_buffer = ssd130x_primary_plane_helper_get_scanout_buffer,
+		.panic_flush = ssd130x_primary_plane_helper_panic_flush,
  	},
  	[SSD132X_FAMILY] = {
  		DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
  		.atomic_check = ssd132x_primary_plane_atomic_check,
  		.atomic_update = ssd132x_primary_plane_atomic_update,
  		.atomic_disable = ssd132x_primary_plane_atomic_disable,
+		.get_scanout_buffer = ssd130x_primary_plane_helper_get_scanout_buffer,
+		.panic_flush = ssd132x_primary_plane_helper_panic_flush,
  	},
  	[SSD133X_FAMILY] = {
  		DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
  		.atomic_check = ssd133x_primary_plane_atomic_check,
  		.atomic_update = ssd133x_primary_plane_atomic_update,
  		.atomic_disable = ssd133x_primary_plane_atomic_disable,
+		.get_scanout_buffer = ssd130x_primary_plane_helper_get_scanout_buffer,
+		.panic_flush = ssd133x_primary_plane_helper_panic_flush,
  	}
  };




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux