On Fri, Jun 14, 2024 at 01:55:46AM GMT, Konrad Dybcio wrote:
>
>
> On 6/13/24 17:15, Marc Gonzalez wrote:
> > From: Arnaud Vrac <avrac@xxxxxxxxxx>
> >
> > Port device nodes from vendor code.
> >
> > Signed-off-by: Arnaud Vrac <avrac@xxxxxxxxxx>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > Signed-off-by: Marc Gonzalez <mgonzalez@xxxxxxxxxx>
> > ---
>
> [...]
>
> > +
> > + hdmi: hdmi-tx@c9a0000 {
> > + compatible = "qcom,hdmi-tx-8998";
> > + reg = <0x0c9a0000 0x50c>,
> > + <0x00780000 0x6220>,
> > + <0x0c9e0000 0x2c>;
> > + reg-names = "core_physical",
> > + "qfprom_physical",
> > + "hdcp_physical";
>
> The way qfprom is accessed (bypassing nvmem APIs) will need to be reworked..
> but since we already have it like that on 8996, I'm fine with batch-reworking
> these some time in the future..
Yes. The whole qfprom / hdcp part needs to be reworked, but it should
not stop the platform from flowing in.
>
> > +
> > + interrupt-parent = <&mdss>;
> > + interrupts = <8>;
> > +
> > + clocks = <&mmcc MDSS_MDP_CLK>,
>
> Not sure if the MDP core clock is necessary here. Pretty sure it only
> powers the display-controller@.. peripheral
It might be, or it might be not. DSI interfaces also use MDP_CLK on
those platforms.
>
> > + <&mmcc MDSS_AHB_CLK>,
> > + <&mmcc MDSS_HDMI_CLK>,
> > + <&mmcc MDSS_HDMI_DP_AHB_CLK>,
> > + <&mmcc MDSS_EXTPCLK_CLK>,
> > + <&mmcc MDSS_AXI_CLK>,
> > + <&mmcc MNOC_AHB_CLK>,
>
> This one is an interconnect clock, drop it
>
> > + <&mmcc MISC_AHB_CLK>;
>
> And please confirm whether this one is necessary
Let me quote the discussion on #linux-msm
<lumag> jhugo, do you know anything about MNOC_AHB_CLK / MISC_AHB_CLK? Should they be enabled for HDMI to work?
<jhugo> lumag: MNOC AHB, yes
<jhugo> lumag: MISC, probably
> > + clock-names =
> > + "mdp_core",
> > + "iface",
> > + "core",
> > + "alt_iface",
> > + "extp",
> > + "bus",
> > + "mnoc",
> > + "iface_mmss";
> > +
[...]
> > +
> > + hdmi_phy: hdmi-phy@c9a0600 {
> > + compatible = "qcom,hdmi-phy-8998";
> > + reg = <0x0c9a0600 0x18b>,
> > + <0x0c9a0a00 0x38>,
> > + <0x0c9a0c00 0x38>,
> > + <0x0c9a0e00 0x38>,
> > + <0x0c9a1000 0x38>,
> > + <0x0c9a1200 0x0e8>;
> > + reg-names = "hdmi_pll",
> > + "hdmi_tx_l0",
> > + "hdmi_tx_l1",
> > + "hdmi_tx_l2",
> > + "hdmi_tx_l3",
> > + "hdmi_phy";
> > +
> > + #clock-cells = <0>;
> > + #phy-cells = <0>;
> > +
> > + clocks = <&mmcc MDSS_AHB_CLK>,
> > + <&gcc GCC_HDMI_CLKREF_CLK>,
> > + <&rpmcc RPM_SMD_XO_CLK_SRC>;
> > + clock-names = "iface",
> > + "ref",
> > + "xo";
>
> GCC_HDMI_CLKREF_CLK is a child of xo, so you can drop the latter.
> It would also be worth confirming whether it's really powering the
> PHY and not the TX.. You can test that by trying to only power on the
> phy (e.g. call the phy_power_on or whatever APIs) with and without the
> clock
I'd prefer to keep it. I think the original DT used one of LN_BB clocks
here, so it might be that the HDMI uses CXO2 / LN_BB instead of the main
CXO.
If somebody can check, which clock is actually used for the HDMI, it
would be really great.
>
> Konrad
>
> --
> linux-phy mailing list
> linux-phy@xxxxxxxxxxxxxxxxxxx
> https://lists.infradead.org/mailman/listinfo/linux-phy
--
With best wishes
Dmitry
