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 <[email protected]> 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/[email protected]?part=7
