LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/4] drm/msm: Increase gpu boost interval
@ 2021-11-18 10:20 Akhil P Oommen
  2021-11-18 10:20 ` [PATCH 2/4] drm/msm: Fix null ptr access msm_ioctl_gem_submit() Akhil P Oommen
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Akhil P Oommen @ 2021-11-18 10:20 UTC (permalink / raw)
  To: freedreno, dri-devel, linux-arm-msm, Rob Clark
  Cc: Jordan Crouse, Matthias Kaehlcke, Jonathan Marek,
	Douglas Anderson, Daniel Vetter, David Airlie, Sean Paul,
	linux-kernel

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/4] drm/msm: Fix null ptr access msm_ioctl_gem_submit()
  2021-11-18 10:20 [PATCH 1/4] drm/msm: Increase gpu boost interval Akhil P Oommen
@ 2021-11-18 10:20 ` Akhil P Oommen
  2021-11-18 10:20 ` [PATCH 3/4] drm/msm/a6xx: Fix uinitialized use of gpu_scid Akhil P Oommen
  2021-11-18 10:20 ` [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump Akhil P Oommen
  2 siblings, 0 replies; 7+ messages in thread
From: Akhil P Oommen @ 2021-11-18 10:20 UTC (permalink / raw)
  To: freedreno, dri-devel, linux-arm-msm, Rob Clark
  Cc: Jordan Crouse, Matthias Kaehlcke, Jonathan Marek,
	Douglas Anderson, Daniel Vetter, David Airlie, Sean Paul,
	linux-kernel

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/4] drm/msm/a6xx: Fix uinitialized use of gpu_scid
  2021-11-18 10:20 [PATCH 1/4] drm/msm: Increase gpu boost interval Akhil P Oommen
  2021-11-18 10:20 ` [PATCH 2/4] drm/msm: Fix null ptr access msm_ioctl_gem_submit() Akhil P Oommen
@ 2021-11-18 10:20 ` Akhil P Oommen
  2021-11-18 10:20 ` [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump Akhil P Oommen
  2 siblings, 0 replies; 7+ messages in thread
From: Akhil P Oommen @ 2021-11-18 10:20 UTC (permalink / raw)
  To: freedreno, dri-devel, linux-arm-msm, Rob Clark
  Cc: Jordan Crouse, Matthias Kaehlcke, Jonathan Marek,
	Douglas Anderson, Daniel Vetter, David Airlie, Dmitry Baryshkov,
	Eric Anholt, Sean Paul, Sharat Masetty, linux-kernel

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump
  2021-11-18 10:20 [PATCH 1/4] drm/msm: Increase gpu boost interval Akhil P Oommen
  2021-11-18 10:20 ` [PATCH 2/4] drm/msm: Fix null ptr access msm_ioctl_gem_submit() Akhil P Oommen
  2021-11-18 10:20 ` [PATCH 3/4] drm/msm/a6xx: Fix uinitialized use of gpu_scid Akhil P Oommen
@ 2021-11-18 10:20 ` Akhil P Oommen
  2021-11-22 18:26   ` Rob Clark
  2 siblings, 1 reply; 7+ messages in thread
From: Akhil P Oommen @ 2021-11-18 10:20 UTC (permalink / raw)
  To: freedreno, dri-devel, linux-arm-msm, Rob Clark
  Cc: Jordan Crouse, Matthias Kaehlcke, Jonathan Marek,
	Douglas Anderson, AngeloGioacchino Del Regno, Bjorn Andersson,
	Christian König, Daniel Vetter, David Airlie,
	Dmitry Baryshkov, Iskren Chernev, Konrad Dybcio, Lee Jones,
	Sai Prakash Ranjan, Sean Paul, Sharat Masetty, linux-kernel

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump
  2021-11-18 10:20 ` [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump Akhil P Oommen
@ 2021-11-22 18:26   ` Rob Clark
  2021-11-22 19:06     ` Rob Clark
  0 siblings, 1 reply; 7+ messages in thread
From: Rob Clark @ 2021-11-22 18:26 UTC (permalink / raw)
  To: Akhil P Oommen
  Cc: freedreno, dri-devel, linux-arm-msm, Jordan Crouse,
	Matthias Kaehlcke, Jonathan Marek, Douglas Anderson,
	AngeloGioacchino Del Regno, Bjorn Andersson,
	Christian König, Daniel Vetter, David Airlie,
	Dmitry Baryshkov, Iskren Chernev, Konrad Dybcio, Lee Jones,
	Sai Prakash Ranjan, Sean Paul, Sharat Masetty,
	Linux Kernel Mailing List

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.
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump
  2021-11-22 18:26   ` Rob Clark
@ 2021-11-22 19:06     ` Rob Clark
  2021-11-23 15:38       ` Akhil P Oommen
  0 siblings, 1 reply; 7+ messages in thread
From: Rob Clark @ 2021-11-22 19:06 UTC (permalink / raw)
  To: Akhil P Oommen
  Cc: freedreno, dri-devel, linux-arm-msm, Jordan Crouse,
	Matthias Kaehlcke, Jonathan Marek, Douglas Anderson,
	AngeloGioacchino Del Regno, Bjorn Andersson,
	Christian König, Daniel Vetter, David Airlie,
	Dmitry Baryshkov, Iskren Chernev, Konrad Dybcio, Lee Jones,
	Sai Prakash Ranjan, Sean Paul, Sharat Masetty,
	Linux Kernel Mailing List

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.
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump
  2021-11-22 19:06     ` Rob Clark
@ 2021-11-23 15:38       ` Akhil P Oommen
  0 siblings, 0 replies; 7+ messages in thread
From: Akhil P Oommen @ 2021-11-23 15:38 UTC (permalink / raw)
  To: Rob Clark
  Cc: Sean Paul, Sai Prakash Ranjan, Jonathan Marek, David Airlie,
	linux-arm-msm, Sharat Masetty, Konrad Dybcio, Douglas Anderson,
	dri-devel, Jordan Crouse, Matthias Kaehlcke, Iskren Chernev,
	AngeloGioacchino Del Regno, Dmitry Baryshkov, Lee Jones,
	Bjorn Andersson, freedreno, Christian König,
	Linux Kernel Mailing List

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.
>>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-23 15:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 10:20 [PATCH 1/4] drm/msm: Increase gpu boost interval Akhil P Oommen
2021-11-18 10:20 ` [PATCH 2/4] drm/msm: Fix null ptr access msm_ioctl_gem_submit() Akhil P Oommen
2021-11-18 10:20 ` [PATCH 3/4] drm/msm/a6xx: Fix uinitialized use of gpu_scid Akhil P Oommen
2021-11-18 10:20 ` [PATCH 4/4] drm/msm/a6xx: Capture gmu log in devcoredump Akhil P Oommen
2021-11-22 18:26   ` Rob Clark
2021-11-22 19:06     ` Rob Clark
2021-11-23 15:38       ` Akhil P Oommen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).