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