| Message ID | 20221006211529.1858-2-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable runtime PM more broadly | expand |
On Thu, Oct 06, 2022 at 04:15:28PM -0500, Mario Limonciello wrote: > - if (pdev->vendor == PCI_VENDOR_ID_AMD && > - (pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_1 || > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_2 || > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_3 || > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_4 || > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_5 || > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6 || > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_7 || > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_8)) Can you add a comment here explaining why this is OK? I think it is easier that way to find out why this is here in the future instead of going through the git blame history. > + if (xhci->hci_version >= 0x102) > xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
[Public] > -----Original Message----- > From: Mika Westerberg <mika.westerberg@linux.intel.com> > Sent: Friday, October 7, 2022 04:55 > To: Limonciello, Mario <Mario.Limonciello@amd.com> > Cc: Mathias Nyman <mathias.nyman@intel.com>; Mehta, Sanju > <Sanju.Mehta@amd.com>; Mathias Nyman > <mathias.nyman@linux.intel.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v2 1/2] xhci-pci: Set runtime PM as default policy on all > xHC 1.2 or later devices > > On Thu, Oct 06, 2022 at 04:15:28PM -0500, Mario Limonciello wrote: > > - if (pdev->vendor == PCI_VENDOR_ID_AMD && > > - (pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_1 || > > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_2 || > > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_3 || > > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_4 || > > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_5 || > > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6 || > > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_7 || > > - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_8)) > > Can you add a comment here explaining why this is OK? I think it is > easier that way to find out why this is here in the future instead of > going through the git blame history. Sure, no problem. I'll hold off sending a v3 though until you and Mathias can double check everything in patch 2/2 is OK to take out and agree with that secondary logic change. > > > + if (xhci->hci_version >= 0x102) > > xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
On 7.10.2022 19.42, Limonciello, Mario wrote: > [Public] > > > >> -----Original Message----- >> From: Mika Westerberg <mika.westerberg@linux.intel.com> >> Sent: Friday, October 7, 2022 04:55 >> To: Limonciello, Mario <Mario.Limonciello@amd.com> >> Cc: Mathias Nyman <mathias.nyman@intel.com>; Mehta, Sanju >> <Sanju.Mehta@amd.com>; Mathias Nyman >> <mathias.nyman@linux.intel.com>; Greg Kroah-Hartman >> <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux- >> kernel@vger.kernel.org >> Subject: Re: [PATCH v2 1/2] xhci-pci: Set runtime PM as default policy on all >> xHC 1.2 or later devices >> >> On Thu, Oct 06, 2022 at 04:15:28PM -0500, Mario Limonciello wrote: >>> - if (pdev->vendor == PCI_VENDOR_ID_AMD && >>> - (pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_1 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_2 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_3 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_4 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_5 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_7 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_8)) >> >> Can you add a comment here explaining why this is OK? I think it is >> easier that way to find out why this is here in the future instead of >> going through the git blame history. > > Sure, no problem. > > I'll hold off sending a v3 though until you and Mathias can double check > everything in patch 2/2 is OK to take out and agree with that secondary logic > change. Lets only enable it for xHCI 1.2 and controllers for now. Avoiding unnecessary regressions. So on Intel side the ALDER_LAKE*, RAPTOR_LAKE and METEOR_LAKE can be removed. I'll double check other Intel hosts before submitting it forward Thanks -Mathias
On 7.10.2022 19.42, Limonciello, Mario wrote: > [Public] > > > >> -----Original Message----- >> From: Mika Westerberg <mika.westerberg@linux.intel.com> >> Sent: Friday, October 7, 2022 04:55 >> To: Limonciello, Mario <Mario.Limonciello@amd.com> >> Cc: Mathias Nyman <mathias.nyman@intel.com>; Mehta, Sanju >> <Sanju.Mehta@amd.com>; Mathias Nyman >> <mathias.nyman@linux.intel.com>; Greg Kroah-Hartman >> <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux- >> kernel@vger.kernel.org >> Subject: Re: [PATCH v2 1/2] xhci-pci: Set runtime PM as default policy on all >> xHC 1.2 or later devices >> >> On Thu, Oct 06, 2022 at 04:15:28PM -0500, Mario Limonciello wrote: >>> - if (pdev->vendor == PCI_VENDOR_ID_AMD && >>> - (pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_1 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_2 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_3 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_4 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_5 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_7 || >>> - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_8)) >> >> Can you add a comment here explaining why this is OK? I think it is >> easier that way to find out why this is here in the future instead of >> going through the git blame history. > > Sure, no problem. > > I'll hold off sending a v3 though until you and Mathias can double check > everything in patch 2/2 is OK to take out and agree with that secondary logic > change. > >> >>> + if (xhci->hci_version >= 0x102) Shouldn't this be ">= 0x120" Thanks Mathias
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index dce6c0ec8d340..0d2d1cea94a4f 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -69,14 +69,6 @@ #define PCI_DEVICE_ID_AMD_PROMONTORYA_3 0x43ba #define PCI_DEVICE_ID_AMD_PROMONTORYA_2 0x43bb #define PCI_DEVICE_ID_AMD_PROMONTORYA_1 0x43bc -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_1 0x161a -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_2 0x161b -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_3 0x161d -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_4 0x161e -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_5 0x15d6 -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6 0x15d7 -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_7 0x161c -#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_8 0x161f #define PCI_DEVICE_ID_ASMEDIA_1042_XHCI 0x1042 #define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI 0x1142 @@ -336,15 +328,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4)) xhci->quirks |= XHCI_NO_SOFT_RETRY; - if (pdev->vendor == PCI_VENDOR_ID_AMD && - (pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_1 || - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_2 || - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_3 || - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_4 || - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_5 || - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6 || - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_7 || - pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_8)) + if (xhci->hci_version >= 0x102) xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW; if (xhci->quirks & XHCI_RESET_ON_RESUME)
For optimal power consumption of USB4 routers the XHCI PCIe endpoint used for tunneling must be in D3. Historically this is accomplished by a long list of PCIe IDs that correspond to these endpoints because the xhci_hcd driver will not default to allowing runtime PM for all devices. As both AMD and Intel have released new products with new XHCI controllers this list continues to grow. In reviewing the XHCI specification v1.2 on page 607 there is already a requirement that the PCI power management states D3hot and D3cold must be supported. In the quirk list, use this to indicate that runtime PM should be allowed on XHCI controllers. Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com> Link: https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/extensible-host-controler-interface-usb-xhci.pdf Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/usb/host/xhci-pci.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)