mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nikolaus Voss <nv@vosn.de>
To: Marek Vasut <marex@denx.de>
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>,
	Liu Ying <victor.liu@nxp.com>,
	Luca Ceresoli <luca.ceresoli@bootlin.com>,
	Fabio Estevam <festevam@denx.de>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Robert Foss <rfoss@kernel.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	miquel.raynal@bootlin.com, nikolaus.voss@haag-streit.com,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drm: bridge: fsl-ldb: fixup mode on freq mismatch
Date: Mon, 09 Dec 2024 10:27:26 +0100	[thread overview]
Message-ID: <027aac3abff3f84a0ebf461653ed6c9b@vosn.de> (raw)
In-Reply-To: <5a6ab24d-6c74-497f-828e-b3e7645d664a@denx.de>

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

  reply	other threads:[~2024-12-09  9:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-03 19:09 Nikolaus Voss
2024-12-03 20:15 ` Marek Vasut
2024-12-04 10:40   ` Nikolaus Voss
2024-12-07 11:46     ` Marek Vasut
2024-12-09  9:27       ` Nikolaus Voss [this message]
2024-12-09 21:51         ` Marek Vasut
2024-12-11 16:47           ` Nikolaus Voss
2024-12-04 12:48 ` kernel test robot
2024-12-04 22:03 ` kernel test robot
2024-12-06 14:08 ` Miquel Raynal
2024-12-07  6:30   ` Nikolaus Voss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=027aac3abff3f84a0ebf461653ed6c9b@vosn.de \
    --to=nv@vosn.de \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=alexander.stein@ew.tq-group.com \
    --cc=andrzej.hajda@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=festevam@denx.de \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luca.ceresoli@bootlin.com \
    --cc=marex@denx.de \
    --cc=miquel.raynal@bootlin.com \
    --cc=neil.armstrong@linaro.org \
    --cc=nikolaus.voss@haag-streit.com \
    --cc=rfoss@kernel.org \
    --cc=victor.liu@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®