Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Use of sleeping functions in an atomic context path. -- commit 3156fa20e2cac505ec1966277bb361639a86a353 Author: Cristian Ciocaltea <[email protected]> drm/rockchip: vop2: Fix resource leak on vop2_enable() error path This commit modifies vop2_enable() to return an int and gracefully unwind resources such as clocks and runtime PM references if initialization steps like rockchip_drm_dma_attach_device() fail. It addresses a resource leak where previously the function returned void without disabling clocks or dropping the PM reference on error. Link: 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 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c [ ... ] > @@ -932,6 +938,17 @@ static void vop2_enable(struct vop2 *vop2) > VOP2_INT_BUS_ERRPR << 16 | VOP2_INT_BUS_ERRPR); > vop2_writel(vop2, RK3568_SYS1_INT_EN, > VOP2_INT_BUS_ERRPR << 16 | VOP2_INT_BUS_ERRPR); > + > + return 0; > + > +err_detach_dma: > + rockchip_drm_dma_detach_device(vop2->drm, vop2->dev); > +err_disable_clks: > + vop2_core_clks_disable_unprepare(vop2); > +err_put_pm: > + pm_runtime_put_sync(vop2->dev); [Severity: High] This is a pre-existing issue, but is it safe to execute a sleeping function here? The DRM subsystem guidelines indicate that CRTC atomic_enable callbacks are atomic context paths where sleeping is strictly forbidden. Since vop2_enable() is called from the vop2_crtc_atomic_enable() callback, wouldn't executing pm_runtime_put_sync() (as well as clk_prepare_enable() and pm_runtime_resume_and_get() earlier in this function) cause system panics such as scheduling while atomic or deadlocks during a display bring-up or modeset? > + > + return ret; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
