Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Sleeping functions called from atomic commit tail path. Pre-existing issues: - [High] Preexisting sleeping functions called from CRTC atomic disable callback. -- commit ba765a9b53922b23dd96c53d16b5f0f7d8aa8c99 Author: Igor Paunovic <[email protected]> drm/rockchip: vop2: Scale the AXI clock to the bandwidth the mode needs This patch dynamically scales the VOP2 AXI clock on RK3588 based on the bandwidth requirements of the video mode. It tracks the clock requirement via a global atomic state object and updates the clock in the atomic commit tail path. Link: https://lore.kernel.org/all/[email protected]/ Link: https://lore.kernel.org/all/20767137.geO5KgaWL5@diego/ [1] > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > index b2f3a579bbe52..cf574e48f9561 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c [ ... ] > +static void vop2_commit_tail_begin(struct rockchip_drm_commit_hooks *hooks, > + struct drm_atomic_commit *state) > +{ > + struct vop2 *vop2 = vop2_from_commit_hooks(hooks); > + struct drm_private_state *old_priv_state, *new_priv_state; > + struct vop2_aclk_state *old_aclk_state; > + unsigned int i; > + > + old_priv_state = drm_atomic_get_old_private_obj_state(state, > &vop2->aclk_obj); > + new_priv_state = drm_atomic_get_new_private_obj_state(state, > &vop2->aclk_obj); > + if (!old_priv_state || !new_priv_state) > + return; > + > + old_aclk_state = to_vop2_aclk_state(old_priv_state); > + > + for (i = 0; i < ARRAY_SIZE(old_aclk_state->pending_commit); i++) { > + struct drm_crtc_commit *commit = > old_aclk_state->pending_commit[i]; > + > + if (!commit) > + continue; > + > + if (drm_crtc_commit_wait(commit)) > + drm_err(vop2->drm, "timed out waiting for the commit on > vp%u\n", i); [Severity: High] Does calling drm_crtc_commit_wait() here cause a kernel warning or potential deadlock? According to the DRM Subsystem Details, drm_atomic_helper_commit_tail() and its variants execute in atomic context where sleeping is forbidden. Since drm_crtc_commit_wait() uses wait_for_completion_timeout() internally, it can sleep and may lead to issues when called from the atomic commit tail path. > + > + drm_crtc_commit_put(commit); > + old_aclk_state->pending_commit[i] = NULL; > + } > + > + vop2_set_aclk_rate(vop2, > + max(vop2_aclk_rate(vop2, old_aclk_state), > + vop2_aclk_rate(vop2, > to_vop2_aclk_state(new_priv_state)))); > +} > + > +/* > + * Every port has moved over: settle at the rate the new configuration needs. > + * Runs before drm_atomic_helper_commit_hw_done(), so the next commit, which > + * waits for hw_done, never sees this write land after its own. > + */ > +static void vop2_commit_tail_end(struct rockchip_drm_commit_hooks *hooks, > + struct drm_atomic_commit *state) > +{ > + struct vop2 *vop2 = vop2_from_commit_hooks(hooks); > + struct drm_private_state *priv_state; > + > + priv_state = drm_atomic_get_new_private_obj_state(state, > &vop2->aclk_obj); > + if (!priv_state) > + return; > + > + vop2_set_aclk_rate(vop2, vop2_aclk_rate(vop2, > to_vop2_aclk_state(priv_state))); [Severity: High] Can this call to vop2_set_aclk_rate() sleep while in an atomic context? This path eventually calls clk_set_rate() which takes a mutex and is a sleeping function. Just like with drm_crtc_commit_wait() above, invoking this from the atomic commit tail could lead to warnings or deadlocks. > +} > + > static void vop2_crtc_atomic_disable(struct drm_crtc *crtc, > struct drm_atomic_commit *state) > { [Severity: High] This is a pre-existing issue, but it appears that the implementation of vop2_crtc_atomic_disable() calls sleeping functions from an atomic context. Specifically, it calls vop2_lock() which acquires a mutex, and wait_for_completion_timeout() for vp->dsp_hold_completion. The subsystem guidelines state that CRTC atomic disable callbacks run in atomic context where sleeping is forbidden. This might also be contributing to kernel warnings or system instability. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
