mbox series

[v2,0/4] Add device links between tunneled USB3 devices and USB4 Host

Message ID 20240830152630.3943215-1-mathias.nyman@linux.intel.com
Headers show
Series Add device links between tunneled USB3 devices and USB4 Host | expand

Message

Mathias Nyman Aug. 30, 2024, 3:26 p.m. UTC
The relationship between a USB4 Host Interface providing USB3 tunnels,
and tunneled USB3 devices is not represented in device hierarchy.

This caused issues with power managment as devices may suspend and
resume in incorrect order.
A device link between the USB4 Host Interface and the USB3 xHCI was
originally added to solve this, preventing the USB4 Host Interface from
suspending if the USB3 xHCI Host was still active.
This unfortunately also prevents USB4 Host Interface from suspending even
if the USB3 xHCI Host is only serving native non-tunneled USB devices.

Improve the current powermanagement situation by creating device links
directly from tunneled USB3 devices to the USB4 Host Interface they depend
on instead of a device link between the hosts.
This way USB4 host may suspend when the last tunneled device is
disconnected.

Intel xHCI hosts are capable of reporting if connected USB3 devices are
tunneled via vendor specific capabilities.
Use this until a standard way is available.

v2:
 Create device link by default if xHC host is not capable of tunnel
 detection but the USB3 ports have tunnel capability reported in ACPI
 _DSD object

Mathias Nyman (4):
  xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
  usb: Add tunnel_mode parameter to usb device structure
  usb: acpi: add device link between tunneled USB3 device and USB4 Host
    Interface
  thunderbolt: Don't create device link from USB4 Host Interface to USB3
    xHC host

 drivers/thunderbolt/acpi.c       | 40 ++++++------------------
 drivers/usb/core/usb-acpi.c      | 53 ++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-ext-caps.h |  5 +++
 drivers/usb/host/xhci-hub.c      | 36 ++++++++++++++++++++++
 drivers/usb/host/xhci.c          | 14 +++++++++
 drivers/usb/host/xhci.h          |  3 +-
 include/linux/usb.h              |  8 +++++
 7 files changed, 128 insertions(+), 31 deletions(-)

Comments

Mika Westerberg Sept. 4, 2024, 5 a.m. UTC | #1
Hi,

On Tue, Sep 03, 2024 at 10:35:00AM -0500, Mario Limonciello wrote:
> On 8/30/2024 10:26, Mathias Nyman wrote:
> > The relationship between a USB4 Host Interface providing USB3 tunnels,
> > and tunneled USB3 devices is not represented in device hierarchy.
> > 
> > This caused issues with power managment as devices may suspend and
> > resume in incorrect order.
> > A device link between the USB4 Host Interface and the USB3 xHCI was
> > originally added to solve this, preventing the USB4 Host Interface from
> > suspending if the USB3 xHCI Host was still active.
> > This unfortunately also prevents USB4 Host Interface from suspending even
> > if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> > 
> > Improve the current powermanagement situation by creating device links
> > directly from tunneled USB3 devices to the USB4 Host Interface they depend
> > on instead of a device link between the hosts.
> > This way USB4 host may suspend when the last tunneled device is
> > disconnected.
> > 
> > Intel xHCI hosts are capable of reporting if connected USB3 devices are
> > tunneled via vendor specific capabilities.
> > Use this until a standard way is available.
> > 
> > v2:
> >   Create device link by default if xHC host is not capable of tunnel
> >   detection but the USB3 ports have tunnel capability reported in ACPI
> >   _DSD object
> > 
> > Mathias Nyman (4):
> >    xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
> >    usb: Add tunnel_mode parameter to usb device structure
> >    usb: acpi: add device link between tunneled USB3 device and USB4 Host
> >      Interface
> >    thunderbolt: Don't create device link from USB4 Host Interface to USB3
> >      xHC host
> > 
> >   drivers/thunderbolt/acpi.c       | 40 ++++++------------------
> >   drivers/usb/core/usb-acpi.c      | 53 ++++++++++++++++++++++++++++++++
> >   drivers/usb/host/xhci-ext-caps.h |  5 +++
> >   drivers/usb/host/xhci-hub.c      | 36 ++++++++++++++++++++++
> >   drivers/usb/host/xhci.c          | 14 +++++++++
> >   drivers/usb/host/xhci.h          |  3 +-
> >   include/linux/usb.h              |  8 +++++
> >   7 files changed, 128 insertions(+), 31 deletions(-)
> > 
> 
> Hello,
> 
> I had a try with this version of the series but I'm missing a device link
> from XHCI controller after applying it.
> 
> Before series these are the two links (6.11-rc6):
> consumer:pci:0000:00:03.1 ->
> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1
> consumer:pci:0000:c4:00.3 ->
> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:c4:00.3
> 
> After 6.11-rc6 + v2 series only this link:
> consumer:pci:0000:00:03.1 ->
> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1

