[PATCH] drm/msm/a6xx: Fix the bogus protect error on X2-85

[PATCH] drm/msm/a6xx: Fix the bogus protect error on X2-85

[Akhil P Oommen]

Update the X2-85 gpu's register protect count configuration with the
correct count_max value to avoid blocking the entire MMIO region from the
UMD.

Protect configurations are a bit complicated on A8xx. There are 2 set of
protect registers with different counts: Global and Pipe-specific. The
last-span-unbound feature is available only on the Pipe-specific protect
registers. Due to this, we cannot use the BUILD_BUG sanity check for A8x
protect configurations, so remove the A840 entry from there.

Fixes: 01ff3bf27215 ("drm/msm/a8xx: Add support for Adreno X2-85 GPU")
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
index 550a53a7865e..38561f26837e 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_catalog.c
@@ -1759,7 +1759,7 @@ static const u32 x285_protect_regs[] = {
 	A6XX_PROTECT_NORDWR(0x27c06, 0x0000),
 };
 
-DECLARE_ADRENO_PROTECT(x285_protect, 64);
+DECLARE_ADRENO_PROTECT(x285_protect, 15);
 
 static const struct adreno_reglist_pipe a840_nonctxt_regs[] = {
 	{ REG_A8XX_CP_SMMU_STREAM_ID_LPAC, 0x00000101, BIT(PIPE_NONE) },
@@ -1966,5 +1966,4 @@ static inline __always_unused void __build_asserts(void)
 	BUILD_BUG_ON(a660_protect.count > a660_protect.count_max);
 	BUILD_BUG_ON(a690_protect.count > a690_protect.count_max);
 	BUILD_BUG_ON(a730_protect.count > a730_protect.count_max);
-	BUILD_BUG_ON(a840_protect.count > a840_protect.count_max);
 }

---
base-commit: ac47870fd795549f03d57e0879fc730c79119f4b
change-id: 20260225-glymur-protect-fix-91b8b7c5e9fa

Best regards,
-- 
Akhil P Oommen <akhilpo@oss.qualcomm.com>

Re: [PATCH] drm/msm/a6xx: Fix the bogus protect error on X2-85

[Konrad Dybcio]

On 2/25/26 8:41 AM, Akhil P Oommen wrote:
> Update the X2-85 gpu's register protect count configuration with the
> correct count_max value to avoid blocking the entire MMIO region from the
> UMD.
> 
> Protect configurations are a bit complicated on A8xx. There are 2 set of
> protect registers with different counts: Global and Pipe-specific. The
> last-span-unbound feature is available only on the Pipe-specific protect
> registers. Due to this, we cannot use the BUILD_BUG sanity check for A8x
> protect configurations, so remove the A840 entry from there.
> 
> Fixes: 01ff3bf27215 ("drm/msm/a8xx: Add support for Adreno X2-85 GPU")
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Claude review: drm/msm/a6xx: Fix the bogus protect error on X2-85

[Claude Code Review Bot]

Overall Series Review

Subject: drm/msm/a6xx: Fix the bogus protect error on X2-85
Author: Akhil P Oommen <akhilpo@oss.qualcomm.com>
Patches: 2
Reviewed: 2026-02-27T13:55:25.487020

---

This is a single-patch fix for an incorrect `count_max` value in the X2-85 (Adreno) GPU protect register configuration, introduced in commit `01ff3bf27215`. The fix is correct and addresses a real hardware-visible bug where usermode drivers (UMD) were blocked from accessing the MMIO region.

The core issue is that `count_max` has **different semantics** between the a6xx and a8xx code paths. In `a6xx_set_cp_protect()`, `count_max` is the total number of protect registers, and the last entry is written to index `count_max - 1`. In `a8xx_set_cp_protect()`, `count_max` is used directly as the pipe protect register index for the "last span infinite range" entry. The x285 GPU was incorrectly given `count_max=64`, causing the infinite-range entry to be written to pipe register index 64, which is out of range (the hardware has pipe protect registers up to index 15). This meant the catch-all protection rule was never properly installed in the pipe-specific registers.

