[PATCH 07/13] drm/msm/gpu: Convert the GPU show function to use the GPU state
Chris Wilson
chris at chris-wilson.co.uk
Thu Jul 12 19:46:45 UTC 2018
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 at 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
More information about the dri-devel
mailing list