On 8/29/2023 10:42 PM, Mario Limonciello wrote:
> commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
> changed pci_bridge_d3_possible() so that any vendor's PCIe ports
> from modern machines (>=2015) are allowed to be put into D3.
>
> Iain reports that USB devices can't be used to wake a Lenovo Z13
> from suspend. This is because the PCIe root port has been put
> into D3 and AMD's platform can't handle USB devices waking from
> a hardware sleep state in this case.
>
> This problem only occurs on Linux, and only when the AMD PMC driver
> is utilized to put the device into a hardware sleep state. Comparing
> the behavior on Windows and Linux, Windows doesn't put the root ports
> into D3.
>
> A variety of approaches were discussed to change PCI core to handle this
> case generically but no consensus was reached. To limit the scope of
> effect only to the affected machines introduce a workaround into the
> amd-pmc driver to only apply to the PCI root ports in affected machines
> when going into hardware sleep.
>
> Link: https://lore.kernel.org/linux-pci/20230818193932.27187-1-mario.limonciello@xxxxxxx/
> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
> Reported-by: Iain Lane <iain@xxxxxxxxxxxxxxxxxxx>
> Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
See if this change can be moved to pmc-quirks.c, besides that change
looks good to me. Thank you.
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> ---
> v15->v16:
> * Only match PCIe root ports with ACPI companions
> * Use constraints when workaround activated
> ---
> drivers/platform/x86/amd/pmc/pmc.c | 39 ++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index eb2a4263814c..6a037447ec5a 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -741,6 +741,41 @@ static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev)
> return 0;
> }
>
> +/* only allow PCIe root ports with a LPS0 constraint configured to go to D3 */
> +static int amd_pmc_rp_wa(struct amd_pmc_dev *pdev)
> +{
> + struct pci_dev *pci_dev = NULL;
> +
> + while ((pci_dev = pci_get_device(PCI_VENDOR_ID_AMD, PCI_ANY_ID, pci_dev))) {
> + struct acpi_device *adev;
> + int constraint;
> +
> + if (!pci_is_pcie(pci_dev) ||
> + !(pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ROOT_PORT))
> + continue;
> +
> + if (pci_dev->current_state == PCI_D3hot ||
> + pci_dev->current_state == PCI_D3cold)
> + continue;
> +
> + adev = ACPI_COMPANION(&pci_dev->dev);
> + if (!adev)
> + continue;
> +
> + constraint = acpi_get_lps0_constraint(adev);
> + if (constraint != ACPI_STATE_UNKNOWN &&
> + constraint >= ACPI_STATE_S3)
> + continue;
> +
> + if (pci_dev->bridge_d3 == 0)
> + continue;
> + pci_dev->bridge_d3 = 0;
> + dev_info(&pci_dev->dev, "Disabling D3 on PCIe root port due lack of constraint\n");
> + }
> +
> + return 0;
> +}
> +
> static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
> {
> struct rtc_device *rtc_device;
> @@ -893,6 +928,10 @@ static int amd_pmc_suspend_handler(struct device *dev)
> case AMD_CPU_ID_CZN:
> rc = amd_pmc_czn_wa_irq1(pdev);
> break;
> + case AMD_CPU_ID_YC:
> + case AMD_CPU_ID_PS:
> + rc = amd_pmc_rp_wa(pdev);
> + break;
> default:
> break;
> }
>
