linux-kernel - Re: [PATCH v8 4/4] drm/tidss: Add OLDI bridge support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DA6PRDARLY70.1CILNJ8YLIOA1@kernel.org>
Date: Tue, 27 May 2025 08:02:46 +0200
From: "Michael Walle" <mwalle@...nel.org>
To: "Aradhya Bhatia" <aradhya.bhatia@...ux.dev>, "Rob Herring"
 <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor
 Dooley" <conor+dt@...nel.org>, "Tomi Valkeinen"
 <tomi.valkeinen@...asonboard.com>, "Jyri Sarha" <jyri.sarha@....fi>
Cc: "Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>, "Thomas
 Zimmermann" <tzimmermann@...e.de>, "Maxime Ripard" <mripard@...nel.org>,
 "David Airlie" <airlied@...il.com>, "Laurent Pinchart"
 <laurent.pinchart@...asonboard.com>, "Simona Vetter" <simona@...ll.ch>,
 "Nishanth Menon" <nm@...com>, "Vignesh Raghavendra" <vigneshr@...com>,
 "Devarsh Thakkar" <devarsht@...com>, "Praneeth Bajjuri" <praneeth@...com>,
 "Udit Kumar" <u-kumar1@...com>, "Jayesh Choudhary" <j-choudhary@...com>,
 "Francesco Dolcini" <francesco@...cini.it>, "Alexander Sverdlin"
 <alexander.sverdlin@...mens.com>, "DRI Development List"
 <dri-devel@...ts.freedesktop.org>, "Devicetree List"
 <devicetree@...r.kernel.org>, "Linux Kernel List"
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 4/4] drm/tidss: Add OLDI bridge support

Hi Aradhya,

On Mon May 26, 2025 at 4:17 PM CEST, Aradhya Bhatia wrote:
> Thank you for reviewing and testing the patches! =)

Thank you for your dedication to bring this feature upstream :)

> On 26/05/25 15:05, Michael Walle wrote:
> > 
> >> +static int get_oldi_mode(struct device_node *oldi_tx, int *companion_instance)
> >> +{
> >> +	struct device_node *companion;
> >> +	struct device_node *port0, *port1;
> >> +	u32 companion_reg;
> >> +	bool secondary_oldi = false;
> >> +	int pixel_order;
> >> +
> >> +	/*
> >> +	 * Find if the OLDI is paired with another OLDI for combined OLDI
> >> +	 * operation (dual-link or clone).
> >> +	 */
> >> +	companion = of_parse_phandle(oldi_tx, "ti,companion-oldi", 0);
> >> +	if (!companion)
> >> +		/*
> >> +		 * The OLDI TX does not have a companion, nor is it a
> >> +		 * secondary OLDI. It will operate independently.
> >> +		 */
> >> +		return OLDI_MODE_SINGLE_LINK;
> > 
> > How is this supposed to work? If I read this code correctly, the
> > second (companion) port is always reported as SINGLE_LINK if its
> > device tree node doesn't have a ti,companion-oldi property. But
> > reading the device tree binding, the companion-old property is only
> > for the first OLDI port.
>
> With this series, the dt-schema for oldi changes a bit as well. Both the
> OLDIs, primary or secondary, need to pass each other's phandles now.
> The "ti,companion-oldi" and "ti,secondary-oldi" properties are not
> mutually exclusive anymore.

Ok, I thought so. But then you'll have to update the binding doc and
example (Patch 2/3) ;)

> Something like this.
>
> &oldi0 {
> 	// primary oldi
> 	ti,companion-oldi = <&oldi1>;
> };
>
>
> &oldi1 {
> 	// secondary oldi
> 	ti,secondary-oldi = true;
> 	ti,companion-oldi = <&oldi0>;
> };
>
>
> If there is no companion for any OLDI dt node, then the OLDI TX will be
> deemed as acting by itself, and in a single-link mode.

And it's possible to still have these properties and treat them as
two distinct transmitters? I'm wondering if it's possible to have
the companion-oldi and secondary-oldi property inside the generic
SoC dtsi, so you don't have to repeat it in every board dts.

If I read the code correctly, the panel has to have the even and odd
pixel properties to be detected as dual-link. Correct? Thus it would
be possible to have

oldi0: oldi@0 {
 	ti,companion-oldi = <&oldi1>;
};

oldi1: oldi@1 {
 	ti,secondary-oldi;
 	ti,companion-oldi = <&oldi0>;
};

in the soc.dtsi and in a board dts:

panel {
	port {
		remote-endpoint = <&oldi0>;
	};
};

Or with a dual link panel:

dualpanel {
	ports {
		port@0 {
			dual-lvds-odd-pixels;
			remote-endpoint = <&oldi0>;
		};

		port@1 {
			dual-lvds-even-pixels;
			remote-endpoint = <&oldi1>;
		};
	};
};

> > 
> > FWIW, I've tested this series and I get twice the clock rate as
> > expected and the second link is reported as "OLDI_MODE_SINGLE_LINK".
> > I'll dig deeper into this tomorrow.
> >
>
> I was able to reproduce this behavior as you mention when the second
> oldi dt does not have a companion-oldi property.
>
> However, upon analysis, I realize that even having the correct dt as I
> mention above, will fall into another bug in the code and fail during
> the OLDI init.
>
> Unfortunately, two wrongs in my setup yesterday caused my testing to
> pass!
>
> I will post another revision, if you want to hold out on debugging
> further!

Sure!

-michael

Download attachment "signature.asc" of type "application/pgp-signature" (298 bytes)