On 11/20/24 7:38 AM, Ying Liu wrote:
[...]
If the DP monitors support typical video modes like 1080p60 with
148.5MHz pixel clock rate, I assume these typical video modes work
still ok with this patch at least. Please help confirm this, since if the
alternative solution(*) doesn't stand, we would know those video
modes still work ok with my solution(fixed PLL rate).
They do not work with the fixed PLL setting.
Why? Did you assign a sensible fixed PLL rate in DT?
Whatever was in imx8mp.dtsi does not really work for all the panels.
Please keep in mind that the use case I have does not include only
1920x1080 "standard" panels, but also other resolutions.
Can you please compare clk_summary output for the failing cases
before and after this patch is applied? I assume that if you use
the fixed PLL rate same to the rate which works before this patch is
applied, the typical video modes still just work after this patch is
applied.
I'm afraid I do not need to support only typical video modes, but also
the other "atypical" modes.
[...]
One really nasty way I can think of is -- use find_node_by_compatible(),
look up all the relevant DT nodes, parse their clock properties, and
check whether they all point to the Video PLL or not.
That's nasty. It looks even more nasty when considering the fact that
i.MX93 LCDIF is also driven by imx-lcdif DRM while only i.MX8MP LCDIF
needs the nasty check, because i.MX93 SoC embeds only one LCDIF.
The check can be skipped based on compatible string.
I agree it is nasty, but it is a start. Are there better ideas ?
Maybe the clock subsystem has a better way, like list "neighbor"
consumers of some specific parent clock or something like that.
What will imx-lcdif DRM look like by using this way? Get the ancestor PLL
clock of pixel clock(media_disp{1,2}_pix_root_clk), list all child clocks
(media_disp1_pix and/or media_disp2_pix + other possible clocks) of the
PLL clock in a string array and find media_disp1_pix + media_disp2_pix
in it?
Doesn't look nice, either.
One other option came to my mind -- place a virtual clock between the
Video PLL and consumers (LCDIF1/2/LDB), and then have the virtual clock
driver do the clock rate negotiation in some .round_rate callback. That
is also nasty, but it is another idea. If there is a clock specifically
implemented to negotiate best upstream clock rate for all of its
consumers, and it is aware of the consumer behavior details and
requirements, maybe that could work ?
[...]
Can something like (*) above be implemented instead, so both Shared
and
separate PLLs would be supported ? That should solve both of our use
cases, right ?
I don't see any clear way to implement something like(*).
Take the 3 i.MX8MP LCDIFs as one graphic card driven by one imx-lcdif
DRM instance? Would it be too intrusive?
Yes, and I think unnecessary, one can simply traverse and parse the DT
to determine the clock assignment?
Yes, people can traverse and parse DT, but it's nasty.
In addition, one may argue that now that CLK_SET_RATE_PARENT flag
is set for the pixel clocks, all potential video modes read from EDID
should be supported when only either LVDS display pipeline or MIPI DSI
display pipeline is active in the shared PLL case. This requires one
single DRM instance to detect single or dual active display pipelines
dynamically, hence this single DRM instance becomes necessary.
Would single virtual clock which do the frequency negotiation between
multiple DRM consumers work too ?
I do not have much to add to the points below.
