Re: [PATCH 1/2] arm64: dts: qcom: monaco: add lt8713sx bridge with displayport

[Vishnu Saini <vishnu.saini@oss.qualcomm.com>]

On Thu, Nov 20, 2025 at 08:26:40AM -0600, Bjorn Andersson wrote:
> On Thu, Nov 20, 2025 at 04:28:05PM +0530, Vishnu Saini wrote:
> > Monaco-evk has LT8713sx which act as DP to 3 DP output
> > converter. Edp PHY from monaco soc is connected to lt8713sx
> > as input and output of lt8713sx is connected to 3 mini DP ports.
> > Two of these ports are available in mainboard and one port
> > is available on Mezz board. lt8713sx is connected to soc over
> > i2c0 and with reset gpio connected to pin6 or ioexpander5.
> > 
> 
> This is good, you're describing the role of LT8713sx and how it's
> connected on the board. Thank you for taking the time to do so!
> 
> I think one aspect that's worth pointing out on its own is the
> placement of the mini DP ports. You do capture it, but the fact that
> we have two on the EVK and one on the expansion board is "hidden" there
> in the middle of the paragraph.
> I think extracting this part of the message into its own paragraph would
> improve the commit message further.
>
Yes, Updated in V2.
 
> > Enable the edp nodes from monaco and enable lontium lt8713sx
> > bridge node.
> > 
> > Co-developed-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> > Signed-off-by: Prahlad Valluru <vvalluru@qti.qualcomm.com>
> > Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> > ---
> >  arch/arm64/boot/dts/qcom/monaco-evk.dts | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/monaco-evk.dts b/arch/arm64/boot/dts/qcom/monaco-evk.dts
> > index bb35893da73d..947807f8a9cb 100644
> > --- a/arch/arm64/boot/dts/qcom/monaco-evk.dts
> > +++ b/arch/arm64/boot/dts/qcom/monaco-evk.dts
> > @@ -317,6 +317,20 @@ &gpu_zap_shader {
> >  	firmware-name = "qcom/qcs8300/a623_zap.mbn";
> >  };
> >  
> 
> Would it be possible to add dp-connector nodes and wire them up to the
> &lt8713sx, like I did in sa8295-adp.dts?
>
Yes, added dp-connectors in V2.
 
> > +&i2c0 {
> > +	pinctrl-0 = <&qup_i2c0_default>;
> > +	pinctrl-names = "default";
> > +
> > +	status = "okay";
> > +
> > +	lt8713sx: lt8713sx@4f {
> 
> This should be bridge@4f.
> 
> Also, unless we need to reference this from a overlay there's no need to
> give it a label.
Yes, done.

> 
> > +		/*Display bridge chip, DP1.4/HDMI2.0/DP++ hub*/
> 
> This comment explains what "lontium,lt8713sx" is. The binding call tell
> me that, so the value of this comment would be for you to tell us what
> it is used for on this particular board - and if that's obvious you can
> omit the comment.

Yes, removed it.

> > +		compatible = "lontium,lt8713sx";
> > +		reg = <0x4f>;
> > +		reset-gpios = <&expander5 6 GPIO_ACTIVE_HIGH>;
> 
> In addition to using an of_graph to describe the connectors that this is
> wired to, it would be nice to have a port describing the relationship to
> the DP controller here as well - so we know where the signal is coming
> from.
> 
> Would that be possible to add?
Yes, added ports and connectors in v2.

> Regards,
> Bjorn
> 
> > +	};
> > +};
> > +
> >  &i2c1 {
> >  	pinctrl-0 = <&qup_i2c1_default>;
> >  	pinctrl-names = "default";
> > @@ -396,6 +410,23 @@ expander6: gpio@3e {
> >  	};
> >  };
> >  
> > +&mdss {
> > +	status = "okay";
> > +};
> > +
> > +&mdss_dp0 {
> > +	status = "okay";
> > +};
> > +
> > +&mdss_dp0_out {
> > +	data-lanes = <0 1 2 3>;
> > +	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
> > +};
> > +
> > +&mdss_dp0_phy {
> > +	status = "okay";
> > +};
> > +
> >  &iris {
> >  	status = "okay";
> >  };
> > @@ -435,6 +466,12 @@ &serdes0 {
> >  };
> >  
> >  &tlmm {
> > +	dp_hot_plug_det: dp-hot-plug-det-state {
> > +		pins = "gpio94";
> > +		function = "edp0_hot";
> > +		bias-disable;
> > +	};
> > +
> >  	ethernet0_default: ethernet0-default-state {
> >  		ethernet0_mdc: ethernet0-mdc-pins {
> >  			pins = "gpio5";
> > @@ -451,6 +488,13 @@ ethernet0_mdio: ethernet0-mdio-pins {
> >  		};
> >  	};
> >  
> > +	qup_i2c0_default: qup-i2c0-state {
> > +		pins = "gpio17", "gpio18";
> > +		function = "qup0_se0";
> > +		drive-strength = <2>;
> > +		bias-pull-up;
> > +	};
> > +
> >  	qup_i2c1_default: qup-i2c1-state {
> >  		pins = "gpio19", "gpio20";
> >  		function = "qup0_se1";
> > 
> > -- 
> > 2.34.1
> >