[RFC 0/3] drm: Add panic handling

[RFC 0/3] drm: Add panic handling

[Noralf Trønnes]

This patchset proposes a way to get output of kernel messages on the
display during panic(). The responsibility of the driver is to provide
a framebuffer with a virtual mapping that the messages can be rendered
on. Only linear RGB is supported in this version.

David Herrmann, who has worked on this problem before[1], prefers to have
a dedicated renderer for the panic case, and a different one for other
uses like console. To try and keep the code simple, I just collect kernel
messages in a circular buffer until a panic situation occurs and then the
messages are rendered on all the framebuffers/devices it can find.
From this point on, all messages are rendered as they come in.

I have added a debugfs file that can trigger various "levels" of panic
handling to aid in testing the drivers. There's one test that is missing
which Daniel Vetter mentions on the DRMJanitors page:

    hardirq context could be achieved by using an ipi to the local processor.

I don't know how to actually do this.

I've come to the understanding that switching buffers can actually be very
tricky for a driver to achieve in atomic context like a panic situation.
And atomically vmapping the buffer being scanned out isn't possible.
Maybe it's possible to kmap_atomic() the pages and write to them individually?

I know that this is low priority for the drm maintainers, so please ignore
this if the timing isn't right and/or the solution is wrong.


Noralf.

[1] drmlog:
    https://lists.freedesktop.org/archives/dri-devel/2014-March/055136.html


Noralf Trønnes (3):
  drm: Add a way to iterate over minors
  drm: Add panic handling
  drm: simpledrm: Add panic handling

 drivers/gpu/drm/Makefile                  |   2 +-
 drivers/gpu/drm/drm_drv.c                 |  33 ++
 drivers/gpu/drm/drm_internal.h            |   4 +
 drivers/gpu/drm/drm_panic.c               | 606 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/simpledrm/simpledrm_drv.c |  24 ++
 include/drm/drmP.h                        |  35 ++
 6 files changed, 703 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_panic.c

--
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 1/3] drm: Add a way to iterate over minors

[Noralf Trønnes]

This adds a way for in-kernel users to iterate over the available
DRM minors. The implementation is oops safe so the panic code
can safely use it.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_drv.c | 30 ++++++++++++++++++++++++++++++
 include/drm/drmP.h        | 13 +++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be27ed3..3b14366 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -292,6 +292,36 @@ void drm_minor_release(struct drm_minor *minor)
 }
 
 /**
+ * drm_minor_lookup - Lookup DRM minor
+ * @minor_id: Minor ID of the DRM-minor
+ *
+ * Looks up the given minor-ID and returns the respective DRM-minor object.
+ * No reference is taken on the underlying device.
+ * See drm_minor_for_each() for iterating over all minors.
+ *
+ * Returns:
+ * Pointer to minor-object or NULL.
+ */
+struct drm_minor *drm_minor_lookup(unsigned int minor_id)
+{
+	struct drm_minor *minor;
+	unsigned long flags;
+	int locked = 1;
+
+	if (oops_in_progress)
+		locked = spin_trylock_irqsave(&drm_minor_lock, flags);
+	else
+		spin_lock_irqsave(&drm_minor_lock, flags);
+
+	minor = idr_find(&drm_minors_idr, minor_id);
+
+	if (locked)
+		spin_unlock_irqrestore(&drm_minor_lock, flags);
+
+	return minor;
+}
+
+/**
  * DOC: driver instance overview
  *
  * A device instance for a drm driver is represented by struct &drm_device. This
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d377865..bc7006e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -974,6 +974,19 @@ void drm_dev_unregister(struct drm_device *dev);
 
 struct drm_minor *drm_minor_acquire(unsigned int minor_id);
 void drm_minor_release(struct drm_minor *minor);
+struct drm_minor *drm_minor_lookup(unsigned int minor_id);
+
+/**
+ * drm_minor_for_each - Iterate over DRM minors
+ * @minor: DRM minor handle
+ * @type: DRM minor type to iterate over
+ * @id: id handle
+ *
+ * Iterate over the registered DRM minors of a given type.
+ */
+#define drm_minor_for_each(minor, type, id)  \
+	for ((id) = 0; (id) < 64; (id)++)  \
+		if (((minor) = drm_minor_lookup((id) + (type) * 64)))
 
 /*@}*/
 
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 2/3] drm: Add panic handling

[Noralf Trønnes]

This adds support for outputting kernel messages on panic().
The drivers that supports it, provides a framebuffer that the
messages can be rendered on.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/Makefile       |   2 +-
 drivers/gpu/drm/drm_drv.c      |   3 +
 drivers/gpu/drm/drm_internal.h |   4 +
 drivers/gpu/drm/drm_panic.c    | 606 +++++++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h             |  22 ++
 5 files changed, 636 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_panic.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index eba32ad..ff04e41 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -12,7 +12,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_info.o drm_debugfs.o drm_encoder_slave.o \
 		drm_trace_points.o drm_global.o drm_prime.o \
 		drm_rect.o drm_vma_manager.o drm_flip_work.o \
-		drm_modeset_lock.o drm_atomic.o drm_bridge.o
+		drm_modeset_lock.o drm_atomic.o drm_bridge.o drm_panic.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 3b14366..457ee91 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -861,6 +861,8 @@ static int __init drm_core_init(void)
 		goto err_p3;
 	}
 
+	drm_panic_init();
+
 	DRM_INFO("Initialized %s %d.%d.%d %s\n",
 		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
 	return 0;
@@ -876,6 +878,7 @@ err_p1:
 
 static void __exit drm_core_exit(void)
 {
+	drm_panic_exit();
 	debugfs_remove(drm_debugfs_root);
 	drm_sysfs_destroy();
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index b86dc9b..7463d9d 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -90,6 +90,10 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
 void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
 void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
 
+/* drm_panic.c */
+void drm_panic_init(void);
+void drm_panic_exit(void);
+
 /* drm_debugfs.c */
 #if defined(CONFIG_DEBUG_FS)
 int drm_debugfs_init(struct drm_minor *minor, int minor_id,
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
new file mode 100644
index 0000000..e185c9d
--- /dev/null
+++ b/drivers/gpu/drm/drm_panic.c
@@ -0,0 +1,606 @@
+/*
+ * Copyright 2016 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <asm/unaligned.h>
+#include <drm/drmP.h>
+#include <linux/console.h>
+#include <linux/debugfs.h>
+#include <linux/font.h>
+#include <linux/kernel.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+struct drm_panic_fb {
+	struct drm_framebuffer *fb;
+	void *vmem;
+	const struct font_desc *font;
+	unsigned int cols;
+	unsigned int rows;
+	unsigned int xpos;
+	unsigned int ypos;
+};
+
+#define DRM_PANIC_MAX_FBS	64
+static struct drm_panic_fb drm_panic_fbs[DRM_PANIC_MAX_FBS];
+
+#define DRM_PANIC_MAX_KMSGS	SZ_4K
+static char *drm_panic_kmsgs;
+static size_t drm_panic_kmsgs_pos;
+
+static bool drm_panic_active;
+
+static void drm_panic_log(const char *fmt, ...);
+
+static inline void drm_panic_draw_pixel(u8 *dst, u32 pixel_format, bool val)
+{
+	switch (pixel_format & ~DRM_FORMAT_BIG_ENDIAN) {
+
+	case DRM_FORMAT_C8:
+	case DRM_FORMAT_RGB332:
+	case DRM_FORMAT_BGR233:
+		*dst = val ? 0xff : 0x00;
+		break;
+
+	case DRM_FORMAT_XRGB4444:
+	case DRM_FORMAT_ARGB4444:
+	case DRM_FORMAT_XBGR4444:
+	case DRM_FORMAT_ABGR4444:
+		put_unaligned(val ? 0x0fff : 0x0000, (u16 *)dst);
+		break;
+
+	case DRM_FORMAT_RGBX4444:
+	case DRM_FORMAT_RGBA4444:
+	case DRM_FORMAT_BGRX4444:
+	case DRM_FORMAT_BGRA4444:
+		put_unaligned(val ? 0xfff0 : 0x0000, (u16 *)dst);
+		break;
+
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_ARGB1555:
+	case DRM_FORMAT_XBGR1555:
+	case DRM_FORMAT_ABGR1555:
+		put_unaligned(val ? 0x7fff : 0x0000, (u16 *)dst);
+		break;
+
+	case DRM_FORMAT_RGBX5551:
+	case DRM_FORMAT_RGBA5551:
+	case DRM_FORMAT_BGRX5551:
+	case DRM_FORMAT_BGRA5551:
+		put_unaligned(val ? 0xfffe : 0x0000, (u16 *)dst);
+		break;
+
+	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_BGR565:
+		put_unaligned(val ? 0xffff : 0x0000, (u16 *)dst);
+		break;
+
+	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_BGR888:
+		dst[0] = val ? 0xff : 0x00;
+		dst[1] = val ? 0xff : 0x00;
+		dst[2] = val ? 0xff : 0x00;
+		break;
+
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
+		put_unaligned(val ? 0x00ffffff : 0x00000000, (u32 *)dst);
+		break;
+
+	case DRM_FORMAT_RGBX8888:
+	case DRM_FORMAT_RGBA8888:
+	case DRM_FORMAT_BGRX8888:
+	case DRM_FORMAT_BGRA8888:
+		put_unaligned(val ? 0xffffff00 : 0x00000000, (u32 *)dst);
+		break;
+
+	case DRM_FORMAT_XRGB2101010:
+	case DRM_FORMAT_ARGB2101010:
+	case DRM_FORMAT_XBGR2101010:
+	case DRM_FORMAT_ABGR2101010:
+		put_unaligned(val ? 0x3fffffff : 0x00000000, (u32 *)dst);
+		break;
+
+	case DRM_FORMAT_RGBX1010102:
+	case DRM_FORMAT_RGBA1010102:
+	case DRM_FORMAT_BGRX1010102:
+	case DRM_FORMAT_BGRA1010102:
+		put_unaligned(val ? 0xfffffffc : 0x00000000, (u32 *)dst);
+		break;
+	}
+}
+
+static void drm_panic_render(struct drm_panic_fb *pfb,
+			     const char *text, unsigned int len)
+{
+	const struct font_desc *font = pfb->font;
+	unsigned int pix_depth, pix_bpp, cpp;
+	unsigned int col = pfb->xpos;
+	unsigned int row = pfb->ypos;
+	unsigned int i, h, w;
+	void *dst, *pos;
+	u8 fontline;
+
+	if ((row + 1) * font->height > pfb->fb->height)
+		return;
+
+	if ((col + len) * font->width > pfb->fb->width)
+		return;
+
+	drm_fb_get_bpp_depth(pfb->fb->pixel_format, &pix_depth, &pix_bpp);
+	cpp = DIV_ROUND_UP(pix_bpp, 8);
+
+	/* TODO: should fb->offsets[0] be added here? */
+	dst = pfb->vmem + (row * font->height * pfb->fb->pitches[0]) +
+	      (col * font->width * cpp);
+
+	for (h = 0; h < font->height; h++) {
+		pos = dst;
+
+		for (i = 0; i < len; i++) {
+			fontline = *(u8 *)(font->data + text[i] * font->height + h);
+
+			for (w = 0; w < font->width; w++) {
+				drm_panic_draw_pixel(pos, pfb->fb->pixel_format,
+						     fontline & BIT(7 - w));
+				pos += cpp;
+			}
+		}
+
+		dst += pfb->fb->pitches[0];
+	}
+}
+
+static void drm_panic_scroll_up(struct drm_panic_fb *pfb)
+{
+	void *src = pfb->vmem + (pfb->font->height * pfb->fb->pitches[0]);
+	size_t len = (pfb->fb->height - pfb->font->height) *
+		     pfb->fb->pitches[0];
+
+	drm_panic_log("%s\n", __func__);
+
+	memmove(pfb->vmem, src, len);
+	memset(pfb->vmem + len, 0, pfb->font->height * pfb->fb->pitches[0]);
+}
+
+static void drm_panic_clear_screen(struct drm_panic_fb *pfb)
+{
+	memset(pfb->vmem, 0, pfb->fb->height * pfb->fb->pitches[0]);
+}
+
+static void drm_panic_log_msg(char *pre, const char *str, unsigned int len)
+{
+	char buf[512];
+
+	if (len > 510)
+		len = 510;
+
+	memcpy(buf, str, len);
+	buf[len] = '\n';
+	buf[len + 1] = '\0';
+
+	drm_panic_log("%s%s", pre, buf);
+}
+
+static void drm_panic_putcs_no_lf(struct drm_panic_fb *pfb,
+				  const char *str, unsigned int len)
+{
+	drm_panic_log("%s(len=%u) x=%u, y=%u\n", __func__, len,
+			pfb->xpos, pfb->ypos);
+
+	if (len <= 0)
+		return;
+
+	drm_panic_log_msg("", str, len);
+
+	drm_panic_render(pfb, str, len);
+
+}
+
+static void drm_panic_putcs(struct drm_panic_fb *pfb,
+			    const char *str, unsigned int num)
+{
+	unsigned int slen;
+	int len = num;
+	char *lf;
+
+	drm_panic_log("%s(num=%u)\n", __func__, num);
+
+	while (len > 0) {
+
+		if (pfb->ypos == pfb->rows) {
+			pfb->ypos--;
+			drm_panic_scroll_up(pfb);
+		}
+
+		lf = strpbrk(str, "\n");
+		if (lf)
+			slen = lf - str;
+		else
+			slen = len;
+
+		if (pfb->xpos + slen > pfb->cols)
+			slen = pfb->cols - pfb->xpos;
+
+		drm_panic_putcs_no_lf(pfb, str, slen);
+
+		len -= slen;
+		str += slen;
+		pfb->xpos += slen;
+
+		if (lf) {
+			str++;
+			len--;
+			pfb->xpos = 0;
+			pfb->ypos++;
+		}
+	}
+}
+
+static void drm_panic_write(const char *str, unsigned int num)
+{
+	unsigned int i;
+
+	if (!num)
+		return;
+
+	drm_panic_log("%s(num=%u)\n", __func__, num);
+
+	for (i = 0; i < DRM_PANIC_MAX_FBS; i++) {
+		if (!drm_panic_fbs[i].fb)
+			break;
+		drm_panic_putcs(&drm_panic_fbs[i], str, num);
+	}
+}
+
+/* this one is serialized by console_lock() */
+static void drm_panic_console_write(struct console *con,
+				    const char *str, unsigned int num)
+{
+	unsigned int i;
+
+	drm_panic_log_msg("->", str, num);
+
+	/* Buffer up messages to be replayed on panic */
+	if (!drm_panic_active) {
+		for (i = 0; i < num; i++) {
+			drm_panic_kmsgs[drm_panic_kmsgs_pos++] = *str++;
+			if (drm_panic_kmsgs_pos == DRM_PANIC_MAX_KMSGS)
+				drm_panic_kmsgs_pos = 0;
+		}
+		return;
+	}
+
+	drm_panic_write(str, num);
+}
+
+static struct console drm_panic_console = {
+	.name =         "drmpanic",
+	.write =        drm_panic_console_write,
+	.flags =        CON_PRINTBUFFER | CON_ENABLED,
+	.index =        0,
+};
+
+/*
+ * The panic() function makes sure that only one CPU is allowed to run it's
+ * code. So when this handler is called, we're alone. No racing with
+ * console.write() is possible.
+ */
+static int drm_panic_handler(struct notifier_block *this, unsigned long ev,
+			     void *ptr)
+{
+	const struct font_desc *font;
+	struct drm_framebuffer *fb;
+	struct drm_panic_fb *pfb;
+	struct drm_minor *minor;
+	unsigned int fbs = 0;
+	void *vmem;
+	int i;
+
+	drm_panic_log("%s\n", __func__);
+
+	drm_panic_active = true;
+
+	drm_minor_for_each(minor, DRM_MINOR_LEGACY, i) {
+		drm_panic_log("Found minor=%d\n", minor->index);
+		if (!minor->dev || !minor->dev->driver ||
+		    !minor->dev->driver->panic) {
+			drm_panic_log("Skipping: No panic handler\n");
+			continue;
+		}
+
+		fb = minor->dev->driver->panic(minor->dev, &vmem);
+		if (!fb) {
+			drm_panic_log("Skipping: Driver returned NULL\n");
+			continue;
+		}
+
+		if (!fb || !vmem || fb->dev != minor->dev || !fb->pitches[0]) {
+			drm_panic_log("Skipping: Failed check\n");
+			continue;
+		}
+
+		/* only 8-bit wide fonts are supported */
+		font = get_default_font(fb->width, fb->height, BIT(7), -1);
+		if (!font) {
+			drm_panic_log("Skipping: No font available\n");
+			continue;
+		}
+
+		pfb = &drm_panic_fbs[fbs++];
+
+		pfb->fb = fb;
+		pfb->vmem = vmem;
+		pfb->font = font;
+		pfb->cols = fb->width / font->width;
+		pfb->rows = fb->height / font->height;
+
+		drm_panic_clear_screen(pfb);
+
+		drm_panic_log("    %ux%u -> %ux%u, %s, %s\n", fb->width,
+				fb->height, pfb->cols, pfb->rows, font->name,
+				drm_get_format_name(fb->pixel_format));
+	}
+
+	if (drm_panic_kmsgs[0]) {
+		/* safeguard in case we interrupted drm_panic_console_write */
+		if (drm_panic_kmsgs_pos >= DRM_PANIC_MAX_KMSGS)
+			drm_panic_kmsgs_pos = 0;
+
+		drm_panic_write(&drm_panic_kmsgs[drm_panic_kmsgs_pos],
+				DRM_PANIC_MAX_KMSGS - drm_panic_kmsgs_pos);
+		drm_panic_write(drm_panic_kmsgs, drm_panic_kmsgs_pos);
+	}
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block drm_panic_block = {
+	.notifier_call = drm_panic_handler,
+};
+
+
+
+#ifdef CONFIG_DEBUG_FS
+
+/* Out of band logging is useful at least in the initial development phase */
+#define DRM_PANIC_LOG_SIZE	SZ_64K
+#define DRM_PANIC_LOG_LINE	128
+#define DRM_PANIC_LOG_ENTRIES	(DRM_PANIC_LOG_SIZE / DRM_PANIC_LOG_LINE)
+
+static char *log_buf;
+static size_t log_pos;
+static struct dentry *drm_panic_logfs_root;
+
+static void drm_panic_log(const char *fmt, ...)
+{
+	va_list args;
+	u32 rem_nsec;
+	char *text;
+	size_t len;
+	u64 sec;
+
+	if (!log_buf || oops_in_progress)
+		return;
+
+	va_start(args, fmt);
+
+	if (log_pos >= DRM_PANIC_LOG_ENTRIES)
+		log_pos = 0;
+
+	text = log_buf + (log_pos++ * DRM_PANIC_LOG_LINE);
+	if (log_pos == DRM_PANIC_LOG_ENTRIES)
+		log_pos = 0;
+
+	sec = div_u64_rem(local_clock(), 1000000000, &rem_nsec);
+
+	len = scnprintf(text, DRM_PANIC_LOG_LINE, "[%5llu.%06u] ", sec,
+			rem_nsec / 1000);
+
+	vscnprintf(text + len, DRM_PANIC_LOG_LINE - len, fmt, args);
+
+	/* Make sure to always have a newline in case of overflow */
+	if (text[DRM_PANIC_LOG_LINE - 2] != '\0')
+		text[DRM_PANIC_LOG_LINE - 2] = '\n';
+
+	va_end(args);
+}
+
+static int drm_panic_log_show(struct seq_file *m, void *v)
+{
+	size_t pos = log_pos;
+	unsigned int i;
+	char *text;
+
+	for (i = 0; i < DRM_PANIC_LOG_ENTRIES; i++) {
+		text = log_buf + (pos++ * DRM_PANIC_LOG_LINE);
+		if (pos == DRM_PANIC_LOG_ENTRIES)
+			pos = 0;
+		if (*text == '\0')
+			continue;
+		seq_puts(m, text);
+	}
+
+	return 0;
+}
+
+static int drm_panic_log_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, drm_panic_log_show, NULL);
+}
+
+static const struct file_operations drm_panic_log_ops = {
+	.owner   = THIS_MODULE,
+	.open    = drm_panic_log_open,
+	.read    = seq_read,
+	.llseek  = seq_lseek,
+	.release = single_release,
+};
+
+/*
+ * Fake/simulate panic() at different levels:
+ * 1: only trigger panic handling internally
+ * 2: add local_irq_disable()
+ * 3: add bust_spinlocks();
+ * 100: don't fake it, do call panic()
+ */
+static int drm_text_fake_panic(unsigned int level)
+{
+#ifndef MODULE
+	int old_loglevel = console_loglevel;
+#endif
+
+	if (!level && level != 100 && level > 3)
+		return -EINVAL;
+
+	if (level == 100)
+		panic("TESTING");
+
+	if (level > 1)
+		local_irq_disable();
+
+#ifndef MODULE
+	console_verbose();
+#endif
+	if (level > 2)
+		bust_spinlocks(1);
+
+	pr_emerg("Kernel panic - not syncing: FAKING=%u, oops_in_progress=%d\n",
+		 level, oops_in_progress);
+
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+	dump_stack();
+#endif
+	/* simulate calling panic_notifier_list */
+	drm_panic_handler(NULL, 0, NULL);
+
+	if (level > 2)
+		bust_spinlocks(0);
+
+#ifndef MODULE
+	console_flush_on_panic();
+#endif
+	pr_emerg("---[ end Kernel panic - not syncing: FAKING\n");
+
+	if (level > 1)
+		local_irq_enable();
+
+#ifndef MODULE
+	console_loglevel = old_loglevel;
+#endif
+
+	return 0;
+}
+
+static ssize_t drm_text_panic_write(struct file *file,
+				    const char __user *user_buf,
+				    size_t count, loff_t *ppos)
+{
+	unsigned long long val;
+	ssize_t ret = 0;
+	char buf[24];
+	size_t size;
+
+	size = min(sizeof(buf) - 1, count);
+	if (copy_from_user(buf, user_buf, size))
+		return -EFAULT;
+
+	buf[size] = '\0';
+	ret = kstrtoull(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = drm_text_fake_panic(val);
+
+	return ret < 0 ? ret : count;
+}
+
+static const struct file_operations drm_text_panic_ops = {
+	.write =        drm_text_panic_write,
+	.open =         simple_open,
+	.llseek =       default_llseek,
+};
+
+static int drm_panic_logfs_init(void)
+{
+	drm_panic_logfs_root = debugfs_create_dir("drm-panic", NULL);
+	if (!drm_panic_logfs_root)
+		return -ENOMEM;
+
+	if (!debugfs_create_file("log", S_IRUGO, drm_panic_logfs_root, NULL,
+			    &drm_panic_log_ops))
+		goto err_remove;
+
+	log_buf = kzalloc(DRM_PANIC_LOG_SIZE, GFP_KERNEL);
+	if (!log_buf)
+		goto err_remove;
+
+	debugfs_create_file("panic", S_IWUSR, drm_panic_logfs_root, NULL,
+			    &drm_text_panic_ops);
+
+	return 0;
+
+err_remove:
+	debugfs_remove_recursive(drm_panic_logfs_root);
+
+	return -ENOMEM;
+}
+
+static void drm_panic_logfs_exit(void)
+{
+	debugfs_remove_recursive(drm_panic_logfs_root);
+	kfree(log_buf);
+	log_buf = NULL;
+}
+
+#else
+
+static int drm_panic_logfs_init(void)
+{
+}
+
+static void drm_panic_logfs_exit(void)
+{
+}
+
+static void drm_panic_log(const char *fmt, ...)
+{
+}
+
+#endif
+
+
+void __init drm_panic_init(void)
+{
+	drm_panic_kmsgs = kzalloc(DRM_PANIC_MAX_KMSGS, GFP_KERNEL);
+	if (!drm_panic_kmsgs) {
+		DRM_ERROR("Failed to setup panic handler\n");
+		return;
+	}
+
+	drm_panic_logfs_init();
+drm_panic_log("%s\n", __func__);
+	register_console(&drm_panic_console);
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &drm_panic_block);
+}
+
+void __exit drm_panic_exit(void)
+{
+	if (!drm_panic_kmsgs)
+		return;
+
+	drm_panic_logfs_exit();
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &drm_panic_block);
+	unregister_console(&drm_panic_console);
+	kfree(drm_panic_kmsgs);
+}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index bc7006e..4e84654 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -550,6 +550,28 @@ struct drm_driver {
 			  bool from_open);
 	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
 
