diff mbox series

[2/2] drm/v3d: client ranges from axi_ids are different with V3D 7.1

Message ID 20250409155504.1093400-3-jmcasanova@igalia.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/v3d: fix client obtained from axi_ids on V3D 4.1 | expand

Commit Message

Jose Maria Casanova Crespo April 9, 2025, 3:55 p.m. UTC
The client mask has been reduced from 8 bits on V3D 4.1 to 7 bits
on V3d 7.1, so the ranges for each client are not compatible.

A new CSD client can now report MMU errors on 7.1

Signed-off-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_irq.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Maíra Canal April 10, 2025, 12:40 p.m. UTC | #1
Hi Chema,

On 09/04/25 12:55, Jose Maria Casanova Crespo wrote:
> The client mask has been reduced from 8 bits on V3D 4.1 to 7 bits
> on V3d 7.1, so the ranges for each client are not compatible.

s/V3d/V3D

> 
> A new CSD client can now report MMU errors on 7.1

How about "On V3D 7.1, the CSD client can also report MMU errors.
Therefore, add its AXI ID to the IDs list."?

Note that a commit message should use the imperative mood:

"Describe your changes in imperative mood, e.g. “make xyzzy do frotz”
instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy to
do frotz”, as if you are giving orders to the codebase to change its
behaviour." [1]

I miss such imperative description in this commit message.

Also, you could add a "Fixes:" tag pointing to the commit that
introduced V3D 7.1. This will allow this commit to go to the stable
trees.

[1] https://docs.kernel.org/process/submitting-patches.html

> 
> Signed-off-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
> ---
>   drivers/gpu/drm/v3d/v3d_irq.c | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
> index 1810743ea7b8..0cc1c7e5b412 100644
> --- a/drivers/gpu/drm/v3d/v3d_irq.c
> +++ b/drivers/gpu/drm/v3d/v3d_irq.c
> @@ -199,12 +199,31 @@ v3d_hub_irq(int irq, void *arg)
>   			{0xA0, 0xA1, "TFU"},
>   			{0xC0, 0xE0, "MMU"},
>   			{0xE0, 0xE1, "GMP"},
> +		}, v3d71_axi_ids[] = {
> +			{0x00, 0x30, "L2T"},
> +			{0x30, 0x38, "CLE"},
> +			{0x38, 0x39, "PTB"},
> +			{0x39, 0x3A, "PSE"},
> +			{0x3A, 0x3B, "CSD"},
> +			{0x40, 0x60, "TLB"},
> +			{0x60, 0x70, "MMU"},
> +			{0x7C, 0x7E, "TFU"},
> +			{0x7F, 0x80, "GMP"},
>   		};
>   		const char *client = "?";
>   
>   		V3D_WRITE(V3D_MMU_CTL, V3D_READ(V3D_MMU_CTL));
>   
> -		if (v3d->ver >= V3D_GEN_41) {
> +		if (v3d->ver >= V3D_GEN_71) {
> +			axi_id = axi_id & 0x7F;
> +			for (size_t i = 0; i < ARRAY_SIZE(v3d71_axi_ids); i++) {
> +				if (axi_id >= v3d71_axi_ids[i].begin &&
> +				    axi_id < v3d71_axi_ids[i].end) {
> +					client = v3d71_axi_ids[i].client;
> +					break;
> +				}
> +			}

What do you think about assigning v3d71_axi_ids or v3d41_axi_ids to an 
temporary variable and move this loop below? Something like,

if (v3d->ver >= V3D_GEN_71) {
	axi_id = axi_id & 0x7F;
	v3d_axi_ids = v3d71_axi_ids;
} else if ... {
	...
}

for (size_t i = 0; i < ARRAY_SIZE(v3d_axi_ids); i++) {
	if (axi_id >= v3d_axi_ids[i].begin
	    && axi_id < v3d_axi_ids[i].end) {
		client = v3d_axi_ids[i].client;
		break;
	}
}

Best Regards,
- Maíra

> +		} else if (v3d->ver >= V3D_GEN_41) {
>   			axi_id = axi_id & 0xFF;
>   			for (size_t i = 0; i < ARRAY_SIZE(v3d41_axi_ids); i++) {
>   				if (axi_id >= v3d41_axi_ids[i].begin &&
diff mbox series

Patch

diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 1810743ea7b8..0cc1c7e5b412 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -199,12 +199,31 @@  v3d_hub_irq(int irq, void *arg)
 			{0xA0, 0xA1, "TFU"},
 			{0xC0, 0xE0, "MMU"},
 			{0xE0, 0xE1, "GMP"},
+		}, v3d71_axi_ids[] = {
+			{0x00, 0x30, "L2T"},
+			{0x30, 0x38, "CLE"},
+			{0x38, 0x39, "PTB"},
+			{0x39, 0x3A, "PSE"},
+			{0x3A, 0x3B, "CSD"},
+			{0x40, 0x60, "TLB"},
+			{0x60, 0x70, "MMU"},
+			{0x7C, 0x7E, "TFU"},
+			{0x7F, 0x80, "GMP"},
 		};
 		const char *client = "?";
 
 		V3D_WRITE(V3D_MMU_CTL, V3D_READ(V3D_MMU_CTL));
 
-		if (v3d->ver >= V3D_GEN_41) {
+		if (v3d->ver >= V3D_GEN_71) {
+			axi_id = axi_id & 0x7F;
+			for (size_t i = 0; i < ARRAY_SIZE(v3d71_axi_ids); i++) {
+				if (axi_id >= v3d71_axi_ids[i].begin &&
+				    axi_id < v3d71_axi_ids[i].end) {
+					client = v3d71_axi_ids[i].client;
+					break;
+				}
+			}
+		} else if (v3d->ver >= V3D_GEN_41) {
 			axi_id = axi_id & 0xFF;
 			for (size_t i = 0; i < ARRAY_SIZE(v3d41_axi_ids); i++) {
 				if (axi_id >= v3d41_axi_ids[i].begin &&