Re: [PATCH v2 05/13] 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 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Sleeping functions are called from CRTC atomic enable and disable callbacks, which execute in atomic context.
- [High] 32-bit integer overflow in MIPI D-PHY target clock calculation before 64-bit promotion.
- [High] 32-bit integer overflow in HDMI PHY link rate calculation.
--

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

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

This commit fixes a 32-bit integer overflow when calculating the maximum
DCLK for the HDMI PHY PLL by redefining VOP2_MAX_DCLK_RATE as an
unsigned long long. This ensures the value is evaluated in 64-bit
arithmetic, preventing high-resolution modes from being wrongly rejected.

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 e966c0a0266ea..0799eb0dd29fc 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
>  
>  /*
>   * bus-format types.

[Severity: High]
This is a pre-existing issue, but do vop2_crtc_atomic_enable() and
vop2_crtc_atomic_disable() call sleeping functions from atomic context?

Looking at drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:

vop2_crtc_atomic_enable() {
    ...
    vop2_lock(vop2);
    ...
}

And:

vop2_crtc_atomic_disable() {
    ...
    vop2_lock(vop2);
    ...
}

According to the DRM display hardware programming guidelines, CRTC atomic
enable and disable callbacks run in atomic context during non-blocking commits,
where sleeping is strictly forbidden.

However, vop2_crtc_atomic_enable() calls vop2_lock() (which takes a mutex),
clk_prepare_enable(), and vop2_enable(). Similarly, vop2_crtc_atomic_disable()
calls vop2_lock(), wait_for_completion_timeout(), clk_disable_unprepare(), and
vop2_disable().

Can this cause kernel warnings, system instability, and potential deadlocks?

[Severity: High]
This isn't a bug introduced by this patch, but does the MIPI D-PHY target clock
calculation in dw_mipi_dsi2_get_lane_mbps() overflow on 32-bit architectures?

Looking at drivers/gpu/drm/rockchip/dw-mipi-dsi2-rockchip.c:

dw_mipi_dsi2_get_lane_mbps() {
    ...
    target_phyclk = DIV_ROUND_CLOSEST_ULL(lane_rate_kbps * lanes * 1000, bpp);
    ...
}

The multiplication lane_rate_kbps * lanes * 1000 is evaluated in 32-bit
arithmetic before DIV_ROUND_CLOSEST_ULL() can promote it to 64-bit. For
high-resolution outputs, this could exceed the 32-bit unsigned maximum and wrap
around.

Does this result in an incorrectly configured PHY clock?

[Severity: High]
This is a pre-existing issue, but does the HDMI PHY link rate calculation in
rk_hdptx_phy_clk_set_rate() also suffer from a 32-bit integer overflow?

Looking at drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:

rk_hdptx_phy_clk_set_rate() {
    ...
    link_rate = DIV_ROUND_CLOSEST_ULL(rate * hdptx->hdmi_cfg.bpc, 8);
    ...
}

The expression rate * hdptx->hdmi_cfg.bpc is evaluated as an unsigned long.
On 32-bit architectures, this is a 32-bit operation. When rate is large (e.g.,
594,000,000 for 4K@60Hz) and bpc is 8, the product exceeds the 32-bit unsigned
limit, causing an overflow before promotion to 64-bit.

Could this yield an invalid HDMI PHY link rate and break display output on
32-bit architectures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-dw-hdmi-qp-yuv-v2-0-b91beb33dedf@collabora.com?part=5