Linux-PCI Archive on lore.kernel.org
 help / Atom feed
* [PATCH 0/2] PCI: Fix runtime PME generation from D3hot
@ 2019-01-07 14:39 Mika Westerberg
  2019-01-07 14:39 ` [PATCH 1/2] Revert "PCI/PME: Implement runtime PM callbacks" Mika Westerberg
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Mika Westerberg @ 2019-01-07 14:39 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki
  Cc: Lukas Wunner, Heiner Kallweit, Sinan Kaya, Keith Busch,
	Oza Pawandeep, Mika Westerberg, linux-pci, linux-kernel

Hi all,

Heiner reported [1] that runtime PME generation of his network card does
not work after commit 0e157e528604 ("PCI/PME: Implement runtime PM
callbacks") that landed in v4.20. Reverting the commit helps but it has
another drawback, which I originally tried to solve with the commit, that
the PCIe hierarchy wakes up immediately after being put into D3cold.

This series of two patches tries to fix both issues so that PME wakes up
from D3hot and that the hierarchy does not wake up immediately from D3cold.

[1] https://www.spinics.net/lists/linux-pci/msg79051.html

Mika Westerberg (2):
  Revert "PCI/PME: Implement runtime PM callbacks"
  PCI: pciehp: Disable Data Link Layer State Changed event on suspend

 drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++--
 drivers/pci/pcie/pme.c           | 27 ---------------------------
 2 files changed, 8 insertions(+), 29 deletions(-)

-- 
2.19.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] Revert "PCI/PME: Implement runtime PM callbacks"
  2019-01-07 14:39 [PATCH 0/2] PCI: Fix runtime PME generation from D3hot Mika Westerberg
@ 2019-01-07 14:39 ` Mika Westerberg
  2019-01-07 22:26   ` Rafael J. Wysocki
  2019-01-07 14:39 ` [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend Mika Westerberg
  2019-01-07 18:54 ` [PATCH 0/2] PCI: Fix runtime PME generation from D3hot Heiner Kallweit
  2 siblings, 1 reply; 8+ messages in thread
From: Mika Westerberg @ 2019-01-07 14:39 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki
  Cc: Lukas Wunner, Heiner Kallweit, Sinan Kaya, Keith Busch,
	Oza Pawandeep, Mika Westerberg, linux-pci, linux-kernel

This reverts commit 0e157e52860441cb26051f131dd0b5ae3187a07b.

Heiner reported that the commit in question prevents his network adapter
from triggering PME and waking up when network cable is plugged.

The commit tried to prevent root port waking up from D3cold immediately
but looks like disabing root port PME interrupt is not the right way to
fix that issue so revert it now. The patch following proposes an
alternative solution to that issue.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=202103
Fixes: 0e157e528604 ("PCI/PME: Implement runtime PM callbacks")
Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/pcie/pme.c | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
index 0dbcf429089f..1a8b85051b1b 100644
--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -432,31 +432,6 @@ static void pcie_pme_remove(struct pcie_device *srv)
 	kfree(get_service_data(srv));
 }
 
-static int pcie_pme_runtime_suspend(struct pcie_device *srv)
-{
-	struct pcie_pme_service_data *data = get_service_data(srv);
-
-	spin_lock_irq(&data->lock);
-	pcie_pme_interrupt_enable(srv->port, false);
-	pcie_clear_root_pme_status(srv->port);
-	data->noirq = true;
-	spin_unlock_irq(&data->lock);
-
-	return 0;
-}
-
-static int pcie_pme_runtime_resume(struct pcie_device *srv)
-{
-	struct pcie_pme_service_data *data = get_service_data(srv);
-
-	spin_lock_irq(&data->lock);
-	pcie_pme_interrupt_enable(srv->port, true);
-	data->noirq = false;
-	spin_unlock_irq(&data->lock);
-
-	return 0;
-}
-
 static struct pcie_port_service_driver pcie_pme_driver = {
 	.name		= "pcie_pme",
 	.port_type	= PCI_EXP_TYPE_ROOT_PORT,
@@ -464,8 +439,6 @@ static struct pcie_port_service_driver pcie_pme_driver = {
 
 	.probe		= pcie_pme_probe,
 	.suspend	= pcie_pme_suspend,
-	.runtime_suspend = pcie_pme_runtime_suspend,
-	.runtime_resume	= pcie_pme_runtime_resume,
 	.resume		= pcie_pme_resume,
 	.remove		= pcie_pme_remove,
 };
-- 
2.19.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend
  2019-01-07 14:39 [PATCH 0/2] PCI: Fix runtime PME generation from D3hot Mika Westerberg
  2019-01-07 14:39 ` [PATCH 1/2] Revert "PCI/PME: Implement runtime PM callbacks" Mika Westerberg
@ 2019-01-07 14:39 ` Mika Westerberg
  2019-01-07 22:26   ` Rafael J. Wysocki
  2019-01-15  0:24   ` Bjorn Helgaas
  2019-01-07 18:54 ` [PATCH 0/2] PCI: Fix runtime PME generation from D3hot Heiner Kallweit
  2 siblings, 2 replies; 8+ messages in thread
From: Mika Westerberg @ 2019-01-07 14:39 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki
  Cc: Lukas Wunner, Heiner Kallweit, Sinan Kaya, Keith Busch,
	Oza Pawandeep, Mika Westerberg, linux-pci, linux-kernel

Commit 0e157e528604 ("PCI/PME: Implement runtime PM callbacks") tried to
solve an issue where the hierarchy immediately wakes up when it is
transitioned into D3cold. It turns out preventing PME propagation in
some PCIe hierarchies not supporting D3cold.

I looked more closely what might cause the immediate wakeup. It happens
when the ACPI power resource of the root port is turned off. The AML
code associated with the _OFF() method of the ACPI power resource
executes PCIe L2/3 ready transition and waits it to complete. Right
after the L2/3 ready transition is started the root port receives PME
from the downstream port.

The simplest hierarchy where this happens looks like this:

  00:1d.0 PCIe Root port
    ^
    |
    v
    05:00.0 PCIe switch #1 upstream port
      06:01.0 PCIe switch #1 downstream hotplug port
        ^
        |
        v
        08:00.0 Pcie switch #2 upstream port

It seems that the PCIe link between the two switches, before
PME_Turn_Off/PME_TO_Ack is complete for the whole hierarchy, goes
inactive and triggers PME towards the root port bringing it back to D0.
Disabling Data Link Layer State Changed event (DLLSCE) prevents the
issue and still allows the downstream hotplug port to notice when a
device is plugged/unplugged.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=202103
Fixes: 0e157e528604 ("PCI/PME: Implement runtime PM callbacks")
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index cd9eae650aa5..6fdaa8d48ebe 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -736,12 +736,18 @@ void pcie_clear_hotplug_events(struct controller *ctrl)
 
 void pcie_enable_interrupt(struct controller *ctrl)
 {
-	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_HPIE, PCI_EXP_SLTCTL_HPIE);
+	u16 mask;
+
+	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
+	pcie_write_cmd(ctrl, mask, mask);
 }
 
 void pcie_disable_interrupt(struct controller *ctrl)
 {
-	pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_HPIE);
+	u16 mask;
+
+	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
+	pcie_write_cmd(ctrl, 0, mask);
 }
 
 /*
-- 
2.19.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] PCI: Fix runtime PME generation from D3hot
  2019-01-07 14:39 [PATCH 0/2] PCI: Fix runtime PME generation from D3hot Mika Westerberg
  2019-01-07 14:39 ` [PATCH 1/2] Revert "PCI/PME: Implement runtime PM callbacks" Mika Westerberg
  2019-01-07 14:39 ` [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend Mika Westerberg
@ 2019-01-07 18:54 ` Heiner Kallweit
  2 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2019-01-07 18:54 UTC (permalink / raw)
  To: Mika Westerberg, Bjorn Helgaas, Rafael J. Wysocki
  Cc: Lukas Wunner, Sinan Kaya, Keith Busch, Oza Pawandeep, linux-pci,
	linux-kernel

On 07.01.2019 15:39, Mika Westerberg wrote:
> Hi all,
> 
> Heiner reported [1] that runtime PME generation of his network card does
> not work after commit 0e157e528604 ("PCI/PME: Implement runtime PM
> callbacks") that landed in v4.20. Reverting the commit helps but it has
> another drawback, which I originally tried to solve with the commit, that
> the PCIe hierarchy wakes up immediately after being put into D3cold.
> 
> This series of two patches tries to fix both issues so that PME wakes up
> from D3hot and that the hierarchy does not wake up immediately from D3cold.
> 
> [1] https://www.spinics.net/lists/linux-pci/msg79051.html
> 
> Mika Westerberg (2):
>   Revert "PCI/PME: Implement runtime PM callbacks"
>   PCI: pciehp: Disable Data Link Layer State Changed event on suspend
> 
>  drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++--
>  drivers/pci/pcie/pme.c           | 27 ---------------------------
>  2 files changed, 8 insertions(+), 29 deletions(-)
> 
Works for me. But that's not a miracle because on my system
CONFIG_HOTPLUG_PCI isn't set and therefore effectively just the
runtime ops are removed again. And we tested before that this
fixes the issue for me. Thanks for the analysis and the fix!

So for the runtime PM PME generation:
Tested-by: Heiner Kallweit <hkallweit1@gmail.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] Revert "PCI/PME: Implement runtime PM callbacks"
  2019-01-07 14:39 ` [PATCH 1/2] Revert "PCI/PME: Implement runtime PM callbacks" Mika Westerberg
@ 2019-01-07 22:26   ` Rafael J. Wysocki
  0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2019-01-07 22:26 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Bjorn Helgaas, Lukas Wunner, Heiner Kallweit, Sinan Kaya,
	Keith Busch, Oza Pawandeep, linux-pci, linux-kernel

On Monday, January 7, 2019 3:39:58 PM CET Mika Westerberg wrote:
> This reverts commit 0e157e52860441cb26051f131dd0b5ae3187a07b.
> 
> Heiner reported that the commit in question prevents his network adapter
> from triggering PME and waking up when network cable is plugged.
> 
> The commit tried to prevent root port waking up from D3cold immediately
> but looks like disabing root port PME interrupt is not the right way to
> fix that issue so revert it now. The patch following proposes an
> alternative solution to that issue.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202103
> Fixes: 0e157e528604 ("PCI/PME: Implement runtime PM callbacks")
> Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/pci/pcie/pme.c | 27 ---------------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
> index 0dbcf429089f..1a8b85051b1b 100644
> --- a/drivers/pci/pcie/pme.c
> +++ b/drivers/pci/pcie/pme.c
> @@ -432,31 +432,6 @@ static void pcie_pme_remove(struct pcie_device *srv)
>  	kfree(get_service_data(srv));
>  }
>  
> -static int pcie_pme_runtime_suspend(struct pcie_device *srv)
> -{
> -	struct pcie_pme_service_data *data = get_service_data(srv);
> -
> -	spin_lock_irq(&data->lock);
> -	pcie_pme_interrupt_enable(srv->port, false);
> -	pcie_clear_root_pme_status(srv->port);
> -	data->noirq = true;
> -	spin_unlock_irq(&data->lock);
> -
> -	return 0;
> -}
> -
> -static int pcie_pme_runtime_resume(struct pcie_device *srv)
> -{
> -	struct pcie_pme_service_data *data = get_service_data(srv);
> -
> -	spin_lock_irq(&data->lock);
> -	pcie_pme_interrupt_enable(srv->port, true);
> -	data->noirq = false;
> -	spin_unlock_irq(&data->lock);
> -
> -	return 0;
> -}
> -
>  static struct pcie_port_service_driver pcie_pme_driver = {
>  	.name		= "pcie_pme",
>  	.port_type	= PCI_EXP_TYPE_ROOT_PORT,
> @@ -464,8 +439,6 @@ static struct pcie_port_service_driver pcie_pme_driver = {
>  
>  	.probe		= pcie_pme_probe,
>  	.suspend	= pcie_pme_suspend,
> -	.runtime_suspend = pcie_pme_runtime_suspend,
> -	.runtime_resume	= pcie_pme_runtime_resume,
>  	.resume		= pcie_pme_resume,
>  	.remove		= pcie_pme_remove,
>  };
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend
  2019-01-07 14:39 ` [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend Mika Westerberg
@ 2019-01-07 22:26   ` Rafael J. Wysocki
  2019-01-15  0:24   ` Bjorn Helgaas
  1 sibling, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2019-01-07 22:26 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Bjorn Helgaas, Lukas Wunner, Heiner Kallweit, Sinan Kaya,
	Keith Busch, Oza Pawandeep, linux-pci, linux-kernel

On Monday, January 7, 2019 3:39:59 PM CET Mika Westerberg wrote:
> Commit 0e157e528604 ("PCI/PME: Implement runtime PM callbacks") tried to
> solve an issue where the hierarchy immediately wakes up when it is
> transitioned into D3cold. It turns out preventing PME propagation in
> some PCIe hierarchies not supporting D3cold.
> 
> I looked more closely what might cause the immediate wakeup. It happens
> when the ACPI power resource of the root port is turned off. The AML
> code associated with the _OFF() method of the ACPI power resource
> executes PCIe L2/3 ready transition and waits it to complete. Right
> after the L2/3 ready transition is started the root port receives PME
> from the downstream port.
> 
> The simplest hierarchy where this happens looks like this:
> 
>   00:1d.0 PCIe Root port
>     ^
>     |
>     v
>     05:00.0 PCIe switch #1 upstream port
>       06:01.0 PCIe switch #1 downstream hotplug port
>         ^
>         |
>         v
>         08:00.0 Pcie switch #2 upstream port
> 
> It seems that the PCIe link between the two switches, before
> PME_Turn_Off/PME_TO_Ack is complete for the whole hierarchy, goes
> inactive and triggers PME towards the root port bringing it back to D0.
> Disabling Data Link Layer State Changed event (DLLSCE) prevents the
> issue and still allows the downstream hotplug port to notice when a
> device is plugged/unplugged.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202103
> Fixes: 0e157e528604 ("PCI/PME: Implement runtime PM callbacks")
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index cd9eae650aa5..6fdaa8d48ebe 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -736,12 +736,18 @@ void pcie_clear_hotplug_events(struct controller *ctrl)
>  
>  void pcie_enable_interrupt(struct controller *ctrl)
>  {
> -	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_HPIE, PCI_EXP_SLTCTL_HPIE);
> +	u16 mask;
> +
> +	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
> +	pcie_write_cmd(ctrl, mask, mask);
>  }
>  
>  void pcie_disable_interrupt(struct controller *ctrl)
>  {
> -	pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_HPIE);
> +	u16 mask;
> +
> +	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
> +	pcie_write_cmd(ctrl, 0, mask);
>  }
>  
>  /*
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend
  2019-01-07 14:39 ` [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend Mika Westerberg
  2019-01-07 22:26   ` Rafael J. Wysocki
@ 2019-01-15  0:24   ` Bjorn Helgaas
  2019-01-21 10:22     ` Mika Westerberg
  1 sibling, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2019-01-15  0:24 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Rafael J. Wysocki, Lukas Wunner, Heiner Kallweit, Sinan Kaya,
	Keith Busch, Oza Pawandeep, linux-pci, linux-kernel

On Mon, Jan 07, 2019 at 05:39:59PM +0300, Mika Westerberg wrote:
> Commit 0e157e528604 ("PCI/PME: Implement runtime PM callbacks") tried to
> solve an issue where the hierarchy immediately wakes up when it is
> transitioned into D3cold. It turns out preventing PME propagation in
> some PCIe hierarchies not supporting D3cold.

I can't quite parse this last sentence.  Is it missing a word?

> I looked more closely what might cause the immediate wakeup. It happens
> when the ACPI power resource of the root port is turned off. The AML
> code associated with the _OFF() method of the ACPI power resource
> executes PCIe L2/3 ready transition and waits it to complete.

waits *for* it ...

> Right
> after the L2/3 ready transition is started the root port receives PME
> from the downstream port.
> 
> The simplest hierarchy where this happens looks like this:
> 
>   00:1d.0 PCIe Root port
>     ^
>     |
>     v
>     05:00.0 PCIe switch #1 upstream port
>       06:01.0 PCIe switch #1 downstream hotplug port
>         ^
>         |
>         v
>         08:00.0 Pcie switch #2 upstream port
> 
> It seems that the PCIe link between the two switches, before
> PME_Turn_Off/PME_TO_Ack is complete for the whole hierarchy, goes
> inactive and triggers PME towards the root port bringing it back to D0.
> Disabling Data Link Layer State Changed event (DLLSCE) prevents the
> issue and still allows the downstream hotplug port to notice when a
> device is plugged/unplugged.

I don't understand the "link goes inactive and triggers PME" part.  Is
that prescribed by the spec, or is this implementation-specific
behavior, or even a bug?

Are there spec references that would be useful here?  PCIe r4.0, sec
5.2 seems like one starting point.  5.3.1.4.2?  5.3.3.2.1 and
following sections seem like they should be very relevant, but I
haven't studied it in detail.

> Link: https://bugzilla.kernel.org/show_bug.cgi?id=202103
> Fixes: 0e157e528604 ("PCI/PME: Implement runtime PM callbacks")
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index cd9eae650aa5..6fdaa8d48ebe 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -736,12 +736,18 @@ void pcie_clear_hotplug_events(struct controller *ctrl)
>  
>  void pcie_enable_interrupt(struct controller *ctrl)
>  {
> -	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_HPIE, PCI_EXP_SLTCTL_HPIE);
> +	u16 mask;
> +
> +	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
> +	pcie_write_cmd(ctrl, mask, mask);
>  }
>  
>  void pcie_disable_interrupt(struct controller *ctrl)
>  {
> -	pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_HPIE);
> +	u16 mask;
> +

I think some sort of comment here would be useful.  It's pretty hard
for the casual reader to figure out what things need to be masked.

> +	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
> +	pcie_write_cmd(ctrl, 0, mask);
>  }
>  
>  /*
> -- 
> 2.19.2
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend
  2019-01-15  0:24   ` Bjorn Helgaas
@ 2019-01-21 10:22     ` Mika Westerberg
  0 siblings, 0 replies; 8+ messages in thread
From: Mika Westerberg @ 2019-01-21 10:22 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rafael J. Wysocki, Lukas Wunner, Heiner Kallweit, Sinan Kaya,
	Keith Busch, Oza Pawandeep, linux-pci, linux-kernel

Hi Bjorn,

Sorry for the delay - I was on a business trip.

On Mon, Jan 14, 2019 at 06:24:10PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 07, 2019 at 05:39:59PM +0300, Mika Westerberg wrote:
> > Commit 0e157e528604 ("PCI/PME: Implement runtime PM callbacks") tried to
> > solve an issue where the hierarchy immediately wakes up when it is
> > transitioned into D3cold. It turns out preventing PME propagation in
> > some PCIe hierarchies not supporting D3cold.
> 
> I can't quite parse this last sentence.  Is it missing a word?

No, but I'm not native speaker :)

> > I looked more closely what might cause the immediate wakeup. It happens
> > when the ACPI power resource of the root port is turned off. The AML
> > code associated with the _OFF() method of the ACPI power resource
> > executes PCIe L2/3 ready transition and waits it to complete.
> 
> waits *for* it ...


OK.

> > Right
> > after the L2/3 ready transition is started the root port receives PME
> > from the downstream port.
> > 
> > The simplest hierarchy where this happens looks like this:
> > 
> >   00:1d.0 PCIe Root port
> >     ^
> >     |
> >     v
> >     05:00.0 PCIe switch #1 upstream port
> >       06:01.0 PCIe switch #1 downstream hotplug port
> >         ^
> >         |
> >         v
> >         08:00.0 Pcie switch #2 upstream port
> > 
> > It seems that the PCIe link between the two switches, before
> > PME_Turn_Off/PME_TO_Ack is complete for the whole hierarchy, goes
> > inactive and triggers PME towards the root port bringing it back to D0.
> > Disabling Data Link Layer State Changed event (DLLSCE) prevents the
> > issue and still allows the downstream hotplug port to notice when a
> > device is plugged/unplugged.
> 
> I don't understand the "link goes inactive and triggers PME" part.  Is
> that prescribed by the spec, or is this implementation-specific
> behavior, or even a bug?

It is not directly described in the spec (unfortunately). In general
they leave a lot for the reader to interpret instead of explaining how
the thing is supposed to be programmed in different situations :(

This is the behavior I see when the hierarchy goes into D3cold. I don't
have any equipment that would let me see what exactly happens there. The
above is my guess of what happens.

> Are there spec references that would be useful here?  PCIe r4.0, sec
> 5.2 seems like one starting point.  5.3.1.4.2?  5.3.3.2.1 and
> following sections seem like they should be very relevant, but I
> haven't studied it in detail.

Sure, I can add those references to the changelog.

> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202103
> > Fixes: 0e157e528604 ("PCI/PME: Implement runtime PM callbacks")
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/pci/hotplug/pciehp_hpc.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> > index cd9eae650aa5..6fdaa8d48ebe 100644
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -736,12 +736,18 @@ void pcie_clear_hotplug_events(struct controller *ctrl)
> >  
> >  void pcie_enable_interrupt(struct controller *ctrl)
> >  {
> > -	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_HPIE, PCI_EXP_SLTCTL_HPIE);
> > +	u16 mask;
> > +
> > +	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
> > +	pcie_write_cmd(ctrl, mask, mask);
> >  }
> >  
> >  void pcie_disable_interrupt(struct controller *ctrl)
> >  {
> > -	pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_HPIE);
> > +	u16 mask;
> > +
> 
> I think some sort of comment here would be useful.  It's pretty hard
> for the casual reader to figure out what things need to be masked.

Sure, I'll add a comment.

> > +	mask = PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_DLLSCE;
> > +	pcie_write_cmd(ctrl, 0, mask);
> >  }
> >  
> >  /*
> > -- 
> > 2.19.2
> > 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 14:39 [PATCH 0/2] PCI: Fix runtime PME generation from D3hot Mika Westerberg
2019-01-07 14:39 ` [PATCH 1/2] Revert "PCI/PME: Implement runtime PM callbacks" Mika Westerberg
2019-01-07 22:26   ` Rafael J. Wysocki
2019-01-07 14:39 ` [PATCH 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend Mika Westerberg
2019-01-07 22:26   ` Rafael J. Wysocki
2019-01-15  0:24   ` Bjorn Helgaas
2019-01-21 10:22     ` Mika Westerberg
2019-01-07 18:54 ` [PATCH 0/2] PCI: Fix runtime PME generation from D3hot Heiner Kallweit

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org linux-pci@archiver.kernel.org
	public-inbox-index linux-pci


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox