Re: [PATCH v2 04/13] drm/rockchip: vop2: Avoid division by zero when computing max_dclk

[sashiko-bot@kernel.org]

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 <cristian.ciocaltea@collabora.com>

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/20260623203310.9F8C81F000E9@smtp.kernel.org/

> 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/20260706-dw-hdmi-qp-yuv-v2-0-b91beb33dedf@collabora.com?part=4