Re: [PATCH] drm: lcdif: Use adjusted_mode .clock instead of .crtc_clock

[Marek Vasut <marex@denx.de>]

On 10/19/24 11:49 PM, Kieran Bingham wrote:
> Quoting Marek Vasut (2024-10-12 21:37:59)
>> On 10/11/24 5:10 AM, Liu Ying wrote:
>>
>> Hi,
>>
>>>>>> This Video PLL1 configuration since moved to &media_blk_ctrl {} , but it is still in the imx8mp.dtsi . Therefore, to make your panel work at the correct desired pixel clock frequency instead of some random one inherited from imx8mp.dtsi, add the following to the pollux DT, I believe that will fix the problem and is the correct fix:
>>>>>>
>>>>>> &media_blk_ctrl {
>>>>>>       // 506800000 = 72400000 * 7 (for single-link LVDS, this is enough)
>>>>>>       // there is no need to multiply the clock by * 2
>>>>>>       assigned-clock-rates = <500000000>, <200000000>, <0>, <0>, <500000000>, <506800000>;
>>>>>
>>>>> This assigns "video_pll1" clock rate to 506.8MHz which is currently not
>>>>> listed in imx_pll1443x_tbl[].
>>>>
>>>> Since commit b09c68dc57c9 ("clk: imx: pll14xx: Support dynamic rates") the 1443x PLLs can be configured to arbitrary rates which for video PLL is desirable as those should produce accurate clock.
>>>
>>> Ack.
>>>
>>>>
>>>>> Does the below patch[1] fix the regression issue? It explicitly sets
>>>>> the clock frequency of the panel timing to 74.25MHz.
>>>>>
>>>>> [1] https://patchwork.freedesktop.org/patch/616905/?series=139266&rev=1
>>>> That patch is wrong, there is an existing entry for this panel in panel-simple.c which is correct and precise, please do not add that kind of imprecise duplicate timings into DT.
>>>
>>> At least the patch[1] is legitimate now to override the display
>>> timing of the panel because the override mode is something
>>> panel-simple.c supports.
>>
>> It may be possible to override the mode, but why would this be the
>> desired if the panel-simple.c already contains valid accurate timings
>> for this particular panel ?
> 
> I'm confused a little here. Why is it that setting the panel timings in
> the DT as per [1] make the display work, while the panel timeings in
> panel-simple alone are not enough?
> 
> Is there some difference in code path for 'how' the panel timings are
> set as to whether they will apply fully or not ?
Because [1] sets inaccurate pixel clock of 74.25 MHz, which can be 
divided down from random default Video PLL setting of 1039.5 MHz set in 
imx8mp.dtsi media_blk_ctrl , 1039.5 / 74.25 = 14 .

The panel-simple pixel clock are 72.4 MHz, to achieve that accurately, 
it is necessary to reconfigure the Video PLL frequency to 506.8 MHz , 
which LCDIFv3 can do, but LDB can not, hence it has to be done in DT for 
now.