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

Reply via email to