Re: [PATCH v3 0/4] drm/tidss: Add OLDI bridge support

From: Tomi Valkeinen
Date: Mon Sep 09 2024 - 05:50:47 EST


On 09/09/2024 12:31, Aradhya Bhatia wrote:
Hi,

Thank you, Francesco and Max, for testing and reporting this!

On 09/09/24 13:45, Tomi Valkeinen wrote:
Hi,

On 06/09/2024 14:43, Francesco Dolcini wrote:
+Max

Hello Aradhya,

On Tue, Jul 16, 2024 at 02:12:44PM +0530, Aradhya Bhatia wrote:
The addition of the 2nd OLDI TX (and a 2nd DSS in AM62Px) creates a need
for some major changes for a full feature experience.

1. The OF graph needs to be updated to accurately show the data flow.
2. The tidss and OLDI drivers now need to support the dual-link and the
    cloned single-link OLDI video signals.
3. The drivers also need to support the case where 2 OLDI TXes are
    connected to 2 different VPs - thereby creating 2 independent
streams
    of single-link OLDI outputs.

Have you considered/tested the use case in which only single link is
used?
You do not mention it here and to me this is a relevant use case.

There is a workaround for this (use option 2, cloned, even if nothing is
connected to the second link), but this seems not correct.

I agree. The whole idea behind the series is to be able to accurately
describe the hardware. Putting the devicetree in clone mode in order to
get the single-link mode working is far from ideal.

Btw, with the fw_devlink=off hack, and removing the second link from k3-am625-sk-microtips-mf101hie-panel.dtso, is still not enough, as the k3-am62-main.dtsi contains the ti,companion-oldi property which makes the driver think it's a cloning case.

So, I think, either the ti,companion-oldi and ti,secondary-oldi should only be set in the overlay when setting up cloning/dual-link, or the companion-oldi property shouldn't actually make a difference, and the selection between clone and single-link should be done via some other means.

We (Max in Cc here) noticed that this specific use case is broken on
your downstream v6.6 TI branch.

Yes, it was been brought to my attention that the single-link usecase is
not working over the downstream ti-6.6 kernel. As I have since
discovered, it's not working on this series either.

For some reason, the supplier-consumer dependency between the OLDI and
the panel devicetree nodes is not getting flagged as `FWLINK_FLAG_CYCLE`
in cases of single-link configuration.

This flag allows the panel driver to continue to probe without waiting
for the OLDI driver (panel's supplier). Absence of the flag getting set
is causing these drivers to keep deferring probe in an endless cycle.

Even with the flag, the OLDI (and tidss) cannot complete probe until the
panel driver is probed and ready. That is because the OLDI (and tidss)
need the panel to continue with the bridge-chain creation.

However, over with the dual-lvds configuration (and as Francesco has
now mentioned the clone configuration as well), the flag gets set by
default, and everything works.

This is what the debug has led to, so far.

Yes, I came to the same conclusion with my debug.


What if you set "fw_devlink=off" kernel boot parameter?


Yes! I haven't tested it, but it seems that this will bypass the
supplier check and let the panel probe continue.


Tomi, any idea, why is this issue happening only for single-link in the
first place? It seems as if having 2 ports inside the panel devicetree
lets the probe mechanism recognize the circular dependency and ignore
the supplier OLDIs?

I have to say I have no idea...

I don't really understand the devlink code here, but I'm guessing that the "cycle" part comes from the fact that with a media graph we have links (remote-endpoint) both ways in the DT data. So it's not possible to say which side is the consumer, which one is the supplier. Thus, it's marked as a cycle and, I think, basically ignored for the probing purpose.

But why not here? I can see the links being created both ways:

/display Linked as a fwnode consumer to /bus@f0000/dss@30200000/oldi-txes/oldi@0
/bus@f0000/dss@30200000/oldi-txes/oldi@0 Linked as a fwnode consumer to /display

but it's never marked as a cycle.

This is the function where the difference comes down to, by the way -
__fw_devlink_relax_cycles(), per my knowledge. I am still on my way to
understand what exactly it is doing and why is it not relaxing the
single-link case.

Yep. The answer is probably somewhere there =).

Tomi