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
