Re: [PATCH v3 06/10] drm/mediatek: mtk_dsi: Add support for MT8189

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] DSI link bandwidth and clock rate calculations ignore DSC (Display Stream Compression), incorrectly rejecting valid high-resolution modes and configuring the PHY clock out of spec.
--

--- Patch [6]: [PATCH v3 06/10] drm/mediatek: mtk_dsi: Add support for MT8189 ---
commit 89dc8fbb4f8f90dfbd3c155b69311aad3902771f
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

drm/mediatek: mtk_dsi: Add support for MT8189

This commit adds support for the DSI IP in the MT8189 SoC. It increases the
maximum link rate to 2.5Gbps and introduces a max_link_rate_mbps parameter
to replace hardcoded values. It also casts calculations to u64 to avoid
overflows with higher pixel clock rates.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 4d5cb91c788f6..dffb6befc749e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -924,7 +926,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>  	}
>  	bit_per_pixel = ret;
>  
> -	dsi->data_rate = DIV_ROUND_UP_ULL(dsi->vm.pixelclock * bit_per_pixel,
> +	dsi->data_rate = DIV_ROUND_UP_ULL((u64)dsi->vm.pixelclock * bit_per_pixel,
>  					  dsi->lanes);
>  
>  	ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);

[Severity: High]
Does this code ignore Display Stream Compression (DSC) when calculating the
PHY clock rate? The calculation uses the uncompressed bits per pixel, which
might set the DSI hardware PHY clock far higher than the actual compressed
link rate requires.

[ ... ]

> @@ -1099,13 +1101,21 @@ mtk_dsi_bridge_mode_valid(struct drm_bridge *bridge,
>  			  const struct drm_display_mode *mode)
>  {
>  	struct mtk_dsi *dsi = bridge_to_dsi(bridge);
> +	const struct mtk_dsi_driver_data *data = dsi->driver_data;
> +	u64 wanted_link_rate, max_link_rate;
>  	int bpp;
>  
>  	bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>  	if (bpp < 0)
>  		return MODE_ERROR;
>  
> -	if (mode->clock * bpp / dsi->lanes > 1500000)
> +	wanted_link_rate = mode->clock;
> +	wanted_link_rate *= bpp;
> +	max_link_rate = data->max_link_rate_mbps;
> +	max_link_rate *= dsi->lanes;
> +	max_link_rate *= KILO;
> +
> +	if (wanted_link_rate > max_link_rate)
>  		return MODE_CLOCK_HIGH;

[Severity: High]
Does this calculation also ignore DSC? 

The wanted_link_rate is calculated using the uncompressed bpp instead of the
compressed bits per pixel. This could incorrectly reject valid high-resolution
display modes if their uncompressed bandwidth exceeds the DSI link's maximum
capacity.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713130103.34654-1-angelogioacchino.delregno@collabora.com?part=6