Re: [PATCH 1/3] drm/bridge: ti-sn65dsi83: fix CHA_DSI_CLK_RANGE rounding

[Louis Chauvet]

On 2/26/26 17:16, Luca Ceresoli wrote:
> The DSI frequency must be in the range:
>
>    (CHA_DSI_CLK_RANGE * 5 MHz) <= DSI freq < ((CHA_DSI_CLK_RANGE + 1) * 5 MHz)
>
> So the register value shouldpoint to the lower range value, but
> DIV_ROUND_UP() rounds the division to the higher range value, resulting in
> an excess of 1 (unless the frequency is an exact multiple of 5 MHz).
>
> For example for a 437100000 MHz clock CHA_DSI_CLK_RANGE should be 87 (0x57):
>
>    (87 * 5 = 435) <= 437.1 < (88 * 5 = 440)
>
> but current code returns 88 (0x58).
>
> Fix the computation by removing the DIV_ROUND_UP().
>
> Fixes: ceb515ba29ba ("drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
> ---
>   drivers/gpu/drm/bridge/ti-sn65dsi83.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index f6736b4457bb..d2a81175d279 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -351,9 +351,9 @@ static u8 sn65dsi83_get_dsi_range(struct sn65dsi83 *ctx,
>   	 *  DSI_CLK = mode clock * bpp / dsi_data_lanes / 2
>   	 * the 2 is there because the bus is DDR.
>   	 */
> -	return DIV_ROUND_UP(clamp((unsigned int)mode->clock *
> -			    mipi_dsi_pixel_format_to_bpp(ctx->dsi->format) /
> -			    ctx->dsi->lanes / 2, 40000U, 500000U), 5000U);
> +	return clamp((unsigned int)mode->clock *
> +		     mipi_dsi_pixel_format_to_bpp(ctx->dsi->format) /
> +		     ctx->dsi->lanes / 2, 40000U, 500000U) / 5000U;

If you need to do a v2, I think it could be nice to introduce one or two 
intermediate variable to allow a human to read this line:

required_bitrate = pixel_clock * bpp;
lane_rate = required_bitrate / lanes / 2;
return clamp(lane_rate) / 5000;

With or without this:

Reviewed-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>

>   }
>   
>   static u8 sn65dsi83_get_dsi_div(struct sn65dsi83 *ctx)