diff mbox series

[v1,2/2] PCI: PM: Enable PME if it can be signaled from D3cold

Message ID 1791325.tdWV9SEqCh@kreacher (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series PCI: PM: Fix handling of device that can only signal PME from D3cold | expand

Commit Message

Rafael J. Wysocki July 29, 2021, 2:49 p.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

PME signaling is only enabled by __pci_enable_wake() if the target
device can signal PME from the given target power state (to avoid
pointless reconfiguration of the device), but if the hierarchy above
the device goes into D3cold, the device itself will end up in D3cold
too, so if it can signal PME from D3cold, it should be enabled to
do so in __pci_enable_wake().

[Note that if the device does not end up in D3cold and it cannot
 signal PME from the original target power state, it will not signal
 PME, so in that case the behavior does not change.]

Link: https://lore.kernel.org/linux-pm/3149540.aeNJFYEL58@kreacher/
Fixes: 5bcc2fb4e815 ("PCI PM: Simplify PCI wake-up code")
Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reported-by: Utkarsh H Patel <utkarsh.h.patel@intel.com>
Reported-by: Koba Ko <koba.ko@canonical.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Mika Westerberg July 30, 2021, 10:28 a.m. UTC | #1 
On Thu, Jul 29, 2021 at 04:49:10PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> PME signaling is only enabled by __pci_enable_wake() if the target
> device can signal PME from the given target power state (to avoid
> pointless reconfiguration of the device), but if the hierarchy above
> the device goes into D3cold, the device itself will end up in D3cold
> too, so if it can signal PME from D3cold, it should be enabled to
> do so in __pci_enable_wake().
> 
> [Note that if the device does not end up in D3cold and it cannot
>  signal PME from the original target power state, it will not signal
>  PME, so in that case the behavior does not change.]
> 
> Link: https://lore.kernel.org/linux-pm/3149540.aeNJFYEL58@kreacher/
> Fixes: 5bcc2fb4e815 ("PCI PM: Simplify PCI wake-up code")
> Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Also this solves the reported issue so,

Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks!
Rafael J. Wysocki July 30, 2021, 12:13 p.m. UTC | #2 
On Fri, Jul 30, 2021 at 12:28 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Thu, Jul 29, 2021 at 04:49:10PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > PME signaling is only enabled by __pci_enable_wake() if the target
> > device can signal PME from the given target power state (to avoid
> > pointless reconfiguration of the device), but if the hierarchy above
> > the device goes into D3cold, the device itself will end up in D3cold
> > too, so if it can signal PME from D3cold, it should be enabled to
> > do so in __pci_enable_wake().
> >
> > [Note that if the device does not end up in D3cold and it cannot
> >  signal PME from the original target power state, it will not signal
> >  PME, so in that case the behavior does not change.]
> >
> > Link: https://lore.kernel.org/linux-pm/3149540.aeNJFYEL58@kreacher/
> > Fixes: 5bcc2fb4e815 ("PCI PM: Simplify PCI wake-up code")
> > Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Also this solves the reported issue so,
>
> Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thank you!

I'll queue up these two patches for 5.15 barring any objections from Bjorn.
diff mbox series

Patch

Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -2491,7 +2491,14 @@  static int __pci_enable_wake(struct pci_
 	if (enable) {
 		int error;
 
-		if (pci_pme_capable(dev, state))
+		/*
+		 * Enable PME signaling if the device can signal PME from
+		 * D3cold regardless of whether or not it can signal PME from
+		 * the current target state, because that will allow it to
+		 * signal PME when the hierarchy above it goes into D3cold and
+		 * the device itself ends up in D3cold as a result of that.
+		 */
+		if (pci_pme_capable(dev, state) || pci_pme_capable(dev, PCI_D3cold))
 			pci_pme_active(dev, true);
 		else
 			ret = 1;