Message ID | 20230708214457.1229-2-mario.limonciello@amd.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f45:0:b0:3ea:f831:8777 with SMTP id v5csp4587484vqx; Sun, 9 Jul 2023 12:11:44 -0700 (PDT) X-Google-Smtp-Source: APBJJlEU77or51PsY17MIb/nM81ET1cHmAaQzEl407UTac/Uz6qvHCogjsIWBWe0BHSTVyYlIbXD X-Received: by 2002:a17:902:ce89:b0:1b9:e1d6:7c7d with SMTP id f9-20020a170902ce8900b001b9e1d67c7dmr729790plg.47.1688929904417; Sun, 09 Jul 2023 12:11:44 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1688929904; cv=pass; d=google.com; s=arc-20160816; b=agWYqCtoxrfzd2GnpSg/kayxdnUGqXYQD9f/a2CY8epfwc2xmVd4EJbKiX7yiEmtxL bwx4eFRRxsa0IkPN9Vd+GPbApL6ZOYRKQ4E6B6t6vBWmU54vSqJ/M2ynBTH6hw7cbTEN IwJDRQaLY5nYQPeoqDkhqD8eB7IcVaihZdNlNgtMT2Q5jxv6eSdQ003l9jqeFNCKqEdh OruCvksas6jwZvKgCXOrVB4IVw2oZmeHUZHfjMSQw2VEAo/zA5OiSYQ4FrAimKGkeR9u j2l8qpYGTF3jmyD8IoyLdL5owKyYf7OGMqdD2dSAj3mj7xlwDMmcZ1XCyt0CZYFQXgNN kGjA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=hevWxkLHIDjLd6V3Vug2xDLsaqM8IxPfJVj9nulxmo0=; fh=D/2p7XbMqy+Z5V5r/WWcz/2WyY57er6uJPjovSqgt2s=; b=GIxYg7yffR0TdHs7B9219cGYsjev+Nz5HNWdQJts7zugE3kFDotzyJ34Bhpnt0omxG /YOBz7KLSYYMwyNGroai86GOhOiQPWW5So3AdoRu8y0IhzZEbumosCWVmA3pIdJkqdE1 7a6tHJkhfJVbFWZ9wh/xV8eCOkB4ti+EdBlJVqSzECTZ4E8vxbwYcGKSmTcn1/sd7l57 94iQKRxuYIa4wzfedI4m4hfCezL+GAcNQ+2n0sUqBCRIeAQ2+huDHgrePgjOnF3MXvga RBMqedI4ac9mjyCNKaTYqvR/0USlc1468rZ1i6NPciJh+6/+oE+k03oVnLd1P0VcitfX d6uQ== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=RjnBiY8j; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y8-20020a17090264c800b001b9c3498526si5135883pli.433.2023.07.09.12.11.29; Sun, 09 Jul 2023 12:11:44 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=RjnBiY8j; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229626AbjGITAk (ORCPT <rfc822;tebrre53rla2o@gmail.com> + 99 others); Sun, 9 Jul 2023 15:00:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59552 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229450AbjGITAi (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 9 Jul 2023 15:00:38 -0400 Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam04on2079.outbound.protection.outlook.com [40.107.101.79]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5EE40128; Sun, 9 Jul 2023 12:00:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=h11nbn2CzIrfzx0fe49tX5Kqj24mWfaY19yXG48FQ8GUVtbDFhV/M403dNoQTqs6D+x7bNKA2Y0JpJ2Cnh1Rn15LpeVoez2n5pMItyg7MT9zqvW31R/y/I07sE7VtpaL2PUqiDfZ0RSX4QWkJ1/ehfEanuQhH5oWXLkbr/myktoY4LnOZwbijPJvL+KB/m5h7ER+GaJyy+JAfFvryA/HYITPzAADzIV/NMpzvFma/FzjJOPmAIYrMWytwWtGPpUyQZk1WSKGqu85x2BPGXCnug/lDVlZWHttJDGy8d/0IleuGb0Ez5QkOvtdgCn/D/g2T5byR2NTJePingfvz8ZPig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=hevWxkLHIDjLd6V3Vug2xDLsaqM8IxPfJVj9nulxmo0=; b=Q0kooEagauFEUxECqArmX0gqH26qHaa8aiOPQV1JswL7hRzUkEoUjRJnv2Q8eibVvfQh5lkToNDNxOLkKNVcxY9dPZvTO7OV4GvjUo6N2JT+AoOvj6c5h4NfICKrOJ93b7PKizqJ6YfQ4F8lm1xAb/AWuS1AhhL7XDbpTBT/B1t2SNp+51Dx7RoW8HarY7Tv8fFFCbQh0LZxPYOvsN6M18FXDTWMm0txzWZgyVwyw5YtKbnv5jnc1dTPiTt6bCRLuGloOri51gZzQCOuJj2WNbi19K93vsGqqWF3rBoEfoL4ZkgkOA2X5KhlnUboB0IY0pfeDtVQLgT7CzxRs/gXeA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=kernel.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=hevWxkLHIDjLd6V3Vug2xDLsaqM8IxPfJVj9nulxmo0=; b=RjnBiY8jgga+HBsQC2UFqiqpEFoNOnVCZom3LC0HmhxB7NwaBZlbvzb/xU+JAIRFdlsIGlKq6qgMUHWqkwSg3wmIZGVGFcjnNBmyKFWxhIPlBDUOzZ/8jj8SvXCxDU+wE3ta4zFy+RsDxSeu/Rv+0q6SdCvY99NSksPJO/8JQFk= Received: from BN0PR03CA0032.namprd03.prod.outlook.com (2603:10b6:408:e7::7) by DS0PR12MB7971.namprd12.prod.outlook.com (2603:10b6:8:14e::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6565.26; Sun, 9 Jul 2023 19:00:34 +0000 Received: from BN8NAM11FT030.eop-nam11.prod.protection.outlook.com (2603:10b6:408:e7:cafe::d0) by BN0PR03CA0032.outlook.office365.com (2603:10b6:408:e7::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6565.30 via Frontend Transport; Sun, 9 Jul 2023 19:00:34 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by BN8NAM11FT030.mail.protection.outlook.com (10.13.177.146) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.6588.18 via Frontend Transport; Sun, 9 Jul 2023 19:00:34 +0000 Received: from SITE-L-T34-2.amd.com (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Sun, 9 Jul 2023 14:00:33 -0500 From: Mario Limonciello <mario.limonciello@amd.com> To: "Rafael J . Wysocki" <rafael@kernel.org>, Bjorn Helgaas <helgaas@kernel.org> CC: <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Len Brown <lenb@kernel.org>, <linux-acpi@vger.kernel.org>, Mario Limonciello <mario.limonciello@amd.com>, <stable@vger.kernel.org>, Iain Lane <iain@orangesquash.org.uk> Subject: [PATCH v6 1/1] PCI: Avoid putting some root ports into D3 on some Ryzen chips Date: Sat, 8 Jul 2023 16:44:57 -0500 Message-ID: <20230708214457.1229-2-mario.limonciello@amd.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230708214457.1229-1-mario.limonciello@amd.com> References: <20230708214457.1229-1-mario.limonciello@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB04.amd.com (10.181.40.145) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BN8NAM11FT030:EE_|DS0PR12MB7971:EE_ X-MS-Office365-Filtering-Correlation-Id: c662de27-856d-4358-3af4-08db80aec690 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: RslJRpSo0vjx1GOpY4NPDe6Lby1ra7Ux+kaTiZd2q0LVShtxoDkuCpxlrCycY5pGnkpUIThZfA9q/GLiMSoyMc68ooiRK2scMU9gZV+aJsSp95hvNZq46enXPCk8u+9IIBrtuvhQ8xdx9ylu8AkbUFRl+CAY59Jo6++Oa5P20nkENAmIzr3+lvTERHKK664SJgV//xU+vZ8S78wNxBR1zHamtnTh7lN/LhTD0tiP/IM46UBnH07dRAQr+91EXsk5+3Vv7SQQRhrpZroUJOfOROACQK0Snp3VPjjb9lDTNaNZ0DhZgUBWd8E5euqjQwi/yGMsdXTIg2ysfZKD9pPN/mJINXp+bBUXORvd+M8gpnGNxzu7C8AEoj2yBqXdaRCYJtUbwByzUS5Dij8q6mYuRbjpE/+MXFG9M3n65lhYSLg0ObvyzhCagzBu3Ke4jF8x7FXZMd0e+tT3+qA1+cg10kywAiapLl6sW3dQseqnLI0yAZ0QePUZXZYIIdBC+a1qQZ4fYShN46BZO+GjyhrE1+m5Et5YauU8Rxvl1VpH3YTSzrn11grczVQb+qGEwlVc9q6ozDSdgbDeMiY+AGvi+m0Fq7YgYsdfzDP7+wlEVwq5nR1dQD/0BSmx8q7ihO5bnq+v3gj+/YVvNc/XaEtQw4JwOD7Egv+/ytqRtXyGqEKvK1XYkuF+va86k4xNHJ0CxhkVVbeNJmTSabQArfRQei8HhZS97h1AFYU9O7N49xWkGVukgrtdnjHZHLg5P7Xa X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230028)(4636009)(396003)(136003)(346002)(376002)(39860400002)(451199021)(46966006)(36840700001)(40470700004)(40460700003)(4326008)(16526019)(186003)(26005)(1076003)(70206006)(70586007)(5660300002)(41300700001)(8676002)(8936002)(478600001)(110136005)(316002)(54906003)(82740400003)(86362001)(81166007)(356005)(44832011)(47076005)(36756003)(7696005)(36860700001)(966005)(6666004)(2906002)(40480700001)(336012)(2616005)(83380400001)(82310400005)(426003)(36900700001);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jul 2023 19:00:34.6594 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: c662de27-856d-4358-3af4-08db80aec690 X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: BN8NAM11FT030.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7971 X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_00,DATE_IN_PAST_12_24, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1770971363356063105 X-GMAIL-MSGID: 1770971363356063105 |
Series |
Fix suspend issues with AMD root ports
|
|
Commit Message
Mario Limonciello
July 8, 2023, 9:44 p.m. UTC
commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
sets the policy that all PCIe ports are allowed to use D3. When
the system is suspended if the port is not power manageable by the
platform and won't be used for wakeup via a PME this sets up the
policy for these ports to go into D3hot.
This policy generally makes sense from an OSPM perspective but it leads
to problems with wakeup from suspend on laptops with AMD chips:
- On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup
interrupt.
- On family 19h model 74h (PCI 0x14eb) this manifests as a system hang.
Add a quirk for the PCI device ID used by the problematic root port on
both chips to ensure that these root ports are not put into D3hot at
suspend.
Cc: stable@vger.kernel.org # 6.1+
Reported-by: Iain Lane <iain@orangesquash.org.uk>
Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121
Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/pci/quirks.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
Comments
On Sat, Jul 08, 2023 at 04:44:57PM -0500, Mario Limonciello wrote: > commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > sets the policy that all PCIe ports are allowed to use D3. When > the system is suspended if the port is not power manageable by the > platform and won't be used for wakeup via a PME this sets up the > policy for these ports to go into D3hot. > > This policy generally makes sense from an OSPM perspective but it leads > to problems with wakeup from suspend on laptops with AMD chips: > > - On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup > interrupt. > - On family 19h model 74h (PCI 0x14eb) this manifests as a system hang. > > Add a quirk for the PCI device ID used by the problematic root port on > both chips to ensure that these root ports are not put into D3hot at > suspend. What is problematic about these root ports? Is this a hardware erratum? Some corner of the ACPI spec that allows undefined behavior? Does AMD have any guidance about generic ways to use D3, or does AMD expect to add quirks piecemeal as problems are discovered? How does Windows handle all this? Adding quirks as we discover random devices that don't behave correctly for reasons unknown is not very sustainable. Bjorn > Cc: stable@vger.kernel.org # 6.1+ > Reported-by: Iain Lane <iain@orangesquash.org.uk> > Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121 > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/pci/quirks.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index 321156ca273d5..e0346073e5855 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -3867,6 +3867,22 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev) > DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL, > PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C, > quirk_apple_poweroff_thunderbolt); > + > +/* > + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot > + * may cause problems when the system attempts wake up from s2idle. > + * > + * On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup > + * interrupt. > + * On family 19h model 74h (PCI 0x14eb) this manifests as a system hang. > + */ > +static void quirk_ryzen_rp_d3(struct pci_dev *pdev) > +{ > + if (!acpi_pci_power_manageable(pdev)) > + pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3; > +} > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14b9, quirk_ryzen_rp_d3); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14eb, quirk_ryzen_rp_d3); > #endif > > /* > -- > 2.34.1 >
On 7/10/2023 14:32, Bjorn Helgaas wrote: > On Sat, Jul 08, 2023 at 04:44:57PM -0500, Mario Limonciello wrote: >> commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") >> sets the policy that all PCIe ports are allowed to use D3. When >> the system is suspended if the port is not power manageable by the >> platform and won't be used for wakeup via a PME this sets up the >> policy for these ports to go into D3hot. >> >> This policy generally makes sense from an OSPM perspective but it leads >> to problems with wakeup from suspend on laptops with AMD chips: >> >> - On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup >> interrupt. >> - On family 19h model 74h (PCI 0x14eb) this manifests as a system hang. >> >> Add a quirk for the PCI device ID used by the problematic root port on >> both chips to ensure that these root ports are not put into D3hot at >> suspend. > > What is problematic about these root ports? Is this a hardware > erratum? I mentioned some of this in earlier versions; but the problem is deeper in the platform firmware and only happens when the root ports are in D3hot across an s2idle cycle. When looked at independently they can be moved in and out of D3hot no problem. From the perspective of hardware designers they say this is a software bug that Linux puts PCI RP into D3hot over Modern Standby when they don't specify for this to be done. I don't expect any erratum for it. > Some corner of the ACPI spec that allows undefined behavior? > These root ports are not reported as power manageable by ACPI. As I mentioned in the cover letter how the OSPM handles this case is outside of any spec AFAICT. > Does AMD have any guidance about generic ways to use D3, or does AMD > expect to add quirks piecemeal as problems are discovered? How does > Windows handle all this? > Windows doesn't put root ports into D3hot over suspend unless they are power managed by ACPI (as described in section 7.3 of the ACPI spec). https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/07_Power_and_Performance_Mgmt/device-power-management-objects.html So on Windows these ports are all in D0 and none of the issues happen. Linux if PCI devices aren't power managed by ACPI will either follow deepest state it can wake from PME or fall back to D3hot. > Adding quirks as we discover random devices that don't behave > correctly for reasons unknown is not very sustainable. I don't disagree but in v5 I tried to align this to the Windows behavior and it wasn't accepted. If you still think v5 is the better approach I'm fine to try to do that again and rewrite the commit message. > > Bjorn > >> Cc: stable@vger.kernel.org # 6.1+ >> Reported-by: Iain Lane <iain@orangesquash.org.uk> >> Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121 >> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> drivers/pci/quirks.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >> index 321156ca273d5..e0346073e5855 100644 >> --- a/drivers/pci/quirks.c >> +++ b/drivers/pci/quirks.c >> @@ -3867,6 +3867,22 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev) >> DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL, >> PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C, >> quirk_apple_poweroff_thunderbolt); >> + >> +/* >> + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot >> + * may cause problems when the system attempts wake up from s2idle. >> + * >> + * On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup >> + * interrupt. >> + * On family 19h model 74h (PCI 0x14eb) this manifests as a system hang. >> + */ >> +static void quirk_ryzen_rp_d3(struct pci_dev *pdev) >> +{ >> + if (!acpi_pci_power_manageable(pdev)) >> + pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3; >> +} >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14b9, quirk_ryzen_rp_d3); >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14eb, quirk_ryzen_rp_d3); >> #endif >> >> /* >> -- >> 2.34.1 >>
[+cc Mika, author of 9d26d3a8f1b0] On Mon, Jul 10, 2023 at 02:44:23PM -0500, Limonciello, Mario wrote: > On 7/10/2023 14:32, Bjorn Helgaas wrote: > > On Sat, Jul 08, 2023 at 04:44:57PM -0500, Mario Limonciello wrote: > > > commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > > > sets the policy that all PCIe ports are allowed to use D3. When > > > the system is suspended if the port is not power manageable by the > > > platform and won't be used for wakeup via a PME this sets up the > > > policy for these ports to go into D3hot. > > > > > > This policy generally makes sense from an OSPM perspective but it leads > > > to problems with wakeup from suspend on laptops with AMD chips: > > > > > > - On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup > > > interrupt. > > > - On family 19h model 74h (PCI 0x14eb) this manifests as a system hang. > > > > > > Add a quirk for the PCI device ID used by the problematic root port on > > > both chips to ensure that these root ports are not put into D3hot at > > > suspend. > > > > What is problematic about these root ports? Is this a hardware > > erratum? > > I mentioned some of this in earlier versions; but the problem is deeper > in the platform firmware and only happens when the root ports are in D3hot > across an s2idle cycle. > > When looked at independently they can be moved in and out of D3hot no > problem. > > From the perspective of hardware designers they say this is a software bug > that Linux puts PCI RP into D3hot over Modern Standby when they don't > specify for this to be done. I don't expect any erratum for it. It sounds like there's someplace the hardware designers specify how this should work? Where is that? "Modern Standby" doesn't mean anything to me, but maybe there's some spec that covers it? > > Some corner of the ACPI spec that allows undefined behavior? > > These root ports are not reported as power manageable by ACPI. > > As I mentioned in the cover letter how the OSPM handles this case is outside > of any spec AFAICT. If it's truly outside the spec, then I don't think Linux should use D3 until we have some standardized way to tell whether it's safe. > > Does AMD have any guidance about generic ways to use D3, or does AMD > > expect to add quirks piecemeal as problems are discovered? How does > > Windows handle all this? > > Windows doesn't put root ports into D3hot over suspend unless they are power > managed by ACPI (as described in section 7.3 of the ACPI spec). > > https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/07_Power_and_Performance_Mgmt/device-power-management-objects.html > > So on Windows these ports are all in D0 and none of the issues happen. Maybe this is the clue we need. My eyes glaze over when reading that section, but if we can come up with a commit log that starts with a sentence from that section and connects the dots all the way to the platform_pci_power_manageable() implementation, that might be a good argument that 9d26d3a8f1b0 was a little too aggressive. > Linux if PCI devices aren't power managed by ACPI will either follow deepest > state it can wake from PME or fall back to D3hot. > > > Adding quirks as we discover random devices that don't behave > > correctly for reasons unknown is not very sustainable. > > I don't disagree but in v5 I tried to align this to the Windows behavior and > it wasn't accepted. I like the fact that v5 ([1] for anybody following along at home) is very generic: @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) ... + if (pci_pcie_type(bridge) == PCI_EXP_TYPE_ROOT_PORT && + !platform_pci_power_manageable(bridge)) + return false; My objection was that we didn't have a clear connection to any specs, so even though the code is perfectly generic, the commit log mentioned specific cases (USB keyboard/mouse on xHCI device connected to USB-C on AMD USB4 router). But maybe we *could* make a convincing generic commit log. I guess we'd also have to explain the PCI_EXP_TYPE_ROOT_PORT part of the patch. Bjorn [1] https://lore.kernel.org/r/20230530163947.230418-2-mario.limonciello@amd.com > > > Cc: stable@vger.kernel.org # 6.1+ > > > Reported-by: Iain Lane <iain@orangesquash.org.uk> > > > Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121 > > > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > > > --- > > > drivers/pci/quirks.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index 321156ca273d5..e0346073e5855 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -3867,6 +3867,22 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev) > > > DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL, > > > PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C, > > > quirk_apple_poweroff_thunderbolt); > > > + > > > +/* > > > + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot > > > + * may cause problems when the system attempts wake up from s2idle. > > > + * > > > + * On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup > > > + * interrupt. > > > + * On family 19h model 74h (PCI 0x14eb) this manifests as a system hang. > > > + */ > > > +static void quirk_ryzen_rp_d3(struct pci_dev *pdev) > > > +{ > > > + if (!acpi_pci_power_manageable(pdev)) > > > + pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3; > > > +} > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14b9, quirk_ryzen_rp_d3); > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14eb, quirk_ryzen_rp_d3); > > > #endif > > > /* > > > -- > > > 2.34.1 > > > >
On 7/10/2023 3:33 PM, Bjorn Helgaas wrote: > > It sounds like there's someplace the hardware designers specify how > this should work? Where is that? "Modern Standby" doesn't mean > anything to me, but maybe there's some spec that covers it? https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby It quickly devolves into Microsoft specific stuff though and I can't find anything interesting to our specific issue. > Maybe this is the clue we need. My eyes glaze over when reading that > section, but if we can come up with a commit log that starts with a > sentence from that section and connects the dots all the way to the > platform_pci_power_manageable() implementation, that might be a good > argument that 9d26d3a8f1b0 was a little too aggressive. Yeah. > I like the fact that v5 ([1] for anybody following along at home) is > very generic: > > @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > ... > + if (pci_pcie_type(bridge) == PCI_EXP_TYPE_ROOT_PORT && > + !platform_pci_power_manageable(bridge)) > + return false; > > My objection was that we didn't have a clear connection to any specs, > so even though the code is perfectly generic, the commit log mentioned > specific cases (USB keyboard/mouse on xHCI device connected to USB-C > on AMD USB4 router). > > But maybe we *could* make a convincing generic commit log. I guess > we'd also have to explain the PCI_EXP_TYPE_ROOT_PORT part of the > patch. OK, I'll take a stab at rewriting the v5 commit message to be more generic as you suggested as a v7. We might be able to drop the PCI_EXP_TYPE_ROOT_PORT part well but I would be more worried about regressions from this.
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 321156ca273d5..e0346073e5855 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -3867,6 +3867,22 @@ static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev) DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C, quirk_apple_poweroff_thunderbolt); + +/* + * Putting PCIe root ports on Ryzen SoCs with USB4 controllers into D3hot + * may cause problems when the system attempts wake up from s2idle. + * + * On family 19h model 44h (PCI 0x14b9) this manifests as a missing wakeup + * interrupt. + * On family 19h model 74h (PCI 0x14eb) this manifests as a system hang. + */ +static void quirk_ryzen_rp_d3(struct pci_dev *pdev) +{ + if (!acpi_pci_power_manageable(pdev)) + pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3; +} +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14b9, quirk_ryzen_rp_d3); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, 0x14eb, quirk_ryzen_rp_d3); #endif /*