Re: drm/bridge: tc358767: Fix use of unadjusted mode in the driver

[Marek Vasut <marex@denx.de>]

On 11/14/24 10:27 AM, Alexander Stein wrote:
> Hi Marek,

Hi,

> Am Samstag, 26. Oktober 2024, 06:10:42 CET schrieb Marek Vasut:
>> The driver configures mostly Pixel PLL from the clock cached in
>> local copy of the mode. Make sure the driver uses adjusted mode
>> which contains the updated Pixel PLL settings negotiated in
>> tc_dpi_atomic_check()/tc_edp_atomic_check().
>>
>> 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 | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index 7968183510e63..0d523322fdd8e 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -1764,7 +1764,7 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge,
>>   {
>>   	struct tc_data *tc = bridge_to_tc(bridge);
>>   
>> -	drm_mode_copy(&tc->mode, mode);
>> +	drm_mode_copy(&tc->mode, adj);
>>   }
>>   
>>   static const struct drm_edid *tc_edid_read(struct drm_bridge *bridge,
>>
> 
> This unfortunately breaks my DSI->DP setup on TQMa8MPxL/MBa8MPxL.
> 
> When I revert it, DP works again. I'm currently running on next-20241114
> with patches regarding DSI initialization.
> 
> before:
> tc358767 1-000f: PLL: requested 148500000 pixelclock, ref 26000000
> tc358767 1-000f: PLL: got 147333333, delta -1166667
> tc358767 1-000f: PLL: 26000000 / 1 / 1 * 17 / 3
> tc358767 1-000f: set mode 1920x1080
> tc358767 1-000f: H margin 148,88 sync 44
> tc358767 1-000f: V margin 36,4 sync 5
> tc358767 1-000f: total: 2200x1125
> 
> after:
> tc358767 1-000f: PLL: requested 147333000 pixelclock, ref 26000000
> tc358767 1-000f: PLL: got 146250000, delta -1083000
> tc358767 1-000f: PLL: 26000000 / 1 / 4 * 45 / 2
> tc358767 1-000f: set mode 1920x1080
> tc358767 1-000f: H margin 148,88 sync 44
> tc358767 1-000f: V margin 36,4 sync 5
> tc358767 1-000f: total: 2200x1125
> 
> The reason this breaks is that the adjusted clock frequency is slightly off
> to the previously calculated one: 147333333 <-> 147333000
> This is because mode clock is only specyfied in KHz. With this incorrect
> input the new result is even lower than requested, thus failing to setup
> correctly.
> 
> As I don't see a quick fix, I propose a revert meanwhile. Any comments?
Sigh, the TC358767 is really a gift that keeps on giving.

Either revert it for now, or give me a week or three until I get back to 
digging in the TC358767. Whichever you prefer. Sorry for the slowness.