+	/**
+	 * @panic:
+	 *
+	 * This function is called on panic() asking for a framebuffer to
+	 * display the panic messages on. It also needs the virtual address
+	 * of the backing buffer.
+	 * This function is optional.
+	 *
+	 * NOTE:
+	 *
+	 * This function is called in an atomic notifier chain and it cannot
+	 * sleep. Care must be taken so the machine is not killed even harder,
+	 * preventing output from going out on serial/netconsole.
+	 *
+	 * RETURNS:
+	 *
+	 * Framebuffer that should be used to display the panic messages,
+	 * alongside the virtual address of the backing buffer, or NULL if
+	 * the driver is unable to provide this.
+	 */
+	struct drm_framebuffer *(*panic)(struct drm_device *dev, void **vmem);
+
 	int (*debugfs_init)(struct drm_minor *minor);
 	void (*debugfs_cleanup)(struct drm_minor *minor);
 
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 3/3] drm: simpledrm: Add panic handling

[Noralf Trønnes]

This enables panic message output support in simpledrm.
simpledrm has a fixed buffer that is set up to be scanned out
and the virtual address is already available.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/simpledrm/simpledrm_drv.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
index a329e4c..ceff617 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c
+++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c
@@ -23,6 +23,29 @@
 #include <drm/drmP.h>
 #include "simpledrm.h"
 
+static struct drm_framebuffer sdrm_panic_fb;
+
+struct drm_framebuffer *sdrm_panic(struct drm_device *dev, void **vmem)
+{
+	struct sdrm_device *sdrm = dev->dev_private;
+	struct drm_framebuffer *fb = &sdrm_panic_fb;
+
+	if (!sdrm)
+		return NULL;
+
+	fb->dev = dev;
+	fb->width = sdrm->fb_width;
+	fb->height = sdrm->fb_height;
+	fb->pixel_format = sdrm->fb_format;
+	drm_fb_get_bpp_depth(fb->pixel_format, &fb->depth,
+			     &fb->bits_per_pixel);
+	fb->pitches[0] = sdrm->fb_stride;
+
+	*vmem = sdrm->fb_map;
+
+	return fb;
+}
+
 static const struct file_operations sdrm_drm_fops = {
 	.owner = THIS_MODULE,
 	.open = drm_open,
@@ -42,6 +65,7 @@ static struct drm_driver sdrm_drm_driver = {
 			   DRIVER_ATOMIC,
 	.fops = &sdrm_drm_fops,
 	.lastclose = sdrm_lastclose,
+	.panic = sdrm_panic,
 
 	.gem_free_object = sdrm_gem_free_object,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
-- 
2.8.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/3] drm: Add a way to iterate over minors

[Daniel Vetter]

On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote:
> This adds a way for in-kernel users to iterate over the available
> DRM minors. The implementation is oops safe so the panic code
> can safely use it.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Why iterate over minors? I'd kinda have expected we'd iterate over devices
instead ... And looking ahead, that seems to be what you actually want.
-Daniel

> ---
>  drivers/gpu/drm/drm_drv.c | 30 ++++++++++++++++++++++++++++++
>  include/drm/drmP.h        | 13 +++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index be27ed3..3b14366 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -292,6 +292,36 @@ void drm_minor_release(struct drm_minor *minor)
>  }
>  
>  /**
> + * drm_minor_lookup - Lookup DRM minor
> + * @minor_id: Minor ID of the DRM-minor
> + *
> + * Looks up the given minor-ID and returns the respective DRM-minor object.
> + * No reference is taken on the underlying device.
> + * See drm_minor_for_each() for iterating over all minors.
> + *
> + * Returns:
> + * Pointer to minor-object or NULL.
> + */
> +struct drm_minor *drm_minor_lookup(unsigned int minor_id)
> +{
> +	struct drm_minor *minor;
> +	unsigned long flags;
> +	int locked = 1;
> +
> +	if (oops_in_progress)
> +		locked = spin_trylock_irqsave(&drm_minor_lock, flags);
> +	else
> +		spin_lock_irqsave(&drm_minor_lock, flags);
> +
> +	minor = idr_find(&drm_minors_idr, minor_id);
> +
> +	if (locked)
> +		spin_unlock_irqrestore(&drm_minor_lock, flags);
> +
> +	return minor;
> +}
> +
> +/**
>   * DOC: driver instance overview
>   *
>   * A device instance for a drm driver is represented by struct &drm_device. This
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index d377865..bc7006e 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -974,6 +974,19 @@ void drm_dev_unregister(struct drm_device *dev);
>  
>  struct drm_minor *drm_minor_acquire(unsigned int minor_id);
>  void drm_minor_release(struct drm_minor *minor);
> +struct drm_minor *drm_minor_lookup(unsigned int minor_id);
> +
> +/**
> + * drm_minor_for_each - Iterate over DRM minors
> + * @minor: DRM minor handle
> + * @type: DRM minor type to iterate over
> + * @id: id handle
> + *
> + * Iterate over the registered DRM minors of a given type.
> + */
> +#define drm_minor_for_each(minor, type, id)  \
> +	for ((id) = 0; (id) < 64; (id)++)  \
> +		if (((minor) = drm_minor_lookup((id) + (type) * 64)))
>  
>  /*@}*/
>  
> -- 
> 2.8.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 2/3] drm: Add panic handling

[Daniel Vetter]

On Tue, Aug 09, 2016 at 07:45:41PM +0200, Noralf Trønnes wrote:
> This adds support for outputting kernel messages on panic().
> The drivers that supports it, provides a framebuffer that the
> messages can be rendered on.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Thinking about how we should implement this in a full-blown driver, I
think we will need a bit more than this. Here's the additional
requirements I've come up that a driver more complex than sdrm would need
for reliable panic handling:

- Multiple outputs, with multiple framebuffer (or one framebuffer and
  multiple offsets within). And since one display might be dead we should
  try really hard to show the oops on all of them. Consequence: I think we
  need to also loop over all drm_crtc in a drm_device, to be able to
  support them all.

- With desktop gpus you pretty much always end up with a tiled
  framebuffer. And from an oops context it's going to be impossible to
  detile that quickly. I think for this we need a helper to draw x/y
  pixels (it's going to be dead slow, but meh), with a default
  implementation which assumes linear layout. Drivers could then frob x/y
  coordinates first in their own logic and call into that function.

- There's a good chance you're showing a video on a yuv buffer
  full-screen. If we don't bother too much with what it looks like, but
  only care about a foreground/background color then it's also easy to
  implement a draw_xy_pixel for these framebuffers. That means though that
  you can't clear things with memset, but that you need to call the
  interface func for each pixel. Yes this is going to be dead slow, but
  who cares as long as the oops eventually shows up ;-)

- The framebuffer size doesn't necessarily match the size of the visible
  part. Probably best if we dig this out from the plane_state directly
  (less fragile code in drivers). Relying on drm_plane_state means this
  will only work on atomic drivers (in legacy drivers this information is
  hidden in driver-private corners), but I think that's totally ok.

- 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).

- Multiple planes which might occlude the primary plane: I think we should
  just bother with the primary plane, and maybe give drivers a
  panic_commit hook where they could try to disable any additional planes.
  But that's not something we need in the first version here at all.

- I think some helpers for creating the vmap would be nice. Should be
  simple for cma-backed framebuffers, and also simple if you use a normal
  shmem backed gem buffer. For cma probably best if drivers don't even
  need to bother with this.

- For driver convenience I think we could check a few things about
  visibility of each plane (e.g. crtc_state->active), and skip if there's
  no chance it can be seen. Doing less means less chances to blow up, and
  higher chances that the one screen which is on actually ends up with the
  oops on it.

- Minimal cleanup. I think we can just leak the vmapping, no harm in that.
  But the kms locks need to be dropped again. Dropping them again might
  increase the odds that the system limps along long enough for logs to
  hit the disks.

Taking this all together, I think the driver interface should be
restructured a bit, and that most of the handling should be on the
drm_framebuffer directly:

struct drm_framebuffer_funcs {
	/* For vmapping the selected framebuffer in a panic context. Must
	 * be super careful about locking (only trylocking allowed), can
	 * return NULL if it didn't work out. The return value is an
	 * opaque cookie which is passed to @panic_draw_xy, it can be
	 * anything: vmap area, structure with more details, just a few
	 * flags, ...
	 */
	void *(panic_vmap)(struct drm_framebuffer *fb);

	/* For drawing pixels onto a framebuffer mapping with @panic_vmap.
	 * This is optional, the default implementation assumes that vmap
	 * points at a linear mapping of the framebuffer.
	 */
	void (panic_draw_xy)(struct drm_framebuffer *fb, void *vmap,
			     int x, int y, bool foreground);
};

Ofc comments need to be fleshed out some more, but the idea is that all
the fb selection and lookup is handled in shared code (and with proper
locking, but only for atomic drivers).

Thoughts?
-Daniel

