[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <027aac3abff3f84a0ebf461653ed6c9b@vosn.de>
Date: Mon, 09 Dec 2024 10:27:26 +0100
From: Nikolaus Voss <nv@...n.de>
To: Marek Vasut <marex@...x.de>
Cc: Alexander Stein <alexander.stein@...tq-group.com>, Liu Ying
<victor.liu@....com>, Luca Ceresoli <luca.ceresoli@...tlin.com>, Fabio
Estevam <festevam@...x.de>, Andrzej Hajda <andrzej.hajda@...el.com>, Neil
Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman
<jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, David Airlie
<airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
miquel.raynal@...tlin.com, nikolaus.voss@...g-streit.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm: bridge: fsl-ldb: fixup mode on freq mismatch
On 07.12.2024 12:46, Marek Vasut wrote:
> On 12/4/24 11:40 AM, Nikolaus Voss wrote:
>>>> LDB clock has to be a fixed multiple of the pixel clock.
>>>> As LDB and pixel clock are derived from different clock sources
>>>
>>> Can you please share the content of /sys/kernel/debug/clk/clk_summary
>>> ?
>>
>> Sure. Without my patch:
>>
>> video_pll1_ref_sel 1 1 0
>> 24000000 0 0 50000 Y deviceless
>> no_connection_id
>> video_pll1 1 1 0 1039500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> video_pll1_bypass 1 1 0 1039500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> video_pll1_out 2 2 0 1039500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> media_ldb 1 1 0
>> 346500000 0 0 50000 Y 32ec0000.blk-
>> ctrl:bridge@5c ldb
>> deviceless
>> no_connection_id
>> media_ldb_root_clk 0 0 0 346500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> media_disp2_pix 1 1 0
>> 51975000 0 0 50000 Y deviceless
>> no_connection_id
>> media_disp2_pix_root_clk 1 1 0
>> 51975000 0 0 50000 Y 32e90000.display-
>> controller pix
>>
>> Here 346500000 (media_ldb) != 7 * 51975000 (media_disp2_pix)
>> -> distorted panel image (if any).
>> The requested panel pixel clock from EDID is 51200000.
>
> Right, this is what Miquel is trying to solve with their series.
>
>> This is the same with my patch:
>>
>> video_pll1_ref_sel 1 1 0
>> 24000000 0 0 50000 Y deviceless
>> no_connection_id
>> video_pll1 1 1 0 1039500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> video_pll1_bypass 1 1 0 1039500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> video_pll1_out 2 2 0 1039500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> media_ldb 1 1 0
>> 346500000 0 0 50000 Y 32ec0000.blk-
>> ctrl:bridge@5c ldb
>> deviceless
>> no_connection_id
>> media_ldb_root_clk 0 0 0 346500000
>> 0 0 50000 Y deviceless
>> no_connection_id
>> media_disp2_pix 1 1 0
>> 49500000 0 0 50000 Y deviceless
>> no_connection_id
>> media_disp2_pix_root_clk 1 1 0
>> 49500000 0 0 50000 Y 32e90000.display-
>> controller pix
>>
>> So, here 346500000 (media_ldb) = 7 * 49500000 (media_disp2_pix).
>> -> stable panel image, but pixel clock reduced to 49.5 MHz from
>> requested 51.2 MHz.
>
> Inaccurate pixel clock and non-60Hz frame rate is not a win either.
Some percents of deviation is usually not visible.
>
>> My conclusion: The clock source is the same
>
> I agree .
>
> You wrote "derived from different clock sources" above,
> keyword:different, which is not correct.
>
>> , nevertheless the
>> ldb/pixel clock constraint cannot be satisfied without either
>> modifying the pll clock or the pixel clock.
> In this particular case, you surely do want to modify the PLL settings
> to achieve accurate pixel clock.
No, in this case there is a 3 percent deviation, resulting in 58 Hz
frame rate instead of 60 Hz.
--
Nikolaus Voss
Powered by blists - more mailing lists