Message-ID: <YxdgZavuLU78lqIL@black.fi.intel.com>
Date:   Tue, 6 Sep 2022 17:59:49 +0300
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Kai-Heng Feng <kai.heng.feng@...onical.com>
Cc:     andreas.noever@...il.com, michael.jamet@...el.com,
        YehezkelShB@...il.com, sanju.mehta@....com,
        mario.limonciello@....com, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] thunderbolt: Resume PCIe bridges after switch is found
 on AMD USB4 controller

On Tue, Sep 06, 2022 at 10:29:03PM +0800, Kai-Heng Feng wrote:
> On Tue, Sep 6, 2022 at 9:37 PM Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> >
> > On Tue, Sep 06, 2022 at 08:57:20PM +0800, Kai-Heng Feng wrote:
> > > On Mon, Sep 5, 2022 at 11:34 PM Mika Westerberg
> > > <mika.westerberg@...ux.intel.com> wrote:
> > > >
> > > > On Mon, Sep 05, 2022 at 11:21:36PM +0800, Kai-Heng Feng wrote:
> > > > > > Hmm, so you see the actual hotplug but the tunneled PCIe link may not be
> > > > > > detected? Does the PCIe "Card Present" (or Data Link Layer Active)
> > > > > > status change at all or is it always 0?
> > > > >
> > > > > It changes only after tb_switch_add() is called.
> > > >
> > > > I doubt tb_switch_add() does anything but instead it is the established
> > > > PCIe tunnel that then shows up as it toggles the Card Present bit or so.
> > > > But that should also trigger PME if the root port is in D3 so you should
> > > > see this wake if everything works accordingly (unless I'm missing
> > > > something).
> > >
> > > You are right. Sometimes it may still fail to detect hotplugged device
> > > right after tb_switch_add().
> > > At which point PCIe tunnels are established? Is it after tb_scan_port()?
> >
> > They are established when userspace writes "1" to ../authorized of the
> > device (not automatically).
> >
> > On Ubuntu that's boltd that handles this so you may need to disable it
> > before you do the experiment.
> 
> In the dmesg it was disabled and "authorized" was 0 originally.

OK.

> >
> > > I found that it's cleaner to wakeup hotplug ports via iterating device
> > > link consumers at the end of tb_scan_port().
> > >
> > > According to your commit b2be2b05cf3b1c7b499d3b05decdcc524879fea7
> > > ("thunderbolt: Create device links from ACPI description"), it states
> > > "The _DSD can be added to tunneled USB3 and PCIe ports, and is needed to
> > > make sure the USB4 NHI is resumed before any of the tunneled ports so
> > > the protocol tunnels get established properly before the actual port
> > > itself is resumed. Othwerwise the USB/PCI core find the link may not be
> > > established and starts tearing down the device stack."
> > >
> > > So isn't waking them up a logical thing to do here?
> >
> > No they should wake up themselves.
> 
> OK.
> 
> >
> > > > So if you do this:
> > > >
> > > > 1. Boot the system up, nothing connected
> > > > 2. Plug in the TBT/USB4 device but do not authorize the PCIe tunnel
> > > > 3. Wait for the TBT/USB4 domain to enter sleep (runtime suspend)
> > > > 4. Authorize the PCIe tunnel
> > > >
> > > >   # echo 1 > .../authorized
> > > >
> > > > The established PCIe tunnel should trigger PME and the root port then
> > > > should be able to detect the PCIe link. Can you add full dmesg with
> > > > "thunderbolt.dyndbg=+p" in the command line to the bug?
> > >
> > > dmesg attached. Unfortunately there's no PME.
> >
> > Hmm, attached to where? Forgot to attach? ;-)
> 
> Oops, it's attached to the Bugzilla now.

Okay I see in the dmesg now that the root port enters D3hot:

[   40.363249] pcieport 0000:00:04.1: saving config space at offset 0x34 (reading 0x50)
[   40.363257] pcieport 0000:00:04.1: saving config space at offset 0x38 (reading 0x0)
[   40.363265] pcieport 0000:00:04.1: saving config space at offset 0x3c (reading 0x200ff)
[   40.365052] pcieport 0000:00:04.1: PME# enabled

and then the PCIe tunnel is authorized:

