[v4 00/10] drm/msm: GPU crash state

[v4 00/10] drm/msm: GPU crash state

[Jordan Crouse]

This is revision 4 implementing a GPU crash state for drm/msm
(https://patchwork.freedesktop.org/series/36097/).  I think its mature enough
to pull out of RFC status and think about merging.

The goal is to store and provide enough information to debug software
and hardware issues on the Adreno hardware in a semi human-readable
format that can also be parsed by scripts.

THe full set of changes here capture basic information about the GPU, the
status and contents of the ringbuffers, a snapshot of the current register state
and the active buffers from the hanging submit.

The data is printed with devcoredump.  For example, after a hang you can get
the data from /sys/class/devcoredump/devcdX/data where X is a unique number.

You can see an example of the output for a simple invalid opcode error on the
db820c here: https://hastebin.com/yivozimoki.bash

v4: Add buffer dump for the active submit. Fix refcount issue with devcoredump.
Change header for a5xx registers to registers-hlsq because I'm told YAML
requires unique tags.
v3: Make recommended changes to ascii85 per Chris Wilson. Use devcoredump to
dump crash states as suggested by Bjorn Andersson and add a new drm_print
facility to facilitate that. Remove the now obsolete 'crash' debugfs node.
Add documentation for the crash dump output.

v2: Convert output to yaml, use ascii85 to dump ringbuffer contents.

Jordan Crouse (10):
  include: Move ascii85 functions from i915 to linux/ascii85.h
  drm: drm_printer: Add printer for devcoredump
  drm/msm/gpu: Capture the state of the GPU
  drm/msm/gpu: Convert the GPU show function to use the GPU state
  drm/msm/gpu: Rearrange the code that collects the task during a hang
  drm/msm/gpu: Capture the GPU state on a GPU hang
  drm/msm/adreno: Convert the show/crash file format
  drm/msm/adreno: Add ringbuffer data to the GPU state
  drm/msm/adreno: Add a5xx specific registers for the GPU state
  drm/msm/gpu: Add the buffer objects from the submit to the crash dump

 Documentation/gpu/drm-msm-crash-dump.txt |  46 ++++++
 drivers/gpu/drm/drm_print.c              |  54 +++++++
 drivers/gpu/drm/i915/i915_gpu_error.c    |  35 +----
 drivers/gpu/drm/msm/Kconfig              |   1 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c    |  30 ++--
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c    |  22 ++-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c    | 243 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 181 ++++++++++++++++++++---
 drivers/gpu/drm/msm/adreno/adreno_gpu.h  |   7 +-
 drivers/gpu/drm/msm/msm_debugfs.c        |  24 ++-
 drivers/gpu/drm/msm/msm_gpu.c            | 143 ++++++++++++++++--
 drivers/gpu/drm/msm/msm_gpu.h            |  67 ++++++++-
 include/drm/drm_print.h                  |  27 ++++
 include/linux/ascii85.h                  |  39 +++++
 14 files changed, 821 insertions(+), 98 deletions(-)
 create mode 100644 Documentation/gpu/drm-msm-crash-dump.txt
 create mode 100644 include/linux/ascii85.h

-- 
2.16.1

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

[PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Jordan Crouse]

The i915 DRM driver very cleverly used ascii85 encoding for their
GPU state file. Move the encode functions to a general header file to
support other drivers that might be interested in the same
functionality.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
 drivers/gpu/drm/i915/i915_gpu_error.c | 35 ++++---------------------------
 include/linux/ascii85.h               | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 31 deletions(-)
 create mode 100644 include/linux/ascii85.h

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 67c902412193..969d967e58c7 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -31,7 +31,7 @@
 #include <linux/stop_machine.h>
 #include <linux/zlib.h>
 #include <drm/drm_print.h>
-
+#include <linux/ascii85.h>
 #include "i915_drv.h"
 
 static inline const struct intel_engine_cs *
@@ -518,35 +518,12 @@ void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...)
 	va_end(args);
 }
 
-static int
-ascii85_encode_len(int len)
-{
-	return DIV_ROUND_UP(len, 4);
-}
-
-static bool
-ascii85_encode(u32 in, char *out)
-{
-	int i;
-
-	if (in == 0)
-		return false;
-
-	out[5] = '\0';
-	for (i = 5; i--; ) {
-		out[i] = '!' + in % 85;
-		in /= 85;
-	}
-
-	return true;
-}
-
 static void print_error_obj(struct drm_i915_error_state_buf *m,
 			    struct intel_engine_cs *engine,
 			    const char *name,
 			    struct drm_i915_error_object *obj)
 {
-	char out[6];
+	char out[ASCII85_BUFSZ];
 	int page;
 
 	if (!obj)
@@ -568,12 +545,8 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
 			len -= obj->unused;
 		len = ascii85_encode_len(len);
 
-		for (i = 0; i < len; i++) {
-			if (ascii85_encode(obj->pages[page][i], out))
-				err_puts(m, out);
-			else
-				err_puts(m, "z");
-		}
+		for (i = 0; i < len; i++)
+			error_puts(m, ascii85_encode(obj->pages[page][i], out));
 	}
 	err_puts(m, "\n");
 }
diff --git a/include/linux/ascii85.h b/include/linux/ascii85.h
new file mode 100644
index 000000000000..322bbed731ae
--- /dev/null
+++ b/include/linux/ascii85.h
@@ -0,0 +1,39 @@
+
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (c) 2008 Intel Corporation
+ * Copyright (c) The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _ASCII85_H_
+#define _ASCII85_H_
+
+#include <linux/kernel.h>
+
+#define ASCII85_BUFSZ 6
+
+static inline long
+ascii85_encode_len(long len)
+{
+	return DIV_ROUND_UP(len, 4);
+}
+
+static inline char *
+ascii85_encode(u32 in, char *out)
+{
+	int i;
+
+	if (in == 0)
+		return "z";
+
+	out[5] = '\0';
+	for (i = 5; i--; ) {
+		out[i] = '!' + in % 85;
+		in /= 85;
+	}
+
+	return out;
+}
+
+#endif
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 02/10] drm: drm_printer: Add printer for devcoredump

[Jordan Crouse]

Add a drm printer suitable for use with the read callback for
devcoredump or any other file operation read() function that
isn't otherwise covered by seq_file.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/drm_print.c | 54 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_print.h     | 27 +++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 781518fd88e3..f6efde48f8b3 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -30,6 +30,60 @@
 #include <drm/drmP.h>
 #include <drm/drm_print.h>
 
+void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf)
+{
+	struct drm_print_iterator *iterator = p->arg;
+
+	ssize_t len;
+
+	if (!iterator->remain)
+		return;
+
+	if (iterator->offset < iterator->start) {
+		char *buf;
+		ssize_t copy;
+
+		/* Figure out how big the string will be */
+		len = snprintf(NULL, 0, "%pV", vaf);
+
+		if (iterator->offset + len <= iterator->start) {
+			iterator->offset += len;
+			return;
+		}
+
+		/* Print the string into a temporary buffer */
+		buf = kmalloc(len + 1,
+			GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
+		if (!buf)
+			return;
+
+		snprintf(buf, len + 1, "%pV", vaf);
+
+		copy = len - (iterator->start - iterator->offset);
+
+		if (copy > iterator->remain)
+			copy = iterator->remain;
+
+		/* Copy out the bit of the string that we need */
+		memcpy(iterator->data,
+			buf + (iterator->start - iterator->offset), copy);
+
+		iterator->offset = iterator->start + copy;
+		iterator->remain -= copy;
+
+		kfree(buf);
+	} else {
+		ssize_t pos = iterator->offset - iterator->start;
+
+		len = scnprintf(((char *) iterator->data) + pos,
+			iterator->remain, "%pV", vaf);
+
+		iterator->offset += len;
+		iterator->remain -= len;
+	}
+}
+EXPORT_SYMBOL(__drm_printfn_coredump);
+
 void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf)
 {
 	seq_printf(p->arg, "%pV", vaf);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2a4a42e59a47..29eee5175eac 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -73,6 +73,7 @@ struct drm_printer {
 	const char *prefix;
 };
 
+void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
@@ -104,6 +105,32 @@ drm_vprintf(struct drm_printer *p, const char *fmt, va_list *va)
 #define drm_printf_indent(printer, indent, fmt, ...) \
 	drm_printf((printer), "%.*s" fmt, (indent), "\t\t\t\t\tX", ##__VA_ARGS__)
 
+struct drm_print_iterator {
+	void *data;
+
+	ssize_t start;
+	ssize_t offset;
+	ssize_t remain;
+};
+
+/**
+ * drm_coredump_printer - construct a &drm_printer that can output to a buffer
+ * from the read function for devcoredump
+ * @iter: A pointer to a struct drm_print_iterator for the read instance
+ *
+ * RETURNS:
+ * The &drm_printer object
+ */
+static inline struct drm_printer
+drm_coredump_printer(struct drm_print_iterator *iter)
+{
+	struct drm_printer p = {
+		.printfn = __drm_printfn_coredump,
+		.arg = iter,
+	};
+	return p;
+}
+
 /**
  * drm_seq_file_printer - construct a &drm_printer that outputs to &seq_file
  * @f:  the &struct seq_file to output to
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 03/10] drm/msm/gpu: Capture the state of the GPU

[Jordan Crouse]

Add the infrastructure to capture the state current state of the
GPU and store it in memory. This is useful for storing the state
of a hung GPU so it can be dumped later.

For now grab the same basic ringbuffer information and registers
that are provided by the debugfs 'gpu' node but obviously this can
be extended to capture a much larger set of GPU information.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c   | 15 +++++++++
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   | 14 +++++++++
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 22 ++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 54 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.h |  3 ++
 drivers/gpu/drm/msm/msm_gpu.h           | 19 ++++++++++++
 6 files changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index 3ebbeb3a9b68..b707b5bca9ab 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -427,6 +427,19 @@ static void a3xx_dump(struct msm_gpu *gpu)
 			gpu_read(gpu, REG_A3XX_RBBM_STATUS));
 	adreno_dump(gpu);
 }
+
+static struct msm_gpu_state *a3xx_gpu_state_get(struct msm_gpu *gpu)
+{
+	struct msm_gpu_state *state = adreno_gpu_state_get(gpu);
+
+	if (IS_ERR(state))
+		return state;
+
+	state->rbbm_status = gpu_read(gpu, REG_A3XX_RBBM_STATUS);
+
+	return state;
+}
+
 /* Register offset defines for A3XX */
 static const unsigned int a3xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
 	REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
@@ -453,6 +466,8 @@ static const struct adreno_gpu_funcs funcs = {
 #ifdef CONFIG_DEBUG_FS
 		.show = a3xx_show,
 #endif
+		.gpu_state_get = a3xx_gpu_state_get,
+		.gpu_state_put = adreno_gpu_state_put,
 	},
 };
 
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 16d3d596638e..17e97ebc1077 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -465,6 +465,18 @@ static void a4xx_show(struct msm_gpu *gpu, struct seq_file *m)
 }
 #endif
 
+static struct msm_gpu_state *a4xx_gpu_state_get(struct msm_gpu *gpu)
+{
+	struct msm_gpu_state *state = adreno_gpu_state_get(gpu);
+
+	if (IS_ERR(state))
+		return state;
+
+	state->rbbm_status = gpu_read(gpu, REG_A4XX_RBBM_STATUS);
+
+	return state;
+}
+
 /* Register offset defines for A4XX, in order of enum adreno_regs */
 static const unsigned int a4xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
 	REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_A4XX_CP_RB_BASE),
@@ -541,6 +553,8 @@ static const struct adreno_gpu_funcs funcs = {
 #ifdef CONFIG_DEBUG_FS
 		.show = a4xx_show,
 #endif
+		.gpu_state_get = a4xx_gpu_state_get,
+		.gpu_state_put = adreno_gpu_state_put,
 	},
 	.get_timestamp = a4xx_get_timestamp,
 };
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index a4f68affc13b..08f25798abdb 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1195,6 +1195,26 @@ static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 	return 0;
 }
 
+static struct msm_gpu_state *a5xx_gpu_state_get(struct msm_gpu *gpu)
+{
+	struct msm_gpu_state *state;
+
+	/*
+	 * Temporarily disable hardware clock gating before going into
+	 * adreno_show to avoid issues while reading the registers
+	 */
+	a5xx_set_hwcg(gpu, false);
+
+	state = adreno_gpu_state_get(gpu);
+
+	if (!IS_ERR(state))
+		state->rbbm_status = gpu_read(gpu, REG_A5XX_RBBM_STATUS);
+
+	a5xx_set_hwcg(gpu, true);
+
+	return state;
+}
+
 #ifdef CONFIG_DEBUG_FS
 static void a5xx_show(struct msm_gpu *gpu, struct seq_file *m)
 {
@@ -1244,6 +1264,8 @@ static const struct adreno_gpu_funcs funcs = {
 		.debugfs_init = a5xx_debugfs_init,
 #endif
 		.gpu_busy = a5xx_gpu_busy,
+		.gpu_state_get = a5xx_gpu_state_get,
+		.gpu_state_put = adreno_gpu_state_put,
 	},
 	.get_timestamp = a5xx_get_timestamp,
 };
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 17d0506d058c..b2ccaf25767c 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -368,6 +368,60 @@ bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
 	return false;
 }
 
+struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu)
+{
+	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+	struct msm_gpu_state *state;
+	int i, count = 0;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	do_gettimeofday(&state->time);
+
+	for (i = 0; i < gpu->nr_rings; i++) {
+		state->ring[i].fence = gpu->rb[i]->memptrs->fence;
+		state->ring[i].seqno = gpu->rb[i]->seqno;
+		state->ring[i].rptr = get_rptr(adreno_gpu, gpu->rb[i]);
+		state->ring[i].wptr = get_wptr(gpu->rb[i]);
+	}
+
+	/* Count the number of registers */
+	for (i = 0; adreno_gpu->registers[i] != ~0; i += 2)
+		count += adreno_gpu->registers[i + 1] -
+			adreno_gpu->registers[i] + 1;
+
+	state->registers = kcalloc(count * 2, sizeof(u32), GFP_KERNEL);
+	if (state->registers) {
+		int pos = 0;
+
+		for (i = 0; adreno_gpu->registers[i] != ~0; i += 2) {
+			uint32_t start = adreno_gpu->registers[i];
+			uint32_t end   = adreno_gpu->registers[i+1];
+			uint32_t addr;
+
+			for (addr = start; addr <= end; addr++) {
+				state->registers[pos++] = addr;
+				state->registers[pos++] = gpu_read(gpu, addr);
+			}
+		}
+
+		state->nr_registers = count;
+	}
+
+	return state;
+}
+
+void adreno_gpu_state_put(struct msm_gpu_state *state)
+{
+	if (IS_ERR_OR_NULL(state))
+		return;
+
+	kfree(state->registers);
+	kfree(state);
+}
+
 #ifdef CONFIG_DEBUG_FS
 void adreno_show(struct msm_gpu *gpu, struct seq_file *m)
 {
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index d6b0e7b813f4..0beb078eb658 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -228,6 +228,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 void adreno_gpu_cleanup(struct adreno_gpu *gpu);
 
 
+struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu);
+void adreno_gpu_state_put(struct msm_gpu_state *state);
+
 /* ringbuffer helpers (the parts that are adreno specific) */
 
 static inline void
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index b8241179175a..4be72a612bec 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -27,6 +27,7 @@
 
 struct msm_gem_submit;
 struct msm_gpu_perfcntr;
+struct msm_gpu_state;
 
 struct msm_gpu_config {
 	const char *ioname;
@@ -69,6 +70,8 @@ struct msm_gpu_funcs {
 	int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor);
 #endif
 	int (*gpu_busy)(struct msm_gpu *gpu, uint64_t *value);
+	struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu);
+	void (*gpu_state_put)(struct msm_gpu_state *state);
 };
 
 struct msm_gpu {
@@ -175,6 +178,22 @@ struct msm_gpu_submitqueue {
 	struct kref ref;
 };
 
+struct msm_gpu_state {
+	struct timeval time;
+
+	struct {
+		u32 fence;
+		u32 seqno;
+		u32 rptr;
+		u32 wptr;
+	} ring[MSM_GPU_MAX_RINGS];
+
+	int nr_registers;
+	u32 *registers;
+
+	u32 rbbm_status;
+};
+
 static inline void gpu_write(struct msm_gpu *gpu, u32 reg, u32 data)
 {
 	msm_writel(data, gpu->mmio + (reg << 2));
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 04/10] drm/msm/gpu: Convert the GPU show function to use the GPU state

[Jordan Crouse]

Convert the existing GPU show function to use the GPU state to
dump the information rather than reading it directly from the hardware.
This will require an additional step to capture the state before
dumping it for the existing nodes but it will greatly facilitate reusing
the same code for dumping a previously captured state from a GPU hang.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c   | 11 +----------
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   | 12 +-----------
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   | 18 +-----------------
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 30 +++++++++++++-----------------
 drivers/gpu/drm/msm/adreno/adreno_gpu.h |  4 ++--
 drivers/gpu/drm/msm/msm_debugfs.c       | 21 +++++++++++++++------
 drivers/gpu/drm/msm/msm_gpu.h           |  3 ++-
 7 files changed, 35 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index b707b5bca9ab..4cffec2b6adc 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -411,15 +411,6 @@ static const unsigned int a3xx_registers[] = {
 	~0   /* sentinel */
 };
 
-#ifdef CONFIG_DEBUG_FS
-static void a3xx_show(struct msm_gpu *gpu, struct seq_file *m)
-{
-	seq_printf(m, "status:   %08x\n",
-			gpu_read(gpu, REG_A3XX_RBBM_STATUS));
-	adreno_show(gpu, m);
-}
-#endif
-
 /* would be nice to not have to duplicate the _show() stuff with printk(): */
 static void a3xx_dump(struct msm_gpu *gpu)
 {
@@ -464,7 +455,7 @@ static const struct adreno_gpu_funcs funcs = {
 		.irq = a3xx_irq,
 		.destroy = a3xx_destroy,
 #ifdef CONFIG_DEBUG_FS
-		.show = a3xx_show,
+		.show = adreno_show,
 #endif
 		.gpu_state_get = a3xx_gpu_state_get,
 		.gpu_state_put = adreno_gpu_state_put,
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 17e97ebc1077..95f08c22e8d7 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -455,16 +455,6 @@ static const unsigned int a4xx_registers[] = {
 	~0 /* sentinel */
 };
 
-#ifdef CONFIG_DEBUG_FS
-static void a4xx_show(struct msm_gpu *gpu, struct seq_file *m)
-{
-	seq_printf(m, "status:   %08x\n",
-			gpu_read(gpu, REG_A4XX_RBBM_STATUS));
-	adreno_show(gpu, m);
-
-}
-#endif
-
 static struct msm_gpu_state *a4xx_gpu_state_get(struct msm_gpu *gpu)
 {
 	struct msm_gpu_state *state = adreno_gpu_state_get(gpu);
@@ -551,7 +541,7 @@ static const struct adreno_gpu_funcs funcs = {
 		.irq = a4xx_irq,
 		.destroy = a4xx_destroy,
 #ifdef CONFIG_DEBUG_FS
-		.show = a4xx_show,
+		.show = adreno_show,
 #endif
 		.gpu_state_get = a4xx_gpu_state_get,
 		.gpu_state_put = adreno_gpu_state_put,
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 08f25798abdb..b0910bbe5190 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1215,22 +1215,6 @@ static struct msm_gpu_state *a5xx_gpu_state_get(struct msm_gpu *gpu)
 	return state;
 }
 
-#ifdef CONFIG_DEBUG_FS
-static void a5xx_show(struct msm_gpu *gpu, struct seq_file *m)
-{
-	seq_printf(m, "status:   %08x\n",
-			gpu_read(gpu, REG_A5XX_RBBM_STATUS));
-
-	/*
-	 * Temporarily disable hardware clock gating before going into
-	 * adreno_show to avoid issues while reading the registers
-	 */
-	a5xx_set_hwcg(gpu, false);
-	adreno_show(gpu, m);
-	a5xx_set_hwcg(gpu, true);
-}
-#endif
-
 static struct msm_ringbuffer *a5xx_active_ring(struct msm_gpu *gpu)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -1260,7 +1244,7 @@ static const struct adreno_gpu_funcs funcs = {
 		.irq = a5xx_irq,
 		.destroy = a5xx_destroy,
 #ifdef CONFIG_DEBUG_FS
-		.show = a5xx_show,
+		.show = adreno_show,
 		.debugfs_init = a5xx_debugfs_init,
 #endif
 		.gpu_busy = a5xx_gpu_busy,
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index b2ccaf25767c..522d47ac36e1 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -423,38 +423,34 @@ void adreno_gpu_state_put(struct msm_gpu_state *state)
 }
 
 #ifdef CONFIG_DEBUG_FS
-void adreno_show(struct msm_gpu *gpu, struct seq_file *m)
+void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
+		struct seq_file *m)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
 	int i;
 
+	if (IS_ERR_OR_NULL(state))
+		return;
+
+	seq_printf(m, "status:   %08x\n", state->rbbm_status);
 	seq_printf(m, "revision: %d (%d.%d.%d.%d)\n",
 			adreno_gpu->info->revn, adreno_gpu->rev.core,
 			adreno_gpu->rev.major, adreno_gpu->rev.minor,
 			adreno_gpu->rev.patchid);
 
 	for (i = 0; i < gpu->nr_rings; i++) {
-		struct msm_ringbuffer *ring = gpu->rb[i];
-
 		seq_printf(m, "rb %d: fence:    %d/%d\n", i,
-			ring->memptrs->fence, ring->seqno);
+			state->ring[i].fence, state->ring[i].seqno);
 
-		seq_printf(m, "      rptr:     %d\n",
-			get_rptr(adreno_gpu, ring));
-		seq_printf(m, "rb wptr:  %d\n", get_wptr(ring));
+		seq_printf(m, "      rptr:     %d\n", state->ring[i].rptr);
+		seq_printf(m, "rb wptr:  %d\n", state->ring[i].wptr);
 	}
 
-	/* dump these out in a form that can be parsed by demsm: */
 	seq_printf(m, "IO:region %s 00000000 00020000\n", gpu->name);
-	for (i = 0; adreno_gpu->registers[i] != ~0; i += 2) {
-		uint32_t start = adreno_gpu->registers[i];
-		uint32_t end   = adreno_gpu->registers[i+1];
-		uint32_t addr;
-
-		for (addr = start; addr <= end; addr++) {
-			uint32_t val = gpu_read(gpu, addr);
-			seq_printf(m, "IO:R %08x %08x\n", addr<<2, val);
-		}
+	for (i = 0; i < state->nr_registers; i++) {
+		seq_printf(m, "IO:R %08x %08x\n",
+			state->registers[i * 2] << 2,
+			state->registers[(i * 2) + 1]);
 	}
 }
 #endif
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 0beb078eb658..815ae98c7fd1 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -215,7 +215,8 @@ void adreno_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
 void adreno_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring);
 bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring);
 #ifdef CONFIG_DEBUG_FS
-void adreno_show(struct msm_gpu *gpu, struct seq_file *m);
+void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
+		struct seq_file *m);
 #endif
 void adreno_dump_info(struct msm_gpu *gpu);
 void adreno_dump(struct msm_gpu *gpu);
@@ -227,7 +228,6 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		int nr_rings);
 void adreno_gpu_cleanup(struct adreno_gpu *gpu);
 
-
 struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu);
 void adreno_gpu_state_put(struct msm_gpu_state *state);
 
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index ba74cb4f94df..fd535dab3d5b 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -25,13 +25,22 @@ static int msm_gpu_show(struct drm_device *dev, struct seq_file *m)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 	struct msm_gpu *gpu = priv->gpu;
+	struct msm_gpu_state *state;
 
-	if (gpu) {
-		seq_printf(m, "%s Status:\n", gpu->name);
-		pm_runtime_get_sync(&gpu->pdev->dev);
-		gpu->funcs->show(gpu, m);
-		pm_runtime_put_sync(&gpu->pdev->dev);
-	}
+	if (!gpu)
+		return 0;
+
+	pm_runtime_get_sync(&gpu->pdev->dev);
+	state = gpu->funcs->gpu_state_get(gpu);
+	pm_runtime_put_sync(&gpu->pdev->dev);
+
+	if (IS_ERR(state))
+		return PTR_ERR(state);
+
+	seq_printf(m, "%s Status:\n", gpu->name);
+	gpu->funcs->show(gpu, state, m);
+
+	gpu->funcs->gpu_state_put(state);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 4be72a612bec..470f3bb5f834 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -65,7 +65,8 @@ struct msm_gpu_funcs {
 	void (*destroy)(struct msm_gpu *gpu);
 #ifdef CONFIG_DEBUG_FS
 	/* show GPU status in debugfs: */
-	void (*show)(struct msm_gpu *gpu, struct seq_file *m);
+	void (*show)(struct msm_gpu *gpu, struct msm_gpu_state *state,
+			struct seq_file *m);
 	/* for generation specific debugfs: */
 	int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor);
 #endif
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 05/10] drm/msm/gpu: Rearrange the code that collects the task during a hang

[Jordan Crouse]

Do a bit of cleanup to prepare for upcoming changes to pass the
hanging task comm and cmdline to the crash dump function.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/msm_gpu.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 1c09acfb4028..2ca354047250 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -314,6 +314,7 @@ static void recover_worker(struct work_struct *work)
 	struct msm_drm_private *priv = dev->dev_private;
 	struct msm_gem_submit *submit;
 	struct msm_ringbuffer *cur_ring = gpu->funcs->active_ring(gpu);
+	char *comm = NULL, *cmd = NULL;
 	int i;
 
 	mutex_lock(&dev->struct_mutex);
@@ -327,7 +328,7 @@ static void recover_worker(struct work_struct *work)
 		rcu_read_lock();
 		task = pid_task(submit->pid, PIDTYPE_PID);
 		if (task) {
-			char *cmd;
+			comm = kstrdup(task->comm, GFP_KERNEL);
 
 			/*
 			 * So slightly annoying, in other paths like
@@ -342,20 +343,21 @@ static void recover_worker(struct work_struct *work)
 			mutex_unlock(&dev->struct_mutex);
 			cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
 			mutex_lock(&dev->struct_mutex);
+		}
+		rcu_read_unlock();
 
+		if (comm && cmd) {
 			dev_err(dev->dev, "%s: offending task: %s (%s)\n",
-				gpu->name, task->comm, cmd);
+				gpu->name, comm, cmd);
 
 			msm_rd_dump_submit(priv->hangrd, submit,
-				"offending task: %s (%s)", task->comm, cmd);
-
-			kfree(cmd);
-		} else {
+				"offending task: %s (%s)", comm, cmd);
+		} else
 			msm_rd_dump_submit(priv->hangrd, submit, NULL);
-		}
-		rcu_read_unlock();
 	}
 
+	kfree(cmd);
+	kfree(comm);
 
 	/*
 	 * Update all the rings with the latest and greatest fence.. this
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 06/10] drm/msm/gpu: Capture the GPU state on a GPU hang

[Jordan Crouse]

Capture the GPU state on a GPU hang and store it for later playback
via the devcoredump facility. Only one crash state is stored at a
time on the assumption that the first hang is usually the most
interesting. The existing crash state can be cleared after capturing
it and then a new one will be captured on the next hang.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 drivers/gpu/drm/msm/Kconfig             |  1 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c   |  2 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  2 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c   |  4 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 36 +++++++++-----
 drivers/gpu/drm/msm/adreno/adreno_gpu.h |  4 +-
 drivers/gpu/drm/msm/msm_debugfs.c       |  7 ++-
 drivers/gpu/drm/msm/msm_gpu.c           | 83 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/msm_gpu.h           | 38 ++++++++++++++-
 9 files changed, 155 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 38cbde971b48..843a9d40c05e 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -12,6 +12,7 @@ config DRM_MSM
 	select SHMEM
 	select TMPFS
 	select QCOM_SCM
+	select WANT_DEV_COREDUMP
 	select SND_SOC_HDMI_CODEC if SND_SOC
 	select SYNC_FILE
 	select PM_OPP
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index 4cffec2b6adc..fc502e412132 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -454,7 +454,7 @@ static const struct adreno_gpu_funcs funcs = {
 		.active_ring = adreno_active_ring,
 		.irq = a3xx_irq,
 		.destroy = a3xx_destroy,
-#ifdef CONFIG_DEBUG_FS
+#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
 		.show = adreno_show,
 #endif
 		.gpu_state_get = a3xx_gpu_state_get,
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 95f08c22e8d7..8129cf037db1 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -540,7 +540,7 @@ static const struct adreno_gpu_funcs funcs = {
 		.active_ring = adreno_active_ring,
 		.irq = a4xx_irq,
 		.destroy = a4xx_destroy,
-#ifdef CONFIG_DEBUG_FS
+#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
 		.show = adreno_show,
 #endif
 		.gpu_state_get = a4xx_gpu_state_get,
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index b0910bbe5190..836a1df1f257 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1243,8 +1243,10 @@ static const struct adreno_gpu_funcs funcs = {
 		.active_ring = a5xx_active_ring,
 		.irq = a5xx_irq,
 		.destroy = a5xx_destroy,
-#ifdef CONFIG_DEBUG_FS
+#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
 		.show = adreno_show,
+#endif
+#if defined(CONFIG_DEBUG_FS)
 		.debugfs_init = a5xx_debugfs_init,
 #endif
 		.gpu_busy = a5xx_gpu_busy,
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 522d47ac36e1..d46ae2ede616 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -378,6 +378,8 @@ struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu)
 	if (!state)
 		return ERR_PTR(-ENOMEM);
 
+	kref_init(&state->ref);
+
 	do_gettimeofday(&state->time);
 
 	for (i = 0; i < gpu->nr_rings; i++) {
@@ -413,18 +415,28 @@ struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu)
 	return state;
 }
 
-void adreno_gpu_state_put(struct msm_gpu_state *state)
+static void adreno_gpu_state_destroy(struct kref *kref)
 {
-	if (IS_ERR_OR_NULL(state))
-		return;
+	struct msm_gpu_state *state = container_of(kref,
+		struct msm_gpu_state, ref);
 
+	kfree(state->comm);
+	kfree(state->cmd);
 	kfree(state->registers);
 	kfree(state);
 }
 
-#ifdef CONFIG_DEBUG_FS
+int adreno_gpu_state_put(struct msm_gpu_state *state)
+{
+	if (IS_ERR_OR_NULL(state))
+		return 1;
+
+	return kref_put(&state->ref, adreno_gpu_state_destroy);
+}
+
+#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
 void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
-		struct seq_file *m)
+		struct drm_printer *p)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
 	int i;
@@ -432,23 +444,23 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 	if (IS_ERR_OR_NULL(state))
 		return;
 
-	seq_printf(m, "status:   %08x\n", state->rbbm_status);
-	seq_printf(m, "revision: %d (%d.%d.%d.%d)\n",
+	drm_printf(p, "status:   %08x\n", state->rbbm_status);
+	drm_printf(p, "revision: %d (%d.%d.%d.%d)\n",
 			adreno_gpu->info->revn, adreno_gpu->rev.core,
 			adreno_gpu->rev.major, adreno_gpu->rev.minor,
 			adreno_gpu->rev.patchid);
 
 	for (i = 0; i < gpu->nr_rings; i++) {
-		seq_printf(m, "rb %d: fence:    %d/%d\n", i,
+		drm_printf(p, "rb %d: fence:    %d/%d\n", i,
 			state->ring[i].fence, state->ring[i].seqno);
 
-		seq_printf(m, "      rptr:     %d\n", state->ring[i].rptr);
-		seq_printf(m, "rb wptr:  %d\n", state->ring[i].wptr);
+		drm_printf(p, "      rptr:     %d\n", state->ring[i].rptr);
+		drm_printf(p, "rb wptr:  %d\n", state->ring[i].wptr);
 	}
 
-	seq_printf(m, "IO:region %s 00000000 00020000\n", gpu->name);
+	drm_printf(p, "IO:region %s 00000000 00020000\n", gpu->name);
 	for (i = 0; i < state->nr_registers; i++) {
-		seq_printf(m, "IO:R %08x %08x\n",
+		drm_printf(p, "IO:R %08x %08x\n",
 			state->registers[i * 2] << 2,
 			state->registers[(i * 2) + 1]);
 	}
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 815ae98c7fd1..077bf1149c0b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -216,7 +216,7 @@ void adreno_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring);
 bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring);
 #ifdef CONFIG_DEBUG_FS
 void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
-		struct seq_file *m);
+		struct drm_printer *p);
 #endif
 void adreno_dump_info(struct msm_gpu *gpu);
 void adreno_dump(struct msm_gpu *gpu);
@@ -229,7 +229,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 void adreno_gpu_cleanup(struct adreno_gpu *gpu);
 
 struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu);
-void adreno_gpu_state_put(struct msm_gpu_state *state);
+int adreno_gpu_state_put(struct msm_gpu_state *state);
 
 /* ringbuffer helpers (the parts that are adreno specific) */
 
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index fd535dab3d5b..165d94b709f9 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -16,6 +16,8 @@
  */
 
 #ifdef CONFIG_DEBUG_FS
+
+#include <linux/debugfs.h>
 #include "msm_drv.h"
 #include "msm_gpu.h"
 #include "msm_kms.h"
@@ -26,6 +28,7 @@ static int msm_gpu_show(struct drm_device *dev, struct seq_file *m)
 	struct msm_drm_private *priv = dev->dev_private;
 	struct msm_gpu *gpu = priv->gpu;
 	struct msm_gpu_state *state;
+	struct drm_printer p = drm_seq_file_printer(m);
 
 	if (!gpu)
 		return 0;
@@ -37,8 +40,8 @@ static int msm_gpu_show(struct drm_device *dev, struct seq_file *m)
 	if (IS_ERR(state))
 		return PTR_ERR(state);
 
-	seq_printf(m, "%s Status:\n", gpu->name);
-	gpu->funcs->show(gpu, state, m);
+	drm_printf(&p, "%s Status:\n", gpu->name);
+	gpu->funcs->show(gpu, state, &p);
 
 	gpu->funcs->gpu_state_put(state);
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 2ca354047250..f36b415e123b 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -20,10 +20,11 @@
 #include "msm_mmu.h"
 #include "msm_fence.h"
 
+#include <generated/utsrelease.h>
 #include <linux/string_helpers.h>
 #include <linux/pm_opp.h>
 #include <linux/devfreq.h>
-
+#include <linux/devcoredump.h>
 
 /*
  * Power Management:
@@ -273,6 +274,83 @@ int msm_gpu_hw_init(struct msm_gpu *gpu)
 	return ret;
 }
 
+#ifdef CONFIG_DEV_COREDUMP
+static ssize_t msm_gpu_devcoredump_read(char *buffer, loff_t offset,
+		size_t count, void *data, size_t datalen)
+{
+	struct msm_gpu *gpu = data;
+	struct drm_print_iterator iter;
+	struct drm_printer p;
+	struct msm_gpu_state *state;
+
+	state = msm_gpu_crashstate_get(gpu);
+	if (!state)
+		return 0;
+
+	iter.data = buffer;
+	iter.offset = 0;
+	iter.start = offset;
+	iter.remain = count;
+
+	p = drm_coredump_printer(&iter);
+
+	drm_printf(&p, "---\n");
+	drm_printf(&p, "kernel: " UTS_RELEASE "\n");
+	drm_printf(&p, "module: " KBUILD_MODNAME "\n");
+	drm_printf(&p, "time: %ld.%ld\n",
+		state->time.tv_sec, state->time.tv_usec);
+	if (state->comm)
+		drm_printf(&p, "comm: %s\n", state->comm);
+	if (state->cmd)
+		drm_printf(&p, "cmdline: %s\n", state->cmd);
+
+	gpu->funcs->show(gpu, state, &p);
+
+	msm_gpu_crashstate_put(gpu);
+
+	return count - iter.remain;
+}
+
+static void msm_gpu_devcoredump_free(void *data)
+{
+	struct msm_gpu *gpu = data;
+
+	msm_gpu_crashstate_put(gpu);
+}
+
+static void msm_gpu_crashstate_capture(struct msm_gpu *gpu, char *comm,
+		char *cmd)
+{
+	struct msm_gpu_state *state;
+
+	/* Only save one crash state at a a time */
+	if (gpu->crashstate)
+		return;
+
+	state = gpu->funcs->gpu_state_get(gpu);
+	if (IS_ERR_OR_NULL(state))
+		return;
+
+	/* Fill in the additional crash state information */
+	state->comm = kstrdup(comm, GFP_KERNEL);
+	state->cmd = kstrdup(cmd, GFP_KERNEL);
+
+	kref_init(&state->ref);
+
+	/* Set the active crash state to be dumped on failure */
+	gpu->crashstate = state;
+
+	/* FIXME: Release the crashstate if this errors out? */
+	dev_coredumpm(gpu->dev->dev, THIS_MODULE, gpu, 0, GFP_KERNEL,
+		msm_gpu_devcoredump_read, msm_gpu_devcoredump_free);
+}
+#else
+static void msm_gpu_crashstate_capture(struct msm_gpu *gpu, char *comm,
+		char *cmd)
+{
+}
+#endif
+
 /*
  * Hangcheck detection for locked gpu:
  */
