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: Re: [PATCH v2 4/7] drm/panfrost: Move perfcnt GPU disable sequence into a helper
Date: Fri, 05 Jun 2026 06:16:24 +1000
Message-ID: <review-patch4-20260604174723.08C381F00893@smtp.kernel.org>
In-Reply-To: <20260604174723.08C381F00893@smtp.kernel.org>
References: <20260604-claude-fixes-v2-0-57c6bd4c1655@collabora.com> <20260604174723.08C381F00893@smtp.kernel.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Verdict: Correct, straightforward refactor.**

Extracts the repeated 5-line GPU write sequence into `panfrost_perfcnt_gpu_=
disable()`. Used in three places: init, fini, and disable_locked.

**One subtle ordering change:** In `panfrost_perfcnt_disable_locked`, the o=
riginal code wrote to the enable registers first, then set the config mode =
to OFF. The new helper sets config mode OFF first:

```c
// New order in helper:
gpu_write(pfdev, GPU_PERFCNT_CFG, GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE=
_OFF));
gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0);
...
```

vs. original disable_locked order:
```c
// Old order:
gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0);
...
gpu_write(pfdev, GPU_PERFCNT_CFG, GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE=
_OFF));
```

The init path already used the "config first" order, so the helper unifies =
on that. This is likely fine =E2=80=94 turning off the mode register first =
should be safe =E2=80=94 but worth noting if the HW cares about the orderin=
g.

---

---
Generated by Claude Code Patch Reviewer