[RFC 0/3] Enable runtime PM more broadly

[RFC 0/3] Enable runtime PM more broadly

[Mario Limonciello]

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.  As we already know which
controller is used for tunneling by the device links specified in
ACPI tables, this is a very good heuristic.

This series uses that as a heuristic, pulls out all the IDs added to
xhci-pci and then adds the new IDs for those *not* used for tunneling
on AMD's Pink Sardine (those are covered by patch 1/3 in this RFC).

If 1/3 and 2/3 are not agreeable, then instead patch 3/3 can be re-spun
to explicitly add the PCI IDs used for the XHCI controller used for
tunneling on AMD's Pink Sardine.

Mario Limonciello (3):
  thunderbolt: Allow XHCI device links to enter runtime pm
  xhci-pci: Remove a number of controllers from the runtime PM allowlist
  xhci-pci: Allow host runtime PM as default for AMD Pink Sardine

 drivers/thunderbolt/acpi.c  |  3 +++
 drivers/usb/host/xhci-pci.c | 26 +++++++-------------------
 2 files changed, 10 insertions(+), 19 deletions(-)

-- 
2.34.1

[RFC 1/3] thunderbolt: Allow XHCI device links to enter runtime pm

[Mario Limonciello]

Both on Intel's and AMD's USB4 designs it's important that the device
link to the XHCI controller used for tunneling is able to go into D3
for appropriate low power consumption features as well as for system
suspend states such as s0i3.

Historically this is accomplished by adding to a hardcoded list in the
XHCI driver, but this requires a change for every single platform.

