[PATCH 1/4] drm/msm: Increase gpu boost interval

[PATCH 1/4] drm/msm: Increase gpu boost interval

[Akhil P Oommen]

Currently, we boost gpu freq after 25ms of inactivity. This regresses
some of the 30 fps usecases where the workload on gpu (at 33ms internval)
is very small which it can finish at the lowest OPP before the deadline.
Lets increase this inactivity threshold to 50ms (same as the current
devfreq interval) to fix this.

Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
---

 drivers/gpu/drm/msm/msm_gpu_devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index d32b729..c4d8920 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -178,7 +178,7 @@ void msm_devfreq_active(struct msm_gpu *gpu)
 	 * interval, then we won't meet the threshold of busyness for
 	 * the governor to ramp up the freq.. so give some boost
 	 */
-	if (idle_time > msm_devfreq_profile.polling_ms/2) {
+	if (idle_time > msm_devfreq_profile.polling_ms) {
 		target_freq *= 2;
 	}
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

[PATCH 2/4] drm/msm: Fix null ptr access msm_ioctl_gem_submit()

[Akhil P Oommen]

Fix the below null pointer dereference in msm_ioctl_gem_submit():

 26545.260705:   Call trace:
 26545.263223:    kref_put+0x1c/0x60
 26545.266452:    msm_ioctl_gem_submit+0x254/0x744
 26545.270937:    drm_ioctl_kernel+0xa8/0x124
 26545.274976:    drm_ioctl+0x21c/0x33c
 26545.278478:    drm_compat_ioctl+0xdc/0xf0
 26545.282428:    __arm64_compat_sys_ioctl+0xc8/0x100
 26545.287169:    el0_svc_common+0xf8/0x250
 26545.291025:    do_el0_svc_compat+0x28/0x54
 26545.295066:    el0_svc_compat+0x10/0x1c
 26545.298838:    el0_sync_compat_handler+0xa8/0xcc
 26545.303403:    el0_sync_compat+0x188/0x1c0
 26545.307445:   Code: d503201f d503201f 52800028 4b0803e8 (b8680008)
 26545.318799:   Kernel panic - not syncing: Oops: Fatal exception

Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
---

 drivers/gpu/drm/msm/msm_gem_submit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 151d19e..bf95b81 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -780,6 +780,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 		args->nr_cmds);
 	if (IS_ERR(submit)) {
 		ret = PTR_ERR(submit);
+		submit = NULL;
 		goto out_unlock;
 	}
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

[PATCH 3/4] drm/msm/a6xx: Fix uinitialized use of gpu_scid

[Akhil P Oommen]

Avoid a possible uninitialized use of gpu_scid variable to fix the
below smatch warning:
	drivers/gpu/drm/msm/adreno/a6xx_gpu.c:1480 a6xx_llc_activate()
	error: uninitialized symbol 'gpu_scid'.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 33da25b..68ee58f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1424,17 +1424,24 @@ static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
 {
 	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
 	struct msm_gpu *gpu = &adreno_gpu->base;
-	u32 gpu_scid, cntl1_regval = 0;
+	u32 cntl1_regval = 0;
 
 	if (IS_ERR(a6xx_gpu->llc_mmio))
 		return;
 
 	if (!llcc_slice_activate(a6xx_gpu->llc_slice)) {
-		gpu_scid = llcc_get_slice_id(a6xx_gpu->llc_slice);
+		u32 gpu_scid = llcc_get_slice_id(a6xx_gpu->llc_slice);
 
 		gpu_scid &= 0x1f;
 		cntl1_regval = (gpu_scid << 0) | (gpu_scid << 5) | (gpu_scid << 10) |
 			       (gpu_scid << 15) | (gpu_scid << 20);
+
+		/* On A660, the SCID programming for UCHE traffic is done in
+		 * A6XX_GBIF_SCACHE_CNTL0[14:10]
+		 */
+		if (adreno_is_a660_family(adreno_gpu))
+			gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL0, (0x1f << 10) |
+				(1 << 8), (gpu_scid << 10) | (1 << 8));
 	}
 
 	/*
@@ -1471,13 +1478,6 @@ static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
 	}
 
 	gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL1, GENMASK(24, 0), cntl1_regval);
-
-	/* On A660, the SCID programming for UCHE traffic is done in
-	 * A6XX_GBIF_SCACHE_CNTL0[14:10]
-	 */
-	if (adreno_is_a660_family(adreno_gpu))
-		gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL0, (0x1f << 10) |
-			(1 << 8), (gpu_scid << 10) | (1 << 8));
 }
 
 static void a6xx_llc_slices_destroy(struct a6xx_gpu *a6xx_gpu)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

[PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump

[Akhil P Oommen]

Capture gmu log in coredump to enhance debugging.

Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 41 +++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/adreno_gpu.c     |  2 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.h     |  2 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index 7501849..9fa3fa6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -42,6 +42,8 @@ struct a6xx_gpu_state {
 	struct a6xx_gpu_state_obj *cx_debugbus;
 	int nr_cx_debugbus;
 
+	struct msm_gpu_state_bo *gmu_log;
+
 	struct list_head objs;
 };
 
@@ -800,6 +802,30 @@ static void a6xx_get_gmu_registers(struct msm_gpu *gpu,
 		&a6xx_state->gmu_registers[2], false);
 }
 
+static void a6xx_get_gmu_log(struct msm_gpu *gpu,
+		struct a6xx_gpu_state *a6xx_state)
+{
+	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+	struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+	struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
+	struct msm_gpu_state_bo *gmu_log;
+
+	gmu_log = state_kcalloc(a6xx_state,
+		1, sizeof(*a6xx_state->gmu_log));
+	if (!gmu_log)
+		return;
+
+	gmu_log->iova = gmu->log.iova;
+	gmu_log->size = gmu->log.size;
+	gmu_log->data = kvzalloc(gmu_log->size, GFP_KERNEL);
+	if (!gmu_log->data)
+		return;
+
+	memcpy(gmu_log->data, gmu->log.virt, gmu->log.size);
+
+	a6xx_state->gmu_log = gmu_log;
+}
+
 #define A6XX_GBIF_REGLIST_SIZE   1
 static void a6xx_get_registers(struct msm_gpu *gpu,
 		struct a6xx_gpu_state *a6xx_state,
@@ -937,6 +963,8 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu)
 
 	a6xx_get_gmu_registers(gpu, a6xx_state);
 
+	a6xx_get_gmu_log(gpu, a6xx_state);
+
 	/* If GX isn't on the rest of the data isn't going to be accessible */
 	if (!a6xx_gmu_gx_is_on(&a6xx_gpu->gmu))
 		return &a6xx_state->base;
@@ -978,6 +1006,9 @@ static void a6xx_gpu_state_destroy(struct kref *kref)
 	struct a6xx_gpu_state *a6xx_state = container_of(state,
 			struct a6xx_gpu_state, base);
 
+	if (a6xx_state->gmu_log && a6xx_state->gmu_log->data)
+		kvfree(a6xx_state->gmu_log->data);
+
 	list_for_each_entry_safe(obj, tmp, &a6xx_state->objs, node)
 		kfree(obj);
 
@@ -1191,6 +1222,16 @@ void a6xx_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 
 	adreno_show(gpu, state, p);
 
+	drm_puts(p, "gmu-log:\n");
+	if (a6xx_state->gmu_log) {
+		struct msm_gpu_state_bo *gmu_log = a6xx_state->gmu_log;
+
+		drm_printf(p, "    iova: 0x%016llx\n", gmu_log->iova);
+		drm_printf(p, "    size: %d\n", gmu_log->size);
+		adreno_show_object(p, &gmu_log->data, gmu_log->size,
+				&gmu_log->encoded);
+	}
+
 	drm_puts(p, "registers:\n");
 	for (i = 0; i < a6xx_state->nr_registers; i++) {
 		struct a6xx_gpu_state_obj *obj = &a6xx_state->registers[i];
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 7486652..7d1ff20 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -630,7 +630,7 @@ static char *adreno_gpu_ascii85_encode(u32 *src, size_t len)
 }
 
 /* len is expected to be in bytes */