@@ -356,6 +434,9 @@ static void recover_worker(struct work_struct *work)
 			msm_rd_dump_submit(priv->hangrd, submit, NULL);
 	}
 
+	/* Record the crash state */
+	msm_gpu_crashstate_capture(gpu, comm, cmd);
+
 	kfree(cmd);
 	kfree(comm);
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 470f3bb5f834..e65f507954c0 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -66,13 +66,13 @@ struct msm_gpu_funcs {
 #ifdef CONFIG_DEBUG_FS
 	/* show GPU status in debugfs: */
 	void (*show)(struct msm_gpu *gpu, struct msm_gpu_state *state,
-			struct seq_file *m);
+			struct drm_printer *p);
 	/* for generation specific debugfs: */
 	int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor);
 #endif
 	int (*gpu_busy)(struct msm_gpu *gpu, uint64_t *value);
 	struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu);
-	void (*gpu_state_put)(struct msm_gpu_state *state);
+	int (*gpu_state_put)(struct msm_gpu_state *state);
 };
 
 struct msm_gpu {
@@ -133,6 +133,8 @@ struct msm_gpu {
 		u64 busy_cycles;
 		ktime_t time;
 	} devfreq;
+
+	struct msm_gpu_state *crashstate;
 };
 
 /* It turns out that all targets use the same ringbuffer size */
@@ -180,6 +182,7 @@ struct msm_gpu_submitqueue {
 };
 
 struct msm_gpu_state {
+	struct kref ref;
 	struct timeval time;
 
 	struct {
@@ -193,6 +196,9 @@ struct msm_gpu_state {
 	u32 *registers;
 
 	u32 rbbm_status;
+
+	char *comm;
+	char *cmd;
 };
 
 static inline void gpu_write(struct msm_gpu *gpu, u32 reg, u32 data)
@@ -274,4 +280,32 @@ static inline void msm_submitqueue_put(struct msm_gpu_submitqueue *queue)
 		kref_put(&queue->ref, msm_submitqueue_destroy);
 }
 
+static inline struct msm_gpu_state *msm_gpu_crashstate_get(struct msm_gpu *gpu)
+{
+	struct msm_gpu_state *state = NULL;
+
+	mutex_lock(&gpu->dev->struct_mutex);
+
+	if (gpu->crashstate) {
+		kref_get(&gpu->crashstate->ref);
+		state = gpu->crashstate;
+	}
+
+	mutex_unlock(&gpu->dev->struct_mutex);
+
+	return state;
+}
+
+static inline void msm_gpu_crashstate_put(struct msm_gpu *gpu)
+{
+	mutex_lock(&gpu->dev->struct_mutex);
+
+	if (gpu->crashstate) {
+		if (gpu->funcs->gpu_state_put(gpu->crashstate))
+			gpu->crashstate = NULL;
+	}
+
+	mutex_unlock(&gpu->dev->struct_mutex);
+}
+
 #endif /* __MSM_GPU_H__ */
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 07/10] drm/msm/adreno: Convert the show/crash file format

[Jordan Crouse]

Convert the format of the 'show' debugfs file and the crash
dump to a  format resembling YAML. This should be easier to
parse and be more flexible for future changes and expansions.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 Documentation/gpu/drm-msm-crash-dump.txt | 30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 21 +++++++++++++--------
 2 files changed, 43 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/gpu/drm-msm-crash-dump.txt

diff --git a/Documentation/gpu/drm-msm-crash-dump.txt b/Documentation/gpu/drm-msm-crash-dump.txt
new file mode 100644
index 000000000000..902d9769f401
--- /dev/null
+++ b/Documentation/gpu/drm-msm-crash-dump.txt
@@ -0,0 +1,30 @@
+# drm/msm GPU crash dump format
+#
+# This is a description of the format of the drm/msm GPU crash dump format that
+# can be read from /sys/kernel/dri/X/show or from devcoredump following a GPU
+# hang or fault
+
+---
+kernel:		# [string] The kernel version as printed by UTS_RELEASE
+module:		# [string] The module that generated the crash dump
+time:		# [seconds.microseconds] The kernel time at crash
+comm:		# [string] comm string for the binary that generated the fault
+		# (if known)
+cmdline:	# [string] the cmdline for the binary that generated the fault
+		# (if known)
+revision:	# [ id core.major.minor.patchlevel] The GPU id followed by the
+		# individual components of the id separated by dots
+rbbm-status:	# [hex] The current value of RBBM_STATUS which shows what GPU
+		# components were in use at the time of the crash
+ringbuffer:	# Ringbuffer data. There will be a sequence for each ringbuffer
+  -id:			# [decimal] Ringbuffer identifier (0 based index)
+   last-fence:		# [decimal] The last fence issued on the ring
+   retired-fence:	# [decimal] THe last fence retired on the ring
+   rptr:		# [decimal] The current read pointer (rptr) for the ring
+   wptr:		# [decimal] The current write pointer (wptr) for the
+			# ring
+registers:	# Sets of register values. This section can be used multiple
+		# times for different ranges of registers. Each register will be
+		# on its own line.
+  - [offset, value]	# offset: [hex] byte offset of the register
+			# value: [hex] value of the register
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index d46ae2ede616..395885504503 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -444,23 +444,28 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 	if (IS_ERR_OR_NULL(state))
 		return;
 
-	drm_printf(p, "status:   %08x\n", state->rbbm_status);
 	drm_printf(p, "revision: %d (%d.%d.%d.%d)\n",
 			adreno_gpu->info->revn, adreno_gpu->rev.core,
 			adreno_gpu->rev.major, adreno_gpu->rev.minor,
 			adreno_gpu->rev.patchid);
 
