From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/v3d: Flush MMU TLB and cache during runtime resume Date: Thu, 04 Jun 2026 15:23:15 +1000 Message-ID: In-Reply-To: <20260530-v3d-fix-rpi4-freezes-v1-2-c2c8307da6ce@igalia.com> References: <20260530-v3d-fix-rpi4-freezes-v1-0-c2c8307da6ce@igalia.com> <20260530-v3d-fix-rpi4-freezes-v1-2-c2c8307da6ce@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Purpose:** `v3d_mmu_set_page_table()` called `v3d_mmu_flush_all()`, which= is gated by `pm_runtime_get_if_active()`. During runtime resume (`v3d_powe= r_resume()`), the PM status is `RPM_RESUMING`, not `RPM_ACTIVE`, so the flu= sh was silently skipped =E2=80=94 leaving stale TLB entries across power cy= cles. **Solution:** Extract the actual flush sequence into a static `v3d_mmu_flus= h_all_locked()` helper that does the writes unconditionally, and have `v3d_= mmu_set_page_table()` call it directly instead of going through the PM-gate= d wrapper. **Analysis of `v3d_mmu.c` changes:** The refactoring is clean: 1. `v3d_mmu_flush_all_locked()` (lines 45-68) contains the bare flush logic= =E2=80=94 MMUC flush + TLB clear, with error reporting. No PM reference ma= nagement. 2. `v3d_mmu_flush_all()` (lines 70-82) becomes a thin wrapper: PM guard, ca= ll locked helper, put PM reference. The `goto pm_put` is removed in favor o= f early return from the locked helper, which simplifies the control flow. 3. `v3d_mmu_set_page_table()` (line 101) now calls `v3d_mmu_flush_all_locke= d()` directly. **Caller analysis:** - `v3d_mmu_set_page_table()` is called from `v3d_power_resume()` and `v3d_r= eset()`. In both cases V3D is known reachable (resume has just re-enabled t= he clock; reset holds a PM ref). So the unconditional flush is correct in b= oth paths. - `v3d_mmu_insert_ptes()` and `v3d_mmu_remove_ptes()` still use the PM-gate= d `v3d_mmu_flush_all()`, which is correct =E2=80=94 those are called from B= O create/destroy paths where the device may or may not be active. - `v3d_irq.c:79` also uses the PM-gated wrapper, which is fine since the IR= Q handler runs while the device is active (and the PM-get confirms it). **Verdict: This is the key fix in the series. The analysis is sound and the= implementation is correct.** --- --- Generated by Claude Code Patch Reviewer