Re: [PATCH 1/2] drm/rockchip: dsi: Add maximum per lane bit rate calculation

[Heiko Stuebner <heiko@sntech.de>]

Am Dienstag, 24. März 2026, 09:58:37 Mitteleuropäische Sommerzeit schrieb Chaoyi Chen:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> Different chips have varying support for the maximum bit rate per lane.
> 
> Add calculation for the maximum per lane bit rate for various chip
> platforms, and relax the bandwidth margin requirements.
> 
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---
>  .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 3547d91b25d3..d3bacfae174e 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -268,6 +268,7 @@ struct rockchip_dw_dsi_chip_data {
>  
>  	unsigned int flags;
>  	unsigned int max_data_lanes;
> +	unsigned long max_bit_rate_per_lane;
>  };
>  
>  struct dw_mipi_dsi_rockchip {
> @@ -565,7 +566,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
>  	int bpp;
>  	unsigned long mpclk, tmp;
>  	unsigned int target_mbps = 1000;
> -	unsigned int max_mbps = dppa_map[ARRAY_SIZE(dppa_map) - 1].max_mbps;
> +	unsigned int max_mbps;
>  	unsigned long best_freq = 0;
>  	unsigned long fvco_min, fvco_max, fin, fout;
>  	unsigned int min_prediv, max_prediv;
> @@ -573,6 +574,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, const struct drm_display_mode *mode,
>  	unsigned long _fbdiv, best_fbdiv;
>  	unsigned long min_delta = ULONG_MAX;
>  
> +	max_mbps = dsi->cdata->max_bit_rate_per_lane;

I may be blind, but how do the values come together?
In the dppa_map table we have mbps values of 89 to 1500
(MHz)

While below the values set are
	.max_bit_rate_per_lane = 1500000000UL,
in Hz I guess

And I don't see the needed conversion somehow.


>  	dsi->format = format;
>  	bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>  	if (bpp < 0) {
> @@ -584,8 +586,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;

Please do this in a separate patch, especially as I would expect some
sort of explanation on why this is ok to do.

Thanks
Heiko

>  		if (tmp < max_mbps)
>  			target_mbps = tmp;
>  		else
> @@ -595,7 +597,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,
>  						 bpp, lanes,
>  						 &dsi->phy_opts.mipi_dphy);
>  		dsi->lane_mbps = target_mbps;
> @@ -1503,6 +1505,7 @@ static const struct rockchip_dw_dsi_chip_data px30_chip_data[] = {
>  						PX30_DSI_FORCETXSTOPMODE), 0),
>  
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1000000000UL,
>  	},
>  	{ /* sentinel */ }
>  };
> @@ -1515,6 +1518,7 @@ static const struct rockchip_dw_dsi_chip_data rk3128_chip_data[] = {
>  						RK3128_DSI_FORCERXMODE |
>  						RK3128_DSI_FORCETXSTOPMODE), 0),
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1000000000UL,
>  	},
>  	{ /* sentinel */ }
>  };
> @@ -1527,6 +1531,7 @@ static const struct rockchip_dw_dsi_chip_data rk3288_chip_data[] = {
>  		.lcdsel_lit = FIELD_PREP_WM16_CONST(RK3288_DSI0_LCDC_SEL, 1),
>  
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1500000000UL,
>  	},
>  	{
>  		.reg = 0xff964000,
> @@ -1535,6 +1540,7 @@ static const struct rockchip_dw_dsi_chip_data rk3288_chip_data[] = {
>  		.lcdsel_lit = FIELD_PREP_WM16_CONST(RK3288_DSI1_LCDC_SEL, 1),
>  
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1500000000UL,
>  	},
>  	{ /* sentinel */ }
>  };
> @@ -1547,6 +1553,7 @@ static const struct rockchip_dw_dsi_chip_data rk3368_chip_data[] = {
>  						RK3368_DSI_FORCETXSTOPMODE |
>  						RK3368_DSI_FORCERXMODE), 0),
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1500000000UL,
>  	},
>  	{ /* sentinel */ }
>  };
> @@ -1634,6 +1641,7 @@ static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>  
>  		.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1500000000UL,
>  	},
>  	{
>  		.reg = 0xff968000,
> @@ -1658,6 +1666,7 @@ static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>  
>  		.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1500000000UL,
>  
>  		.dphy_rx_init = rk3399_dphy_tx1rx1_init,
>  		.dphy_rx_power_on = rk3399_dphy_tx1rx1_power_on,
> @@ -1674,6 +1683,7 @@ static const struct rockchip_dw_dsi_chip_data rk3506_chip_data[] = {
>  			     FIELD_PREP_WM16_CONST(RK3506_DSI_FORCERXMODE, 0) |
>  			     FIELD_PREP_WM16_CONST(RK3506_DSI_FORCETXSTOPMODE, 0)),
>  		.max_data_lanes = 2,
> +		.max_bit_rate_per_lane = 1500000000UL,
>  	},
>  	{ /* sentinel */ }
>  };
> @@ -1687,6 +1697,7 @@ static const struct rockchip_dw_dsi_chip_data rk3568_chip_data[] = {
>  			     FIELD_PREP_WM16_CONST(RK3568_DSI0_TURNDISABLE, 0) |
>  			     FIELD_PREP_WM16_CONST(RK3568_DSI0_FORCERXMODE, 0)),
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1200000000UL,
>  	},
>  	{
>  		.reg = 0xfe070000,
> @@ -1696,6 +1707,7 @@ static const struct rockchip_dw_dsi_chip_data rk3568_chip_data[] = {
>  			     FIELD_PREP_WM16_CONST(RK3568_DSI1_TURNDISABLE, 0) |
>  			     FIELD_PREP_WM16_CONST(RK3568_DSI1_FORCERXMODE, 0)),
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1200000000UL,
>  	},
>  	{ /* sentinel */ }
>  };
> @@ -1708,6 +1720,7 @@ static const struct rockchip_dw_dsi_chip_data rv1126_chip_data[] = {
>  			     FIELD_PREP_WM16_CONST(RV1126_DSI_FORCERXMODE, 0) |
>  			     FIELD_PREP_WM16_CONST(RV1126_DSI_FORCETXSTOPMODE, 0)),
>  		.max_data_lanes = 4,
> +		.max_bit_rate_per_lane = 1000000000UL,
>  	},
>  	{ /* sentinel */ }
>  };
>