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

[Marek Vasut]

On 10/8/24 12:07 PM, Isaac Scott wrote:
> On Mon, 2024-10-07 at 20:06 +0200, Marek Vasut wrote:
>> On 10/7/24 7:01 PM, Isaac Scott wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On Sat, 2024-07-06 at 02:16 +0200, Marek Vasut wrote:
>>>> On 6/24/24 11:19 AM, Alexander Stein wrote:
>>>>> Am Freitag, 31. Mai 2024, 22:27:21 CEST schrieb Marek Vasut:
>>>>>> In case an upstream bridge modified the required clock
>>>>>> frequency
>>>>>> in its .atomic_check callback by setting adjusted_mode.clock
>>>>>> ,
>>>>>> make sure that clock frequency is generated by the LCDIFv3
>>>>>> block.
>>>>>>
>>>>>> This is useful e.g. when LCDIFv3 feeds DSIM which feeds
>>>>>> TC358767
>>>>>> with (e)DP output, where the TC358767 expects precise timing
>>>>>> on
>>>>>> its input side, the precise timing must be generated by the
>>>>>> LCDIF.
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>>
>>>>> With the other rc358767 patches in place, this does the trick.
>>>>> Reviewed-by: Alexander Stein <alexander.stein at ew.tq-group.com>
>>>>
>>>> I'll pick this up next week if there is no objection.
>>>
>>> Unfortunately, this has caused a regression that is present in
>>> v6.12-
>>> rc1 on the i.MX8MP PHYTEC Pollux using the
>>> arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts. The
>>> display is the edt,etml1010g3dra panel, as per the upstream dts. We
>>> bisected to this commit, and reverting this change fixed the
>>> screen.
>>>
>>> We then tried to retest this on top of v6.12-rc2, and found we also
>>> had
>>> to revert commit ff06ea04e4cf3ba2f025024776e83bfbdfa05155 ("clk:
>>> imx:
>>> clk-imx8mp: Allow media_disp pixel clock reconfigure parent rate")
>>> alongside this. Reverting these two commits makes the display work
>>> again at -rc2.
>>>
>>> Do you have any suggestions on anything we might be missing on our
>>> end?
>>> Please let me know if there's anything you'd like me to test as I'm
>>> not
>>> sure what the underlying fault was here.
>> I believe what is going on is that the LCDIF cannot configure its
>> upstream clock because something else is already using those clock
>> and
>> it set those clock to a specific frequency. LCDIF is now trying to
>> configure those clock to match the LVDS panel, and it fails, so it
>> tries
>> to set some approximate clock and that is not good enough for the
>> LVDS
>> panel.
>>
>> Can you share dump of /sys/kernel/debug/clk/clk_summary on failing
>> and
>> working system ? You might see the difference around the "video"
>> clock.
>>
>> (I have seen this behavior before, the fix was usually a matter of
>> moving one of the LCDIFs to another upstream clock like PLL3, so it
>> can
>> pick well matching output clock instead of some horrid approximation
>> which then drives the panel likely out of specification)
> 
> Hi Marek,
> 
> Please find attached the clk_summary for v6.12-rc2 before and after the
> reversion (the one after the reversion is 6.12-rc2_summary_postfix).
How come "media_mipi_phy1_ref" is on Video PLL1 before the revert ?

Does it start working if you move "media_mipi_phy1_ref" to osc_24m ? 
(probably not)

Also, why is the LDB configured to 74 MHz instead of 519 MHz now ? That 
is really odd. I'll see if I can reproduce this later today.