| Message ID | 20180712185930.2492-8-jcrouse@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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(-)