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: call atomic helper's plane state check even if no CRTC Date: Tue, 31 Mar 2026 16:44:30 +1000 Message-ID: In-Reply-To: <20260331060126.1291966-4-zhengxingda@iscas.ac.cn> References: <20260331060126.1291966-1-zhengxingda@iscas.ac.cn> <20260331060126.1291966-4-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, with note** Removes the early `return 0` when `!crtc` and instead passes `NULL` as `crt= c_state` to `drm_atomic_helper_check_plane_state()`. Looking at the helper = implementation, when `crtc` is NULL: - Line 907: `WARN_ON(plane_state->crtc && ...)` =E2=80=94 short-circuits si= nce `plane_state->crtc` is NULL, no NULL deref. - Line 912-915: If `!fb`, sets `visible =3D false` and returns 0. - Line 918-921: If `!plane_state->crtc` (and fb is set), fires `WARN_ON` an= d sets `visible =3D false`. So this is safe. The key benefit is that `plane_state->visible` and `plane_= state->src`/`dst` get properly initialized even when the plane is being dis= abled, which is important for correctness. The `Fixes` tag is appropriate. Also removes the `WARN_ON(!crtc_state)` after `drm_atomic_get_new_crtc_stat= e()` =E2=80=94 this is fine since that condition would indicate a framework= bug and the helper already has its own warnings. --- Generated by Claude Code Patch Reviewer