Hi Michael,
On 27/05/25 11:32, Michael Walle wrote:
> 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) ;)
>
Ah, that's right. The example wasn't updated there. Thank you! =)
>> 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>;
> };
> };
In this case, the secondary OLDI (oldi1) would remain disabled from
soc.dtsi.
I gave this a quick try. Turns out, of_parse_phandle() is not able to
return an error when primary OLDI tries to find a companion -- which is
important for the driver to detect an absence of any secondary OLDI.
Since the driver code registers a companion for primary OLDI, and
further does not find the "dual-lvds-{odd,even}-pixels" properties,
the driver ends up trying for a Clone Mode.
So, for single-link , we'd have to actively delete the "companion-oldi"
property, in the board.dts/panel.dtso.
But, say, the disabled-node's phandle parse is circumvented, somehow,
and we don't need to delete the property explicitly.
There would still be one concern, I am afraid.
In AM67A DSS (future scope at the moment), the 2 OLDIs can act
independently. Like a 2x Independent Single-Link. Both the OLDI dt nodes
will be enabled.
So, if the soc.dtsi has them already connected, then the
board.dts/panel.dtso would still need to explicitly delete those
properties to get the 2 OLDI TXes to work independently.
--
Regards
Aradhya
>
> 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
