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/14/2023 09:17, Lukas Wunner wrote:
> On Wed, Sep 13, 2023 at 11:36:49AM -0500, Mario Limonciello wrote:
>> On 9/13/2023 09:31, Lukas Wunner wrote:
>>> If this only affects system sleep, not runtime PM, what you can do is
>>> define a DECLARE_PCI_FIXUP_SUSPEND_LATE() which calls pci_d3cold_disable()
>>> and also define a DECLARE_PCI_FIXUP_CLASS_RESUME_EARLY() which calls
>>> pci_d3cold_enable().
>>>
>>> And I think you can make those calls conditional on pm_suspend_no_platform()
>>> to constrain to s2idle.
>>>
>>> User space should still be able to influence runtime PM via the
>>> d3cold_allowed flag (unless I'm missing something).
>>
>> The part you're missing is that D3hot is affected by this issue too,
>> otherwise it would be a good proposal.
> 
> I recall that in an earlier version of the patch, you solved the issue
> by amending pci_bridge_d3_possible().
> 
> Changing the dev->no_d3cold flag indirectly influences the bridge_d3
> flag (through pci_dev_check_d3cold() and pci_bridge_d3_update()).
> 
> If dev->no_d3cold is set on a device below a port, that port is
> prevented from entring D3hot because it would result in the
> device effectively being in D3cold.
> 
> So you might want to take a closer look at this approach despite
> the flag suggesting that it only influences D3cold.
> 

Ah; I hadn't considered setting it on a device below the port. In this 
particular situation the only devices below the root port are USB 
controllers.

If those devices don't go into D3 the system can't enter hardware sleep.