From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A2B8DC5DF62 for ; Wed, 6 Nov 2019 13:29:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7A81A214D8 for ; Wed, 6 Nov 2019 13:29:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731892AbfKFN3N (ORCPT ); Wed, 6 Nov 2019 08:29:13 -0500 Received: from mga03.intel.com ([134.134.136.65]:35175 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731876AbfKFN3L (ORCPT ); Wed, 6 Nov 2019 08:29:11 -0500 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Nov 2019 05:29:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,274,1569308400"; d="scan'208";a="212773427" Received: from lahna.fi.intel.com (HELO lahna) ([10.237.72.163]) by fmsmga001.fm.intel.com with SMTP; 06 Nov 2019 05:29:05 -0800 Received: by lahna (sSMTP sendmail emulation); Wed, 06 Nov 2019 15:29:04 +0200 Date: Wed, 6 Nov 2019 15:29:04 +0200 From: Mika Westerberg To: Bjorn Helgaas Cc: "Rafael J. Wysocki" , Len Brown , Lukas Wunner , Keith Busch , Alex Williamson , Alexandru Gagniuc , Kai-Heng Feng , Paul Menzel , Nicholas Johnson , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/2] PCI: Add missing link delays required by the PCIe spec Message-ID: <20191106132904.GL2552@lahna.fi.intel.com> References: <20191105152832.GC2552@lahna.fi.intel.com> <20191105161017.GA219591@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191105161017.GA219591@google.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.12.1 (2019-06-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 05, 2019 at 10:10:17AM -0600, Bjorn Helgaas wrote: > > > I actually suspect there *is* a dependency -- we should respect the > > > Target Link Speed and and width so the link resumes in the same > > > configuration it was before suspend. And I suspect that may require > > > an explicit retrain after restoring PCI_EXP_LNKCTL2. > > > > According the PCIe spec the PCI_EXP_LNKCTL2 Target Link Speed is marked > > as RWS (S for sticky) so I suspect its value is retained after reset in > > the same way as PME bits. Assuming I understood it correctly. > > This patch is about coming from D3cold, isn't it? I don't think we > can assume sticky bits are preserved in D3cold (except maybe when > auxiliary power is enabled). Indeed, good point. I see some GPU drivers are programming Target Link Speed which will not be retained after the hierarchy is put into D3cold and back. I think this potential problem is not related to the missing link delays this patch is addressing, though. It has been existing in pci_restore_pcie_state() already (where it restores PCI_EXP_LNKCTL2). I think this can be solved as a separate patch by doing something like: 1. In pci_restore_pcie_state() check if the saved Target Link Speed differs from what is in the register currently. 2. Restore the value as we already do now. 3. If there the speed differs then trigger link retrain. 4. Restore rest of the root/downstream port state. It is not clear if we need to do anything for upstream ports (PCIe 5.0 sec 6.11 talks about doing this on upstream component e.g downstream port). After this there will be the link delay (added by this patch) which takes care of waiting for the downstream component to be accessible (even after retrain). However, I'm not sure how this can be properly tested. Maybe hacking some downstream port to lower the speed, enter D3cold and then resume it and see if the Target Link Speed gets updated correctly.