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.