Re: [PATCH 2/6] drm/bridge: tc358767: Use tc_pxl_pll_calc() to correct adjusted_mode clock

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi Marek,

Am Montag, 3. Juni 2024, 23:27:34 CEST schrieb Marek Vasut:
> On 6/3/24 2:45 PM, Alexander Stein wrote:
> 
> Hi,
> 
> >> @@ -1631,6 +1643,18 @@ static int tc_edp_atomic_check(struct drm_bridge *bridge,
> >>   			       struct drm_crtc_state *crtc_state,
> >>   			       struct drm_connector_state *conn_state)
> >>   {
> >> +	struct tc_data *tc = bridge_to_tc(bridge);
> >> +	int adjusted_clock = 0;
> >> +	int ret;
> >> +
> >> +	ret = tc_pxl_pll_calc(tc, clk_get_rate(tc->refclk),
> >> +			      crtc_state->adjusted_mode.clock * 1000,
> >> +			      &adjusted_clock, NULL);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	crtc_state->adjusted_mode.clock = adjusted_clock / 1000;
> > 
> > This is prone to rounding errors. Debug output in my case:
> >> [   16.007127] tc358767 1-000f: enable video stream
> >> [   16.007148] tc358767 1-000f: PLL: requested 148500000 pixelclock, ref 26000000
> >> [   16.007163] tc358767 1-000f: PLL: got 147333333, delta -1166667
> >> [   16.007169] tc358767 1-000f: PLL: 26000000 / 1 / 1 * 17 / 3
> >> [   16.027112] tc358767 1-000f: set mode 1920x1080
> >> [   16.027138] tc358767 1-000f: H margin 148,88 sync 44
> >> [   16.027144] tc358767 1-000f: V margin 36,4 sync 5
> >> [   16.027150] tc358767 1-000f: total: 2200x1125
> >> [   16.059426] tc358767 1-000f: PLL: requested 147333000 pixelclock, ref 26000000
> >> [   16.059455] tc358767 1-000f: PLL: got 146250000, delta -1083000
> >> [   16.059461] tc358767 1-000f: PLL: 26000000 / 1 / 4 * 45 / 2
> >> [   16.095724] tc358767 1-000f: PLL: requested 146250000 pixelclock, ref 26000000
> >> [   16.095739] tc358767 1-000f: PLL: got 146250000, delta 0
> >> [   16.095745] tc358767 1-000f: PLL: 26000000 / 1 / 4 * 45 / 2
> > 
> > The accuracy degrades with each call, until a full kHz frequency is reached,
> > because drm_display_mode.clock only accounts for kHz, but the PLL
> > calculation takes Hz into account.
> 
> Hmmmmm, I need to take a closer look at this one.
> 
> Do you have any quick hints ?

No, sorry. I'm not sure about those VFIFO overruns/underruns you mentioned
in the commit message. Does this maybe only apply to DPI input?
At least for 148.5MHz (1080p) apparently it is not possible to that
exact clock anyway.

> > BTW: Which platform are you testing on?
> 
> MX8MP with LCDIFv3 -> DSIM -> TC9595 -> DP output.
> 
> The TC9595 is 2nd generation (automotive?) replacement for TC358767 (1st 
> generation replacement is TC358867) .

Same here. But fail to get output on a DP monitor if I'm running from
external refclk. Using DSICLK/4 seems necessary for some reason, but it
is very unreliable to get a proper image.
Which display are you using? Do you mind sharing your DT?

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/