[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fba91fbb-e819-4b08-9845-fa1138773113@denx.de>
Date: Mon, 9 Dec 2024 22:51:12 +0100
From: Marek Vasut <marex@...x.de>
To: Nikolaus Voss <nv@...n.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 12/9/24 10:27 AM, Nikolaus Voss wrote:
> 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.
The PLL is accurate, so this kind of non-60 Hz frame rate compromise
really should not be necessary.
>>> 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.
Consider e.g. 60 FPS video playback, on 58 Hz refresh panel it will
suffer from some stutter . It is better to aim for the 60 Hz then .
Powered by blists - more mailing lists