Currently every time a vendor introduces a new USB4 controller changes
need to be made in xhci-pci to add the PCI IDs representing the XHCI
controller used for tunneling.
Due to low power management needs every single integrated Intel and
AMD controller have needed to be added. Recent discussions have come
at using the xHC interface version to decide that runtime PM should
be enabled.
v1->v2:
* Drop existing patches
* New patches to use xHC interface version instead
Mario Limonciello (2):
xhci-pci: Set runtime PM as default policy on all xHC 1.2 or later
devices
xhci-pci: Lower the requirement for runtime PM version to 1.0
drivers/usb/host/xhci-pci.c | 53 +------------------------------------
1 file changed, 1 insertion(+), 52 deletions(-)
--
2.34.1
The XHCI specification has a changelog of new mandatory requirements
in the appendix. Between versions 1.0 and 1.2 the D3 support was
not made a new mandatory requirement. As such, all 1.0 controllers
should be safe to allow runtime PM.
This should allow dropping the entire list of controllers from the
driver.
Link: https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/extensible-host-controler-interface-usb-xhci.pdf p639+
Signed-off-by: Mario Limonciello <[email protected]>
---
This patch is intentionally split from the first, as I would like Intel
to confirm that all these devices really meet 1.0 or newer xHC version
to avoid causing a potential regression if one was pre-1.0.
drivers/usb/host/xhci-pci.c | 37 +------------------------------------
1 file changed, 1 insertion(+), 36 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 0d2d1cea94a4f..ceb14b3648ed7 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -46,23 +46,7 @@
#define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI 0x1aa8
#define PCI_DEVICE_ID_INTEL_APL_XHCI 0x5aa8
#define PCI_DEVICE_ID_INTEL_DNV_XHCI 0x19d0
-#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_XHCI 0x15b5
-#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_XHCI 0x15b6
-#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_XHCI 0x15c1
-#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_XHCI 0x15db
-#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_XHCI 0x15d4
-#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_XHCI 0x15e9
-#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_XHCI 0x15ec
-#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI 0x15f0
-#define PCI_DEVICE_ID_INTEL_ICE_LAKE_XHCI 0x8a13
#define PCI_DEVICE_ID_INTEL_CML_XHCI 0xa3af
-#define PCI_DEVICE_ID_INTEL_TIGER_LAKE_XHCI 0x9a13
-#define PCI_DEVICE_ID_INTEL_MAPLE_RIDGE_XHCI 0x1138
-#define PCI_DEVICE_ID_INTEL_ALDER_LAKE_XHCI 0x461e
-#define PCI_DEVICE_ID_INTEL_ALDER_LAKE_N_XHCI 0x464e
-#define PCI_DEVICE_ID_INTEL_ALDER_LAKE_PCH_XHCI 0x51ed
-#define PCI_DEVICE_ID_INTEL_RAPTOR_LAKE_XHCI 0xa71e
-#define PCI_DEVICE_ID_INTEL_METEOR_LAKE_XHCI 0x7ec0
#define PCI_DEVICE_ID_AMD_RENOIR_XHCI 0x1639
#define PCI_DEVICE_ID_AMD_PROMONTORYA_4 0x43b9
@@ -249,25 +233,6 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
pdev->device == PCI_DEVICE_ID_INTEL_DNV_XHCI))
xhci->quirks |= XHCI_MISSING_CAS;
- if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
- (pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ICE_LAKE_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_TIGER_LAKE_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_MAPLE_RIDGE_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ALDER_LAKE_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ALDER_LAKE_N_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_ALDER_LAKE_PCH_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_RAPTOR_LAKE_XHCI ||
- pdev->device == PCI_DEVICE_ID_INTEL_METEOR_LAKE_XHCI))
- xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
-
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
pdev->device == PCI_DEVICE_ID_EJ168) {
xhci->quirks |= XHCI_RESET_ON_RESUME;
@@ -328,7 +293,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 (xhci->hci_version >= 0x102)
+ if (xhci->hci_version >= 0x100)
xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
if (xhci->quirks & XHCI_RESET_ON_RESUME)
--
2.34.1
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 <[email protected]>
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 <[email protected]>
---
drivers/usb/host/xhci-pci.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
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)
--
2.34.1
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 <[email protected]>
> Sent: Friday, October 7, 2022 04:55
> To: Limonciello, Mario <[email protected]>
> Cc: Mathias Nyman <[email protected]>; Mehta, Sanju
> <[email protected]>; Mathias Nyman
> <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; [email protected]; linux-
> [email protected]
> 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 <[email protected]>
>> Sent: Friday, October 7, 2022 04:55
>> To: Limonciello, Mario <[email protected]>
>> Cc: Mathias Nyman <[email protected]>; Mehta, Sanju
>> <[email protected]>; Mathias Nyman
>> <[email protected]>; Greg Kroah-Hartman
>> <[email protected]>; [email protected]; linux-
>> [email protected]
>> 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 <[email protected]>
>> Sent: Friday, October 7, 2022 04:55
>> To: Limonciello, Mario <[email protected]>
>> Cc: Mathias Nyman <[email protected]>; Mehta, Sanju
>> <[email protected]>; Mathias Nyman
>> <[email protected]>; Greg Kroah-Hartman
>> <[email protected]>; [email protected]; linux-
>> [email protected]
>> 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