-	for (i = 0; i < gpu->nr_rings; i++) {
-		drm_printf(p, "rb %d: fence:    %d/%d\n", i,
-			state->ring[i].fence, state->ring[i].seqno);
+	drm_printf(p, "rbbm-status: 0x%08x\n", state->rbbm_status);
+
+	drm_printf(p, "ringbuffer:\n");
 
-		drm_printf(p, "      rptr:     %d\n", state->ring[i].rptr);
-		drm_printf(p, "rb wptr:  %d\n", state->ring[i].wptr);
+	for (i = 0; i < gpu->nr_rings; i++) {
+		drm_printf(p, "  - id: %d\n", i);
+		drm_printf(p, "    last-fence: %d\n", state->ring[i].seqno);
+		drm_printf(p, "    retired-fence: %d\n", state->ring[i].fence);
+		drm_printf(p, "    rptr: %d\n", state->ring[i].rptr);
+		drm_printf(p, "    wptr: %d\n", state->ring[i].wptr);
 	}
 
-	drm_printf(p, "IO:region %s 00000000 00020000\n", gpu->name);
+	drm_printf(p, "registers:\n");
+	drm_printf(p, "  - [offset, value]\n");
+
 	for (i = 0; i < state->nr_registers; i++) {
-		drm_printf(p, "IO:R %08x %08x\n",
+		drm_printf(p, "  - [0x%04x, 0x%08x]\n",
 			state->registers[i * 2] << 2,
 			state->registers[(i * 2) + 1]);
 	}
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 08/10] drm/msm/adreno: Add ringbuffer data to the GPU state

[Jordan Crouse]

Add the contents of each ringbuffer to the GPU state and dump the
data in the crash file encoded with ascii85. To save space only
the used portions of the ringbuffer are dumped.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 Documentation/gpu/drm-msm-crash-dump.txt |  5 ++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 41 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/msm_gpu.h            |  2 ++
 3 files changed, 48 insertions(+)

diff --git a/Documentation/gpu/drm-msm-crash-dump.txt b/Documentation/gpu/drm-msm-crash-dump.txt
index 902d9769f401..df25afd10602 100644
--- a/Documentation/gpu/drm-msm-crash-dump.txt
+++ b/Documentation/gpu/drm-msm-crash-dump.txt
@@ -23,6 +23,11 @@ ringbuffer:	# Ringbuffer data. There will be a sequence for each ringbuffer
    rptr:		# [decimal] The current read pointer (rptr) for the ring
    wptr:		# [decimal] The current write pointer (wptr) for the
 			# ring
+   size:		# [decimal] The maximum size of the ring programmed in
+			# the hardware
+   data:		# [ascii85] The contents of the ring encoded as ascii85.
+			# Only the unused portions of the ring will be printed
+			# (up to a maximum of 'size' bytes)
 registers:	# Sets of register values. This section can be used multiple
 		# times for different ranges of registers. Each register will be
 		# on its own line.
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 395885504503..a6414b6d5c0b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -17,6 +17,7 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/ascii85.h>
 #include <linux/pm_opp.h>
 #include "adreno_gpu.h"
 #include "msm_gem.h"
@@ -383,10 +384,30 @@ struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu)
 	do_gettimeofday(&state->time);
 
 	for (i = 0; i < gpu->nr_rings; i++) {
+		int size = 0, j;
+
 		state->ring[i].fence = gpu->rb[i]->memptrs->fence;
 		state->ring[i].seqno = gpu->rb[i]->seqno;
 		state->ring[i].rptr = get_rptr(adreno_gpu, gpu->rb[i]);
 		state->ring[i].wptr = get_wptr(gpu->rb[i]);
+
+		/*
+		 * Only copy used parts of the ring buffers (this should save
+		 * data size for lightly used rings)
+		 */
+		for (j = 0; j < MSM_GPU_RINGBUFFER_SZ >> 2; j++)
+			if (gpu->rb[i]->start[j])
+				size = j;
+
+		if (size) {
+			state->ring[i].data = kmalloc((size + 1) << 2,
+				GFP_KERNEL);
+			if (state->ring[i].data) {
+				memcpy(state->ring[i].data, gpu->rb[i]->start,
+				(size + 1) << 2);
+				state->ring[i].data_size = (size + 1) << 2;
+			}
+		}
 	}
 
 	/* Count the number of registers */
@@ -417,9 +438,13 @@ struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu)
 
 static void adreno_gpu_state_destroy(struct kref *kref)
 {
+	int i;
 	struct msm_gpu_state *state = container_of(kref,
 		struct msm_gpu_state, ref);
 
+	for (i = 0; i < ARRAY_SIZE(state->ring); i++)
+		kfree(state->ring[i].data);
+
 	kfree(state->comm);
 	kfree(state->cmd);
 	kfree(state->registers);
@@ -459,6 +484,22 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 		drm_printf(p, "    retired-fence: %d\n", state->ring[i].fence);
 		drm_printf(p, "    rptr: %d\n", state->ring[i].rptr);
 		drm_printf(p, "    wptr: %d\n", state->ring[i].wptr);
+		drm_printf(p, "    size: %d\n", MSM_GPU_RINGBUFFER_SZ);
+
+		if (state->ring[i].data && state->ring[i].data_size) {
+			u32 *ptr = (u32 *) state->ring[i].data;
+			char out[ASCII85_BUFSZ];
+			long len = ascii85_encode_len(state->ring[i].data_size);
+			int j;
+
+			drm_printf(p, "    data: !!ascii85 |\n");
+			drm_printf(p, "     ");
+
+			for (j = 0; j < len; j++)
+				drm_printf(p, ascii85_encode(ptr[j], out));
+
+			drm_printf(p, "\n");
+		}
 	}
 
 	drm_printf(p, "registers:\n");
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index e65f507954c0..48f7b21f1cae 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -190,6 +190,8 @@ struct msm_gpu_state {
 		u32 seqno;
 		u32 rptr;
 		u32 wptr;
+		void *data;
+		int data_size;
 	} ring[MSM_GPU_MAX_RINGS];
 
 	int nr_registers;
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 09/10] drm/msm/adreno: Add a5xx specific registers for the GPU state

[Jordan Crouse]

HLSQ, SP and TP registers are only accessible from a special
aperture and to make matters worse the aperture is blocked from
the CPU on targets that can support secure rendering. Luckily the
GPU hardware has its own purpose built register dumper that can
access the registers from the aperture.  Add a5xx specific code
to program the crashdumper and retrieve the wayward registers
and dump them for the crash state.

Also, remove a block of registers the regular CPU accessible
list that aren't useful for debug which helps reduce the size
of the crash state file by a goodly amount.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 Documentation/gpu/drm-msm-crash-dump.txt |   4 +
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c    |   8 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c    |   8 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c    | 237 +++++++++++++++++++++++++++++--
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  |  23 +--
 drivers/gpu/drm/msm/adreno/adreno_gpu.h  |   4 +-
 6 files changed, 253 insertions(+), 31 deletions(-)

diff --git a/Documentation/gpu/drm-msm-crash-dump.txt b/Documentation/gpu/drm-msm-crash-dump.txt
index df25afd10602..f84a47a9ca92 100644
--- a/Documentation/gpu/drm-msm-crash-dump.txt
+++ b/Documentation/gpu/drm-msm-crash-dump.txt
@@ -33,3 +33,7 @@ registers:	# Sets of register values. This section can be used multiple
 		# on its own line.
   - [offset, value]	# offset: [hex] byte offset of the register
 			# value: [hex] value of the register
