-----Original Message----- From: Stephen Boyd swboyd@chromium.org Sent: Friday, March 18, 2022 2:53 AM To: Sankeerth Billakanti (QUIC) quic_sbillaka@quicinc.com; devicetree@vger.kernel.org; dri-devel@lists.freedesktop.org; freedreno@lists.freedesktop.org; linux-arm-msm@vger.kernel.org; linux- kernel@vger.kernel.org Cc: robdclark@gmail.com; seanpaul@chromium.org; quic_kalyant quic_kalyant@quicinc.com; Abhinav Kumar (QUIC) quic_abhinavk@quicinc.com; dianders@chromium.org; Kuogee Hsieh (QUIC) quic_khsieh@quicinc.com; agross@kernel.org; bjorn.andersson@linaro.org; robh+dt@kernel.org; krzk+dt@kernel.org; sean@poorly.run; airlied@linux.ie; daniel@ffwll.ch; thierry.reding@gmail.com; sam@ravnborg.org; dmitry.baryshkov@linaro.org; quic_vproddut quic_vproddut@quicinc.com Subject: Re: [PATCH v5 2/9] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
Quoting Sankeerth Billakanti (2022-03-16 10:35:47)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts index e2efbdd..2df654e 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts @@ -7,6 +7,7 @@
/dts-v1/;
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h> #include "sc7280-idp.dtsi" #include "sc7280-idp-ec-h1.dtsi"
@@ -21,6 +22,27 @@ chosen { stdout-path = "serial0:115200n8"; };
edp_3v3_regulator: edp-3v3-regulator {compatible = "regulator-fixed";regulator-name = "edp_3v3_regulator";regulator-min-microvolt = <3300000>;regulator-max-microvolt = <3300000>;gpio = <&tlmm 80 GPIO_ACTIVE_HIGH>;enable-active-high;pinctrl-names = "default";pinctrl-0 = <&edp_panel_power>;};vreg_edp_bp: vreg-edp-bp-regulator {compatible = "regulator-fixed";regulator-name = "vreg_edp_bp";regulator-always-on;regulator-boot-on;};};
&apps_rsc { @@ -76,6 +98,58 @@ ap_ts_pen_1v8: &i2c13 { }; };
+&mdss {
status = "okay";+};
+&mdss_dp {
status = "okay";pinctrl-names = "default";pinctrl-0 = <&dp_hot_plug_det>;data-lanes = <0 1>;vdda-1p2-supply = <&vreg_l6b_1p2>;vdda-0p9-supply = <&vreg_l1b_0p8>; };+&mdss_edp {
status = "okay";data-lanes = <0 1 2 3>;Is this property necessary? It looks like the default.
Will remove it
vdda-1p2-supply = <&vreg_l6b_1p2>;vdda-0p9-supply = <&vreg_l10c_0p8>;aux-bus {Can this move to sc7280.dtsi and get a phandle?
Okay, I will move this to sc7280.dtsi like below. Shall I define the required properties under &mdss_edp_panel node in the sc7280-crd3.dts?
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -3283,6 +3283,18 @@
status = "disabled";
+ aux-bus { + mdss_edp_panel: panel { + compatible = "edp-panel"; + + port { + mdss_edp_panel_in: endpoint { + remote-endpoint = <&mdss_edp_out>; + }; + }; + }; + }; + ports { #address-cells = <1>; #size-cells = <0>; @@ -3296,7 +3308,9 @@
port@1 { reg = <1>; - mdss_edp_out: endpoint { }; + mdss_edp_out: endpoint { + remote-endpoint = <&mdss_edp_panel_in>; + }; }; };
edp_panel: edp-panel {I'd prefer
edp_panel: panel {
because there's only one panel node at this level.
Okay. I will change it.
compatible = "edp-panel";power-supply = <&edp_3v3_regulator>;This is board specific, but I thought it was on the qcard so we should move this to sc7280-qcard.dtsi?
Qcard is used only for herobrine as of now according to the code. We defined this only for CRD. We will discuss this internally to understand the plan ahead.
ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;edp_panel_in: endpoint {This can be shortened to
port { edp_panel_in: endpoint {according to panel-edp.yaml
Okay. I will do it
remote-endpoint = <&mdss_edp_out>;};};};};};+};
+&mdss_edp_out {
remote-endpoint = <&edp_panel_in>; };+&mdss_edp_phy {
status = "okay";+};
+&mdss_mdp {
status = "okay";+};
&nvme_3v3_regulator { gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>; }; @@ -84,7 +158,26 @@ ap_ts_pen_1v8: &i2c13 { pins = "gpio51"; };
+&pm8350c_gpios {
edp_bl_power: edp-bl-power {Is this used in a later patch?
Yes, will move it to that patch.
pins = "gpio7";function = "normal";qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;};edp_bl_pwm: edp-bl-pwm {Is this used in a later patch? Can it be moved to the patch that uses it?
Yes, will move it to that patch. We split the patch to exclude the dependent pwm nodes so that Bjorn can merge this patch. But we will move all the related nodes to the next patch
pins = "gpio8";function = "func1";qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;};+};
&tlmm {
edp_panel_power: edp-panel-power {pins = "gpio80";function = "gpio";function of gpio is unnecessary. Where is the bias and drive-strength settings?
Will add it
};tp_int_odl: tp-int-odl { pins = "gpio7"; function = "gpio";-- 2.7.4