diff mbox series

[v2,2/6] arm64: dts: qcom: qcs6490-rb3gen2: Add DP output

Message ID 20240326-rb3gen2-dp-connector-v2-2-a9f1bc32ecaf@quicinc.com (mailing list archive)
State Accepted
Commit 756efb7cb7293b0d971f661405e044cceab13d99
Headers show
Series arm64: dts: qcom: qcs6490-rb3gen2: Enable two displays | expand

Commit Message

Bjorn Andersson March 27, 2024, 2:04 a.m. UTC
The RB3Gen2 board comes with a mini DP connector, describe this, enable
MDSS, DP controller and the PHY that drives this.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 40 ++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Konrad Dybcio March 27, 2024, 5:20 p.m. UTC | #1
On 27.03.2024 3:04 AM, Bjorn Andersson wrote:
> The RB3Gen2 board comes with a mini DP connector, describe this, enable
> MDSS, DP controller and the PHY that drives this.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
Dmitry Baryshkov March 28, 2024, 1:51 a.m. UTC | #2
On Wed, 27 Mar 2024 at 04:04, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> The RB3Gen2 board comes with a mini DP connector, describe this, enable
> MDSS, DP controller and the PHY that drives this.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 40 ++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 63ebe0774f1d..f90bf3518e98 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -39,6 +39,20 @@ chosen {
>                 stdout-path = "serial0:115200n8";
>         };
>
> +       dp-connector {
> +               compatible = "dp-connector";
> +               label = "DP";
> +               type = "mini";
> +
> +               hpd-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;

Is it the standard hpd gpio? If so, is there any reason for using it
through dp-connector rather than as a native HPD signal?

> +
> +               port {
> +                       dp_connector_in: endpoint {
> +                               remote-endpoint = <&mdss_edp_out>;
> +                       };
> +               };
> +       };
> +
>         reserved-memory {
>                 xbl_mem: xbl@80700000 {
>                         reg = <0x0 0x80700000 0x0 0x100000>;
> @@ -471,6 +485,25 @@ &gcc {
>                            <GCC_WPSS_RSCP_CLK>;
>  };
>
> +&mdss {
> +       status = "okay";
> +};
> +
> +&mdss_edp {
> +       status = "okay";
> +};
> +
> +&mdss_edp_out {
> +       data-lanes = <0 1 2 3>;
> +       link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> +
> +       remote-endpoint = <&dp_connector_in>;
> +};
> +
> +&mdss_edp_phy {
> +       status = "okay";
> +};
> +
>  &qupv3_id_0 {
>         status = "okay";
>  };
> @@ -511,3 +544,10 @@ &usb_1_qmpphy {
>  &wifi {
>         memory-region = <&wlan_fw_mem>;
>  };
> +
> +/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
> +
> +&edp_hot_plug_det {
> +       function = "gpio";
> +       bias-disable;
> +};
>
> --
> 2.25.1
>
Bjorn Andersson March 28, 2024, 3:07 a.m. UTC | #3
On Thu, Mar 28, 2024 at 03:51:54AM +0200, Dmitry Baryshkov wrote:
> On Wed, 27 Mar 2024 at 04:04, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> >
> > The RB3Gen2 board comes with a mini DP connector, describe this, enable
> > MDSS, DP controller and the PHY that drives this.
> >
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 40 ++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > index 63ebe0774f1d..f90bf3518e98 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > @@ -39,6 +39,20 @@ chosen {
> >                 stdout-path = "serial0:115200n8";
> >         };
> >
> > +       dp-connector {
> > +               compatible = "dp-connector";
> > +               label = "DP";
> > +               type = "mini";
> > +
> > +               hpd-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
> 
> Is it the standard hpd gpio? If so, is there any reason for using it
> through dp-connector rather than as a native HPD signal?
> 

I added it because you asked for it. That said, I do like having it
clearly defined in the devicetree.

Regards,
Bjorn
Dmitry Baryshkov March 28, 2024, 7:17 a.m. UTC | #4
On Thu, 28 Mar 2024 at 05:07, Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Thu, Mar 28, 2024 at 03:51:54AM +0200, Dmitry Baryshkov wrote:
> > On Wed, 27 Mar 2024 at 04:04, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> > >
> > > The RB3Gen2 board comes with a mini DP connector, describe this, enable
> > > MDSS, DP controller and the PHY that drives this.
> > >
> > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 40 ++++++++++++++++++++++++++++
> > >  1 file changed, 40 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > index 63ebe0774f1d..f90bf3518e98 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > @@ -39,6 +39,20 @@ chosen {
> > >                 stdout-path = "serial0:115200n8";
> > >         };
> > >
> > > +       dp-connector {
> > > +               compatible = "dp-connector";
> > > +               label = "DP";
> > > +               type = "mini";
> > > +
> > > +               hpd-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
> >
> > Is it the standard hpd gpio? If so, is there any reason for using it
> > through dp-connector rather than as a native HPD signal?
> >
>
> I added it because you asked for it. That said, I do like having it
> clearly defined in the devicetree.

I asked for the dp-connector device, not for the HPD function change.
Bjorn Andersson March 29, 2024, 1:37 a.m. UTC | #5
On Thu, Mar 28, 2024 at 09:17:45AM +0200, Dmitry Baryshkov wrote:
> On Thu, 28 Mar 2024 at 05:07, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Thu, Mar 28, 2024 at 03:51:54AM +0200, Dmitry Baryshkov wrote:
> > > On Wed, 27 Mar 2024 at 04:04, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> > > >
> > > > The RB3Gen2 board comes with a mini DP connector, describe this, enable
> > > > MDSS, DP controller and the PHY that drives this.
> > > >
> > > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 40 ++++++++++++++++++++++++++++
> > > >  1 file changed, 40 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > index 63ebe0774f1d..f90bf3518e98 100644
> > > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > @@ -39,6 +39,20 @@ chosen {
> > > >                 stdout-path = "serial0:115200n8";
> > > >         };
> > > >
> > > > +       dp-connector {
> > > > +               compatible = "dp-connector";
> > > > +               label = "DP";
> > > > +               type = "mini";
> > > > +
> > > > +               hpd-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
> > >
> > > Is it the standard hpd gpio? If so, is there any reason for using it
> > > through dp-connector rather than as a native HPD signal?
> > >
> >
> > I added it because you asked for it. That said, I do like having it
> > clearly defined in the devicetree.
> 
> I asked for the dp-connector device, not for the HPD function change.
> 

I didn't realize that you could have a dp-connector device without
defining the hpd-gpios, but it looks like you're right.

Do we have any reason for using the internal HPD, when we're already
spending the memory to allocate the dp-connector device?


PS. It's recommended that you dynamically switch to GPIO-based HPD in
lower-power scenarios, as this allow you to turn off the DP controller
and still detect plug events...

Regards,
Bjorn
Dmitry Baryshkov March 29, 2024, 1:44 a.m. UTC | #6
On Fri, 29 Mar 2024 at 03:37, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> On Thu, Mar 28, 2024 at 09:17:45AM +0200, Dmitry Baryshkov wrote:
> > On Thu, 28 Mar 2024 at 05:07, Bjorn Andersson <andersson@kernel.org> wrote:
> > >
> > > On Thu, Mar 28, 2024 at 03:51:54AM +0200, Dmitry Baryshkov wrote:
> > > > On Wed, 27 Mar 2024 at 04:04, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> > > > >
> > > > > The RB3Gen2 board comes with a mini DP connector, describe this, enable
> > > > > MDSS, DP controller and the PHY that drives this.
> > > > >
> > > > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > > > > ---
> > > > >  arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 40 ++++++++++++++++++++++++++++
> > > > >  1 file changed, 40 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > > index 63ebe0774f1d..f90bf3518e98 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > > > > @@ -39,6 +39,20 @@ chosen {
> > > > >                 stdout-path = "serial0:115200n8";
> > > > >         };
> > > > >
> > > > > +       dp-connector {
> > > > > +               compatible = "dp-connector";
> > > > > +               label = "DP";
> > > > > +               type = "mini";
> > > > > +
> > > > > +               hpd-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
> > > >
> > > > Is it the standard hpd gpio? If so, is there any reason for using it
> > > > through dp-connector rather than as a native HPD signal?
> > > >
> > >
> > > I added it because you asked for it. That said, I do like having it
> > > clearly defined in the devicetree.
> >
> > I asked for the dp-connector device, not for the HPD function change.
> >
>
> I didn't realize that you could have a dp-connector device without
> defining the hpd-gpios, but it looks like you're right.
>
> Do we have any reason for using the internal HPD, when we're already
> spending the memory to allocate the dp-connector device?

No, no particular reason. I was trying to understand if there was any
reason for that from your side.

Then:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> PS. It's recommended that you dynamically switch to GPIO-based HPD in
> lower-power scenarios, as this allow you to turn off the DP controller
> and still detect plug events...

I don't think rb3g2 is a low-power device, but I think this is still a
valid argument.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
index 63ebe0774f1d..f90bf3518e98 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
@@ -39,6 +39,20 @@  chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	dp-connector {
+		compatible = "dp-connector";
+		label = "DP";
+		type = "mini";
+
+		hpd-gpios = <&tlmm 60 GPIO_ACTIVE_HIGH>;
+
+		port {
+			dp_connector_in: endpoint {
+				remote-endpoint = <&mdss_edp_out>;
+			};
+		};
+	};
+
 	reserved-memory {
 		xbl_mem: xbl@80700000 {
 			reg = <0x0 0x80700000 0x0 0x100000>;
@@ -471,6 +485,25 @@  &gcc {
 			   <GCC_WPSS_RSCP_CLK>;
 };
 
+&mdss {
+	status = "okay";
+};
+
+&mdss_edp {
+	status = "okay";
+};
+
+&mdss_edp_out {
+	data-lanes = <0 1 2 3>;
+	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
+
+	remote-endpoint = <&dp_connector_in>;
+};
+
+&mdss_edp_phy {
+	status = "okay";
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -511,3 +544,10 @@  &usb_1_qmpphy {
 &wifi {
 	memory-region = <&wlan_fw_mem>;
 };
+
+/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
+
+&edp_hot_plug_det {
+	function = "gpio";
+	bias-disable;
+};