+
+registers-hlsq: # (5xx only) Same format as registers. Register data that
+		# only accessible from the HLSQ aperture captured by the
+		# HW based crashdumper
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index fc502e412132..669c2d4b070d 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -421,10 +421,12 @@ static void a3xx_dump(struct msm_gpu *gpu)
 
 static struct msm_gpu_state *a3xx_gpu_state_get(struct msm_gpu *gpu)
 {
-	struct msm_gpu_state *state = adreno_gpu_state_get(gpu);
+	struct msm_gpu_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
 
-	if (IS_ERR(state))
-		return state;
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	adreno_gpu_state_get(gpu, state);
 
 	state->rbbm_status = gpu_read(gpu, REG_A3XX_RBBM_STATUS);
 
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index 8129cf037db1..7c4e6dc1ed59 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -457,10 +457,12 @@ static const unsigned int a4xx_registers[] = {
 
 static struct msm_gpu_state *a4xx_gpu_state_get(struct msm_gpu *gpu)
 {
-	struct msm_gpu_state *state = adreno_gpu_state_get(gpu);
+	struct msm_gpu_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
 
-	if (IS_ERR(state))
-		return state;
+	if (!state)
+		return ERR_PTR(-ENOMEM);
+
+	adreno_gpu_state_get(gpu, state);
 
 	state->rbbm_status = gpu_read(gpu, REG_A4XX_RBBM_STATUS);
 
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 836a1df1f257..0090fec862c1 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -19,6 +19,7 @@
 #include <linux/soc/qcom/mdt_loader.h>
 #include <linux/pm_opp.h>
 #include <linux/nvmem-consumer.h>
+#include <linux/iopoll.h>
 #include "msm_gem.h"
 #include "msm_mmu.h"
 #include "a5xx_gpu.h"
@@ -1123,8 +1124,9 @@ static const u32 a5xx_registers[] = {
 	0xE800, 0xE806, 0xE810, 0xE89A, 0xE8A0, 0xE8A4, 0xE8AA, 0xE8EB,
 	0xE900, 0xE905, 0xEB80, 0xEB8F, 0xEBB0, 0xEBB0, 0xEC00, 0xEC05,
 	0xEC08, 0xECE9, 0xECF0, 0xECF0, 0xEA80, 0xEA80, 0xEA82, 0xEAA3,
-	0xEAA5, 0xEAC2, 0xA800, 0xA8FF, 0xAC60, 0xAC60, 0xB000, 0xB97F,
-	0xB9A0, 0xB9BF, ~0
+	0xEAA5, 0xEAC2, 0xA800, 0xA800, 0xA820, 0xA828, 0xA840, 0xA87D,
+	0XA880, 0xA88D, 0xA890, 0xA8A3, 0xA8D0, 0xA8D8, 0xA8E0, 0xA8F5,
+	0xAC60, 0xAC60, ~0,
 };
 
 static void a5xx_dump(struct msm_gpu *gpu)
@@ -1195,25 +1197,234 @@ static int a5xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value)
 	return 0;
 }
 
+struct a5xx_crashdumper {
+	void *ptr;
+	struct drm_gem_object *bo;
+	u64 iova;
+};
+
+struct a5xx_gpu_state {
+	struct msm_gpu_state base;
+	u32 *hlsqregs;
+};
+
+#define gpu_poll_timeout(gpu, addr, val, cond, interval, timeout) \
+	readl_poll_timeout((gpu)->mmio + ((addr) << 2), val, cond, \
+		interval, timeout)
+
+static int a5xx_crashdumper_init(struct msm_gpu *gpu,
+		struct a5xx_crashdumper *dumper)
+{
+	dumper->ptr = msm_gem_kernel_new_locked(gpu->dev,
+		SZ_1M, MSM_BO_UNCACHED, gpu->aspace,
+		&dumper->bo, &dumper->iova);
+
+	if (IS_ERR(dumper->ptr))
+		return PTR_ERR(dumper->ptr);
+
+	return 0;
+}
+
+static void a5xx_crashdumper_free(struct msm_gpu *gpu,
+		struct a5xx_crashdumper *dumper)
+{
+	msm_gem_put_iova(dumper->bo, gpu->aspace);
+	msm_gem_put_vaddr(dumper->bo);
+
+	drm_gem_object_unreference(dumper->bo);
+}
+
+static int a5xx_crashdumper_run(struct msm_gpu *gpu,
+		struct a5xx_crashdumper *dumper)
+{
+	u32 val;
+
+	if (IS_ERR_OR_NULL(dumper->ptr))
+		return -EINVAL;
+
+	gpu_write64(gpu, REG_A5XX_CP_CRASH_SCRIPT_BASE_LO,
+		REG_A5XX_CP_CRASH_SCRIPT_BASE_HI, dumper->iova);
+
+	gpu_write(gpu, REG_A5XX_CP_CRASH_DUMP_CNTL, 1);
+
+	return gpu_poll_timeout(gpu, REG_A5XX_CP_CRASH_DUMP_CNTL, val,
+		val & 0x04, 100, 10000);
+}
+
+/*
+ * These are a list of the registers that need to be read through the HLSQ
+ * aperture through the crashdumper.  These are not nominally accessible from
+ * the CPU on a secure platform.
+ */
+static const struct {
+	u32 type;
+	u32 regoffset;
+	u32 count;
+} a5xx_hlsq_aperture_regs[] = {
+	{ 0x35, 0xe00, 0x32 },   /* HSLQ non-context */
+	{ 0x31, 0x2080, 0x1 },   /* HLSQ 2D context 0 */
+	{ 0x33, 0x2480, 0x1 },   /* HLSQ 2D context 1 */
+	{ 0x32, 0xe780, 0x62 },  /* HLSQ 3D context 0 */
+	{ 0x34, 0xef80, 0x62 },  /* HLSQ 3D context 1 */
+	{ 0x3f, 0x0ec0, 0x40 },  /* SP non-context */
+	{ 0x3d, 0x2040, 0x1 },   /* SP 2D context 0 */
+	{ 0x3b, 0x2440, 0x1 },   /* SP 2D context 1 */
+	{ 0x3e, 0xe580, 0x170 }, /* SP 3D context 0 */
+	{ 0x3c, 0xed80, 0x170 }, /* SP 3D context 1 */
+	{ 0x3a, 0x0f00, 0x1c },  /* TP non-context */
+	{ 0x38, 0x2000, 0xa },   /* TP 2D context 0 */
+	{ 0x36, 0x2400, 0xa },   /* TP 2D context 1 */
+	{ 0x39, 0xe700, 0x80 },  /* TP 3D context 0 */
+	{ 0x37, 0xef00, 0x80 },  /* TP 3D context 1 */
+};
+
+static void a5xx_gpu_state_get_hlsq_regs(struct msm_gpu *gpu,
+		struct a5xx_gpu_state *a5xx_state)
+{
+	struct a5xx_crashdumper dumper = { 0 };
+	u32 offset, count = 0;
+	u64 *ptr;
+	int i;
+
+	if (a5xx_crashdumper_init(gpu, &dumper))
+		return;
+
+	/* The script will be written at offset 0 */
+	ptr = dumper.ptr;
+
+	/* Start writing the data at offset 256k */
+	offset = dumper.iova + (256 * SZ_1K);
+
+	/* Count how many additional registers to get from the HLSQ aperture */
+	for (i = 0; i < ARRAY_SIZE(a5xx_hlsq_aperture_regs); i++)
+		count += a5xx_hlsq_aperture_regs[i].count;
+
+	a5xx_state->hlsqregs = kcalloc(count, sizeof(u32), GFP_KERNEL);
+	if (!a5xx_state->hlsqregs)
+		return;
+
+	/* Build the crashdump script */
+	for (i = 0; i < ARRAY_SIZE(a5xx_hlsq_aperture_regs); i++) {
+		u32 type = a5xx_hlsq_aperture_regs[i].type;
+		u32 c = a5xx_hlsq_aperture_regs[i].count;
+
+		/* Write the register to select the desired bank */
+		*ptr++ = ((u64) type << 8);
+		*ptr++ = (((u64) REG_A5XX_HLSQ_DBG_READ_SEL) << 44) |
+			(1 << 21) | 1;
+
+		*ptr++ = offset;
+		*ptr++ = (((u64) REG_A5XX_HLSQ_DBG_AHB_READ_APERTURE) << 44)
+			| c;
+
+		offset += c * sizeof(u32);
+	}
+
+	/* Write two zeros to close off the script */
+	*ptr++ = 0;
+	*ptr++ = 0;
+
+	if (a5xx_crashdumper_run(gpu, &dumper)) {
+		kfree(a5xx_state->hlsqregs);
+		a5xx_crashdumper_free(gpu, &dumper);
+		return;
+	}
+
+	/* Copy the data from the crashdumper to the state */
+	memcpy(a5xx_state->hlsqregs, dumper.ptr + (256 * SZ_1K),
+		count * sizeof(u32));
+
+	a5xx_crashdumper_free(gpu, &dumper);
+}
+
 static struct msm_gpu_state *a5xx_gpu_state_get(struct msm_gpu *gpu)
 {
-	struct msm_gpu_state *state;
+	struct a5xx_gpu_state *a5xx_state = kzalloc(sizeof(*a5xx_state),
+			GFP_KERNEL);
 
-	/*
-	 * Temporarily disable hardware clock gating before going into
-	 * adreno_show to avoid issues while reading the registers
-	 */
+	if (!a5xx_state)
+		return ERR_PTR(-ENOMEM);
+
+	/* Temporarily disable hardware clock gating before reading the hw */
 	a5xx_set_hwcg(gpu, false);
 
-	state = adreno_gpu_state_get(gpu);
+	/* First get the generic state from the adreno core */
+	adreno_gpu_state_get(gpu, &(a5xx_state->base));
+
+	a5xx_state->base.rbbm_status = gpu_read(gpu, REG_A5XX_RBBM_STATUS);
 
-	if (!IS_ERR(state))
-		state->rbbm_status = gpu_read(gpu, REG_A5XX_RBBM_STATUS);
+	/* Get the HLSQ regs with the help of the crashdumper */
+	a5xx_gpu_state_get_hlsq_regs(gpu, a5xx_state);
 
 	a5xx_set_hwcg(gpu, true);
 
-	return state;
+	return &a5xx_state->base;
+}
+
+static void a5xx_gpu_state_destroy(struct kref *kref)
+{
+	struct msm_gpu_state *state = container_of(kref,
+		struct msm_gpu_state, ref);
+	struct a5xx_gpu_state *a5xx_state = container_of(state,
+		struct a5xx_gpu_state, base);
+
+	kfree(a5xx_state->hlsqregs);
+
+	adreno_gpu_state_destroy(state);
+	kfree(a5xx_state);
+}
+
+int a5xx_gpu_state_put(struct msm_gpu_state *state)
+{
+	if (IS_ERR_OR_NULL(state))
+		return 1;
+
+	return kref_put(&state->ref, a5xx_gpu_state_destroy);
+}
+
+
+#if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
+void a5xx_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
+		struct drm_printer *p)
+{
+	int i, j;
+	u32 pos = 0;
+	struct a5xx_gpu_state *a5xx_state = container_of(state,
+		struct a5xx_gpu_state, base);
+
+	if (IS_ERR_OR_NULL(state))
+		return;
+
+	adreno_show(gpu, state, p);
+
+	/* Dump the additional a5xx HLSQ registers */
+	if (!a5xx_state->hlsqregs)
+		return;
+
+	drm_printf(p, "registers-hlsq:\n");
+	drm_printf(p, "  - [offset, value]\n");
+
+	for (i = 0; i < ARRAY_SIZE(a5xx_hlsq_aperture_regs); i++) {
+		u32 o = a5xx_hlsq_aperture_regs[i].regoffset;
+		u32 c = a5xx_hlsq_aperture_regs[i].count;
+
+		for (j = 0; j < c; j++, pos++, o++) {
+			/*
+			 * To keep the crashdump simple we pull the entire range
+			 * for each register type but not all of the registers
+			 * in the range are valid. Fortunately invalid registers
+			 * stick out like a sore thumb with a value of
+			 * 0xdeadbeef
+			 */
+			if (a5xx_state->hlsqregs[pos] == 0xdeadbeef)
+				continue;
+
+			drm_printf(p, "  - [0x%04x, 0x%08x]\n",
+				o << 2, a5xx_state->hlsqregs[pos]);
+		}
+	}
 }
+#endif
 
 static struct msm_ringbuffer *a5xx_active_ring(struct msm_gpu *gpu)
 {
@@ -1244,14 +1455,14 @@ static const struct adreno_gpu_funcs funcs = {
 		.irq = a5xx_irq,
 		.destroy = a5xx_destroy,
 #if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
-		.show = adreno_show,
+		.show = a5xx_show,
 #endif
 #if defined(CONFIG_DEBUG_FS)
 		.debugfs_init = a5xx_debugfs_init,
 #endif
 		.gpu_busy = a5xx_gpu_busy,
 		.gpu_state_get = a5xx_gpu_state_get,
-		.gpu_state_put = adreno_gpu_state_put,
+		.gpu_state_put = a5xx_gpu_state_put,
 	},
 	.get_timestamp = a5xx_get_timestamp,
 };
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index a6414b6d5c0b..b92bcbde688e 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -369,16 +369,11 @@ bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
 	return false;
 }
 
-struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu)
+int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
-	struct msm_gpu_state *state;
 	int i, count = 0;
 
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return ERR_PTR(-ENOMEM);
-
 	kref_init(&state->ref);
 
 	do_gettimeofday(&state->time);
@@ -433,14 +428,12 @@ struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu)
 		state->nr_registers = count;
 	}
 
-	return state;
+	return 0;
 }
 
-static void adreno_gpu_state_destroy(struct kref *kref)
+void adreno_gpu_state_destroy(struct msm_gpu_state *state)
 {
 	int i;
-	struct msm_gpu_state *state = container_of(kref,
-		struct msm_gpu_state, ref);
 
 	for (i = 0; i < ARRAY_SIZE(state->ring); i++)
 		kfree(state->ring[i].data);
@@ -448,6 +441,14 @@ static void adreno_gpu_state_destroy(struct kref *kref)
 	kfree(state->comm);
 	kfree(state->cmd);
 	kfree(state->registers);
+}
+
+static void adreno_gpu_state_kref_destroy(struct kref *kref)
+{
+	struct msm_gpu_state *state = container_of(kref,
+		struct msm_gpu_state, ref);
+
+	adreno_gpu_state_destroy(state);
 	kfree(state);
 }
 
@@ -456,7 +457,7 @@ int adreno_gpu_state_put(struct msm_gpu_state *state)
 	if (IS_ERR_OR_NULL(state))
 		return 1;
 
-	return kref_put(&state->ref, adreno_gpu_state_destroy);
+	return kref_put(&state->ref, adreno_gpu_state_kref_destroy);
 }
 
 #if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 077bf1149c0b..942d69563090 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -228,7 +228,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		int nr_rings);
 void adreno_gpu_cleanup(struct adreno_gpu *gpu);
 
-struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu);
+void adreno_gpu_state_destroy(struct msm_gpu_state *state);
+
+int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
 int adreno_gpu_state_put(struct msm_gpu_state *state);
 
 /* ringbuffer helpers (the parts that are adreno specific) */
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH 10/10] drm/msm/gpu: Add the buffer objects from the submit to the crash dump

[Jordan Crouse]

On a hang copy out the contents of the buffer objects attached to the
guilty submission and print them in the crash dump report.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---
 Documentation/gpu/drm-msm-crash-dump.txt |  7 +++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 54 ++++++++++++++++++++++++++------
 drivers/gpu/drm/msm/msm_gpu.c            | 48 ++++++++++++++++++++++++++--
 drivers/gpu/drm/msm/msm_gpu.h            |  9 ++++++
 4 files changed, 105 insertions(+), 13 deletions(-)

diff --git a/Documentation/gpu/drm-msm-crash-dump.txt b/Documentation/gpu/drm-msm-crash-dump.txt
index f84a47a9ca92..930e4c970a62 100644
--- a/Documentation/gpu/drm-msm-crash-dump.txt
+++ b/Documentation/gpu/drm-msm-crash-dump.txt
@@ -28,6 +28,13 @@ ringbuffer:	# Ringbuffer data. There will be a sequence for each ringbuffer
    data:		# [ascii85] The contents of the ring encoded as ascii85.
 			# Only the unused portions of the ring will be printed
 			# (up to a maximum of 'size' bytes)
