Re: [Freedreno] [PATCH 4/8] drm/msm/gpu: Convert the GPU show function to use the GPU state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux