From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/msm/a8xx: Make a8xx_recover IFPC safe
Date: Tue, 28 Apr 2026 15:05:24 +1000
Message-ID: <review-patch5-20260427-gfx-clk-fixes-v2-5-797e54b3d464@oss.qualcomm.com>
In-Reply-To: <20260427-gfx-clk-fixes-v2-5-797e54b3d464@oss.qualcomm.com>
References: <20260427-gfx-clk-fixes-v2-0-797e54b3d464@oss.qualcomm.com> <20260427-gfx-clk-fixes-v2-5-797e54b3d464@oss.qualcomm.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Author:** Akhil P Oommen

```c
-	if (hang_debug)
-		a8xx_dump(gpu);
...
-	/* Halt SQE first */
-	gpu_write(gpu, REG_A8XX_CP_SQE_CNTL, 3);
+	if (adreno_gpu->funcs->gx_is_on(adreno_gpu)) {
+		/*
+		 * Sometimes crashstate capture is skipped, so SQE should be
+		 * halted here again
+		 */
+		gpu_write(gpu, REG_A8XX_CP_SQE_CNTL, 3);
+
+		if (hang_debug)
+			a8xx_dump(gpu);
+	}
```

This guards GX-domain MMIO accesses behind a `gx_is_on()` check, matching what `a6xx_recover()` already does. This is important because if GX has already collapsed via IFPC, accessing GX MMIO would be invalid.

Note the SQE halt and dump are now skipped when GX is off, which means we lose debug information in that case. The comment acknowledges this ("Sometimes crashstate capture is skipped"). This is an acceptable tradeoff since the alternative is a bus fault.

**Question for the author:** In `a6xx_recover()`, `adreno_dump_info()` is also guarded by the `gx_is_on()` check. Here it remains unguarded. Does `adreno_dump_info()` also access GX-domain MMIO on A8xx, or is it safe to call unconditionally?

No blocking issues.

---

---
Generated by Claude Code Patch Reviewer