Re: [PATCH v1 4/4] arm64: dts: qcom: sc7280: add edp display dt nodes
From: Stephen Boyd <hidden>
Date: 2021-08-18 20:03:15
Also in:
dri-devel, linux-arm-msm, lkml
Quoting Krishna Manikandan (2021-08-18 03:27:04)
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index aadf55d..5be318e 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi@@ -1412,7 +1412,7 @@ reg = <0 0xaf00000 0 0x20000>; clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_DISP_GPLL0_CLK_SRC>, - <0>, <0>, <0>, <0>, <0>, <0>; + <0>, <0>, <0>, <0>, <&edp_phy 0>, <&edp_phy 1>; clock-names = "bi_tcxo", "gcc_disp_gpll0_clk", "dsi0_phy_pll_out_byteclk", "dsi0_phy_pll_out_dsiclk",@@ -1493,6 +1493,12 @@ remote-endpoint = <&dsi0_in>; }; };
Newline here please.
quoted hunk ↗ jump to hunk
+ port@1 { + reg = <1>; + dpu_intf5_out: endpoint { + remote-endpoint = <&edp_in>; + }; + }; }; mdp_opp_table: mdp-opp-table {@@ -1608,6 +1614,101 @@ status = "disabled"; }; + + msm_edp: edp@aea0000 { + status = "disabled";
Please pick a place to put status disabled. I don't know what qcom maintainers want, but please be consistent.
+ compatible = "qcom,sc7280-edp"; + reg = <0 0xaea0000 0 0x200>, + <0 0xaea0200 0 0x200>, + <0 0xaea0400 0 0xc00>, + <0 0xaea1000 0 0x400>; + + interrupt-parent = <&mdss>; + interrupts = <14 IRQ_TYPE_NONE>;
Drop flags.
+ + clocks = <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_EDP_CLKREF_EN>, + <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&dispcc DISP_CC_MDSS_EDP_AUX_CLK>, + <&dispcc DISP_CC_MDSS_EDP_LINK_CLK>, + <&dispcc DISP_CC_MDSS_EDP_LINK_INTF_CLK>, + <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK>; + clock-names = "core_xo", "core_ref", + "core_iface", "core_aux", "ctrl_link", + "ctrl_link_iface", "stream_pixel";
One line per string please.
+ #clock-cells = <1>; + assigned-clocks = <&dispcc DISP_CC_MDSS_EDP_LINK_CLK_SRC>, + <&dispcc DISP_CC_MDSS_EDP_PIXEL_CLK_SRC>; + assigned-clock-parents = <&edp_phy 0>, <&edp_phy 1>; + + phys = <&edp_phy>; + phy-names = "dp"; + + vdda-1p2-supply = <&vreg_l6b_1p2>; + vdda-0p9-supply = <&vreg_l10c_0p8>;
Can this be done here? Probably needs to move to the board dts/dtsi file.
+ operating-points-v2 = <&edp_opp_table>; + power-domains = <&rpmhpd SC7280_CX>; + + pinctrl-names = "default"; + pinctrl-0 = <&edp_hot_plug_det>, <&edp_panel_power_on>; + + panel-bklt-gpio = <&pm8350c_gpios 7 GPIO_ACTIVE_HIGH>; + panel-pwm-gpio = <&pm8350c_gpios 8 GPIO_ACTIVE_HIGH>;
Please no panel-bklt-gpio and panel-pwm-gpio properties.
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ edp_in: endpoint {
+ remote-endpoint = <&dpu_intf5_out>;
+ };
+ };
+ };
+
+ edp_opp_table: edp-opp-table {
edp_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_nom>;
+ };
+
+ opp-810000000 {
+ opp-hz = /bits/ 64 <810000000>;
+ required-opps = <&rpmhpd_opp_nom>;
+ };
+ };
+ };
+
+ edp_phy: phy@aec2000 {Good job on using phy!
+ status = "disabled"; + compatible = "qcom,sc7280-edp-phy"; + reg = <0 0xaec2a00 0 0x19c>, + <0 0xaec2200 0 0xa0>, + <0 0xaec2600 0 0xa0>, + <0 0xaec2000 0 0x1c0>; + + clocks = <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_EDP_CLKREF_EN>; + clock-names = "aux", "cfg_ahb"; + + vdda-pll-supply = <&vreg_l6b_1p2>; + vdda-phy-supply = <&vreg_l10c_0p8>;
Again, still question the ability to put this here vs. in IDP file.
quoted hunk ↗ jump to hunk
+ + #clock-cells = <1>; + #phy-cells = <0>; + }; }; pdc: interrupt-controller@b220000 {@@ -1704,6 +1805,30 @@ function = "qup13"; }; + edp_hot_plug_det: edp-hot-plug-det { + pinmux { + pins = "gpio60"; + function = "edp_hot"; + }; + pinconf { + pins = "gpio60"; + bias-pull-down; + input-enable; + };
Move pinconf stuff to board file (and combine nodes as mka stated).
+ };
+
+ edp_panel_power_on: edp-panel-power-on {
+ pinmux {
+ pins = "gpio80";
+ function = "gpio";
+ };
+ pinconf {
+ pins = "gpio80";
+ bias-disable;
+ output-high;
+ };
+ };
+
sdc1_on: sdc1-on {
clk {
pins = "sdc1_clk";
--
2.7.4