Skip to contentSkip to conversation

drm/msm/adreno: Make adreno_gpu_state_get() return void

PATCH Superseded Sharat Masetty 3 participants 5 messages Dec 10, 2018 – Dec 11, 2018 2 messages not in the archive

Subsystems (4)
DRM drivers Arm Qualcomm mailing list DRM driver for Qualcomm display hardware DRM driver for Qualcomm Adreno GPUs

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Sharat Masetty

author
#0 lore raw

PATCH 1/2 drm/msm/adreno: Make adreno_gpu_state_get() return void

We are not really checking the state of the adreno_gpu_state_get() function at the callers and in addition the state capture is mostly a best effort service, so make the function return void.

Signed-off-by Sharat Masetty <[email protected]>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 +---
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +1 -3
index 1ca4bea..40bcf32 100644
@@ -380,7 +380,7 @@ bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
 	return false;
 }
 
-int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
+void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
 	int i, count = 0;

@@ -437,8 +437,6 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 
 		state->nr_registers = count;
 	}
-
-	return 0;
 }
 
 void adreno_gpu_state_destroy(struct msm_gpu_state *state)
drivers/gpu/drm/msm/adreno/adreno_gpu.h +1 -1
index 4973c8c..d4834b3 100644
@@ -235,7 +235,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 
 void adreno_gpu_state_destroy(struct msm_gpu_state *state);
 
-int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
+void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
 int adreno_gpu_state_put(struct msm_gpu_state *state);
 
 /* ringbuffer helpers (the parts that are adreno specific) */
Signature · 6 lines
1.9.1

_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
#1 lore raw

PATCH 2/2 drm/msm/a6xx: Fix NULL dereference during crashstate capture

The gpu crashstate's base objects registers pointer can be NULL if the target implementation decides to capture the register dump on its own. This patch simply checks for NULL before dereferencing.

Signed-off-by Sharat Masetty <[email protected]>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +10 -5
index 40bcf32..a39cebc 100644
@@ -415,6 +415,9 @@ void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 		}
 	}
 
+	if (!adreno_gpu->registers)
+		return;
+
 	/* Count the number of registers */
 	for (i = 0; adreno_gpu->registers[i] != ~0; i += 2)
 		count += adreno_gpu->registers[i + 1] -

@@ -550,12 +553,14 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 		}
 	}
 
-	drm_puts(p, "registers:\n");
+	if (state->nr_registers > 0) {
+		drm_puts(p, "registers:\n");
 
-	for (i = 0; i < state->nr_registers; i++) {
-		drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
-			state->registers[i * 2] << 2,
-			state->registers[(i * 2) + 1]);
+		for (i = 0; i < state->nr_registers; i++) {
+			drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
+					state->registers[i * 2] << 2,
+					state->registers[(i * 2) + 1]);
+		}
 	}
 }
 #endif
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

A message that is not in the archive (sent off-list or not yet received): 1544443462-28736-2-git-send-email-smasetty-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org

#3 lore raw

Re: [PATCH 2/2] drm/msm/a6xx: Fix NULL dereference during crashstate capture

Quoted 14 lines · Sharat Masetty

The gpu crashstate's base objects registers pointer can be NULL if the target implementation decides to capture the register dump on its own. This patch simply checks for NULL before dereferencing.

Signed-off-by Sharat Masetty <[email protected]>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +0 -0
index 40bcf32..a39cebc 100644

drivers/gpu/drm/msm/adreno/adreno_gpu.c void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state) · line 420

Quoted 6 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +3 -0
@@ -415,6 +415,9 @@ void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 		}
 	}
 
+	if (!adreno_gpu->registers)
+		return;
+

This looks good - we should get it in the 4.21 pull.

Quoted 10 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +2 -1
 	/* Count the number of registers */
 	for (i = 0; adreno_gpu->registers[i] != ~0; i += 2)
 		count += adreno_gpu->registers[i + 1] -

@@ -550,12 +553,14 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 		}
 	}
 
