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

[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 ?

> 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) .