Do you have device connected? The link will now be created only when
there is a device router with USB 3.x hub/device connected.
Mario Limonciello Sept. 4, 2024, 5:38 p.m. UTC | #2
On 9/4/2024 00:00, Mika Westerberg wrote:
> Hi,
> 
> On Tue, Sep 03, 2024 at 10:35:00AM -0500, Mario Limonciello wrote:
>> On 8/30/2024 10:26, Mathias Nyman wrote:
>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>
>>> This caused issues with power managment as devices may suspend and
>>> resume in incorrect order.
>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>> originally added to solve this, preventing the USB4 Host Interface from
>>> suspending if the USB3 xHCI Host was still active.
>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>
>>> Improve the current powermanagement situation by creating device links
>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>> on instead of a device link between the hosts.
>>> This way USB4 host may suspend when the last tunneled device is
>>> disconnected.
>>>
>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>> tunneled via vendor specific capabilities.
>>> Use this until a standard way is available.
>>>
>>> v2:
>>>    Create device link by default if xHC host is not capable of tunnel
>>>    detection but the USB3 ports have tunnel capability reported in ACPI
>>>    _DSD object
>>>
>>> Mathias Nyman (4):
>>>     xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>     usb: Add tunnel_mode parameter to usb device structure
>>>     usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>       Interface
>>>     thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>       xHC host
>>>
>>>    drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>    drivers/usb/core/usb-acpi.c      | 53 ++++++++++++++++++++++++++++++++
>>>    drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>    drivers/usb/host/xhci-hub.c      | 36 ++++++++++++++++++++++
>>>    drivers/usb/host/xhci.c          | 14 +++++++++
>>>    drivers/usb/host/xhci.h          |  3 +-
>>>    include/linux/usb.h              |  8 +++++
>>>    7 files changed, 128 insertions(+), 31 deletions(-)
>>>
>>
>> Hello,
>>
>> I had a try with this version of the series but I'm missing a device link
>> from XHCI controller after applying it.
>>
>> Before series these are the two links (6.11-rc6):
>> consumer:pci:0000:00:03.1 ->
>> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1
>> consumer:pci:0000:c4:00.3 ->
>> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:c4:00.3
>>
>> After 6.11-rc6 + v2 series only this link:
>> consumer:pci:0000:00:03.1 ->
>> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1
> 
> Do you have device connected? The link will now be created only when
> there is a device router with USB 3.x hub/device connected.

Ah thanks for clarifying.
I connected a TBT4-UDZ and I see another link show up now.

0000:c4:00.6/consumer:usb:8-1 -> 
../../../virtual/devlink/pci:0000:c4:00.6--usb:8-1

However, something seems wrong with runtime PM when the device is 
disconnected.  Let me show you:

❯ ls -l /sys/bus/pci/drivers/thunderbolt/*/consumer*
lrwxrwxrwx 1 root root 0 Sep  4 12:33 
/sys/bus/pci/drivers/thunderbolt/0000:c4:00.5/consumer:pci:0000:00:03.1 
-> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1/
lrwxrwxrwx 1 root root 0 Sep  4 12:33 
/sys/bus/pci/drivers/thunderbolt/0000:c4:00.6/consumer:pci:0000:00:04.1 
-> ../../../virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1/
❯ cat /sys/bus/pci/drivers/thunderbolt/*/power/runtime_status
suspended
suspended

(connect dock)

❯ ls -l /sys/bus/pci/drivers/thunderbolt/*/consumer*
lrwxrwxrwx 1 root root 0 Sep  4 12:33 
/sys/bus/pci/drivers/thunderbolt/0000:c4:00.5/consumer:pci:0000:00:03.1 
-> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1/
lrwxrwxrwx 1 root root 0 Sep  4 12:33 
/sys/bus/pci/drivers/thunderbolt/0000:c4:00.6/consumer:pci:0000:00:04.1 
-> ../../../virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1/
lrwxrwxrwx 1 root root 0 Sep  4 12:34 
/sys/bus/pci/drivers/thunderbolt/0000:c4:00.6/consumer:usb:8-1 -> 
../../../virtual/devlink/pci:0000:c4:00.6--usb:8-1/
❯ cat /sys/bus/pci/drivers/thunderbolt/*/power/runtime_status
suspended
active

(unconnect dock and wait at least autosuspend_delay)

❯ ls -l /sys/bus/pci/drivers/thunderbolt/*/consumer*
lrwxrwxrwx 1 root root 0 Sep  4 12:33 
/sys/bus/pci/drivers/thunderbolt/0000:c4:00.5/consumer:pci:0000:00:03.1 
-> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1/
lrwxrwxrwx 1 root root 0 Sep  4 12:33 
/sys/bus/pci/drivers/thunderbolt/0000:c4:00.6/consumer:pci:0000:00:04.1 
-> ../../../virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1/
❯ cat /sys/bus/pci/drivers/thunderbolt/*/power/runtime_status
suspended
active

I would have expected the USB4 host router to go back into runtime PM, 
but it doesn't anymore until I reboot the system.
Mario Limonciello Sept. 5, 2024, 6:57 p.m. UTC | #3
On 9/5/2024 00:53, Mika Westerberg wrote:
> On Wed, Sep 04, 2024 at 12:38:15PM -0500, Mario Limonciello wrote:
>> ❯ ls -l /sys/bus/pci/drivers/thunderbolt/*/consumer*
>> lrwxrwxrwx 1 root root 0 Sep  4 12:33
>> /sys/bus/pci/drivers/thunderbolt/0000:c4:00.5/consumer:pci:0000:00:03.1 ->
>> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1/
>> lrwxrwxrwx 1 root root 0 Sep  4 12:33
>> /sys/bus/pci/drivers/thunderbolt/0000:c4:00.6/consumer:pci:0000:00:04.1 ->
>> ../../../virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1/
>> ❯ cat /sys/bus/pci/drivers/thunderbolt/*/power/runtime_status
>> suspended
>> active
>>
>> I would have expected the USB4 host router to go back into runtime PM, but
>> it doesn't anymore until I reboot the system.
> 
> Yes, it should enter runtime suspend after a while. Would you mind
> sharing dmesg around this?

I was capturing artifacts when all of a sudden it started to work.  Then 
I remembered I updated the BIOS on this system very recently.  This is a 
pre-production BIOS.

I downgraded back to old BIOS and everything works as you expect, so 
there is some BIOS bug at play.  I'll see if others can reproduce my 
result and drive a BIOS solution.

Sorry for the noise on the patch, all is great!

Tested-by: Mario Limonciello <mario.limonciello@amd.com>

