[v5,11/12] arm64: dts: qcom: sc8280xp-crd: Enable EDP

  From: Bjorn Andersson <bjorn.andersson@linaro.org>

The SC8280XP CRD has a EDP display on MDSS0 DP3, enable relevant nodes
and link it together with the backlight control.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---

Changes since v4:
- None

 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 72 ++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)
  

On Wed, Dec 07, 2022 at 02:00:11PM -0800, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> The SC8280XP CRD has a EDP display on MDSS0 DP3, enable relevant nodes
> and link it together with the backlight control.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
> 
> Changes since v4:
> - None
> 
>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 72 ++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index f09810e3d956..a7d2384cbbe8 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> @@ -20,7 +20,7 @@ aliases {
>  		serial0 = &qup2_uart17;
>  	};
>  
> -	backlight {
> +	backlight: backlight {
>  		compatible = "pwm-backlight";
>  		pwms = <&pmc8280c_lpg 3 1000000>;
>  		enable-gpios = <&pmc8280_1_gpios 8 GPIO_ACTIVE_HIGH>;
> @@ -34,6 +34,22 @@ chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
>  
> +	vreg_edp_3p3: regulator-edp-3p3 {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VREG_EDP_3P3";

Please use the net name from the schematics here (i.e. "VCC3LCD").

> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		gpio = <&tlmm 25 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&edp_reg_en>;
> +
> +		regulator-boot-on;
> +	};
> +
>  	vreg_edp_bl: regulator-edp-bl {
>  		compatible = "regulator-fixed";
>  
> @@ -230,6 +246,54 @@ vreg_l9d: ldo9 {
>  	};
>  };
>  
> +&dispcc0 {
> +	status = "okay";
> +};
> +
> +&mdss0 {
> +	status = "okay";
> +};
> +
> +&mdss0_dp3 {
> +	compatible = "qcom,sc8280xp-edp";
> +	status = "okay";

Please move the status property last (i.e. after data-lanes).

> +
> +	data-lanes = <0 1 2 3>;
> +
> +	aux-bus {
> +		panel {
> +			compatible = "edp-panel";
> +			power-supply = <&vreg_edp_3p3>;
> +
> +			backlight = <&backlight>;
> +
> +			ports {
> +				port {
> +					edp_panel_in: endpoint {
> +						remote-endpoint = <&mdss0_dp3_out>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +
> +	ports {
> +		port@1 {
> +			reg = <1>;
> +			mdss0_dp3_out: endpoint {
> +				remote-endpoint = <&edp_panel_in>;
> +			};
> +		};
> +	};
> +};
> +
> +&mdss0_dp3_phy {
> +	status = "okay";

Same here.

> +
> +	vdda-phy-supply = <&vreg_l6b>;
> +	vdda-pll-supply = <&vreg_l3b>;
> +};
> +
>  &pcie2a {
>  	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
>  	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
> @@ -496,6 +560,12 @@ hastings_reg_en: hastings-reg-en-state {
>  &tlmm {
>  	gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
>  
> +	edp_reg_en: edp-reg-en-state {
> +		pins = "gpio25";
> +		function = "gpio";
> +		output-enable;

'output-enable' is not valid for tlmm and causes the settings to be
rejected:

	sc8280xp-tlmm f100000.pinctrl: pin_config_group_set op failed for group 25
	reg-fixed-voltage regulator-edp-3p3: Error applying setting, reverse things back

> +	};
> +
>  	kybd_default: kybd-default-state {
>  		disable-pins {
>  			pins = "gpio102";

Johan
  

On Thu, 8 Dec 2022 at 00:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
>
> The SC8280XP CRD has a EDP display on MDSS0 DP3, enable relevant nodes
> and link it together with the backlight control.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>
> Changes since v4:
> - None
>
>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 72 ++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index f09810e3d956..a7d2384cbbe8 100644

[skipped]

> @@ -230,6 +246,54 @@ vreg_l9d: ldo9 {
>         };
>  };
>
> +&dispcc0 {
> +       status = "okay";
> +};
> +
> +&mdss0 {
> +       status = "okay";
> +};
> +
> +&mdss0_dp3 {
> +       compatible = "qcom,sc8280xp-edp";
> +       status = "okay";
> +
> +       data-lanes = <0 1 2 3>;

I hope to land Kuogee patches that move data-lanes to the endpoint
node, where they belong. Do we have any good way to proceed here?
Or would it be easier to land this patch as is and then, maybe next
cycle, move the property?

> +
> +       aux-bus {
> +               panel {
> +                       compatible = "edp-panel";
> +                       power-supply = <&vreg_edp_3p3>;
> +
> +                       backlight = <&backlight>;
> +
> +                       ports {
> +                               port {
> +                                       edp_panel_in: endpoint {
> +                                               remote-endpoint = <&mdss0_dp3_out>;
> +                                       };
> +                               };
> +                       };
> +               };
> +       };
> +
> +       ports {
> +               port@1 {
> +                       reg = <1>;
> +                       mdss0_dp3_out: endpoint {
> +                               remote-endpoint = <&edp_panel_in>;
> +                       };
> +               };
> +       };
> +};
> +
> +&mdss0_dp3_phy {
> +       status = "okay";
> +
> +       vdda-phy-supply = <&vreg_l6b>;
> +       vdda-pll-supply = <&vreg_l3b>;
> +};
> +
>  &pcie2a {
>         perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
>         wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
> @@ -496,6 +560,12 @@ hastings_reg_en: hastings-reg-en-state {
>  &tlmm {
>         gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
>
> +       edp_reg_en: edp-reg-en-state {
> +               pins = "gpio25";
> +               function = "gpio";
> +               output-enable;
> +       };
> +
>         kybd_default: kybd-default-state {
>                 disable-pins {
>                         pins = "gpio102";
> --
> 2.37.3
>
  

On Fri, Dec 09, 2022 at 11:35:23AM +0100, Johan Hovold wrote:
> On Wed, Dec 07, 2022 at 02:00:11PM -0800, Bjorn Andersson wrote:
> > From: Bjorn Andersson <bjorn.andersson@linaro.org>
> > 
> > The SC8280XP CRD has a EDP display on MDSS0 DP3, enable relevant nodes
> > and link it together with the backlight control.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> > 
> > Changes since v4:
> > - None
> > 
> >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 72 ++++++++++++++++++++++-
> >  1 file changed, 71 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > index f09810e3d956..a7d2384cbbe8 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > @@ -20,7 +20,7 @@ aliases {
> >  		serial0 = &qup2_uart17;
> >  	};
> >  
> > -	backlight {
> > +	backlight: backlight {
> >  		compatible = "pwm-backlight";
> >  		pwms = <&pmc8280c_lpg 3 1000000>;
> >  		enable-gpios = <&pmc8280_1_gpios 8 GPIO_ACTIVE_HIGH>;
> > @@ -34,6 +34,22 @@ chosen {
> >  		stdout-path = "serial0:115200n8";
> >  	};
> >  
> > +	vreg_edp_3p3: regulator-edp-3p3 {
> > +		compatible = "regulator-fixed";
> > +
> > +		regulator-name = "VREG_EDP_3P3";
> 
> Please use the net name from the schematics here (i.e. "VCC3LCD").
> 

This is the name used in the CRD schematics.

> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +
> > +		gpio = <&tlmm 25 GPIO_ACTIVE_HIGH>;
> > +		enable-active-high;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&edp_reg_en>;
> > +
> > +		regulator-boot-on;
> > +	};
> > +
> >  	vreg_edp_bl: regulator-edp-bl {
> >  		compatible = "regulator-fixed";
> >  
> > @@ -230,6 +246,54 @@ vreg_l9d: ldo9 {
> >  	};
> >  };
> >  
> > +&dispcc0 {
> > +	status = "okay";
> > +};
> > +
> > +&mdss0 {
> > +	status = "okay";
> > +};
> > +
> > +&mdss0_dp3 {
> > +	compatible = "qcom,sc8280xp-edp";
> > +	status = "okay";
> 
> Please move the status property last (i.e. after data-lanes).
> 

Sorry for missing that.

> > +
> > +	data-lanes = <0 1 2 3>;
> > +
> > +	aux-bus {
> > +		panel {
> > +			compatible = "edp-panel";
> > +			power-supply = <&vreg_edp_3p3>;
> > +
> > +			backlight = <&backlight>;
> > +
> > +			ports {
> > +				port {
> > +					edp_panel_in: endpoint {
> > +						remote-endpoint = <&mdss0_dp3_out>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> > +	ports {
> > +		port@1 {
> > +			reg = <1>;
> > +			mdss0_dp3_out: endpoint {
> > +				remote-endpoint = <&edp_panel_in>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&mdss0_dp3_phy {
> > +	status = "okay";
> 
> Same here.
> 

Ditto.

> > +
> > +	vdda-phy-supply = <&vreg_l6b>;
> > +	vdda-pll-supply = <&vreg_l3b>;
> > +};
> > +
> >  &pcie2a {
> >  	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
> >  	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
> > @@ -496,6 +560,12 @@ hastings_reg_en: hastings-reg-en-state {
> >  &tlmm {
> >  	gpio-reserved-ranges = <74 6>, <83 4>, <125 2>, <128 2>, <154 7>;
> >  
> > +	edp_reg_en: edp-reg-en-state {
> > +		pins = "gpio25";
> > +		function = "gpio";
> > +		output-enable;
> 
> 'output-enable' is not valid for tlmm and causes the settings to be
> rejected:
> 
> 	sc8280xp-tlmm f100000.pinctrl: pin_config_group_set op failed for group 25
> 	reg-fixed-voltage regulator-edp-3p3: Error applying setting, reverse things back
> 

Thanks for spotting that, it doesn't seem to be needed for the gpio-regulator
driver anyways...

Regards,
Bjorn

> > +	};
> > +
> >  	kybd_default: kybd-default-state {
> >  		disable-pins {
> >  			pins = "gpio102";
> 
> Johan
  

On Tue, Dec 13, 2022 at 07:10:14AM -0800, Bjorn Andersson wrote:
> On Fri, Dec 09, 2022 at 11:35:23AM +0100, Johan Hovold wrote:
 
> > > +	edp_reg_en: edp-reg-en-state {
> > > +		pins = "gpio25";
> > > +		function = "gpio";
> > > +		output-enable;
> > 
> > 'output-enable' is not valid for tlmm and causes the settings to be
> > rejected:
> > 
> > 	sc8280xp-tlmm f100000.pinctrl: pin_config_group_set op failed for group 25
> > 	reg-fixed-voltage regulator-edp-3p3: Error applying setting, reverse things back
> > 
> 
> Thanks for spotting that, it doesn't seem to be needed for the gpio-regulator
> driver anyways...

I noticed that the firmware on both CRD and X13s sets the drive strength
to 16 here. Should we specify that too (and disable the pull up)
instead of relying on the firmware configuration?

Johan