We have a very good proxy that it's safe to do this since the firmware
has indicated the device link needs to be made.  So opt all XHCI
controllers with these device links into runtime PM.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/thunderbolt/acpi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/thunderbolt/acpi.c b/drivers/thunderbolt/acpi.c
index b1f0dc8df47cd..5ffca085a47e3 100644
--- a/drivers/thunderbolt/acpi.c
+++ b/drivers/thunderbolt/acpi.c
@@ -75,6 +75,9 @@ static acpi_status tb_acpi_add_link(acpi_handle handle, u32 level, void *data,
 		 pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM))) {
 		const struct device_link *link;
 
+		if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
+			pm_runtime_allow(&pdev->dev);
+
 		/*
 		 * Make them both active first to make sure the NHI does
 		 * not runtime suspend before the consumer. The
-- 
2.34.1

Re: [RFC 1/3] thunderbolt: Allow XHCI device links to enter runtime pm

[Mika Westerberg]

Hi Mario,

On Mon, Oct 03, 2022 at 11:12:23PM -0500, Mario Limonciello wrote:
> Both on Intel's and AMD's USB4 designs it's important that the device
> link to the XHCI controller used for tunneling is able to go into D3
> for appropriate low power consumption features as well as for system
> suspend states such as s0i3.
> 
> Historically this is accomplished by adding to a hardcoded list in the
> XHCI driver, but this requires a change for every single platform.
> 
> We have a very good proxy that it's safe to do this since the firmware
> has indicated the device link needs to be made.  So opt all XHCI
> controllers with these device links into runtime PM.

This is good idea.

However, it misses the fact that we have FW CM as well in Intel
integrated TBT platforms (ICL, TGL and ADL) and with those you don't
have the device link (I think ADL has it for both, though) so we would
still need to keep the list in xHCI.

Re: [RFC 1/3] thunderbolt: Allow XHCI device links to enter runtime pm

[Limonciello, Mario]

On 10/4/2022 00:04, Mika Westerberg wrote:
> Hi Mario,
> 
> On Mon, Oct 03, 2022 at 11:12:23PM -0500, Mario Limonciello wrote:
>> Both on Intel's and AMD's USB4 designs it's important that the device
>> link to the XHCI controller used for tunneling is able to go into D3
>> for appropriate low power consumption features as well as for system
>> suspend states such as s0i3.
>>
>> Historically this is accomplished by adding to a hardcoded list in the
>> XHCI driver, but this requires a change for every single platform.
>>
>> We have a very good proxy that it's safe to do this since the firmware
>> has indicated the device link needs to be made.  So opt all XHCI
>> controllers with these device links into runtime PM.
> 
> This is good idea.
> 
> However, it misses the fact that we have FW CM as well in Intel
> integrated TBT platforms (ICL, TGL and ADL) and with those you don't
> have the device link (I think ADL has it for both, though) so we would
> still need to keep the list in xHCI.

Can you double check the firmware for ADL for me whether it has it for 
both?  I'll respin the series and drop at least the ICL and TGL reverts 
from patch 2.

Re: [RFC 1/3] thunderbolt: Allow XHCI device links to enter runtime pm

[Mika Westerberg]

On Tue, Oct 04, 2022 at 06:38:47AM -0500, Limonciello, Mario wrote:
> On 10/4/2022 00:04, Mika Westerberg wrote:
> > Hi Mario,
> > 
> > On Mon, Oct 03, 2022 at 11:12:23PM -0500, Mario Limonciello wrote:
> > > Both on Intel's and AMD's USB4 designs it's important that the device
> > > link to the XHCI controller used for tunneling is able to go into D3
> > > for appropriate low power consumption features as well as for system
> > > suspend states such as s0i3.
> > > 
> > > Historically this is accomplished by adding to a hardcoded list in the
> > > XHCI driver, but this requires a change for every single platform.
> > > 
> > > We have a very good proxy that it's safe to do this since the firmware
> > > has indicated the device link needs to be made.  So opt all XHCI
> > > controllers with these device links into runtime PM.
> > 
> > This is good idea.
> > 
> > However, it misses the fact that we have FW CM as well in Intel
> > integrated TBT platforms (ICL, TGL and ADL) and with those you don't
> > have the device link (I think ADL has it for both, though) so we would
> > still need to keep the list in xHCI.
> 
> Can you double check the firmware for ADL for me whether it has it for both?
> I'll respin the series and drop at least the ICL and TGL reverts from patch

Yes, ADL has it for both.

While doing that, I wonder if it would be easier to understand (and
follow) if all this is done in the xHCI side? It can also look for the
property and unblock runtime PM based on that.

[RFC 2/3] xhci-pci: Remove a number of controllers from the runtime PM allowlist

[Mario Limonciello]

When creating a device link from the XHCI PCI device to the USB4 router
the thunderbolt driver will opt the XHCI PCI device into runtime PM.

As such it's not necessary to include a hardcoded list of these XHCI
controllers.  This is effectively a full or partial revert of the following
commits:

* commit 8ffdc53a60049 ("xhci-pci: Allow host runtime PM as default for
Intel Meteor Lake xHCI")
* commit 7516da47a349e ("xhci-pci: Allow host runtime PM as default for
Intel Raptor Lake xHCI")
* commit 74f55a62c4c35 ("xhci: Allow host runtime PM as default for
Intel Alder Lake N xHCI")
* commit b813511135e8b ("xhci-pci: Allow host runtime PM as default for
Intel Alder Lake xHCI")
* commit 6a7c533d4a185 ("xhci-pci: Allow host runtime PM as default for
Intel Tiger Lake xHCI")
* commit 07a594f353655 ("xhci-pci: Allow host runtime PM as default also
for Intel Ice Lake xHCI")
* commit f886d4fbb7c97 ("usb: xhci: Extend support for runtime power
management for AMD's Yellow carp.")

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/usb/host/xhci-pci.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index dce6c0ec8d340..44defa8b796f7 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -54,15 +54,9 @@
 #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
@@ -73,10 +67,8 @@
 #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_AMD_YELLOW_CARP_XHCI_5		0x161c
+#define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6		0x161f
 
 #define PCI_DEVICE_ID_ASMEDIA_1042_XHCI			0x1042
 #define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI		0x1142
@@ -266,14 +258,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *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))
+	     pdev->device == PCI_DEVICE_ID_INTEL_ALDER_LAKE_PCH_XHCI))
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
@@ -342,9 +328,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	    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))
+	    pdev->device == PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6))
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (xhci->quirks & XHCI_RESET_ON_RESUME)
-- 
2.34.1

[RFC 3/3] xhci-pci: Allow host runtime PM as default for AMD Pink Sardine

[Mario Limonciello]

The XHCI controllers not connected to the USB4 controller via a device
link can support D3. For optimal runtime power consumption on AMD Pink
Sardine, all XHCI controllers must support runtime suspend.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/usb/host/xhci-pci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 44defa8b796f7..a569b29a46a99 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -69,6 +69,8 @@
 #define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_4		0x161e
 #define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_5		0x161c
 #define PCI_DEVICE_ID_AMD_YELLOW_CARP_XHCI_6		0x161f
+#define PCI_DEVICE_ID_AMD_PINK_SARDINE_XHCI_1		0x15b9
+#define PCI_DEVICE_ID_AMD_PINK_SARDINE_XHCI_2		0x15ba
 
 #define PCI_DEVICE_ID_ASMEDIA_1042_XHCI			0x1042
 #define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI		0x1142
@@ -328,7 +330,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	    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_6 ||
+	    pdev->device == PCI_DEVICE_ID_AMD_PINK_SARDINE_XHCI_1 ||
+	    pdev->device == PCI_DEVICE_ID_AMD_PINK_SARDINE_XHCI_2))
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (xhci->quirks & XHCI_RESET_ON_RESUME)
-- 
2.34.1