| Message ID | 20230424214859.3109-1-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2a821fc3136d5d99dcb9de152be8a052ca27d870 |
| Headers | show |
| Series | [v2] xhci-pci: Only run d3cold avoidance quirk for s2idle | expand |
On 25.4.2023 0.48, Mario Limonciello wrote: > Donghun reports that a notebook that has an AMD Ryzen 5700U but supports > S3 has problems with USB after resuming from suspend. The issue was > bisected down to commit d1658268e439 ("usb: pci-quirks: disable D3cold on > xhci suspend for s2idle on AMD Renoir"). > > As this issue only happens on S3, narrow the broken D3cold quirk to only > run in s2idle. > > Fixes: d1658268e439 ("usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir") > Reported-and-tested-by: Donghun Yoon <donghun.yoon@lge.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > v1->v2: > * Guard against CONFIG_SUSPEND not being set Thanks, Checkpatch complains that it wants a link to the issue after a "Reported-by:" tag. Is there any public report of this? Thanks Mathias
On 5/3/2023 8:14 AM, Mathias Nyman wrote: > On 25.4.2023 0.48, Mario Limonciello wrote: >> Donghun reports that a notebook that has an AMD Ryzen 5700U but supports >> S3 has problems with USB after resuming from suspend. The issue was >> bisected down to commit d1658268e439 ("usb: pci-quirks: disable >> D3cold on >> xhci suspend for s2idle on AMD Renoir"). >> >> As this issue only happens on S3, narrow the broken D3cold quirk to only >> run in s2idle. >> >> Fixes: d1658268e439 ("usb: pci-quirks: disable D3cold on xhci suspend >> for s2idle on AMD Renoir") >> Reported-and-tested-by: Donghun Yoon <donghun.yoon@lge.com> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> v1->v2: >> * Guard against CONFIG_SUSPEND not being set > > Thanks, > Checkpatch complains that it wants a link to the issue after a > "Reported-by:" tag. > > Is there any public report of this? > > Thanks > Mathias > > It was reported privately to me by Donghun, and they confirmed from private testing that this change helps the issue. I noticed that message from checkpatch too before I sent this up, but I figured it's better to give Donghun attribution rather than drop the tag.
On 3.5.2023 16.59, Limonciello, Mario wrote: > > On 5/3/2023 8:14 AM, Mathias Nyman wrote: >> On 25.4.2023 0.48, Mario Limonciello wrote: >>> Donghun reports that a notebook that has an AMD Ryzen 5700U but supports >>> S3 has problems with USB after resuming from suspend. The issue was >>> bisected down to commit d1658268e439 ("usb: pci-quirks: disable D3cold on >>> xhci suspend for s2idle on AMD Renoir"). >>> >>> As this issue only happens on S3, narrow the broken D3cold quirk to only >>> run in s2idle. >>> >>> Fixes: d1658268e439 ("usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir") >>> Reported-and-tested-by: Donghun Yoon <donghun.yoon@lge.com> >>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>> --- >>> v1->v2: >>> * Guard against CONFIG_SUSPEND not being set >> >> Thanks, >> Checkpatch complains that it wants a link to the issue after a "Reported-by:" tag. >> >> Is there any public report of this? >> >> Thanks >> Mathias >> >> > It was reported privately to me by Donghun, and they confirmed from private testing > that this change helps the issue. > > I noticed that message from checkpatch too before I sent this up, but I figured it's better > to give Donghun attribution rather than drop the tag. > Agree, thanks for the info Mathias
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 6db07ca419c3..1fb727f5c496 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -13,6 +13,7 @@ #include <linux/module.h> #include <linux/acpi.h> #include <linux/reset.h> +#include <linux/suspend.h> #include "xhci.h" #include "xhci-trace.h" @@ -194,7 +195,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->device == PCI_DEVICE_ID_AMD_RENOIR_XHCI) - xhci->quirks |= XHCI_BROKEN_D3COLD; + xhci->quirks |= XHCI_BROKEN_D3COLD_S2I; if (pdev->vendor == PCI_VENDOR_ID_INTEL) { xhci->quirks |= XHCI_LPM_SUPPORT; @@ -609,9 +610,16 @@ static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup) * Systems with the TI redriver that loses port status change events * need to have the registers polled during D3, so avoid D3cold. */ - if (xhci->quirks & (XHCI_COMP_MODE_QUIRK | XHCI_BROKEN_D3COLD)) + if (xhci->quirks & XHCI_COMP_MODE_QUIRK) pci_d3cold_disable(pdev); +#ifdef CONFIG_SUSPEND + /* d3cold is broken, but only when s2idle is used */ + if (pm_suspend_target_state == PM_SUSPEND_TO_IDLE && + xhci->quirks & (XHCI_BROKEN_D3COLD_S2I)) + pci_d3cold_disable(pdev); +#endif + if (xhci->quirks & XHCI_PME_STUCK_QUIRK) xhci_pme_quirk(hcd); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 786002bb35db..3818359603cc 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1901,7 +1901,7 @@ struct xhci_hcd { #define XHCI_DISABLE_SPARSE BIT_ULL(38) #define XHCI_SG_TRB_CACHE_SIZE_QUIRK BIT_ULL(39) #define XHCI_NO_SOFT_RETRY BIT_ULL(40) -#define XHCI_BROKEN_D3COLD BIT_ULL(41) +#define XHCI_BROKEN_D3COLD_S2I BIT_ULL(41) #define XHCI_EP_CTX_BROKEN_DCS BIT_ULL(42) #define XHCI_SUSPEND_RESUME_CLKS BIT_ULL(43) #define XHCI_RESET_TO_DEFAULT BIT_ULL(44)
Donghun reports that a notebook that has an AMD Ryzen 5700U but supports S3 has problems with USB after resuming from suspend. The issue was bisected down to commit d1658268e439 ("usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir"). As this issue only happens on S3, narrow the broken D3cold quirk to only run in s2idle. Fixes: d1658268e439 ("usb: pci-quirks: disable D3cold on xhci suspend for s2idle on AMD Renoir") Reported-and-tested-by: Donghun Yoon <donghun.yoon@lge.com> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- v1->v2: * Guard against CONFIG_SUSPEND not being set --- drivers/usb/host/xhci-pci.c | 12 ++++++++++-- drivers/usb/host/xhci.h | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-)