lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ed914413fd0e9dc38887b139da733499@vosn.de>
Date: Wed, 11 Dec 2024 17:47:46 +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

Hi Marek,

On 09.12.2024 22:51, Marek Vasut wrote:
> 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 .

This is a relevant use case, I agree. But even with a dedicated video
PLL (what a luxury in the embedded world!) you will eventually have to
drop or double a frame as the clocks are asynchronous. And the sync
problem still exists with 25 or 50 FPS videos.

-- 
Nikolaus Voss

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