+bos:		# List of buffers from the hanging submission (if known)
+  -iova:		# [hex] GPU address of the buffer
+   size:		# [decimal] Size of the buffer (in bytes)
+   data:		# [ascii85] The contents of the buffer encoded as
+			# ascii85. Only the contents of buffers marked as
+			# readable are dumped. Trailing zeros at the end of the
+			# buffer won't be dumped.
 registers:	# Sets of register values. This section can be used multiple
 		# times for different ranges of registers. Each register will be
 		# on its own line.
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index b92bcbde688e..5daca5ba7e06 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -438,6 +438,10 @@ void adreno_gpu_state_destroy(struct msm_gpu_state *state)
 	for (i = 0; i < ARRAY_SIZE(state->ring); i++)
 		kfree(state->ring[i].data);
 
+	for (i = 0; state->bos && i < state->nr_bos; i++)
+		kfree(state->bos[i].data);
+
+	kfree(state->bos);
 	kfree(state->comm);
 	kfree(state->cmd);
 	kfree(state->registers);
@@ -461,6 +465,35 @@ int adreno_gpu_state_put(struct msm_gpu_state *state)
 }
 
 #if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
+
+static void adreno_show_object(struct drm_printer *p, u32 *ptr, int len)
+{
+	char out[ASCII85_BUFSZ];
+	long l, datalen, i;
+
+	if (!ptr || !len)
+		return;
+
+	/*
+	 * Only dump the non-zero part of the buffer - rarely will any data
+	 * completely fill the entire allocated size of the buffer
+	 */
+	for (datalen = 0, i = 0; i < len >> 2; i++) {
+		if (ptr[i])
+			datalen = i << 2;
+	}
+
+	l = ascii85_encode_len(datalen);
+
+	drm_printf(p, "    data: !!ascii85 |\n");
+	drm_printf(p, "     ");
+
+	for (i = 0; i < l; i++)
+		drm_printf(p, ascii85_encode(ptr[i], out));
+
+	drm_printf(p, "\n");
+}
+
 void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 		struct drm_printer *p)
 {
@@ -487,19 +520,20 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 		drm_printf(p, "    wptr: %d\n", state->ring[i].wptr);
 		drm_printf(p, "    size: %d\n", MSM_GPU_RINGBUFFER_SZ);
 
-		if (state->ring[i].data && state->ring[i].data_size) {
-			u32 *ptr = (u32 *) state->ring[i].data;
-			char out[ASCII85_BUFSZ];
-			long len = ascii85_encode_len(state->ring[i].data_size);
-			int j;
+		adreno_show_object(p, state->ring[i].data,
+			state->ring[i].data_size);
+	}
 
-			drm_printf(p, "    data: !!ascii85 |\n");
-			drm_printf(p, "     ");
+	if (state->bos) {
+		drm_printf(p, "bos:\n");
 
-			for (j = 0; j < len; j++)
-				drm_printf(p, ascii85_encode(ptr[j], out));
+		for (i = 0; i < state->nr_bos; i++) {
+			drm_printf(p, "  - iova: 0x%016llx\n",
+				state->bos[i].iova);
+			drm_printf(p, "    size: %ld\n", state->bos[i].size);
 
-			drm_printf(p, "\n");
+			adreno_show_object(p, state->bos[i].data,
+				state->bos[i].size);
 		}
 	}
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index f36b415e123b..92395c5ef442 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -318,8 +318,39 @@ static void msm_gpu_devcoredump_free(void *data)
 	msm_gpu_crashstate_put(gpu);
 }
 
-static void msm_gpu_crashstate_capture(struct msm_gpu *gpu, char *comm,
-		char *cmd)
+static void msm_gpu_crashstate_get_bo(struct msm_gpu_state *state,
+		struct msm_gem_object *obj, u64 iova, u32 flags)
+{
+	struct msm_gpu_state_bo *state_bo = &state->bos[state->nr_bos];
+
+	/* Don't record write only objects */
+
+	state_bo->size = obj->base.size;
+	state_bo->iova = iova;
+
+	/* Only store the data for buffer objects marked for read */
+	if ((flags & MSM_SUBMIT_BO_READ)) {
+		void *ptr;
+
+		state_bo->data = kmalloc(obj->base.size, GFP_KERNEL);
+		if (!state_bo->data)
+			return;
+
+		ptr = msm_gem_get_vaddr_active(&obj->base);
+		if (IS_ERR(ptr)) {
+			kfree(state_bo->data);
+			return;
+		}
+
+		memcpy(state_bo->data, ptr, obj->base.size);
+		msm_gem_put_vaddr(&obj->base);
+	}
+
+	state->nr_bos++;
+}
+
+static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
+		struct msm_gem_submit *submit, char *comm, char *cmd)
 {
 	struct msm_gpu_state *state;
 
@@ -335,6 +366,17 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu, char *comm,
 	state->comm = kstrdup(comm, GFP_KERNEL);
 	state->cmd = kstrdup(cmd, GFP_KERNEL);
 
+	if (submit) {
+		int i;
+
+		state->bos = kcalloc(submit->nr_bos,
+			sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
+
+		for (i = 0; state->bos && i < submit->nr_bos; i++)
+			msm_gpu_crashstate_get_bo(state, submit->bos[i].obj,
+				submit->bos[i].iova, submit->bos[i].flags);
+	}
+
 	kref_init(&state->ref);
 
 	/* Set the active crash state to be dumped on failure */
@@ -435,7 +477,7 @@ static void recover_worker(struct work_struct *work)
 	}
 
 	/* Record the crash state */
-	msm_gpu_crashstate_capture(gpu, comm, cmd);
+	msm_gpu_crashstate_capture(gpu, submit, comm, cmd);
 
 	kfree(cmd);
 	kfree(comm);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 48f7b21f1cae..8242c6e0f107 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -181,6 +181,12 @@ struct msm_gpu_submitqueue {
 	struct kref ref;
 };
 
+struct msm_gpu_state_bo {
+	u64 iova;
+	size_t size;
+	void *data;
+};
+
 struct msm_gpu_state {
 	struct kref ref;
 	struct timeval time;
@@ -201,6 +207,9 @@ struct msm_gpu_state {
 
 	char *comm;
 	char *cmd;
+
+	int nr_bos;
+	struct msm_gpu_state_bo *bos;
 };
 
 static inline void gpu_write(struct msm_gpu *gpu, u32 reg, u32 data)
-- 
2.16.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Jordan Crouse]

On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> The i915 DRM driver very cleverly used ascii85 encoding for their
> GPU state file. Move the encode functions to a general header file to
> support other drivers that might be interested in the same
> functionality.

In a previous version of this patch, Chris asked what tree I wanted this applied
to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
picking the rest up for msm-next for 4.18 but I'm okay with putting this
particular patch wherever it is easiest for the maintainers.

Jordan

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
>  drivers/gpu/drm/i915/i915_gpu_error.c | 35 ++++---------------------------
>  include/linux/ascii85.h               | 39 +++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 31 deletions(-)
>  create mode 100644 include/linux/ascii85.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 67c902412193..969d967e58c7 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -31,7 +31,7 @@
>  #include <linux/stop_machine.h>
>  #include <linux/zlib.h>
>  #include <drm/drm_print.h>
> -
> +#include <linux/ascii85.h>
>  #include "i915_drv.h"
>  
>  static inline const struct intel_engine_cs *
> @@ -518,35 +518,12 @@ void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...)
>  	va_end(args);
>  }
>  
> -static int
> -ascii85_encode_len(int len)
> -{
> -	return DIV_ROUND_UP(len, 4);
> -}
> -
> -static bool
> -ascii85_encode(u32 in, char *out)
> -{
> -	int i;
> -
> -	if (in == 0)
> -		return false;
> -
> -	out[5] = '\0';
> -	for (i = 5; i--; ) {
> -		out[i] = '!' + in % 85;
> -		in /= 85;
> -	}
> -
> -	return true;
> -}
> -
>  static void print_error_obj(struct drm_i915_error_state_buf *m,
>  			    struct intel_engine_cs *engine,
>  			    const char *name,
>  			    struct drm_i915_error_object *obj)
>  {
> -	char out[6];
> +	char out[ASCII85_BUFSZ];
>  	int page;
>  
>  	if (!obj)
> @@ -568,12 +545,8 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
>  			len -= obj->unused;
>  		len = ascii85_encode_len(len);
>  
> -		for (i = 0; i < len; i++) {
> -			if (ascii85_encode(obj->pages[page][i], out))
> -				err_puts(m, out);
> -			else
> -				err_puts(m, "z");
> -		}
> +		for (i = 0; i < len; i++)
> +			error_puts(m, ascii85_encode(obj->pages[page][i], out));
>  	}
>  	err_puts(m, "\n");
>  }
> diff --git a/include/linux/ascii85.h b/include/linux/ascii85.h
> new file mode 100644
> index 000000000000..322bbed731ae
> --- /dev/null
> +++ b/include/linux/ascii85.h
> @@ -0,0 +1,39 @@
> +
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (c) 2008 Intel Corporation
> + * Copyright (c) The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef _ASCII85_H_
> +#define _ASCII85_H_
> +
> +#include <linux/kernel.h>
> +
> +#define ASCII85_BUFSZ 6
> +
> +static inline long
> +ascii85_encode_len(long len)
> +{
> +	return DIV_ROUND_UP(len, 4);
> +}
> +
> +static inline char *
> +ascii85_encode(u32 in, char *out)
> +{
> +	int i;
> +
> +	if (in == 0)
> +		return "z";
> +
> +	out[5] = '\0';
> +	for (i = 5; i--; ) {
> +		out[i] = '!' + in % 85;
> +		in /= 85;
> +	}
> +
> +	return out;
> +}
> +
> +#endif
> -- 
> 2.16.1
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Intel-gfx] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Chris Wilson]

Quoting Jordan Crouse (2018-04-05 23:06:53)
> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> > The i915 DRM driver very cleverly used ascii85 encoding for their
> > GPU state file. Move the encode functions to a general header file to
> > support other drivers that might be interested in the same
> > functionality.
> 
> In a previous version of this patch, Chris asked what tree I wanted this applied
> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
> picking the rest up for msm-next for 4.18 but I'm okay with putting this
> particular patch wherever it is easiest for the maintainers.

We are a bit late to sneak it into the 4.17 drm base via i915. I don't
anticipate any problems (for i915) with this patch going in through
msm-next, so happy to have it land there and percolate back to i915 3
months later.
-Chris
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 02/10] drm: drm_printer: Add printer for devcoredump

[Chris Wilson]

Quoting Jordan Crouse (2018-04-05 23:00:48)
> +struct drm_print_iterator {
> +       void *data;
> +
> +       ssize_t start;
> +       ssize_t offset;
> +       ssize_t remain;
> +};

Neat, we should be able to use to replace struct
drm_i915_error_state_buf (or at least the seq_file side).
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/10] drm: drm_printer: Add printer for devcoredump

[Chris Wilson]

Quoting Chris Wilson (2018-04-06 11:42:25)
> Quoting Jordan Crouse (2018-04-05 23:00:48)
> > +struct drm_print_iterator {
> > +       void *data;
> > +
> > +       ssize_t start;
> > +       ssize_t offset;
> > +       ssize_t remain;
> > +};
> 
> Neat, we should be able to use to replace struct
> drm_i915_error_state_buf (or at least the seq_file side).

Though comparing against which, we have loff_t for that interface.
But that's immaterial for an in-memory file. (So nothing to see here,
please move along.)
-Chris
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 03/10] drm/msm/gpu: Capture the state of the GPU

