On Thu, 30 May 2024 at 15:34, Marc Gonzalez <mgonzalez@xxxxxxxxxx> wrote:
>
> On 28/05/2024 02:45, Dmitry Baryshkov wrote:
>
> > While I don't see anything wrong with this patch, maybe it's better to
> > include it into the patchset that adds all HDMI nodes to the msm8998.dtsi.
>
> Here is my current diff:
> Do I just need to split it up, and it's good to go?
> (Doubtful++)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml b/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml
> index 83fe4b39b56f4..78607ee3e2e84 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-qmp.yaml
> @@ -14,6 +14,7 @@ properties:
> compatible:
> enum:
> - qcom,hdmi-phy-8996
> + - qcom,hdmi-phy-8998
>
> reg:
> maxItems: 6
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index e5f051f5a92de..182d80c2ab942 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -1434,6 +1434,34 @@ blsp2_spi6_default: blsp2-spi6-default-state {
> drive-strength = <6>;
> bias-disable;
> };
> +
> + hdmi_cec_default: hdmi-cec-default-state {
> + pins = "gpio31";
> + function = "hdmi_cec";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + hdmi_ddc_default: hdmi-ddc-default-state {
> + pins = "gpio32", "gpio33";
> + function = "hdmi_ddc";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +
> + hdmi_hpd_default: hdmi-hpd-default-state {
> + pins = "gpio34";
> + function = "hdmi_hot";
> + drive-strength = <16>;
> + bias-pull-down;
> + };
> +
> + hdmi_hpd_sleep: hdmi-hpd-sleep-state {
> + pins = "gpio34";
> + function = "hdmi_hot";
> + drive-strength = <2>;
> + bias-pull-down;
> + };
> };
>
> remoteproc_mss: remoteproc@4080000 {
> @@ -2757,7 +2785,7 @@ mmcc: clock-controller@c8c0000 {
> <&mdss_dsi0_phy 0>,
> <&mdss_dsi1_phy 1>,
> <&mdss_dsi1_phy 0>,
> - <0>,
> + <&hdmi_phy 0>,
> <0>,
> <0>,
> <&gcc GCC_MMSS_GPLL0_DIV_CLK>;
> @@ -2862,6 +2890,14 @@ dpu_intf2_out: endpoint {
> remote-endpoint = <&mdss_dsi1_in>;
> };
> };
> +
> + port@2 {
> + reg = <2>;
> +
> + dpu_intf3_out: endpoint {
> + remote-endpoint = <&hdmi_in>;
> + };
> + };
> };
> };
>
> @@ -3017,6 +3053,103 @@ mdss_dsi1_phy: phy@c996400 {
>
> status = "disabled";
> };
> +
> + hdmi: hdmi-tx@c9a0000 {
> + compatible = "qcom,hdmi-tx-8998";
> + reg = <0x0c9a0000 0x50c>,
> + <0x00780000 0x6220>,
> + <0x0c9e0000 0x2c>;
> + reg-names = "core_physical",
> + "qfprom_physical",
> + "hdcp_physical";
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
Just <8>. MDSS doesn't have IRQ types.
> +
> + 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";
This device was neither validated nor described properly in the DT
schema. There are several other issues here.
> +
> + phys = <&hdmi_phy>;
> + phy-names = "hdmi_phy";
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&hdmi_hpd_default
> + &hdmi_ddc_default
> + &hdmi_cec_default>;
> + pinctrl-1 = <&hdmi_hpd_sleep
> + &hdmi_ddc_default
> + &hdmi_cec_default>;
> +
> + power-domains = <&rpmpd MSM8998_VDDCX>;
Is it? I don't see this in msm-4.4
> +
> + #sound-dai-cells = <1>;
> +
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + hdmi_in: endpoint {
> + remote-endpoint = <&dpu_intf3_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + hdmi_out: endpoint {
> + };
> + };
> + };
> + };
> +
> + 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>,
> + <&xo>;
This is most likely RPM_SMD_XO_CLK_SRC or maybe RPM_SMD_LN_BB_CLK1
> + clock-names =
> + "iface",
> + "ref",
> + "xo";
> + power-domains = <&rpmpd MSM8998_VDDMX>;
Is it?
> +
> + status = "disabled";
> + };
> };
>
> venus: video-codec@cc00000 {
>
--
With best wishes
Dmitry
