diff mbox series

[v3] xhci-pci: Set runtime PM as default policy on all xHC 1.2 or later devices

Message ID 20221010160022.647-1-mario.limonciello@amd.com (mailing list archive)
State New, archived
Headers show
Series [v3] xhci-pci: Set runtime PM as default policy on all xHC 1.2 or later devices | expand

Commit Message

Mario Limonciello Oct. 10, 2022, 4 p.m. UTC
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. The following controllers are known to be xHC 1.2 and
dropped explicitly:
* AMD Yellow Carp
* Intel Alder Lake
* Intel Meteor Lake
* Intel Raptor Lake

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>
---
v2->v3:
 * Add a comment for future discovery (Mika)
 * Correct version from 0x102 to 0x120 (Mathias)
 * Drop some Intel controllers too (Mathias)
 * Drop second patch (Mathias)
---
 drivers/usb/host/xhci-pci.c | 31 +++----------------------------
 1 file changed, 3 insertions(+), 28 deletions(-)

Comments

Mika Westerberg Oct. 11, 2022, 5:11 a.m. UTC | #1
On Mon, Oct 10, 2022 at 11:00:21AM -0500, Mario Limonciello wrote:
> 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. The following controllers are known to be xHC 1.2 and
> dropped explicitly:
> * AMD Yellow Carp
> * Intel Alder Lake
> * Intel Meteor Lake
> * Intel Raptor Lake
> 
> 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>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Mathias Nyman Oct. 11, 2022, 1:16 p.m. UTC | #2
On 11.10.2022 8.11, Mika Westerberg wrote:
> On Mon, Oct 10, 2022 at 11:00:21AM -0500, Mario Limonciello wrote:
>> 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. The following controllers are known to be xHC 1.2 and
>> dropped explicitly:
>> * AMD Yellow Carp
>> * Intel Alder Lake
>> * Intel Meteor Lake
>> * Intel Raptor Lake
>>
>> 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>
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks, added to queue

-Mathias
Mario Limonciello Oct. 11, 2022, 4:46 p.m. UTC | #3
[Public]

> -----Original Message-----
> From: Mathias Nyman <mathias.nyman@linux.intel.com>
> Sent: Tuesday, October 11, 2022 08:16
> To: Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello,
> Mario <Mario.Limonciello@amd.com>
> Cc: Mathias Nyman <mathias.nyman@intel.com>; Mehta, Sanju
> <Sanju.Mehta@amd.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v3] xhci-pci: Set runtime PM as default policy on all xHC
> 1.2 or later devices
> 
> On 11.10.2022 8.11, Mika Westerberg wrote:
> > On Mon, Oct 10, 2022 at 11:00:21AM -0500, Mario Limonciello wrote:
> >> 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. The following controllers are known to be xHC 1.2 and
> >> dropped explicitly:
> >> * AMD Yellow Carp
> >> * Intel Alder Lake
> >> * Intel Meteor Lake
> >> * Intel Raptor Lake
> >>
> >> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> >> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> w.intel.com%2Fcontent%2Fdam%2Fwww%2Fpublic%2Fus%2Fen%2Fdocum
> ents%2Ftechnical-specifications%2Fextensible-host-controler-interface-usb-
> xhci.pdf&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7Cb286e9c
> 63e9e4c3a1a3708daab8a9b23%7C3dd8961fe4884e608e11a82d994e183d%7C0
> %7C0%7C638010909687542135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
> 7C%7C%7C&amp;sdata=CetIs1VuAqj8nNBoLnXaGncw6Sl4JcImYY67JpVxyjg%
> 3D&amp;reserved=0
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >
> > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Thanks, added to queue

Thanks!
For my own clarity - is this something you'll still take to 6.1, or are you meaning
6.2 queue at this point?

This thread originated from enabling Pink Sardine.  Wherever it lands after it's
baked for $long_enough I would like to bring it back to stable eventually too.
If you think it's too risky for stable later on, can we do separate set of commits
that adds and then removes the IDs for pink sardine that can go to stable?  This
would at least mirror more closely what has been done historically for the other
USB4 products.
Mathias Nyman Oct. 12, 2022, 8:34 a.m. UTC | #4
On 11.10.2022 19.46, Limonciello, Mario wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Sent: Tuesday, October 11, 2022 08:16
>> To: Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello,
>> Mario <Mario.Limonciello@amd.com>
>> Cc: Mathias Nyman <mathias.nyman@intel.com>; Mehta, Sanju
>> <Sanju.Mehta@amd.com>; Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH v3] xhci-pci: Set runtime PM as default policy on all xHC
>> 1.2 or later devices
>>
>> On 11.10.2022 8.11, Mika Westerberg wrote:
>>> On Mon, Oct 10, 2022 at 11:00:21AM -0500, Mario Limonciello wrote:
>>>> 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. The following controllers are known to be xHC 1.2 and
>>>> dropped explicitly:
>>>> * AMD Yellow Carp
>>>> * Intel Alder Lake
>>>> * Intel Meteor Lake
>>>> * Intel Raptor Lake
>>>>
>>>> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>> Link:
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
>> w.intel.com%2Fcontent%2Fdam%2Fwww%2Fpublic%2Fus%2Fen%2Fdocum
>> ents%2Ftechnical-specifications%2Fextensible-host-controler-interface-usb-
>> xhci.pdf&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7Cb286e9c
>> 63e9e4c3a1a3708daab8a9b23%7C3dd8961fe4884e608e11a82d994e183d%7C0
>> %7C0%7C638010909687542135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
>> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>> 7C%7C%7C&amp;sdata=CetIs1VuAqj8nNBoLnXaGncw6Sl4JcImYY67JpVxyjg%
>> 3D&amp;reserved=0
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>
>> Thanks, added to queue
> 
> Thanks!
> For my own clarity - is this something you'll still take to 6.1, or are you meaning
> 6.2 queue at this point?

Was planning on sending it to usb-next after 6.1-rc1 is out, so ending up in 6.2
But if there's some benefit in having this already in 6.1 then I don't object that either.

> 
> This thread originated from enabling Pink Sardine.  Wherever it lands after it's
> baked for $long_enough I would like to bring it back to stable eventually too.
> If you think it's too risky for stable later on, can we do separate set of commits
> that adds and then removes the IDs for pink sardine that can go to stable?  This
> would at least mirror more closely what has been done historically for the other
> USB4 products.

I think we can add this to stable. It's fine for Intel xHCI 1.2 hosts.

Thanks
-Mathias
Mario Limonciello Oct. 12, 2022, 12:59 p.m. UTC | #5
On 10/12/22 03:34, Mathias Nyman wrote:
> On 11.10.2022 19.46, Limonciello, Mario wrote:
>> [Public]
>>
>>> -----Original Message-----
>>> From: Mathias Nyman <mathias.nyman@linux.intel.com>
>>> Sent: Tuesday, October 11, 2022 08:16
>>> To: Mika Westerberg <mika.westerberg@linux.intel.com>; Limonciello,
>>> Mario <Mario.Limonciello@amd.com>
>>> Cc: Mathias Nyman <mathias.nyman@intel.com>; Mehta, Sanju
>>> <Sanju.Mehta@amd.com>; Greg Kroah-Hartman
>>> <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; linux-
>>> kernel@vger.kernel.org
>>> Subject: Re: [PATCH v3] xhci-pci: Set runtime PM as default policy on 
>>> all xHC
>>> 1.2 or later devices
>>>
>>> On 11.10.2022 8.11, Mika Westerberg wrote:
>>>> On Mon, Oct 10, 2022 at 11:00:21AM -0500, Mario Limonciello wrote:
>>>>> 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. The following controllers are known to be xHC 
>>>>> 1.2 and
>>>>> dropped explicitly:
>>>>> * AMD Yellow Carp
>>>>> * Intel Alder Lake
>>>>> * Intel Meteor Lake
>>>>> * Intel Raptor Lake
>>>>>
>>>>> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>>> Link:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
>>> w.intel.com%2Fcontent%2Fdam%2Fwww%2Fpublic%2Fus%2Fen%2Fdocum
>>> ents%2Ftechnical-specifications%2Fextensible-host-controler-interface-usb-
>>> xhci.pdf&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7Cb286e9c
>>> 63e9e4c3a1a3708daab8a9b23%7C3dd8961fe4884e608e11a82d994e183d%7C0
>>> %7C0%7C638010909687542135%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC
>>> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>>> 7C%7C%7C&amp;sdata=CetIs1VuAqj8nNBoLnXaGncw6Sl4JcImYY67JpVxyjg%
>>> 3D&amp;reserved=0
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>
>>>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>>
>>> Thanks, added to queue
>>
>> Thanks!
>> For my own clarity - is this something you'll still take to 6.1, or 
>> are you meaning
>> 6.2 queue at this point?
> 
> Was planning on sending it to usb-next after 6.1-rc1 is out, so ending 
> up in 6.2
> But if there's some benefit in having this already in 6.1 then I don't 
> object that either.
> 

It's a blocker causing higher power consumption on some of the affected 
platforms.  So if you're amenable, I would like to see it for 6.1 so 
these things have a better chance at passing energy certifications.

>>
>> This thread originated from enabling Pink Sardine.  Wherever it lands 
>> after it's
>> baked for $long_enough I would like to bring it back to stable 
>> eventually too.
>> If you think it's too risky for stable later on, can we do separate 
>> set of commits
>> that adds and then removes the IDs for pink sardine that can go to 
>> stable?  This
>> would at least mirror more closely what has been done historically for 
>> the other
>> USB4 products.
> 
> I think we can add this to stable. It's fine for Intel xHCI 1.2 hosts.
> 

OK thanks.
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index dce6c0ec8d340..fd6f2698b3ea4 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -58,25 +58,12 @@ 
 #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
 #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
@@ -268,12 +255,7 @@  static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *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))
+	     pdev->device == PCI_DEVICE_ID_INTEL_MAPLE_RIDGE_XHCI))
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
@@ -336,15 +318,8 @@  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))
+	/* xHC spec requires PCI devices to support D3hot and D3cold */
+	if (xhci->hci_version >= 0x120)
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (xhci->quirks & XHCI_RESET_ON_RESUME)