lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] PCI/ACPI: Don't assume D3 support if a device is power manageable
On Wed, Oct 19, 2022 at 7:59 PM Mario Limonciello
<mario.limonciello@amd.com> 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@amd.com>
> ---
> 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
>

\
 
 \ /
  Last update: 2022-10-19 20:15    [W:0.022 / U:0.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site