[Chris Wilson]

Quoting Jordan Crouse (2018-04-05 23:00:49)
> +struct msm_gpu_state {
> +       struct timeval time;

My recommendation would be to make this a kreffed struct. You already
have multiple uncoordinated consumers so managing the lifetime would
seem to be a point of concern?
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/10] drm/msm/gpu: Convert the GPU show function to use the GPU state

[Chris Wilson]

Quoting Jordan Crouse (2018-04-05 23:00:50)
> diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
> index ba74cb4f94df..fd535dab3d5b 100644
> --- a/drivers/gpu/drm/msm/msm_debugfs.c
> +++ b/drivers/gpu/drm/msm/msm_debugfs.c
> @@ -25,13 +25,22 @@ static int msm_gpu_show(struct drm_device *dev, struct seq_file *m)
>  {
>         struct msm_drm_private *priv = dev->dev_private;
>         struct msm_gpu *gpu = priv->gpu;
> +       struct msm_gpu_state *state;
>  
> -       if (gpu) {
> -               seq_printf(m, "%s Status:\n", gpu->name);
> -               pm_runtime_get_sync(&gpu->pdev->dev);
> -               gpu->funcs->show(gpu, m);
> -               pm_runtime_put_sync(&gpu->pdev->dev);
> -       }
> +       if (!gpu)
> +               return 0;
> +
> +       pm_runtime_get_sync(&gpu->pdev->dev);
> +       state = gpu->funcs->gpu_state_get(gpu);
> +       pm_runtime_put_sync(&gpu->pdev->dev);
> +
> +       if (IS_ERR(state))
> +               return PTR_ERR(state);
> +
> +       seq_printf(m, "%s Status:\n", gpu->name);
> +       gpu->funcs->show(gpu, state, m);
> +
> +       gpu->funcs->gpu_state_put(state);

Ah. This be trickier than it appears thanks to how seq_file tries to
keep the interface simple :)

For a large buffer, seq_file will call the show multiple times to
convert it into a single string, which it then iterates over. (iirc)

Ideally, you grab the error state on open, and then use the
drm_printer_iterator you have to feed the chunks to seqfs. At a minimum,
I do recommend you stick the get into the seq_open callback, as my
memory says the show will be called multiple times.
-Chris
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 07/10] drm/msm/adreno: Convert the show/crash file format

[Chris Wilson]

Quoting Jordan Crouse (2018-04-05 23:00:53)
> Convert the format of the 'show' debugfs file and the crash
> dump to a  format resembling YAML. This should be easier to
> parse and be more flexible for future changes and expansions.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

+1, I've been trying to work up the courage to make the conversion to
yaml for i915 as well.

My gut feeling says we at least want to move the indentation handling to
drm_print.

>         for (i = 0; i < state->nr_registers; i++) {
> -               drm_printf(p, "IO:R %08x %08x\n",
> +               drm_printf(p, "  - [0x%04x, 0x%08x]\n",

Hmm, sequence of sequences. The alternative would be
   - {offset:%x, value:%x}

May I ask why you would pick one over the other?

I like the verbosity of having the nodes, it should also mean that the
order is immaterial and we could extend it with say name:, comments: or
whatever, without affecting the parser.

Otoh, the [] are more compact.
-Chris
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 10/10] drm/msm/gpu: Add the buffer objects from the submit to the crash dump

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3284 bytes --]

Hi Jordan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robclark/msm-next]
[also build test WARNING on next-20180406]
[cannot apply to v4.16]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jordan-Crouse/drm-msm-GPU-crash-state/20180406-170513
base:   git://people.freedesktop.org/~robclark/linux msm-next
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/msm/adreno/adreno_gpu.c: In function 'adreno_show':
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c:533:31: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t {aka unsigned int}' [-Wformat=]
       drm_printf(p, "    size: %ld\n", state->bos[i].size);
                                ~~^     ~~~~~~~~~~~~~~~~~~
                                %d

vim +533 drivers/gpu/drm/msm/adreno/adreno_gpu.c

   496	
   497	void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
   498			struct drm_printer *p)
   499	{
   500		struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
   501		int i;
   502	
   503		if (IS_ERR_OR_NULL(state))
   504			return;
   505	
   506		drm_printf(p, "revision: %d (%d.%d.%d.%d)\n",
   507				adreno_gpu->info->revn, adreno_gpu->rev.core,
   508				adreno_gpu->rev.major, adreno_gpu->rev.minor,
   509				adreno_gpu->rev.patchid);
   510	
   511		drm_printf(p, "rbbm-status: 0x%08x\n", state->rbbm_status);
   512	
   513		drm_printf(p, "ringbuffer:\n");
   514	
   515		for (i = 0; i < gpu->nr_rings; i++) {
   516			drm_printf(p, "  - id: %d\n", i);
   517			drm_printf(p, "    last-fence: %d\n", state->ring[i].seqno);
   518			drm_printf(p, "    retired-fence: %d\n", state->ring[i].fence);
   519			drm_printf(p, "    rptr: %d\n", state->ring[i].rptr);
   520			drm_printf(p, "    wptr: %d\n", state->ring[i].wptr);
   521			drm_printf(p, "    size: %d\n", MSM_GPU_RINGBUFFER_SZ);
   522	
   523			adreno_show_object(p, state->ring[i].data,
   524				state->ring[i].data_size);
   525		}
   526	
   527		if (state->bos) {
   528			drm_printf(p, "bos:\n");
   529	
   530			for (i = 0; i < state->nr_bos; i++) {
   531				drm_printf(p, "  - iova: 0x%016llx\n",
   532					state->bos[i].iova);
 > 533				drm_printf(p, "    size: %ld\n", state->bos[i].size);
   534	
   535				adreno_show_object(p, state->bos[i].data,
   536					state->bos[i].size);
   537			}
   538		}
   539	
   540		drm_printf(p, "registers:\n");
   541		drm_printf(p, "  - [offset, value]\n");
   542	
   543		for (i = 0; i < state->nr_registers; i++) {
   544			drm_printf(p, "  - [0x%04x, 0x%08x]\n",
   545				state->registers[i * 2] << 2,
   546				state->registers[(i * 2) + 1]);
   547		}
   548	}
   549	#endif
   550	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43141 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]

Hi Jordan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robclark/msm-next]
[also build test ERROR on v4.16 next-20180406]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jordan-Crouse/drm-msm-GPU-crash-state/20180406-170513
base:   git://people.freedesktop.org/~robclark/linux msm-next
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_gpu_error.c: In function 'print_error_obj':
>> drivers/gpu//drm/i915/i915_gpu_error.c:549:4: error: implicit declaration of function 'error_puts'; did you mean 'err_puts'? [-Werror=implicit-function-declaration]
       error_puts(m, ascii85_encode(obj->pages[page][i], out));
       ^~~~~~~~~~
       err_puts
   cc1: some warnings being treated as errors

vim +549 drivers/gpu//drm/i915/i915_gpu_error.c

   520	
   521	static void print_error_obj(struct drm_i915_error_state_buf *m,
   522				    struct intel_engine_cs *engine,
   523				    const char *name,
   524				    struct drm_i915_error_object *obj)
   525	{
   526		char out[ASCII85_BUFSZ];
   527		int page;
   528	
   529		if (!obj)
   530			return;
   531	
   532		if (name) {
   533			err_printf(m, "%s --- %s = 0x%08x %08x\n",
   534				   engine ? engine->name : "global", name,
   535				   upper_32_bits(obj->gtt_offset),
   536				   lower_32_bits(obj->gtt_offset));
   537		}
   538	
   539		err_compression_marker(m);
   540		for (page = 0; page < obj->page_count; page++) {
   541			int i, len;
   542	
   543			len = PAGE_SIZE;
   544			if (page == obj->page_count - 1)
   545				len -= obj->unused;
   546			len = ascii85_encode_len(len);
   547	
   548			for (i = 0; i < len; i++)
 > 549				error_puts(m, ascii85_encode(obj->pages[page][i], out));
   550		}
   551		err_puts(m, "\n");
   552	}
   553	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40815 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Intel-gfx] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1136 bytes --]

Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org> writes:

> Quoting Jordan Crouse (2018-04-05 23:06:53)
>> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
>> > The i915 DRM driver very cleverly used ascii85 encoding for their
>> > GPU state file. Move the encode functions to a general header file to
>> > support other drivers that might be interested in the same
>> > functionality.
>> 
>> In a previous version of this patch, Chris asked what tree I wanted this applied
>> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
>> picking the rest up for msm-next for 4.18 but I'm okay with putting this
>> particular patch wherever it is easiest for the maintainers.
>
> We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> anticipate any problems (for i915) with this patch going in through
> msm-next, so happy to have it land there and percolate back to i915 3
> months later.

I'd love to have it in drm-misc-next, so I can build a similar hang dump
interface for vc5.  But most importantly, I'd like to have it somewhere
soon.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Intel-gfx] [Freedreno] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Daniel Vetter]

On Mon, Apr 16, 2018 at 10:52:59AM -0700, Eric Anholt wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Jordan Crouse (2018-04-05 23:06:53)
> >> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> >> > The i915 DRM driver very cleverly used ascii85 encoding for their
> >> > GPU state file. Move the encode functions to a general header file to
> >> > support other drivers that might be interested in the same
> >> > functionality.
> >> 
> >> In a previous version of this patch, Chris asked what tree I wanted this applied
> >> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
> >> picking the rest up for msm-next for 4.18 but I'm okay with putting this
> >> particular patch wherever it is easiest for the maintainers.
> >
> > We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> > anticipate any problems (for i915) with this patch going in through
> > msm-next, so happy to have it land there and percolate back to i915 3
> > months later.
> 
> I'd love to have it in drm-misc-next, so I can build a similar hang dump
> interface for vc5.  But most importantly, I'd like to have it somewhere
> soon.

Maarten will likely send the first pull for drm-misc-next at the end of
this week. So if you just push it there today, there's no hold-up for msm
(or anyone else) really.
-Daniel
-- 
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: [Intel-gfx] [PATCH 01/10] include: Move ascii85 functions from i915 to linux/ascii85.h

[Jordan Crouse]

On Mon, Apr 16, 2018 at 10:52:59AM -0700, Eric Anholt wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Jordan Crouse (2018-04-05 23:06:53)
> >> On Thu, Apr 05, 2018 at 04:00:47PM -0600, Jordan Crouse wrote:
> >> > The i915 DRM driver very cleverly used ascii85 encoding for their
> >> > GPU state file. Move the encode functions to a general header file to
> >> > support other drivers that might be interested in the same
> >> > functionality.
> >> 
> >> In a previous version of this patch, Chris asked what tree I wanted this applied
> >> to, and the answer is: I'm not sure?  I'm hoping that Rob will be cool with
> >> picking the rest up for msm-next for 4.18 but I'm okay with putting this
> >> particular patch wherever it is easiest for the maintainers.
> >
> > We are a bit late to sneak it into the 4.17 drm base via i915. I don't
> > anticipate any problems (for i915) with this patch going in through
> > msm-next, so happy to have it land there and percolate back to i915 3
> > months later.
> 
> I'd love to have it in drm-misc-next, so I can build a similar hang dump
> interface for vc5.  But most importantly, I'd like to have it somewhere
> soon.

I'll fix the bot error and push it up again today.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno