[PATCH v2] drm: bridge: fsl-ldb: fixup mode on freq mismatch

[Nikolaus Voss]

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 at 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 at 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