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.