| Message ID | 8cc61db5-2920-4dd1-8132-5af434fb05b1@freebox.fr (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] arm64: dts: qcom: msm8998: add HDMI GPIOs | expand |
On Mon, May 27, 2024 at 05:40:15PM +0200, Marc Gonzalez wrote: > MSM8998 GPIO pin controller reference design defines: > > - CEC: pin 31 > - DDC: pin 32,33 > - HPD: pin 34 > > Downstream vendor code for reference: > > https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400 > > mdss_hdmi_{cec,ddc,hpd}_{active,suspend} > > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > --- > arch/arm64/boot/dts/qcom/msm8998.dtsi | 42 +++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) 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.
[...] > + hdmi_cec_default: hdmi-cec-default-state { > + pins = "gpio31"; > + function = "hdmi_cec"; > + drive-strength = <2>; > + bias-pull-up; > + }; > + > + hdmi_cec_sleep: hdmi-cec-sleep-state { > + pins = "gpio31"; > + function = "hdmi_cec"; > + drive-strength = <2>; > + bias-pull-up; > + }; It's super strange that both states are identical. Usually, the pull-up is disabled and the GPIO is unmuxed (i.e. function = "gpio"). If that's not the case for whatever reason, you can drop the sleep variants and simply reference the leftover one from both pinctrl-0 and pinctrl-1. Konrad
On 28/05/2024 14:31, Konrad Dybcio wrote: > [...] > >> + hdmi_cec_default: hdmi-cec-default-state { >> + pins = "gpio31"; >> + function = "hdmi_cec"; >> + drive-strength = <2>; >> + bias-pull-up; >> + }; >> + >> + hdmi_cec_sleep: hdmi-cec-sleep-state { >> + pins = "gpio31"; >> + function = "hdmi_cec"; >> + drive-strength = <2>; >> + bias-pull-up; >> + }; > > It's super strange that both states are identical. Usually, the pull-up > is disabled and the GPIO is unmuxed (i.e. function = "gpio"). If that's > not the case for whatever reason, you can drop the sleep variants and > simply reference the leftover one from both pinctrl-0 and pinctrl-1. Patch is a direct translation of the vendor code: https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400 I suppose it wouldn't be the first time that vendor code is doing something odd. Though, I'm a bit confused why you would single out hdmi-cec when hdmi_ddc is the same? Regards
On 5/28/24 14:45, Marc Gonzalez wrote: > On 28/05/2024 14:31, Konrad Dybcio wrote: > >> [...] >> >>> + hdmi_cec_default: hdmi-cec-default-state { >>> + pins = "gpio31"; >>> + function = "hdmi_cec"; >>> + drive-strength = <2>; >>> + bias-pull-up; >>> + }; >>> + >>> + hdmi_cec_sleep: hdmi-cec-sleep-state { >>> + pins = "gpio31"; >>> + function = "hdmi_cec"; >>> + drive-strength = <2>; >>> + bias-pull-up; >>> + }; >> >> It's super strange that both states are identical. Usually, the pull-up >> is disabled and the GPIO is unmuxed (i.e. function = "gpio"). If that's >> not the case for whatever reason, you can drop the sleep variants and >> simply reference the leftover one from both pinctrl-0 and pinctrl-1. > > Patch is a direct translation of the vendor code: > https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400 > > I suppose it wouldn't be the first time that vendor code > is doing something odd. > > Though, I'm a bit confused why you would single out hdmi-cec > when hdmi_ddc is the same? As in, me in the above message or the vendor devicetree? If the former, it's just an example If the latter, the muxing function differs so they must have their own separate nodes Konrad
On 28/05/2024 14:57, Konrad Dybcio wrote: > On 5/28/24 14:45, Marc Gonzalez wrote: > >> On 28/05/2024 14:31, Konrad Dybcio wrote: >> >>> [...] >>> >>>> + hdmi_cec_default: hdmi-cec-default-state { >>>> + pins = "gpio31"; >>>> + function = "hdmi_cec"; >>>> + drive-strength = <2>; >>>> + bias-pull-up; >>>> + }; >>>> + >>>> + hdmi_cec_sleep: hdmi-cec-sleep-state { >>>> + pins = "gpio31"; >>>> + function = "hdmi_cec"; >>>> + drive-strength = <2>; >>>> + bias-pull-up; >>>> + }; >>> >>> It's super strange that both states are identical. Usually, the pull-up >>> is disabled and the GPIO is unmuxed (i.e. function = "gpio"). If that's >>> not the case for whatever reason, you can drop the sleep variants and >>> simply reference the leftover one from both pinctrl-0 and pinctrl-1. >> >> Patch is a direct translation of the vendor code: >> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400 >> >> I suppose it wouldn't be the first time that vendor code >> is doing something odd. >> >> Though, I'm a bit confused why you would single out hdmi-cec >> when hdmi_ddc is the same? > > As in, me in the above message or the vendor devicetree? > > If the former, it's just an example > > If the latter, the muxing function differs so they must have their > own separate nodes I meant: You (rightly) point out that hdmi_cec_default & hdmi_cec_sleep nodes are identical in my patch. I stated that, in fact, hdmi_ddc_default & hdmi_ddc_sleep nodes are ALSO identical in my patch. And the reason they are identical in my patch is because they are identical in the vendor code downstream: mdss_hdmi_cec_active & mdss_hdmi_cec_suspend mdss_hdmi_ddc_active & mdss_hdmi_ddc_suspend If I understand correctly, you are saying I should delete hdmi_cec_sleep and hdmi_ddc_sleep nodes, and refer to the default nodes in the hdmi pinctrl-1 prop? Regards
On 5/28/24 15:10, Marc Gonzalez wrote: > On 28/05/2024 14:57, Konrad Dybcio wrote: > >> On 5/28/24 14:45, Marc Gonzalez wrote: >> >>> On 28/05/2024 14:31, Konrad Dybcio wrote: >>> >>>> [...] >>>> >>>>> + hdmi_cec_default: hdmi-cec-default-state { >>>>> + pins = "gpio31"; >>>>> + function = "hdmi_cec"; >>>>> + drive-strength = <2>; >>>>> + bias-pull-up; >>>>> + }; >>>>> + >>>>> + hdmi_cec_sleep: hdmi-cec-sleep-state { >>>>> + pins = "gpio31"; >>>>> + function = "hdmi_cec"; >>>>> + drive-strength = <2>; >>>>> + bias-pull-up; >>>>> + }; >>>> >>>> It's super strange that both states are identical. Usually, the pull-up >>>> is disabled and the GPIO is unmuxed (i.e. function = "gpio"). If that's >>>> not the case for whatever reason, you can drop the sleep variants and >>>> simply reference the leftover one from both pinctrl-0 and pinctrl-1. >>> >>> Patch is a direct translation of the vendor code: >>> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400 >>> >>> I suppose it wouldn't be the first time that vendor code >>> is doing something odd. >>> >>> Though, I'm a bit confused why you would single out hdmi-cec >>> when hdmi_ddc is the same? >> >> As in, me in the above message or the vendor devicetree? >> >> If the former, it's just an example >> >> If the latter, the muxing function differs so they must have their >> own separate nodes > > I meant: > > You (rightly) point out that > hdmi_cec_default & hdmi_cec_sleep nodes are identical > in my patch. > > I stated that, in fact, > hdmi_ddc_default & hdmi_ddc_sleep nodes are ALSO identical > in my patch. > > And the reason they are identical in my patch is because > they are identical in the vendor code downstream: > mdss_hdmi_cec_active & mdss_hdmi_cec_suspend > mdss_hdmi_ddc_active & mdss_hdmi_ddc_suspend > > > If I understand correctly, you are saying I should delete > hdmi_cec_sleep and hdmi_ddc_sleep nodes, and refer > to the default nodes in the hdmi pinctrl-1 prop? > yep :) Konrad
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>; + + 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"; + + 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>; + + #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>; + clock-names = + "iface", + "ref", + "xo"; + power-domains = <&rpmpd MSM8998_VDDMX>; + + status = "disabled"; + }; }; venus: video-codec@cc00000 {
On Thu, 30 May 2024 at 15:34, Marc Gonzalez <mgonzalez@freebox.fr> 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 { >
On 30/05/2024 15:06, Dmitry Baryshkov wrote: > This device was neither validated nor described properly in the DT > schema. There are several other issues here. Do you mean dtbs_check or dt_binding_check or something else? I think I changed everything you pointed out. (I tried to remain as close as possible to msm8996.) diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.yaml b/Documentation/devicetree/bindings/display/msm/hdmi.yaml index 47e97669821c3..9fc49ae9ee387 100644 --- a/Documentation/devicetree/bindings/display/msm/hdmi.yaml +++ b/Documentation/devicetree/bindings/display/msm/hdmi.yaml @@ -19,6 +19,7 @@ properties: - qcom,hdmi-tx-8974 - qcom,hdmi-tx-8994 - qcom,hdmi-tx-8996 + - qcom,hdmi-tx-8998 clocks: minItems: 1 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..268bb83efccce 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,90 @@ 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>; + + clocks = <&mmcc MDSS_MDP_CLK>, + <&mmcc MDSS_AHB_CLK>, + <&mmcc MDSS_HDMI_CLK>, + <&mmcc MDSS_HDMI_DP_AHB_CLK>, + <&mmcc MDSS_EXTPCLK_CLK>; + clock-names = + "mdp_core", + "iface", + "core", + "alt_iface", + "extp"; + + phys = <&hdmi_phy>; + #sound-dai-cells = <1>; + + 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>; + + 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>, + <&rpmcc RPM_SMD_XO_CLK_SRC>; + clock-names = "iface", + "ref", + "xo"; + + status = "disabled"; + }; }; venus: video-codec@cc00000 { I get /dev/tty1 on the TV. And the following command displays test patterns as expected: # modetest -Mmsm -a -s 33:#0 -P 34@82:1920x1080+0+0@XR24 -P 40@82:200x200+35+300@AR24 -P 46@82:200x200+310+300@AR24 setting mode 1920x1080-60.00Hz on connectors 33, crtc 82 testing 1920x1080@XR24 on plane 34, crtc 82 testing 200x200@AR24 on plane 40, crtc 82 testing 200x200@AR24 on plane 46, crtc 82 Regards
On Thu, 30 May 2024 at 19:45, Marc Gonzalez <mgonzalez@freebox.fr> wrote: > > On 30/05/2024 15:06, Dmitry Baryshkov wrote: > > > This device was neither validated nor described properly in the DT > > schema. There are several other issues here. > > Do you mean dtbs_check or dt_binding_check or something else? dtbs_check > > I think I changed everything you pointed out. > (I tried to remain as close as possible to msm8996.) Yes, this is better now. > > diff --git a/Documentation/devicetree/bindings/display/msm/hdmi.yaml b/Documentation/devicetree/bindings/display/msm/hdmi.yaml > index 47e97669821c3..9fc49ae9ee387 100644 > --- a/Documentation/devicetree/bindings/display/msm/hdmi.yaml > +++ b/Documentation/devicetree/bindings/display/msm/hdmi.yaml > @@ -19,6 +19,7 @@ properties: > - qcom,hdmi-tx-8974 > - qcom,hdmi-tx-8994 > - qcom,hdmi-tx-8996 > + - qcom,hdmi-tx-8998 If you scroll the file down, you'll see that this is not enough. > > clocks: > minItems: 1 > 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..268bb83efccce 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,90 @@ 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>; > + > + clocks = <&mmcc MDSS_MDP_CLK>, > + <&mmcc MDSS_AHB_CLK>, > + <&mmcc MDSS_HDMI_CLK>, > + <&mmcc MDSS_HDMI_DP_AHB_CLK>, > + <&mmcc MDSS_EXTPCLK_CLK>; > + clock-names = > + "mdp_core", > + "iface", > + "core", > + "alt_iface", > + "extp"; 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. > + > + phys = <&hdmi_phy>; > + #sound-dai-cells = <1>; > + > + 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>; > + > + 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>, > + <&rpmcc RPM_SMD_XO_CLK_SRC>; > + clock-names = "iface", > + "ref", > + "xo"; > + > + status = "disabled"; > + }; > }; > > venus: video-codec@cc00000 { > > > I get /dev/tty1 on the TV. > > And the following command displays test patterns as expected: > # modetest -Mmsm -a -s 33:#0 -P 34@82:1920x1080+0+0@XR24 -P 40@82:200x200+35+300@AR24 -P 46@82:200x200+310+300@AR24 > setting mode 1920x1080-60.00Hz on connectors 33, crtc 82 > testing 1920x1080@XR24 on plane 34, crtc 82 > testing 200x200@AR24 on plane 40, crtc 82 > testing 200x200@AR24 on plane 46, crtc 82 > > > Regards >
On 30.05.2024 3:06 PM, Dmitry Baryshkov wrote: [...] >> + power-domains = <&rpmpd MSM8998_VDDCX>; > > Is it? I don't see this in msm-4.4 Yes, it is. msm-4.4 says VDD_DIG which is &pm8998_s1_level which is CX As for the PHY, it's a safe guess to assume it's backed by MX. Maybe Jeff could chime in with a confirmation. Konrad
On Fri, 31 May 2024 at 15:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 30.05.2024 3:06 PM, Dmitry Baryshkov wrote: > > [...] > > >> + power-domains = <&rpmpd MSM8998_VDDCX>; > > > > Is it? I don't see this in msm-4.4 > > Yes, it is. msm-4.4 says VDD_DIG which is &pm8998_s1_level which is CX Oh, my... > > As for the PHY, it's a safe guess to assume it's backed by MX. Maybe > Jeff could chime in with a confirmation. > > Konrad
On 5/31/2024 7:55 AM, Dmitry Baryshkov wrote: > On Fri, 31 May 2024 at 15:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> On 30.05.2024 3:06 PM, Dmitry Baryshkov wrote: >> >> [...] >> >>>> + power-domains = <&rpmpd MSM8998_VDDCX>; >>> >>> Is it? I don't see this in msm-4.4 >> >> Yes, it is. msm-4.4 says VDD_DIG which is &pm8998_s1_level which is CX > > Oh, my... > >> >> As for the PHY, it's a safe guess to assume it's backed by MX. Maybe >> Jeff could chime in with a confirmation. Sounds right, but I'm not finding anything in the documentation. -Jeff
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? 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";
On Mon, 3 Jun 2024 at 16:06, Marc Gonzalez <mgonzalez@freebox.fr> 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"; >
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi index edf379c28e1e1..ec4e967ed9b2a 100644 --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi @@ -1424,6 +1424,48 @@ 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_cec_sleep: hdmi-cec-sleep-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_ddc_sleep: hdmi-ddc-sleep-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 {
MSM8998 GPIO pin controller reference design defines: - CEC: pin 31 - DDC: pin 32,33 - HPD: pin 34 Downstream vendor code for reference: https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi#L2324-2400 mdss_hdmi_{cec,ddc,hpd}_{active,suspend} Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> --- arch/arm64/boot/dts/qcom/msm8998.dtsi | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)