-static void adreno_show_object(struct drm_printer *p, void **ptr, int len,
+void adreno_show_object(struct drm_printer *p, void **ptr, int len,
 		bool *encoded)
 {
 	if (!*ptr || !len)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 225c277..6762308 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -306,6 +306,8 @@ void adreno_gpu_state_destroy(struct msm_gpu_state *state);
 
 int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
 int adreno_gpu_state_put(struct msm_gpu_state *state);
+void adreno_show_object(struct drm_printer *p, void **ptr, int len,
+		bool *encoded);
 
 /*
  * Common helper function to initialize the default address space for arm-smmu
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump

[Rob Clark]

On Thu, Nov 18, 2021 at 2:21 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote:
>
> Capture gmu log in coredump to enhance debugging.
>
> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
> ---
>
>  drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 41 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c     |  2 +-
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h     |  2 ++
>  3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> index 7501849..9fa3fa6 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> @@ -42,6 +42,8 @@ struct a6xx_gpu_state {
>         struct a6xx_gpu_state_obj *cx_debugbus;
>         int nr_cx_debugbus;
>
> +       struct msm_gpu_state_bo *gmu_log;
> +
>         struct list_head objs;
>  };
>
> @@ -800,6 +802,30 @@ static void a6xx_get_gmu_registers(struct msm_gpu *gpu,
>                 &a6xx_state->gmu_registers[2], false);
>  }
>
> +static void a6xx_get_gmu_log(struct msm_gpu *gpu,
> +               struct a6xx_gpu_state *a6xx_state)
> +{
> +       struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> +       struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> +       struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
> +       struct msm_gpu_state_bo *gmu_log;
> +
> +       gmu_log = state_kcalloc(a6xx_state,
> +               1, sizeof(*a6xx_state->gmu_log));
> +       if (!gmu_log)
> +               return;
> +
> +       gmu_log->iova = gmu->log.iova;
> +       gmu_log->size = gmu->log.size;
> +       gmu_log->data = kvzalloc(gmu_log->size, GFP_KERNEL);
> +       if (!gmu_log->data)
> +               return;
> +
> +       memcpy(gmu_log->data, gmu->log.virt, gmu->log.size);
> +
> +       a6xx_state->gmu_log = gmu_log;
> +}
> +
>  #define A6XX_GBIF_REGLIST_SIZE   1
>  static void a6xx_get_registers(struct msm_gpu *gpu,
>                 struct a6xx_gpu_state *a6xx_state,
> @@ -937,6 +963,8 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu)
>
>         a6xx_get_gmu_registers(gpu, a6xx_state);
>
> +       a6xx_get_gmu_log(gpu, a6xx_state);
> +
>         /* If GX isn't on the rest of the data isn't going to be accessible */
>         if (!a6xx_gmu_gx_is_on(&a6xx_gpu->gmu))
>                 return &a6xx_state->base;
> @@ -978,6 +1006,9 @@ static void a6xx_gpu_state_destroy(struct kref *kref)
>         struct a6xx_gpu_state *a6xx_state = container_of(state,
>                         struct a6xx_gpu_state, base);
>
> +       if (a6xx_state->gmu_log && a6xx_state->gmu_log->data)
> +               kvfree(a6xx_state->gmu_log->data);
> +
>         list_for_each_entry_safe(obj, tmp, &a6xx_state->objs, node)
>                 kfree(obj);
>
> @@ -1191,6 +1222,16 @@ void a6xx_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
>
>         adreno_show(gpu, state, p);
>
> +       drm_puts(p, "gmu-log:\n");
> +       if (a6xx_state->gmu_log) {
> +               struct msm_gpu_state_bo *gmu_log = a6xx_state->gmu_log;
> +
> +               drm_printf(p, "    iova: 0x%016llx\n", gmu_log->iova);
> +               drm_printf(p, "    size: %d\n", gmu_log->size);

fwiw, that wants to be:

 +               drm_printf(p, "    size: %zu\n", gmu_log->size);

with that fixed, r-b

BR,
-R

> +               adreno_show_object(p, &gmu_log->data, gmu_log->size,
> +                               &gmu_log->encoded);
> +       }
> +
>         drm_puts(p, "registers:\n");
>         for (i = 0; i < a6xx_state->nr_registers; i++) {
>                 struct a6xx_gpu_state_obj *obj = &a6xx_state->registers[i];
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 7486652..7d1ff20 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -630,7 +630,7 @@ static char *adreno_gpu_ascii85_encode(u32 *src, size_t len)
>  }
>
>  /* len is expected to be in bytes */
> -static void adreno_show_object(struct drm_printer *p, void **ptr, int len,
> +void adreno_show_object(struct drm_printer *p, void **ptr, int len,
>                 bool *encoded)
>  {
>         if (!*ptr || !len)
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 225c277..6762308 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -306,6 +306,8 @@ void adreno_gpu_state_destroy(struct msm_gpu_state *state);
>
>  int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
>  int adreno_gpu_state_put(struct msm_gpu_state *state);
> +void adreno_show_object(struct drm_printer *p, void **ptr, int len,
> +               bool *encoded);
>
>  /*
>   * Common helper function to initialize the default address space for arm-smmu
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation.
>

Re: [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump

[Rob Clark]

On Mon, Nov 22, 2021 at 10:26 AM Rob Clark <robdclark@gmail.com> wrote:
>
> On Thu, Nov 18, 2021 at 2:21 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote:
> >
> > Capture gmu log in coredump to enhance debugging.
> >
> > Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
> > ---
> >
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 41 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.c     |  2 +-
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.h     |  2 ++
> >  3 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> > index 7501849..9fa3fa6 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
> > @@ -42,6 +42,8 @@ struct a6xx_gpu_state {
> >         struct a6xx_gpu_state_obj *cx_debugbus;
> >         int nr_cx_debugbus;
> >
> > +       struct msm_gpu_state_bo *gmu_log;
> > +
> >         struct list_head objs;
> >  };
> >
> > @@ -800,6 +802,30 @@ static void a6xx_get_gmu_registers(struct msm_gpu *gpu,
> >                 &a6xx_state->gmu_registers[2], false);
> >  }
> >
> > +static void a6xx_get_gmu_log(struct msm_gpu *gpu,
> > +               struct a6xx_gpu_state *a6xx_state)
> > +{
> > +       struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> > +       struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> > +       struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
> > +       struct msm_gpu_state_bo *gmu_log;
> > +
> > +       gmu_log = state_kcalloc(a6xx_state,
> > +               1, sizeof(*a6xx_state->gmu_log));
> > +       if (!gmu_log)
> > +               return;
> > +
> > +       gmu_log->iova = gmu->log.iova;
> > +       gmu_log->size = gmu->log.size;
> > +       gmu_log->data = kvzalloc(gmu_log->size, GFP_KERNEL);
> > +       if (!gmu_log->data)
> > +               return;
> > +
> > +       memcpy(gmu_log->data, gmu->log.virt, gmu->log.size);
> > +
> > +       a6xx_state->gmu_log = gmu_log;
> > +}
> > +
> >  #define A6XX_GBIF_REGLIST_SIZE   1
> >  static void a6xx_get_registers(struct msm_gpu *gpu,
> >                 struct a6xx_gpu_state *a6xx_state,
> > @@ -937,6 +963,8 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu)
> >
> >         a6xx_get_gmu_registers(gpu, a6xx_state);
> >
> > +       a6xx_get_gmu_log(gpu, a6xx_state);
> > +
> >         /* If GX isn't on the rest of the data isn't going to be accessible */
> >         if (!a6xx_gmu_gx_is_on(&a6xx_gpu->gmu))
> >                 return &a6xx_state->base;
> > @@ -978,6 +1006,9 @@ static void a6xx_gpu_state_destroy(struct kref *kref)
> >         struct a6xx_gpu_state *a6xx_state = container_of(state,
> >                         struct a6xx_gpu_state, base);
> >
> > +       if (a6xx_state->gmu_log && a6xx_state->gmu_log->data)
> > +               kvfree(a6xx_state->gmu_log->data);
> > +
> >         list_for_each_entry_safe(obj, tmp, &a6xx_state->objs, node)
> >                 kfree(obj);
> >
> > @@ -1191,6 +1222,16 @@ void a6xx_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
> >
> >         adreno_show(gpu, state, p);
> >
> > +       drm_puts(p, "gmu-log:\n");
> > +       if (a6xx_state->gmu_log) {
> > +               struct msm_gpu_state_bo *gmu_log = a6xx_state->gmu_log;
> > +
> > +               drm_printf(p, "    iova: 0x%016llx\n", gmu_log->iova);
> > +               drm_printf(p, "    size: %d\n", gmu_log->size);
>
> fwiw, that wants to be:
>
>  +               drm_printf(p, "    size: %zu\n", gmu_log->size);
>
> with that fixed, r-b

Hmm, actually, I seem to be getting an empty log.. is special gmu fw,
or non-fused device needed for this to work?

BR,
-R

> BR,
> -R
>
> > +               adreno_show_object(p, &gmu_log->data, gmu_log->size,
> > +                               &gmu_log->encoded);
> > +       }
> > +
> >         drm_puts(p, "registers:\n");
> >         for (i = 0; i < a6xx_state->nr_registers; i++) {
> >                 struct a6xx_gpu_state_obj *obj = &a6xx_state->registers[i];
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index 7486652..7d1ff20 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -630,7 +630,7 @@ static char *adreno_gpu_ascii85_encode(u32 *src, size_t len)
> >  }
> >
> >  /* len is expected to be in bytes */
> > -static void adreno_show_object(struct drm_printer *p, void **ptr, int len,
> > +void adreno_show_object(struct drm_printer *p, void **ptr, int len,
> >                 bool *encoded)
> >  {
> >         if (!*ptr || !len)
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index 225c277..6762308 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -306,6 +306,8 @@ void adreno_gpu_state_destroy(struct msm_gpu_state *state);
> >
> >  int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
> >  int adreno_gpu_state_put(struct msm_gpu_state *state);
> > +void adreno_show_object(struct drm_printer *p, void **ptr, int len,
> > +               bool *encoded);
> >
> >  /*
> >   * Common helper function to initialize the default address space for arm-smmu
> > --
> > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > of Code Aurora Forum, hosted by The Linux Foundation.
> >

Re: [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump

[Akhil P Oommen]

On 11/23/2021 12:36 AM, Rob Clark wrote:
> On Mon, Nov 22, 2021 at 10:26 AM Rob Clark <robdclark@gmail.com> wrote:
>>
>> On Thu, Nov 18, 2021 at 2:21 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote:
>>>
>>> Capture gmu log in coredump to enhance debugging.
>>>
>>> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
>>> ---
>>>
>>>   drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 41 +++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/msm/adreno/adreno_gpu.c     |  2 +-
>>>   drivers/gpu/drm/msm/adreno/adreno_gpu.h     |  2 ++
>>>   3 files changed, 44 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
>>> index 7501849..9fa3fa6 100644
>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
>>> @@ -42,6 +42,8 @@ struct a6xx_gpu_state {
>>>          struct a6xx_gpu_state_obj *cx_debugbus;
>>>          int nr_cx_debugbus;
>>>
>>> +       struct msm_gpu_state_bo *gmu_log;
>>> +
>>>          struct list_head objs;
>>>   };
>>>
>>> @@ -800,6 +802,30 @@ static void a6xx_get_gmu_registers(struct msm_gpu *gpu,
>>>                  &a6xx_state->gmu_registers[2], false);
>>>   }
>>>
>>> +static void a6xx_get_gmu_log(struct msm_gpu *gpu,
>>> +               struct a6xx_gpu_state *a6xx_state)
>>> +{
>>> +       struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
>>> +       struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
>>> +       struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
>>> +       struct msm_gpu_state_bo *gmu_log;
>>> +
>>> +       gmu_log = state_kcalloc(a6xx_state,
>>> +               1, sizeof(*a6xx_state->gmu_log));
>>> +       if (!gmu_log)
>>> +               return;
>>> +
>>> +       gmu_log->iova = gmu->log.iova;
>>> +       gmu_log->size = gmu->log.size;
>>> +       gmu_log->data = kvzalloc(gmu_log->size, GFP_KERNEL);
>>> +       if (!gmu_log->data)
>>> +               return;
>>> +
>>> +       memcpy(gmu_log->data, gmu->log.virt, gmu->log.size);
>>> +
>>> +       a6xx_state->gmu_log = gmu_log;
>>> +}
>>> +
>>>   #define A6XX_GBIF_REGLIST_SIZE   1
>>>   static void a6xx_get_registers(struct msm_gpu *gpu,
>>>                  struct a6xx_gpu_state *a6xx_state,
>>> @@ -937,6 +963,8 @@ struct msm_gpu_state *a6xx_gpu_state_get(struct msm_gpu *gpu)
>>>
>>>          a6xx_get_gmu_registers(gpu, a6xx_state);
>>>
>>> +       a6xx_get_gmu_log(gpu, a6xx_state);
>>> +
>>>          /* If GX isn't on the rest of the data isn't going to be accessible */
>>>          if (!a6xx_gmu_gx_is_on(&a6xx_gpu->gmu))
>>>                  return &a6xx_state->base;
>>> @@ -978,6 +1006,9 @@ static void a6xx_gpu_state_destroy(struct kref *kref)
>>>          struct a6xx_gpu_state *a6xx_state = container_of(state,
>>>                          struct a6xx_gpu_state, base);
>>>
>>> +       if (a6xx_state->gmu_log && a6xx_state->gmu_log->data)
>>> +               kvfree(a6xx_state->gmu_log->data);
>>> +
>>>          list_for_each_entry_safe(obj, tmp, &a6xx_state->objs, node)
>>>                  kfree(obj);
>>>
>>> @@ -1191,6 +1222,16 @@ void a6xx_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
>>>
>>>          adreno_show(gpu, state, p);
>>>
>>> +       drm_puts(p, "gmu-log:\n");
>>> +       if (a6xx_state->gmu_log) {
>>> +               struct msm_gpu_state_bo *gmu_log = a6xx_state->gmu_log;
>>> +
>>> +               drm_printf(p, "    iova: 0x%016llx\n", gmu_log->iova);
>>> +               drm_printf(p, "    size: %d\n", gmu_log->size);
>>
>> fwiw, that wants to be:
>>
>>   +               drm_printf(p, "    size: %zu\n", gmu_log->size);
>>
>> with that fixed, r-b
> 
> Hmm, actually, I seem to be getting an empty log.. is special gmu fw,
> or non-fused device needed for this to work?
> 
> BR,
> -R

No, there is no special fw. I tested this on 7c3 and it worked for me. 
a618/a630 has an old version of gmu firmware which is pretty different 
from the newer ones. Let me check.

-Akhil.

> 
>> BR,
>> -R
>>
>>> +               adreno_show_object(p, &gmu_log->data, gmu_log->size,
>>> +                               &gmu_log->encoded);
>>> +       }
>>> +
>>>          drm_puts(p, "registers:\n");
>>>          for (i = 0; i < a6xx_state->nr_registers; i++) {
>>>                  struct a6xx_gpu_state_obj *obj = &a6xx_state->registers[i];
>>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>>> index 7486652..7d1ff20 100644
>>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>>> @@ -630,7 +630,7 @@ static char *adreno_gpu_ascii85_encode(u32 *src, size_t len)
>>>   }
>>>
>>>   /* len is expected to be in bytes */
>>> -static void adreno_show_object(struct drm_printer *p, void **ptr, int len,
>>> +void adreno_show_object(struct drm_printer *p, void **ptr, int len,
>>>                  bool *encoded)
>>>   {
>>>          if (!*ptr || !len)
>>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>>> index 225c277..6762308 100644
>>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>>> @@ -306,6 +306,8 @@ void adreno_gpu_state_destroy(struct msm_gpu_state *state);
>>>
>>>   int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
>>>   int adreno_gpu_state_put(struct msm_gpu_state *state);
>>> +void adreno_show_object(struct drm_printer *p, void **ptr, int len,
>>> +               bool *encoded);
>>>
>>>   /*
>>>    * Common helper function to initialize the default address space for arm-smmu
>>> --
>>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>>> of Code Aurora Forum, hosted by The Linux Foundation.
>>>