Re: [PATCH v18 2/2] PCI: Add a quirk for AMD PCIe root ports w/ USB4 controllers

[Mario Limonciello <mario.limonciello@amd.com>]

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.

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.