On Mon, 3 Jun 2024 at 16:06, Marc Gonzalez <mgonzalez@xxxxxxxxxx> wrote:
>
> On 30/05/2024 18:50, Dmitry Baryshkov wrote:
>
> > Ok, you have dropped several clocks, which I think might be required
> > for the device to function. For example, msm8996 doesn't have
> > MNOC_AHB_CLK, while msm8998 has it. It might be that we should be
> > enabling the clock via the interconnect driver instead (or maybe it is
> > handled by RPM?).
> >
> > Let's hope that we can sort the clocks. I have no other issues remaining.
>
> For quick reference:
>
> msm8996-sde.dtsi (VENDOR)
>
> sde_hdmi_tx: qcom,hdmi_tx_8996@9a0000 {
> compatible = "qcom,hdmi-tx-8996";
>
> reg = <0x009a0000 0x50c>,
> <0x00070000 0x6158>,
> <0x009e0000 0xfff>;
> reg-names = "core_physical",
> "qfprom_physical",
> "hdcp_physical";
> clocks = <&clock_mmss clk_mdss_mdp_vote_clk>,
> <&clock_mmss clk_mdss_ahb_clk>,
> <&clock_mmss clk_mdss_hdmi_clk>,
> <&clock_mmss clk_mdss_hdmi_ahb_clk>,
> <&clock_mmss clk_mdss_extpclk_clk>;
> clock-names =
> "mdp_core_clk",
> "iface_clk",
> "core_clk",
> "alt_iface_clk",
> "extp_clk";
> interrupt-parent = <&sde_kms>;
> interrupts = <8 0>;
> hpd-gdsc-supply = <&gdsc_mdss>;
> qcom,hdmi-tx-hpd-gpio = <&pm8994_mpps 4 0>;
> pinctrl-names = "default", "sleep";
> pinctrl-0 = <&mdss_hdmi_hpd_active
> &mdss_hdmi_ddc_active
> &mdss_hdmi_cec_active>;
> pinctrl-1 = <&mdss_hdmi_hpd_suspend
> &mdss_hdmi_ddc_suspend
> &mdss_hdmi_cec_suspend>;
>
> sde_hdmi_audio: qcom,sde-hdmi-audio-rx {
> compatible = "qcom,msm-hdmi-audio-codec-rx";
> };
> };
>
>
>
> msm8996.dtsi (MAINLINE)
>
> mdss_hdmi: hdmi-tx@9a0000 {
> compatible = "qcom,hdmi-tx-8996";
> reg = <0x009a0000 0x50c>,
> <0x00070000 0x6158>,
> <0x009e0000 0xfff>;
> reg-names = "core_physical",
> "qfprom_physical",
> "hdcp_physical";
>
> interrupt-parent = <&mdss>;
> interrupts = <8>;
>
> clocks = <&mmcc MDSS_MDP_CLK>,
> <&mmcc MDSS_AHB_CLK>,
> <&mmcc MDSS_HDMI_CLK>,
> <&mmcc MDSS_HDMI_AHB_CLK>,
> <&mmcc MDSS_EXTPCLK_CLK>;
> clock-names =
> "mdp_core",
> "iface",
> "core",
> "alt_iface",
> "extp";
>
> phys = <&mdss_hdmi_phy>;
> #sound-dai-cells = <1>;
>
> status = "disabled";
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> mdss_hdmi_in: endpoint {
> remote-endpoint = <&mdp5_intf3_out>;
> };
> };
> };
> };
>
>
>
> msm8998-sde.dtsi (VENDOR)
>
> sde_hdmi_tx: qcom,hdmi_tx_8998@c9a0000 {
> cell-index = <0>;
> compatible = "qcom,hdmi-tx-8998";
> reg = <0xc9a0000 0x50c>,
> <0x780000 0x621c>,
> <0xc9e0000 0x28>;
> reg-names = "core_physical", "qfprom_physical", "hdcp_physical";
> interrupt-parent = <&sde_kms>;
> interrupts = <8 0>;
> interrupt-controller;
> #interrupt-cells = <1>;
> qcom,hdmi-tx-ddc-clk-gpio = <&tlmm 32 0>;
> qcom,hdmi-tx-ddc-data-gpio = <&tlmm 33 0>;
> qcom,hdmi-tx-hpd-gpio = <&tlmm 34 0>;
> qcom,hdmi-tx-hpd5v-gpio = <&tlmm 133 0>;
> pinctrl-names = "default", "sleep";
> pinctrl-0 = <&mdss_hdmi_hpd_active
> &mdss_hdmi_ddc_active
> &mdss_hdmi_5v_active>;
> pinctrl-1 = <&mdss_hdmi_hpd_suspend
> &mdss_hdmi_ddc_suspend
> &mdss_hdmi_5v_suspend>;
> hpd-gdsc-supply = <&gdsc_mdss>;
> qcom,supply-names = "hpd-gdsc";
> qcom,min-voltage-level = <0>;
> qcom,max-voltage-level = <0>;
> qcom,enable-load = <0>;
> qcom,disable-load = <0>;
>
> clocks = <&clock_mmss clk_mmss_mnoc_ahb_clk>,
> <&clock_mmss clk_mmss_mdss_ahb_clk>,
> <&clock_mmss clk_mmss_mdss_hdmi_clk>,
> <&clock_mmss clk_mmss_mdss_mdp_clk>,
> <&clock_mmss clk_mmss_mdss_hdmi_dp_ahb_clk>,
> <&clock_mmss clk_mmss_mdss_extpclk_clk>,
> <&clock_mmss clk_mmss_mnoc_ahb_clk>,
> <&clock_mmss clk_mmss_misc_ahb_clk>,
> <&clock_mmss clk_mmss_mdss_axi_clk>;
> clock-names = "hpd_mnoc_clk", "hpd_iface_clk",
> "hpd_core_clk", "hpd_mdp_core_clk",
> "hpd_alt_iface_clk", "core_extp_clk",
> "mnoc_clk","hpd_misc_ahb_clk",
> "hpd_bus_clk";
>
> /*qcom,mdss-fb-map = <&mdss_fb2>;*/
> qcom,pluggable;
> };
>
>
>
> IIUC the discussion on IRC, the additional clocks are required,
> so the binding should be more like this:
>
> +++ b/Documentation/devicetree/bindings/display/msm/hdmi.yaml
> @@ -19,14 +19,15 @@ properties:
> - qcom,hdmi-tx-8974
> - qcom,hdmi-tx-8994
> - qcom,hdmi-tx-8996
> + - qcom,hdmi-tx-8998
>
> clocks:
> minItems: 1
> - maxItems: 5
> + maxItems: 8
>
> clock-names:
> minItems: 1
> - maxItems: 5
> + maxItems: 8
>
> reg:
> minItems: 1
> @@ -151,6 +152,27 @@ allOf:
> - const: extp
> hdmi-mux-supplies: false
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,hdmi-tx-8998
> + then:
> + properties:
> + clocks:
> + minItems: 8
> + clock-names:
> + items:
> + - const: mdp_core
> + - const: mnoc
> + - const: iface
> + - const: bus
> + - const: iface_mmss
> + - const: core
> + - const: alt_iface
> + - const: extp
>
>
>
> So this is good?
Yes
>
> clocks = <&mmcc MDSS_MDP_CLK>,
> <&mmcc MNOC_AHB_CLK>,
> <&mmcc MDSS_AHB_CLK>,
> <&mmcc MDSS_AXI_CLK>,
> <&mmcc MISC_AHB_CLK>,
> <&mmcc MDSS_HDMI_CLK>,
> <&mmcc MDSS_HDMI_DP_AHB_CLK>,
> <&mmcc MDSS_EXTPCLK_CLK>;
> clock-names =
> "mdp_core",
> "mnoc",
> "iface",
> "bus",
> "iface_mmss",
> "core",
> "alt_iface",
> "extp";
>
--
With best wishes
Dmitry
