[PATCH] drm/bridge: tc358767: Improve DPI output pixel clock accuracy

[Neil Armstrong]

On 26/10/2024 06:11, Marek Vasut wrote:
> The Pixel PLL is not very capable and may come up with wildly inaccurate
> clock. Since DPI panels are often tolerant to slightly higher pixel clock
> without being operated outside of specification, calculate two Pixel PLL
> settings for DPI output, one for desired output pixel clock and one for
> output pixel clock with 1% increase, and then pick the result which is
> closer to the desired pixel clock and use it as the Pixel PLL settings.
> 
> For the Chefree CH101 panel with 13 MHz Xtal input clock, the frequency
> without this patch is 65 MHz which is out of the panel specification of
> 68.9..73.4 MHz, while with this patch it is 71.5 MHz which is well within
> the specification and far more accurate.
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda at intel.com>
> Cc: David Airlie <airlied at gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec at gmail.com>
> Cc: Jonas Karlman <jonas at kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Maxime Ripard <mripard at kernel.org>
> Cc: Neil Armstrong <neil.armstrong at linaro.org>
> Cc: Robert Foss <rfoss at kernel.org>
> Cc: Simona Vetter <simona at ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: dri-devel at lists.freedesktop.org
> ---
>   drivers/gpu/drm/bridge/tc358767.c | 28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 0d523322fdd8e..7e1a7322cec70 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1682,15 +1682,39 @@ static int tc_dpi_atomic_check(struct drm_bridge *bridge,
>   			       struct drm_connector_state *conn_state)
>   {
>   	struct tc_data *tc = bridge_to_tc(bridge);
> -	int adjusted_clock = 0;
> +	int adjusted_clock_0p = 0;
> +	int adjusted_clock_1p = 0;
> +	int adjusted_clock;
>   	int ret;
>   
> +	/*
> +	 * Calculate adjusted clock pixel and find out what the PLL can
> +	 * produce. The PLL is limited, so the clock might be inaccurate.
> +	 */
>   	ret = tc_pxl_pll_calc(tc, clk_get_rate(tc->refclk),
>   			      crtc_state->mode.clock * 1000,
> -			      &adjusted_clock, NULL);
> +			      &adjusted_clock_0p, NULL);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Calculate adjusted pixel clock with 1% faster requested pixel
> +	 * clock and find out what the PLL can produce. This may in fact
> +	 * be closer to the expected pixel clock of the output.
> +	 */
> +	ret = tc_pxl_pll_calc(tc, clk_get_rate(tc->refclk),
> +			      crtc_state->mode.clock * 1010,
> +			      &adjusted_clock_1p, NULL);
>   	if (ret)
>   		return ret;
>   
> +	/* Pick the more accurate of the two calculated results. */
> +	if (crtc_state->mode.clock * 1010 - adjusted_clock_1p <
> +	    crtc_state->mode.clock * 1000 - adjusted_clock_0p)
> +		adjusted_clock = adjusted_clock_1p;
> +	else
> +		adjusted_clock = adjusted_clock_0p;
> +
>   	crtc_state->adjusted_mode.clock = adjusted_clock / 1000;
>   
>   	/* DSI->DPI interface clock limitation: upto 100 MHz */

It looks like you want to go around a not very well written tc_pxl_pll_calc()

Seems the functions would either need a rewrite or perhaps use CCF instead by
declaring all the pre-div/PLL/post-div.

Neil