Re: [PATCH 4/9] drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Calling sleepable functions (`mutex_lock` and CCF APIs) inside an atomic context.
--

commit daeb5d187e2f25bcf364f65898a9cf55a75558aa
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

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

This commit refactors the DCLK parent switch logic for HDMI0 and HDMI1
PHY PLLs. It consolidates duplicated clk_get_parent() and clk_set_parent()
calls into a single shared code path based on the active HDMI endpoint.

> 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
[ ... ]
> @@ -1932,42 +1932,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() here
result in a sleep inside an atomic context?

The vop2_crtc_atomic_enable() function executes as a CRTC atomic enable
callback, which runs in an atomic context where sleeping is forbidden.

Both clk_set_parent() and clk_prepare_enable() (called earlier in this
function) acquire sleepable mutexes, and vop2_lock() internally calls
mutex_lock(). Can this lead to kernel warnings or potential deadlocks
during the atomic commit phase?

> +			if (ret < 0)
> +				drm_warn(vop2->drm,
> +					 "Failed to switch DCLK to HDMI PHY PLL: %d\n",
> +					 ret);
> +		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=4