Re: [PATCH v4 3/3] drm/rockchip: dsi: Relax the lane rate margin requirements

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete fix: Integer overflow in external D-PHY pixel clock calculation
--

--- Patch [3]: [PATCH v4 3/3] drm/rockchip: dsi: Relax the lane rate margin requirements ---
commit 6f9a68488ce9227bbef69a3b1f33676d60dad2a4
Author: Chaoyi Chen <[email protected]>

drm/rockchip: dsi: Relax the lane rate margin requirements

This commit adjusts the DSI lane rate calculation to reduce the bandwidth
overhead from 25% to about 11%. It updates both the target_mbps calculation
and the parameter passed to phy_mipi_dphy_get_default_config().

> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index ddb3a441787e6..c4de442675b79 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
[ ... ]
> @@ -589,8 +589,8 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
>  
>  	mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
>  	if (mpclk) {
> -		/* take 1 / 0.8, since mbps must big than bandwidth of RGB */
> -		tmp = mpclk * (bpp / lanes) * 10 / 8;
> +		/* take 1 / 0.9, since mbps must big than bandwidth of RGB */
> +		tmp = DIV_ROUND_UP(mpclk * bpp * 10, lanes * 9);
>  		if (tmp < max_mbps)
>  			target_mbps = tmp;
>  		else
> @@ -600,7 +600,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
>  
>  	/* for external phy only a the mipi_dphy_config is necessary */
>  	if (dsi->phy) {
> -		phy_mipi_dphy_get_default_config(mode->clock * 1000 * 10 / 8,
> +		phy_mipi_dphy_get_default_config((u32)mode->clock * 1000 * 10 / 9,

[Severity: Medium]
Will this (u32) cast still cause an integer overflow for high pixel clocks?

If userspace requests a display mode with a pixel clock exceeding ~429.5 MHz
(e.g., 594 MHz for 4K@60Hz), the intermediate product in
dw_mipi_dsi_get_lane_mbps() will be:

((u32)594000 * 1000 * 10 = 5,940,000,000)

This overflows 32-bit unsigned arithmetic and wraps around to 1,645,032,704.

As a result, the PHY is incorrectly configured with ~182 MHz instead
of 660 MHz, leading to insufficient bandwidth and display failures.

Could we use a 64-bit cast like (u64) to fully prevent overflow during the
intermediate calculation here?

>  						 bpp, lanes,
>  						 &dsi->phy_opts.mipi_dphy);
>  		dsi->lane_mbps = target_mbps;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3