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: verisilicon: fill plane's vs_format in atomic_check Date: Tue, 31 Mar 2026 16:44:30 +1000 Message-ID: In-Reply-To: <20260331060126.1291966-5-zhengxingda@iscas.ac.cn> References: <20260331060126.1291966-1-zhengxingda@iscas.ac.cn> <20260331060126.1291966-5-zhengxingda@iscas.ac.cn> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Status: Good** This is the payoff patch. It: 1. Adds `struct vs_format format` to `vs_plane_state`. 2. Copies the format in `vs_plane_duplicate_state()` so it persists across = state duplication. 3. In `atomic_check`, after `drm_atomic_helper_check_plane_state()` succeed= s and confirms visibility, converts the format and stores it in the vs_plan= e_state. 4. In `atomic_update`, reads from `vs_state->format` instead of calling `dr= m_format_to_vs_format()` inline. The `drm_WARN_ON_ONCE` around the format conversion failure in `atomic_chec= k` is appropriate =E2=80=94 since the format list is controlled by the driv= er, an unsupported format reaching this point indicates a driver bug (the f= ormat should have been rejected earlier), so a warning + error return is th= e right response. The early return on `!new_plane_state->visible` before the format conversio= n is correct =E2=80=94 if the plane isn't visible, there's no framebuffer t= o convert, and `fb` could be NULL. The `memcpy` of `vs_format` in `vs_plane_duplicate_state()` could alternati= vely just assign `vs_state->format =3D vs_state_old->format` (struct assign= ment), but `memcpy` is fine and common in kernel code. --- Generated by Claude Code Patch Reviewer