This is revision t implementing a GPU crash state for drm/msm (https://patchwork.freedesktop.org/series/36097/). This patchset fixes a few things that the build bot found.
The object of this code 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.
v7: Add EXPORT_SYMBOL for __drm_puts_coredump and use %zd to print a size_t variable for the bo dump thanks to the ever vigilant zero one bot. v6: Add drm_puts() and use it in the appropriate place. Clean up a few minor bugs here and there. v5: Fix symbol error in i915_gpu_error.c thanks to 01 dot org bot. Added open/release functions for the show debugfs file to get the state per Chris Wilson. Slightly modified the register output format to be more YAML friendly also per Chris. 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 (13): include: Move ascii85 functions from i915 to linux/ascii85.h drm: drm_printer: Add printer for devcoredump drm: Add drm_puts() to complement drm_printf() drm: Add a -puts() function for the seq_file printer drm: Add put callback for the coredump printer 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 | 58 ++++++ drivers/gpu/drm/drm_print.c | 132 +++++++++++++ drivers/gpu/drm/i915/i915_gpu_error.c | 34 +--- 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 | 242 +++++++++++++++++++++-- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 184 +++++++++++++++-- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 10 +- drivers/gpu/drm/msm/msm_debugfs.c | 93 ++++++++- drivers/gpu/drm/msm/msm_gpu.c | 143 +++++++++++++- drivers/gpu/drm/msm/msm_gpu.h | 67 ++++++- include/drm/drm_print.h | 33 ++++ include/linux/ascii85.h | 39 ++++ 14 files changed, 988 insertions(+), 100 deletions(-) create mode 100644 Documentation/gpu/drm-msm-crash-dump.txt create mode 100644 include/linux/ascii85.h
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.
v3: Fix error_puts -> err_puts pointed out by the 01.org bot v2: Update API to be cleaner for the caller as suggested by Chris Wilson
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org --- drivers/gpu/drm/i915/i915_gpu_error.c | 34 +++-------------------- include/linux/ascii85.h | 39 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 30 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 df234dc23274..284e899ca8ff 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -31,6 +31,7 @@ #include <linux/stop_machine.h> #include <linux/zlib.h> #include <drm/drm_print.h> +#include <linux/ascii85.h>
#include "i915_gpu_error.h" #include "i915_drv.h" @@ -522,35 +523,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) @@ -572,12 +550,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++) + err_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..11b9146a3bc4 --- /dev/null +++ b/include/linux/ascii85.h @@ -0,0 +1,39 @@ + +/* + * SPDX-License-Identifier: GPL-2.0 + * + * Copyright (c) 2008 Intel Corporation + * Copyright (c) 2018 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
Quoting Jordan Crouse (2018-07-12 19:59:18)
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.
v3: Fix error_puts -> err_puts pointed out by the 01.org bot v2: Update API to be cleaner for the caller as suggested by Chris Wilson
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
+static inline long +ascii85_encode_len(long len) +{
return DIV_ROUND_UP(len, 4);+}
+static inline char *
const char * to avoid a compiler warning with return "z".
Looks like that will be ok with the callers.
With that, Reviewed-by: Chris Wilson chris@chris-wilson.co.uk -Chris
Add a drm printer suitable for use with the read callback for devcoredump or other suitable buffer based output format that isn't otherwise covered by seq_file.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org --- drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 27 ++++++++++++++ 2 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index b25f98f33f6c..03d1f98e5ac7 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,80 @@ #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; + + /* Figure out how big the string will be */ + len = snprintf(NULL, 0, "%pV", vaf); + + if (iterator->offset < iterator->start) { + char *buf; + ssize_t copy; + + 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 { + char *buf; + ssize_t pos = iterator->offset - iterator->start; + + if (len < iterator->remain) { + snprintf(((char *) iterator->data) + pos, + iterator->remain, "%pV", vaf); + + iterator->offset += len; + iterator->remain -= 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 out the remaining bits */ + memcpy(iterator->data + pos, buf, iterator->remain); + + iterator->offset += iterator->remain; + iterator->remain = 0; + + kfree(buf); + } +} +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 e1a46e9991cc..0ea440fb5ec3 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
Quoting Jordan Crouse (2018-07-12 19:59:19)
Add a drm printer suitable for use with the read callback for devcoredump or other suitable buffer based output format that isn't otherwise covered by seq_file.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 27 ++++++++++++++ 2 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index b25f98f33f6c..03d1f98e5ac7 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,80 @@ #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;/* Figure out how big the string will be */len = snprintf(NULL, 0, "%pV", vaf);
I was thinking there's some duplication here (kmalloc + snprintf) that could be reduced to kasprintf here. Is avoiding that allocation important or frequent enough to merit open coding?
It's pity the kernel's printk doesn't support %n, so that leaves with
buf = kasprintf(GFP_... , "%pV", vaf); if (!buf) return;
len = strlen(buf);
and even the copy + increment looks like it can then be factored to share more code. -Chris
On Thu, Jul 12, 2018 at 08:40:55PM +0100, Chris Wilson wrote:
Quoting Jordan Crouse (2018-07-12 19:59:19)
Add a drm printer suitable for use with the read callback for devcoredump or other suitable buffer based output format that isn't otherwise covered by seq_file.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 27 ++++++++++++++ 2 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index b25f98f33f6c..03d1f98e5ac7 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,80 @@ #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;/* Figure out how big the string will be */len = snprintf(NULL, 0, "%pV", vaf);I was thinking there's some duplication here (kmalloc + snprintf) that could be reduced to kasprintf here. Is avoiding that allocation important or frequent enough to merit open coding?
It's pity the kernel's printk doesn't support %n, so that leaves with
buf = kasprintf(GFP_... , "%pV", vaf); if (!buf) return;
len = strlen(buf);
and even the copy + increment looks like it can then be factored to share more code.
I could profile it to see if avoiding the allocation is worth while.
I have a use case that prints approximately 1MB (stupid GPU, be less complex) so that is enough to be able to see noticeable deltas if they exist.
Jordan
On Thu, Jul 12, 2018 at 08:40:55PM +0100, Chris Wilson wrote:
Quoting Jordan Crouse (2018-07-12 19:59:19)
Add a drm printer suitable for use with the read callback for devcoredump or other suitable buffer based output format that isn't otherwise covered by seq_file.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 27 ++++++++++++++ 2 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index b25f98f33f6c..03d1f98e5ac7 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,80 @@ #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;/* Figure out how big the string will be */len = snprintf(NULL, 0, "%pV", vaf);I was thinking there's some duplication here (kmalloc + snprintf) that could be reduced to kasprintf here. Is avoiding that allocation important or frequent enough to merit open coding?
It's pity the kernel's printk doesn't support %n, so that leaves with
buf = kasprintf(GFP_... , "%pV", vaf); if (!buf) return;
len = strlen(buf);
and even the copy + increment looks like it can then be factored to share more code.
I did a quick test - using kasprintf() unconditionally increased the total time in my use case by about 4x. I think this was mainly due to this case:
if (iterator->offset + len <= iterator->start) { .... return; }
That said, I was able to organize the code so that we can reuse much of the same code for both the printf and puts funcs which saves us a bit of churn later.
Jordan
On Thu, Jul 12, 2018 at 12:59:19PM -0600, Jordan Crouse wrote:
Add a drm printer suitable for use with the read callback for devcoredump or other suitable buffer based output format that isn't otherwise covered by seq_file.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
Hm, why not add seq_file support to dev_coredump? Neither git blame nor google sched any light on why seq_file wasn't picked over the custom read interface ...
Adding Johannes and Greg about this.
If we go with this, one comment below.
drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 27 ++++++++++++++ 2 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index b25f98f33f6c..03d1f98e5ac7 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,80 @@ #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;
- /* Figure out how big the string will be */
- len = snprintf(NULL, 0, "%pV", vaf);
- if (iterator->offset < iterator->start) {
- char *buf;
- ssize_t copy;
- 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 {
- char *buf;
- ssize_t pos = iterator->offset - iterator->start;
- if (len < iterator->remain) {
- snprintf(((char *) iterator->data) + pos,
- iterator->remain, "%pV", vaf);
- iterator->offset += len;
- iterator->remain -= 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 out the remaining bits */
- memcpy(iterator->data + pos, buf, iterator->remain);
- iterator->offset += iterator->remain;
- iterator->remain = 0;
- kfree(buf);
- }
+} +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 e1a46e9991cc..0ea440fb5ec3 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
Bit more flesh for the kerneldoc would be good here, maybe even with a small in-line example. Definitely a link to dev_coredumpm() which I assume is the function you're going to use this with.
Pls also make sure it all looks nice using make htmldocs. -Daniel
- 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.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jul 12, 2018 at 09:46:58PM +0200, Daniel Vetter wrote:
On Thu, Jul 12, 2018 at 12:59:19PM -0600, Jordan Crouse wrote:
Add a drm printer suitable for use with the read callback for devcoredump or other suitable buffer based output format that isn't otherwise covered by seq_file.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
Hm, why not add seq_file support to dev_coredump? Neither git blame nor google sched any light on why seq_file wasn't picked over the custom read interface ...
Adding Johannes and Greg about this.
Main reason was that this is used for devcoredump which has its own similar but not quite seq_file compatible callback. If there is synergy to be had there that would be great because reinventing the wheel isn't fun.
If we go with this, one comment below.
drivers/gpu/drm/drm_print.c | 74 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 27 ++++++++++++++ 2 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index b25f98f33f6c..03d1f98e5ac7 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,80 @@ #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;
- /* Figure out how big the string will be */
- len = snprintf(NULL, 0, "%pV", vaf);
- if (iterator->offset < iterator->start) {
- char *buf;
- ssize_t copy;
- 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 {
- char *buf;
- ssize_t pos = iterator->offset - iterator->start;
- if (len < iterator->remain) {
- snprintf(((char *) iterator->data) + pos,
- iterator->remain, "%pV", vaf);
- iterator->offset += len;
- iterator->remain -= 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 out the remaining bits */
- memcpy(iterator->data + pos, buf, iterator->remain);
- iterator->offset += iterator->remain;
- iterator->remain = 0;
- kfree(buf);
- }
+} +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 e1a46e9991cc..0ea440fb5ec3 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
Bit more flesh for the kerneldoc would be good here, maybe even with a small in-line example. Definitely a link to dev_coredumpm() which I assume is the function you're going to use this with.
Pls also make sure it all looks nice using make htmldocs.
Can do. Thanks.
-Daniel
- 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.17.1
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
Hm, why not add seq_file support to dev_coredump? Neither git blame nor google sched any light on why seq_file wasn't picked over the custom read interface ...
Adding Johannes and Greg about this.
Main reason was that this is used for devcoredump which has its own similar but not quite seq_file compatible callback. If there is synergy to be had there that would be great because reinventing the wheel isn't fun.
Adding or changing it to seq_file is fine with me, I don't think we really need the devm_coredump() much these days since we have the vmalloc one.
(apologies for the footer and all - I'm on vacation and in a hurry)
johannes
Add drm_puts() for a much faster path to print constant strings into a drm_printer object with memcpy and friends. This can shave seconds off of really large outputs such as GPU dumps.
If the drm_printer object supports a custom puts function then use that otherwise fall back to the slower legacy printf call.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org --- drivers/gpu/drm/drm_print.c | 9 +++++++++ include/drm/drm_print.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 03d1f98e5ac7..8fd489248a50 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -122,6 +122,15 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf) } EXPORT_SYMBOL(__drm_printfn_debug);
+void drm_puts(struct drm_printer *p, const char *str) +{ + if (p->puts) + p->puts(p, str); + else + drm_printf(p, "%s", str); +} +EXPORT_SYMBOL(drm_puts); + /** * drm_printf - print to a &drm_printer stream * @p: the &drm_printer diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 0ea440fb5ec3..b16f4ecaa984 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -69,6 +69,7 @@ struct drm_printer { /* private: */ void (*printfn)(struct drm_printer *p, struct va_format *vaf); + void (*puts)(struct drm_printer *p, const char *str); void *arg; const char *prefix; }; @@ -80,6 +81,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
__printf(2, 3) void drm_printf(struct drm_printer *p, const char *f, ...); +void drm_puts(struct drm_printer *p, const char *str);
__printf(2, 0) /**
On Thu, Jul 12, 2018 at 12:59:20PM -0600, Jordan Crouse wrote:
Add drm_puts() for a much faster path to print constant strings into a drm_printer object with memcpy and friends. This can shave seconds off of really large outputs such as GPU dumps.
If the drm_printer object supports a custom puts function then use that otherwise fall back to the slower legacy printf call.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
drivers/gpu/drm/drm_print.c | 9 +++++++++ include/drm/drm_print.h | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 03d1f98e5ac7..8fd489248a50 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -122,6 +122,15 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf) } EXPORT_SYMBOL(__drm_printfn_debug);
Please some small kerneldoc here, with that
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
btw want commit rights for drm-misc for stuff like this? -Daniel
+void drm_puts(struct drm_printer *p, const char *str) +{
- if (p->puts)
p->puts(p, str);- else
drm_printf(p, "%s", str);+} +EXPORT_SYMBOL(drm_puts);
/**
- drm_printf - print to a &drm_printer stream
- @p: the &drm_printer
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 0ea440fb5ec3..b16f4ecaa984 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -69,6 +69,7 @@ struct drm_printer { /* private: */ void (*printfn)(struct drm_printer *p, struct va_format *vaf);
- void (*puts)(struct drm_printer *p, const char *str); void *arg; const char *prefix;
}; @@ -80,6 +81,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
__printf(2, 3) void drm_printf(struct drm_printer *p, const char *f, ...); +void drm_puts(struct drm_printer *p, const char *str);
__printf(2, 0) /** -- 2.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Add a puts() function to use seq_puts() to help speed up up print time for constant strings.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org --- drivers/gpu/drm/drm_print.c | 6 ++++++ include/drm/drm_print.h | 2 ++ 2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 8fd489248a50..bef8f0ec5d73 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -104,6 +104,12 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf) } EXPORT_SYMBOL(__drm_printfn_coredump);
+void __drm_puts_seq_file(struct drm_printer *p, const char *str) +{ + seq_puts(p->arg, str); +} +EXPORT_SYMBOL(__drm_puts_seq_file); + 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 b16f4ecaa984..3bc6ba4b7b2c 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -76,6 +76,7 @@ struct drm_printer {
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_puts_seq_file(struct drm_printer *p, const char *str); void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
@@ -144,6 +145,7 @@ static inline struct drm_printer drm_seq_file_printer(struct seq_file *f) { struct drm_printer p = { .printfn = __drm_printfn_seq_file, + .puts = __drm_puts_seq_file, .arg = f, }; return p;
On Thu, Jul 12, 2018 at 12:59:21PM -0600, Jordan Crouse wrote:
Add a puts() function to use seq_puts() to help speed up up print time for constant strings.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_print.c | 6 ++++++ include/drm/drm_print.h | 2 ++ 2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 8fd489248a50..bef8f0ec5d73 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -104,6 +104,12 @@ void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf) } EXPORT_SYMBOL(__drm_printfn_coredump);
+void __drm_puts_seq_file(struct drm_printer *p, const char *str) +{
- seq_puts(p->arg, str);
+} +EXPORT_SYMBOL(__drm_puts_seq_file);
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 b16f4ecaa984..3bc6ba4b7b2c 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -76,6 +76,7 @@ struct drm_printer {
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_puts_seq_file(struct drm_printer *p, const char *str); void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
@@ -144,6 +145,7 @@ static inline struct drm_printer drm_seq_file_printer(struct seq_file *f) { struct drm_printer p = { .printfn = __drm_printfn_seq_file,
.puts = __drm_puts_seq_file,-- 2.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Add a put function for the coredump printer to bypass printf() for constant strings for a speed boost.
v2: Add EXPORT_SYMBOL for _drm_puts_coredump Signed-off-by: Jordan Crouse jcrouse@codeaurora.org --- drivers/gpu/drm/drm_print.c | 43 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 2 ++ 2 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index bef8f0ec5d73..ff20f4a764c8 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,49 @@ #include <drm/drmP.h> #include <drm/drm_print.h>
+void __drm_puts_coredump(struct drm_printer *p, const char *str) +{ + struct drm_print_iterator *iterator = p->arg; + + ssize_t len; + + if (!iterator->remain) + return; + + if (iterator->offset < iterator->start) { + ssize_t copy; + + len = strlen(str); + + if (iterator->offset + len <= iterator->start) { + iterator->offset += len; + return; + } + + 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, + str + (iterator->start - iterator->offset), copy); + + iterator->offset = iterator->start + copy; + iterator->remain -= copy; + } else { + ssize_t pos = iterator->offset - iterator->start; + + len = min_t(ssize_t, strlen(str), iterator->remain); + + memcpy(iterator->data + pos, str, len); + + iterator->offset += len; + iterator->remain -= len; + } +} +EXPORT_SYMBOL(__drm_puts_coredump); + void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf) { struct drm_print_iterator *iterator = p->arg; diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 3bc6ba4b7b2c..2a903ee7b428 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -75,6 +75,7 @@ struct drm_printer { };
void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf); +void __drm_puts_coredump(struct drm_printer *p, const char *str); void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf); void __drm_puts_seq_file(struct drm_printer *p, const char *str); void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); @@ -129,6 +130,7 @@ drm_coredump_printer(struct drm_print_iterator *iter) { struct drm_printer p = { .printfn = __drm_printfn_coredump, + .puts = __drm_puts_coredump, .arg = iter, }; return p;
Quoting Jordan Crouse (2018-07-12 19:59:22)
Add a put function for the coredump printer to bypass printf() for constant strings for a speed boost.
v2: Add EXPORT_SYMBOL for _drm_puts_coredump Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
drivers/gpu/drm/drm_print.c | 43 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 2 ++ 2 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index bef8f0ec5d73..ff20f4a764c8 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,49 @@ #include <drm/drmP.h> #include <drm/drm_print.h>
+void __drm_puts_coredump(struct drm_printer *p, const char *str) +{
struct drm_print_iterator *iterator = p->arg;ssize_t len;if (!iterator->remain)return;if (iterator->offset < iterator->start) {ssize_t copy;len = strlen(str);
printfn_coredump looks like it would then wrap puts_coredump? -Chris
On Thu, Jul 12, 2018 at 2:59 PM, Jordan Crouse jcrouse@codeaurora.org wrote:
Add a put function for the coredump printer to bypass printf() for constant strings for a speed boost.
s/put/puts/ (and in the $subject)
BR, -R
v2: Add EXPORT_SYMBOL for _drm_puts_coredump Signed-off-by: Jordan Crouse jcrouse@codeaurora.org
drivers/gpu/drm/drm_print.c | 43 +++++++++++++++++++++++++++++++++++++ include/drm/drm_print.h | 2 ++ 2 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index bef8f0ec5d73..ff20f4a764c8 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -30,6 +30,49 @@ #include <drm/drmP.h> #include <drm/drm_print.h>
+void __drm_puts_coredump(struct drm_printer *p, const char *str) +{
struct drm_print_iterator *iterator = p->arg;ssize_t len;if (!iterator->remain)return;if (iterator->offset < iterator->start) {ssize_t copy;len = strlen(str);if (iterator->offset + len <= iterator->start) {iterator->offset += len;return;}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,str + (iterator->start - iterator->offset), copy);iterator->offset = iterator->start + copy;iterator->remain -= copy;} else {ssize_t pos = iterator->offset - iterator->start;len = min_t(ssize_t, strlen(str), iterator->remain);memcpy(iterator->data + pos, str, len);iterator->offset += len;iterator->remain -= len;}+} +EXPORT_SYMBOL(__drm_puts_coredump);
void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf) { struct drm_print_iterator *iterator = p->arg; diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 3bc6ba4b7b2c..2a903ee7b428 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -75,6 +75,7 @@ struct drm_printer { };
void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf); +void __drm_puts_coredump(struct drm_printer *p, const char *str); void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf); void __drm_puts_seq_file(struct drm_printer *p, const char *str); void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); @@ -129,6 +130,7 @@ drm_coredump_printer(struct drm_print_iterator *iter) { struct drm_printer p = { .printfn = __drm_printfn_coredump,
.puts = __drm_puts_coredump, .arg = iter, }; return p;-- 2.17.1
Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Add the infrastructure to capture the current state of the GPU and store it in memory so that 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 should 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 d39400e5bc42..9e85e4f7016d 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 bcbf9f2a29f9..c7a0d278c59e 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) { + u32 start = adreno_gpu->registers[i]; + u32 end = adreno_gpu->registers[i + 1]; + u32 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 bc9ec27e9ed8..734e31a9631f 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -229,6 +229,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));
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 | 3 +- drivers/gpu/drm/msm/msm_debugfs.c | 92 ++++++++++++++++++++++--- drivers/gpu/drm/msm/msm_gpu.h | 3 +- 7 files changed, 104 insertions(+), 65 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 9e85e4f7016d..5f1aab3c1cb1 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 c7a0d278c59e..0e937eedcec5 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 734e31a9631f..90b6b59252af 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -216,7 +216,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); diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c index 1ff3fda245d1..c3da12179888 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.c +++ b/drivers/gpu/drm/msm/msm_debugfs.c @@ -16,26 +16,100 @@ */
#ifdef CONFIG_DEBUG_FS +#include <linux/debugfs.h> #include "msm_drv.h" #include "msm_gpu.h" #include "msm_kms.h" #include "msm_debugfs.h"
-static int msm_gpu_show(struct drm_device *dev, struct seq_file *m) +struct msm_gpu_show_priv { + struct msm_gpu_state *state; + struct drm_device *dev; +}; + +static int msm_gpu_show(struct seq_file *m, void *arg) +{ + struct msm_gpu_show_priv *show_priv = m->private; + struct msm_drm_private *priv = show_priv->dev->dev_private; + struct msm_gpu *gpu = priv->gpu; + int ret; + + ret = mutex_lock_interruptible(&show_priv->dev->struct_mutex); + if (ret) + return ret; + + seq_printf(m, "%s Status:\n", gpu->name); + gpu->funcs->show(gpu, show_priv->state, m); + + mutex_unlock(&show_priv->dev->struct_mutex); + + return 0; +} + +static int msm_gpu_release(struct inode *inode, struct file *file) +{ + struct seq_file *m = file->private_data; + struct msm_gpu_show_priv *show_priv = m->private; + struct msm_drm_private *priv = show_priv->dev->dev_private; + struct msm_gpu *gpu = priv->gpu; + int ret; + + ret = mutex_lock_interruptible(&show_priv->dev->struct_mutex); + if (ret) + return ret; + + gpu->funcs->gpu_state_put(show_priv->state); + mutex_unlock(&show_priv->dev->struct_mutex); + + kfree(show_priv); + + return single_release(inode, file); +} + +static int msm_gpu_open(struct inode *inode, struct file *file) { + struct drm_device *dev = inode->i_private; struct msm_drm_private *priv = dev->dev_private; struct msm_gpu *gpu = priv->gpu; + struct msm_gpu_show_priv *show_priv; + int ret;
- 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 -ENODEV; + + show_priv = kmalloc(sizeof(*show_priv), GFP_KERNEL); + if (!show_priv) + return -ENOMEM; + + ret = mutex_lock_interruptible(&dev->struct_mutex); + if (ret) + return ret; + + pm_runtime_get_sync(&gpu->pdev->dev); + show_priv->state = gpu->funcs->gpu_state_get(gpu); + pm_runtime_put_sync(&gpu->pdev->dev); + + mutex_unlock(&dev->struct_mutex); + + if (IS_ERR(show_priv->state)) { + ret = PTR_ERR(show_priv->state); + kfree(show_priv); + return ret; }
- return 0; + show_priv->dev = dev; + + return single_open(file, msm_gpu_show, show_priv); }
+static const struct file_operations msm_gpu_fops = { + .owner = THIS_MODULE, + .open = msm_gpu_open, + .read = seq_read, + .llseek = seq_lseek, + .release = msm_gpu_release, +}; + static int msm_gem_show(struct drm_device *dev, struct seq_file *m) { struct msm_drm_private *priv = dev->dev_private; @@ -105,7 +179,6 @@ static int show_locked(struct seq_file *m, void *arg) }
static struct drm_info_list msm_debugfs_list[] = { - {"gpu", show_locked, 0, msm_gpu_show}, {"gem", show_locked, 0, msm_gem_show}, { "mm", show_locked, 0, msm_mm_show }, { "fb", show_locked, 0, msm_fb_show }, @@ -158,6 +231,9 @@ int msm_debugfs_init(struct drm_minor *minor) return ret; }
+ debugfs_create_file("gpu", S_IRUSR, minor->debugfs_root, + dev, &msm_gpu_fops); + if (priv->kms->funcs->debugfs_init) { ret = priv->kms->funcs->debugfs_init(priv->kms, minor); if (ret) 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
Quoting Jordan Crouse (2018-07-12 19:59:24)
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 | 3 +- drivers/gpu/drm/msm/msm_debugfs.c | 92 ++++++++++++++++++++++--- drivers/gpu/drm/msm/msm_gpu.h | 3 +- 7 files changed, 104 insertions(+), 65 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 9e85e4f7016d..5f1aab3c1cb1 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 c7a0d278c59e..0e937eedcec5 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 734e31a9631f..90b6b59252af 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -216,7 +216,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); diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c index 1ff3fda245d1..c3da12179888 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.c +++ b/drivers/gpu/drm/msm/msm_debugfs.c @@ -16,26 +16,100 @@ */
#ifdef CONFIG_DEBUG_FS +#include <linux/debugfs.h> #include "msm_drv.h" #include "msm_gpu.h" #include "msm_kms.h" #include "msm_debugfs.h"
-static int msm_gpu_show(struct drm_device *dev, struct seq_file *m) +struct msm_gpu_show_priv {
struct msm_gpu_state *state;struct drm_device *dev;+};
+static int msm_gpu_show(struct seq_file *m, void *arg) +{
struct msm_gpu_show_priv *show_priv = m->private;struct msm_drm_private *priv = show_priv->dev->dev_private;struct msm_gpu *gpu = priv->gpu;int ret;ret = mutex_lock_interruptible(&show_priv->dev->struct_mutex);if (ret)return ret;
My immediate reaction to seeing any struct_mutex is to say, couldn't you just use rcu to protect obtaining the state kref and never touch struct_mutex? But I'm not sure how high on the scale of seething hatred for struct_mutex msm lies. -Chris
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
Quoting Jordan Crouse (2018-07-12 19:59:25)
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);
Under rcu_read_lock(), GFP_KERNEL is not allowed, you need GFP_NOWAIT or some such (or grab a reference to the pid and drop rcu then GFP_KERNEL). -Chris
On Thu, Jul 12, 2018 at 3:48 PM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Jordan Crouse (2018-07-12 19:59:25)
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);Under rcu_read_lock(), GFP_KERNEL is not allowed, you need GFP_NOWAIT or some such (or grab a reference to the pid and drop rcu then GFP_KERNEL).
I started looking at a similar issue w/ our use of kstrdup_quotable_cmdline() under rcu_read_lock().. I *guess* I hadn't noticed it before due to different RCU kconfig?
I can use GFP_ATOMIC, and I can fix kstrdup_quotable_cmdline() to actually use gfp flags passed in for kmalloc() (and similar bug in kstrdup_quotable_file()).. but get_cmdline() still grabs mmap_sem which complains under rcu_read_lock()..
is there any way to ensure the tast_struct sticks around long enough to get it's cmdline without holding rcu_read_lock()? I couldn't find any refcnt'ing on task_struct itself, which makes this seem a bit unsolveable :-/
BR, -R
On 8/4/2018 10:47 PM, Rob Clark wrote:
On Thu, Jul 12, 2018 at 3:48 PM Chris Wilson chris@chris-wilson.co.uk wrote:
Quoting Jordan Crouse (2018-07-12 19:59:25)
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);Under rcu_read_lock(), GFP_KERNEL is not allowed, you need GFP_NOWAIT or some such (or grab a reference to the pid and drop rcu then GFP_KERNEL).
I started looking at a similar issue w/ our use of kstrdup_quotable_cmdline() under rcu_read_lock().. I *guess* I hadn't noticed it before due to different RCU kconfig?
I can use GFP_ATOMIC, and I can fix kstrdup_quotable_cmdline() to actually use gfp flags passed in for kmalloc() (and similar bug in kstrdup_quotable_file()).. but get_cmdline() still grabs mmap_sem which complains under rcu_read_lock()..
is there any way to ensure the tast_struct sticks around long enough to get it's cmdline without holding rcu_read_lock()? I couldn't find any refcnt'ing on task_struct itself, which makes this seem a bit unsolveable :-/
I have been seeing similar issues on my downstream setup and was looking into fixing this actively. Here is a way to have the task stay afloat and revert to GFP_KERNEL https://patchwork.freedesktop.org/patch/256397/ Please review... I tried this out and it does work.
BR,
-R
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 | 6 +- drivers/gpu/drm/msm/msm_debugfs.c | 5 +- drivers/gpu/drm/msm/msm_gpu.c | 83 ++++++++++++++++++++++++- drivers/gpu/drm/msm/msm_gpu.h | 38 ++++++++++- 9 files changed, 154 insertions(+), 23 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 5f1aab3c1cb1..16074fa6bf1e 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 0e937eedcec5..163542487e2c 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 90b6b59252af..4a868aaf1a70 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -215,9 +215,9 @@ void adreno_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, struct msm_file_private *ctx); 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 +#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); #endif void adreno_dump_info(struct msm_gpu *gpu); void adreno_dump(struct msm_gpu *gpu); @@ -231,7 +231,7 @@ 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 c3da12179888..f0da0d3c8a80 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.c +++ b/drivers/gpu/drm/msm/msm_debugfs.c @@ -29,6 +29,7 @@ struct msm_gpu_show_priv {
static int msm_gpu_show(struct seq_file *m, void *arg) { + struct drm_printer p = drm_seq_file_printer(m); struct msm_gpu_show_priv *show_priv = m->private; struct msm_drm_private *priv = show_priv->dev->dev_private; struct msm_gpu *gpu = priv->gpu; @@ -38,8 +39,8 @@ static int msm_gpu_show(struct seq_file *m, void *arg) if (ret) return ret;
- seq_printf(m, "%s Status:\n", gpu->name); - gpu->funcs->show(gpu, show_priv->state, m); + drm_printf(&p, "%s Status:\n", gpu->name); + gpu->funcs->show(gpu, show_priv->state, &p);
mutex_unlock(&show_priv->dev->struct_mutex);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 2ca354047250..1945736fc448 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,81 @@ 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 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); + + /* 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 +432,11 @@ static void recover_worker(struct work_struct *work) msm_rd_dump_submit(priv->hangrd, submit, NULL); }
+ /* Record the crash state */ + pm_runtime_get_sync(&gpu->pdev->dev); + msm_gpu_crashstate_capture(gpu, comm, cmd); + pm_runtime_put_sync(&gpu->pdev->dev); + 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__ */
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 | 46 ++++++++++++++++++++++++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 20 ++++++----- 2 files changed, 58 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..930e4c970a62 --- /dev/null +++ b/Documentation/gpu/drm-msm-crash-dump.txt @@ -0,0 +1,46 @@ +# 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 + 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) +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. + - [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/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 163542487e2c..15fe0d029ba6 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -444,23 +444,27 @@ 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_puts(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_puts(p, "registers:\n"); + for (i = 0; i < state->nr_registers; i++) { - drm_printf(p, "IO:R %08x %08x\n", + drm_printf(p, " - { offset: 0x%04x, value: 0x%08x }\n", state->registers[i * 2] << 2, state->registers[(i * 2) + 1]); }
On Thu, Jul 12, 2018 at 12:59:27PM -0600, Jordan Crouse wrote:
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 | 46 ++++++++++++++++++++++++
.rst would be nice, plus including it some msm.rst overview file like we have for other drivers. For drivers we also tend to omit the drm prefix in the documentation. -Daniel
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 20 ++++++----- 2 files changed, 58 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..930e4c970a62 --- /dev/null +++ b/Documentation/gpu/drm-msm-crash-dump.txt @@ -0,0 +1,46 @@ +# 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- 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)+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.
- [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 crashdumperdiff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c index 163542487e2c..15fe0d029ba6 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c @@ -444,23 +444,27 @@ 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_puts(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_puts(p, "registers:\n");
- for (i = 0; i < state->nr_registers; i++) {
drm_printf(p, "IO:R %08x %08x\n",
drm_printf(p, " - { offset: 0x%04x, value: 0x%08x }\n", state->registers[i * 2] << 2, state->registers[(i * 2) + 1]);-- 2.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 930e4c970a62..7083075c6f87 100644 --- a/Documentation/gpu/drm-msm-crash-dump.txt +++ b/Documentation/gpu/drm-msm-crash-dump.txt @@ -35,6 +35,11 @@ bos: # List of buffers from the hanging submission (if known) # ascii85. Only the contents of buffers marked as # readable are dumped. Trailing zeros at the end of the # buffer won't be dumped. + 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 15fe0d029ba6..92acce377253 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_puts(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;
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 --- 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 | 236 ++++++++++++++++++++++-- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 23 +-- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 4 +- 5 files changed, 248 insertions(+), 31 deletions(-)
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 16074fa6bf1e..bd84f71d27d8 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,233 @@ 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"); + + 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, " - { offset: 0x%04x, value: 0x%08x }\n", + o << 2, a5xx_state->hlsqregs[pos]); + } + } } +#endif
static struct msm_ringbuffer *a5xx_active_ring(struct msm_gpu *gpu) { @@ -1244,14 +1454,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 92acce377253..be81fe1f6a35 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 4a868aaf1a70..4406776597fd 100644 --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h @@ -230,7 +230,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_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) */
For hangs, dump copy out the contents of the buffer objects attached to the guilty submission and print them in the crash dump report.
v2: Use %zd to print the size of the buffer correctly
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org --- Documentation/gpu/drm-msm-crash-dump.txt | 7 +++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 58 ++++++++++++++++++++---- drivers/gpu/drm/msm/msm_gpu.c | 48 ++++++++++++++++++-- drivers/gpu/drm/msm/msm_gpu.h | 9 ++++ 4 files changed, 109 insertions(+), 13 deletions(-)
diff --git a/Documentation/gpu/drm-msm-crash-dump.txt b/Documentation/gpu/drm-msm-crash-dump.txt index 7083075c6f87..b9804dd112db 100644 --- a/Documentation/gpu/drm-msm-crash-dump.txt +++ b/Documentation/gpu/drm-msm-crash-dump.txt @@ -40,6 +40,13 @@ bos: # List of buffers from the hanging submission (if known) 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 be81fe1f6a35..ce8b7af0a77d 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++) + kvfree(state->bos[i].data); + + kfree(state->bos); kfree(state->comm); kfree(state->cmd); kfree(state->registers); @@ -461,6 +465,39 @@ 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; + } + + /* Skip printing the object if it is empty */ + if (datalen == 0) + return; + + l = ascii85_encode_len(datalen); + + drm_puts(p, " data: !!ascii85 |\n"); + drm_puts(p, " "); + + for (i = 0; i < l; i++) + drm_puts(p, ascii85_encode(ptr[i], out)); + + drm_puts(p, "\n"); +} + void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, struct drm_printer *p) { @@ -487,19 +524,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_puts(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 1945736fc448..03e62f6ea2b6 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 = kvmalloc(obj->base.size, GFP_KERNEL); + if (!state_bo->data) + return; + + ptr = msm_gem_get_vaddr_active(&obj->base); + if (IS_ERR(ptr)) { + kvfree(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); + } + /* Set the active crash state to be dumped on failure */ gpu->crashstate = state;
@@ -434,7 +476,7 @@ static void recover_worker(struct work_struct *work)
/* Record the crash state */ pm_runtime_get_sync(&gpu->pdev->dev); - msm_gpu_crashstate_capture(gpu, comm, cmd); + msm_gpu_crashstate_capture(gpu, submit, comm, cmd); pm_runtime_put_sync(&gpu->pdev->dev);
kfree(cmd); 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)
dri-devel@lists.freedesktop.org
-
Berg, Johannes -
Chris Wilson -
Daniel Vetter -
Jordan Crouse -
Rob Clark -
Sharat Masetty