| Message ID | 20240613054204.5850-1-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Verify devices transition from D3cold to D0 | expand |
Hi Mario, On Thu, Jun 13, 2024 at 12:42:00AM -0500, Mario Limonciello wrote: > Gary has reported that when a dock is plugged into a system at the same > time the autosuspend delay has tripped that the USB4 stack malfunctions. > > Messages show up like this: > > ``` > thunderbolt 0000:e5:00.6: ring_interrupt_active: interrupt for TX ring 0 is already enabled > ``` > > Furthermore the USB4 router is non-functional at this point. Once the USB4 domain starts the sleep transition, it cannot be interrupted by anything so it always should go through full sleep transition and only then back from sleep. > Those messages happen because the device is still in D3cold at the time > that the PCI core handed control back to the USB4 connection manager > (thunderbolt). This is weird. Yes we should be getting the wake from the hotplug but that should happen only after the domain is fully in sleep (D3cold). The BIOS ACPI code is supposed to deal with this. > The issue is that it takes time for a device to enter D3cold and do a > conventional reset, and then more time for it to exit D3cold. > > This appears not to be a new problem; previously there were very similar > reports from Ryzen XHCI controllers. Quirks were added for those. > Furthermore; adding extra logging it's apparent that other PCI devices > in the system can take more than 10ms to recover from D3cold as well. They can take anything up to 100ms after the link has trained.
On 6/18/2024 08:14, Mika Westerberg wrote: > Hi Mario, > > On Thu, Jun 13, 2024 at 12:42:00AM -0500, Mario Limonciello wrote: >> Gary has reported that when a dock is plugged into a system at the same >> time the autosuspend delay has tripped that the USB4 stack malfunctions. >> >> Messages show up like this: >> >> ``` >> thunderbolt 0000:e5:00.6: ring_interrupt_active: interrupt for TX ring 0 is already enabled >> ``` >> >> Furthermore the USB4 router is non-functional at this point. > > Once the USB4 domain starts the sleep transition, it cannot be > interrupted by anything so it always should go through full sleep > transition and only then back from sleep. > >> Those messages happen because the device is still in D3cold at the time >> that the PCI core handed control back to the USB4 connection manager >> (thunderbolt). > > This is weird. Yes we should be getting the wake from the hotplug but > that should happen only after the domain is fully in sleep (D3cold). The > BIOS ACPI code is supposed to deal with this. Is that from from experience or do you mean there is a spec behavior? IE I'm wondering if we have different "expectations" from different company's hardware designers. > >> The issue is that it takes time for a device to enter D3cold and do a >> conventional reset, and then more time for it to exit D3cold. >> >> This appears not to be a new problem; previously there were very similar >> reports from Ryzen XHCI controllers. Quirks were added for those. >> Furthermore; adding extra logging it's apparent that other PCI devices >> in the system can take more than 10ms to recover from D3cold as well. > > They can take anything up to 100ms after the link has trained. Right; so currently there is nothing checking they really made it back to D0 after calling pci_power_up(). I feel like we've "mostly" gotten lucky. If you guys agree with this patch series direction this could potentially mean cleaning up more code that exists for random delays in the PCI core too.
On Tue, Jun 18, 2024 at 11:56:50AM -0500, Mario Limonciello wrote: > On 6/18/2024 08:14, Mika Westerberg wrote: > > Hi Mario, > > > > On Thu, Jun 13, 2024 at 12:42:00AM -0500, Mario Limonciello wrote: > > > Gary has reported that when a dock is plugged into a system at the same > > > time the autosuspend delay has tripped that the USB4 stack malfunctions. > > > > > > Messages show up like this: > > > > > > ``` > > > thunderbolt 0000:e5:00.6: ring_interrupt_active: interrupt for TX ring 0 is already enabled > > > ``` > > > > > > Furthermore the USB4 router is non-functional at this point. > > > > Once the USB4 domain starts the sleep transition, it cannot be > > interrupted by anything so it always should go through full sleep > > transition and only then back from sleep. > > > > > Those messages happen because the device is still in D3cold at the time > > > that the PCI core handed control back to the USB4 connection manager > > > (thunderbolt). > > > > This is weird. Yes we should be getting the wake from the hotplug but > > that should happen only after the domain is fully in sleep (D3cold). The > > BIOS ACPI code is supposed to deal with this. > > Is that from from experience or do you mean there is a spec behavior? > > IE I'm wondering if we have different "expectations" from different > company's hardware designers. The spec and the CM guide "imply" this behaviour as far as I can tell, so that the "sleep event" is done completely once started. I guess this can be interpreted differently too because it is not explicitly said there. Can you ask AMD HW folks if this is their interpretation too? Basically when we get "Sleep Ready" bit set for all the routers in the domain and turn off power (send PERST) there cannot be wake events until that is fully completed. There is typically a timeout mechanism in the BIOS side (part of the power off method) that waits for the PCIe links to enter L2 before it triggers PERST. We have seen an issue on our side that if this L2 transition is not completed in time a wake event triggered but that was a BIOS issue. > > > The issue is that it takes time for a device to enter D3cold and do a > > > conventional reset, and then more time for it to exit D3cold. > > > > > > This appears not to be a new problem; previously there were very similar > > > reports from Ryzen XHCI controllers. Quirks were added for those. > > > Furthermore; adding extra logging it's apparent that other PCI devices > > > in the system can take more than 10ms to recover from D3cold as well. > > > > They can take anything up to 100ms after the link has trained. > > Right; so currently there is nothing checking they really made it back to D0 > after calling pci_power_up(). I feel like we've "mostly" gotten lucky. We do have pci_bridge_wait_for_secondary_bus() there but I guess that's not called for integrated PCIe endpoints such as xHCI and the USB4 Host Interface. They too enter D3cold once power is turned off so I agree we might have gotten lucky here with the D3hot 10ms delay.
On 6/19/2024 00:29, Mika Westerberg wrote: > On Tue, Jun 18, 2024 at 11:56:50AM -0500, Mario Limonciello wrote: >> On 6/18/2024 08:14, Mika Westerberg wrote: >>> Hi Mario, >>> >>> On Thu, Jun 13, 2024 at 12:42:00AM -0500, Mario Limonciello wrote: >>>> Gary has reported that when a dock is plugged into a system at the same >>>> time the autosuspend delay has tripped that the USB4 stack malfunctions. >>>> >>>> Messages show up like this: >>>> >>>> ``` >>>> thunderbolt 0000:e5:00.6: ring_interrupt_active: interrupt for TX ring 0 is already enabled >>>> ``` >>>> >>>> Furthermore the USB4 router is non-functional at this point. >>> >>> Once the USB4 domain starts the sleep transition, it cannot be >>> interrupted by anything so it always should go through full sleep >>> transition and only then back from sleep. >>> >>>> Those messages happen because the device is still in D3cold at the time >>>> that the PCI core handed control back to the USB4 connection manager >>>> (thunderbolt). >>> >>> This is weird. Yes we should be getting the wake from the hotplug but >>> that should happen only after the domain is fully in sleep (D3cold). The >>> BIOS ACPI code is supposed to deal with this. >> >> Is that from from experience or do you mean there is a spec behavior? >> >> IE I'm wondering if we have different "expectations" from different >> company's hardware designers. > > The spec and the CM guide "imply" this behaviour as far as I can tell, > so that the "sleep event" is done completely once started. I guess this > can be interpreted differently too because it is not explicitly said > there. > > Can you ask AMD HW folks if this is their interpretation too? Basically > when we get "Sleep Ready" bit set for all the routers in the domain and > turn off power (send PERST) there cannot be wake events until that is > fully completed. > > There is typically a timeout mechanism in the BIOS side (part of the > power off method) that waits for the PCIe links to enter L2 before it > triggers PERST. We have seen an issue on our side that if this L2 > transition is not completed in time a wake event triggered but that was > a BIOS issue. Sure thing. I'll discuss it with them and get back with the results. > >>>> The issue is that it takes time for a device to enter D3cold and do a >>>> conventional reset, and then more time for it to exit D3cold. >>>> >>>> This appears not to be a new problem; previously there were very similar >>>> reports from Ryzen XHCI controllers. Quirks were added for those. >>>> Furthermore; adding extra logging it's apparent that other PCI devices >>>> in the system can take more than 10ms to recover from D3cold as well. >>> >>> They can take anything up to 100ms after the link has trained. >> >> Right; so currently there is nothing checking they really made it back to D0 >> after calling pci_power_up(). I feel like we've "mostly" gotten lucky. > > We do have pci_bridge_wait_for_secondary_bus() there but I guess that's > not called for integrated PCIe endpoints such as xHCI and the USB4 Host > Interface. They too enter D3cold once power is turned off so I agree we > might have gotten lucky here with the D3hot 10ms delay. Yup; exactly. I think when Gary found this he was seeing other integrated devices not being ready in time too, but IIRC there wasn't functional impact from them like there is on XHCI and USB4.
On 6/19/2024 13:50, Mario Limonciello wrote: > On 6/19/2024 00:29, Mika Westerberg wrote: >> On Tue, Jun 18, 2024 at 11:56:50AM -0500, Mario Limonciello wrote: >>> On 6/18/2024 08:14, Mika Westerberg wrote: >>>> Hi Mario, >>>> >>>> On Thu, Jun 13, 2024 at 12:42:00AM -0500, Mario Limonciello wrote: >>>>> Gary has reported that when a dock is plugged into a system at the >>>>> same >>>>> time the autosuspend delay has tripped that the USB4 stack >>>>> malfunctions. >>>>> >>>>> Messages show up like this: >>>>> >>>>> ``` >>>>> thunderbolt 0000:e5:00.6: ring_interrupt_active: interrupt for TX >>>>> ring 0 is already enabled >>>>> ``` >>>>> >>>>> Furthermore the USB4 router is non-functional at this point. >>>> >>>> Once the USB4 domain starts the sleep transition, it cannot be >>>> interrupted by anything so it always should go through full sleep >>>> transition and only then back from sleep. >>>> >>>>> Those messages happen because the device is still in D3cold at the >>>>> time >>>>> that the PCI core handed control back to the USB4 connection manager >>>>> (thunderbolt). >>>> >>>> This is weird. Yes we should be getting the wake from the hotplug but >>>> that should happen only after the domain is fully in sleep (D3cold). >>>> The >>>> BIOS ACPI code is supposed to deal with this. >>> >>> Is that from from experience or do you mean there is a spec behavior? >>> >>> IE I'm wondering if we have different "expectations" from different >>> company's hardware designers. >> >> The spec and the CM guide "imply" this behaviour as far as I can tell, >> so that the "sleep event" is done completely once started. I guess this >> can be interpreted differently too because it is not explicitly said >> there. >> >> Can you ask AMD HW folks if this is their interpretation too? Basically >> when we get "Sleep Ready" bit set for all the routers in the domain and >> turn off power (send PERST) there cannot be wake events until that is >> fully completed. >> >> There is typically a timeout mechanism in the BIOS side (part of the >> power off method) that waits for the PCIe links to enter L2 before it >> triggers PERST. We have seen an issue on our side that if this L2 >> transition is not completed in time a wake event triggered but that was >> a BIOS issue. > > Sure thing. I'll discuss it with them and get back with the results. From the hardware team they describe this as an abnormal state that they don't expect. I don't believe there is anything in the BIOS to prevent it though. I could discuss options for this with the BIOS team in the future for the USB4 router ACPI device, but as this "seems" to be the same problem as XHCI controllers going back at least 5 generations with those quirks I put reverts in this series I think a general kernel solution to make "sure" that devices have transitioned is the better way to go. > >> >>>>> The issue is that it takes time for a device to enter D3cold and do a >>>>> conventional reset, and then more time for it to exit D3cold. >>>>> >>>>> This appears not to be a new problem; previously there were very >>>>> similar >>>>> reports from Ryzen XHCI controllers. Quirks were added for those. >>>>> Furthermore; adding extra logging it's apparent that other PCI devices >>>>> in the system can take more than 10ms to recover from D3cold as well. >>>> >>>> They can take anything up to 100ms after the link has trained. >>> >>> Right; so currently there is nothing checking they really made it >>> back to D0 >>> after calling pci_power_up(). I feel like we've "mostly" gotten lucky. >> >> We do have pci_bridge_wait_for_secondary_bus() there but I guess that's >> not called for integrated PCIe endpoints such as xHCI and the USB4 Host >> Interface. They too enter D3cold once power is turned off so I agree we >> might have gotten lucky here with the D3hot 10ms delay. > > Yup; exactly. I think when Gary found this he was seeing other > integrated devices not being ready in time too, but IIRC there wasn't > functional impact from them like there is on XHCI and USB4. > > Bjorn, Considering the above, can you please review this series?
On Tue, Jun 25, 2024 at 10:43:20AM -0500, Mario Limonciello wrote: > On 6/19/2024 13:50, Mario Limonciello wrote: > > On 6/19/2024 00:29, Mika Westerberg wrote: > > > On Tue, Jun 18, 2024 at 11:56:50AM -0500, Mario Limonciello wrote: > > > > On 6/18/2024 08:14, Mika Westerberg wrote: > > > > > Hi Mario, > > > > > > > > > > On Thu, Jun 13, 2024 at 12:42:00AM -0500, Mario Limonciello wrote: > > > > > > Gary has reported that when a dock is plugged into a > > > > > > system at the same > > > > > > time the autosuspend delay has tripped that the USB4 > > > > > > stack malfunctions. > > > > > > > > > > > > Messages show up like this: > > > > > > > > > > > > ``` > > > > > > thunderbolt 0000:e5:00.6: ring_interrupt_active: > > > > > > interrupt for TX ring 0 is already enabled > > > > > > ``` > > > > > > > > > > > > Furthermore the USB4 router is non-functional at this point. > > > > > > > > > > Once the USB4 domain starts the sleep transition, it cannot be > > > > > interrupted by anything so it always should go through full sleep > > > > > transition and only then back from sleep. > > > > > > > > > > > Those messages happen because the device is still in > > > > > > D3cold at the time > > > > > > that the PCI core handed control back to the USB4 connection manager > > > > > > (thunderbolt). > > > > > > > > > > This is weird. Yes we should be getting the wake from the hotplug but > > > > > that should happen only after the domain is fully in sleep > > > > > (D3cold). The > > > > > BIOS ACPI code is supposed to deal with this. > > > > > > > > Is that from from experience or do you mean there is a spec behavior? > > > > > > > > IE I'm wondering if we have different "expectations" from different > > > > company's hardware designers. > > > > > > The spec and the CM guide "imply" this behaviour as far as I can tell, > > > so that the "sleep event" is done completely once started. I guess this > > > can be interpreted differently too because it is not explicitly said > > > there. > > > > > > Can you ask AMD HW folks if this is their interpretation too? Basically > > > when we get "Sleep Ready" bit set for all the routers in the domain and > > > turn off power (send PERST) there cannot be wake events until that is > > > fully completed. > > > > > > There is typically a timeout mechanism in the BIOS side (part of the > > > power off method) that waits for the PCIe links to enter L2 before it > > > triggers PERST. We have seen an issue on our side that if this L2 > > > transition is not completed in time a wake event triggered but that was > > > a BIOS issue. > > > > Sure thing. I'll discuss it with them and get back with the results. > > From the hardware team they describe this as an abnormal state that they > don't expect. I don't believe there is anything in the BIOS to prevent it > though. Okay thanks for checking. > > I could discuss options for this with the BIOS team in the future for the > USB4 router ACPI device, but as this "seems" to be the same problem as XHCI > controllers going back at least 5 generations with those quirks I put > reverts in this series I think a general kernel solution to make "sure" that > devices have transitioned is the better way to go. Agreed.
On 6/13/2024 0:42, Mario Limonciello wrote: > Gary has reported that when a dock is plugged into a system at the same > time the autosuspend delay has tripped that the USB4 stack malfunctions. > > Messages show up like this: > > ``` > thunderbolt 0000:e5:00.6: ring_interrupt_active: interrupt for TX ring 0 is already enabled > ``` > > Furthermore the USB4 router is non-functional at this point. > > Those messages happen because the device is still in D3cold at the time > that the PCI core handed control back to the USB4 connection manager > (thunderbolt). > > The issue is that it takes time for a device to enter D3cold and do a > conventional reset, and then more time for it to exit D3cold. > > This appears not to be a new problem; previously there were very similar > reports from Ryzen XHCI controllers. Quirks were added for those. > Furthermore; adding extra logging it's apparent that other PCI devices > in the system can take more than 10ms to recover from D3cold as well. > > This series add a wait into pci_power_up() specifically for D3cold exit and > then drops the quirks that were previously used for the Ryzen XHCI controllers. > > Mario Limonciello (4): > PCI: Check PCI_PM_CTRL instead of PCI_COMMAND in pci_dev_wait() > PCI: Verify functions currently in D3cold have entered D0 > PCI: Allow Ryzen XHCI controllers into D3cold and drop delays > PCI: Drop Radeon quirk for Macbook Pro 8.2 > > drivers/pci/pci.c | 21 ++++++++++++++++----- > drivers/pci/quirks.c | 25 ------------------------- > drivers/usb/host/xhci-pci.c | 11 ----------- > 3 files changed, 16 insertions(+), 41 deletions(-) > Bjorn, Mathias, Any feedback for this series? I did check and it still applies to 6.10-rc7 as is, but if you want me to rebase on linux-pci happy to do so. At least Mika and I seem in agreement from other comments in the thread on this directionally. Thanks,