Hi Rafael, On Mon, Jul 12, 2021 at 07:06:47PM +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > Some PCIe devices only support PME (Power Management Event) from > D3cold. One example is the 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- > > In those cases, if the device is expected to generate wakeup events > from its final power state, pci_target_state() returns D0, which > prevents the PCIe port the device is connected to from entering any > low-power states too. However, if the device were allowed to go into > D3hot, its parent PCIe port would also be able to go into D3 and if > it goes into D3cold, it would cause the endpoint device to end up in > D3cold too (as per the PCI PM spec v1.2, Table 6-1), in which case > the endpoint would be able to signal PME. This means that the system > could be put into a lower-power configuration without sacrificing the > the given device's ability to generate PME. > > In order to avoid missing that opportunity, extend pci_pme_capable() > to check the device's parent in the special case when the target > state is D3hot and the device can only signal PME from D3cold and > update pci_target_state() to return the current target state if > pci_pme_capable() returns 'true' for it. Thanks a lot for this! I tried the patch and unfortunately it does not solve the issue but I think I know what the problem is, see below. > Link: https://lore.kernel.org/linux-pm/20210617123653.58640-1-mika.westerberg@xxxxxxxxxxxxxxx > Reported-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > Reported-by: Utkarsh H Patel <utkarsh.h.patel@xxxxxxxxx> > Reported-by: Koba Ko <koba.ko@xxxxxxxxxxxxx> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > --- > > Hi, > > Anyone who can reproduce the problem described in the changelog, > please test the patch and let me know the result. > > Thanks! > > --- > drivers/pci/pci.c | 38 ++++++++++++++++++++++++++------------ > 1 file changed, 26 insertions(+), 12 deletions(-) > > Index: linux-pm/drivers/pci/pci.c > =================================================================== > --- linux-pm.orig/drivers/pci/pci.c > +++ linux-pm/drivers/pci/pci.c > @@ -2298,10 +2298,29 @@ void pci_pme_wakeup_bus(struct pci_bus * > */ > bool pci_pme_capable(struct pci_dev *dev, pci_power_t state) > { > + struct pci_dev *parent; > + > if (!dev->pm_cap) > return false; > > - return !!(dev->pme_support & (1 << state)); > + if (dev->pme_support & (1 << state)) > + return true; > + > + /* > + * Special case: The target state is D3hot and the device only supports > + * signaling PME from D3cold, but it is a PCIe device whose parent port > + * can go into D3cold. In that case, if the device is allowed to go > + * into D3hot, the parent port can go into D3cold which will cause the > + * device to end up in D3cold, so it will be able to signal PME from the > + * final state. > + */ > + if (state != PCI_D3hot || !(dev->pme_support & (1 << PCI_D3cold))) > + return false; > + > + parent = dev->bus->self; > + return pci_bridge_d3_possible(parent) && > + platform_pci_power_manageable(parent) && Here. We cannot assume that the parent has ACPI node either. For instance when PCIe is tunneled over TBT/USB4 there can be several PCIe switches between the problem endpoint and the root port (which has the ACPI node of course). I think the safe assumption is to check for the root port and whether platform_pci_power_manageable() returns true for it but I may be missing something. > + platform_pci_choose_state(parent) == PCI_D3cold; > } > EXPORT_SYMBOL(pci_pme_capable); > > @@ -2595,17 +2614,12 @@ static pci_power_t pci_target_state(stru > if (dev->current_state == PCI_D3cold) > target_state = PCI_D3cold; > > - if (wakeup) { > - /* > - * Find the deepest state from which the device can generate > - * PME#. > - */ > - if (dev->pme_support) { > - while (target_state > - && !(dev->pme_support & (1 << target_state))) > - target_state--; > - } > - } > + if (!wakeup || !dev->pme_support || pci_pme_capable(dev, target_state)) > + return target_state; > + > + /* Find the deepest state from which the device can generate PME#. */ > + while (target_state && !(dev->pme_support & (1 << target_state))) > + target_state--; > > return target_state; > } > >
