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
signature.asc
Description: PGP signature