> ---
>  drivers/gpu/drm/Makefile       |   2 +-
>  drivers/gpu/drm/drm_drv.c      |   3 +
>  drivers/gpu/drm/drm_internal.h |   4 +
>  drivers/gpu/drm/drm_panic.c    | 606 +++++++++++++++++++++++++++++++++++++++++
>  include/drm/drmP.h             |  22 ++
>  5 files changed, 636 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_panic.c
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index eba32ad..ff04e41 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -12,7 +12,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
>  		drm_info.o drm_debugfs.o drm_encoder_slave.o \
>  		drm_trace_points.o drm_global.o drm_prime.o \
>  		drm_rect.o drm_vma_manager.o drm_flip_work.o \
> -		drm_modeset_lock.o drm_atomic.o drm_bridge.o
> +		drm_modeset_lock.o drm_atomic.o drm_bridge.o drm_panic.o
>  
>  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 3b14366..457ee91 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -861,6 +861,8 @@ static int __init drm_core_init(void)
>  		goto err_p3;
>  	}
>  
> +	drm_panic_init();
> +
>  	DRM_INFO("Initialized %s %d.%d.%d %s\n",
>  		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
>  	return 0;
> @@ -876,6 +878,7 @@ err_p1:
>  
>  static void __exit drm_core_exit(void)
>  {
> +	drm_panic_exit();
>  	debugfs_remove(drm_debugfs_root);
>  	drm_sysfs_destroy();
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index b86dc9b..7463d9d 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -90,6 +90,10 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
>  void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
>  void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
>  
> +/* drm_panic.c */
> +void drm_panic_init(void);
> +void drm_panic_exit(void);
> +
>  /* drm_debugfs.c */
>  #if defined(CONFIG_DEBUG_FS)
>  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> new file mode 100644
> index 0000000..e185c9d
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -0,0 +1,606 @@
> +/*
> + * Copyright 2016 Noralf Trønnes
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <asm/unaligned.h>
> +#include <drm/drmP.h>
> +#include <linux/console.h>
> +#include <linux/debugfs.h>
> +#include <linux/font.h>
> +#include <linux/kernel.h>
> +#include <linux/seq_file.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +
> +struct drm_panic_fb {
> +	struct drm_framebuffer *fb;
> +	void *vmem;
> +	const struct font_desc *font;
> +	unsigned int cols;
> +	unsigned int rows;
> +	unsigned int xpos;
> +	unsigned int ypos;
> +};
> +
> +#define DRM_PANIC_MAX_FBS	64
> +static struct drm_panic_fb drm_panic_fbs[DRM_PANIC_MAX_FBS];
> +
> +#define DRM_PANIC_MAX_KMSGS	SZ_4K
> +static char *drm_panic_kmsgs;
> +static size_t drm_panic_kmsgs_pos;
> +
> +static bool drm_panic_active;
> +
> +static void drm_panic_log(const char *fmt, ...);
> +
> +static inline void drm_panic_draw_pixel(u8 *dst, u32 pixel_format, bool val)
> +{
> +	switch (pixel_format & ~DRM_FORMAT_BIG_ENDIAN) {
> +
> +	case DRM_FORMAT_C8:
> +	case DRM_FORMAT_RGB332:
> +	case DRM_FORMAT_BGR233:
> +		*dst = val ? 0xff : 0x00;
> +		break;
> +
> +	case DRM_FORMAT_XRGB4444:
> +	case DRM_FORMAT_ARGB4444:
> +	case DRM_FORMAT_XBGR4444:
> +	case DRM_FORMAT_ABGR4444:
> +		put_unaligned(val ? 0x0fff : 0x0000, (u16 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_RGBX4444:
> +	case DRM_FORMAT_RGBA4444:
> +	case DRM_FORMAT_BGRX4444:
> +	case DRM_FORMAT_BGRA4444:
> +		put_unaligned(val ? 0xfff0 : 0x0000, (u16 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_XRGB1555:
> +	case DRM_FORMAT_ARGB1555:
> +	case DRM_FORMAT_XBGR1555:
> +	case DRM_FORMAT_ABGR1555:
> +		put_unaligned(val ? 0x7fff : 0x0000, (u16 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_RGBX5551:
> +	case DRM_FORMAT_RGBA5551:
> +	case DRM_FORMAT_BGRX5551:
> +	case DRM_FORMAT_BGRA5551:
> +		put_unaligned(val ? 0xfffe : 0x0000, (u16 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_RGB565:
> +	case DRM_FORMAT_BGR565:
> +		put_unaligned(val ? 0xffff : 0x0000, (u16 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_RGB888:
> +	case DRM_FORMAT_BGR888:
> +		dst[0] = val ? 0xff : 0x00;
> +		dst[1] = val ? 0xff : 0x00;
> +		dst[2] = val ? 0xff : 0x00;
> +		break;
> +
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_ARGB8888:
> +	case DRM_FORMAT_XBGR8888:
> +	case DRM_FORMAT_ABGR8888:
> +		put_unaligned(val ? 0x00ffffff : 0x00000000, (u32 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_RGBX8888:
> +	case DRM_FORMAT_RGBA8888:
> +	case DRM_FORMAT_BGRX8888:
> +	case DRM_FORMAT_BGRA8888:
> +		put_unaligned(val ? 0xffffff00 : 0x00000000, (u32 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_XRGB2101010:
> +	case DRM_FORMAT_ARGB2101010:
> +	case DRM_FORMAT_XBGR2101010:
> +	case DRM_FORMAT_ABGR2101010:
> +		put_unaligned(val ? 0x3fffffff : 0x00000000, (u32 *)dst);
> +		break;
> +
> +	case DRM_FORMAT_RGBX1010102:
> +	case DRM_FORMAT_RGBA1010102:
> +	case DRM_FORMAT_BGRX1010102:
> +	case DRM_FORMAT_BGRA1010102:
> +		put_unaligned(val ? 0xfffffffc : 0x00000000, (u32 *)dst);
> +		break;
> +	}
> +}
> +
> +static void drm_panic_render(struct drm_panic_fb *pfb,
> +			     const char *text, unsigned int len)
> +{
> +	const struct font_desc *font = pfb->font;
> +	unsigned int pix_depth, pix_bpp, cpp;
> +	unsigned int col = pfb->xpos;
> +	unsigned int row = pfb->ypos;
> +	unsigned int i, h, w;
> +	void *dst, *pos;
> +	u8 fontline;
> +
> +	if ((row + 1) * font->height > pfb->fb->height)
> +		return;
> +
> +	if ((col + len) * font->width > pfb->fb->width)
> +		return;
> +
> +	drm_fb_get_bpp_depth(pfb->fb->pixel_format, &pix_depth, &pix_bpp);
> +	cpp = DIV_ROUND_UP(pix_bpp, 8);
> +
> +	/* TODO: should fb->offsets[0] be added here? */
> +	dst = pfb->vmem + (row * font->height * pfb->fb->pitches[0]) +
> +	      (col * font->width * cpp);
> +
> +	for (h = 0; h < font->height; h++) {
> +		pos = dst;
> +
> +		for (i = 0; i < len; i++) {
> +			fontline = *(u8 *)(font->data + text[i] * font->height + h);
> +
> +			for (w = 0; w < font->width; w++) {
> +				drm_panic_draw_pixel(pos, pfb->fb->pixel_format,
> +						     fontline & BIT(7 - w));
> +				pos += cpp;
> +			}
> +		}
> +
> +		dst += pfb->fb->pitches[0];
> +	}
> +}
> +
> +static void drm_panic_scroll_up(struct drm_panic_fb *pfb)
> +{
> +	void *src = pfb->vmem + (pfb->font->height * pfb->fb->pitches[0]);
> +	size_t len = (pfb->fb->height - pfb->font->height) *
> +		     pfb->fb->pitches[0];
> +
> +	drm_panic_log("%s\n", __func__);
> +
> +	memmove(pfb->vmem, src, len);
> +	memset(pfb->vmem + len, 0, pfb->font->height * pfb->fb->pitches[0]);
> +}
> +
> +static void drm_panic_clear_screen(struct drm_panic_fb *pfb)
> +{
> +	memset(pfb->vmem, 0, pfb->fb->height * pfb->fb->pitches[0]);
> +}
> +
> +static void drm_panic_log_msg(char *pre, const char *str, unsigned int len)
> +{
> +	char buf[512];
> +
> +	if (len > 510)
> +		len = 510;
> +
> +	memcpy(buf, str, len);
> +	buf[len] = '\n';
> +	buf[len + 1] = '\0';
> +
> +	drm_panic_log("%s%s", pre, buf);
> +}
> +
> +static void drm_panic_putcs_no_lf(struct drm_panic_fb *pfb,
> +				  const char *str, unsigned int len)
> +{
> +	drm_panic_log("%s(len=%u) x=%u, y=%u\n", __func__, len,
> +			pfb->xpos, pfb->ypos);
> +
> +	if (len <= 0)
> +		return;
> +
> +	drm_panic_log_msg("", str, len);
> +
> +	drm_panic_render(pfb, str, len);
> +
> +}
> +
> +static void drm_panic_putcs(struct drm_panic_fb *pfb,
> +			    const char *str, unsigned int num)
> +{
> +	unsigned int slen;
> +	int len = num;
> +	char *lf;
> +
> +	drm_panic_log("%s(num=%u)\n", __func__, num);
> +
> +	while (len > 0) {
> +
> +		if (pfb->ypos == pfb->rows) {
> +			pfb->ypos--;
> +			drm_panic_scroll_up(pfb);
> +		}
> +
> +		lf = strpbrk(str, "\n");
> +		if (lf)
> +			slen = lf - str;
> +		else
> +			slen = len;
> +
> +		if (pfb->xpos + slen > pfb->cols)
> +			slen = pfb->cols - pfb->xpos;
> +
> +		drm_panic_putcs_no_lf(pfb, str, slen);
> +
> +		len -= slen;
> +		str += slen;
> +		pfb->xpos += slen;
> +
> +		if (lf) {
> +			str++;
> +			len--;
> +			pfb->xpos = 0;
> +			pfb->ypos++;
> +		}
> +	}
> +}
> +
> +static void drm_panic_write(const char *str, unsigned int num)
> +{
> +	unsigned int i;
> +
> +	if (!num)
> +		return;
> +
> +	drm_panic_log("%s(num=%u)\n", __func__, num);
> +
> +	for (i = 0; i < DRM_PANIC_MAX_FBS; i++) {
> +		if (!drm_panic_fbs[i].fb)
> +			break;
> +		drm_panic_putcs(&drm_panic_fbs[i], str, num);
> +	}
> +}
> +
> +/* this one is serialized by console_lock() */
> +static void drm_panic_console_write(struct console *con,
> +				    const char *str, unsigned int num)
> +{
> +	unsigned int i;
> +
> +	drm_panic_log_msg("->", str, num);
> +
> +	/* Buffer up messages to be replayed on panic */
> +	if (!drm_panic_active) {
> +		for (i = 0; i < num; i++) {
> +			drm_panic_kmsgs[drm_panic_kmsgs_pos++] = *str++;
> +			if (drm_panic_kmsgs_pos == DRM_PANIC_MAX_KMSGS)
> +				drm_panic_kmsgs_pos = 0;
> +		}
> +		return;
> +	}
> +
> +	drm_panic_write(str, num);
> +}
> +
> +static struct console drm_panic_console = {
> +	.name =         "drmpanic",
> +	.write =        drm_panic_console_write,
> +	.flags =        CON_PRINTBUFFER | CON_ENABLED,
> +	.index =        0,
> +};
> +
> +/*
> + * The panic() function makes sure that only one CPU is allowed to run it's
> + * code. So when this handler is called, we're alone. No racing with
> + * console.write() is possible.
> + */
> +static int drm_panic_handler(struct notifier_block *this, unsigned long ev,
> +			     void *ptr)
> +{
> +	const struct font_desc *font;
> +	struct drm_framebuffer *fb;
> +	struct drm_panic_fb *pfb;
> +	struct drm_minor *minor;
> +	unsigned int fbs = 0;
> +	void *vmem;
> +	int i;
> +
> +	drm_panic_log("%s\n", __func__);
> +
> +	drm_panic_active = true;
> +
> +	drm_minor_for_each(minor, DRM_MINOR_LEGACY, i) {
> +		drm_panic_log("Found minor=%d\n", minor->index);
> +		if (!minor->dev || !minor->dev->driver ||
> +		    !minor->dev->driver->panic) {
> +			drm_panic_log("Skipping: No panic handler\n");
> +			continue;
> +		}
> +
> +		fb = minor->dev->driver->panic(minor->dev, &vmem);
> +		if (!fb) {
> +			drm_panic_log("Skipping: Driver returned NULL\n");
> +			continue;
> +		}
> +
> +		if (!fb || !vmem || fb->dev != minor->dev || !fb->pitches[0]) {
> +			drm_panic_log("Skipping: Failed check\n");
> +			continue;
> +		}
> +
> +		/* only 8-bit wide fonts are supported */
> +		font = get_default_font(fb->width, fb->height, BIT(7), -1);
> +		if (!font) {
> +			drm_panic_log("Skipping: No font available\n");
> +			continue;
> +		}
> +
> +		pfb = &drm_panic_fbs[fbs++];
> +
> +		pfb->fb = fb;
> +		pfb->vmem = vmem;
> +		pfb->font = font;
> +		pfb->cols = fb->width / font->width;
> +		pfb->rows = fb->height / font->height;
> +
> +		drm_panic_clear_screen(pfb);
> +
> +		drm_panic_log("    %ux%u -> %ux%u, %s, %s\n", fb->width,
> +				fb->height, pfb->cols, pfb->rows, font->name,
> +				drm_get_format_name(fb->pixel_format));
> +	}
> +
> +	if (drm_panic_kmsgs[0]) {
> +		/* safeguard in case we interrupted drm_panic_console_write */
> +		if (drm_panic_kmsgs_pos >= DRM_PANIC_MAX_KMSGS)
> +			drm_panic_kmsgs_pos = 0;
> +
> +		drm_panic_write(&drm_panic_kmsgs[drm_panic_kmsgs_pos],
> +				DRM_PANIC_MAX_KMSGS - drm_panic_kmsgs_pos);
> +		drm_panic_write(drm_panic_kmsgs, drm_panic_kmsgs_pos);
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block drm_panic_block = {
> +	.notifier_call = drm_panic_handler,
> +};
> +
> +
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +/* Out of band logging is useful at least in the initial development phase */
> +#define DRM_PANIC_LOG_SIZE	SZ_64K
> +#define DRM_PANIC_LOG_LINE	128
> +#define DRM_PANIC_LOG_ENTRIES	(DRM_PANIC_LOG_SIZE / DRM_PANIC_LOG_LINE)
> +
> +static char *log_buf;
> +static size_t log_pos;
> +static struct dentry *drm_panic_logfs_root;
> +
> +static void drm_panic_log(const char *fmt, ...)
> +{
> +	va_list args;
> +	u32 rem_nsec;
> +	char *text;
> +	size_t len;
> +	u64 sec;
> +
> +	if (!log_buf || oops_in_progress)
> +		return;
> +
> +	va_start(args, fmt);
> +
> +	if (log_pos >= DRM_PANIC_LOG_ENTRIES)
> +		log_pos = 0;
> +
> +	text = log_buf + (log_pos++ * DRM_PANIC_LOG_LINE);
> +	if (log_pos == DRM_PANIC_LOG_ENTRIES)
> +		log_pos = 0;
> +
> +	sec = div_u64_rem(local_clock(), 1000000000, &rem_nsec);
> +
> +	len = scnprintf(text, DRM_PANIC_LOG_LINE, "[%5llu.%06u] ", sec,
> +			rem_nsec / 1000);
> +
> +	vscnprintf(text + len, DRM_PANIC_LOG_LINE - len, fmt, args);
> +
> +	/* Make sure to always have a newline in case of overflow */
> +	if (text[DRM_PANIC_LOG_LINE - 2] != '\0')
> +		text[DRM_PANIC_LOG_LINE - 2] = '\n';
> +
> +	va_end(args);
> +}
> +
> +static int drm_panic_log_show(struct seq_file *m, void *v)
> +{
> +	size_t pos = log_pos;
> +	unsigned int i;
> +	char *text;
> +
> +	for (i = 0; i < DRM_PANIC_LOG_ENTRIES; i++) {
> +		text = log_buf + (pos++ * DRM_PANIC_LOG_LINE);
> +		if (pos == DRM_PANIC_LOG_ENTRIES)
> +			pos = 0;
> +		if (*text == '\0')
> +			continue;
> +		seq_puts(m, text);
> +	}
> +
> +	return 0;
> +}
> +
> +static int drm_panic_log_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, drm_panic_log_show, NULL);
> +}
> +
> +static const struct file_operations drm_panic_log_ops = {
> +	.owner   = THIS_MODULE,
> +	.open    = drm_panic_log_open,
> +	.read    = seq_read,
> +	.llseek  = seq_lseek,
> +	.release = single_release,
> +};
> +
> +/*
> + * Fake/simulate panic() at different levels:
> + * 1: only trigger panic handling internally
> + * 2: add local_irq_disable()
> + * 3: add bust_spinlocks();
> + * 100: don't fake it, do call panic()
> + */
> +static int drm_text_fake_panic(unsigned int level)
> +{
> +#ifndef MODULE
> +	int old_loglevel = console_loglevel;
> +#endif
> +
> +	if (!level && level != 100 && level > 3)
> +		return -EINVAL;
> +
> +	if (level == 100)
> +		panic("TESTING");
> +
> +	if (level > 1)
> +		local_irq_disable();
> +
> +#ifndef MODULE
> +	console_verbose();
> +#endif
> +	if (level > 2)
> +		bust_spinlocks(1);
> +
> +	pr_emerg("Kernel panic - not syncing: FAKING=%u, oops_in_progress=%d\n",
> +		 level, oops_in_progress);
> +
> +#ifdef CONFIG_DEBUG_BUGVERBOSE
> +	dump_stack();
> +#endif
> +	/* simulate calling panic_notifier_list */
> +	drm_panic_handler(NULL, 0, NULL);
> +
> +	if (level > 2)
> +		bust_spinlocks(0);
> +
> +#ifndef MODULE
> +	console_flush_on_panic();
> +#endif
> +	pr_emerg("---[ end Kernel panic - not syncing: FAKING\n");
> +
> +	if (level > 1)
> +		local_irq_enable();
> +
> +#ifndef MODULE
> +	console_loglevel = old_loglevel;
> +#endif
> +
> +	return 0;
> +}
> +
> +static ssize_t drm_text_panic_write(struct file *file,
> +				    const char __user *user_buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	unsigned long long val;
> +	ssize_t ret = 0;
> +	char buf[24];
> +	size_t size;
> +
> +	size = min(sizeof(buf) - 1, count);
> +	if (copy_from_user(buf, user_buf, size))
> +		return -EFAULT;
> +
> +	buf[size] = '\0';
> +	ret = kstrtoull(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_text_fake_panic(val);
> +
> +	return ret < 0 ? ret : count;
> +}
> +
> +static const struct file_operations drm_text_panic_ops = {
> +	.write =        drm_text_panic_write,
> +	.open =         simple_open,
> +	.llseek =       default_llseek,
> +};
> +
> +static int drm_panic_logfs_init(void)
> +{
> +	drm_panic_logfs_root = debugfs_create_dir("drm-panic", NULL);
> +	if (!drm_panic_logfs_root)
> +		return -ENOMEM;
> +
> +	if (!debugfs_create_file("log", S_IRUGO, drm_panic_logfs_root, NULL,
> +			    &drm_panic_log_ops))
> +		goto err_remove;
> +
> +	log_buf = kzalloc(DRM_PANIC_LOG_SIZE, GFP_KERNEL);
> +	if (!log_buf)
> +		goto err_remove;
> +
> +	debugfs_create_file("panic", S_IWUSR, drm_panic_logfs_root, NULL,
> +			    &drm_text_panic_ops);
> +
> +	return 0;
> +
> +err_remove:
> +	debugfs_remove_recursive(drm_panic_logfs_root);
> +
> +	return -ENOMEM;
> +}
> +
> +static void drm_panic_logfs_exit(void)
> +{
> +	debugfs_remove_recursive(drm_panic_logfs_root);
> +	kfree(log_buf);
> +	log_buf = NULL;
> +}
> +
> +#else
> +
> +static int drm_panic_logfs_init(void)
> +{
> +}
> +
> +static void drm_panic_logfs_exit(void)
> +{
> +}
> +
> +static void drm_panic_log(const char *fmt, ...)
> +{
> +}
> +
> +#endif
> +
> +
> +void __init drm_panic_init(void)
> +{
> +	drm_panic_kmsgs = kzalloc(DRM_PANIC_MAX_KMSGS, GFP_KERNEL);
> +	if (!drm_panic_kmsgs) {
> +		DRM_ERROR("Failed to setup panic handler\n");
> +		return;
> +	}
> +
> +	drm_panic_logfs_init();
> +drm_panic_log("%s\n", __func__);
> +	register_console(&drm_panic_console);
> +	atomic_notifier_chain_register(&panic_notifier_list,
> +				       &drm_panic_block);
> +}
> +
> +void __exit drm_panic_exit(void)
> +{
> +	if (!drm_panic_kmsgs)
> +		return;
> +
> +	drm_panic_logfs_exit();
> +	atomic_notifier_chain_unregister(&panic_notifier_list,
> +					 &drm_panic_block);
> +	unregister_console(&drm_panic_console);
> +	kfree(drm_panic_kmsgs);
> +}
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index bc7006e..4e84654 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -550,6 +550,28 @@ struct drm_driver {
>  			  bool from_open);
>  	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
>  
> +	/**
> +	 * @panic:
> +	 *
> +	 * This function is called on panic() asking for a framebuffer to
> +	 * display the panic messages on. It also needs the virtual address
> +	 * of the backing buffer.
> +	 * This function is optional.
> +	 *
> +	 * NOTE:
> +	 *
> +	 * This function is called in an atomic notifier chain and it cannot
> +	 * sleep. Care must be taken so the machine is not killed even harder,
> +	 * preventing output from going out on serial/netconsole.
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * Framebuffer that should be used to display the panic messages,
> +	 * alongside the virtual address of the backing buffer, or NULL if
> +	 * the driver is unable to provide this.
> +	 */
> +	struct drm_framebuffer *(*panic)(struct drm_device *dev, void **vmem);
> +
>  	int (*debugfs_init)(struct drm_minor *minor);
>  	void (*debugfs_cleanup)(struct drm_minor *minor);
>  
> -- 
> 2.8.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 2/3] drm: Add panic handling

[Daniel Vetter]

On Wed, Aug 10, 2016 at 11:15:29AM +0200, Daniel Vetter wrote:
> On Tue, Aug 09, 2016 at 07:45:41PM +0200, Noralf Trønnes wrote:
> > This adds support for outputting kernel messages on panic().
> > The drivers that supports it, provides a framebuffer that the
> > messages can be rendered on.
> > 
> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> 
> Thinking about how we should implement this in a full-blown driver, I
> think we will need a bit more than this. Here's the additional
> requirements I've come up that a driver more complex than sdrm would need
> for reliable panic handling:
> 
> - Multiple outputs, with multiple framebuffer (or one framebuffer and
>   multiple offsets within). And since one display might be dead we should
>   try really hard to show the oops on all of them. Consequence: I think we
>   need to also loop over all drm_crtc in a drm_device, to be able to
>   support them all.
> 
> - With desktop gpus you pretty much always end up with a tiled
>   framebuffer. And from an oops context it's going to be impossible to
>   detile that quickly. I think for this we need a helper to draw x/y
>   pixels (it's going to be dead slow, but meh), with a default
>   implementation which assumes linear layout. Drivers could then frob x/y
>   coordinates first in their own logic and call into that function.
> 
> - There's a good chance you're showing a video on a yuv buffer
>   full-screen. If we don't bother too much with what it looks like, but
>   only care about a foreground/background color then it's also easy to
>   implement a draw_xy_pixel for these framebuffers. That means though that
>   you can't clear things with memset, but that you need to call the
>   interface func for each pixel. Yes this is going to be dead slow, but
>   who cares as long as the oops eventually shows up ;-)
> 
> - The framebuffer size doesn't necessarily match the size of the visible
>   part. Probably best if we dig this out from the plane_state directly
>   (less fragile code in drivers). Relying on drm_plane_state means this
>   will only work on atomic drivers (in legacy drivers this information is
>   hidden in driver-private corners), but I think that's totally ok.
> 
> - 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).
> 
> - Multiple planes which might occlude the primary plane: I think we should
>   just bother with the primary plane, and maybe give drivers a
>   panic_commit hook where they could try to disable any additional planes.
>   But that's not something we need in the first version here at all.
> 
> - I think some helpers for creating the vmap would be nice. Should be
>   simple for cma-backed framebuffers, and also simple if you use a normal
>   shmem backed gem buffer. For cma probably best if drivers don't even
>   need to bother with this.
> 
> - For driver convenience I think we could check a few things about
>   visibility of each plane (e.g. crtc_state->active), and skip if there's
>   no chance it can be seen. Doing less means less chances to blow up, and
>   higher chances that the one screen which is on actually ends up with the
>   oops on it.
> 
> - Minimal cleanup. I think we can just leak the vmapping, no harm in that.
>   But the kms locks need to be dropped again. Dropping them again might
>   increase the odds that the system limps along long enough for logs to
>   hit the disks.
> 
> Taking this all together, I think the driver interface should be
> restructured a bit, and that most of the handling should be on the
> drm_framebuffer directly:
> 
> struct drm_framebuffer_funcs {
> 	/* For vmapping the selected framebuffer in a panic context. Must
> 	 * be super careful about locking (only trylocking allowed), can
> 	 * return NULL if it didn't work out. The return value is an
> 	 * opaque cookie which is passed to @panic_draw_xy, it can be
> 	 * anything: vmap area, structure with more details, just a few
> 	 * flags, ...
> 	 */
> 	void *(panic_vmap)(struct drm_framebuffer *fb);
> 
> 	/* For drawing pixels onto a framebuffer mapping with @panic_vmap.
> 	 * This is optional, the default implementation assumes that vmap
> 	 * points at a linear mapping of the framebuffer.
> 	 */
> 	void (panic_draw_xy)(struct drm_framebuffer *fb, void *vmap,
> 			     int x, int y, bool foreground);
> };
> 
> Ofc comments need to be fleshed out some more, but the idea is that all
> the fb selection and lookup is handled in shared code (and with proper
> locking, but only for atomic drivers).
> 
> Thoughts?

