On Wed, Oct 19, 2022 at 7:59 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
>
> On some firmware configurations where D3 is not supported for
> "AMD Pink Sardine" the PCIe root port for tunneling will still be
> opted into runtime PM since `acpi_pci_bridge_d3` returns true.
>
> This later causes the device link between the USB4 router and the
> PCIe root port for tunneling to misbehave. The USB4 router may
> enters D3 via runtime PM, but the PCIe root port for tunneling
> remains in D0. The expectation is the USB4 router should also
> remain in D0 since the PCIe root port for tunneling remained in D0.
>
> `acpi_pci_bridge_d3` has a number of checks, but starts out with an
> assumption that if a device is power manageable introduced from
> commit c6e331312ebf ("PCI/ACPI: Whitelist hotplug ports for D3 if power
> managed by ACPI") that it will support D3. This is not a valid
> assumption, as the PCIe root port for tunneling has power resources
> but does not support D3hot or D3cold.
>
> Instead of making this assertion from the power resources check
> immediately, move the check to later on, which will have validated
> that D3hot or D3cold can actually be reached.
>
> This fixes the USB4 router going into D3 when the firmware says that
> the PCIe root port for tunneling can't handle it.
>
> Fixes: dff6139015dc6 ("PCI/ACPI: Allow D3 only if Root Port can signal and wake from D3")
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> drivers/pci/pci-acpi.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a46fec776ad77..1e774a4645663 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -984,10 +984,6 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
> if (acpi_pci_disabled || !dev->is_hotplug_bridge)
> return false;
>
> - /* Assume D3 support if the bridge is power-manageable by ACPI. */
> - if (acpi_pci_power_manageable(dev))
> - return true;
> -
> rpdev = pcie_find_root_port(dev);
> if (!rpdev)
> return false;
> @@ -1023,6 +1019,10 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
> obj->integer.value == 1)
> return true;
>
> + /* Assume D3 support if the bridge is power-manageable by ACPI. */
> + if (acpi_pci_power_manageable(dev))
> + return true;
> +
> return false;
return acpi_pci_power_manageable(dev);
would be simpler.
Otherwise it looks OK to me.
> }
>
> --
> 2.34.1
>