> 
> I tried on my system and it works as expected (the device-power.sh is
> internal script that dumps the device power states, I can share if you
> want).
> 
> Devices 0d.2/3 are the host routers. 07.[0-3] are the PCIe tunnels (not
> really used here but shown for completeness, I don't have PCIe tunnel
> enabled to the hub). 0d.0 is the xHCI controller.
> 
> Plug in USB4 hub
> ----------------
> 
> # device-power.sh -s
> All PCI devices (software state)
> ...
> 0000:00:07.0 8086:7ec4 Status: [D3cold] Real status: [    D0] Runtime PM: [suspended]
> 0000:00:07.1 8086:7ec5 Status: [D3cold] Real status: [    D0] Runtime PM: [suspended]
> 0000:00:07.2 8086:7ec6 Status: [D3cold] Real status: [    D0] Runtime PM: [suspended]
> 0000:00:07.3 8086:7ec7 Status: [D3cold] Real status: [    D0] Runtime PM: [suspended]
> ...
> 0000:00:0d.0 8086:7ec0 Status: [D3cold] Real status: [    D0] Runtime PM: [suspended]
> 0000:00:0d.2 8086:7ec2 Status: [    D0] Real status: [    D0] Runtime PM: [   active]
> 0000:00:0d.3 8086:7ec3 Status: [    D0] Real status: [    D0] Runtime PM: [   active]
> 
> Unplug and wait for the autosuspend_delay
> -----------------------------------------
> 
> # device-power.sh -s
> All PCI devices (software state)
> ...
> 0000:00:07.0 8086:7ec4 Status: [D3cold] Real status: [D3cold] Runtime PM: [suspended]
> 0000:00:07.1 8086:7ec5 Status: [D3cold] Real status: [D3cold] Runtime PM: [suspended]
> 0000:00:07.2 8086:7ec6 Status: [D3cold] Real status: [D3cold] Runtime PM: [suspended]
> 0000:00:07.3 8086:7ec7 Status: [D3cold] Real status: [D3cold] Runtime PM: [suspended]
> ...
> 0000:00:0d.0 8086:7ec0 Status: [D3cold] Real status: [D3cold] Runtime PM: [suspended]
> 0000:00:0d.2 8086:7ec2 Status: [D3cold] Real status: [D3cold] Runtime PM: [suspended]
> 0000:00:0d.3 8086:7ec3 Status: [D3cold] Real status: [D3cold] Runtime PM: [suspended]
Mathias Nyman Sept. 6, 2024, 1:41 p.m. UTC | #4
On 5.9.2024 21.57, Mario Limonciello wrote:
> On 9/5/2024 00:53, Mika Westerberg wrote:
>> On Wed, Sep 04, 2024 at 12:38:15PM -0500, Mario Limonciello wrote:
>>> ❯ ls -l /sys/bus/pci/drivers/thunderbolt/*/consumer*
>>> lrwxrwxrwx 1 root root 0 Sep  4 12:33
>>> /sys/bus/pci/drivers/thunderbolt/0000:c4:00.5/consumer:pci:0000:00:03.1 ->
>>> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1/
>>> lrwxrwxrwx 1 root root 0 Sep  4 12:33
>>> /sys/bus/pci/drivers/thunderbolt/0000:c4:00.6/consumer:pci:0000:00:04.1 ->
>>> ../../../virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1/
>>> ❯ cat /sys/bus/pci/drivers/thunderbolt/*/power/runtime_status
>>> suspended
>>> active
>>>
>>> I would have expected the USB4 host router to go back into runtime PM, but
>>> it doesn't anymore until I reboot the system.
>>
>> Yes, it should enter runtime suspend after a while. Would you mind
>> sharing dmesg around this?
> 
> I was capturing artifacts when all of a sudden it started to work.  Then I remembered I updated the BIOS on this system very recently.  This is a pre-production BIOS.
> 
> I downgraded back to old BIOS and everything works as you expect, so there is some BIOS bug at play.  I'll see if others can reproduce my result and drive a BIOS solution.
> 
> Sorry for the noise on the patch, all is great!
> 
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> 

Thanks for testing it

-Mathias