One more: This is quite a bit of work to get going, I estimate that it's
good enough to fill a full gsoc internship, so about 2-3 months of work,
but for someone unexperienced, unlike you ;-)
-Daniel

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/Makefile       |   2 +-
> >  drivers/gpu/drm/drm_drv.c      |   3 +
> >  drivers/gpu/drm/drm_internal.h |   4 +
> >  drivers/gpu/drm/drm_panic.c    | 606 +++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drmP.h             |  22 ++
> >  5 files changed, 636 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/gpu/drm/drm_panic.c
> > 
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index eba32ad..ff04e41 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -12,7 +12,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
> >  		drm_info.o drm_debugfs.o drm_encoder_slave.o \
> >  		drm_trace_points.o drm_global.o drm_prime.o \
> >  		drm_rect.o drm_vma_manager.o drm_flip_work.o \
> > -		drm_modeset_lock.o drm_atomic.o drm_bridge.o
> > +		drm_modeset_lock.o drm_atomic.o drm_bridge.o drm_panic.o
> >  
> >  drm-$(CONFIG_COMPAT) += drm_ioc32.o
> >  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 3b14366..457ee91 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -861,6 +861,8 @@ static int __init drm_core_init(void)
> >  		goto err_p3;
> >  	}
> >  
> > +	drm_panic_init();
> > +
> >  	DRM_INFO("Initialized %s %d.%d.%d %s\n",
> >  		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
> >  	return 0;
> > @@ -876,6 +878,7 @@ err_p1:
> >  
> >  static void __exit drm_core_exit(void)
> >  {
> > +	drm_panic_exit();
> >  	debugfs_remove(drm_debugfs_root);
> >  	drm_sysfs_destroy();
> >  
> > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> > index b86dc9b..7463d9d 100644
> > --- a/drivers/gpu/drm/drm_internal.h
> > +++ b/drivers/gpu/drm/drm_internal.h
> > @@ -90,6 +90,10 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
> >  void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
> >  void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
> >  
> > +/* drm_panic.c */
> > +void drm_panic_init(void);
> > +void drm_panic_exit(void);
> > +
> >  /* drm_debugfs.c */
> >  #if defined(CONFIG_DEBUG_FS)
> >  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> > diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> > new file mode 100644
> > index 0000000..e185c9d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_panic.c
> > @@ -0,0 +1,606 @@
> > +/*
> > + * Copyright 2016 Noralf Trønnes
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <asm/unaligned.h>
> > +#include <drm/drmP.h>
> > +#include <linux/console.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/font.h>
> > +#include <linux/kernel.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/slab.h>
> > +#include <linux/uaccess.h>
> > +
> > +struct drm_panic_fb {
> > +	struct drm_framebuffer *fb;
> > +	void *vmem;
> > +	const struct font_desc *font;
> > +	unsigned int cols;
> > +	unsigned int rows;
> > +	unsigned int xpos;
> > +	unsigned int ypos;
> > +};
> > +
> > +#define DRM_PANIC_MAX_FBS	64
> > +static struct drm_panic_fb drm_panic_fbs[DRM_PANIC_MAX_FBS];
> > +
> > +#define DRM_PANIC_MAX_KMSGS	SZ_4K
> > +static char *drm_panic_kmsgs;
> > +static size_t drm_panic_kmsgs_pos;
> > +
> > +static bool drm_panic_active;
> > +
> > +static void drm_panic_log(const char *fmt, ...);
> > +
> > +static inline void drm_panic_draw_pixel(u8 *dst, u32 pixel_format, bool val)
> > +{
> > +	switch (pixel_format & ~DRM_FORMAT_BIG_ENDIAN) {
> > +
> > +	case DRM_FORMAT_C8:
> > +	case DRM_FORMAT_RGB332:
> > +	case DRM_FORMAT_BGR233:
> > +		*dst = val ? 0xff : 0x00;
> > +		break;
> > +
> > +	case DRM_FORMAT_XRGB4444:
> > +	case DRM_FORMAT_ARGB4444:
> > +	case DRM_FORMAT_XBGR4444:
> > +	case DRM_FORMAT_ABGR4444:
> > +		put_unaligned(val ? 0x0fff : 0x0000, (u16 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_RGBX4444:
> > +	case DRM_FORMAT_RGBA4444:
> > +	case DRM_FORMAT_BGRX4444:
> > +	case DRM_FORMAT_BGRA4444:
> > +		put_unaligned(val ? 0xfff0 : 0x0000, (u16 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_XRGB1555:
> > +	case DRM_FORMAT_ARGB1555:
> > +	case DRM_FORMAT_XBGR1555:
> > +	case DRM_FORMAT_ABGR1555:
> > +		put_unaligned(val ? 0x7fff : 0x0000, (u16 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_RGBX5551:
> > +	case DRM_FORMAT_RGBA5551:
> > +	case DRM_FORMAT_BGRX5551:
> > +	case DRM_FORMAT_BGRA5551:
> > +		put_unaligned(val ? 0xfffe : 0x0000, (u16 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_RGB565:
> > +	case DRM_FORMAT_BGR565:
> > +		put_unaligned(val ? 0xffff : 0x0000, (u16 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_RGB888:
> > +	case DRM_FORMAT_BGR888:
> > +		dst[0] = val ? 0xff : 0x00;
> > +		dst[1] = val ? 0xff : 0x00;
> > +		dst[2] = val ? 0xff : 0x00;
> > +		break;
> > +
> > +	case DRM_FORMAT_XRGB8888:
> > +	case DRM_FORMAT_ARGB8888:
> > +	case DRM_FORMAT_XBGR8888:
> > +	case DRM_FORMAT_ABGR8888:
> > +		put_unaligned(val ? 0x00ffffff : 0x00000000, (u32 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_RGBX8888:
> > +	case DRM_FORMAT_RGBA8888:
> > +	case DRM_FORMAT_BGRX8888:
> > +	case DRM_FORMAT_BGRA8888:
> > +		put_unaligned(val ? 0xffffff00 : 0x00000000, (u32 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_XRGB2101010:
> > +	case DRM_FORMAT_ARGB2101010:
> > +	case DRM_FORMAT_XBGR2101010:
> > +	case DRM_FORMAT_ABGR2101010:
> > +		put_unaligned(val ? 0x3fffffff : 0x00000000, (u32 *)dst);
> > +		break;
> > +
> > +	case DRM_FORMAT_RGBX1010102:
> > +	case DRM_FORMAT_RGBA1010102:
> > +	case DRM_FORMAT_BGRX1010102:
> > +	case DRM_FORMAT_BGRA1010102:
> > +		put_unaligned(val ? 0xfffffffc : 0x00000000, (u32 *)dst);
> > +		break;
> > +	}
> > +}
> > +
> > +static void drm_panic_render(struct drm_panic_fb *pfb,
> > +			     const char *text, unsigned int len)
> > +{
> > +	const struct font_desc *font = pfb->font;
> > +	unsigned int pix_depth, pix_bpp, cpp;
> > +	unsigned int col = pfb->xpos;
> > +	unsigned int row = pfb->ypos;
> > +	unsigned int i, h, w;
> > +	void *dst, *pos;
> > +	u8 fontline;
> > +
> > +	if ((row + 1) * font->height > pfb->fb->height)
> > +		return;
> > +
> > +	if ((col + len) * font->width > pfb->fb->width)
> > +		return;
> > +
> > +	drm_fb_get_bpp_depth(pfb->fb->pixel_format, &pix_depth, &pix_bpp);
> > +	cpp = DIV_ROUND_UP(pix_bpp, 8);
> > +
> > +	/* TODO: should fb->offsets[0] be added here? */
> > +	dst = pfb->vmem + (row * font->height * pfb->fb->pitches[0]) +
> > +	      (col * font->width * cpp);
> > +
> > +	for (h = 0; h < font->height; h++) {
> > +		pos = dst;
> > +
> > +		for (i = 0; i < len; i++) {
> > +			fontline = *(u8 *)(font->data + text[i] * font->height + h);
> > +
> > +			for (w = 0; w < font->width; w++) {
> > +				drm_panic_draw_pixel(pos, pfb->fb->pixel_format,
> > +						     fontline & BIT(7 - w));
> > +				pos += cpp;
> > +			}
> > +		}
> > +
> > +		dst += pfb->fb->pitches[0];
> > +	}
> > +}
> > +
> > +static void drm_panic_scroll_up(struct drm_panic_fb *pfb)
> > +{
> > +	void *src = pfb->vmem + (pfb->font->height * pfb->fb->pitches[0]);
> > +	size_t len = (pfb->fb->height - pfb->font->height) *
> > +		     pfb->fb->pitches[0];
> > +
> > +	drm_panic_log("%s\n", __func__);
> > +
> > +	memmove(pfb->vmem, src, len);
> > +	memset(pfb->vmem + len, 0, pfb->font->height * pfb->fb->pitches[0]);
> > +}
> > +
> > +static void drm_panic_clear_screen(struct drm_panic_fb *pfb)
> > +{
> > +	memset(pfb->vmem, 0, pfb->fb->height * pfb->fb->pitches[0]);
> > +}
> > +
> > +static void drm_panic_log_msg(char *pre, const char *str, unsigned int len)
> > +{
> > +	char buf[512];
> > +
> > +	if (len > 510)
> > +		len = 510;
> > +
> > +	memcpy(buf, str, len);
> > +	buf[len] = '\n';
> > +	buf[len + 1] = '\0';
> > +
> > +	drm_panic_log("%s%s", pre, buf);
> > +}
> > +
> > +static void drm_panic_putcs_no_lf(struct drm_panic_fb *pfb,
> > +				  const char *str, unsigned int len)
> > +{
> > +	drm_panic_log("%s(len=%u) x=%u, y=%u\n", __func__, len,
> > +			pfb->xpos, pfb->ypos);
> > +
> > +	if (len <= 0)
> > +		return;
> > +
> > +	drm_panic_log_msg("", str, len);
> > +
> > +	drm_panic_render(pfb, str, len);
> > +
> > +}
> > +
> > +static void drm_panic_putcs(struct drm_panic_fb *pfb,
> > +			    const char *str, unsigned int num)
> > +{
> > +	unsigned int slen;
> > +	int len = num;
> > +	char *lf;
> > +
> > +	drm_panic_log("%s(num=%u)\n", __func__, num);
> > +
> > +	while (len > 0) {
> > +
> > +		if (pfb->ypos == pfb->rows) {
> > +			pfb->ypos--;
> > +			drm_panic_scroll_up(pfb);
> > +		}
> > +
> > +		lf = strpbrk(str, "\n");
> > +		if (lf)
> > +			slen = lf - str;
> > +		else
> > +			slen = len;
> > +
> > +		if (pfb->xpos + slen > pfb->cols)
> > +			slen = pfb->cols - pfb->xpos;
> > +
> > +		drm_panic_putcs_no_lf(pfb, str, slen);
> > +
> > +		len -= slen;
> > +		str += slen;
> > +		pfb->xpos += slen;
> > +
> > +		if (lf) {
> > +			str++;
> > +			len--;
> > +			pfb->xpos = 0;
> > +			pfb->ypos++;
> > +		}
> > +	}
> > +}
> > +
> > +static void drm_panic_write(const char *str, unsigned int num)
> > +{
> > +	unsigned int i;
> > +
> > +	if (!num)
> > +		return;
> > +
> > +	drm_panic_log("%s(num=%u)\n", __func__, num);
> > +
> > +	for (i = 0; i < DRM_PANIC_MAX_FBS; i++) {
> > +		if (!drm_panic_fbs[i].fb)
> > +			break;
> > +		drm_panic_putcs(&drm_panic_fbs[i], str, num);
> > +	}
> > +}
> > +
> > +/* this one is serialized by console_lock() */
> > +static void drm_panic_console_write(struct console *con,
> > +				    const char *str, unsigned int num)
> > +{
> > +	unsigned int i;
> > +
> > +	drm_panic_log_msg("->", str, num);
> > +
> > +	/* Buffer up messages to be replayed on panic */
> > +	if (!drm_panic_active) {
> > +		for (i = 0; i < num; i++) {
> > +			drm_panic_kmsgs[drm_panic_kmsgs_pos++] = *str++;
> > +			if (drm_panic_kmsgs_pos == DRM_PANIC_MAX_KMSGS)
> > +				drm_panic_kmsgs_pos = 0;
> > +		}
> > +		return;
> > +	}
> > +
> > +	drm_panic_write(str, num);
> > +}
> > +
> > +static struct console drm_panic_console = {
> > +	.name =         "drmpanic",
> > +	.write =        drm_panic_console_write,
> > +	.flags =        CON_PRINTBUFFER | CON_ENABLED,
> > +	.index =        0,
> > +};
> > +
> > +/*
> > + * The panic() function makes sure that only one CPU is allowed to run it's
> > + * code. So when this handler is called, we're alone. No racing with
> > + * console.write() is possible.
> > + */
> > +static int drm_panic_handler(struct notifier_block *this, unsigned long ev,
> > +			     void *ptr)
> > +{
> > +	const struct font_desc *font;
> > +	struct drm_framebuffer *fb;
> > +	struct drm_panic_fb *pfb;
> > +	struct drm_minor *minor;
> > +	unsigned int fbs = 0;
> > +	void *vmem;
> > +	int i;
> > +
> > +	drm_panic_log("%s\n", __func__);
> > +
> > +	drm_panic_active = true;
> > +
> > +	drm_minor_for_each(minor, DRM_MINOR_LEGACY, i) {
> > +		drm_panic_log("Found minor=%d\n", minor->index);
> > +		if (!minor->dev || !minor->dev->driver ||
> > +		    !minor->dev->driver->panic) {
> > +			drm_panic_log("Skipping: No panic handler\n");
> > +			continue;
> > +		}
> > +
> > +		fb = minor->dev->driver->panic(minor->dev, &vmem);
> > +		if (!fb) {
> > +			drm_panic_log("Skipping: Driver returned NULL\n");
> > +			continue;
> > +		}
> > +
> > +		if (!fb || !vmem || fb->dev != minor->dev || !fb->pitches[0]) {
> > +			drm_panic_log("Skipping: Failed check\n");
> > +			continue;
> > +		}
> > +
> > +		/* only 8-bit wide fonts are supported */
> > +		font = get_default_font(fb->width, fb->height, BIT(7), -1);
> > +		if (!font) {
> > +			drm_panic_log("Skipping: No font available\n");
> > +			continue;
> > +		}
> > +
> > +		pfb = &drm_panic_fbs[fbs++];
> > +
> > +		pfb->fb = fb;
> > +		pfb->vmem = vmem;
> > +		pfb->font = font;
> > +		pfb->cols = fb->width / font->width;
> > +		pfb->rows = fb->height / font->height;
> > +
> > +		drm_panic_clear_screen(pfb);
> > +
> > +		drm_panic_log("    %ux%u -> %ux%u, %s, %s\n", fb->width,
> > +				fb->height, pfb->cols, pfb->rows, font->name,
> > +				drm_get_format_name(fb->pixel_format));
> > +	}
> > +
> > +	if (drm_panic_kmsgs[0]) {
> > +		/* safeguard in case we interrupted drm_panic_console_write */
> > +		if (drm_panic_kmsgs_pos >= DRM_PANIC_MAX_KMSGS)
> > +			drm_panic_kmsgs_pos = 0;
> > +
> > +		drm_panic_write(&drm_panic_kmsgs[drm_panic_kmsgs_pos],
> > +				DRM_PANIC_MAX_KMSGS - drm_panic_kmsgs_pos);
> > +		drm_panic_write(drm_panic_kmsgs, drm_panic_kmsgs_pos);
> > +	}
> > +
> > +	return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block drm_panic_block = {
> > +	.notifier_call = drm_panic_handler,
> > +};
> > +
> > +
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +
> > +/* Out of band logging is useful at least in the initial development phase */
> > +#define DRM_PANIC_LOG_SIZE	SZ_64K
> > +#define DRM_PANIC_LOG_LINE	128
> > +#define DRM_PANIC_LOG_ENTRIES	(DRM_PANIC_LOG_SIZE / DRM_PANIC_LOG_LINE)
> > +
> > +static char *log_buf;
> > +static size_t log_pos;
> > +static struct dentry *drm_panic_logfs_root;
> > +
> > +static void drm_panic_log(const char *fmt, ...)
> > +{
> > +	va_list args;
> > +	u32 rem_nsec;
> > +	char *text;
> > +	size_t len;
> > +	u64 sec;
> > +
> > +	if (!log_buf || oops_in_progress)
> > +		return;
> > +
> > +	va_start(args, fmt);
> > +
> > +	if (log_pos >= DRM_PANIC_LOG_ENTRIES)
> > +		log_pos = 0;
> > +
> > +	text = log_buf + (log_pos++ * DRM_PANIC_LOG_LINE);
> > +	if (log_pos == DRM_PANIC_LOG_ENTRIES)
> > +		log_pos = 0;
> > +
> > +	sec = div_u64_rem(local_clock(), 1000000000, &rem_nsec);
> > +
> > +	len = scnprintf(text, DRM_PANIC_LOG_LINE, "[%5llu.%06u] ", sec,
> > +			rem_nsec / 1000);
> > +
> > +	vscnprintf(text + len, DRM_PANIC_LOG_LINE - len, fmt, args);
> > +
> > +	/* Make sure to always have a newline in case of overflow */
> > +	if (text[DRM_PANIC_LOG_LINE - 2] != '\0')
> > +		text[DRM_PANIC_LOG_LINE - 2] = '\n';
> > +
> > +	va_end(args);
> > +}
> > +
> > +static int drm_panic_log_show(struct seq_file *m, void *v)
> > +{
> > +	size_t pos = log_pos;
> > +	unsigned int i;
> > +	char *text;
> > +
> > +	for (i = 0; i < DRM_PANIC_LOG_ENTRIES; i++) {
> > +		text = log_buf + (pos++ * DRM_PANIC_LOG_LINE);
> > +		if (pos == DRM_PANIC_LOG_ENTRIES)
> > +			pos = 0;
> > +		if (*text == '\0')
> > +			continue;
> > +		seq_puts(m, text);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int drm_panic_log_open(struct inode *inode, struct file *file)
> > +{
> > +	return single_open(file, drm_panic_log_show, NULL);
> > +}
> > +
> > +static const struct file_operations drm_panic_log_ops = {
> > +	.owner   = THIS_MODULE,
> > +	.open    = drm_panic_log_open,
> > +	.read    = seq_read,
> > +	.llseek  = seq_lseek,
> > +	.release = single_release,
> > +};
> > +
> > +/*
> > + * Fake/simulate panic() at different levels:
> > + * 1: only trigger panic handling internally
> > + * 2: add local_irq_disable()
> > + * 3: add bust_spinlocks();
> > + * 100: don't fake it, do call panic()
> > + */
> > +static int drm_text_fake_panic(unsigned int level)
> > +{
> > +#ifndef MODULE
> > +	int old_loglevel = console_loglevel;
> > +#endif
> > +
> > +	if (!level && level != 100 && level > 3)
> > +		return -EINVAL;
> > +
> > +	if (level == 100)
> > +		panic("TESTING");
> > +
> > +	if (level > 1)
> > +		local_irq_disable();
> > +
> > +#ifndef MODULE
> > +	console_verbose();
> > +#endif
> > +	if (level > 2)
> > +		bust_spinlocks(1);
> > +
> > +	pr_emerg("Kernel panic - not syncing: FAKING=%u, oops_in_progress=%d\n",
> > +		 level, oops_in_progress);
> > +
> > +#ifdef CONFIG_DEBUG_BUGVERBOSE
> > +	dump_stack();
> > +#endif
> > +	/* simulate calling panic_notifier_list */
> > +	drm_panic_handler(NULL, 0, NULL);
> > +
> > +	if (level > 2)
> > +		bust_spinlocks(0);
> > +
> > +#ifndef MODULE
> > +	console_flush_on_panic();
> > +#endif
> > +	pr_emerg("---[ end Kernel panic - not syncing: FAKING\n");
> > +
> > +	if (level > 1)
> > +		local_irq_enable();
> > +
> > +#ifndef MODULE
> > +	console_loglevel = old_loglevel;
> > +#endif
> > +
> > +	return 0;
> > +}
> > +
> > +static ssize_t drm_text_panic_write(struct file *file,
> > +				    const char __user *user_buf,
> > +				    size_t count, loff_t *ppos)
> > +{
> > +	unsigned long long val;
> > +	ssize_t ret = 0;
> > +	char buf[24];
> > +	size_t size;
> > +
> > +	size = min(sizeof(buf) - 1, count);
> > +	if (copy_from_user(buf, user_buf, size))
> > +		return -EFAULT;
> > +
> > +	buf[size] = '\0';
> > +	ret = kstrtoull(buf, 0, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = drm_text_fake_panic(val);
> > +
> > +	return ret < 0 ? ret : count;
> > +}
> > +
> > +static const struct file_operations drm_text_panic_ops = {
> > +	.write =        drm_text_panic_write,
> > +	.open =         simple_open,
> > +	.llseek =       default_llseek,
> > +};
> > +
> > +static int drm_panic_logfs_init(void)
> > +{
> > +	drm_panic_logfs_root = debugfs_create_dir("drm-panic", NULL);
> > +	if (!drm_panic_logfs_root)
> > +		return -ENOMEM;
> > +
> > +	if (!debugfs_create_file("log", S_IRUGO, drm_panic_logfs_root, NULL,
> > +			    &drm_panic_log_ops))
> > +		goto err_remove;
> > +
> > +	log_buf = kzalloc(DRM_PANIC_LOG_SIZE, GFP_KERNEL);
> > +	if (!log_buf)
> > +		goto err_remove;
> > +
> > +	debugfs_create_file("panic", S_IWUSR, drm_panic_logfs_root, NULL,
> > +			    &drm_text_panic_ops);
> > +
> > +	return 0;
> > +
> > +err_remove:
> > +	debugfs_remove_recursive(drm_panic_logfs_root);
> > +
> > +	return -ENOMEM;
> > +}
> > +
> > +static void drm_panic_logfs_exit(void)
> > +{
> > +	debugfs_remove_recursive(drm_panic_logfs_root);
> > +	kfree(log_buf);
> > +	log_buf = NULL;
> > +}
> > +
> > +#else
> > +
> > +static int drm_panic_logfs_init(void)
> > +{
> > +}
> > +
> > +static void drm_panic_logfs_exit(void)
> > +{
> > +}
> > +
> > +static void drm_panic_log(const char *fmt, ...)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +
> > +void __init drm_panic_init(void)
> > +{
> > +	drm_panic_kmsgs = kzalloc(DRM_PANIC_MAX_KMSGS, GFP_KERNEL);
> > +	if (!drm_panic_kmsgs) {
> > +		DRM_ERROR("Failed to setup panic handler\n");
> > +		return;
> > +	}
> > +
> > +	drm_panic_logfs_init();
> > +drm_panic_log("%s\n", __func__);
> > +	register_console(&drm_panic_console);
> > +	atomic_notifier_chain_register(&panic_notifier_list,
> > +				       &drm_panic_block);
> > +}
> > +
> > +void __exit drm_panic_exit(void)
> > +{
> > +	if (!drm_panic_kmsgs)
> > +		return;
> > +
> > +	drm_panic_logfs_exit();
> > +	atomic_notifier_chain_unregister(&panic_notifier_list,
> > +					 &drm_panic_block);
> > +	unregister_console(&drm_panic_console);
> > +	kfree(drm_panic_kmsgs);
> > +}
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index bc7006e..4e84654 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -550,6 +550,28 @@ struct drm_driver {
> >  			  bool from_open);
> >  	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
> >  
> > +	/**
> > +	 * @panic:
> > +	 *
> > +	 * This function is called on panic() asking for a framebuffer to
> > +	 * display the panic messages on. It also needs the virtual address
> > +	 * of the backing buffer.
> > +	 * This function is optional.
> > +	 *
> > +	 * NOTE:
> > +	 *
> > +	 * This function is called in an atomic notifier chain and it cannot
> > +	 * sleep. Care must be taken so the machine is not killed even harder,
> > +	 * preventing output from going out on serial/netconsole.
> > +	 *
> > +	 * RETURNS:
> > +	 *
> > +	 * Framebuffer that should be used to display the panic messages,
> > +	 * alongside the virtual address of the backing buffer, or NULL if
> > +	 * the driver is unable to provide this.
> > +	 */
> > +	struct drm_framebuffer *(*panic)(struct drm_device *dev, void **vmem);
> > +
> >  	int (*debugfs_init)(struct drm_minor *minor);
> >  	void (*debugfs_cleanup)(struct drm_minor *minor);
> >  
> > -- 
> > 2.8.2
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/3] drm: Add a way to iterate over minors

[Noralf Trønnes]

Den 10.08.2016 10:43, skrev Daniel Vetter:
> On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote:
>> This adds a way for in-kernel users to iterate over the available
>> DRM minors. The implementation is oops safe so the panic code
>> can safely use it.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Why iterate over minors? I'd kinda have expected we'd iterate over devices
> instead ... And looking ahead, that seems to be what you actually want.

I did this because I couldn't find a list of drm_devices anywhere. Only 
minors.
But I can do a drm_for_each_device() based on the minor "list".

Noralf.

> -Daniel
>
>> ---
>>   drivers/gpu/drm/drm_drv.c | 30 ++++++++++++++++++++++++++++++
>>   include/drm/drmP.h        | 13 +++++++++++++
>>   2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index be27ed3..3b14366 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -292,6 +292,36 @@ void drm_minor_release(struct drm_minor *minor)
>>   }
>>   
>>   /**
>> + * drm_minor_lookup - Lookup DRM minor
>> + * @minor_id: Minor ID of the DRM-minor
>> + *
>> + * Looks up the given minor-ID and returns the respective DRM-minor object.
>> + * No reference is taken on the underlying device.
>> + * See drm_minor_for_each() for iterating over all minors.
>> + *
>> + * Returns:
>> + * Pointer to minor-object or NULL.
>> + */
>> +struct drm_minor *drm_minor_lookup(unsigned int minor_id)
>> +{
>> +	struct drm_minor *minor;
>> +	unsigned long flags;
>> +	int locked = 1;
>> +
>> +	if (oops_in_progress)
>> +		locked = spin_trylock_irqsave(&drm_minor_lock, flags);
>> +	else
>> +		spin_lock_irqsave(&drm_minor_lock, flags);
>> +
>> +	minor = idr_find(&drm_minors_idr, minor_id);
>> +
>> +	if (locked)
>> +		spin_unlock_irqrestore(&drm_minor_lock, flags);
>> +
>> +	return minor;
>> +}
>> +
>> +/**
>>    * DOC: driver instance overview
>>    *
>>    * A device instance for a drm driver is represented by struct &drm_device. This
>> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
>> index d377865..bc7006e 100644
>> --- a/include/drm/drmP.h
>> +++ b/include/drm/drmP.h
>> @@ -974,6 +974,19 @@ void drm_dev_unregister(struct drm_device *dev);
>>   
>>   struct drm_minor *drm_minor_acquire(unsigned int minor_id);
>>   void drm_minor_release(struct drm_minor *minor);
>> +struct drm_minor *drm_minor_lookup(unsigned int minor_id);
>> +
>> +/**
>> + * drm_minor_for_each - Iterate over DRM minors
>> + * @minor: DRM minor handle
>> + * @type: DRM minor type to iterate over
>> + * @id: id handle
>> + *
>> + * Iterate over the registered DRM minors of a given type.
>> + */
>> +#define drm_minor_for_each(minor, type, id)  \
>> +	for ((id) = 0; (id) < 64; (id)++)  \
>> +		if (((minor) = drm_minor_lookup((id) + (type) * 64)))
>>   
>>   /*@}*/
>>   
>> -- 
>> 2.8.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/3] drm: Add a way to iterate over minors

[Daniel Vetter]

On Wed, Aug 10, 2016 at 4:27 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 10.08.2016 10:43, skrev Daniel Vetter:
>>
>> On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote:
>>>
>>> This adds a way for in-kernel users to iterate over the available
>>> DRM minors. The implementation is oops safe so the panic code
>>> can safely use it.
>>>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>
>> Why iterate over minors? I'd kinda have expected we'd iterate over devices
>> instead ... And looking ahead, that seems to be what you actually want.
>
>
> I did this because I couldn't find a list of drm_devices anywhere. Only
> minors.
> But I can do a drm_for_each_device() based on the minor "list".

Hm, the drm drivers are all part of the drm class, we should be able
to iterate them I think. Otherwise let's just add a new list instead
of jumping through hoops.
New list with it's own lock (which we then trylock from the panic
handler) would be safest I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 2/3] drm: Add panic handling

