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

[sashiko-bot@kernel.org]

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

New issues:
- [High] Integer division truncation in bandwidth calculation causes DSI link underflow when combined with the newly reduced margin.

Pre-existing issues:
- [High] 32-bit signed integer overflow in PHY pixel clock calculation for displays with high pixel clocks (> 214.7 MHz).
- [High] Unsigned long overflow on 32-bit platforms within the core PHY configuration calculation.
--

--- Patch [3]: [PATCH v3 3/3] drm/rockchip: dsi: Relax the lane rate margin requirements ---
commit 93b5a610ac5007bdda923b4d779bc2ab6fd40792
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>

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

The commit reduces the DSI lane rate margin from 25% to about 11% to
accommodate platforms limited to 1Gbps lane rates. It updates the lane rate
calculation in dw_mipi_dsi_get_lane_mbps() and adjusts the PHY configuration
parameter accordingly.

> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 557708bc4faa4..bb2b35571af73 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 = mpclk * (bpp / lanes) * 10 / 9;

[Severity: High]
Does the integer division (bpp / lanes) cause a truncation that leads to DSI
link underflow with this new reduced margin?

For example, with a panel using RGB666 (18 bpp) over 4 lanes, (18 / 4)
evaluates to 4 instead of 4.5.

With the old margin (10 / 8), the multiplier was 4 * 1.25 = 5.0, which safely
exceeded the 4.5 raw multiplier requirement.

With the new margin (10 / 9), the allocated bandwidth calculation becomes
4 * 10 / 9 = 4.44. Since 4.44 is strictly less than 4.5, the configured lane
rate provides less bandwidth than the raw pixel payload requires, which could
lead to FIFO underflows.

[ ... ]

> @@ -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(mode->clock * 1000 * 10 / 9,

[Severity: High]
This isn't a bug introduced by this patch, but does mode->clock * 1000 * 10
silently overflow 32-bit signed integers for high pixel clocks?

Because mode->clock is a signed 32-bit integer (representing kHz) and the
literals are also 32-bit signed ints, the intermediate product overflows
INT_MAX (2,147,483,647) for any pixel clock exceeding ~214.7 MHz. The result
would wrap to a negative value.

[Severity: High]
This is a pre-existing issue, but does passing this margined pixel clock
to phy_mipi_dphy_get_default_config() overflow on 32-bit platforms?

When the inflated pixel clock reaches phy_mipi_dphy_calc_config() in
drivers/phy/phy-core-mipi-dphy.c, it computes:

    hs_clk_rate = pixel_clock * bpp;

On 32-bit architectures, pixel_clock is a 32-bit unsigned long. If
mode->clock is roughly 165 MHz, pixel_clock becomes
165,000 * 1000 * 10 / 9 = 183,333,333. Multiplying by bpp (e.g., 24) gives
4,400,000,000, which silently overflows the 32-bit ULONG_MAX before the
result is assigned to the 64-bit hs_clk_rate.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810034139.148-1-kernel@airkyi.com?part=3