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@xxxxxxxxxxxxxx> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html