Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The function `vop2_crtc_atomic_enable` calls sleeping functions, violating the DRM subsystem guideline that CRTC atomic enable callbacks execute in atomic context. -- commit d9247f0e6928075b3e607f5959fafadc7413c070 Author: Cristian Ciocaltea <[email protected]> drm/rockchip: vop2: Avoid division by zero when computing max_dclk This commit avoids a division by zero in vop2_crtc_atomic_enable() by defaulting bpc to 8 if vcstate->output_bpc is 0, which happens for non-HDMI outputs. Closes: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > index c2328b35466de..e966c0a0266ea 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > @@ -1920,8 +1920,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc > *crtc, > * to 4K@60Hz, if available, otherwise keep using the system CRU. > */ > if (vop2->pll_hdmiphy0 || vop2->pll_hdmiphy1) { > - unsigned long max_dclk = > DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8, > - > vcstate->output_bpc); > + unsigned int bpc = vcstate->output_bpc ?: 8; > + unsigned long max_dclk = > DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8, bpc); > + > if (clock <= max_dclk) { > drm_for_each_encoder_mask(encoder, crtc->dev, > crtc_state->encoder_mask) { > struct rockchip_encoder *rkencoder = > to_rockchip_encoder(encoder); [Severity: High] This is a pre-existing issue, but does vop2_crtc_atomic_enable() safely execute in an atomic context? CRTC atomic enable callbacks can execute in an atomic context during non-blocking commits, where sleeping is forbidden. However, vop2_crtc_atomic_enable() invokes sleeping functions. It starts with vop2_lock() (which wraps mutex_lock) and clk_prepare_enable(): drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:vop2_crtc_atomic_enable() { ... vop2_lock(vop2); ret = clk_prepare_enable(vp->dclk); ... } Additionally, further down in the function, clk_set_parent() is called, which also acquires the clock framework's sleeping mutex: drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:vop2_crtc_atomic_enable() { ... if (pll_hdmiphy) { if (!vp->dclk_src) vp->dclk_src = clk_get_parent(vp->dclk); ret = clk_set_parent(vp->dclk, pll_hdmiphy); ... } Calling sleeping functions from an atomic context can cause kernel warnings, potential deadlocks, and system instability. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
