| Message ID | 20240916091344.27607-1-quic_mukhopad@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arm64: dts: qcom: sa8775p: add DisplayPort device node | expand |
On Mon, Sep 16, 2024 at 02:43:44PM GMT, Soutrik Mukhopadhyay wrote: > Add device tree node for the DisplayPort controller > and eDP PHY found on the Qualcomm SA8775P SoC. Not quite. You are also enabling it for the RIDE platforms, not just the SA8775p SoC. > > Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com> > --- > This patch depends on following series: > https://lore.kernel.org/all/20240816-sa8775p-mm-v3-v1-0-77d53c3c0cef@quicinc.com/ > https://lore.kernel.org/all/20240912071437.1708969-1-quic_mahap@quicinc.com/ > https://lore.kernel.org/all/20240913103755.7290-1-quic_mukhopad@quicinc.com/ Also please provide mdss_dp1 device nodes, you have documented them in the patch "drm/msm/dp: Add DisplayPort controller for SA8775P" > --- > arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 23 +++++ > arch/arm64/boot/dts/qcom/sa8775p.dtsi | 114 ++++++++++++++++++++- > 2 files changed, 136 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi > index 0c1b21def4b6..728b4cda8353 100644 > --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi > +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi > @@ -421,6 +421,23 @@ > status = "okay"; > }; > > +&mdss0 { > + status = "okay"; > +}; > + > +&mdss0_dp0 { > + status = "okay"; > +}; > + > +&mdss0_dp0_out { > + data-lanes = <0 1 2 3>; > + link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>; > +}; > + > +&mdss0_edp_phy0 { > + status = "okay"; > +}; > + > &pmm8654au_0_gpios { > gpio-line-names = "DS_EN", > "POFF_COMPLETE", > @@ -527,6 +544,12 @@ > }; > > &tlmm { > + dp_hot_plug_det: dp-hot-plug-det-state { > + pins = "gpio101"; > + function = "edp0_hot"; > + bias-disable; > + }; > + > ethernet0_default: ethernet0-default-state { > ethernet0_mdc: ethernet0-mdc-pins { > pins = "gpio8"; > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi > index 7747965e7e46..a04150c29565 100644 > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi > @@ -3339,6 +3339,18 @@ > interrupt-parent = <&mdss0>; > interrupts = <0>; > > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dpu_intf0_out: endpoint { > + remote-endpoint = <&mdss0_dp0_in>; > + }; > + }; > + }; > + > mdss0_mdp_opp_table: opp-table { > compatible = "operating-points-v2"; > > @@ -3363,6 +3375,106 @@ > }; > }; > }; > + > + mdss0_edp_phy0: phy@aec2a00 { > + compatible = "qcom,sa8775p-edp-phy"; > + > + reg = <0x0 0xaec2a00 0x0 0x200>, > + <0x0 0xaec2200 0x0 0xd0>, > + <0x0 0xaec2600 0x0 0xd0>, > + <0x0 0xaec2000 0x0 0x1c8>; Please ident on the angle bracket. > + > + clocks = <&rpmhcc RPMH_CXO_CLK>, It it really CXO? > + <&gcc GCC_EDP_REF_CLKREF_EN>; And this isn't cfg_ahb. > + clock-names = "aux", > + "cfg_ahb"; > + > + vdda-phy-supply = <&vreg_l1c>; > + vdda-pll-supply = <&vreg_l4a>; regulators are not a part of the SoC > + #clock-cells = <1>; > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > + mdss0_dp0: displayport-controller@af54000 { > + compatible = "qcom,sa8775p-dp"; > + > + pinctrl-0 = <&dp_hot_plug_det>; > + pinctrl-names = "default"; > + > + reg = <0x0 0xaf54000 0x0 0x104>, > + <0x0 0xaf54200 0x0 0x0c0>, > + <0x0 0xaf55000 0x0 0x770>, > + <0x0 0xaf56000 0x0 0x09c>; Wrong indentation. > + > + interrupt-parent = <&mdss0>; > + interrupts = <12>; > + > + clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_AUX_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK>; > + clock-names = "core_iface", > + "core_aux", > + "ctrl_link", > + "ctrl_link_iface", > + "stream_pixel"; And here. > + assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>; > + assigned-clock-parents = <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>; > + phys = <&mdss0_edp_phy0>; > + phy-names = "dp"; > + > + operating-points-v2 = <&dp_opp_table>; > + power-domains = <&rpmhpd SA8775P_MMCX>; > + > + #sound-dai-cells = <0>; > + > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + mdss0_dp0_in: endpoint { > + remote-endpoint = <&dpu_intf0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + mdss0_dp0_out: endpoint { }; > + }; > + }; > + > + dp_opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + opp-160000000 { > + opp-hz = /bits/ 64 <160000000>; > + required-opps = <&rpmhpd_opp_low_svs>; > + }; > + > + opp-270000000 { > + opp-hz = /bits/ 64 <270000000>; > + required-opps = <&rpmhpd_opp_svs>; > + }; > + > + opp-540000000 { > + opp-hz = /bits/ 64 <540000000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + }; > + > + opp-810000000 { > + opp-hz = /bits/ 64 <810000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + }; > + }; > }; > > dispcc0: clock-controller@af00000 { > @@ -3372,7 +3484,7 @@ > <&rpmhcc RPMH_CXO_CLK>, > <&rpmhcc RPMH_CXO_CLK_A>, > <&sleep_clk>, > - <0>, <0>, <0>, <0>, > + <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>, <0>, <0>, > <0>, <0>, <0>, <0>; > power-domains = <&rpmhpd SA8775P_MMCX>; > #clock-cells = <1>; > -- > 2.17.1 >
On 16.09.2024 4:01 PM, Dmitry Baryshkov wrote: > On Mon, Sep 16, 2024 at 02:43:44PM GMT, Soutrik Mukhopadhyay wrote: >> Add device tree node for the DisplayPort controller >> and eDP PHY found on the Qualcomm SA8775P SoC. > > Not quite. You are also enabling it for the RIDE platforms, not just the > SA8775p SoC. (the patches should be split into soc and board parts) [...] >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + dpu_intf0_out: endpoint { Please add a newline between the last property and the subnode Konrad
On Mon, Sep 16, 2024 at 02:43:44PM GMT, Soutrik Mukhopadhyay wrote: > Add device tree node for the DisplayPort controller > and eDP PHY found on the Qualcomm SA8775P SoC. > Please split this in a change for the platform (.dtsi) which defines _all_ the DPTX and DP PHYs, and then a ride dts change which enables all the ports available on the Ride. If there are platform ports that are not accessible on any hardware, state in the commit message which ones you tested and which ones you didn't test. > Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com> > --- > This patch depends on following series: > https://lore.kernel.org/all/20240816-sa8775p-mm-v3-v1-0-77d53c3c0cef@quicinc.com/ > https://lore.kernel.org/all/20240912071437.1708969-1-quic_mahap@quicinc.com/ > https://lore.kernel.org/all/20240913103755.7290-1-quic_mukhopad@quicinc.com/ Please hold off resubmitting this series until there's conclusion on these dependencies. > > --- > arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 23 +++++ > arch/arm64/boot/dts/qcom/sa8775p.dtsi | 114 ++++++++++++++++++++- > 2 files changed, 136 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi > index 0c1b21def4b6..728b4cda8353 100644 > --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi > +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi > @@ -421,6 +421,23 @@ > status = "okay"; > }; > > +&mdss0 { > + status = "okay"; > +}; > + > +&mdss0_dp0 { > + status = "okay"; > +}; > + > +&mdss0_dp0_out { > + data-lanes = <0 1 2 3>; > + link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>; > +}; > + > +&mdss0_edp_phy0 { > + status = "okay"; > +}; > + > &pmm8654au_0_gpios { > gpio-line-names = "DS_EN", > "POFF_COMPLETE", > @@ -527,6 +544,12 @@ > }; > > &tlmm { > + dp_hot_plug_det: dp-hot-plug-det-state { > + pins = "gpio101"; > + function = "edp0_hot"; > + bias-disable; > + }; > + > ethernet0_default: ethernet0-default-state { > ethernet0_mdc: ethernet0-mdc-pins { > pins = "gpio8"; > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi > index 7747965e7e46..a04150c29565 100644 > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi > @@ -3339,6 +3339,18 @@ > interrupt-parent = <&mdss0>; > interrupts = <0>; > > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + dpu_intf0_out: endpoint { > + remote-endpoint = <&mdss0_dp0_in>; > + }; > + }; > + }; > + > mdss0_mdp_opp_table: opp-table { > compatible = "operating-points-v2"; > > @@ -3363,6 +3375,106 @@ > }; > }; > }; > + > + mdss0_edp_phy0: phy@aec2a00 { > + compatible = "qcom,sa8775p-edp-phy"; Is this really a eDP PHY, not a DP/eDP combo phy? I would prefer that we keep the label prefix "mdssM_dpN" on the DP TX and DP PHY nodes, to keep them neatly sorted in the dts. (If you name half mdssM_edp_phyN and half mdssM_dp_phyN we're going to have a mess) > + > + reg = <0x0 0xaec2a00 0x0 0x200>, > + <0x0 0xaec2200 0x0 0xd0>, > + <0x0 0xaec2600 0x0 0xd0>, > + <0x0 0xaec2000 0x0 0x1c8>; > + > + clocks = <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_EDP_REF_CLKREF_EN>; > + clock-names = "aux", > + "cfg_ahb"; > + > + vdda-phy-supply = <&vreg_l1c>; > + vdda-pll-supply = <&vreg_l4a>; > + #clock-cells = <1>; > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > + mdss0_dp0: displayport-controller@af54000 { > + compatible = "qcom,sa8775p-dp"; > + > + pinctrl-0 = <&dp_hot_plug_det>; Don't make references from .dtsi to labels defined in .dts. Regards, Bjorn > + pinctrl-names = "default"; > + > + reg = <0x0 0xaf54000 0x0 0x104>, > + <0x0 0xaf54200 0x0 0x0c0>, > + <0x0 0xaf55000 0x0 0x770>, > + <0x0 0xaf56000 0x0 0x09c>; > + > + interrupt-parent = <&mdss0>; > + interrupts = <12>; > + > + clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_AUX_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK>; > + clock-names = "core_iface", > + "core_aux", > + "ctrl_link", > + "ctrl_link_iface", > + "stream_pixel"; > + assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>, > + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>; > + assigned-clock-parents = <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>; > + phys = <&mdss0_edp_phy0>; > + phy-names = "dp"; > + > + operating-points-v2 = <&dp_opp_table>; > + power-domains = <&rpmhpd SA8775P_MMCX>; > + > + #sound-dai-cells = <0>; > + > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + mdss0_dp0_in: endpoint { > + remote-endpoint = <&dpu_intf0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + mdss0_dp0_out: endpoint { }; > + }; > + }; > + > + dp_opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + opp-160000000 { > + opp-hz = /bits/ 64 <160000000>; > + required-opps = <&rpmhpd_opp_low_svs>; > + }; > + > + opp-270000000 { > + opp-hz = /bits/ 64 <270000000>; > + required-opps = <&rpmhpd_opp_svs>; > + }; > + > + opp-540000000 { > + opp-hz = /bits/ 64 <540000000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + }; > + > + opp-810000000 { > + opp-hz = /bits/ 64 <810000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + }; > + }; > }; > > dispcc0: clock-controller@af00000 { > @@ -3372,7 +3484,7 @@ > <&rpmhcc RPMH_CXO_CLK>, > <&rpmhcc RPMH_CXO_CLK_A>, > <&sleep_clk>, > - <0>, <0>, <0>, <0>, > + <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>, <0>, <0>, > <0>, <0>, <0>, <0>; > power-domains = <&rpmhpd SA8775P_MMCX>; > #clock-cells = <1>; > -- > 2.17.1 >
On 9/16/2024 7:31 PM, Dmitry Baryshkov wrote: > On Mon, Sep 16, 2024 at 02:43:44PM GMT, Soutrik Mukhopadhyay wrote: >> Add device tree node for the DisplayPort controller >> and eDP PHY found on the Qualcomm SA8775P SoC. > Not quite. You are also enabling it for the RIDE platforms, not just the > SA8775p SoC. Sure , we will update this accordingly in the next version. > >> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com> >> --- >> This patch depends on following series: >> https://lore.kernel.org/all/20240816-sa8775p-mm-v3-v1-0-77d53c3c0cef@quicinc.com/ >> https://lore.kernel.org/all/20240912071437.1708969-1-quic_mahap@quicinc.com/ >> https://lore.kernel.org/all/20240913103755.7290-1-quic_mukhopad@quicinc.com/ > Also please provide mdss_dp1 device nodes, you have documented them in > the patch "drm/msm/dp: Add DisplayPort controller for SA8775P" Sure, we will include both the mdss_dp0 and mdss_dp1 device nodes, as per the documentation, in the next version. > >> --- >> arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 23 +++++ >> arch/arm64/boot/dts/qcom/sa8775p.dtsi | 114 ++++++++++++++++++++- >> 2 files changed, 136 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi >> index 0c1b21def4b6..728b4cda8353 100644 >> --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi >> @@ -421,6 +421,23 @@ >> status = "okay"; >> }; >> >> +&mdss0 { >> + status = "okay"; >> +}; >> + >> +&mdss0_dp0 { >> + status = "okay"; >> +}; >> + >> +&mdss0_dp0_out { >> + data-lanes = <0 1 2 3>; >> + link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>; >> +}; >> + >> +&mdss0_edp_phy0 { >> + status = "okay"; >> +}; >> + >> &pmm8654au_0_gpios { >> gpio-line-names = "DS_EN", >> "POFF_COMPLETE", >> @@ -527,6 +544,12 @@ >> }; >> >> &tlmm { >> + dp_hot_plug_det: dp-hot-plug-det-state { >> + pins = "gpio101"; >> + function = "edp0_hot"; >> + bias-disable; >> + }; >> + >> ethernet0_default: ethernet0-default-state { >> ethernet0_mdc: ethernet0-mdc-pins { >> pins = "gpio8"; >> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi >> index 7747965e7e46..a04150c29565 100644 >> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi >> @@ -3339,6 +3339,18 @@ >> interrupt-parent = <&mdss0>; >> interrupts = <0>; >> >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + dpu_intf0_out: endpoint { >> + remote-endpoint = <&mdss0_dp0_in>; >> + }; >> + }; >> + }; >> + >> mdss0_mdp_opp_table: opp-table { >> compatible = "operating-points-v2"; >> >> @@ -3363,6 +3375,106 @@ >> }; >> }; >> }; >> + >> + mdss0_edp_phy0: phy@aec2a00 { >> + compatible = "qcom,sa8775p-edp-phy"; >> + >> + reg = <0x0 0xaec2a00 0x0 0x200>, >> + <0x0 0xaec2200 0x0 0xd0>, >> + <0x0 0xaec2600 0x0 0xd0>, >> + <0x0 0xaec2000 0x0 0x1c8>; > Please ident on the angle bracket. Sure, we will update this in the next version. > >> + >> + clocks = <&rpmhcc RPMH_CXO_CLK>, > It it really CXO? Sure, we will address the necessary clock changes for both "aux" and "cfg_ahb" in the next version. > >> + <&gcc GCC_EDP_REF_CLKREF_EN>; > And this isn't cfg_ahb. > >> + clock-names = "aux", >> + "cfg_ahb"; >> + >> + vdda-phy-supply = <&vreg_l1c>; >> + vdda-pll-supply = <&vreg_l4a>; > regulators are not a part of the SoC Sure, we will move the regulators to a different file where all the board specific changes are a part of, in the next version. > >> + #clock-cells = <1>; >> + #phy-cells = <0>; >> + >> + status = "disabled"; >> + }; >> + >> + mdss0_dp0: displayport-controller@af54000 { >> + compatible = "qcom,sa8775p-dp"; >> + >> + pinctrl-0 = <&dp_hot_plug_det>; >> + pinctrl-names = "default"; >> + >> + reg = <0x0 0xaf54000 0x0 0x104>, >> + <0x0 0xaf54200 0x0 0x0c0>, >> + <0x0 0xaf55000 0x0 0x770>, >> + <0x0 0xaf56000 0x0 0x09c>; > Wrong indentation. Sure, we will update with the correct indentation at all places in the next version. > >> + >> + interrupt-parent = <&mdss0>; >> + interrupts = <12>; >> + >> + clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_AUX_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK>; >> + clock-names = "core_iface", >> + "core_aux", >> + "ctrl_link", >> + "ctrl_link_iface", >> + "stream_pixel"; > And here. > >> + assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>; >> + assigned-clock-parents = <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>; >> + phys = <&mdss0_edp_phy0>; >> + phy-names = "dp"; >> + >> + operating-points-v2 = <&dp_opp_table>; >> + power-domains = <&rpmhpd SA8775P_MMCX>; >> + >> + #sound-dai-cells = <0>; >> + >> + status = "disabled"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + mdss0_dp0_in: endpoint { >> + remote-endpoint = <&dpu_intf0_out>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + mdss0_dp0_out: endpoint { }; >> + }; >> + }; >> + >> + dp_opp_table: opp-table { >> + compatible = "operating-points-v2"; >> + >> + opp-160000000 { >> + opp-hz = /bits/ 64 <160000000>; >> + required-opps = <&rpmhpd_opp_low_svs>; >> + }; >> + >> + opp-270000000 { >> + opp-hz = /bits/ 64 <270000000>; >> + required-opps = <&rpmhpd_opp_svs>; >> + }; >> + >> + opp-540000000 { >> + opp-hz = /bits/ 64 <540000000>; >> + required-opps = <&rpmhpd_opp_svs_l1>; >> + }; >> + >> + opp-810000000 { >> + opp-hz = /bits/ 64 <810000000>; >> + required-opps = <&rpmhpd_opp_nom>; >> + }; >> + }; >> + }; >> }; >> >> dispcc0: clock-controller@af00000 { >> @@ -3372,7 +3484,7 @@ >> <&rpmhcc RPMH_CXO_CLK>, >> <&rpmhcc RPMH_CXO_CLK_A>, >> <&sleep_clk>, >> - <0>, <0>, <0>, <0>, >> + <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>, <0>, <0>, >> <0>, <0>, <0>, <0>; >> power-domains = <&rpmhpd SA8775P_MMCX>; >> #clock-cells = <1>; >> -- >> 2.17.1 >>
On 9/17/2024 5:05 AM, Konrad Dybcio wrote: > On 16.09.2024 4:01 PM, Dmitry Baryshkov wrote: >> On Mon, Sep 16, 2024 at 02:43:44PM GMT, Soutrik Mukhopadhyay wrote: >>> Add device tree node for the DisplayPort controller >>> and eDP PHY found on the Qualcomm SA8775P SoC. >> Not quite. You are also enabling it for the RIDE platforms, not just the >> SA8775p SoC. > (the patches should be split into soc and board parts) > > [...] Sure, we will be splitting the board and the soc parts into different patches in the next version. > >>> + ports { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + port@0 { >>> + reg = <0>; >>> + dpu_intf0_out: endpoint { > Please add a newline between the last property and the subnode > > Konrad Sure, we will update this in the next version.
On 9/17/2024 12:59 PM, Bjorn Andersson wrote: > On Mon, Sep 16, 2024 at 02:43:44PM GMT, Soutrik Mukhopadhyay wrote: >> Add device tree node for the DisplayPort controller >> and eDP PHY found on the Qualcomm SA8775P SoC. >> > Please split this in a change for the platform (.dtsi) which defines > _all_ the DPTX and DP PHYs, and then a ride dts change which enables all > the ports available on the Ride. > > If there are platform ports that are not accessible on any hardware, > state in the commit message which ones you tested and which ones you > didn't test. Sure, we will be splitting the change for platform, consisting of all the DPTX and DP PHY definitions and a ride dts, for enabling the ports available, in two different patches with updated commit messages, in the next version. > >> Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com> >> --- >> This patch depends on following series: >> https://lore.kernel.org/all/20240816-sa8775p-mm-v3-v1-0-77d53c3c0cef@quicinc.com/ >> https://lore.kernel.org/all/20240912071437.1708969-1-quic_mahap@quicinc.com/ >> https://lore.kernel.org/all/20240913103755.7290-1-quic_mukhopad@quicinc.com/ > Please hold off resubmitting this series until there's conclusion on > these dependencies. > >> >> --- >> arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 23 +++++ >> arch/arm64/boot/dts/qcom/sa8775p.dtsi | 114 ++++++++++++++++++++- >> 2 files changed, 136 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi >> index 0c1b21def4b6..728b4cda8353 100644 >> --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi >> @@ -421,6 +421,23 @@ >> status = "okay"; >> }; >> >> +&mdss0 { >> + status = "okay"; >> +}; >> + >> +&mdss0_dp0 { >> + status = "okay"; >> +}; >> + >> +&mdss0_dp0_out { >> + data-lanes = <0 1 2 3>; >> + link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>; >> +}; >> + >> +&mdss0_edp_phy0 { >> + status = "okay"; >> +}; >> + >> &pmm8654au_0_gpios { >> gpio-line-names = "DS_EN", >> "POFF_COMPLETE", >> @@ -527,6 +544,12 @@ >> }; >> >> &tlmm { >> + dp_hot_plug_det: dp-hot-plug-det-state { >> + pins = "gpio101"; >> + function = "edp0_hot"; >> + bias-disable; >> + }; >> + >> ethernet0_default: ethernet0-default-state { >> ethernet0_mdc: ethernet0-mdc-pins { >> pins = "gpio8"; >> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi >> index 7747965e7e46..a04150c29565 100644 >> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi >> @@ -3339,6 +3339,18 @@ >> interrupt-parent = <&mdss0>; >> interrupts = <0>; >> >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + dpu_intf0_out: endpoint { >> + remote-endpoint = <&mdss0_dp0_in>; >> + }; >> + }; >> + }; >> + >> mdss0_mdp_opp_table: opp-table { >> compatible = "operating-points-v2"; >> >> @@ -3363,6 +3375,106 @@ >> }; >> }; >> }; >> + >> + mdss0_edp_phy0: phy@aec2a00 { >> + compatible = "qcom,sa8775p-edp-phy"; > Is this really a eDP PHY, not a DP/eDP combo phy? > > I would prefer that we keep the label prefix "mdssM_dpN" on the DP TX > and DP PHY nodes, to keep them neatly sorted in the dts. (If you name > half mdssM_edp_phyN and half mdssM_dp_phyN we're going to have a mess) Can we use the following naming conventions for clarity : mdssM_dpN_phy to refer to any DP PHY nodes and mdssM_dpN to refer to DP TX nodes. For example, the phy being used by dp0 on mdss0 can be referred by mdss0_dp0_phy, whereas the phy being used by dp1 on mdss0 can be referred by mdss0_dp1_phy. > >> + >> + reg = <0x0 0xaec2a00 0x0 0x200>, >> + <0x0 0xaec2200 0x0 0xd0>, >> + <0x0 0xaec2600 0x0 0xd0>, >> + <0x0 0xaec2000 0x0 0x1c8>; >> + >> + clocks = <&rpmhcc RPMH_CXO_CLK>, >> + <&gcc GCC_EDP_REF_CLKREF_EN>; >> + clock-names = "aux", >> + "cfg_ahb"; >> + >> + vdda-phy-supply = <&vreg_l1c>; >> + vdda-pll-supply = <&vreg_l4a>; >> + #clock-cells = <1>; >> + #phy-cells = <0>; >> + >> + status = "disabled"; >> + }; >> + >> + mdss0_dp0: displayport-controller@af54000 { >> + compatible = "qcom,sa8775p-dp"; >> + >> + pinctrl-0 = <&dp_hot_plug_det>; > Don't make references from .dtsi to labels defined in .dts. > > Regards, > Bjorn Sure, we will make the necessary changes in the next version. > >> + pinctrl-names = "default"; >> + >> + reg = <0x0 0xaf54000 0x0 0x104>, >> + <0x0 0xaf54200 0x0 0x0c0>, >> + <0x0 0xaf55000 0x0 0x770>, >> + <0x0 0xaf56000 0x0 0x09c>; >> + >> + interrupt-parent = <&mdss0>; >> + interrupts = <12>; >> + >> + clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_AUX_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK>; >> + clock-names = "core_iface", >> + "core_aux", >> + "ctrl_link", >> + "ctrl_link_iface", >> + "stream_pixel"; >> + assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>, >> + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>; >> + assigned-clock-parents = <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>; >> + phys = <&mdss0_edp_phy0>; >> + phy-names = "dp"; >> + >> + operating-points-v2 = <&dp_opp_table>; >> + power-domains = <&rpmhpd SA8775P_MMCX>; >> + >> + #sound-dai-cells = <0>; >> + >> + status = "disabled"; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + mdss0_dp0_in: endpoint { >> + remote-endpoint = <&dpu_intf0_out>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + mdss0_dp0_out: endpoint { }; >> + }; >> + }; >> + >> + dp_opp_table: opp-table { >> + compatible = "operating-points-v2"; >> + >> + opp-160000000 { >> + opp-hz = /bits/ 64 <160000000>; >> + required-opps = <&rpmhpd_opp_low_svs>; >> + }; >> + >> + opp-270000000 { >> + opp-hz = /bits/ 64 <270000000>; >> + required-opps = <&rpmhpd_opp_svs>; >> + }; >> + >> + opp-540000000 { >> + opp-hz = /bits/ 64 <540000000>; >> + required-opps = <&rpmhpd_opp_svs_l1>; >> + }; >> + >> + opp-810000000 { >> + opp-hz = /bits/ 64 <810000000>; >> + required-opps = <&rpmhpd_opp_nom>; >> + }; >> + }; >> + }; >> }; >> >> dispcc0: clock-controller@af00000 { >> @@ -3372,7 +3484,7 @@ >> <&rpmhcc RPMH_CXO_CLK>, >> <&rpmhcc RPMH_CXO_CLK_A>, >> <&sleep_clk>, >> - <0>, <0>, <0>, <0>, >> + <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>, <0>, <0>, >> <0>, <0>, <0>, <0>; >> power-domains = <&rpmhpd SA8775P_MMCX>; >> #clock-cells = <1>; >> -- >> 2.17.1 >>
diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi index 0c1b21def4b6..728b4cda8353 100644 --- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi +++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi @@ -421,6 +421,23 @@ status = "okay"; }; +&mdss0 { + status = "okay"; +}; + +&mdss0_dp0 { + status = "okay"; +}; + +&mdss0_dp0_out { + data-lanes = <0 1 2 3>; + link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>; +}; + +&mdss0_edp_phy0 { + status = "okay"; +}; + &pmm8654au_0_gpios { gpio-line-names = "DS_EN", "POFF_COMPLETE", @@ -527,6 +544,12 @@ }; &tlmm { + dp_hot_plug_det: dp-hot-plug-det-state { + pins = "gpio101"; + function = "edp0_hot"; + bias-disable; + }; + ethernet0_default: ethernet0-default-state { ethernet0_mdc: ethernet0-mdc-pins { pins = "gpio8"; diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi index 7747965e7e46..a04150c29565 100644 --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi @@ -3339,6 +3339,18 @@ interrupt-parent = <&mdss0>; interrupts = <0>; + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dpu_intf0_out: endpoint { + remote-endpoint = <&mdss0_dp0_in>; + }; + }; + }; + mdss0_mdp_opp_table: opp-table { compatible = "operating-points-v2"; @@ -3363,6 +3375,106 @@ }; }; }; + + mdss0_edp_phy0: phy@aec2a00 { + compatible = "qcom,sa8775p-edp-phy"; + + reg = <0x0 0xaec2a00 0x0 0x200>, + <0x0 0xaec2200 0x0 0xd0>, + <0x0 0xaec2600 0x0 0xd0>, + <0x0 0xaec2000 0x0 0x1c8>; + + clocks = <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_EDP_REF_CLKREF_EN>; + clock-names = "aux", + "cfg_ahb"; + + vdda-phy-supply = <&vreg_l1c>; + vdda-pll-supply = <&vreg_l4a>; + #clock-cells = <1>; + #phy-cells = <0>; + + status = "disabled"; + }; + + mdss0_dp0: displayport-controller@af54000 { + compatible = "qcom,sa8775p-dp"; + + pinctrl-0 = <&dp_hot_plug_det>; + pinctrl-names = "default"; + + reg = <0x0 0xaf54000 0x0 0x104>, + <0x0 0xaf54200 0x0 0x0c0>, + <0x0 0xaf55000 0x0 0x770>, + <0x0 0xaf56000 0x0 0x09c>; + + interrupt-parent = <&mdss0>; + interrupts = <12>; + + clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>, + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_AUX_CLK>, + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK>, + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>, + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK>; + clock-names = "core_iface", + "core_aux", + "ctrl_link", + "ctrl_link_iface", + "stream_pixel"; + assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>, + <&dispcc0 MDSS_DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>; + assigned-clock-parents = <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>; + phys = <&mdss0_edp_phy0>; + phy-names = "dp"; + + operating-points-v2 = <&dp_opp_table>; + power-domains = <&rpmhpd SA8775P_MMCX>; + + #sound-dai-cells = <0>; + + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + mdss0_dp0_in: endpoint { + remote-endpoint = <&dpu_intf0_out>; + }; + }; + + port@1 { + reg = <1>; + mdss0_dp0_out: endpoint { }; + }; + }; + + dp_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-160000000 { + opp-hz = /bits/ 64 <160000000>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-270000000 { + opp-hz = /bits/ 64 <270000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-540000000 { + opp-hz = /bits/ 64 <540000000>; + required-opps = <&rpmhpd_opp_svs_l1>; + }; + + opp-810000000 { + opp-hz = /bits/ 64 <810000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + }; + }; }; dispcc0: clock-controller@af00000 { @@ -3372,7 +3484,7 @@ <&rpmhcc RPMH_CXO_CLK>, <&rpmhcc RPMH_CXO_CLK_A>, <&sleep_clk>, - <0>, <0>, <0>, <0>, + <&mdss0_edp_phy0 0>, <&mdss0_edp_phy0 1>, <0>, <0>, <0>, <0>, <0>, <0>; power-domains = <&rpmhpd SA8775P_MMCX>; #clock-cells = <1>;
Add device tree node for the DisplayPort controller and eDP PHY found on the Qualcomm SA8775P SoC. Signed-off-by: Soutrik Mukhopadhyay <quic_mukhopad@quicinc.com> --- This patch depends on following series: https://lore.kernel.org/all/20240816-sa8775p-mm-v3-v1-0-77d53c3c0cef@quicinc.com/ https://lore.kernel.org/all/20240912071437.1708969-1-quic_mahap@quicinc.com/ https://lore.kernel.org/all/20240913103755.7290-1-quic_mukhopad@quicinc.com/ --- arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 23 +++++ arch/arm64/boot/dts/qcom/sa8775p.dtsi | 114 ++++++++++++++++++++- 2 files changed, 136 insertions(+), 1 deletion(-)