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/12/2023 23:25, Lukas Wunner wrote:
> On Tue, Sep 12, 2023 at 11:08:32PM -0500, Mario Limonciello wrote:
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -2752,6 +2752,11 @@ int pci_prepare_to_sleep(struct pci_dev *dev)
>>   	if (target_state == PCI_POWER_ERROR)
>>   		return -EIO;
>>   
>> +	/* quirk to avoid setting D3 */
>> +	if (wakeup && dev->dev_flags & PCI_DEV_FLAGS_NO_WAKE_D3 &&
>> +	   (target_state == PCI_D3hot || target_state == PCI_D3cold))
>> +		target_state = PCI_D0;
>> +
>>   	pci_enable_wake(dev, target_state, wakeup);
>>   
>>   	error = pci_set_power_state(dev, target_state);
> 
> Would it be possible to just add the affected system to
> bridge_d3_blacklist[]?

It's initially reported on Lenovo Z13, but it affects all Rembrandt and 
Phoenix machines that have USB4 controller enabled.

It's reproduced on every OEM system I have access to.

> 
> Or would that defeat power management of other (non-affected)
> Root Ports in the same machine?
> 
> There's already PCI_DEV_FLAGS_NO_D3, would it be possible to just
> reuse that instead of adding another codepath for D3 quirks?
> 

The root port can handle D3 (including wakeup) at runtime fine.
Issue occurs only during s2idle w/ hardware sleep.

In v16/v17 (see cover letter for links) Rafael suggested to tie this 
specifically to suspend behavior and when wakeup flag is set.

I didn't think it was appropriate to overload the existing flag because 
of this difference.

> Thanks,
> 
> Lukas