| Message ID | 1579867572-17188-1-git-send-email-harigovi@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] arm64: dts: sc7180: add display dt nodes | expand |
On Fri 24 Jan 04:06 PST 2020, Harigovindan P wrote: > Add display, DSI hardware DT nodes for sc7180. > > Signed-off-by: Harigovindan P <harigovi@codeaurora.org> Thanks for respinning this Harigovindan, just a few more small things below. Are the drivers ready for me to merge this? > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts [..] > +&pm6150l_gpio { > + disp_pins { You can omit this subnode level, i.e. just put disp_pins_default directly in &pm6150l_gpio. > + disp_pins_default: disp_pins_default{ > + pins = "gpio3"; > + function = "func1"; > + qcom,drive-strength = <2>; > + power-source = <0>; > + bias-disable; > + output-low; > + }; > + }; > +}; > + > &qspi_clk { > pinconf { > pins = "gpio63"; > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index 3bc3f64..3ebc45b 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -1184,6 +1184,130 @@ > #power-domain-cells = <1>; > }; > > + mdss: display_subsystem@ae00000 { Whenever possible, use - and not _ in node names. Regards, Bjorn
Hi, On Fri, Jan 24, 2020 at 4:07 AM Harigovindan P <harigovi@codeaurora.org> wrote: > > Add display, DSI hardware DT nodes for sc7180. > > Signed-off-by: Harigovindan P <harigovi@codeaurora.org> > --- > > Changes in v1: > -Added display DT nodes for sc7180 > Changes in v2: > -Renamed node names > -Corrected code alignments > -Removed extra new line > -Added DISP AHB clock for register access > under display_subsystem node for global settings > Changes in v3: > -Modified node names > -Modified hard coded values > -Removed mdss reg entry > > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 58 +++++++++++++++ > arch/arm64/boot/dts/qcom/sc7180.dtsi | 124 ++++++++++++++++++++++++++++++++ > 2 files changed, 182 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > index 388f50a..c77aab7 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > @@ -7,6 +7,7 @@ > > /dts-v1/; > > +#include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/regulator/qcom,rpmh-regulator.h> > #include "sc7180.dtsi" > #include "pm6150.dtsi" > @@ -232,6 +233,50 @@ > }; > }; > > +&dsi_controller { > + status = "okay"; > + > + vdda-supply = <&vreg_l3c_1p2>; > + > + panel@0 { > + compatible = "visionox,rm69299-1080p-display"; I don't think the bindings for this panel have landed anywhere, have they? Maybe keep the IDP patch separate from the main sc7180 patch so that we can land the main sc7180 patch even if the idp patch isn't quite ready? ...and maybe ping whoever is supposed to add support for this panel to tell them to get working on it. > + reg = <0>; > + > + vdda-supply = <&vreg_l8c_1p8>; > + vdd3p3-supply = <&vreg_l18a_2p8>; > + > + pinctrl-names = "default", "suspend"; > + pinctrl-0 = <&disp_pins_default>; > + pinctrl-1 = <&disp_pins_default>; > + > + reset-gpios = <&pm6150l_gpio 3 GPIO_ACTIVE_HIGH>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + panel0_in: endpoint { > + remote-endpoint = <&dsi0_out>; > + }; > + }; > + }; > + }; > + > + ports { > + port@1 { > + endpoint { > + remote-endpoint = <&panel0_in>; > + data-lanes = <0 1 2 3>; > + }; > + }; > + }; > +}; > + > +&dsi_phy { > + status = "okay"; The above doesn't do anything because the dsi_phy you add to sc7180.dtsi doesn't have a status = "disabled"; ...but probably it (and most of the other components that you're adding) should. The idea is that if it ever makes sense that a board might be built with this SoC but _not_ hook up a given component that it should start out "disabled" in the main SoC dtsi file. > +}; > + > &qspi { > status = "okay"; > pinctrl-names = "default"; > @@ -289,6 +334,19 @@ > > /* PINCTRL - additions to nodes defined in sc7180.dtsi */ > > +&pm6150l_gpio { > + disp_pins { > + disp_pins_default: disp_pins_default{ As Bjorn mentioned, node name should use "-" instead of "_". Also add a space before your "{" > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index 3bc3f64..3ebc45b 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -1184,6 +1184,130 @@ > #power-domain-cells = <1>; > }; > > + mdss: display_subsystem@ae00000 { > + compatible = "qcom,sc7180-mdss"; > + reg = <0 0x0ae00000 0 0x1000>; > + reg-names = "mdss"; > + > + power-domains = <&dispcc MDSS_GDSC>; You definitely can't land your patch until the "dispcc" node is added, but it's not. You should be mentioning somewhere (after the cut?) that you depend on the series to add dispcc. ...speaking of which, I just posted up a v2 of that. See: https://lore.kernel.org/r/20200124144154.v2.10.I1a4b93fb005791e29a9dcf288fc8bd459a555a59@changeid ...speaking of which, can you please change your patch to replace the bogus <0> in the dispcc for the DSI PHY, providing the clocks for "dsi_phy_pll_byte" and "dsi_phy_pll_pixel"? See <https://crrev.com/c/2017974/3> > + > + clocks = <&gcc GCC_DISP_AHB_CLK>, > + <&gcc GCC_DISP_HF_AXI_CLK>, > + <&dispcc DISP_CC_MDSS_AHB_CLK>, > + <&dispcc DISP_CC_MDSS_MDP_CLK>; > + clock-names = "iface", "gcc_bus", "ahb", "core"; > + > + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>; > + assigned-clock-rates = <300000000>; > + > + interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-controller; > + #interrupt-cells = <1>; > + > + iommus = <&apps_smmu 0x800 0x2>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + mdp: display_controller@ae01000 { Did you test this? As far I can tell this change between v2 and v3 broke things because the node name "mdp" is magic. In "drivers/gpu/drm/msm/msm_drv.c" you can find the function: static int compare_name_mdp(struct device *dev, void *data) { return (strstr(dev_name(dev), "mdp") != NULL); } ...so unless that function changes your device tree won't work. Maybe Bjorn can comment since I think he's the one that suggested you change this, but IMO change it back to "mdp@ae01000" for now and then start a separate patch series about transitioning this if people still want it. See <https://crrev.com/c/2020395/1>. -Doug
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts index 388f50a..c77aab7 100644 --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts @@ -7,6 +7,7 @@ /dts-v1/; +#include <dt-bindings/gpio/gpio.h> #include <dt-bindings/regulator/qcom,rpmh-regulator.h> #include "sc7180.dtsi" #include "pm6150.dtsi" @@ -232,6 +233,50 @@ }; }; +&dsi_controller { + status = "okay"; + + vdda-supply = <&vreg_l3c_1p2>; + + panel@0 { + compatible = "visionox,rm69299-1080p-display"; + reg = <0>; + + vdda-supply = <&vreg_l8c_1p8>; + vdd3p3-supply = <&vreg_l18a_2p8>; + + pinctrl-names = "default", "suspend"; + pinctrl-0 = <&disp_pins_default>; + pinctrl-1 = <&disp_pins_default>; + + reset-gpios = <&pm6150l_gpio 3 GPIO_ACTIVE_HIGH>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + panel0_in: endpoint { + remote-endpoint = <&dsi0_out>; + }; + }; + }; + }; + + ports { + port@1 { + endpoint { + remote-endpoint = <&panel0_in>; + data-lanes = <0 1 2 3>; + }; + }; + }; +}; + +&dsi_phy { + status = "okay"; +}; + &qspi { status = "okay"; pinctrl-names = "default"; @@ -289,6 +334,19 @@ /* PINCTRL - additions to nodes defined in sc7180.dtsi */ +&pm6150l_gpio { + disp_pins { + disp_pins_default: disp_pins_default{ + pins = "gpio3"; + function = "func1"; + qcom,drive-strength = <2>; + power-source = <0>; + bias-disable; + output-low; + }; + }; +}; + &qspi_clk { pinconf { pins = "gpio63"; diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 3bc3f64..3ebc45b 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -1184,6 +1184,130 @@ #power-domain-cells = <1>; }; + mdss: display_subsystem@ae00000 { + compatible = "qcom,sc7180-mdss"; + reg = <0 0x0ae00000 0 0x1000>; + reg-names = "mdss"; + + power-domains = <&dispcc MDSS_GDSC>; + + clocks = <&gcc GCC_DISP_AHB_CLK>, + <&gcc GCC_DISP_HF_AXI_CLK>, + <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&dispcc DISP_CC_MDSS_MDP_CLK>; + clock-names = "iface", "gcc_bus", "ahb", "core"; + + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>; + assigned-clock-rates = <300000000>; + + interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #interrupt-cells = <1>; + + iommus = <&apps_smmu 0x800 0x2>; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + + mdp: display_controller@ae01000 { + compatible = "qcom,sc7180-dpu"; + reg = <0 0x0ae01000 0 0x8f000>, + <0 0x0aeb0000 0 0x2008>, + <0 0x0af03000 0 0x16>; + reg-names = "mdp", "vbif", "disp_cc"; + + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&dispcc DISP_CC_MDSS_ROT_CLK>, + <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>, + <&dispcc DISP_CC_MDSS_MDP_CLK>, + <&dispcc DISP_CC_MDSS_VSYNC_CLK>; + clock-names = "iface", "rot", "lut", "core", + "vsync"; + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>, + <&dispcc DISP_CC_MDSS_VSYNC_CLK>; + assigned-clock-rates = <300000000>, + <19200000>; + + interrupt-parent = <&mdss>; + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dpu_intf1_out: endpoint { + remote-endpoint = <&dsi0_in>; + }; + }; + }; + }; + + dsi_controller: dsi_controller@ae94000 { + compatible = "qcom,mdss-dsi-ctrl"; + reg = <0 0x0ae94000 0 0x400>; + reg-names = "dsi_ctrl"; + + interrupt-parent = <&mdss>; + interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; + + clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>, + <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>, + <&dispcc DISP_CC_MDSS_PCLK0_CLK>, + <&dispcc DISP_CC_MDSS_ESC0_CLK>, + <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&gcc GCC_DISP_HF_AXI_CLK>; + clock-names = "byte", + "byte_intf", + "pixel", + "core", + "iface", + "bus"; + + phys = <&dsi_phy>; + phy-names = "dsi"; + + #address-cells = <1>; + #size-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + dsi0_in: endpoint { + remote-endpoint = <&dpu_intf1_out>; + }; + }; + + port@1 { + reg = <1>; + dsi0_out: endpoint { + }; + }; + }; + }; + + dsi_phy: dsi_phy@ae94400 { + compatible = "qcom,dsi-phy-10nm"; + reg = <0 0x0ae94400 0 0x200>, + <0 0x0ae94600 0 0x280>, + <0 0x0ae94a00 0 0x1e0>; + reg-names = "dsi_phy", + "dsi_phy_lane", + "dsi_pll"; + + #clock-cells = <1>; + #phy-cells = <0>; + + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>; + clock-names = "iface"; + }; + }; + pdc: interrupt-controller@b220000 { compatible = "qcom,sc7180-pdc", "qcom,pdc"; reg = <0 0x0b220000 0 0x30000>;
Add display, DSI hardware DT nodes for sc7180. Signed-off-by: Harigovindan P <harigovi@codeaurora.org> --- Changes in v1: -Added display DT nodes for sc7180 Changes in v2: -Renamed node names -Corrected code alignments -Removed extra new line -Added DISP AHB clock for register access under display_subsystem node for global settings Changes in v3: -Modified node names -Modified hard coded values -Removed mdss reg entry arch/arm64/boot/dts/qcom/sc7180-idp.dts | 58 +++++++++++++++ arch/arm64/boot/dts/qcom/sc7180.dtsi | 124 ++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+)