[Noralf Trønnes]

Den 10.08.2016 11:15, skrev Daniel Vetter:
> On Tue, Aug 09, 2016 at 07:45:41PM +0200, Noralf Trønnes wrote:
>> This adds support for outputting kernel messages on panic().
>> The drivers that supports it, provides a framebuffer that the
>> messages can be rendered on.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Thinking about how we should implement this in a full-blown driver, I
> think we will need a bit more than this. Here's the additional
> requirements I've come up that a driver more complex than sdrm would need
> for reliable panic handling:
>
> - Multiple outputs, with multiple framebuffer (or one framebuffer and
>    multiple offsets within). And since one display might be dead we should
>    try really hard to show the oops on all of them. Consequence: I think we
>    need to also loop over all drm_crtc in a drm_device, to be able to
>    support them all.

How about this:

/*
  * The panic() function makes sure that only one CPU is allowed to run it's
  * code. So this handler is only called once.
  *
  * Prior to calling the panic handlers, panic() calls smp_send_stop(). If
  * that went well, there's only one CPU running, but this is no guarantee.
  */
static int drm_panic_handler(struct notifier_block *this, unsigned long ev,
                              void *ptr)
{
<declarations>

         drm_for_each_device(drm, id) {
                 if (!drm->driver ||
                     !(drm->driver->driver_features & DRIVER_ATOMIC))
                         continue;

                 drm_for_each_crtc(crtc, drm) {
                         crtc_locked = ww_mutex_trylock(&crtc->mutex.mutex);

                         if (!crtc->enabled || !crtc->primary)
                                 goto crtc_unlock;

                         if (!crtc->state || !crtc->state->active ||
                             !crtc->state->state)
                                 goto crtc_unlock;

                         state = crtc->state->state;
                         plane = crtc->primary;
                         plane_locked = 
ww_mutex_trylock(&plane->mutex.mutex);

                         plane_state = 
drm_atomic_get_existing_plane_state(state, plane);
                         if (!plane_state || !plane_state->visible)
                                 goto plane_unlock;

                         fb = plane->fb;

                         if (!fb || !fb->funcs || !fb->funcs->panic_vmap)
                                 goto plane_unlock;

                         /* only 8-bit wide fonts are supported */
                         font = get_default_font(fb->width, fb->height, 
BIT(7), -1);
                         if (!font)
                                 goto plane_unlock;

                         vmap = fb->funcs->panic_vmap(fb);
                         if (!vmap)
                                 goto plane_unlock;

                 /*
                  * How do I find the actual width/height from the plane 
state
                  * as you mention further down?
                  */
                         width = ??
                         height = ??

                         pfb = &drm_panic_fbs[fbs++];
                         pfb->fb = fb;
                         pfb->vmap = vmap;
                         pfb->font = font;
                         pfb->cols = width / font->width;
                         pfb->rows = height / font->height;

                 /*
                  * how to unlock?
                  * ww_mutex_unlock() doc says:
                  * This function must not be used in interrupt context
                  */
                 plane_unlock:
                         if (plane_locked)
                                 somehow_unlock();
                 crtc_unlock:
                         if (crtc_locked)
                                 somehow_unlock();
                 }
         }

         drm_panic_active = true;
}



> - With desktop gpus you pretty much always end up with a tiled
>    framebuffer. And from an oops context it's going to be impossible to
>    detile that quickly. I think for this we need a helper to draw x/y
>    pixels (it's going to be dead slow, but meh), with a default
>    implementation which assumes linear layout. Drivers could then frob x/y
>    coordinates first in their own logic and call into that function.
>
> - There's a good chance you're showing a video on a yuv buffer
>    full-screen. If we don't bother too much with what it looks like, but
>    only care about a foreground/background color then it's also easy to
>    implement a draw_xy_pixel for these framebuffers. That means though that
>    you can't clear things with memset, but that you need to call the
>    interface func for each pixel. Yes this is going to be dead slow, but
>    who cares as long as the oops eventually shows up ;-)

Let's give it a try, the yuv part is just guesswork:

void drm_panic_draw_xy(struct drm_framebuffer *fb, void *vmap,
                        int x, int y, bool foreground)
{
         void *dst;

         dst = vmap + fb->offsets[0] + (y * fb->pitches[0]);

         switch (fb->pixel_format & ~DRM_FORMAT_BIG_ENDIAN) {

         case DRM_FORMAT_C8:

         case DRM_FORMAT_RGB332:
         case DRM_FORMAT_BGR233:

         case DRM_FORMAT_NV12:
         case DRM_FORMAT_NV21:
         case DRM_FORMAT_NV16:
         case DRM_FORMAT_NV61:
         case DRM_FORMAT_NV24:
         case DRM_FORMAT_NV42:

         case DRM_FORMAT_YUV410:
         case DRM_FORMAT_YVU410:
         case DRM_FORMAT_YUV411:
         case DRM_FORMAT_YVU411:
         case DRM_FORMAT_YUV420:
         case DRM_FORMAT_YVU420:
         case DRM_FORMAT_YUV422:
         case DRM_FORMAT_YVU422:
         case DRM_FORMAT_YUV444:
         case DRM_FORMAT_YVU444:
                 dst += x;
                 *(u8 *)dst = foreground ? 0xff : 0x00;
                 break;

         case DRM_FORMAT_YUYV:
         case DRM_FORMAT_YVYU:
                 dst += x * sizeof(u32);
                 put_unaligned(foreground ? 0xff00ff00 : 0x00000000, 
(u32 *)dst);
                 break;

         case DRM_FORMAT_UYVY:
         case DRM_FORMAT_VYUY:
                 dst += x * sizeof(u32);
                 put_unaligned(foreground ? 0x00ff00ff : 0x00000000, 
(u32 *)dst);
                 break;

         case DRM_FORMAT_XRGB8888:
         case DRM_FORMAT_ARGB8888:
         case DRM_FORMAT_XBGR8888:
         case DRM_FORMAT_ABGR8888:
                 dst += x * sizeof(u32);
                 put_unaligned(foreground ? 0x00ffffff : 0x00000000, 
(u32 *)dst);
                 break;

         /* and then the other rgb formats */
         }
}

> - The framebuffer size doesn't necessarily match the size of the visible
>    part. Probably best if we dig this out from the plane_state directly
>    (less fragile code in drivers). Relying on drm_plane_state means this
>    will only work on atomic drivers (in legacy drivers this information is
>    hidden in driver-private corners), but I think that's totally ok.
>
> - 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).

Is this for the case where panic() fails to stop the other cpus?
Or can preemption happen even though panic() calls local_irq_disable()?

> - Multiple planes which might occlude the primary plane: I think we should
>    just bother with the primary plane, and maybe give drivers a
>    panic_commit hook where they could try to disable any additional planes.
>    But that's not something we need in the first version here at all.
>
> - I think some helpers for creating the vmap would be nice. Should be
>    simple for cma-backed framebuffers, and also simple if you use a normal
>    shmem backed gem buffer. For cma probably best if drivers don't even
>    need to bother with this.

I don't know about shmem, but cma could look like this:

/**
  * drm_fb_cma_panic_vmap - Helper function for the
  *                         &drm_framebuffer_funcs->panic_vmap callback
  * @fb: DRM framebuffer
  *
  * This function is used in a panic() situation to get to the virtual 
address
  * of the backing buffer for rendering kernel messages.
  * There's no need to set the &drm_framebuffer_funcs->panic_draw_xy 
callback,
  * since the default implementation will suffice.
  *
  * Note: A PRIME imported buffer will _not_ have it's vaddr set.
  *
  * Returns:
  * The virtual address of the backing object, or NULL.
  */
void *drm_fb_cma_panic_vmap(struct drm_framebuffer *fb)
{
         struct drm_fb_cma *fb_cma = to_fb_cma(fb);
         struct drm_gem_cma_object *cma_obj = fb_cma->obj[0];

         return cma_obj ? cma_obj->vaddr : NULL;
}
EXPORT_SYMBOL(drm_fb_cma_panic_vmap);

  static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
         .destroy        = drm_fb_cma_destroy,
         .create_handle  = drm_fb_cma_create_handle,
+       .panic_vmap     = drm_fb_cma_panic_vmap,
  };

> - For driver convenience I think we could check a few things about
>    visibility of each plane (e.g. crtc_state->active), and skip if there's
>    no chance it can be seen. Doing less means less chances to blow up, and
>    higher chances that the one screen which is on actually ends up with the
>    oops on it.
>
> - Minimal cleanup. I think we can just leak the vmapping, no harm in that.
>    But the kms locks need to be dropped again. Dropping them again might
>    increase the odds that the system limps along long enough for logs to
>    hit the disks.
>
> Taking this all together, I think the driver interface should be
> restructured a bit, and that most of the handling should be on the
> drm_framebuffer directly:
>
> struct drm_framebuffer_funcs {
> 	/* For vmapping the selected framebuffer in a panic context. Must
> 	 * be super careful about locking (only trylocking allowed), can
> 	 * return NULL if it didn't work out. The return value is an
> 	 * opaque cookie which is passed to @panic_draw_xy, it can be
> 	 * anything: vmap area, structure with more details, just a few
> 	 * flags, ...
> 	 */
> 	void *(panic_vmap)(struct drm_framebuffer *fb);
>
> 	/* For drawing pixels onto a framebuffer mapping with @panic_vmap.
> 	 * This is optional, the default implementation assumes that vmap
> 	 * points at a linear mapping of the framebuffer.
> 	 */
> 	void (panic_draw_xy)(struct drm_framebuffer *fb, void *vmap,
> 			     int x, int y, bool foreground);
> };
>
> Ofc comments need to be fleshed out some more, but the idea is that all
> the fb selection and lookup is handled in shared code (and with proper
> locking, but only for atomic drivers).
>
> Thoughts?

I like this, it appears to be very flexible.
Will be interesting to see how fast/slow this is, in the linear case,
compared to what I did.


Noralf.

