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/rockchip: vop2: Wait for layer cfg done before switching LAYERSEL_REGDONE_SEL Date: Tue, 05 May 2026 08:08:45 +1000 Message-ID: In-Reply-To: <20260504-vop2-layer-cfg-tmout-v1-2-730226a7331e@collabora.com> References: <20260504-vop2-layer-cfg-tmout-v1-0-730226a7331e@collabora.com> <20260504-vop2-layer-cfg-tmout-v1-2-730226a7331e@collabora.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 **Verdict: Good fix, correct.** This patch reorders the operations so that: 1. **First**: wait for the previous VP's layer config to latch (the wait ne= eds the old VP's vsync to fire) 2. **Then**: write RK3568_OVL_CTRL with LAYERSEL_REGDONE_SEL set to the cur= rent VP 3. **Then**: handle port_sel changes Before this patch, LAYERSEL_REGDONE_SEL was switched to the current VP *bef= ore* the wait, meaning the previous VP's vsync could no longer trigger the = shadow register latch =E2=80=94 a classic ordering bug. The moved block: ```c if (layer_sel !=3D old_layer_sel || port_sel !=3D old_port_sel) ovl_ctrl |=3D FIELD_PREP(RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL, vp->id); vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl); if (port_sel !=3D old_port_sel) { vop2_writel(vop2, RK3568_OVL_PORT_SEL, port_sel); vop2_cfg_done(vp); rk3568_vop2_wait_for_port_mux_done(vop2); } ``` now appears *after* the layer cfg done wait, which is correct. Note: the `vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl)` is unconditional (= always writes ovl_ctrl even when LAYERSEL_REGDONE_SEL is not set). This was= the same before the patch so it's not a regression =E2=80=94 but it means = we always write OVL_CTRL even in the no-change case. Not a bug, just an obs= ervation about pre-existing behavior. No issues. --- --- Generated by Claude Code Patch Reviewer