On Tue, 2024-10-08 at 23:48 +0200, Marek Vasut wrote:
> 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@xxxxxxx>
> > > > > >
> > > > > > With the other rc358767 patches in place, this does the
> > > > > > trick.
> > > > > > Reviewed-by: Alexander Stein
> > > > > > <alexander.stein@xxxxxxxxxxxxxxx>
> > > > >
> > > > > 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).
> Thank you, this helped greatly.
>
> I believe I know why it used to kind-of work for you, but I'm afraid
> this used to work by sheer chance and it does not really work
> correctly
> for the panel you use, even if the panel likely does show the correct
> content. But, there is a way to make it work properly for the panel
> you use.
>
> First of all, the pixel clock never really matched the panel-simple.c
> pixel clock for the edt_etml1010g3dra_timing:
>
> $ grep '\<media_disp2_pix\>' 6.12-rc2_summary_postfix
> media_disp2_pix 1 1 0 74250000 ...
> ^^^^^^^^
>
> $ grep -A 1 edt_etml1010g3dra_timing drivers/gpu/drm/panel/panel-
> simple.c
> static const struct display_timing edt_etml1010g3dra_timing = {
> .pixelclock = { 66300000, 72400000, 78900000 },
> ^^^^^^^^
>
> The pixel clock are within tolerance, but there is a discrepancy
> 74250000 != 72400000 .
>
> Since commit 94e6197dadc9 ("arm64: dts: imx8mp: Add LCDIF2 & LDB
> nodes")
> the IMX8MP_VIDEO_PLL1_OUT is set to a very specific frequency of
> 1039500000 Hz, which tidily divides by 2 to 519750000 Hz (which is
> your
> LVDS serializer frequency) and divides by 7 to 74250000 Hz which is
> your
> LCDIF pixel clock.
>
> 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>;
> };
>
> Can you please test whether this works and the pixel clock are
> accurate
> in /sys/kernel/debug/clk/clk_summary ?
Interestingly, after making the change you suggested to imx8mp-
phyboard-pollux-rdk.dts before the two reversions, the display now
seems to work. Please see below for the relevant section of the new
clk_summary referring to media_disp2_pix:
video_pll1_ref_sel 1 1 0 24000000
0 0 50000 Y deviceless
no_connection_id
video_pll1 1 1 0
506800000 0 0 50000 Y deviceless
no_connection_
video_pll1_bypass 1 1 0
506800000 0 0 50000 Y deviceless
no_connecti
video_pll1_out 2 2 0
506800000 0 0 50000 Y deviceless
no_conne
media_ldb 1 1 0
506800000 0 0 50000 Y deviceless
no_co
media_ldb_root_clk 1 1 0
506800000 0 0 50000 Y
32ec0000.blk-ctrl:bridge@5c l
deviceless no
media_disp2_pix 1 1 0 72400000
0 0 50000 Y deviceless
no_co
media_disp2_pix_root_clk 1 1 0
72400000 0 0 50000 Y
32e90000.display-controller
32ec0000.blk-ctrl di
deviceless no
media_disp1_pix 0 0 0
506800000 0 0 50000 N deviceless
no_co
media_disp1_pix_root_clk 0 0 0
506800000 0 0 50000 N
32ec0000.blk-ctrl
deviceless no
media_mipi_phy1_ref 0 0 0 23036364
0 0 50000 N deviceless
no_co
media_mipi_phy1_ref_root 0 0 0
23036364 0 0 50000 N
32ec0000.blk-ctrl
The media_disp2_pix clock now seems to be correct at 724000000 after
your changes.
