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: Rename drm_atomic_state Date: Wed, 01 Apr 2026 07:41:18 +1000 Message-ID: In-Reply-To: <20260331-drm-drm-atomic-update-v2-0-7e8fe6ddcd32@kernel.org> References: <20260331-drm-drm-atomic-update-v2-0-7e8fe6ddcd32@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: drm: Rename drm_atomic_state Author: Maxime Ripard Patches: 2 Reviewed: 2026-04-01T07:41:18.012570 --- This is a v2 of a long-discussed mechanical rename: `drm_atomic_state` =E2= =86=92 `drm_atomic_commit`, addressing a genuine source of confusion docume= nted in the DRM TODO. The rename clarifies that this struct represents a pa= rtial atomic commit (a state update for some objects), not the entire devic= e state =E2=80=94 while per-object state structs (`drm_crtc_state`, `drm_pl= ane_state`, etc.) remain correctly named as full object state. The series is 2 patches: a massive tree-wide rename (354 files, ~3400 line = changes) and a trivial todo cleanup. Only patch 2/2 is present in the mbox = =E2=80=94 patch 1/2 (the actual rename) was likely too large for the mailin= g list filter. This limits what can be reviewed in detail. **Key observations:** 1. **Naming choice is reasonable.** v1 proposed `drm_atomic_update`; v2 cha= nged to `drm_atomic_commit`, which aligns well with existing terminology (`= drm_atomic_helper_commit`, `drm_atomic_nonblocking_commit`, etc.) and with = what the struct actually represents =E2=80=94 a commit transaction. 2. **Conflict risk is very high.** Touching 354 files means this will confl= ict with virtually any in-flight DRM work. The cover letter correctly notes= this should go through drm-next directly to minimize impact, but coordinat= ion with subsystem maintainers (i915, amdgpu, msm, nouveau, vc4, etc.) will= be essential. 3. **Cannot verify completeness** of the rename without patch 1/2. The diff= stat looks thorough, covering core DRM, all major drivers, headers, tests, = and documentation. But any missed instances would cause build failures. 4. **The `drm_atomic_commit` name creates a potential naming collision** wi= th the existing function `drm_atomic_commit()` in `drm_atomic.c`. The struc= t and function would share the same base name. While C allows this (struct = tags and function names are in different namespaces), it could cause confus= ion in documentation and conversation =E2=80=94 "drm_atomic_commit" could r= efer to either the struct or the function. This should be explicitly acknow= ledged. --- Generated by Claude Code Patch Reviewer