| Message ID | 20220906183334.203787-4-luca@z3ntu.xyz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MDSS support for MSM8953 | expand |
On Tue, 6 Sept 2022 at 21:36, Luca Weiss <luca@z3ntu.xyz> wrote: > > From: Vladimir Lypak <vladimir.lypak@gmail.com> > > Add the MDSS, MDP and DSI nodes that are found on msm8953 SoC. > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- > Changes since v2: > - add "core" clock for mdss as suggested by Dmitry Baryshkov > > arch/arm64/boot/dts/qcom/msm8953.dtsi | 210 ++++++++++++++++++++++++++ > 1 file changed, 210 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi b/arch/arm64/boot/dts/qcom/msm8953.dtsi > index 3d11331e78d2..580333141a66 100644 > --- a/arch/arm64/boot/dts/qcom/msm8953.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi > @@ -726,6 +726,216 @@ tcsr_phy_clk_scheme_sel: syscon@193f044 { > reg = <0x193f044 0x4>; > }; > > + mdss: mdss@1a00000 { > + compatible = "qcom,mdss"; > + > + reg = <0x1a00000 0x1000>, > + <0x1ab0000 0x1040>; > + reg-names = "mdss_phys", > + "vbif_phys"; > + > + power-domains = <&gcc MDSS_GDSC>; > + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; > + > + interrupt-controller; > + #interrupt-cells = <1>; > + > + clocks = <&gcc GCC_MDSS_AHB_CLK>, > + <&gcc GCC_MDSS_AXI_CLK>, > + <&gcc GCC_MDSS_VSYNC_CLK>, > + <&gcc GCC_MDSS_MDP_CLK>; > + clock-names = "iface", > + "bus", > + "vsync", > + "core"; > + > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + status = "disabled"; > + > + mdp: mdp@1a01000 { > + compatible = "qcom,mdp5"; Could you please change this to "qcom,msm8953-mdp5", "qcom,mdp5". > + reg = <0x1a01000 0x89000>; > + reg-names = "mdp_phys"; > + [skipped] > + > + dsi0_phy: dsi-phy@1a94400 { Let's probably use a generic name 'phy' here and for dsi1_phy. The rest looks good to me. > + compatible = "qcom,dsi-phy-14nm-8953"; > + reg = <0x1a94400 0x100>, > + <0x1a94500 0x300>, > + <0x1a94800 0x188>; > + reg-names = "dsi_phy", > + "dsi_phy_lane", > + "dsi_pll"; > + > + #clock-cells = <1>; > + #phy-cells = <0>; > + > + clocks = <&gcc GCC_MDSS_AHB_CLK>, <&xo_board>; > + clock-names = "iface", "ref"; > + > + status = "disabled"; > + };
Hi Dmitry, On Dienstag, 6. September 2022 21:41:11 CEST Dmitry Baryshkov wrote: > On Tue, 6 Sept 2022 at 21:36, Luca Weiss <luca@z3ntu.xyz> wrote: > > From: Vladimir Lypak <vladimir.lypak@gmail.com> > > > > Add the MDSS, MDP and DSI nodes that are found on msm8953 SoC. > > > > Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > > --- > > Changes since v2: > > - add "core" clock for mdss as suggested by Dmitry Baryshkov > > > > arch/arm64/boot/dts/qcom/msm8953.dtsi | 210 ++++++++++++++++++++++++++ > > 1 file changed, 210 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi > > b/arch/arm64/boot/dts/qcom/msm8953.dtsi index 3d11331e78d2..580333141a66 > > 100644 > > --- a/arch/arm64/boot/dts/qcom/msm8953.dtsi > > +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi > > @@ -726,6 +726,216 @@ tcsr_phy_clk_scheme_sel: syscon@193f044 { > > > > reg = <0x193f044 0x4>; > > > > }; > > > > + mdss: mdss@1a00000 { > > + compatible = "qcom,mdss"; > > + > > + reg = <0x1a00000 0x1000>, > > + <0x1ab0000 0x1040>; > > + reg-names = "mdss_phys", > > + "vbif_phys"; > > + > > + power-domains = <&gcc MDSS_GDSC>; > > + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; > > + > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + > > + clocks = <&gcc GCC_MDSS_AHB_CLK>, > > + <&gcc GCC_MDSS_AXI_CLK>, > > + <&gcc GCC_MDSS_VSYNC_CLK>, > > + <&gcc GCC_MDSS_MDP_CLK>; > > + clock-names = "iface", > > + "bus", > > + "vsync", > > + "core"; > > + > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + status = "disabled"; > > + > > + mdp: mdp@1a01000 { > > + compatible = "qcom,mdp5"; > > Could you please change this to "qcom,msm8953-mdp5", "qcom,mdp5". This would be the first dtsi using the two compatibles then, correct? Are there any plans to adjust other SoCs? > > > + reg = <0x1a01000 0x89000>; > > + reg-names = "mdp_phys"; > > + > > [skipped] > > > + > > + dsi0_phy: dsi-phy@1a94400 { > > Let's probably use a generic name 'phy' here and for dsi1_phy. Here also, the bindings examples all use dsi-phy@, are there any plans to change that and adjust other dtsi files? > > The rest looks good to me. Thanks! Regards Luca > > > + compatible = "qcom,dsi-phy-14nm-8953"; > > + reg = <0x1a94400 0x100>, > > + <0x1a94500 0x300>, > > + <0x1a94800 0x188>; > > + reg-names = "dsi_phy", > > + "dsi_phy_lane", > > + "dsi_pll"; > > + > > + #clock-cells = <1>; > > + #phy-cells = <0>; > > + > > + clocks = <&gcc GCC_MDSS_AHB_CLK>, > > <&xo_board>; + clock-names = "iface", > > "ref"; > > + > > + status = "disabled"; > > + };
On 06/09/2022 19:33, Luca Weiss wrote:
> + phy-names = "dsi";
Hi Luca.
It looks like the phy-names property drop will go through.
Suggest dropping the above for your V2.
https://lore.kernel.org/all/20220907000105.786265-6-bryan.odonoghue@linaro.org/T/
---
bod
On 07/09/2022 18:30, Luca Weiss wrote: > Hi Dmitry, > > On Dienstag, 6. September 2022 21:41:11 CEST Dmitry Baryshkov wrote: >> On Tue, 6 Sept 2022 at 21:36, Luca Weiss <luca@z3ntu.xyz> wrote: >>> From: Vladimir Lypak <vladimir.lypak@gmail.com> >>> >>> Add the MDSS, MDP and DSI nodes that are found on msm8953 SoC. >>> >>> Signed-off-by: Vladimir Lypak <vladimir.lypak@gmail.com> >>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz> >>> --- >>> Changes since v2: >>> - add "core" clock for mdss as suggested by Dmitry Baryshkov >>> >>> arch/arm64/boot/dts/qcom/msm8953.dtsi | 210 ++++++++++++++++++++++++++ >>> 1 file changed, 210 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi >>> b/arch/arm64/boot/dts/qcom/msm8953.dtsi index 3d11331e78d2..580333141a66 >>> 100644 >>> --- a/arch/arm64/boot/dts/qcom/msm8953.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi >>> @@ -726,6 +726,216 @@ tcsr_phy_clk_scheme_sel: syscon@193f044 { >>> >>> reg = <0x193f044 0x4>; >>> >>> }; >>> >>> + mdss: mdss@1a00000 { >>> + compatible = "qcom,mdss"; >>> + >>> + reg = <0x1a00000 0x1000>, >>> + <0x1ab0000 0x1040>; >>> + reg-names = "mdss_phys", >>> + "vbif_phys"; >>> + >>> + power-domains = <&gcc MDSS_GDSC>; >>> + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; >>> + >>> + interrupt-controller; >>> + #interrupt-cells = <1>; >>> + >>> + clocks = <&gcc GCC_MDSS_AHB_CLK>, >>> + <&gcc GCC_MDSS_AXI_CLK>, >>> + <&gcc GCC_MDSS_VSYNC_CLK>, >>> + <&gcc GCC_MDSS_MDP_CLK>; >>> + clock-names = "iface", >>> + "bus", >>> + "vsync", >>> + "core"; >>> + >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + ranges; >>> + >>> + status = "disabled"; >>> + >>> + mdp: mdp@1a01000 { >>> + compatible = "qcom,mdp5"; >> >> Could you please change this to "qcom,msm8953-mdp5", "qcom,mdp5". > > This would be the first dtsi using the two compatibles then, correct? Are there > any plans to adjust other SoCs? Yes, this is a long-going plan. Having just "qcom,mdp5" doesn't allow switching between mdp5 and dpu1 drivers. Thus I'd ask to add per-SoC compat strings. It's up to you (and Rob/Krzysztof) whether to leave just one compat string or have both of them: a per-soc one and a generic one. > >> >>> + reg = <0x1a01000 0x89000>; >>> + reg-names = "mdp_phys"; >>> + >> >> [skipped] >> >>> + >>> + dsi0_phy: dsi-phy@1a94400 { >> >> Let's probably use a generic name 'phy' here and for dsi1_phy. > > Here also, the bindings examples all use dsi-phy@, are there any plans to > change that and adjust other dtsi files? Yes, sc7280 already uses phy@ for both DSI and eDP PHYs. > >> >> The rest looks good to me. > > Thanks!
On 08/09/2022 18:13, Dmitry Baryshkov wrote: >>>> >>>> + mdss: mdss@1a00000 { >>>> + compatible = "qcom,mdss"; >>>> + >>>> + reg = <0x1a00000 0x1000>, >>>> + <0x1ab0000 0x1040>; >>>> + reg-names = "mdss_phys", >>>> + "vbif_phys"; >>>> + >>>> + power-domains = <&gcc MDSS_GDSC>; >>>> + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; >>>> + >>>> + interrupt-controller; >>>> + #interrupt-cells = <1>; >>>> + >>>> + clocks = <&gcc GCC_MDSS_AHB_CLK>, >>>> + <&gcc GCC_MDSS_AXI_CLK>, >>>> + <&gcc GCC_MDSS_VSYNC_CLK>, >>>> + <&gcc GCC_MDSS_MDP_CLK>; >>>> + clock-names = "iface", >>>> + "bus", >>>> + "vsync", >>>> + "core"; >>>> + >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + ranges; >>>> + >>>> + status = "disabled"; >>>> + >>>> + mdp: mdp@1a01000 { >>>> + compatible = "qcom,mdp5"; >>> >>> Could you please change this to "qcom,msm8953-mdp5", "qcom,mdp5". >> >> This would be the first dtsi using the two compatibles then, correct? Are there >> any plans to adjust other SoCs? > > Yes, this is a long-going plan. Having just "qcom,mdp5" doesn't allow > switching between mdp5 and dpu1 drivers. Thus I'd ask to add per-SoC > compat strings. > > It's up to you (and Rob/Krzysztof) whether to leave just one compat > string or have both of them: a per-soc one and a generic one. If device can bind to generic fallback ("qcom,mdp5") and still work somehow, then the fallback is OK. However if generic "qcom,mdp5" does not work at all, let's just choose something which is matching current patterns. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi b/arch/arm64/boot/dts/qcom/msm8953.dtsi index 3d11331e78d2..580333141a66 100644 --- a/arch/arm64/boot/dts/qcom/msm8953.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi @@ -726,6 +726,216 @@ tcsr_phy_clk_scheme_sel: syscon@193f044 { reg = <0x193f044 0x4>; }; + mdss: mdss@1a00000 { + compatible = "qcom,mdss"; + + reg = <0x1a00000 0x1000>, + <0x1ab0000 0x1040>; + reg-names = "mdss_phys", + "vbif_phys"; + + power-domains = <&gcc MDSS_GDSC>; + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>; + + interrupt-controller; + #interrupt-cells = <1>; + + clocks = <&gcc GCC_MDSS_AHB_CLK>, + <&gcc GCC_MDSS_AXI_CLK>, + <&gcc GCC_MDSS_VSYNC_CLK>, + <&gcc GCC_MDSS_MDP_CLK>; + clock-names = "iface", + "bus", + "vsync", + "core"; + + #address-cells = <1>; + #size-cells = <1>; + ranges; + + status = "disabled"; + + mdp: mdp@1a01000 { + compatible = "qcom,mdp5"; + reg = <0x1a01000 0x89000>; + reg-names = "mdp_phys"; + + interrupt-parent = <&mdss>; + interrupts = <0>; + + power-domains = <&gcc MDSS_GDSC>; + + clocks = <&gcc GCC_MDSS_AHB_CLK>, + <&gcc GCC_MDSS_AXI_CLK>, + <&gcc GCC_MDSS_MDP_CLK>, + <&gcc GCC_MDSS_VSYNC_CLK>; + clock-names = "iface", + "bus", + "core", + "vsync"; + + iommus = <&apps_iommu 0x15>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + mdp5_intf1_out: endpoint { + remote-endpoint = <&dsi0_in>; + }; + }; + + port@1 { + reg = <1>; + mdp5_intf2_out: endpoint { + remote-endpoint = <&dsi1_in>; + }; + }; + }; + }; + + dsi0: dsi@1a94000 { + compatible = "qcom,mdss-dsi-ctrl"; + reg = <0x1a94000 0x400>; + reg-names = "dsi_ctrl"; + + interrupt-parent = <&mdss>; + interrupts = <4>; + + assigned-clocks = <&gcc BYTE0_CLK_SRC>, + <&gcc PCLK0_CLK_SRC>; + assigned-clock-parents = <&dsi0_phy 0>, + <&dsi0_phy 1>; + + clocks = <&gcc GCC_MDSS_MDP_CLK>, + <&gcc GCC_MDSS_AHB_CLK>, + <&gcc GCC_MDSS_AXI_CLK>, + <&gcc GCC_MDSS_BYTE0_CLK>, + <&gcc GCC_MDSS_PCLK0_CLK>, + <&gcc GCC_MDSS_ESC0_CLK>; + clock-names = "mdp_core", + "iface", + "bus", + "byte", + "pixel", + "core"; + + phys = <&dsi0_phy>; + phy-names = "dsi"; + + #address-cells = <1>; + #size-cells = <0>; + + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi0_in: endpoint { + remote-endpoint = <&mdp5_intf1_out>; + }; + }; + + port@1 { + reg = <1>; + dsi0_out: endpoint { + }; + }; + }; + }; + + dsi0_phy: dsi-phy@1a94400 { + compatible = "qcom,dsi-phy-14nm-8953"; + reg = <0x1a94400 0x100>, + <0x1a94500 0x300>, + <0x1a94800 0x188>; + reg-names = "dsi_phy", + "dsi_phy_lane", + "dsi_pll"; + + #clock-cells = <1>; + #phy-cells = <0>; + + clocks = <&gcc GCC_MDSS_AHB_CLK>, <&xo_board>; + clock-names = "iface", "ref"; + + status = "disabled"; + }; + + dsi1: dsi@1a96000 { + compatible = "qcom,mdss-dsi-ctrl"; + reg = <0x1a96000 0x400>; + reg-names = "dsi_ctrl"; + + interrupt-parent = <&mdss>; + interrupts = <5>; + + assigned-clocks = <&gcc BYTE1_CLK_SRC>, + <&gcc PCLK1_CLK_SRC>; + assigned-clock-parents = <&dsi1_phy 0>, + <&dsi1_phy 1>; + + clocks = <&gcc GCC_MDSS_MDP_CLK>, + <&gcc GCC_MDSS_AHB_CLK>, + <&gcc GCC_MDSS_AXI_CLK>, + <&gcc GCC_MDSS_BYTE1_CLK>, + <&gcc GCC_MDSS_PCLK1_CLK>, + <&gcc GCC_MDSS_ESC1_CLK>; + clock-names = "mdp_core", + "iface", + "bus", + "byte", + "pixel", + "core"; + + phys = <&dsi1_phy>; + phy-names = "dsi"; + + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi1_in: endpoint { + remote-endpoint = <&mdp5_intf2_out>; + }; + }; + + port@1 { + reg = <1>; + dsi1_out: endpoint { + }; + }; + }; + }; + + dsi1_phy: dsi-phy@1a96400 { + compatible = "qcom,dsi-phy-14nm-8953"; + reg = <0x1a96400 0x100>, + <0x1a96500 0x300>, + <0x1a96800 0x188>; + reg-names = "dsi_phy", + "dsi_phy_lane", + "dsi_pll"; + + #clock-cells = <1>; + #phy-cells = <0>; + + clocks = <&gcc GCC_MDSS_AHB_CLK>, <&xo_board>; + clock-names = "iface", "ref"; + + status = "disabled"; + }; + }; + apps_iommu: iommu@1e00000 { compatible = "qcom,msm8953-iommu", "qcom,msm-iommu-v1"; ranges = <0 0x1e20000 0x20000>;