Claude review: drm/msm/a6xx: Limit GXPD votes to recovery in A8x

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Author:** Akhil P Oommen

This is the largest and most complex patch. It introduces `a6xx_gmu_gxpd_get()` and `a6xx_gmu_gxpd_put()` helpers that behave differently for A8x vs older generations.

**`a6xx_gmu_gxpd_get()`:**
```c
+	if (adreno_gpu->info->family >= ADRENO_8XX_GEN1)
+		return 0;
+	return pm_runtime_get_sync(gmu->gxpd);
```

For A8x, this is a no-op. For older gens, it does the existing `pm_runtime_get_sync()`.

**`a6xx_gmu_gxpd_put()`:**
```c
+	if (adreno_gpu->info->family < ADRENO_8XX_GEN1)
+		return pm_runtime_put_sync(gmu->gxpd);
+
+	if (adreno_gpu->funcs->gx_is_on(adreno_gpu)) {
+		pm_runtime_get_sync(gmu->gxpd);
+		dev_pm_genpd_synced_poweroff(gmu->gxpd);
+		pm_runtime_put_sync(gmu->gxpd);
+	}
```

For A8x during the "put" path, if GX is still on, it does a get/synced_poweroff/put sequence to force-collapse the GDSC.

**Issue 1 -- Asymmetric get/put for A8x:** On A8x, `gxpd_get()` is a no-op but `gxpd_put()` may do `pm_runtime_get_sync() + pm_runtime_put_sync()`. This is fine from a refcount perspective since the get/put pair is balanced within `gxpd_put()` itself, but the naming is misleading. The function isn't really a "put" for A8x -- it's "force-collapse GX if it's still on". Consider a comment or renaming.

**Issue 2 -- Error path in `a6xx_gmu_resume()`:** The error path at `rpm_put:` now calls:
```c
+	a6xx_gmu_gxpd_put(gmu);
```
For A8x, if GX happens to be on during this error path, `gxpd_put()` will do a `pm_runtime_get_sync()` + `dev_pm_genpd_synced_poweroff()` + `pm_runtime_put_sync()`. This seems overly aggressive for a probe/resume error path. In the original code, this error path called `pm_runtime_put(gmu->gxpd)` which just dropped the refcount obtained earlier. Since `gxpd_get()` is now a no-op on A8x, the error path should also be a no-op -- the force-collapse logic is only appropriate for the `a6xx_gmu_stop()` shutdown path. This could cause unexpected behavior on the error path.

**Issue 3 -- Return value of `a6xx_gmu_gxpd_get()`:** On pre-A8x, `pm_runtime_get_sync()` can return a positive value on success or a negative error. The existing code at the call site in `a6xx_gmu_resume()` doesn't check the return value (it didn't before either), so this is a pre-existing issue, not introduced by this patch.

**Issue 4 -- Removed blank line:** There's a removed blank line between the end of `gdsc_gx_do_nothing_enable` export and the new functions:
```c
 }
 EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
+
+static int a6xx_gmu_gxpd_get(...)
```
Wait, that's in gdsc.c (patch 1). In patch 6, there's also a stray blank line removal:
```c
-
 
 int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
```
This looks like an extra blank line was removed, which is fine.

**Issue 5 -- Stop path change:** In `a6xx_gmu_stop()`, the original code had:
```c
	if (!IS_ERR_OR_NULL(gmu->gxpd))
		pm_runtime_put_sync(gmu->gxpd);
```
The new `a6xx_gmu_gxpd_put()` already checks `IS_ERR_OR_NULL(gmu->gxpd)`, so this is functionally equivalent for pre-A8x. For A8x, the stop path now does the force-collapse dance, which is the intended behavior during recovery (since `a6xx_gmu_stop()` is called from the recovery path when `gmu->hung` is true).

Overall the design intent is sound, but the error-path behavior in resume deserves closer attention to ensure `gxpd_put()` on A8x doesn't do the force-collapse when it shouldn't.

---
Generated by Claude Code Patch Reviewer