The fix is straightforward and correct, though I have minor suggestions below.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/a6xx: Fix the bogus protect error on X2-85

[Claude Code Review Bot]

Patch Review

**The `count_max` fix (correct):**

```c
-DECLARE_ADRENO_PROTECT(x285_protect, 64);
+DECLARE_ADRENO_PROTECT(x285_protect, 15);
```

This is correct. Looking at `a8xx_set_cp_protect()` in `a8xx_gpu.c:246-259`:

```c
for (i = 0; i < protect->count; i++) {
    if (protect->regs[i]) {
        gpu_write(gpu, REG_A8XX_CP_PROTECT_GLOBAL(i), protect->regs[i]);
        final_cfg = protect->regs[i];
    }
}
a8xx_write_pipe(gpu, PIPE_BR, REG_A8XX_CP_PROTECT_PIPE(protect->count_max), final_cfg);
a8xx_write_pipe(gpu, PIPE_BV, REG_A8XX_CP_PROTECT_PIPE(protect->count_max), final_cfg);
```

For a8xx, all entries go into **Global** protect registers (no count limit enforced by `count_max`), and `count_max` is used solely as the index into the **Pipe-specific** protect registers for the infinite-range entry. Setting it to 15 makes x285 consistent with the a840 configuration (`DECLARE_ADRENO_PROTECT(a840_protect, 15)` at `a6xx_catalog.c:1883`), which is the correct last pipe protect register index for this hardware generation.

**The BUILD_BUG_ON removal (necessary but deserves comment):**

```c
-	BUILD_BUG_ON(a840_protect.count > a840_protect.count_max);
```

This removal is necessary because the `count > count_max` check is meaningless for a8xx — `count` is the number of **global** protect entries (54 for a840, 52 for x285) while `count_max` is the **pipe** register index (15). These are different address spaces and there's no reason `count` must be <= `count_max`.

**Minor observations / suggestions:**

1. **Missing replacement sanity check**: The patch removes the only compile-time check for a8xx protect configurations without adding an alternative. It would be valuable to add a check that `count_max` doesn't exceed the hardware's actual pipe protect register count, or at minimum a comment explaining why the a8xx entries are intentionally excluded from the `__build_asserts()` checks.

2. **The `count_max` field is semantically overloaded**: For a6xx GPUs, `count_max` means "total number of protect registers" (used as `count_max - 1` for the last index). For a8xx GPUs, `count_max` is the direct pipe register index for the infinite-range entry. This dual meaning is confusing and contributed to the original bug. Consider renaming the field or adding a code comment in `adreno_gpu.h` near the `struct adreno_protect` definition documenting this distinction. This isn't something the patch needs to fix, but it's worth noting.

3. **Commit message nit**: The commit message says "remove the A840 entry from there" which might confuse reviewers into thinking the a840 protect configuration itself is being removed. A clearer phrasing might be: "remove the A840 BUILD_BUG_ON check, which is invalid for a8xx since count_max represents the pipe register index, not the global register count."

4. **No BUILD_BUG_ON was ever added for x285_protect**: The original commit `01ff3bf27215` added x285 with `count_max=64` but didn't add a corresponding `BUILD_BUG_ON(x285_protect.count > x285_protect.count_max)`. If it had, the bug would have been hidden (52 < 64 passes), so this is moot — but it's worth noting that even the existing `BUILD_BUG_ON` for `a840_protect` (with count=54 > count_max=15) appears to not actually fire at compile time, likely because struct member access through a `static const` variable isn't treated as a constant expression by the compiler. This means these checks may have been silently no-ops for a8xx all along.

Overall: the fix is correct and should be applied. The suggestions above are minor and could be addressed as follow-ups.

---
Generated by Claude Code Patch Reviewer