[   58.936258] thunderbolt 0000:64:00.6: 0:5 <-> 2:6 (PCI): activating
[   58.936270] thunderbolt 0000:64:00.6: activating PCIe Down path from 0:5 to 2:6
[   58.936434] thunderbolt 0000:64:00.6: 2:1: Writing hop 1
[   58.936443] thunderbolt 0000:64:00.6: 2:1:  In HopID: 8 => Out port: 6 Out HopID: 8
[   58.936449] thunderbolt 0000:64:00.6: 2:1:   Weight: 1 Priority: 3 Credits: 32 Drop: 0
[   58.936453] thunderbolt 0000:64:00.6: 2:1:    Counter enabled: 0 Counter index: 2047
[   58.936457] thunderbolt 0000:64:00.6: 2:1:   Flow Control (In/Eg): 1/0 Shared Buffer (In/Eg): 0/0
[   58.936460] thunderbolt 0000:64:00.6: 2:1:   Unknown1: 0x0 Unknown2: 0x0 Unknown3: 0x0
[   58.936786] thunderbolt 0000:64:00.6: 0:5: Writing hop 0
[   58.936795] thunderbolt 0000:64:00.6: 0:5:  In HopID: 8 => Out port: 2 Out HopID: 8
[   58.936800] thunderbolt 0000:64:00.6: 0:5:   Weight: 1 Priority: 3 Credits: 7 Drop: 0
[   58.936803] thunderbolt 0000:64:00.6: 0:5:    Counter enabled: 0 Counter index: 2047
[   58.936807] thunderbolt 0000:64:00.6: 0:5:   Flow Control (In/Eg): 1/1 Shared Buffer (In/Eg): 0/0
[   58.936810] thunderbolt 0000:64:00.6: 0:5:   Unknown1: 0x0 Unknown2: 0x0 Unknown3: 0x0
[   58.936971] thunderbolt 0000:64:00.6: path activation complete
[   58.936979] thunderbolt 0000:64:00.6: activating PCIe Up path from 2:6 to 0:5
[   58.937129] thunderbolt 0000:64:00.6: 0:2: Writing hop 1
[   58.937138] thunderbolt 0000:64:00.6: 0:2:  In HopID: 8 => Out port: 5 Out HopID: 8
[   58.937143] thunderbolt 0000:64:00.6: 0:2:   Weight: 1 Priority: 3 Credits: 32 Drop: 0
[   58.937147] thunderbolt 0000:64:00.6: 0:2:    Counter enabled: 0 Counter index: 2047
[   58.937150] thunderbolt 0000:64:00.6: 0:2:   Flow Control (In/Eg): 1/0 Shared Buffer (In/Eg): 0/0
[   58.937154] thunderbolt 0000:64:00.6: 0:2:   Unknown1: 0x0 Unknown2: 0x0 Unknown3: 0x0
[   58.937453] thunderbolt 0000:64:00.6: 2:6: Writing hop 0
[   58.937462] thunderbolt 0000:64:00.6: 2:6:  In HopID: 8 => Out port: 1 Out HopID: 8
[   58.937467] thunderbolt 0000:64:00.6: 2:6:   Weight: 1 Priority: 3 Credits: 7 Drop: 0
[   58.937471] thunderbolt 0000:64:00.6: 2:6:    Counter enabled: 0 Counter index: 2047
[   58.937474] thunderbolt 0000:64:00.6: 2:6:   Flow Control (In/Eg): 1/1 Shared Buffer (In/Eg): 0/0
[   58.937478] thunderbolt 0000:64:00.6: 2:6:   Unknown1: 0x0 Unknown2: 0x0 Unknown3: 0x0
[   58.937633] thunderbolt 0000:64:00.6: path activation complete

but this is not detected by the root port so after a while the domain is
suspended as well:

[   84.327759] thunderbolt 0000:64:00.6: 0: suspending switch

What should happen right after the PCIe tunnel activation is that the root port
should get PME / hotplug and it should then find the devices but this for some
reason is not happening.

This reminded me that in Intel hardware there is an ACPI power resource that is
shared between related devices. IIRC there is _PR0() method under the root
port, xHCI and the TBT NHI that returns the same power resource. Now, when the
power resource is turned on for any of the devices the kernel wakes up the rest
too to make sure they get properly re-initialized if they went into
D0unitialized or something like that. The commit that added this is
4533771c1e53 ("ACPI / PM: Introduce concept of a _PR0 dependent device").

I'm not entirely sure if that applies here because none of the devices seem to
enter D3cold, though. Is this working on Windows side and do you know if it
goes into D3cold too?

Powered by blists - more mailing lists