> -Daniel
>
>> ---
>>   drivers/gpu/drm/Makefile       |   2 +-
>>   drivers/gpu/drm/drm_drv.c      |   3 +
>>   drivers/gpu/drm/drm_internal.h |   4 +
>>   drivers/gpu/drm/drm_panic.c    | 606 +++++++++++++++++++++++++++++++++++++++++
>>   include/drm/drmP.h             |  22 ++
>>   5 files changed, 636 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/drm_panic.c
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index eba32ad..ff04e41 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -12,7 +12,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
>>   		drm_info.o drm_debugfs.o drm_encoder_slave.o \
>>   		drm_trace_points.o drm_global.o drm_prime.o \
>>   		drm_rect.o drm_vma_manager.o drm_flip_work.o \
>> -		drm_modeset_lock.o drm_atomic.o drm_bridge.o
>> +		drm_modeset_lock.o drm_atomic.o drm_bridge.o drm_panic.o
>>   
>>   drm-$(CONFIG_COMPAT) += drm_ioc32.o
>>   drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 3b14366..457ee91 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -861,6 +861,8 @@ static int __init drm_core_init(void)
>>   		goto err_p3;
>>   	}
>>   
>> +	drm_panic_init();
>> +
>>   	DRM_INFO("Initialized %s %d.%d.%d %s\n",
>>   		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
>>   	return 0;
>> @@ -876,6 +878,7 @@ err_p1:
>>   
>>   static void __exit drm_core_exit(void)
>>   {
>> +	drm_panic_exit();
>>   	debugfs_remove(drm_debugfs_root);
>>   	drm_sysfs_destroy();
>>   
>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>> index b86dc9b..7463d9d 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -90,6 +90,10 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
>>   void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
>>   void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
>>   
>> +/* drm_panic.c */
>> +void drm_panic_init(void);
>> +void drm_panic_exit(void);
>> +
>>   /* drm_debugfs.c */
>>   #if defined(CONFIG_DEBUG_FS)
>>   int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
>> new file mode 100644
>> index 0000000..e185c9d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_panic.c
>> @@ -0,0 +1,606 @@
>> +/*
>> + * Copyright 2016 Noralf Trønnes
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <asm/unaligned.h>
>> +#include <drm/drmP.h>
>> +#include <linux/console.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/font.h>
>> +#include <linux/kernel.h>
>> +#include <linux/seq_file.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +
>> +struct drm_panic_fb {
>> +	struct drm_framebuffer *fb;
>> +	void *vmem;
>> +	const struct font_desc *font;
>> +	unsigned int cols;
>> +	unsigned int rows;
>> +	unsigned int xpos;
>> +	unsigned int ypos;
>> +};
>> +
>> +#define DRM_PANIC_MAX_FBS	64
>> +static struct drm_panic_fb drm_panic_fbs[DRM_PANIC_MAX_FBS];
>> +
>> +#define DRM_PANIC_MAX_KMSGS	SZ_4K
>> +static char *drm_panic_kmsgs;
>> +static size_t drm_panic_kmsgs_pos;
>> +
>> +static bool drm_panic_active;
>> +
>> +static void drm_panic_log(const char *fmt, ...);
>> +
>> +static inline void drm_panic_draw_pixel(u8 *dst, u32 pixel_format, bool val)
>> +{
>> +	switch (pixel_format & ~DRM_FORMAT_BIG_ENDIAN) {
>> +
>> +	case DRM_FORMAT_C8:
>> +	case DRM_FORMAT_RGB332:
>> +	case DRM_FORMAT_BGR233:
>> +		*dst = val ? 0xff : 0x00;
>> +		break;
>> +
>> +	case DRM_FORMAT_XRGB4444:
>> +	case DRM_FORMAT_ARGB4444:
>> +	case DRM_FORMAT_XBGR4444:
>> +	case DRM_FORMAT_ABGR4444:
>> +		put_unaligned(val ? 0x0fff : 0x0000, (u16 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_RGBX4444:
>> +	case DRM_FORMAT_RGBA4444:
>> +	case DRM_FORMAT_BGRX4444:
>> +	case DRM_FORMAT_BGRA4444:
>> +		put_unaligned(val ? 0xfff0 : 0x0000, (u16 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_XRGB1555:
>> +	case DRM_FORMAT_ARGB1555:
>> +	case DRM_FORMAT_XBGR1555:
>> +	case DRM_FORMAT_ABGR1555:
>> +		put_unaligned(val ? 0x7fff : 0x0000, (u16 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_RGBX5551:
>> +	case DRM_FORMAT_RGBA5551:
>> +	case DRM_FORMAT_BGRX5551:
>> +	case DRM_FORMAT_BGRA5551:
>> +		put_unaligned(val ? 0xfffe : 0x0000, (u16 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_RGB565:
>> +	case DRM_FORMAT_BGR565:
>> +		put_unaligned(val ? 0xffff : 0x0000, (u16 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_RGB888:
>> +	case DRM_FORMAT_BGR888:
>> +		dst[0] = val ? 0xff : 0x00;
>> +		dst[1] = val ? 0xff : 0x00;
>> +		dst[2] = val ? 0xff : 0x00;
>> +		break;
>> +
>> +	case DRM_FORMAT_XRGB8888:
>> +	case DRM_FORMAT_ARGB8888:
>> +	case DRM_FORMAT_XBGR8888:
>> +	case DRM_FORMAT_ABGR8888:
>> +		put_unaligned(val ? 0x00ffffff : 0x00000000, (u32 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_RGBX8888:
>> +	case DRM_FORMAT_RGBA8888:
>> +	case DRM_FORMAT_BGRX8888:
>> +	case DRM_FORMAT_BGRA8888:
>> +		put_unaligned(val ? 0xffffff00 : 0x00000000, (u32 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_XRGB2101010:
>> +	case DRM_FORMAT_ARGB2101010:
>> +	case DRM_FORMAT_XBGR2101010:
>> +	case DRM_FORMAT_ABGR2101010:
>> +		put_unaligned(val ? 0x3fffffff : 0x00000000, (u32 *)dst);
>> +		break;
>> +
>> +	case DRM_FORMAT_RGBX1010102:
>> +	case DRM_FORMAT_RGBA1010102:
>> +	case DRM_FORMAT_BGRX1010102:
>> +	case DRM_FORMAT_BGRA1010102:
>> +		put_unaligned(val ? 0xfffffffc : 0x00000000, (u32 *)dst);
>> +		break;
>> +	}
>> +}
>> +
>> +static void drm_panic_render(struct drm_panic_fb *pfb,
>> +			     const char *text, unsigned int len)
>> +{
>> +	const struct font_desc *font = pfb->font;
>> +	unsigned int pix_depth, pix_bpp, cpp;
>> +	unsigned int col = pfb->xpos;
>> +	unsigned int row = pfb->ypos;
>> +	unsigned int i, h, w;
>> +	void *dst, *pos;
>> +	u8 fontline;
>> +
>> +	if ((row + 1) * font->height > pfb->fb->height)
>> +		return;
>> +
>> +	if ((col + len) * font->width > pfb->fb->width)
>> +		return;
>> +
>> +	drm_fb_get_bpp_depth(pfb->fb->pixel_format, &pix_depth, &pix_bpp);
>> +	cpp = DIV_ROUND_UP(pix_bpp, 8);
>> +
>> +	/* TODO: should fb->offsets[0] be added here? */
>> +	dst = pfb->vmem + (row * font->height * pfb->fb->pitches[0]) +
>> +	      (col * font->width * cpp);
>> +
>> +	for (h = 0; h < font->height; h++) {
>> +		pos = dst;
>> +
>> +		for (i = 0; i < len; i++) {
>> +			fontline = *(u8 *)(font->data + text[i] * font->height + h);
>> +
>> +			for (w = 0; w < font->width; w++) {
>> +				drm_panic_draw_pixel(pos, pfb->fb->pixel_format,
>> +						     fontline & BIT(7 - w));
>> +				pos += cpp;
>> +			}
>> +		}
>> +
>> +		dst += pfb->fb->pitches[0];
>> +	}
>> +}
>> +
>> +static void drm_panic_scroll_up(struct drm_panic_fb *pfb)
>> +{
>> +	void *src = pfb->vmem + (pfb->font->height * pfb->fb->pitches[0]);
>> +	size_t len = (pfb->fb->height - pfb->font->height) *
>> +		     pfb->fb->pitches[0];
>> +
>> +	drm_panic_log("%s\n", __func__);
>> +
>> +	memmove(pfb->vmem, src, len);
>> +	memset(pfb->vmem + len, 0, pfb->font->height * pfb->fb->pitches[0]);
>> +}
>> +
>> +static void drm_panic_clear_screen(struct drm_panic_fb *pfb)
>> +{
>> +	memset(pfb->vmem, 0, pfb->fb->height * pfb->fb->pitches[0]);
>> +}
>> +
>> +static void drm_panic_log_msg(char *pre, const char *str, unsigned int len)
>> +{
>> +	char buf[512];
>> +
>> +	if (len > 510)
>> +		len = 510;
>> +
>> +	memcpy(buf, str, len);
>> +	buf[len] = '\n';
>> +	buf[len + 1] = '\0';
>> +
>> +	drm_panic_log("%s%s", pre, buf);
>> +}
>> +
>> +static void drm_panic_putcs_no_lf(struct drm_panic_fb *pfb,
>> +				  const char *str, unsigned int len)
>> +{
>> +	drm_panic_log("%s(len=%u) x=%u, y=%u\n", __func__, len,
>> +			pfb->xpos, pfb->ypos);
>> +
>> +	if (len <= 0)
>> +		return;
>> +
>> +	drm_panic_log_msg("", str, len);
>> +
>> +	drm_panic_render(pfb, str, len);
>> +
>> +}
>> +
>> +static void drm_panic_putcs(struct drm_panic_fb *pfb,
>> +			    const char *str, unsigned int num)
>> +{
>> +	unsigned int slen;
>> +	int len = num;
>> +	char *lf;
>> +
>> +	drm_panic_log("%s(num=%u)\n", __func__, num);
>> +
>> +	while (len > 0) {
>> +
>> +		if (pfb->ypos == pfb->rows) {
>> +			pfb->ypos--;
>> +			drm_panic_scroll_up(pfb);
>> +		}
>> +
>> +		lf = strpbrk(str, "\n");
>> +		if (lf)
>> +			slen = lf - str;
>> +		else
>> +			slen = len;
>> +
>> +		if (pfb->xpos + slen > pfb->cols)
>> +			slen = pfb->cols - pfb->xpos;
>> +
>> +		drm_panic_putcs_no_lf(pfb, str, slen);
>> +
>> +		len -= slen;
>> +		str += slen;
>> +		pfb->xpos += slen;
>> +
>> +		if (lf) {
>> +			str++;
>> +			len--;
>> +			pfb->xpos = 0;
>> +			pfb->ypos++;
>> +		}
>> +	}
>> +}
>> +
>> +static void drm_panic_write(const char *str, unsigned int num)
>> +{
>> +	unsigned int i;
>> +
>> +	if (!num)
>> +		return;
>> +
>> +	drm_panic_log("%s(num=%u)\n", __func__, num);
>> +
>> +	for (i = 0; i < DRM_PANIC_MAX_FBS; i++) {
>> +		if (!drm_panic_fbs[i].fb)
>> +			break;
>> +		drm_panic_putcs(&drm_panic_fbs[i], str, num);
>> +	}
>> +}
>> +
>> +/* this one is serialized by console_lock() */
>> +static void drm_panic_console_write(struct console *con,
>> +				    const char *str, unsigned int num)
>> +{
>> +	unsigned int i;
>> +
>> +	drm_panic_log_msg("->", str, num);
>> +
>> +	/* Buffer up messages to be replayed on panic */
>> +	if (!drm_panic_active) {
>> +		for (i = 0; i < num; i++) {
>> +			drm_panic_kmsgs[drm_panic_kmsgs_pos++] = *str++;
>> +			if (drm_panic_kmsgs_pos == DRM_PANIC_MAX_KMSGS)
>> +				drm_panic_kmsgs_pos = 0;
>> +		}
>> +		return;
>> +	}
>> +
>> +	drm_panic_write(str, num);
>> +}
>> +
>> +static struct console drm_panic_console = {
>> +	.name =         "drmpanic",
>> +	.write =        drm_panic_console_write,
>> +	.flags =        CON_PRINTBUFFER | CON_ENABLED,
>> +	.index =        0,
>> +};
>> +
>> +/*
>> + * The panic() function makes sure that only one CPU is allowed to run it's
>> + * code. So when this handler is called, we're alone. No racing with
>> + * console.write() is possible.
>> + */
>> +static int drm_panic_handler(struct notifier_block *this, unsigned long ev,
>> +			     void *ptr)
>> +{
>> +	const struct font_desc *font;
>> +	struct drm_framebuffer *fb;
>> +	struct drm_panic_fb *pfb;
>> +	struct drm_minor *minor;
>> +	unsigned int fbs = 0;
>> +	void *vmem;
>> +	int i;
>> +
>> +	drm_panic_log("%s\n", __func__);
>> +
>> +	drm_panic_active = true;
>> +
>> +	drm_minor_for_each(minor, DRM_MINOR_LEGACY, i) {
>> +		drm_panic_log("Found minor=%d\n", minor->index);
>> +		if (!minor->dev || !minor->dev->driver ||
>> +		    !minor->dev->driver->panic) {
>> +			drm_panic_log("Skipping: No panic handler\n");
>> +			continue;
>> +		}
>> +
>> +		fb = minor->dev->driver->panic(minor->dev, &vmem);
>> +		if (!fb) {
>> +			drm_panic_log("Skipping: Driver returned NULL\n");
>> +			continue;
>> +		}
>> +
>> +		if (!fb || !vmem || fb->dev != minor->dev || !fb->pitches[0]) {
>> +			drm_panic_log("Skipping: Failed check\n");
>> +			continue;
>> +		}
>> +
>> +		/* only 8-bit wide fonts are supported */
>> +		font = get_default_font(fb->width, fb->height, BIT(7), -1);
>> +		if (!font) {
>> +			drm_panic_log("Skipping: No font available\n");
>> +			continue;
>> +		}
>> +
>> +		pfb = &drm_panic_fbs[fbs++];
>> +
>> +		pfb->fb = fb;
>> +		pfb->vmem = vmem;
>> +		pfb->font = font;
>> +		pfb->cols = fb->width / font->width;
>> +		pfb->rows = fb->height / font->height;
>> +
>> +		drm_panic_clear_screen(pfb);
>> +
>> +		drm_panic_log("    %ux%u -> %ux%u, %s, %s\n", fb->width,
>> +				fb->height, pfb->cols, pfb->rows, font->name,
>> +				drm_get_format_name(fb->pixel_format));
>> +	}
>> +
>> +	if (drm_panic_kmsgs[0]) {
>> +		/* safeguard in case we interrupted drm_panic_console_write */
>> +		if (drm_panic_kmsgs_pos >= DRM_PANIC_MAX_KMSGS)
>> +			drm_panic_kmsgs_pos = 0;
>> +
>> +		drm_panic_write(&drm_panic_kmsgs[drm_panic_kmsgs_pos],
>> +				DRM_PANIC_MAX_KMSGS - drm_panic_kmsgs_pos);
>> +		drm_panic_write(drm_panic_kmsgs, drm_panic_kmsgs_pos);
>> +	}
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block drm_panic_block = {
>> +	.notifier_call = drm_panic_handler,
>> +};
>> +
>> +
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +
>> +/* Out of band logging is useful at least in the initial development phase */
>> +#define DRM_PANIC_LOG_SIZE	SZ_64K
>> +#define DRM_PANIC_LOG_LINE	128
>> +#define DRM_PANIC_LOG_ENTRIES	(DRM_PANIC_LOG_SIZE / DRM_PANIC_LOG_LINE)
>> +
>> +static char *log_buf;
>> +static size_t log_pos;
>> +static struct dentry *drm_panic_logfs_root;
>> +
>> +static void drm_panic_log(const char *fmt, ...)
>> +{
>> +	va_list args;
>> +	u32 rem_nsec;
>> +	char *text;
>> +	size_t len;
>> +	u64 sec;
>> +
>> +	if (!log_buf || oops_in_progress)
>> +		return;
>> +
>> +	va_start(args, fmt);
>> +
>> +	if (log_pos >= DRM_PANIC_LOG_ENTRIES)
>> +		log_pos = 0;
>> +
>> +	text = log_buf + (log_pos++ * DRM_PANIC_LOG_LINE);
>> +	if (log_pos == DRM_PANIC_LOG_ENTRIES)
>> +		log_pos = 0;
>> +
>> +	sec = div_u64_rem(local_clock(), 1000000000, &rem_nsec);
>> +
>> +	len = scnprintf(text, DRM_PANIC_LOG_LINE, "[%5llu.%06u] ", sec,
>> +			rem_nsec / 1000);
>> +
>> +	vscnprintf(text + len, DRM_PANIC_LOG_LINE - len, fmt, args);
>> +
>> +	/* Make sure to always have a newline in case of overflow */
>> +	if (text[DRM_PANIC_LOG_LINE - 2] != '\0')
>> +		text[DRM_PANIC_LOG_LINE - 2] = '\n';
>> +
>> +	va_end(args);
>> +}
>> +
>> +static int drm_panic_log_show(struct seq_file *m, void *v)
>> +{
>> +	size_t pos = log_pos;
>> +	unsigned int i;
>> +	char *text;
>> +
>> +	for (i = 0; i < DRM_PANIC_LOG_ENTRIES; i++) {
>> +		text = log_buf + (pos++ * DRM_PANIC_LOG_LINE);
>> +		if (pos == DRM_PANIC_LOG_ENTRIES)
>> +			pos = 0;
>> +		if (*text == '\0')
>> +			continue;
>> +		seq_puts(m, text);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int drm_panic_log_open(struct inode *inode, struct file *file)
>> +{
>> +	return single_open(file, drm_panic_log_show, NULL);
>> +}
>> +
>> +static const struct file_operations drm_panic_log_ops = {
>> +	.owner   = THIS_MODULE,
>> +	.open    = drm_panic_log_open,
>> +	.read    = seq_read,
>> +	.llseek  = seq_lseek,
>> +	.release = single_release,
>> +};
>> +
>> +/*
>> + * Fake/simulate panic() at different levels:
>> + * 1: only trigger panic handling internally
>> + * 2: add local_irq_disable()
>> + * 3: add bust_spinlocks();
>> + * 100: don't fake it, do call panic()
>> + */
>> +static int drm_text_fake_panic(unsigned int level)
>> +{
>> +#ifndef MODULE
>> +	int old_loglevel = console_loglevel;
>> +#endif
>> +
>> +	if (!level && level != 100 && level > 3)
>> +		return -EINVAL;
>> +
>> +	if (level == 100)
>> +		panic("TESTING");
>> +
>> +	if (level > 1)
>> +		local_irq_disable();
>> +
>> +#ifndef MODULE
>> +	console_verbose();
>> +#endif
>> +	if (level > 2)
>> +		bust_spinlocks(1);
>> +
>> +	pr_emerg("Kernel panic - not syncing: FAKING=%u, oops_in_progress=%d\n",
>> +		 level, oops_in_progress);
>> +
>> +#ifdef CONFIG_DEBUG_BUGVERBOSE
>> +	dump_stack();
>> +#endif
>> +	/* simulate calling panic_notifier_list */
>> +	drm_panic_handler(NULL, 0, NULL);
>> +
>> +	if (level > 2)
>> +		bust_spinlocks(0);
>> +
>> +#ifndef MODULE
>> +	console_flush_on_panic();
>> +#endif
>> +	pr_emerg("---[ end Kernel panic - not syncing: FAKING\n");
>> +
>> +	if (level > 1)
>> +		local_irq_enable();
>> +
>> +#ifndef MODULE
>> +	console_loglevel = old_loglevel;
>> +#endif
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t drm_text_panic_write(struct file *file,
>> +				    const char __user *user_buf,
>> +				    size_t count, loff_t *ppos)
>> +{
>> +	unsigned long long val;
>> +	ssize_t ret = 0;
>> +	char buf[24];
>> +	size_t size;
>> +
>> +	size = min(sizeof(buf) - 1, count);
>> +	if (copy_from_user(buf, user_buf, size))
>> +		return -EFAULT;
>> +
>> +	buf[size] = '\0';
>> +	ret = kstrtoull(buf, 0, &val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = drm_text_fake_panic(val);
>> +
>> +	return ret < 0 ? ret : count;
>> +}
>> +
>> +static const struct file_operations drm_text_panic_ops = {
>> +	.write =        drm_text_panic_write,
>> +	.open =         simple_open,
>> +	.llseek =       default_llseek,
>> +};
>> +
>> +static int drm_panic_logfs_init(void)
>> +{
>> +	drm_panic_logfs_root = debugfs_create_dir("drm-panic", NULL);
>> +	if (!drm_panic_logfs_root)
>> +		return -ENOMEM;
>> +
>> +	if (!debugfs_create_file("log", S_IRUGO, drm_panic_logfs_root, NULL,
>> +			    &drm_panic_log_ops))
>> +		goto err_remove;
>> +
>> +	log_buf = kzalloc(DRM_PANIC_LOG_SIZE, GFP_KERNEL);
>> +	if (!log_buf)
>> +		goto err_remove;
>> +
>> +	debugfs_create_file("panic", S_IWUSR, drm_panic_logfs_root, NULL,
>> +			    &drm_text_panic_ops);
>> +
>> +	return 0;
>> +
>> +err_remove:
>> +	debugfs_remove_recursive(drm_panic_logfs_root);
>> +
>> +	return -ENOMEM;
>> +}
>> +
>> +static void drm_panic_logfs_exit(void)
>> +{
>> +	debugfs_remove_recursive(drm_panic_logfs_root);
>> +	kfree(log_buf);
>> +	log_buf = NULL;
>> +}
>> +
>> +#else
>> +
>> +static int drm_panic_logfs_init(void)
>> +{
>> +}
>> +
>> +static void drm_panic_logfs_exit(void)
>> +{
>> +}
>> +
>> +static void drm_panic_log(const char *fmt, ...)
>> +{
>> +}
>> +
>> +#endif
>> +
>> +
>> +void __init drm_panic_init(void)
>> +{
>> +	drm_panic_kmsgs = kzalloc(DRM_PANIC_MAX_KMSGS, GFP_KERNEL);
>> +	if (!drm_panic_kmsgs) {
>> +		DRM_ERROR("Failed to setup panic handler\n");
>> +		return;
>> +	}
>> +
>> +	drm_panic_logfs_init();
>> +drm_panic_log("%s\n", __func__);
>> +	register_console(&drm_panic_console);
>> +	atomic_notifier_chain_register(&panic_notifier_list,
>> +				       &drm_panic_block);
>> +}
>> +
>> +void __exit drm_panic_exit(void)
>> +{
>> +	if (!drm_panic_kmsgs)
>> +		return;
>> +
>> +	drm_panic_logfs_exit();
>> +	atomic_notifier_chain_unregister(&panic_notifier_list,
>> +					 &drm_panic_block);
>> +	unregister_console(&drm_panic_console);
>> +	kfree(drm_panic_kmsgs);
>> +}
>> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
>> index bc7006e..4e84654 100644
>> --- a/include/drm/drmP.h
>> +++ b/include/drm/drmP.h
>> @@ -550,6 +550,28 @@ struct drm_driver {
>>   			  bool from_open);
>>   	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
>>   
>> +	/**
>> +	 * @panic:
>> +	 *
>> +	 * This function is called on panic() asking for a framebuffer to
>> +	 * display the panic messages on. It also needs the virtual address
>> +	 * of the backing buffer.
>> +	 * This function is optional.
>> +	 *
>> +	 * NOTE:
>> +	 *
>> +	 * This function is called in an atomic notifier chain and it cannot
>> +	 * sleep. Care must be taken so the machine is not killed even harder,
>> +	 * preventing output from going out on serial/netconsole.
>> +	 *
>> +	 * RETURNS:
>> +	 *
>> +	 * Framebuffer that should be used to display the panic messages,
>> +	 * alongside the virtual address of the backing buffer, or NULL if
>> +	 * the driver is unable to provide this.
>> +	 */
>> +	struct drm_framebuffer *(*panic)(struct drm_device *dev, void **vmem);
>> +
>>   	int (*debugfs_init)(struct drm_minor *minor);
>>   	void (*debugfs_cleanup)(struct drm_minor *minor);
>>   
>> -- 
>> 2.8.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 2/3] drm: Add panic handling

[Daniel Vetter]

On Thu, Aug 11, 2016 at 10:46:53PM +0200, Noralf Trønnes wrote:
> 
> Den 10.08.2016 11:15, skrev Daniel Vetter:
> > On Tue, Aug 09, 2016 at 07:45:41PM +0200, Noralf Trønnes wrote:
> > > This adds support for outputting kernel messages on panic().
> > > The drivers that supports it, provides a framebuffer that the
> > > messages can be rendered on.
> > > 
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > Thinking about how we should implement this in a full-blown driver, I
> > think we will need a bit more than this. Here's the additional
> > requirements I've come up that a driver more complex than sdrm would need
> > for reliable panic handling:
> > 
> > - Multiple outputs, with multiple framebuffer (or one framebuffer and
> >    multiple offsets within). And since one display might be dead we should
> >    try really hard to show the oops on all of them. Consequence: I think we
> >    need to also loop over all drm_crtc in a drm_device, to be able to
> >    support them all.
> 
> How about this:
> 
> /*
>  * The panic() function makes sure that only one CPU is allowed to run it's
>  * code. So this handler is only called once.
>  *
>  * Prior to calling the panic handlers, panic() calls smp_send_stop(). If
>  * that went well, there's only one CPU running, but this is no guarantee.
>  */
> static int drm_panic_handler(struct notifier_block *this, unsigned long ev,
>                              void *ptr)
> {
> <declarations>
> 
>         drm_for_each_device(drm, id) {
>                 if (!drm->driver ||
>                     !(drm->driver->driver_features & DRIVER_ATOMIC))
>                         continue;
> 
>                 drm_for_each_crtc(crtc, drm) {
>                         crtc_locked = ww_mutex_trylock(&crtc->mutex.mutex);

If you cant get the lock you must skip. Most likely someone else is
tampering with the crtc right now.

> 
>                         if (!crtc->enabled || !crtc->primary)
>                                 goto crtc_unlock;
> 
>                         if (!crtc->state || !crtc->state->active ||
>                             !crtc->state->state)
>                                 goto crtc_unlock;
> 
>                         state = crtc->state->state;
>                         plane = crtc->primary;
>                         plane_locked =
> ww_mutex_trylock(&plane->mutex.mutex);
> 
>                         plane_state =

Same here with the planes.

> drm_atomic_get_existing_plane_state(state, plane);
>                         if (!plane_state || !plane_state->visible)
>                                 goto plane_unlock;
> 
>                         fb = plane->fb;
> 
>                         if (!fb || !fb->funcs || !fb->funcs->panic_vmap)
>                                 goto plane_unlock;
> 
>                         /* only 8-bit wide fonts are supported */
>                         font = get_default_font(fb->width, fb->height,
> BIT(7), -1);
>                         if (!font)
>                                 goto plane_unlock;
> 
>                         vmap = fb->funcs->panic_vmap(fb);
>                         if (!vmap)
>                                 goto plane_unlock;
> 
>                 /*
>                  * How do I find the actual width/height from the plane
> state
>                  * as you mention further down?
>                  */
>                         width = ??
>                         height = ??

plane_state->src contains the clipped source rectangle in 16.16 fixed
point (on latest drm-misc, soon also in drm-next). Everything outside of
that area is not visible (on this plane/crtc combo).

> 
>                         pfb = &drm_panic_fbs[fbs++];
>                         pfb->fb = fb;
>                         pfb->vmap = vmap;
>                         pfb->font = font;
>                         pfb->cols = width / font->width;
>                         pfb->rows = height / font->height;
> 
>                 /*
>                  * how to unlock?
>                  * ww_mutex_unlock() doc says:
>                  * This function must not be used in interrupt context

It /should/ work, probably a copypaste error from ww_mutex_lock (that one
can't be used from interrupt context). Just try and then test this with
full CONFIG_PROVE_LOCKING enabled, that should catch any issue (if there
is one).

>                  */
>                 plane_unlock:
>                         if (plane_locked)
>                                 somehow_unlock();
>                 crtc_unlock:
>                         if (crtc_locked)
>                                 somehow_unlock();
>                 }
>         }
> 
>         drm_panic_active = true;
> }
> 
> 
> 
> > - With desktop gpus you pretty much always end up with a tiled
> >    framebuffer. And from an oops context it's going to be impossible to
> >    detile that quickly. I think for this we need a helper to draw x/y
> >    pixels (it's going to be dead slow, but meh), with a default
> >    implementation which assumes linear layout. Drivers could then frob x/y
> >    coordinates first in their own logic and call into that function.
> > 
> > - There's a good chance you're showing a video on a yuv buffer
> >    full-screen. If we don't bother too much with what it looks like, but
> >    only care about a foreground/background color then it's also easy to
> >    implement a draw_xy_pixel for these framebuffers. That means though that
> >    you can't clear things with memset, but that you need to call the
> >    interface func for each pixel. Yes this is going to be dead slow, but
> >    who cares as long as the oops eventually shows up ;-)
> 
> Let's give it a try, the yuv part is just guesswork:

I think for now we could just omit the yuv stuff. The idea behind draw_xy
was more that it would make it possible, not that we need it from day 1.

> 
> void drm_panic_draw_xy(struct drm_framebuffer *fb, void *vmap,
>                        int x, int y, bool foreground)
> {
>         void *dst;
> 
>         dst = vmap + fb->offsets[0] + (y * fb->pitches[0]);
> 
>         switch (fb->pixel_format & ~DRM_FORMAT_BIG_ENDIAN) {
> 
>         case DRM_FORMAT_C8:
> 
>         case DRM_FORMAT_RGB332:
>         case DRM_FORMAT_BGR233:
> 
>         case DRM_FORMAT_NV12:
>         case DRM_FORMAT_NV21:
>         case DRM_FORMAT_NV16:
>         case DRM_FORMAT_NV61:
>         case DRM_FORMAT_NV24:
>         case DRM_FORMAT_NV42:
> 
>         case DRM_FORMAT_YUV410:
>         case DRM_FORMAT_YVU410:
>         case DRM_FORMAT_YUV411:
>         case DRM_FORMAT_YVU411:
>         case DRM_FORMAT_YUV420:
>         case DRM_FORMAT_YVU420:
>         case DRM_FORMAT_YUV422:
>         case DRM_FORMAT_YVU422:
>         case DRM_FORMAT_YUV444:
>         case DRM_FORMAT_YVU444:
>                 dst += x;
>                 *(u8 *)dst = foreground ? 0xff : 0x00;
>                 break;
> 
>         case DRM_FORMAT_YUYV:
>         case DRM_FORMAT_YVYU:
>                 dst += x * sizeof(u32);
>                 put_unaligned(foreground ? 0xff00ff00 : 0x00000000, (u32
> *)dst);
>                 break;
> 
>         case DRM_FORMAT_UYVY:
>         case DRM_FORMAT_VYUY:
>                 dst += x * sizeof(u32);
>                 put_unaligned(foreground ? 0x00ff00ff : 0x00000000, (u32
> *)dst);
>                 break;
> 
>         case DRM_FORMAT_XRGB8888:
>         case DRM_FORMAT_ARGB8888:
>         case DRM_FORMAT_XBGR8888:
>         case DRM_FORMAT_ABGR8888:
>                 dst += x * sizeof(u32);
>                 put_unaligned(foreground ? 0x00ffffff : 0x00000000, (u32
> *)dst);
>                 break;
> 
>         /* and then the other rgb formats */
>         }
> }
> 
> > - The framebuffer size doesn't necessarily match the size of the visible
> >    part. Probably best if we dig this out from the plane_state directly
> >    (less fragile code in drivers). Relying on drm_plane_state means this
> >    will only work on atomic drivers (in legacy drivers this information is
> >    hidden in driver-private corners), but I think that's totally ok.
> > 
> > - 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).
> 
> Is this for the case where panic() fails to stop the other cpus?
> Or can preemption happen even though panic() calls local_irq_disable()?

Everything should be stopped, but if you can't get locks then some other
thread might be changing sw/hw state and leave a complete mess behind.
Better to not touch anything.

The other reason for unlocking is testing (that one I totally forgot). It
would be great if we could test all this code through some debugfs hack.
Best case would be firing up an nmi (don't ask me how, would need to look
too) or at least a hardirq and then call the drm panic handling code from
that. I think the only thing we need in addition to make this work is a
panic_vunmap callback to release the vmap region again (if needed). CMA
won't need this, so that hook should be optional.

But with that we can then exercise the panic code in e.g. a CI
environment, without killing the machine (unlike a real panic). And that
hopefully makes sure the code keeps working even in the future.

One more: If it's not possible to get into a real hardirq context the
simplest option would be to call local_irq_disable()/local_irq_enable()
around the call to the drm panic handler. That will at least tell lockdep
and all the built-in debug checks the right thing.

One more on top about locking: If we make the drm panic handling testable
like this we need to hold the crtc/plane locks until panic_vunmap has been
called.

> > - Multiple planes which might occlude the primary plane: I think we should
> >    just bother with the primary plane, and maybe give drivers a
> >    panic_commit hook where they could try to disable any additional planes.
> >    But that's not something we need in the first version here at all.
> > 
> > - I think some helpers for creating the vmap would be nice. Should be
> >    simple for cma-backed framebuffers, and also simple if you use a normal
> >    shmem backed gem buffer. For cma probably best if drivers don't even
> >    need to bother with this.
> 
> I don't know about shmem, but cma could look like this:
> 
> /**
>  * drm_fb_cma_panic_vmap - Helper function for the
>  *                         &drm_framebuffer_funcs->panic_vmap callback
>  * @fb: DRM framebuffer
>  *
>  * This function is used in a panic() situation to get to the virtual
> address
>  * of the backing buffer for rendering kernel messages.
>  * There's no need to set the &drm_framebuffer_funcs->panic_draw_xy
> callback,
>  * since the default implementation will suffice.
>  *
>  * Note: A PRIME imported buffer will _not_ have it's vaddr set.
>  *
>  * Returns:
>  * The virtual address of the backing object, or NULL.
>  */
> void *drm_fb_cma_panic_vmap(struct drm_framebuffer *fb)
> {
>         struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>         struct drm_gem_cma_object *cma_obj = fb_cma->obj[0];
> 
>         return cma_obj ? cma_obj->vaddr : NULL;
> }
> EXPORT_SYMBOL(drm_fb_cma_panic_vmap);
> 
>  static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
>         .destroy        = drm_fb_cma_destroy,
>         .create_handle  = drm_fb_cma_create_handle,
> +       .panic_vmap     = drm_fb_cma_panic_vmap,
>  };

Yup, that looks good.

> > - For driver convenience I think we could check a few things about
> >    visibility of each plane (e.g. crtc_state->active), and skip if there's
> >    no chance it can be seen. Doing less means less chances to blow up, and
> >    higher chances that the one screen which is on actually ends up with the
> >    oops on it.
> > 
> > - Minimal cleanup. I think we can just leak the vmapping, no harm in that.
> >    But the kms locks need to be dropped again. Dropping them again might
> >    increase the odds that the system limps along long enough for logs to
> >    hit the disks.
> > 
> > Taking this all together, I think the driver interface should be
> > restructured a bit, and that most of the handling should be on the
> > drm_framebuffer directly:
> > 
> > struct drm_framebuffer_funcs {
> > 	/* For vmapping the selected framebuffer in a panic context. Must
> > 	 * be super careful about locking (only trylocking allowed), can
> > 	 * return NULL if it didn't work out. The return value is an
> > 	 * opaque cookie which is passed to @panic_draw_xy, it can be
> > 	 * anything: vmap area, structure with more details, just a few
> > 	 * flags, ...
> > 	 */
> > 	void *(panic_vmap)(struct drm_framebuffer *fb);
> > 
> > 	/* For drawing pixels onto a framebuffer mapping with @panic_vmap.
> > 	 * This is optional, the default implementation assumes that vmap
> > 	 * points at a linear mapping of the framebuffer.
> > 	 */
> > 	void (panic_draw_xy)(struct drm_framebuffer *fb, void *vmap,
> > 			     int x, int y, bool foreground);
> > };
> > 
> > Ofc comments need to be fleshed out some more, but the idea is that all
> > the fb selection and lookup is handled in shared code (and with proper
> > locking, but only for atomic drivers).
> > 
> > Thoughts?
> 
> I like this, it appears to be very flexible.
> Will be interesting to see how fast/slow this is, in the linear case,
> compared to what I did.

Awesome, looking forward to the patches.
-Daniel

> 
> 
> Noralf.
> 
> > -Daniel
> > 
> > > ---
> > >   drivers/gpu/drm/Makefile       |   2 +-
> > >   drivers/gpu/drm/drm_drv.c      |   3 +
> > >   drivers/gpu/drm/drm_internal.h |   4 +
> > >   drivers/gpu/drm/drm_panic.c    | 606 +++++++++++++++++++++++++++++++++++++++++
> > >   include/drm/drmP.h             |  22 ++
> > >   5 files changed, 636 insertions(+), 1 deletion(-)
> > >   create mode 100644 drivers/gpu/drm/drm_panic.c
> > > 
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index eba32ad..ff04e41 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -12,7 +12,7 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
> > >   		drm_info.o drm_debugfs.o drm_encoder_slave.o \
> > >   		drm_trace_points.o drm_global.o drm_prime.o \
> > >   		drm_rect.o drm_vma_manager.o drm_flip_work.o \
> > > -		drm_modeset_lock.o drm_atomic.o drm_bridge.o
> > > +		drm_modeset_lock.o drm_atomic.o drm_bridge.o drm_panic.o
> > >   drm-$(CONFIG_COMPAT) += drm_ioc32.o
> > >   drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
> > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > index 3b14366..457ee91 100644
> > > --- a/drivers/gpu/drm/drm_drv.c
> > > +++ b/drivers/gpu/drm/drm_drv.c
> > > @@ -861,6 +861,8 @@ static int __init drm_core_init(void)
> > >   		goto err_p3;
> > >   	}
> > > +	drm_panic_init();
> > > +
> > >   	DRM_INFO("Initialized %s %d.%d.%d %s\n",
> > >   		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
> > >   	return 0;
> > > @@ -876,6 +878,7 @@ err_p1:
> > >   static void __exit drm_core_exit(void)
> > >   {
> > > +	drm_panic_exit();
> > >   	debugfs_remove(drm_debugfs_root);
> > >   	drm_sysfs_destroy();
> > > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> > > index b86dc9b..7463d9d 100644
> > > --- a/drivers/gpu/drm/drm_internal.h
> > > +++ b/drivers/gpu/drm/drm_internal.h
> > > @@ -90,6 +90,10 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
> > >   void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
> > >   void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
> > > +/* drm_panic.c */
> > > +void drm_panic_init(void);
> > > +void drm_panic_exit(void);
> > > +
> > >   /* drm_debugfs.c */
> > >   #if defined(CONFIG_DEBUG_FS)
> > >   int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> > > diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> > > new file mode 100644
> > > index 0000000..e185c9d
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/drm_panic.c
> > > @@ -0,0 +1,606 @@
> > > +/*
> > > + * Copyright 2016 Noralf Trønnes
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include <asm/unaligned.h>
> > > +#include <drm/drmP.h>
> > > +#include <linux/console.h>
> > > +#include <linux/debugfs.h>
> > > +#include <linux/font.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/seq_file.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/uaccess.h>
> > > +
> > > +struct drm_panic_fb {
> > > +	struct drm_framebuffer *fb;
> > > +	void *vmem;
> > > +	const struct font_desc *font;
> > > +	unsigned int cols;
> > > +	unsigned int rows;
> > > +	unsigned int xpos;
> > > +	unsigned int ypos;
> > > +};
> > > +
> > > +#define DRM_PANIC_MAX_FBS	64
> > > +static struct drm_panic_fb drm_panic_fbs[DRM_PANIC_MAX_FBS];
> > > +
> > > +#define DRM_PANIC_MAX_KMSGS	SZ_4K
> > > +static char *drm_panic_kmsgs;
> > > +static size_t drm_panic_kmsgs_pos;
> > > +
> > > +static bool drm_panic_active;
> > > +
> > > +static void drm_panic_log(const char *fmt, ...);
> > > +
> > > +static inline void drm_panic_draw_pixel(u8 *dst, u32 pixel_format, bool val)
> > > +{
> > > +	switch (pixel_format & ~DRM_FORMAT_BIG_ENDIAN) {
> > > +
> > > +	case DRM_FORMAT_C8:
> > > +	case DRM_FORMAT_RGB332:
> > > +	case DRM_FORMAT_BGR233:
> > > +		*dst = val ? 0xff : 0x00;
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_XRGB4444:
> > > +	case DRM_FORMAT_ARGB4444:
> > > +	case DRM_FORMAT_XBGR4444:
> > > +	case DRM_FORMAT_ABGR4444:
> > > +		put_unaligned(val ? 0x0fff : 0x0000, (u16 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_RGBX4444:
> > > +	case DRM_FORMAT_RGBA4444:
> > > +	case DRM_FORMAT_BGRX4444:
> > > +	case DRM_FORMAT_BGRA4444:
> > > +		put_unaligned(val ? 0xfff0 : 0x0000, (u16 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_XRGB1555:
> > > +	case DRM_FORMAT_ARGB1555:
> > > +	case DRM_FORMAT_XBGR1555:
> > > +	case DRM_FORMAT_ABGR1555:
> > > +		put_unaligned(val ? 0x7fff : 0x0000, (u16 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_RGBX5551:
> > > +	case DRM_FORMAT_RGBA5551:
> > > +	case DRM_FORMAT_BGRX5551:
> > > +	case DRM_FORMAT_BGRA5551:
> > > +		put_unaligned(val ? 0xfffe : 0x0000, (u16 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_RGB565:
> > > +	case DRM_FORMAT_BGR565:
> > > +		put_unaligned(val ? 0xffff : 0x0000, (u16 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_RGB888:
> > > +	case DRM_FORMAT_BGR888:
> > > +		dst[0] = val ? 0xff : 0x00;
> > > +		dst[1] = val ? 0xff : 0x00;
> > > +		dst[2] = val ? 0xff : 0x00;
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_XRGB8888:
> > > +	case DRM_FORMAT_ARGB8888:
> > > +	case DRM_FORMAT_XBGR8888:
> > > +	case DRM_FORMAT_ABGR8888:
> > > +		put_unaligned(val ? 0x00ffffff : 0x00000000, (u32 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_RGBX8888:
> > > +	case DRM_FORMAT_RGBA8888:
> > > +	case DRM_FORMAT_BGRX8888:
> > > +	case DRM_FORMAT_BGRA8888:
> > > +		put_unaligned(val ? 0xffffff00 : 0x00000000, (u32 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_XRGB2101010:
> > > +	case DRM_FORMAT_ARGB2101010:
> > > +	case DRM_FORMAT_XBGR2101010:
> > > +	case DRM_FORMAT_ABGR2101010:
> > > +		put_unaligned(val ? 0x3fffffff : 0x00000000, (u32 *)dst);
> > > +		break;
> > > +
> > > +	case DRM_FORMAT_RGBX1010102:
> > > +	case DRM_FORMAT_RGBA1010102:
> > > +	case DRM_FORMAT_BGRX1010102:
> > > +	case DRM_FORMAT_BGRA1010102:
> > > +		put_unaligned(val ? 0xfffffffc : 0x00000000, (u32 *)dst);
> > > +		break;
> > > +	}
> > > +}
> > > +
> > > +static void drm_panic_render(struct drm_panic_fb *pfb,
> > > +			     const char *text, unsigned int len)
> > > +{
> > > +	const struct font_desc *font = pfb->font;
> > > +	unsigned int pix_depth, pix_bpp, cpp;
> > > +	unsigned int col = pfb->xpos;
> > > +	unsigned int row = pfb->ypos;
> > > +	unsigned int i, h, w;
> > > +	void *dst, *pos;
> > > +	u8 fontline;
> > > +
> > > +	if ((row + 1) * font->height > pfb->fb->height)
> > > +		return;
> > > +
> > > +	if ((col + len) * font->width > pfb->fb->width)
> > > +		return;
> > > +
> > > +	drm_fb_get_bpp_depth(pfb->fb->pixel_format, &pix_depth, &pix_bpp);
> > > +	cpp = DIV_ROUND_UP(pix_bpp, 8);
> > > +
> > > +	/* TODO: should fb->offsets[0] be added here? */
> > > +	dst = pfb->vmem + (row * font->height * pfb->fb->pitches[0]) +
> > > +	      (col * font->width * cpp);
> > > +
> > > +	for (h = 0; h < font->height; h++) {
> > > +		pos = dst;
> > > +
> > > +		for (i = 0; i < len; i++) {
> > > +			fontline = *(u8 *)(font->data + text[i] * font->height + h);
> > > +
> > > +			for (w = 0; w < font->width; w++) {
> > > +				drm_panic_draw_pixel(pos, pfb->fb->pixel_format,
> > > +						     fontline & BIT(7 - w));
> > > +				pos += cpp;
> > > +			}
> > > +		}
> > > +
> > > +		dst += pfb->fb->pitches[0];
> > > +	}
> > > +}
> > > +
> > > +static void drm_panic_scroll_up(struct drm_panic_fb *pfb)
> > > +{
> > > +	void *src = pfb->vmem + (pfb->font->height * pfb->fb->pitches[0]);
> > > +	size_t len = (pfb->fb->height - pfb->font->height) *
> > > +		     pfb->fb->pitches[0];
> > > +
> > > +	drm_panic_log("%s\n", __func__);
> > > +
> > > +	memmove(pfb->vmem, src, len);
> > > +	memset(pfb->vmem + len, 0, pfb->font->height * pfb->fb->pitches[0]);
> > > +}
> > > +
> > > +static void drm_panic_clear_screen(struct drm_panic_fb *pfb)
> > > +{
> > > +	memset(pfb->vmem, 0, pfb->fb->height * pfb->fb->pitches[0]);
> > > +}
> > > +
> > > +static void drm_panic_log_msg(char *pre, const char *str, unsigned int len)
> > > +{
> > > +	char buf[512];
> > > +
> > > +	if (len > 510)
> > > +		len = 510;
> > > +
> > > +	memcpy(buf, str, len);
> > > +	buf[len] = '\n';
> > > +	buf[len + 1] = '\0';
> > > +
> > > +	drm_panic_log("%s%s", pre, buf);
> > > +}
> > > +
> > > +static void drm_panic_putcs_no_lf(struct drm_panic_fb *pfb,
> > > +				  const char *str, unsigned int len)
> > > +{
> > > +	drm_panic_log("%s(len=%u) x=%u, y=%u\n", __func__, len,
> > > +			pfb->xpos, pfb->ypos);
> > > +
> > > +	if (len <= 0)
> > > +		return;
> > > +
> > > +	drm_panic_log_msg("", str, len);
> > > +
> > > +	drm_panic_render(pfb, str, len);
> > > +
> > > +}
> > > +
> > > +static void drm_panic_putcs(struct drm_panic_fb *pfb,
> > > +			    const char *str, unsigned int num)
> > > +{
> > > +	unsigned int slen;
> > > +	int len = num;
> > > +	char *lf;
> > > +
> > > +	drm_panic_log("%s(num=%u)\n", __func__, num);
> > > +
> > > +	while (len > 0) {
> > > +
> > > +		if (pfb->ypos == pfb->rows) {
> > > +			pfb->ypos--;
> > > +			drm_panic_scroll_up(pfb);
> > > +		}
> > > +
> > > +		lf = strpbrk(str, "\n");
> > > +		if (lf)
> > > +			slen = lf - str;
> > > +		else
> > > +			slen = len;
> > > +
> > > +		if (pfb->xpos + slen > pfb->cols)
> > > +			slen = pfb->cols - pfb->xpos;
> > > +
> > > +		drm_panic_putcs_no_lf(pfb, str, slen);
> > > +
> > > +		len -= slen;
> > > +		str += slen;
> > > +		pfb->xpos += slen;
> > > +
> > > +		if (lf) {
> > > +			str++;
> > > +			len--;
> > > +			pfb->xpos = 0;
> > > +			pfb->ypos++;
> > > +		}
> > > +	}
> > > +}
> > > +
> > > +static void drm_panic_write(const char *str, unsigned int num)
> > > +{
> > > +	unsigned int i;
> > > +
> > > +	if (!num)
> > > +		return;
> > > +
> > > +	drm_panic_log("%s(num=%u)\n", __func__, num);
> > > +
> > > +	for (i = 0; i < DRM_PANIC_MAX_FBS; i++) {
> > > +		if (!drm_panic_fbs[i].fb)
> > > +			break;
> > > +		drm_panic_putcs(&drm_panic_fbs[i], str, num);
> > > +	}
> > > +}
> > > +
> > > +/* this one is serialized by console_lock() */
> > > +static void drm_panic_console_write(struct console *con,
> > > +				    const char *str, unsigned int num)
> > > +{
> > > +	unsigned int i;
> > > +
> > > +	drm_panic_log_msg("->", str, num);
> > > +
> > > +	/* Buffer up messages to be replayed on panic */
> > > +	if (!drm_panic_active) {
> > > +		for (i = 0; i < num; i++) {
> > > +			drm_panic_kmsgs[drm_panic_kmsgs_pos++] = *str++;
> > > +			if (drm_panic_kmsgs_pos == DRM_PANIC_MAX_KMSGS)
> > > +				drm_panic_kmsgs_pos = 0;
> > > +		}
> > > +		return;
> > > +	}
> > > +
> > > +	drm_panic_write(str, num);
> > > +}
> > > +
> > > +static struct console drm_panic_console = {
> > > +	.name =         "drmpanic",
> > > +	.write =        drm_panic_console_write,
> > > +	.flags =        CON_PRINTBUFFER | CON_ENABLED,
> > > +	.index =        0,
> > > +};
> > > +
> > > +/*
> > > + * The panic() function makes sure that only one CPU is allowed to run it's
> > > + * code. So when this handler is called, we're alone. No racing with
> > > + * console.write() is possible.
> > > + */
> > > +static int drm_panic_handler(struct notifier_block *this, unsigned long ev,
> > > +			     void *ptr)
> > > +{
> > > +	const struct font_desc *font;
> > > +	struct drm_framebuffer *fb;
> > > +	struct drm_panic_fb *pfb;
> > > +	struct drm_minor *minor;
> > > +	unsigned int fbs = 0;
> > > +	void *vmem;
> > > +	int i;
> > > +
> > > +	drm_panic_log("%s\n", __func__);
> > > +
> > > +	drm_panic_active = true;
> > > +
> > > +	drm_minor_for_each(minor, DRM_MINOR_LEGACY, i) {
> > > +		drm_panic_log("Found minor=%d\n", minor->index);
> > > +		if (!minor->dev || !minor->dev->driver ||
> > > +		    !minor->dev->driver->panic) {
> > > +			drm_panic_log("Skipping: No panic handler\n");
> > > +			continue;
> > > +		}
> > > +
> > > +		fb = minor->dev->driver->panic(minor->dev, &vmem);
> > > +		if (!fb) {
> > > +			drm_panic_log("Skipping: Driver returned NULL\n");
> > > +			continue;
> > > +		}
> > > +
> > > +		if (!fb || !vmem || fb->dev != minor->dev || !fb->pitches[0]) {
> > > +			drm_panic_log("Skipping: Failed check\n");
> > > +			continue;
> > > +		}
> > > +
> > > +		/* only 8-bit wide fonts are supported */
> > > +		font = get_default_font(fb->width, fb->height, BIT(7), -1);
> > > +		if (!font) {
> > > +			drm_panic_log("Skipping: No font available\n");
> > > +			continue;
> > > +		}
> > > +
> > > +		pfb = &drm_panic_fbs[fbs++];
> > > +
> > > +		pfb->fb = fb;
> > > +		pfb->vmem = vmem;
> > > +		pfb->font = font;
> > > +		pfb->cols = fb->width / font->width;
> > > +		pfb->rows = fb->height / font->height;
> > > +
> > > +		drm_panic_clear_screen(pfb);
> > > +
> > > +		drm_panic_log("    %ux%u -> %ux%u, %s, %s\n", fb->width,
> > > +				fb->height, pfb->cols, pfb->rows, font->name,
> > > +				drm_get_format_name(fb->pixel_format));
> > > +	}
> > > +
> > > +	if (drm_panic_kmsgs[0]) {
> > > +		/* safeguard in case we interrupted drm_panic_console_write */
> > > +		if (drm_panic_kmsgs_pos >= DRM_PANIC_MAX_KMSGS)
> > > +			drm_panic_kmsgs_pos = 0;
> > > +
> > > +		drm_panic_write(&drm_panic_kmsgs[drm_panic_kmsgs_pos],
> > > +				DRM_PANIC_MAX_KMSGS - drm_panic_kmsgs_pos);
> > > +		drm_panic_write(drm_panic_kmsgs, drm_panic_kmsgs_pos);
> > > +	}
> > > +
> > > +	return NOTIFY_DONE;
> > > +}
> > > +
> > > +static struct notifier_block drm_panic_block = {
> > > +	.notifier_call = drm_panic_handler,
> > > +};
> > > +
> > > +
> > > +
> > > +#ifdef CONFIG_DEBUG_FS
> > > +
> > > +/* Out of band logging is useful at least in the initial development phase */
> > > +#define DRM_PANIC_LOG_SIZE	SZ_64K
> > > +#define DRM_PANIC_LOG_LINE	128
> > > +#define DRM_PANIC_LOG_ENTRIES	(DRM_PANIC_LOG_SIZE / DRM_PANIC_LOG_LINE)
> > > +
> > > +static char *log_buf;
> > > +static size_t log_pos;
> > > +static struct dentry *drm_panic_logfs_root;
> > > +
> > > +static void drm_panic_log(const char *fmt, ...)
> > > +{
> > > +	va_list args;
> > > +	u32 rem_nsec;
> > > +	char *text;
> > > +	size_t len;
> > > +	u64 sec;
> > > +
> > > +	if (!log_buf || oops_in_progress)
> > > +		return;
> > > +
> > > +	va_start(args, fmt);
> > > +
> > > +	if (log_pos >= DRM_PANIC_LOG_ENTRIES)
> > > +		log_pos = 0;
> > > +
> > > +	text = log_buf + (log_pos++ * DRM_PANIC_LOG_LINE);
> > > +	if (log_pos == DRM_PANIC_LOG_ENTRIES)
> > > +		log_pos = 0;
> > > +
> > > +	sec = div_u64_rem(local_clock(), 1000000000, &rem_nsec);
> > > +
> > > +	len = scnprintf(text, DRM_PANIC_LOG_LINE, "[%5llu.%06u] ", sec,
> > > +			rem_nsec / 1000);
> > > +
> > > +	vscnprintf(text + len, DRM_PANIC_LOG_LINE - len, fmt, args);
> > > +
> > > +	/* Make sure to always have a newline in case of overflow */
> > > +	if (text[DRM_PANIC_LOG_LINE - 2] != '\0')
> > > +		text[DRM_PANIC_LOG_LINE - 2] = '\n';
> > > +
> > > +	va_end(args);
> > > +}
> > > +
> > > +static int drm_panic_log_show(struct seq_file *m, void *v)
> > > +{
> > > +	size_t pos = log_pos;
> > > +	unsigned int i;
> > > +	char *text;
> > > +
> > > +	for (i = 0; i < DRM_PANIC_LOG_ENTRIES; i++) {
> > > +		text = log_buf + (pos++ * DRM_PANIC_LOG_LINE);
> > > +		if (pos == DRM_PANIC_LOG_ENTRIES)
> > > +			pos = 0;
> > > +		if (*text == '\0')
> > > +			continue;
> > > +		seq_puts(m, text);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int drm_panic_log_open(struct inode *inode, struct file *file)
> > > +{
> > > +	return single_open(file, drm_panic_log_show, NULL);
> > > +}
> > > +
> > > +static const struct file_operations drm_panic_log_ops = {
> > > +	.owner   = THIS_MODULE,
> > > +	.open    = drm_panic_log_open,
> > > +	.read    = seq_read,
> > > +	.llseek  = seq_lseek,
> > > +	.release = single_release,
> > > +};
> > > +
> > > +/*
> > > + * Fake/simulate panic() at different levels:
> > > + * 1: only trigger panic handling internally
> > > + * 2: add local_irq_disable()
> > > + * 3: add bust_spinlocks();
> > > + * 100: don't fake it, do call panic()
> > > + */
> > > +static int drm_text_fake_panic(unsigned int level)
> > > +{
> > > +#ifndef MODULE
> > > +	int old_loglevel = console_loglevel;
> > > +#endif
> > > +
> > > +	if (!level && level != 100 && level > 3)
> > > +		return -EINVAL;
> > > +
> > > +	if (level == 100)
> > > +		panic("TESTING");
> > > +
> > > +	if (level > 1)
> > > +		local_irq_disable();
> > > +
> > > +#ifndef MODULE
> > > +	console_verbose();
> > > +#endif
> > > +	if (level > 2)
> > > +		bust_spinlocks(1);
> > > +
> > > +	pr_emerg("Kernel panic - not syncing: FAKING=%u, oops_in_progress=%d\n",
> > > +		 level, oops_in_progress);
> > > +
> > > +#ifdef CONFIG_DEBUG_BUGVERBOSE
> > > +	dump_stack();
> > > +#endif
> > > +	/* simulate calling panic_notifier_list */
> > > +	drm_panic_handler(NULL, 0, NULL);
> > > +
> > > +	if (level > 2)
> > > +		bust_spinlocks(0);
> > > +
> > > +#ifndef MODULE
> > > +	console_flush_on_panic();
> > > +#endif
> > > +	pr_emerg("---[ end Kernel panic - not syncing: FAKING\n");
> > > +
> > > +	if (level > 1)
> > > +		local_irq_enable();
> > > +
> > > +#ifndef MODULE
> > > +	console_loglevel = old_loglevel;
> > > +#endif
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static ssize_t drm_text_panic_write(struct file *file,
> > > +				    const char __user *user_buf,
> > > +				    size_t count, loff_t *ppos)
> > > +{
> > > +	unsigned long long val;
> > > +	ssize_t ret = 0;
> > > +	char buf[24];
> > > +	size_t size;
> > > +
> > > +	size = min(sizeof(buf) - 1, count);
> > > +	if (copy_from_user(buf, user_buf, size))
> > > +		return -EFAULT;
> > > +
> > > +	buf[size] = '\0';
> > > +	ret = kstrtoull(buf, 0, &val);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = drm_text_fake_panic(val);
> > > +
> > > +	return ret < 0 ? ret : count;
> > > +}
> > > +
> > > +static const struct file_operations drm_text_panic_ops = {
> > > +	.write =        drm_text_panic_write,
> > > +	.open =         simple_open,
> > > +	.llseek =       default_llseek,
> > > +};
> > > +
> > > +static int drm_panic_logfs_init(void)
> > > +{
> > > +	drm_panic_logfs_root = debugfs_create_dir("drm-panic", NULL);
> > > +	if (!drm_panic_logfs_root)
> > > +		return -ENOMEM;
> > > +
> > > +	if (!debugfs_create_file("log", S_IRUGO, drm_panic_logfs_root, NULL,
> > > +			    &drm_panic_log_ops))
> > > +		goto err_remove;
> > > +
> > > +	log_buf = kzalloc(DRM_PANIC_LOG_SIZE, GFP_KERNEL);
> > > +	if (!log_buf)
> > > +		goto err_remove;
> > > +
> > > +	debugfs_create_file("panic", S_IWUSR, drm_panic_logfs_root, NULL,
> > > +			    &drm_text_panic_ops);
> > > +
> > > +	return 0;
> > > +
> > > +err_remove:
> > > +	debugfs_remove_recursive(drm_panic_logfs_root);
> > > +
> > > +	return -ENOMEM;
> > > +}
> > > +
> > > +static void drm_panic_logfs_exit(void)
> > > +{
> > > +	debugfs_remove_recursive(drm_panic_logfs_root);
> > > +	kfree(log_buf);
> > > +	log_buf = NULL;
> > > +}
> > > +
> > > +#else
> > > +
> > > +static int drm_panic_logfs_init(void)
> > > +{
> > > +}
> > > +
> > > +static void drm_panic_logfs_exit(void)
> > > +{
> > > +}
> > > +
> > > +static void drm_panic_log(const char *fmt, ...)
> > > +{
> > > +}
> > > +
> > > +#endif
> > > +
> > > +
> > > +void __init drm_panic_init(void)
> > > +{
> > > +	drm_panic_kmsgs = kzalloc(DRM_PANIC_MAX_KMSGS, GFP_KERNEL);
> > > +	if (!drm_panic_kmsgs) {
> > > +		DRM_ERROR("Failed to setup panic handler\n");
> > > +		return;
> > > +	}
> > > +
> > > +	drm_panic_logfs_init();
> > > +drm_panic_log("%s\n", __func__);
> > > +	register_console(&drm_panic_console);
> > > +	atomic_notifier_chain_register(&panic_notifier_list,
> > > +				       &drm_panic_block);
> > > +}
> > > +
> > > +void __exit drm_panic_exit(void)
> > > +{
> > > +	if (!drm_panic_kmsgs)
> > > +		return;
> > > +
> > > +	drm_panic_logfs_exit();
> > > +	atomic_notifier_chain_unregister(&panic_notifier_list,
> > > +					 &drm_panic_block);
> > > +	unregister_console(&drm_panic_console);
> > > +	kfree(drm_panic_kmsgs);
> > > +}
> > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > > index bc7006e..4e84654 100644
> > > --- a/include/drm/drmP.h
> > > +++ b/include/drm/drmP.h
> > > @@ -550,6 +550,28 @@ struct drm_driver {
> > >   			  bool from_open);
> > >   	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
> > > +	/**
> > > +	 * @panic:
> > > +	 *
> > > +	 * This function is called on panic() asking for a framebuffer to
> > > +	 * display the panic messages on. It also needs the virtual address
> > > +	 * of the backing buffer.
> > > +	 * This function is optional.
> > > +	 *
> > > +	 * NOTE:
> > > +	 *
> > > +	 * This function is called in an atomic notifier chain and it cannot
> > > +	 * sleep. Care must be taken so the machine is not killed even harder,
> > > +	 * preventing output from going out on serial/netconsole.
> > > +	 *
> > > +	 * RETURNS:
> > > +	 *
> > > +	 * Framebuffer that should be used to display the panic messages,
> > > +	 * alongside the virtual address of the backing buffer, or NULL if
> > > +	 * the driver is unable to provide this.
> > > +	 */
> > > +	struct drm_framebuffer *(*panic)(struct drm_device *dev, void **vmem);
> > > +
> > >   	int (*debugfs_init)(struct drm_minor *minor);
> > >   	void (*debugfs_cleanup)(struct drm_minor *minor);
> > > -- 
> > > 2.8.2
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/3] drm: Add a way to iterate over minors

[David Herrmann]

Hey

On Wed, Aug 10, 2016 at 4:34 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Aug 10, 2016 at 4:27 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
>> Den 10.08.2016 10:43, skrev Daniel Vetter:
>>>
>>> On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote:
>>>>
>>>> This adds a way for in-kernel users to iterate over the available
>>>> DRM minors. The implementation is oops safe so the panic code
>>>> can safely use it.
>>>>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>
>>> Why iterate over minors? I'd kinda have expected we'd iterate over devices
>>> instead ... And looking ahead, that seems to be what you actually want.
>>
>>
>> I did this because I couldn't find a list of drm_devices anywhere. Only
>> minors.
>> But I can do a drm_for_each_device() based on the minor "list".
>
> Hm, the drm drivers are all part of the drm class, we should be able
> to iterate them I think. Otherwise let's just add a new list instead
> of jumping through hoops.
> New list with it's own lock (which we then trylock from the panic
> handler) would be safest I think.

You can use class_for_each_device() (or register a class_interface to
be notified of new entries as well). However, that will iterate all
minors, so you need to filter for the primary one. I think that should
be sufficient, no need for a separate list.

Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel