LinuxLists.cc - Apple Thunderbolt Display chaining

2022-03-30 04:06:46

Subject: Apple Thunderbolt Display chaining

G'day Mika,

Back in 2019 you assisted me with an issue on an iMac with a "Light
Ridge" controller running a pair of Apple 27" Thunderbolt displays. At
the time I commented they worked when plugged into a port each, however
they don't work when chained.

Back then things crashed horribly. Now they don't crash, and the PCI
devices all work but there is an issue with the display.

I'm currently testing on vanilla git-head commit
ae085d7f9365de7da27ab5c0d16b12d51ea7fca9 (Sunday March 27th). A recent
kernel with all the "for 5.18" patches.

On both the iMac with the "Light Ridge" controller and a MacBookPro with
a "Falcon Ridge" controller the result is the same.

Plugged into a port each, they work perfectly.

If I chain them, the first one plugged in works and when the second is
plugged in it's almost like the link is being starved of bandwidth.
Graphical displays like a desktop, or a terminal break up, tear or
exhibit almost white noise/snow on both displays. Solid colours with no
gradient sometimes displays cleanly until something else is displayed
and then it breaks up.

I've attached 3 clean-boot dmesg from the Laptop.

001 is "Boot up, then plug one display into each port".
002 is "Boot up, plug a display into one port and then plug the second
display into the first".
003 is "Chain the displays together, boot up then plug the first chained
display in".

002 and 003 wind up with the same result and the same corruption. The
corruption is also identical on the iMac with a different GPU and the
older "Light Ridge" controller.

All of these configurations work under OSX (10.13 on the iMac and 10.15
on the Laptop).

This isn't important or urgent, however any insight you can provide
would be appreciated.

Regards,
Brad

Attachments:

001 (131.46 kB)
002 (135.38 kB)
003 (131.87 kB)
Download all attachments

2022-03-30 17:13:21

by Mika Westerberg

[permalink] [raw]

Subject: Re: Apple Thunderbolt Display chaining

Hi,

On Wed, Mar 30, 2022 at 10:24:35PM +0800, Brad Campbell wrote:
> Nope, that did the same thing. I wonder though. I'm testing it on the laptop and that reports :
> [ 0.442832] thunderbolt 0000:07:00.0: Thunderbolt 2 Switch: 8086:156d (Revision: 0, TB Version: 2)
>
> Changing "if (in->sw->generation == 1)" to "if (in->sw->generation == 2)" on the laptop solves that.

Heh, indeed I forgot that this is Falcon Ridge.

> I can't test hotplug properly on the iMac due to the radeon training issue.
>
> The laptop still has the issue of a cold boot working in one socket
> and not the other, but hot plug is working correctly.

Let's try this one next:

diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index a473cc7d9a8d..7150b5bc5403 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -865,6 +865,8 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
struct tb_tunnel *tunnel;
struct tb_path **paths;
struct tb_path *path;
+ struct tb_port *port;
+ int link_nr;

if (WARN_ON(!in->cap_adap || !out->cap_adap))
return NULL;
@@ -883,22 +885,34 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,

paths = tunnel->paths;

+ /*
+ * Hard code the lane for both DP IN adapters when first
+ * generation hardware is present in the topology.
+ */
+ link_nr = 1;
+ tb_for_each_port_on_path(in, out, port) {
+ if (tb_port_is_null(port) && port->sw->generation == 1) {
+ link_nr = in->port == 11 ? 1 : 0;
+ break;
+ }
+ }
+
path = tb_path_alloc(tb, in, TB_DP_VIDEO_HOPID, out, TB_DP_VIDEO_HOPID,
- 1, "Video");
+ link_nr, "Video");
if (!path)
goto err_free;
tb_dp_init_video_path(path);
paths[TB_DP_VIDEO_PATH_OUT] = path;

path = tb_path_alloc(tb, in, TB_DP_AUX_TX_HOPID, out,
- TB_DP_AUX_TX_HOPID, 1, "AUX TX");
+ TB_DP_AUX_TX_HOPID, link_nr, "AUX TX");
if (!path)
goto err_free;
tb_dp_init_aux_path(path);
paths[TB_DP_AUX_PATH_OUT] = path;

path = tb_path_alloc(tb, out, TB_DP_AUX_RX_HOPID, in,
- TB_DP_AUX_RX_HOPID, 1, "AUX RX");
+ TB_DP_AUX_RX_HOPID, link_nr, "AUX RX");
if (!path)
goto err_free;
tb_dp_init_aux_path(path);

2022-03-31 02:20:42

by Mika Westerberg

[permalink] [raw]

Subject: Re: Apple Thunderbolt Display chaining

Hi,

On Wed, Mar 30, 2022 at 09:19:52PM +0800, Brad Campbell wrote:
> Hey Mika,
>
> On 30/3/22 18:18, Mika Westerberg wrote:
> > Hi,
> >
> > On Tue, Mar 29, 2022 at 10:06:35PM +0800, Brad Campbell wrote:
> >>> Indeed, I did not add this to the "discovery" path yet.
> >>>
> >>> I wonder what happens if you change this:
> >>>
> >>> + tunnel = tb_tunnel_alloc_dp(tb, in, out, available_up, available_down,
> >>> + first ? 0 : 1);
> >>>
> >>> to this in your tree:
> >>>
> >>> + tunnel = tb_tunnel_alloc_dp(tb, in, out, available_up, available_down,
> >>> + first ? 1 : 0);
> >>>
> >>
> >> Here's where it gets all "Apple..y". On the iMac this does the job no matter which
> >> port the chain is plugged into. Boots and displays correctly first time, every time.
> >>
> >> It turns out on the laptop, one port works and the other doesn't. Changing the order
> >> simply changes which port works. So I assume the EFI sets up the first display using
> >> the first lane if it's in the first port, and the second if it's in the second.
> >>
> >> That means had I managed to perform the first test in the "other port" consistently,
> >> it would have worked there also.
> >
> > Can you try the below patch too? I hard-code the lane based on the
> > DP adapter number in TBT gen1.
> >
> > Let's first figure out proper solution to this issue and then look at
> > the other one.
> >
> > diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
> > index a473cc7d9a8d..97d36a7bb527 100644
> > --- a/drivers/thunderbolt/tunnel.c
> > +++ b/drivers/thunderbolt/tunnel.c
> > @@ -865,6 +865,7 @@ struct tb_tunnel *tb_tunnel_alloc_dp(struct tb *tb, struct tb_port *in,
>
> This one works from cold boot on both machines regardless of the port the chain is plugged into.
> It fails on both machines on any hotplug with the symptoms of allocating them both the same link.
> I added an extra debug into tunnel.c and verified that.

Hm, okay. What if you change this:

link_nr = in->port == 11 ? 1 : 0;

to this

link_nr = in->port == 11 ? 0 : 1;

Please also keep the debugging you added too so we can see if it now
uses both lanes.

2022-03-31 02:36:15