Re: Apple Thunderbolt Display chaining

[Mika Westerberg]

On Fri, Apr 01, 2022 at 11:05:27PM +0800, Brad Campbell wrote:
> On 1/4/22 22:30, Mika Westerberg wrote:
> > Hi,
> > 
> > On Fri, Apr 01, 2022 at 01:48:13PM +0800, Brad Campbell wrote:
> > > That I can do. I didn't crop or grep as I wasn't sure what might be additionally relevant.
> > > 2 machines, 2 ports, 4 cold boots. Each time just the Thnuderbolt displays chained.
> > 
> > Thanks for the logs! It looks like the Apple EFI CM always uses the lane
> > 1 adapter.
> 
> Thanks for sticking with me. This must be a minuscule corner case that
> is buried in a dark room behind a filing cabinet.

Well, it is just that nobody else reported the issue before ;-) I don't
have Light Ridge or Falcon Ridge based Apple hardware anymore so these
won't show up in my regular testing.

> > Can you try the below patch? Fingers crossed that it solves the
> > chaining issue for both ;-) I had to patch the test.c because otherwise
> > unit tests fail to compile when enabled.
> > 
> > This should now setup the second tunnel always through the lane 0
> > adapter and thus share the bandwidth equally between the two lanes.
> > 
> > @@ -1604,7 +1604,7 @@ static void tb_test_tunnel_3dp(struct kunit *test)
> >   	KUNIT_ASSERT_EQ(test, tunnel1->npaths, 3);
> >   	KUNIT_ASSERT_EQ(test, tunnel1->paths[0]->path_length, 3);
> > -	tunnel2 = tb_tunnel_alloc_dp(NULL, in2, out2, 0, 0);
> > +	tunnel2 = tb_tunnel_alloc_dp(NULL, in2, out2, 1, 0, 0);
> >   	KUNIT_ASSERT_TRUE(test, tunnel2 != NULL);
> >   	KUNIT_EXPECT_EQ(test, tunnel2->type, TB_TUNNEL_DP);
> >   	KUNIT_EXPECT_PTR_EQ(test, tunnel2->src_port, in2);
> > @@ -1612,7 +1612,7 @@ static void tb_test_tunnel_3dp(struct kunit *test)
> >   	KUNIT_ASSERT_EQ(test, tunnel2->npaths, 3);
> >   	KUNIT_ASSERT_EQ(test, tunnel2->paths[0]->path_length, 4);
> > -	tunnel3 = tb_tunnel_alloc_dp(NULL, in3, out3, 0, 0);
> > +	tunnel3 = tb_tunnel_alloc_dp(NULL, in3, out3, 1, 0, 0);
> >   	KUNIT_ASSERT_TRUE(test, tunnel3 != NULL);
> >   	KUNIT_EXPECT_EQ(test, tunnel3->type, TB_TUNNEL_DP);
> >   	KUNIT_EXPECT_PTR_EQ(test, tunnel3->src_port, in3);
> 
> These 2 chunks don't apply.
> I can't find a tb_test_tunnel_3dp in any of the trees I have (either
> head or stable). In any case I've updated to current head and applied
> without those chunks.

Indeed, I forgot to drop that hunk. It is based on my local branch that
has some additional code but good that you managed to work it around.

> This fixes *all* cases on the MacBookPro. Cold boot on either port, and hot plug likewise

That's great!

> On the iMac it cold boots on either port. If I boot with the displays plugged in, then unplug-replug we get :
> 
> Apr  1 22:49:41 bkmac kernel: [   24.480306] [drm:radeon_dp_link_train [radeon]] *ERROR* displayport link status failed
> Apr  1 22:49:41 bkmac kernel: [   24.481309] [drm:radeon_dp_link_train [radeon]] *ERROR* clock recovery failed
> 
> The last head on the chain comes up, the first doesn't.
> 
> If I plug the displays in *after* the chime and before the kernel starts up, then both heads come up and hotplug works fine excepting after the second unplug we get lots of this :
> 
> Apr  1 22:52:54 bkmac kernel: [   37.313493] thunderbolt 0000:07:00.0: 0: timeout reading config space 1 from 0x39
> Apr  1 22:52:54 bkmac kernel: [   37.313513] thunderbolt 0000:07:00.0: 0:c: DP IN available
> Apr  1 22:52:54 bkmac kernel: [   37.763482] thunderbolt 0000:07:00.0: 0: timeout reading config space 1 from 0x39
> Apr  1 22:52:54 bkmac kernel: [   37.763489] thunderbolt 0000:07:00.0: 0:d: DP IN available
> Apr  1 22:52:54 bkmac kernel: [   37.763491] thunderbolt 0000:07:00.0: no suitable DP IN adapter available, not tunneling
> Apr  1 22:52:54 bkmac kernel: [   37.763495] thunderbolt 0000:07:00.0: 0:4: got unplug event for disconnected port, ignoring
> Apr  1 22:52:54 bkmac kernel: [   37.763496] thunderbolt 0000:07:00.0: 0:3: got unplug event for disconnected port, ignoring
> Apr  1 22:52:54 bkmac kernel: [   37.763498] thunderbolt 0000:07:00.0: 0:3: got unplug event for disconnected port, ignoring
> 
> And the thunderbolt ports then fail to respond to any plug event until rebooted.

This makes me suspect that there is something else still going on with
the Light Ridge controller that we are missing in the Linux driver.

> Feels like it's nailed excepting some DP routing on the Radeon, but
> I've not been able to put my finger on how that works.
> With your very first patch, hotplug works on the iMac.

Hm, you mean you don't see the timeout errors and stuff with the first
patch applied?

I think I will make this current one a proper patch and submit upstream
later this week (will CC you too). For the iMac issue we may need to
debug it further. Not sure if you answered this one already but on iMac
on macOS does it work always when you plug in the whole chain?