On Thu, Feb 8, 2018 at 12:31 PM, Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote: > 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 | 4 ++-- > drivers/gpu/drm/msm/msm_debugfs.c | 21 +++++++++++++++------ > drivers/gpu/drm/msm/msm_gpu.h | 3 ++- > 7 files changed, 35 insertions(+), 64 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > index c351292..4b9284a 100644 > --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > @@ -410,15 +410,6 @@ static irqreturn_t a3xx_irq(struct msm_gpu *gpu) > ~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) > { > @@ -463,7 +454,7 @@ static struct msm_gpu_state *a3xx_gpu_state_get(struct msm_gpu *gpu) > .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 faf5d60..b8dcbb1 100644 > --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c > @@ -454,16 +454,6 @@ static irqreturn_t a4xx_irq(struct msm_gpu *gpu) > ~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); > @@ -550,7 +540,7 @@ static int a4xx_get_timestamp(struct msm_gpu *gpu, uint64_t *value) > .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 08f2579..b0910bb 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 int a5xx_gpu_busy(struct msm_gpu *gpu, uint64_t *value) > .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 35b77f0..af776f1 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > @@ -437,38 +437,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 0beb078..815ae98 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > @@ -215,7 +215,8 @@ void adreno_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit, > void adreno_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring); > bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring); > #ifdef CONFIG_DEBUG_FS > -void adreno_show(struct msm_gpu *gpu, struct seq_file *m); > +void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, > + struct seq_file *m); > #endif > void adreno_dump_info(struct msm_gpu *gpu); > void adreno_dump(struct msm_gpu *gpu); > @@ -227,7 +228,6 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev, > int nr_rings); > void adreno_gpu_cleanup(struct adreno_gpu *gpu); > > - > struct msm_gpu_state *adreno_gpu_state_get(struct msm_gpu *gpu); > void adreno_gpu_state_put(struct msm_gpu_state *state); > > diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c > index ba74cb4..fd535da 100644 > --- a/drivers/gpu/drm/msm/msm_debugfs.c > +++ b/drivers/gpu/drm/msm/msm_debugfs.c > @@ -25,13 +25,22 @@ static int msm_gpu_show(struct drm_device *dev, struct seq_file *m) > { > struct msm_drm_private *priv = dev->dev_private; > struct msm_gpu *gpu = priv->gpu; > + struct msm_gpu_state *state; > > - if (gpu) { > - seq_printf(m, "%s Status:\n", gpu->name); > - pm_runtime_get_sync(&gpu->pdev->dev); > - gpu->funcs->show(gpu, m); > - pm_runtime_put_sync(&gpu->pdev->dev); > - } > + if (!gpu) > + return 0; > + > + pm_runtime_get_sync(&gpu->pdev->dev); > + state = gpu->funcs->gpu_state_get(gpu); > + pm_runtime_put_sync(&gpu->pdev->dev); > + > + if (IS_ERR(state)) > + return PTR_ERR(state); > + > + seq_printf(m, "%s Status:\n", gpu->name); > + gpu->funcs->show(gpu, state, m); > + > + gpu->funcs->gpu_state_put(state); > > return 0; > } > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h > index 4be72a6..470f3bb 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); I wonder, now that 'state msm_gpu_state' captures all the state, is there really much point for this to be a vfunc still? BR, -R > /* for generation specific debugfs: */ > int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor); > #endif > -- > 1.9.1 > > _______________________________________________ > Freedreno mailing list > Freedreno@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/freedreno -- 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