[PATCH v5 2/9] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
Sankeerth Billakanti (QUIC)
quic_sbillaka at quicinc.com
Fri Mar 25 13:30:58 UTC 2022
> -----Original Message-----
> From: Stephen Boyd <swboyd at chromium.org>
> Sent: Friday, March 18, 2022 2:53 AM
> To: Sankeerth Billakanti (QUIC) <quic_sbillaka at quicinc.com>;
> devicetree at vger.kernel.org; dri-devel at lists.freedesktop.org;
> freedreno at lists.freedesktop.org; linux-arm-msm at vger.kernel.org; linux-
> kernel at vger.kernel.org
> Cc: robdclark at gmail.com; seanpaul at chromium.org; quic_kalyant
> <quic_kalyant at quicinc.com>; Abhinav Kumar (QUIC)
> <quic_abhinavk at quicinc.com>; dianders at chromium.org; Kuogee Hsieh
> (QUIC) <quic_khsieh at quicinc.com>; agross at kernel.org;
> bjorn.andersson at linaro.org; robh+dt at kernel.org; krzk+dt at kernel.org;
> sean at poorly.run; airlied at linux.ie; daniel at ffwll.ch;
> thierry.reding at gmail.com; sam at ravnborg.org;
> dmitry.baryshkov at linaro.org; quic_vproddut <quic_vproddut at 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 at 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 at 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
> >
More information about the dri-devel
mailing list