Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider: - [High] drm/rockchip: vop2: vop2_plane_atomic_check() adds unrelated CRTC on pageflips - [High] drm/rockchip: vop2: stale splice_mode and incomplete plane conflict checks - [High] drm/rockchip: vop2: vop2_create_crtcs() binds splice_win as VP2 primary plane - [High] drm/rockchip: vop2: broken CFG_DONE latching on splice disable and modeset - [High] drm/rockchip: vop2: back-to-back rk3568_vop2_setup_layer_mixer() times out - [Medium] drm/rockchip: vop2: vop2_crtc_mode_valid() checks runtime splice_mode_right - [Medium] drm/rockchip: vop2: unaligned Esmart downscale and NV15/20/30 split rects --
commit fb88a534f1ee73c5e200206f6544bce1ff6db310 Author: Chaoyi Chen <[email protected]> drm/rockchip: vop2: Support 8K output by splicing two video ports This commit adds support for 8K display modes on RK3588 VOP2 by splicing two video ports and splitting each plane horizontally across a pair of hardware windows. It updates CRTC, plane, and overlay post-processing configuration to reserve and program the partner video port and windows. > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > index b3d6574bbd96a..acf8efe57719f 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c [ ... ] > @@ -1012,13 +1013,32 @@ static void vop2_crtc_atomic_disable(struct drm_crtc > *crtc, > struct drm_atomic_commit *state) > { > struct vop2_video_port *vp = to_vop2_video_port(crtc); > + struct rockchip_crtc_state *old_vcstate; > + struct vop2_video_port *splice_vp = NULL; > struct vop2 *vop2 = vp->vop2; > struct drm_crtc_state *old_crtc_state; > + struct drm_plane *plane; > int ret; > > vop2_lock(vop2); > > old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc); > + old_vcstate = to_rockchip_crtc_state(old_crtc_state); > + splice_vp = old_vcstate->splice_mode ? vop2_get_splice_vp(vp) : NULL; > + > + /* > + * The right windows are not tracked by the plane states, disable > + * them before the video ports are put into standby. > + */ > + if (splice_vp) { > + drm_atomic_crtc_state_for_each_plane(plane, old_crtc_state) { > + struct vop2_win *win = to_vop2_win(plane)->splice_win; > + > + if (win) > + vop2_win_disable(win); > + } > + } > + > drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false); > > drm_crtc_vblank_off(crtc); > @@ -1034,6 +1054,9 @@ static void vop2_crtc_atomic_disable(struct drm_crtc > *crtc, > > vop2_crtc_enable_irq(vp, VP_INT_DSP_HOLD_VALID); > > + if (splice_vp) > + vop2->ops->setup_splice(vp, false); > + > vop2_vp_write(vp, RK3568_VP_DSP_CTRL, RK3568_VP_DSP_CTRL__STANDBY); [ ... ] > @@ -1064,6 +1092,202 @@ static void vop2_crtc_atomic_disable(struct drm_crtc > *crtc, [ ... ] > +static int vop2_calc_splice_rects(struct drm_plane_state *pstate, > + struct drm_crtc_state *cstate, > + struct drm_rect *left_src, > + struct drm_rect *left_dst, > + struct drm_rect *right_src, > + struct drm_rect *right_dst) > +{ > + struct drm_rect *src = &pstate->src; > + struct drm_rect *dst = &pstate->dst; > + struct drm_framebuffer *fb = pstate->fb; > + int half_hdisplay = cstate->adjusted_mode.crtc_hdisplay >> 1; > + int hscale = drm_rect_calc_hscale(src, dst, 0, INT_MAX); > + int dst_w = drm_rect_width(dst); > + int src_w = drm_rect_width(src) >> 16; > + int left_dst_w, left_src_w, right_dst_w; > + > + if (hscale < 0) > + return hscale; > + > + left_dst_w = min(half_hdisplay, dst->x2) - dst->x1; > + if (left_dst_w < 0) > + left_dst_w = 0; > + right_dst_w = dst_w - left_dst_w; > + > + if (!right_dst_w) > + left_src_w = src_w; > + else > + left_src_w = (left_dst_w * hscale) >> 16; > + > + /* > + * Make sure the yrgb/uv mst of the right window are byte aligned > + * with full pixel. > + */ > + if (right_dst_w) { > + if (fb->format->format == DRM_FORMAT_NV15 || > + fb->format->format == DRM_FORMAT_NV20 || > + fb->format->format == DRM_FORMAT_NV30) > + left_src_w &= ~0x7; > + else if (fb->format->is_yuv) > + left_src_w &= ~0x1; > + } [Severity: Medium] Can vop2_calc_splice_rects() in drivers/gpu/drm/rockchip/rockchip_drm_vop2.c produce unaligned split rectangles for Esmart downscaling and NV15/NV20/NV30 formats? First, in vop2_plane_atomic_check(), Esmart windows reject downscaling when the source width is odd: if (!vop2_cluster_window(win) && src_w > dest_w && (src_w & 1)) { drm_dbg_kms(vop2->drm, "Esmart windows cannot downscale odd-width source regions\n"); return -EINVAL; } That check only inspects the unsplit plane's src_w and dest_w. When right_dst_w > 0 and !fb->format->is_yuv (RGB formats), vop2_calc_splice_rects() computes left_src_w = (left_dst_w * hscale) >> 16 without rounding it to an even width. An even total src_w downscaled on Esmart0/1 or Esmart2/3 can split into an odd left_src_w and an odd right_src_w, bypassing the check in vop2_plane_atomic_check() and programming both Esmart hardware windows to downscale odd-width source regions. Second, for DRM_FORMAT_NV15, DRM_FORMAT_NV20, and DRM_FORMAT_NV30 (10-bit packed YUV where 8 pixels occupy 10 bytes), vop2_calc_splice_rects() masks left_src_w &= ~0x7 so the right window's start offset is byte-aligned on an 8-pixel boundary. However, right_src->x1 is set to src->x1 + (left_src_w << 16), and vop2_plane_atomic_check() only checks (src->x1 >> 16) % 2. When src->x1 >> 16 is 2, 4, or 6, neither left_src->x1 >> 16 nor right_src->x1 >> 16 is a multiple of 8, causing drm_format_info_min_pitch() in vop2_win_atomic_update() to compute non-byte-aligned yrgb_mst and uv_mst DMA addresses. Should vop2_calc_splice_rects() and vop2_plane_atomic_check() enforce 8-pixel alignment on src_x for NV15/NV20/NV30 and ensure split Esmart downscale source widths are even? [ ... ] > @@ -1122,15 +1347,43 @@ static int vop2_plane_atomic_check(struct drm_plane > *plane, > return -EINVAL; > } > > - if (src_w > vop2_data->max_input.width || > - src_h > vop2_data->max_input.height) { > + max_src_w = vop2_data->max_input.width; > + if (vop2_vp_uses_splice(vp, cstate)) > + max_src_w *= 2; > + > + if (src_w > max_src_w || src_h > vop2_data->max_input.height) { > drm_dbg_kms(vop2->drm, "Invalid source: %dx%d. max input: > %dx%d\n", > - src_w, src_h, > - vop2_data->max_input.width, > - vop2_data->max_input.height); > + src_w, src_h, max_src_w, > vop2_data->max_input.height); > return -EINVAL; > } > > + if (vop2_vp_uses_splice(vp, cstate)) { > + ret = vop2_plane_splice_check(pstate, cstate); > + if (ret) > + return ret; > + } > + > + if (win->left_win) { > + struct drm_plane_state *lstate; > + struct drm_crtc_state *lcstate; > + > + lstate = drm_atomic_get_plane_state(astate, > &win->left_win->base); > + if (IS_ERR(lstate)) > + return PTR_ERR(lstate); > + > + if (lstate->crtc) { > + lcstate = drm_atomic_get_crtc_state(astate, > lstate->crtc); > + if (IS_ERR(lcstate)) > + return PTR_ERR(lcstate); > + > + if (to_rockchip_crtc_state(lcstate)->splice_mode) { > + drm_dbg_kms(vop2->drm, "%s is in use by %s in > splice mode\n", > + win->left_win->data->name, > win->data->name); > + return -EBUSY; > + } > + } > + } [Severity: High] Does vop2_plane_atomic_check() in drivers/gpu/drm/rockchip/rockchip_drm_vop2.c pull an unrelated CRTC into non-modeset pageflip commits whenever win->left_win is set? In vop2_win_init(), win->left_win is permanently populated at probe time for Cluster1-win0, Cluster3-win0, Esmart1-win0, and Esmart3-win0. When userspace submits a normal non-modeset plane update or pageflip (where astate->allow_modeset is false) using any of those planes (for example, VP1's primary plane Cluster1-win0 or VP3's primary plane Cluster3-win0, even when no CRTC is using splice mode), vop2_plane_atomic_check() unconditionally calls drm_atomic_get_plane_state(astate, &win->left_win->base) and, when lstate->crtc is non-NULL, drm_atomic_get_crtc_state(astate, lstate->crtc). When win->left_win is active on another CRTC (such as Cluster0-win0 on VP0), adding that unrelated CRTC state to a commit with allow_modeset == false triggers the kernel warning in drm_atomic_check_only(): if (affected_crtc != requested_crtc) { drm_dbg_atomic(dev, "driver added CRTC to commit: requested 0x%x, affected 0x%0x\n", requested_crtc, affected_crtc); WARN(!state->allow_modeset, "adding CRTC not allowed without modesets: requested 0x%x, affected 0x%0x\n", requested_crtc, affected_crtc); } In addition, pulling the other CRTC and plane into astate causes stall_checks() in drm_atomic_helper_setup_commit() to fail independent non-blocking pageflips with -EBUSY whenever the other CRTC has an in-flight flip, and attaches a new drm_crtc_commit to the other CRTC so that drm_atomic_helper_commit_planes() reprograms and waits for vblank on the unrelated CRTC on every flip. [ ... ] > @@ -1524,6 +1824,16 @@ static enum drm_mode_status > vop2_crtc_mode_valid(struct drm_crtc *crtc, > { > struct vop2_video_port *vp = to_vop2_video_port(crtc); > > + /* > + * For RK3588, VP0 and VP1 will be both used in splice mode. All display > + * modes of the right VP should be set as invalid when vop2 is working > in > + * splice mode. > + */ > + if (vp->splice_mode_right) { > + DRM_DEV_DEBUG(vp->vop2->dev, "vp%d is in splice mode right\n", > vp->id); > + return MODE_BAD; > + } > + > if (mode->hdisplay > vp->data->max_output.width) > return MODE_BAD_HVALUE; [Severity: Medium] Is it safe for vop2_crtc_mode_valid() in drivers/gpu/drm/rockchip/rockchip_drm_vop2.c to reject modes based on the runtime vp->splice_mode_right flag? vp->splice_mode_right is updated under vop2_lock(vop2) during commit_tail in vop2_crtc_atomic_enable() and vop2_crtc_atomic_disable(). Reading it unlocked in vop2_crtc_mode_valid() causes drm_helper_probe_single_connector_modes() to prune all modes on VP1's connector while VP0 is running in 8K splice mode. Furthermore, because drm_atomic_helper_check_modeset() invokes mode_valid() before atomic_check and before commit_tail clears vp->splice_mode_right, any atomic commit (or DRM_MODE_ATOMIC_TEST_ONLY check) that switches VP0 out of 8K splice mode and enables VP1 in the same transaction will be rejected by vop2_crtc_mode_valid() with MODE_BAD (-EINVAL). [ ... ] > @@ -1984,21 +2319,100 @@ static int vop2_crtc_atomic_check(struct drm_crtc > *crtc, > struct drm_atomic_commit *state) > { > struct vop2_video_port *vp = to_vop2_video_port(crtc); > + struct vop2 *vop2 = vp->vop2; > + const struct vop2_video_port_data *vp_data = &vop2->data->vp[vp->id]; > + struct drm_crtc_state *crtc_state = > drm_atomic_get_new_crtc_state(state, crtc); > + struct rockchip_crtc_state *vcstate = > to_rockchip_crtc_state(crtc_state); > + struct vop2_video_port *splice_vp; > struct drm_plane *plane; > int nplanes = 0; > - struct drm_crtc_state *crtc_state = > drm_atomic_get_new_crtc_state(state, crtc); > int ret; > > ret = vop2_crtc_atomic_check_gamma(vp, crtc, state, crtc_state); > if (ret) > return ret; > > + /* Check if left_vp is in use */ > + if (crtc_state->enable && drm_atomic_crtc_needs_modeset(crtc_state)) { > + struct vop2_video_port *left_vp = > vop2_find_left_splice_vp(vop2, vp->id); > + > + if (left_vp && left_vp->crtc.state) { > + struct drm_crtc_state *lstate, *lold; > + > + lstate = drm_atomic_get_crtc_state(state, > &left_vp->crtc); > + if (IS_ERR(lstate)) > + return PTR_ERR(lstate); > + > + lold = drm_atomic_get_old_crtc_state(state, > &left_vp->crtc); > + if (to_rockchip_crtc_state(lold)->splice_mode) { > + drm_dbg_kms(vop2->drm, "vp%d is in use by vp%d > in splice mode\n", > + vp->id, left_vp->id); > + return -EBUSY; > + } > + } > + } [Severity: High] Can the splice conflict checks across vop2_plane_atomic_check() and vop2_crtc_atomic_check() in drivers/gpu/drm/rockchip/rockchip_drm_vop2.c examine stale splice_mode state or miss plane conflicts? There appear to be four related issues in these checks: 1. In drm_atomic_helper_check_planes(), all plane->atomic_check() callbacks (vop2_plane_atomic_check()) run before any crtc->atomic_check() callbacks (vop2_crtc_atomic_check()). Because vcstate->splice_mode is only updated in vop2_crtc_atomic_check(), when vop2_plane_atomic_check() checks to_rockchip_crtc_state(lcstate)->splice_mode, it reads the stale splice_mode value duplicated from the previous commit rather than whether lstate->crtc uses splice mode in the new commit. 2. In vop2_crtc_atomic_check() above, lstate is fetched via drm_atomic_get_crtc_state(state, &left_vp->crtc) and left unused, while the check inspects to_rockchip_crtc_state(lold)->splice_mode on the old CRTC state. If userspace switches left_vp (VP0) from an 8K splice mode to a non-splice mode (or disables VP0) and enables vp (VP1) in the same atomic commit, both vop2_crtc_atomic_check() and vop2_plane_atomic_check() falsely reject the commit with -EBUSY. 3. In vop2_plane_atomic_check(), the early return for invisible planes: if (!pstate->visible) return 0; runs before vop2_plane_splice_check() and before the if (win->left_win) check. A plane attached to a CRTC (pstate->crtc != NULL) that is clipped off-screen (!pstate->visible) remains in crtc_state->plane_mask. During commit, rk3568_vop2_setup_overlay() in drivers/gpu/drm/rockchip/rockchip_vop2_reg.c iterates over crtc->state->plane_mask via drm_atomic_crtc_for_each_plane() and reassigns the window in RK3568_OVL_PORT_SEL, while vop2_plane_atomic_update() calls vop2_plane_atomic_disable() -> vop2_win_disable(win) on the active right splice window. 4. Unlike vop2_crtc_atomic_check() (which checks both rstate->crtc and old_rstate->crtc), vop2_plane_atomic_check() only checks if (lstate->crtc) and does not check old_lstate->crtc. If win->left_win is disabled on an active splice CRTC in the same commit that win is enabled on another CRTC, vop2_crtc_atomic_begin() on the other CRTC can migrate win's RK3568_OVL_PORT_SEL before win->left_win is disabled, and vop2_plane_atomic_disable(win->left_win) will race with vop2_plane_atomic_update(win) on the shared hardware window. [ ... ] > @@ -2018,6 +2432,7 @@ static void vop2_crtc_atomic_flush(struct drm_crtc > *crtc, > struct drm_crtc_state *old_crtc_state = > drm_atomic_get_old_crtc_state(state, crtc); > struct vop2_video_port *vp = to_vop2_video_port(crtc); > struct vop2 *vop2 = vp->vop2; > + bool splice = false; > > /* In case of modeset, gamma lut update already happened in atomic > enable */ > if (!drm_atomic_crtc_needs_modeset(crtc_state) && > crtc_state->color_mgmt_changed) > @@ -2025,7 +2440,12 @@ static void vop2_crtc_atomic_flush(struct drm_crtc > *crtc, > > vop2_post_config(crtc, false, crtc_state, old_crtc_state); > > - vop2_cfg_done(vp); > + /* When the splice_mode state changes, an update is also required */ > + if (to_rockchip_crtc_state(crtc_state)->splice_mode || > + to_rockchip_crtc_state(old_crtc_state)->splice_mode) > + splice = true; > + > + vop2_cfg_done(vp, splice); [Severity: High] What happens to RK3568_REG_CFG_DONE when disabling a spliced CRTC or modesetting it from an 8K splice mode to a non-splice mode? In vop2_crtc_atomic_disable() in drivers/gpu/drm/rockchip/rockchip_drm_vop2.c, when old_vcstate->splice_mode is true, the function disables the right windows via vop2_win_disable(win), disables the left windows via drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, false), and clears RK3568_LUT_PORT_SEL__SPLICE_EN via vop2->ops->setup_splice(vp, false) before writing RK3568_VP_DSP_CTRL__STANDBY, without calling vop2_cfg_done(). If the CRTC is being disabled (crtc_state->active == false), drm_atomic_helper_commit_planes() is called with DRM_PLANE_COMMIT_ACTIVE_ONLY, so vop2_crtc_atomic_flush() is skipped and vop2_cfg_done() is never called to latch the disabled windows. Conversely, if the CRTC is modesetting from an 8K splice mode (old_crtc_state->splice_mode == true) to a non-splice mode (crtc_state->splice_mode == false, crtc_state->active == true), vop2_crtc_atomic_disable() has already cleared RK3568_LUT_PORT_SEL__SPLICE_EN and splice_vp (VP1) remains in standby with no running dclk or vsync. When vop2_crtc_atomic_flush() then sets splice = true because to_rockchip_crtc_state(old_crtc_state)->splice_mode is true, vop2_cfg_done(vp, true) sets BIT(splice_vp_id) in RK3568_REG_CFG_DONE. Because SPLICE_EN is already 0 and splice_vp is in standby, BIT(splice_vp_id) in RK3568_REG_CFG_DONE will never clear, causing subsequent layer updates in rk3568_vop2_setup_layer_mixer() to see cfg_done & ~own_cfg_done_mask != 0 and busy-wait for 50 ms until timeout in rk3568_vop2_wait_for_layer_cfg_done(). [ ... ] > @@ -2510,6 +2930,7 @@ static int vop2_create_crtcs(struct vop2 *vop2) > struct device_node *port; > struct vop2_video_port *vp; > struct vop2_win *win; > + bool vp_used[ROCKCHIP_MAX_CRTC] = {}; > u32 possible_crtcs; > int i, j, nvp, nvps = 0; > int ret; > @@ -2543,9 +2964,23 @@ static int vop2_create_crtcs(struct vop2 *vop2) > return dev_err_probe(drm->dev, -ENOENT, > "no port node found for > video_port%d\n", i); > vp->crtc.port = port; > - nvps++; > + vp_used[i] = true; > } > > + /* reserved vp for 8K mode */ > + for (i = 0; i < vop2_data->nr_vps; i++) { > + if (!(vop2_data->feature & VOP2_FEATURE_SPLICE) || !vp_used[i]) > + continue; > + if (vop2->vps[i].data->max_output.width <= > VOP2_MAX_VP_OUTPUT_WIDTH) > + continue; > + > + vp_used[vop2->vps[i].data->splice_vp_id] = true; > + } > + > + for (i = 0; i < vop2_data->nr_vps; i++) > + if (vp_used[i]) > + nvps++; > + > nvp = 0; > /* Register a primary plane for every crtc */ > for (i = 0; i < vop2_data->nr_vps; i++) { [Severity: High] In vop2_create_crtcs() in drivers/gpu/drm/rockchip/rockchip_drm_vop2.c, what happens on RK3588 boards where VP0 and VP2 (or VP3) are enabled in device tree while VP1 is unconnected? The loop above marks vp_used[1] = true for VP1 (splice_vp_id = 1), but vp1->crtc.port remains NULL when VP1 has no DT remote node. In the primary plane registration loop immediately following: for (i = 0; i < vop2_data->nr_vps; i++) { vp = &vop2->vps[i]; if (!vp->crtc.port) continue; for (j = 0; j < vop2->registered_num_wins; j++) { win = &vop2->win[j]; VP0 (i = 0) gets Cluster0-win0 as its primary plane, VP1 (i = 1) is skipped because !vp->crtc.port, and then VP2 (i = 2) or VP3 (i = 3) is assigned the next unregistered DRM_PLANE_TYPE_PRIMARY window, which is Cluster1-win0 (Cluster0-win0's splice_win), with possible_crtcs restricted exclusively to that CRTC, while Cluster2-win0 becomes an overlay plane. Because Cluster1-win0 is the right splice partner of VP0's primary plane Cluster0-win0, won't this make 8K splice mode on VP0 and normal display output on VP2/VP3 mutually exclusive (-EBUSY) on boards with unconnected VP1? [ ... ] > diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > index c44648e1bc0d6..08f9f8cac8789 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c > +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c [ ... ] > @@ -2286,7 +2331,7 @@ static void rk3568_vop2_setup_layer_mixer(struct > vop2_video_port *vp) > if (layer_sel != vop2->old_layer_sel && atv_layer_sel != > vop2->old_layer_sel) { > cfg_done = vop2_readl(vop2, RK3568_REG_CFG_DONE); > cfg_done &= (BIT(vop2->data->nr_vps) - 1); > - cfg_done &= ~BIT(vp->id); > + cfg_done &= ~own_cfg_done_mask; > /* > * Changes of other VPs' overlays have not taken effect > */ > @@ -2295,13 +2340,13 @@ static void rk3568_vop2_setup_layer_mixer(struct > vop2_video_port *vp) > } > > if (layer_sel != vop2->old_layer_sel || port_sel != vop2->old_port_sel) > - ovl_ctrl |= FIELD_PREP(RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL, > vp->id); > + ovl_ctrl |= FIELD_PREP(RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL, > main_vp->id); > vop2_writel(vop2, RK3568_OVL_CTRL, ovl_ctrl); > > if (port_sel != vop2->old_port_sel) { > vop2->old_port_sel = port_sel; > vop2_writel(vop2, RK3568_OVL_PORT_SEL, port_sel); > - vop2_cfg_done(vp); > + vop2_cfg_done(main_vp, vcstate->splice_mode); > rk3568_vop2_wait_for_port_mux_done(vop2); > } [ ... ] > @@ -2354,33 +2399,59 @@ static void rk3568_vop2_setup_dly_for_windows(struct > vop2_video_port *vp) [ ... ] > +static void rk3568_vop2_setup_overlay(struct vop2_video_port *vp) > +{ > + struct rockchip_crtc_state *vcstate = > to_rockchip_crtc_state(vp->crtc.state); > + struct vop2_video_port *splice_vp = NULL; > + struct drm_plane *plane; > + > + vp->win_mask = 0; > + if (vcstate->splice_mode) { > + splice_vp = vop2_get_splice_vp(vp); > + splice_vp->win_mask = 0; > + } > + > + /* The right video port shares the plane state of the left one */ > + drm_atomic_crtc_for_each_plane(plane, &vp->crtc) { > + struct vop2_win *win = to_vop2_win(plane); > + > + vp->win_mask |= BIT(win->data->phys_id); > + if (splice_vp && win->splice_win) > + splice_vp->win_mask |= > BIT(win->splice_win->data->phys_id); > + } > + > + rk3568_vop2_setup_overlay_for_vp(vp); > + if (splice_vp) > + rk3568_vop2_setup_overlay_for_vp(splice_vp); > +} [Severity: High] Does calling rk3568_vop2_setup_overlay_for_vp(vp) followed immediately by rk3568_vop2_setup_overlay_for_vp(splice_vp) in rk3568_vop2_setup_overlay() in drivers/gpu/drm/rockchip/rockchip_vop2_reg.c cause a 50 ms busy-wait timeout in rk3568_vop2_setup_layer_mixer()? During vop2_crtc_atomic_begin(), the first call to rk3568_vop2_setup_layer_mixer(vp) updates vop2->old_layer_sel = layer_sel and writes the shadowed register RK3568_OVL_LAYER_SEL. Because vop2_cfg_done() is not called until vop2_crtc_atomic_flush(), reading RK3568_OVL_LAYER_SEL still returns the pre-commit active hardware value. In the immediate second call to rk3568_vop2_setup_layer_mixer(splice_vp): 1. atv_layer_sel = vop2_readl(vop2, RK3568_OVL_LAYER_SEL) still equals the old hardware value, so atv_layer_sel != vop2->old_layer_sel is true, and layer_sel != vop2->old_layer_sel is also true when splice_vp updates its right-half layers. If any other video port (such as VP2 or VP3) has a pending CFG_DONE bit in RK3568_REG_CFG_DONE, rk3568_vop2_setup_layer_mixer(splice_vp) calls rk3568_vop2_wait_for_layer_cfg_done(vop2), which busy-polls for vop2_readl(vop2, RK3568_OVL_LAYER_SEL) == vop2->old_layer_sel. Because RK3568_OVL_CTRL__LAYERSEL_REGDONE_SEL was set to main_vp->id by the first call and vop2_cfg_done(main_vp, ...) has not been called yet, vop2->old_layer_sel cannot latch during vop2_crtc_atomic_begin(), guaranteeing a 50 ms spin timeout. 2. If port_sel != vop2->old_port_sel in the second call rk3568_vop2_setup_layer_mixer(splice_vp), it calls vop2_cfg_done(main_vp, vcstate->splice_mode) and waits for vsync in rk3568_vop2_wait_for_port_mux_done(vop2), which latches the first call's left-half RK3568_OVL_LAYER_SEL a frame early before splice_vp's RK3568_OVL_LAYER_SEL and window registers have been programmed. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
