diff mbox series

drm/bridge: tc358767: Improve DPI output pixel clock accuracy

Message ID 20241026041136.247671-1-marex@denx.de (mailing list archive)
State New, archived
Headers show
Series drm/bridge: tc358767: Improve DPI output pixel clock accuracy | expand

Commit Message

Marek Vasut Oct. 26, 2024, 4:11 a.m. UTC
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@denx.de>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/bridge/tc358767.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

Comments

Maxime Ripard Oct. 28, 2024, 9:26 a.m. UTC | #1
On Sat, Oct 26, 2024 at 06:11:12AM +0200, 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.

The typical tolerance we've used is .5%, which is recommended by VESA in
several specs. Differing from it for a good reason is ok I guess, but
you need to document why.

Maxime
Neil Armstrong Oct. 28, 2024, 9:45 a.m. UTC | #2
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@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@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
diff mbox series

Patch

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 */