Re: [PATCH v6 07/11] 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:

Pre-existing issues:
- [High] DSI link rate calculations ignore DSC compression, breaking high-resolution support.
--

--- Patch [7]: [PATCH v6 07/11] drm/mediatek: mtk_dsi: Add support for MT8189 ---
commit d3eecfadd8fa60f56b5a609d3b36986202e453a5
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, which supports a
higher link rate of 2.5Gbps. It introduces a max_link_rate_mbps parameter to
the driver data and updates the mode_valid callback to use this value
instead of a hardcoded 1.5Gbps limit.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index e1803f1b9c587..95f7d94bdad3b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c

[ ... ]

> @@ -926,7 +928,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);

[Severity: High]
This is a pre-existing issue, but does this calculation break DSC-enabled
panels? When DSC is used, bit_per_pixel appears to be the uncompressed
bpp. Using the uncompressed bpp instead of the compressed bpp for the
link rate calculation will configure the hs_clk much higher than intended,
which might exceed hardware limits or cause FIFO underflows.

>  	ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);

[ ... ]

> @@ -1101,13 +1103,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)

[Severity: High]
This is a pre-existing issue, but will wanted_link_rate incorrectly reject
high-resolution compressed modes here? Since wanted_link_rate uses the
uncompressed bpp rather than the compressed bpp, it looks like high
resolution modes that require DSC to fit within the max_link_rate_mbps
will be erroneously rejected as MODE_CLOCK_HIGH.

>  		return MODE_CLOCK_HIGH;
>  
>  	if (dsi->dsc) {

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