Thread (20 messages) flat view 20 messages, 2 authors, 4d ago

Re: [PATCH v4 09/14] drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch

From: sashiko-bot@kernel.org
Date: 2026-09-02 23:01:50
Also in: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Sleeping functions clk_set_parent() and mutex_lock() called from CRTC atomic enable callback.
--

commit afbd808f729085c5fb27d8d2795c3ac8b9bd1609
Author: Cristian Ciocaltea [off-list ref]

drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch

This commit refactors the DCLK parent switch logic for HDMI0 and HDMI1
PHY PLLs into a single shared code path instead of repeating the parent
selection and clock switch for each endpoint.
quoted hunk ↗ jump to hunk
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
@@ -1964,42 +1964,34 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
[ ... ]
+		if (pll_hdmiphy) {
+			if (!vp->dclk_src)
+				vp->dclk_src = clk_get_parent(vp->dclk);
+
+			ret = clk_set_parent(vp->dclk, pll_hdmiphy);
[Severity: High]
This is a pre-existing issue, but does calling clk_set_parent() in this
function violate atomic context rules?

The vop2_crtc_atomic_enable() callback runs in an atomic context during a
display modeset, where sleeping is strictly forbidden by DRM subsystem
guidelines. 

Because clk_set_parent() might sleep, could this lead to system instability,
potential deadlocks, or kernel warnings when reconfiguring the display?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-dw-hdmi-qp-yuv-v4-0-fb45bf4147eb@collabora.com?part=9
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help