| Message ID | 20190107143959.75267-3-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Fix runtime PME generation from D3hot | expand |
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); > } > > /* >
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 >
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 > >
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); } /*
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(-)