Re: [PATCH 14/18] drm/mediatek: dpi: add support for dpi clock

From: Alexandre Mergnat
Date: Tue Apr 16 2024 - 10:54:25 EST



On 24/10/2023 11:12, AngeloGioacchino Del Regno wrote:
Il 23/10/23 16:40, amergnat@xxxxxxxxxxxx ha scritto:
From: Fabien Parent <fparent@xxxxxxxxxxxx>

MT8365 requires an additional clock for DPI. Add support for that
additional clock.

Signed-off-by: Fabien Parent <fparent@xxxxxxxxxxxx>
Signed-off-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>

I'm not convinced that this is right... at all.

From a fast check of the MT8365 DPI clocks, I can see that the DPI0 clock declares
parent VPLL_DPIX (a fixed clock), but nothing ever has VPLL_DPIX_EN (which is the
GATE clock, enabling output of DPIx VPLL?).

But then, there's even more: no clock ever references the CLK_TOP_DPI0_SEL nor the
CLK_TOP_DPI1_SEL gate, which is a PLL parent selector... in other platforms, that
is muxing through the TVDPLL, but on MT8365 that is LVDSPLL?!

AFAI see into mt8365 documentation, there is no TVDPLL, only LVDSPLL


I have many questions now:
* Two PLLs are apparently brought up, but which one is the right one?!
  * Is the LVDS PLL really used for DisplayPort? (dpi0_sel)

Seems to be LVDS

enable prepare protect duty hardware
clock count count count rate accuracy phase cycle enable
-------------------------------------------------------------------------------------------------------
clk26m 18 19 1 26000000 0 0 Y
vpll_dpix 1 1 0 75000000 0 0 50000 Y
mm_flvdstx_pxl 0 0 0 75000000 0 0 50000 N
mm_dpi0_dpi0 1 1 0 75000000 0 0 50000 Y
vpll_dpix_en 0 0 0 75000000 0 0 50000 N
lvdspll 1 1 0 283999497 0 0 50000 Y
lvdspll_d16 0 0 0 17749968 0 0 50000 Y
lvdspll_d8 0 0 0 35499937 0 0 50000 Y
lvdspll_d4 0 0 0 70999874 0 0 50000 Y
lvdspll_d2 1 1 0 141999748 0 0 50000 Y
dpi0_sel 1 1 0 141999748 0 0 50000 Y
dpi1_sel 0 0 0 141999748 0 0 50000 N
mmpll 1 1 0 456999909 0 0 50000 Y
mmpll_ck 1 1 0 456999909 0 0 50000 Y
mm_sel 15 15 0 456999909 0 0 50000 Y
mm_dpi0 1 1 0 456999909 0 0 50000 Y



* Are you sure that CLK_MM_DPI0_DPI0's parent shouldn't be dpi0_sel instead?

I'm agree with you. After few change, it works.

- GATE_MM0(CLK_MM_DPI0_DPI0, "mm_dpi0_dpi0", "vpll_dpix", 20),
+ GATE_MM0(CLK_MM_DPI0_DPI0, "mm_dpi0_dpi0", "dpi0_sel", 20),

- clocks = <&topckgen CLK_TOP_DPI0_SEL>,
+ clocks = <&mmsys CLK_MM_DPI0_DPI0>,


enable prepare protect duty hardware
clock count count count rate accuracy phase cycle enable
-------------------------------------------------------------------------------------------------------
vpll_dpix 0 0 0 75000000 0 0 50000 Y
mm_flvdstx_pxl 0 0 0 75000000 0 0 50000 N
vpll_dpix_en 0 0 0 75000000 0 0 50000 N

lvdspll 1 1 0 283999497 0 0 50000 Y
lvdspll_d16 0 0 0 17749968 0 0 50000 Y
lvdspll_d8 0 0 0 35499937 0 0 50000 Y
lvdspll_d4 0 0 0 70999874 0 0 50000 Y
lvdspll_d2 1 1 0 141999748 0 0 50000 Y
dpi0_sel 1 1 0 141999748 0 0 50000 Y
mm_dpi0_dpi0 1 1 0 141999748 0 0 50000 Y
dpi1_sel 0 0 0 141999748 0 0 50000 N

mmpll 1 1 0 456999909 0 0 50000 Y
mmpll_d2 0 0 0 228499954 0 0 50000 Y
mmpll_ck 1 1 0 456999909 0 0 50000 Y
mm_sel 15 15 0 456999909 0 0 50000 Y
mm_dpi0 1 1 0 456999909 0 0 50000 Y


* Where is DPI1 in this SoC? Why is there a dpi1_sel clock, but no MM clock
  for the DPI1 controller? Is there any DPI1 controller, even?!

DPI1 isn't documented.

  * Why is there a DPI1 MUX, if there's no DPI1 controller?!

Good question, I don't know. Legacy of the downstream code.

That will be fixed for the next version.

--
Regards,
Alexandre