Re: [PATCH 04/10] drm/msm/gpu: Convert the GPU show function to use the GPU state

2018-05-23 Thread Sharat Masetty 



On 4/18/2018 4:14 AM, Jordan Crouse 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 
---
  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   | 92 ++---
  drivers/gpu/drm/msm/msm_gpu.h   |  3 +-
  7 files changed, 104 insertions(+), 66 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 08f25798abdb..b0910bbe5190 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 b2ccaf25767c..522d47ac36e1 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

Re: [PATCH 04/10] drm/msm/gpu: Convert the GPU show function to use the GPU state

2018-04-06 Thread Chris Wilson 
Quoting Jordan Crouse (2018-04-05 23:00:50)
> diff --git a/drivers/gpu/drm/msm/msm_debugfs.c 
> b/drivers/gpu/drm/msm/msm_debugfs.c
> index ba74cb4f94df..fd535dab3d5b 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);

Ah. This be trickier than it appears thanks to how seq_file tries to
keep the interface simple :)

For a large buffer, seq_file will call the show multiple times to
convert it into a single string, which it then iterates over. (iirc)

Ideally, you grab the error state on open, and then use the
drm_printer_iterator you have to feed the chunks to seqfs. At a minimum,
I do recommend you stick the get into the seq_open callback, as my
memory says the show will be called multiple times.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel