Re: [PATCH v3 06/14] drm/rockchip: vop2: Fix VOP2_MAX_DCLK_RATE overflow on 32-bit

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The driver calls multiple sleeping functions from CRTC atomic enable, disable, and flush callbacks, violating DRM atomic context rules.
--

--- Patch [6]: [PATCH v3 06/14] drm/rockchip: vop2: Fix VOP2_MAX_DCLK_RATE overflow on 32-bit ---
commit 4edf4257240708c6864fcd134b29f53d0456d685
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: vop2: Fix VOP2_MAX_DCLK_RATE overflow on 32-bit

VOP2_MAX_DCLK_RATE is defined as an unsigned long and is used scaled by
the color-depth ratio. On 32-bit architectures, this can overflow when
multiplied. This patch changes the constant to unsigned long long to ensure
64-bit arithmetic is used.

Link: 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 e9f5d015b6458..2cd04bedaa2c0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -102,7 +102,7 @@ enum vop2_afbc_format {
>  	VOP2_AFBC_FMT_INVALID = -1,
>  };
>  
> -#define VOP2_MAX_DCLK_RATE		600000000UL
> +#define VOP2_MAX_DCLK_RATE		600000000ULL

[Severity: High]
This is a pre-existing issue, but I noticed some potential atomic context
violations in this file.

During non-blocking commits, the DRM core invokes CRTC callbacks like
vop2_crtc_atomic_enable(), vop2_crtc_atomic_disable(), and
vop2_crtc_atomic_flush(). These run in an atomic context where sleeping is
strictly forbidden.

However, the driver appears to invoke sleeping functions on these paths.
For example, in vop2_crtc_atomic_enable():

    vop2_lock(vop2);
    
    ...
    
    ret = clk_prepare_enable(vp->dclk);

Similarly, in vop2_crtc_atomic_disable():

    vop2_lock(vop2);

And in the flush path via vop2_crtc_atomic_try_set_gamma_locked():

    vop2_lock(vop2);

Because vop2_lock() acquires a mutex and clk_prepare_enable() can sleep,
does this violate the atomic context requirement and potentially cause system
instability or deadlocks during display state updates?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-dw-hdmi-qp-yuv-v3-0-a4a982a9f2e7@collabora.com?part=6