On 10/3/2023 14:59, Bjorn Helgaas wrote:
On Tue, Oct 03, 2023 at 02:24:26PM -0500, Mario Limonciello wrote:
On 10/3/2023 14:16, Bjorn Helgaas wrote:
On Tue, Oct 03, 2023 at 01:37:34PM -0500, Mario Limonciello wrote:
On 10/3/2023 13:31, Bjorn Helgaas wrote:
...
That's one thing I liked about the v20 iteration -- instead of
pci_d3cold_disable(), we changed dev->pme_support, which should mean
that we only avoid D3hot/D3cold if we need PMEs while in those states,
so I assumed that we *could* use D3 when we don't need the wakeups.
If you think it's worth spinning again for this optimization I think a
device_may_wakeup() check on the root port can achieve the same result as
the v20 PME solution did, but without the walking of a tree in the quirk.
Why would we use device_may_wakeup() here? That seems like too much
assumption about the suspend path,
Because that's what pci_target_state() passes as well to determine if a
wakeup is needed.
That's exactly what I mean about having too many assumptions here
about other parts of the kernel. I like pme_support because it's the
most specific piece of information about the issue and we don't have
to know anything about how pci_target_state() works to understand it.
Got it.
and we already have the Root Port
pci_dev, so rp->pme_support is available. What about something like
this:
It includes the round trip to config space which Lukas called out as
negative previously but it should work.
True. But I can't get too excited about one config read in the resume
path.
+ rp = pcie_find_root_port(dev);
+ if (!rp->pm_cap)
+ return;
+
+ rp->pme_support &= ~((PCI_PM_CAP_PME_D3hot|PCI_PM_CAP_PME_D3cold) >>
+ PCI_PM_CAP_PME_SHIFT);
Is it actually necessary to look up the Root Port here? Would it be
enough if we removed D3 from the xHCI devices (0x162e, 0x162f, 0x1668,
0x1669), e.g., just do this:
dev->pme_support &= ~((PCI_PM_CAP_PME_D3hot|PCI_PM_CAP_PME_D3cold) >>
PCI_PM_CAP_PME_SHIFT);
I assume that if we knew the xHCI couldn't generate wakeups from D3,
we would leave the xHCI in D0, and that would mean we'd also leave the
Root Port in D0?
Or is the desired behavior that we put the xHCI in D3hot/cold and only
leave the the Root Port in D0?
Bjorn
The intended behavior is that the PCI end points go into D3, but the
root port stays in D0. That matches how Windows behaves.
