mbox series

[RFC,0/3] Enable runtime PM more broadly

Message ID 20221004041225.1462336-1-mario.limonciello@amd.com
Headers show
Series Enable runtime PM more broadly | expand

Message

Mario Limonciello Oct. 4, 2022, 4:12 a.m. UTC
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(-)

Comments

Mario Limonciello Oct. 4, 2022, 11:38 a.m. UTC | #1
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.
Mika Westerberg Oct. 4, 2022, 12:46 p.m. UTC | #2
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.