On 10/22/24 8:13 AM, Liu Ying wrote:
[...]
This patch would cause the below in-flight LDB bridge driver
patch[1] fail to do display mode validation upon display modes
read from LVDS to HDMI converter IT6263's DDC I2C bus.
Why ?
Mode validation is affected only for dual LVDS link mode.
For single LVDS link mode, this patch does open more display
modes read from the DDC I2C bus. The reason behind is that
LVDS serial clock rate/pixel clock rate = 3.5 for dual LVDS
link mode, while it's 7 for single LVDS link mode.
In my system, "video_pll1" clock rate is assigned to 1.0395GHz
in imx8mp.dtsi. For 1920x1080-60.00Hz with 148.5MHz pixel
clock rate, "media_ldb" clock rate is 519.75MHz and
"media_disp2_pix" clock rate is 148.5MHz, which is fine for
dual LVDS link mode(x3.5). For newly opened up 1920x1080-59.94Hz
with 148.352MHz pixel clock rate, "video_pll1" clock rate will
be changed to 519.232MHz, "media_ldb" clock rate is 519.232MHz
and "media_disp2_pix" clock rate is wrongly set to 519.232MHz
too because "media_disp2_pix" clock cannot handle the 3.5
division ratio from "video_pll1_out" clock running at
519.232MHz. See the below clk_summary.
Shouldn't this patch help exactly with that ?
No, it doesn't help but only makes clk_round_rate() called in
LDB driver's .mode_valid() against 148.352MHz return 148.352MHz
which allows the unexpected 1920x1080-59.94Hz display mode.
Why is 1920x1080-59.94Hz mode unexpected in the first place ?
I assume your display device reports that it supports this mode, and now
the scanout engine and LDB can generate this mode too ? Or does the
display device NOT support this mode ?
It should allow you to set video_pll1_out to whatever is necessary by LDB first, fixate that frequency, and the LCDIFv3 would then be forced to use /7 divider from faster Video PLL1 , right ?
Yes, it allows that for single-link LVDS use cases.
And, __no__, for dual-link LVDS use cases because the
video_pll1_out clock rate needs to be 2x the LVDS serial clock
rate.
Can't the LDB still set the Video PLL frequency to whatever it needs
first, fixate it, and only then let the LCDIFv3 divide the frequency
down ? (sorry, I am a bit tired today, maybe I am missing the obvious)
video_pll1_ref_sel 1 1 0 24000000 0 0 50000 Y deviceless no_connection_id
video_pll1 1 1 0 519232000 0 0 50000 Y deviceless no_connection_id
video_pll1_bypass 1 1 0 519232000 0 0 50000 Y deviceless no_connection_id
video_pll1_out 2 2 0 519232000 0 0 50000 Y deviceless no_connection_id
media_ldb 1 1 0 519232000 0 0 50000 Y deviceless no_connection_id
media_ldb_root_clk 1 1 0 519232000 0 0 50000 Y 32ec0000.blk-ctrl:bridge@5c ldb
deviceless no_connection_id
media_disp1_pix 0 0 0 129808000 0 0 50000 N deviceless no_connection_id
media_disp1_pix_root_clk 0 0 0 129808000 0 0 50000 N 32e80000.display-controller pix
32ec0000.blk-ctrl disp1
deviceless no_connection_id
media_disp2_pix 1 1 0 519232000 0 0 50000 Y deviceless no_connection_id
media_disp2_pix_root_clk 1 1 0 519232000 0 0 50000 Y 32e90000.display-controller pix
32ec0000.blk-ctrl disp2
deviceless no_connection_id
Single LVDS link mode is not affected because "media_disp2_pix"
clock can handle the 7 division ratio.
To support the dual LVDS link mode, "video_pll1" clock rate needs
to be x2 "media_ldb" clock rate so that "media_disp2_pix" clock
can use 7 division ratio to achieve the /3.5 clock rate comparing
to "media_ldb" clock rate. However, "video_pll1" is not seen by
LDB driver thus not directly controlled by it. This is another
reason why assigning a reasonable "video_pll1" clock rate in DT
makes sense.
I agree that _right_now_, the DT clock assignment is the only option.
I would like to see that DT part disappear and instead would prefer if the LDB/LCDIF could figure out the clock tree configuration themselves.
I think we'll live with the assigned clock rate in DT, because the
i.MX8MP LDB and Samsung MIPI DSI display pipelines need to share a
video PLL, like I mentioned in comments for patch 2.
They do NOT need to share a PLL, you can use e.g. PLL3 for one and Video
PLL for the other if the requirement is accurate pixel clock .
[...]
