On Tuesday, June 15, 2021 5:00:58 PM CEST Mika Westerberg wrote: > Some PCIe devices only support PME (Power Management Event) from D3cold. > One example is ASMedia xHCI controller: > > 11:00.0 USB controller: ASMedia Technology Inc. ASM1042A USB 3.0 Host Controller (prog-if 30 [XHCI]) > ... > Capabilities: [78] Power Management version 3 > Flags: PMEClk- DSI- D1- D2- AuxCurrent=55mA PME(D0-,D1-,D2-,D3hot-,D3cold+) > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- > > With such devices, if it has wake enabled, the kernel selects lowest > possible power state to be D0 in pci_target_state(). This is problematic > because it prevents the root port it is connected to enter low power > state too which makes the system consume more energy than necessary. > > The problem in pci_target_state() is that it only accounts the "current" > device state, so when the bridge above it (a root port for instance) is > transitioned into D3hot the device transitions into D3cold. This is > because when the root port is first transitioned into D3hot then the > ACPI power resource is turned off which puts the PCIe link to L2/L3 (and > the root port and the device are in D3cold). If the root port is kept in > D3hot it still means that the device below it is still effectively in > D3cold as no configuration messages pass through. Furthermore the > implementation note of PCIe 5.0 sec 5.3.1.4 says that the device should > expect to be transitioned into D3cold soon after its link transitions > into L2/L3 Ready state. > > Taking the above into consideration, instead of forcing the device stay > in D0 we modify pci_target_state() to return D3hot in this special case > and make __pci_enable_wake() to enable PME too in this case. > > Reported-by: Utkarsh H Patel <utkarsh.h.patel@xxxxxxxxx> > Reported-by: Koba Ko <koba.ko@xxxxxxxxxxxxx> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> Any chance to resend with a CC to linux-pm? > --- > Hi all, > > This is third version of the patch. I changed this according to what Rafael > suggested, so that the pci_target_state() returns D3hot for these devices > and pci_enable_wake() then enables PME from D3cold. This solves the problem > in my test system. > > @Utkarsh, @Koba, I appreciate if you could try this one too. > > I also dropped the Tested-by tag from Koba Ko and Acked-by from Kai-Heng > Feng as this is not the same patch anymore. > > The previous version can be seen here: > > https://lore.kernel.org/linux-pci/20210531133435.53259-1-mika.westerberg@xxxxxxxxxxxxxxx/ > > drivers/pci/pci.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index b717680377a9..6605f85a1d63 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2485,7 +2485,13 @@ static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable > if (enable) { > int error; > > - if (pci_pme_capable(dev, state)) > + /* > + * Enable PME if device is capable from given state. > + * Special case is device that can only generate PME > + * from D3cold then we enable PME too. > + */ > + if (pci_pme_capable(dev, state) || > + (state == PCI_D3hot && pci_pme_capable(dev, PCI_D3cold))) > pci_pme_active(dev, true); > else > ret = 1; > @@ -2595,6 +2601,16 @@ static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup) > * PME#. > */ > if (dev->pme_support) { > + /* > + * Special case if device supports only PME from > + * D3cold but not from D3hot we still return > + * D3hot. > + */ > + if (target_state == PCI_D3hot && > + !(dev->pme_support & (1 << PCI_D3hot)) && > + (dev->pme_support & (1 << PCI_D3cold))) > + return target_state; > + > while (target_state > && !(dev->pme_support & (1 << target_state))) > target_state--; >
