On 9/14/2023 07:32, Bjorn Helgaas wrote:
On Wed, Sep 13, 2023 at 11:59:00PM -0500, Mario Limonciello wrote:
On 9/13/2023 16:16, Mario Limonciello wrote:
On 9/13/2023 16:05, Bjorn Helgaas wrote:
[cut]
I expect it to be an ongoing issue. I also expect unless we use
constraints or convince the firmware team to add a _S0W object with a
value of "0" for the sake of Linux that we will be adding
IDs every year
to wherever this lands as we reproduce it on newer SoCs.
So maybe the way to go is to make the AMD PMC driver set a flag for
Root Ports on suspend or similar.
I like the quirk approach. When PMC is involved, the device behavior
doesn't conform to what it advertised via PME_Support.
The v18 quirk isn't connected to PMC at all, so IIUC it avoids
D3hot/D3cold unnecessarily when amd/pmc is not loaded.
Technically someone could; but realistically no one will be using these
machines without amd-pmc.
The battery life over suspend would be abhorrent.
I don't object to avoiding D3hot/D3cold unconditionally. Presumably
we *could* save a little power by using them when amd/pci isn't
loaded, but amd/pci would have to iterate through all PCI devices when
it loads, save previous state, do the quirk, and then restore the
previous state on module unload. And it would have to use notifiers
or assume no Root Port hotplug. All sounds kind of complicated.
Yeah this does sound needlessly complicated.
Maybe it would even be enough to just clear dev->pme_support so we
know wakeups don't work. It would be a pretty big benefit if we
didn't have to add another bit and complicate pci_prepare_to_sleep()
or pci_target_state().
I don't think clearing PME support entirely is going to help. The
reason is that pci_target_state() will fall back to PCI_D3hot when
dev->pme_support is fully cleared.
I think that clearing *just the bits* for D3hot and D3cold in PME
support should work though. I'll test this.
I did confirm this works properly.
Assuming it works how about if we put the quirk to clear the
D3hot/D3cold PME support bit in
drivers/platform/x86/amd/pmc/pmc-quirks.c?
It's still a quirk file and it makes it very clear that this behavior is
caused by what amd-pmc does.
I've got it coded up like this and working, so please let me know if this
approach is amenable and I'll drop an updated version.
If you would prefer it to be in pci/quirks.c I believe I can do either.
If the quirk is in a loadable module, as opposed to being built-in,
does it get applied to the relevant Root Ports when the module is
loaded? I didn't look exhaustively, but I don't see a reference to
pci_fixup_device() in the module load path.
Bjorn
Right; when done in a module it would be done with code that is part of
probe.
So it has the implication that it would prevent D3hot/D3cold for this
root port at runtime as well.
If you think it should be tied to pci_fixup_device() calls then it needs
to be built-in.
