Il 27/11/24 08:02, CK Hu (胡俊光) ha scritto:
On Tue, 2024-11-26 at 10:25 +0100, AngeloGioacchino Del Regno wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content.
Il 26/11/24 04:07, CK Hu (胡俊光) ha scritto:
On Mon, 2024-11-25 at 17:55 +0100, AngeloGioacchino Del Regno wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content.
Il 22/11/24 08:23, CK Hu (胡俊光) ha scritto:
Hi, Angelo:
On Wed, 2024-11-20 at 13:44 +0100, AngeloGioacchino Del Regno wrote:
External email : Please do not click links or open attachments until you have verified the sender or the content.
Add support for the DPI block found in the MT8195 and MT8188 SoCs.
Inside of the SoC, this block is directly connected to the HDMI IP.
In MT8173, DPI0 is directly connected to HDMI.
The first version of this driver is just for MT8173 DPI0.
Does MT8173 DPI0 need this modification?
Or this modification is just for MT8188 and MT8195, then the description should be more than 'directly connected'.
This is only for MT8188 and MT8195, and MT8173 does *not* need any modification.
Please, what would you like to see in the description of this commit?
This patch does four jobs.
1. Enable/disable tvd_clk for MT8195/MT8188 DPI.
2. Do not set pixel clock for MT8195/MT8188 DPI.
3. New DPI_INPUT_XXX and DPI_OUTPUT_XXX control for MT8195/MT8188 DPI.
4. Do not power on/off for MT8195/MT8188 DPI.
Maybe you should break into 4 patches and each one has different reason.
Yeah I thought about that as well, but there's a fundamental issue with splitting
the thing in multiple patches...
For enabling the tvd_clk in a separate patch, there's no problem - however, for the
others....
1. We need to introduce support for MT8195/88 DPI-HDMI, or the other patches would
not make sense (nor apply, anyway); then
2. We stop setting pixel clock with another patch; then
3. we don't power on/off, etc etc
The problem with doing it like so is that the patch #1 that I described would be
introducing *faulty code*, because the support for that really depends on all of
the others being present (otherwise the block won't work correctly).
So... if you want, I can easily split out the tvd_clk enable/disable, but splitting
the rest wouldn't be clean.
Besides, keep in mind that... actually... for anything else that is not MT8195/88
DPI0 (so, for other SoCs' DPI and for 95/88 DPINTF) the tvd_clk is already getting
enabled by its child.. so, for those ones, a call to enable tvd_clk does exactly
nothing apart from incrementing (enable) or decrementing (disable) the refcount for
this clock by 1.
This means that the enablement/disablement of tvd_clk is actually important only
for the MT8195/88 DPI and has literally no effect on anything else that is
currently supported by the mtk_dpi driver anyway.
Still - if you want me to split out the tvd_clk en/dis patch, just confirm and I
will split that one out...
For #1 and #2, I've not reviewed the HDMI driver. Is the clock control influenced by new HDMI driver.
It kinda is - the HDMI-TX block gets its clock from the HDMI PHY's clock gen,
but eventually it is the HDMI driver that tells to the PHY driver what clock it
actually wants.
For #1, clk_prepare_enable() is ungating the clock that would otherwise gate the
PHY's PLL output to the HDMI block.
If it is software reason, maybe we can modify the new HDMI driver and make DPI driver consistent with MT8173.
If it is hardware reason. just describe the hardware reason.
Alright - the hardware reason is that the HDMIPHY generates the clock for the HDMI
TX block, and that enabling the clock assigned to tvd_clk is necessary to ungate
the PHY's ckgen output to the HDMI-TX (and I think - but not sure as I haven't
analyzed that yet - that HDMI-RX should have the same gating technique, but that's
definitely out of scope for this submission).
I think tvd_clk is the clock source of DPI, HDMI, and HDMI-PHY, so these hardware could work in the same frequency.
That means drivers of DPI, HDMI, and HDMI-PHY are equal to control tvd_clk.
In MT8173. software choose DPI driver to control tvd_clk.
In MT8195, software choose HDMI-PHY driver to control tvd_clk.
Yes, but in MT8195 the tvd is gated by a clock that is controller by the HDMI
driver only, and not by the PHY - so, PHY sets the frequency, mtk_hdmi_v2 ungates
that to the HDMITX block (with clk_prepare_enable(tvd_clk)).
I would like to have the same control flow.
If "HDMI-PHY driver to control tvd_clk" is better, we could temporarily let MT8195 has different flow with MT8173.
So, is "HDMI-PHY driver to control tvd_clk" better?
I'm not sure I understand this last part, can you please rephrase?
For #4, I don't know why DPI do not control power by its self?
Even though other driver may control the same power, power manager has reference count,
so each driver could control the same power by its self.
#4 is there both for a SW and for a HW reason.
The HW reason is that the DPI shall be powered on in a specific sequence in regard
to HDMI-TX, due to the setup that is required by both (and ungating clocks before
full configuration happens would lock up the hw block).
The SW reason is that mtk_crtc.c calls mtk_crtc_ddp_hw_init()->mtk_ddp_comp_start()
in its .atomic_enable() callback, which happens in the wrong sequence in regard to
HDMI because of the "natural" components order in the DRM framework (for MT8195/88!
because for the others it either is the inverse or it does not matter - so for
performance it's okay for it to be like that both on older SoCs and on DPINTF for
95/88) and this means that we *must not* call dpi_power_on() at that time but
we must rather follow the atomic_enable()/bridge_enable() order imposed by DRM
*also* for the clock en/dis calls in DPI.
It looks like the #4 could be a separate patch.
The commit message is what you describe here.
And
if (!dpi->conf->support_hdmi_power_sequence)
mtk_dpi_power_on();
This means that I'd have to introduce the "hdmi power sequence" before actually
introducing the real support for MT8195 HDMI....
I honestly don't like that "too much", but it's fine, I don't have *too strong*
opinions about that, so I will separate #4 as you suggested for v2.
Cheers,
Angelo
Regards,
CK
Cheers,
Angelo
Regards,
CK
Cheers,
Angelo
Regards,
CK
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
---