-	drm_puts(p, "registers:\n");
+	if (state->nr_registers > 0) {
+		drm_puts(p, "registers:\n");

drivers/gpu/drm/msm/adreno/adreno_gpu.c void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, · line 563

Quoted 10 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +5 -4
 
-	for (i = 0; i < state->nr_registers; i++) {
-		drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
-			state->registers[i * 2] << 2,
-			state->registers[(i * 2) + 1]);
+		for (i = 0; i < state->nr_registers; i++) {
+			drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
+					state->registers[i * 2] << 2,
+					state->registers[(i * 2) + 1]);
+		}

I don't think we need the extra indentation here - something like

for (i = 0; i < state->nr_registers; i++) {
+ if (i == 0)

+		drm_puts(p, "Registers:\n");
	drm_printf(p, " - { offset: 0x%04x, value: 0x%08x }\n",

would suffice since we won't go into the loop if state->nr_registers == 0.

Jordan

Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
#4 lore raw

Re: [PATCH 2/2] drm/msm/a6xx: Fix NULL dereference during crashstate capture

In reply to a message not in the archive
Quoted 3 lines · Sharat Masetty

The gpu crashstate's base objects registers pointer can be NULL if the target implementation decides to capture the register dump on its own. This patch simply checks for NULL before dereferencing.

Hi Sharat - this doesn't apply against msm-next - it looks like a similar fix has already been done.

Jordan

Quoted 42 lines
Signed-off-by Sharat Masetty <[email protected]>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +10 -5
index 40bcf32..a39cebc 100644
@@ -415,6 +415,9 @@ void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 		}
 	}
 
+	if (!adreno_gpu->registers)
+		return;
+
 	/* Count the number of registers */
 	for (i = 0; adreno_gpu->registers[i] != ~0; i += 2)
 		count += adreno_gpu->registers[i + 1] -

@@ -550,12 +553,14 @@ void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
 		}
 	}
 
-	drm_puts(p, "registers:\n");
+	if (state->nr_registers > 0) {
+		drm_puts(p, "registers:\n");
 
-	for (i = 0; i < state->nr_registers; i++) {
-		drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
-			state->registers[i * 2] << 2,
-			state->registers[(i * 2) + 1]);
+		for (i = 0; i < state->nr_registers; i++) {
+			drm_printf(p, "  - { offset: 0x%04x, value: 0x%08x }\n",
+					state->registers[i * 2] << 2,
+					state->registers[(i * 2) + 1]);
+		}
 	}
 }
 #endif
Signature · 1 line
1.9.1
Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno

A message that is not in the archive (sent off-list or not yet received): 1544443462-28736-1-git-send-email-smasetty-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org

Reviewed-by
#6 lore raw
Quoted 3 lines · Sharat Masetty

We are not really checking the state of the adreno_gpu_state_get() function at the callers and in addition the state capture is mostly a best effort service, so make the function return void.

Reviewed-by Jordan Crouse <[email protected]>
Quoted 44 lines
Signed-off-by Sharat Masetty <[email protected]>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 +---
 drivers/gpu/drm/msm/adreno/adreno_gpu.h | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +1 -3
index 1ca4bea..40bcf32 100644
@@ -380,7 +380,7 @@ bool adreno_idle(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
 	return false;
 }
 
-int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
+void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
 	int i, count = 0;

@@ -437,8 +437,6 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state)
 
 		state->nr_registers = count;
 	}
-
-	return 0;
 }
 
 void adreno_gpu_state_destroy(struct msm_gpu_state *state)
drivers/gpu/drm/msm/adreno/adreno_gpu.h +1 -1
index 4973c8c..d4834b3 100644
@@ -235,7 +235,7 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 
 void adreno_gpu_state_destroy(struct msm_gpu_state *state);
 
-int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
+void adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
 int adreno_gpu_state_put(struct msm_gpu_state *state);
 
 /* ringbuffer helpers (the parts that are adreno specific) */
Signature · 1 line
1.9.1
Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno