diff mbox series

[v5,2/9] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD

Message ID 1647452154-16361-3-git-send-email-quic_sbillaka@quicinc.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for the eDP panel on sc7280 CRD | expand

Commit Message

Sankeerth Billakanti (QUIC) March 16, 2022, 5:35 p.m. UTC
Enable support for eDP interface via aux_bus on CRD platform.

Signed-off-by: Sankeerth Billakanti <quic_sbillaka@quicinc.com>
---

Changes in v5:
  - Change the order of patches
  - Remove the backlight nodes
  - Remove the bias setting
  - Fix compilation issue
  - Model VREG_EDP_BP for backlight power

Changes in v4:
  - Create new patch for name changes
  - Remove output-low

Changes in v3:
  - Sort the nodes alphabetically
  - Use - instead of _ as node names
  - Place the backlight and panel nodes under root
  - Change the name of edp_out to mdss_edp_out
  - Change the names of regulator nodes
  - Delete unused properties in the board file


Changes in v2:
  - Sort node references alphabetically
  - Improve readability
  - Move the pwm pinctrl to pwm node
  - Move the regulators to root
  - Define backlight power
  - Remove dummy regulator node
  - Cleanup pinctrl definitions

 arch/arm64/boot/dts/qcom/sc7280-crd.dts | 93 +++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

Comments

Stephen Boyd March 17, 2022, 9:23 p.m. UTC | #1
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.

> +       vdda-1p2-supply = <&vreg_l6b_1p2>;
> +       vdda-0p9-supply = <&vreg_l10c_0p8>;
> +
> +       aux-bus {

Can this move to sc7280.dtsi and get a phandle?

> +               edp_panel: edp-panel {

I'd prefer

	edp_panel: panel {

because there's only one panel node at this level.

> +                       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?

> +                       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

> +                                               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?

> +               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?

> +               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?

> +       };
> +
>         tp_int_odl: tp-int-odl {
>                 pins = "gpio7";
>                 function = "gpio";
> --
> 2.7.4
>
Doug Anderson March 18, 2022, 5:20 p.m. UTC | #2
Hi,

On Wed, Mar 16, 2022 at 10:36 AM Sankeerth Billakanti
<quic_sbillaka@quicinc.com> wrote:
>
> Enable support for eDP interface via aux_bus on CRD platform.
>
> Signed-off-by: Sankeerth Billakanti <quic_sbillaka@quicinc.com>
> ---
>
> Changes in v5:
>   - Change the order of patches
>   - Remove the backlight nodes
>   - Remove the bias setting
>   - Fix compilation issue
>   - Model VREG_EDP_BP for backlight power
>
> Changes in v4:
>   - Create new patch for name changes
>   - Remove output-low
>
> Changes in v3:
>   - Sort the nodes alphabetically
>   - Use - instead of _ as node names
>   - Place the backlight and panel nodes under root
>   - Change the name of edp_out to mdss_edp_out
>   - Change the names of regulator nodes
>   - Delete unused properties in the board file
>
>
> Changes in v2:
>   - Sort node references alphabetically
>   - Improve readability
>   - Move the pwm pinctrl to pwm node
>   - Move the regulators to root
>   - Define backlight power
>   - Remove dummy regulator node
>   - Cleanup pinctrl definitions
>
>  arch/arm64/boot/dts/qcom/sc7280-crd.dts | 93 +++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)

At a high level, I'd expect your patch to be based upon Matthias's
series, AKA the 4 patches from:

https://lore.kernel.org/r/20220316172814.v1.1.I2deda8f2cd6adfbb525a97d8fee008a8477b7b0e@changeid/

I'll leave it up to you about whether you care to support eDP on the
old CRD1/2 or just on CRD3. Personally I'd think CRD3 would be enough.

Then, I'd expect your patch to mostly incorporate
<https://crrev.com/c/3379844>, though that patch was written before
aux-bus support so the panel would need to go in a different place.

Stephen already gave some comments and basing on Matthias's patches
will be a pretty big change, so I probably won't comment lots more.


> +&mdss_edp {
> +       status = "okay";
> +
> +       data-lanes = <0 1 2 3>;
> +       vdda-1p2-supply = <&vreg_l6b_1p2>;
> +       vdda-0p9-supply = <&vreg_l10c_0p8>;
> +
> +       aux-bus {
> +               edp_panel: edp-panel {

As Stephen pointed out, it should be called "panel".
Sankeerth Billakanti (QUIC) March 25, 2022, 1:30 p.m. UTC | #3
> -----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
> >
Sankeerth Billakanti (QUIC) March 25, 2022, 1:41 p.m. UTC | #4
> -----Original Message-----
> From: Doug Anderson <dianders@chromium.org>
> Sent: Friday, March 18, 2022 10:51 PM
> To: Sankeerth Billakanti (QUIC) <quic_sbillaka@quicinc.com>
> Cc: dri-devel <dri-devel@lists.freedesktop.org>; linux-arm-msm <linux-arm-
> msm@vger.kernel.org>; freedreno <freedreno@lists.freedesktop.org>;
> LKML <linux-kernel@vger.kernel.org>; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Rob Clark
> <robdclark@gmail.com>; Sean Paul <seanpaul@chromium.org>; Stephen
> Boyd <swboyd@chromium.org>; quic_kalyant <quic_kalyant@quicinc.com>;
> Abhinav Kumar (QUIC) <quic_abhinavk@quicinc.com>; Kuogee Hsieh (QUIC)
> <quic_khsieh@quicinc.com>; Andy Gross <agross@kernel.org>;
> bjorn.andersson@linaro.org; Rob Herring <robh+dt@kernel.org>;
> krzk+dt@kernel.org; Sean Paul <sean@poorly.run>; David Airlie
> <airlied@linux.ie>; Daniel Vetter <daniel@ffwll.ch>; Thierry Reding
> <thierry.reding@gmail.com>; Sam Ravnborg <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
> 
> Hi,
> 
> On Wed, Mar 16, 2022 at 10:36 AM Sankeerth Billakanti
> <quic_sbillaka@quicinc.com> wrote:
> >
> > Enable support for eDP interface via aux_bus on CRD platform.
> >
> > Signed-off-by: Sankeerth Billakanti <quic_sbillaka@quicinc.com>
> > ---
> >
> > Changes in v5:
> >   - Change the order of patches
> >   - Remove the backlight nodes
> >   - Remove the bias setting
> >   - Fix compilation issue
> >   - Model VREG_EDP_BP for backlight power
> >
> > Changes in v4:
> >   - Create new patch for name changes
> >   - Remove output-low
> >
> > Changes in v3:
> >   - Sort the nodes alphabetically
> >   - Use - instead of _ as node names
> >   - Place the backlight and panel nodes under root
> >   - Change the name of edp_out to mdss_edp_out
> >   - Change the names of regulator nodes
> >   - Delete unused properties in the board file
> >
> >
> > Changes in v2:
> >   - Sort node references alphabetically
> >   - Improve readability
> >   - Move the pwm pinctrl to pwm node
> >   - Move the regulators to root
> >   - Define backlight power
> >   - Remove dummy regulator node
> >   - Cleanup pinctrl definitions
> >
> >  arch/arm64/boot/dts/qcom/sc7280-crd.dts | 93
> > +++++++++++++++++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> 
> At a high level, I'd expect your patch to be based upon Matthias's series, AKA
> the 4 patches from:
> 
> https://lore.kernel.org/r/20220316172814.v1.1.I2deda8f2cd6adfbb525a97d8f
> ee008a8477b7b0e@changeid/
> 
> I'll leave it up to you about whether you care to support eDP on the old
> CRD1/2 or just on CRD3. Personally I'd think CRD3 would be enough.
> 
> Then, I'd expect your patch to mostly incorporate
> <https://crrev.com/c/3379844>, though that patch was written before aux-
> bus support so the panel would need to go in a different place.
> 
> Stephen already gave some comments and basing on Matthias's patches will
> be a pretty big change, so I probably won't comment lots more.
> 
> 

I rebased my change on top of Matthias's changes now. We are discussing about the qcard changes internally to understand the way ahead.
I believe all my current changes are localized to the crd-r3 files only for the qyalcomm crd3.1

I want to have a different series for c and dt changes to expedite review process. May I separate the c changes from this series?

> > +&mdss_edp {
> > +       status = "okay";
> > +
> > +       data-lanes = <0 1 2 3>;
> > +       vdda-1p2-supply = <&vreg_l6b_1p2>;
> > +       vdda-0p9-supply = <&vreg_l10c_0p8>;
> > +
> > +       aux-bus {
> > +               edp_panel: edp-panel {
> 
> As Stephen pointed out, it should be called "panel".

Okay. Will make that change
Doug Anderson March 25, 2022, 1:44 p.m. UTC | #5
Hi,

On Fri, Mar 25, 2022 at 6:41 AM Sankeerth Billakanti (QUIC)
<quic_sbillaka@quicinc.com> wrote:
>
> > -----Original Message-----
> > From: Doug Anderson <dianders@chromium.org>
> > Sent: Friday, March 18, 2022 10:51 PM
> > To: Sankeerth Billakanti (QUIC) <quic_sbillaka@quicinc.com>
> > Cc: dri-devel <dri-devel@lists.freedesktop.org>; linux-arm-msm <linux-arm-
> > msm@vger.kernel.org>; freedreno <freedreno@lists.freedesktop.org>;
> > LKML <linux-kernel@vger.kernel.org>; open list:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Rob Clark
> > <robdclark@gmail.com>; Sean Paul <seanpaul@chromium.org>; Stephen
> > Boyd <swboyd@chromium.org>; quic_kalyant <quic_kalyant@quicinc.com>;
> > Abhinav Kumar (QUIC) <quic_abhinavk@quicinc.com>; Kuogee Hsieh (QUIC)
> > <quic_khsieh@quicinc.com>; Andy Gross <agross@kernel.org>;
> > bjorn.andersson@linaro.org; Rob Herring <robh+dt@kernel.org>;
> > krzk+dt@kernel.org; Sean Paul <sean@poorly.run>; David Airlie
> > <airlied@linux.ie>; Daniel Vetter <daniel@ffwll.ch>; Thierry Reding
> > <thierry.reding@gmail.com>; Sam Ravnborg <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
> >
> > Hi,
> >
> > On Wed, Mar 16, 2022 at 10:36 AM Sankeerth Billakanti
> > <quic_sbillaka@quicinc.com> wrote:
> > >
> > > Enable support for eDP interface via aux_bus on CRD platform.
> > >
> > > Signed-off-by: Sankeerth Billakanti <quic_sbillaka@quicinc.com>
> > > ---
> > >
> > > Changes in v5:
> > >   - Change the order of patches
> > >   - Remove the backlight nodes
> > >   - Remove the bias setting
> > >   - Fix compilation issue
> > >   - Model VREG_EDP_BP for backlight power
> > >
> > > Changes in v4:
> > >   - Create new patch for name changes
> > >   - Remove output-low
> > >
> > > Changes in v3:
> > >   - Sort the nodes alphabetically
> > >   - Use - instead of _ as node names
> > >   - Place the backlight and panel nodes under root
> > >   - Change the name of edp_out to mdss_edp_out
> > >   - Change the names of regulator nodes
> > >   - Delete unused properties in the board file
> > >
> > >
> > > Changes in v2:
> > >   - Sort node references alphabetically
> > >   - Improve readability
> > >   - Move the pwm pinctrl to pwm node
> > >   - Move the regulators to root
> > >   - Define backlight power
> > >   - Remove dummy regulator node
> > >   - Cleanup pinctrl definitions
> > >
> > >  arch/arm64/boot/dts/qcom/sc7280-crd.dts | 93
> > > +++++++++++++++++++++++++++++++++
> > >  1 file changed, 93 insertions(+)
> >
> > At a high level, I'd expect your patch to be based upon Matthias's series, AKA
> > the 4 patches from:
> >
> > https://lore.kernel.org/r/20220316172814.v1.1.I2deda8f2cd6adfbb525a97d8f
> > ee008a8477b7b0e@changeid/
> >
> > I'll leave it up to you about whether you care to support eDP on the old
> > CRD1/2 or just on CRD3. Personally I'd think CRD3 would be enough.
> >
> > Then, I'd expect your patch to mostly incorporate
> > <https://crrev.com/c/3379844>, though that patch was written before aux-
> > bus support so the panel would need to go in a different place.
> >
> > Stephen already gave some comments and basing on Matthias's patches will
> > be a pretty big change, so I probably won't comment lots more.
> >
> >
>
> I rebased my change on top of Matthias's changes now. We are discussing about the qcard changes internally to understand the way ahead.
> I believe all my current changes are localized to the crd-r3 files only for the qyalcomm crd3.1
>
> I want to have a different series for c and dt changes to expedite review process. May I separate the c changes from this series?

I'd have no problems with that. They go into different trees and if it
makes it easier to get a new version of the driver out while you're
figuring out what to do about the dts then I'd say let's do it.

-Doug
Stephen Boyd March 25, 2022, 7:51 p.m. UTC | #6
Quoting Sankeerth Billakanti (QUIC) (2022-03-25 06:30:58)
>
>
> > > +       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?

The below patch looks good.

>
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3283,6 +3283,18 @@
>
>                                 status = "disabled";
diff mbox series

Patch

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>;
+	vdda-1p2-supply = <&vreg_l6b_1p2>;
+	vdda-0p9-supply = <&vreg_l10c_0p8>;
+
+	aux-bus {
+		edp_panel: edp-panel {
+			compatible = "edp-panel";
+
+			power-supply = <&edp_3v3_regulator>;
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					edp_panel_in: endpoint {
+						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 {
+		pins = "gpio7";
+		function = "normal";
+		qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+	};
+
+	edp_bl_pwm: edp-bl-pwm {
+		pins = "gpio8";
+		function = "func1";
+		qcom,drive-strength = <PMIC_GPIO_STRENGTH_LOW>;
+	};
+};
+
 &tlmm {
+	edp_panel_power: edp-panel-power {
+		pins = "gpio80";
+		function = "gpio";
+	};
+
 	tp_int_odl: tp-int-odl {
 		pins = "gpio7";
 		function = "gpio";