Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: James Ettle <james@ettle.org.uk>
Cc: "吳昊澄 Ricky" <ricky_wu@realtek.com>,
	"Rui Feng" <rui_feng@realsil.com.cn>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Len Brown" <lenb@kernel.org>,
	"Puranjay Mohan" <puranjay12@gmail.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Jacopo De Simoi" <wilderkde@gmail.com>
Subject: Re: rtsx_pci not restoring ASPM state after suspend/resume
Date: Tue, 28 Jul 2020 18:06:03 -0500
Message-ID: <20200728230603.GA1870954@bjorn-Precision-5520> (raw)
In-Reply-To: <e051ac790380f04be4eec6937032b7dcd411ec77.camel@ettle.org.uk>

On Tue, Jul 28, 2020 at 09:57:55PM +0100, James Ettle wrote:
> On Mon, 2020-07-27 at 16:47 -0500, Bjorn Helgaas wrote:
> > 
> > I don't see anything in rtsx that enables L0s.  Can you collect
> > the dmesg log when booting with "pci=earlydump"?  That will show
> > whether the BIOS left it this way.  The PCI core isn't supposed to
> > do this, so if it did, we need to fix that.
> 
> dmesg log attached to the bugzilla as:
> 
> https://bugzilla.kernel.org/attachment.cgi?id=290655

Thank you!  BIOS left the Root Port and both Endpoints with L1 enabled
but L0s disabled.

Next question is how L0s got enabled.  We don't see anything in the
driver that does that.  And lspci claims L0s was enabled on the
Endpoints even without any udev or other manual fiddling.

I tried to deduce the problem from the code in aspm.c, but I didn't
see the problem.  If you have the ability to build a kernel with a
debug patch, can you boot with the patch below and collect the dmesg
log?

FWIW, here's the analysis of the earlydump output that shows L0s
disabled:

  #define PCI_CAPABILITY_LIST     0x34
  #define  PCI_CAP_ID_MSI         0x05
  #define  PCI_CAP_ID_EXP         0x10    /* PCI Express */
  #define PCI_EXP_LNKCTL          16
  #define  PCI_EXP_LNKCTL_ASPM_L0S 0x0001
  #define  PCI_EXP_LNKCTL_ASPM_L1  0x0002
  #define  PCI_EXP_LNKCTL_CCC     0x0040
  #define  PCI_EXP_LNKCTL_CLKREQ_EN 0x0100

  00:1d.0:
    0034: 40        Offset of first capability
    0040: 8010      PCIe capability ID, next cap at 0x80
    0050: 0042      PCIe Link Control: CCC L1
  01:00.0:
    0034: 40        Offset of first capability
    0040: 7005      MSI capability ID (0x05), next cap at 0x70
    0070: b010      PCIe capability ID (0x10), next cap at 0xb0
    0080: 0142      PCIe Link Control: CLKREQ_EN CCC L1
  01:00.1:
    0034: 40        Offset of first capability
    0040: 7005      MSI capability ID (0x05), next cap at 0x70
    0070: b010      PCIe capability ID (0x10), next cap at 0xb0
    0080: 0142      PCIe Link Control: CLKREQ_EN CCC L1


commit 555db6d25963 ("debug")
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Tue Jul 28 17:52:58 2020 -0500

    debug
    

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index b17e5ffd31b1..262f35883a2a 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -577,6 +577,11 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	pcie_get_aspm_reg(parent, &upreg);
 	pcie_get_aspm_reg(child, &dwreg);
 
+	pci_info(parent, "%s support %x enabled %x\n", __func__, upreg.support,
+		 upreg.enabled);
+	pci_info(child, "%s support %x enabled %x\n", __func__, dwreg.support,
+		 dwreg.enabled);
+
 	/*
 	 * Setup L0s state
 	 *
@@ -629,6 +634,10 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	/* Setup initial capable state. Will be updated later */
 	link->aspm_capable = link->aspm_support;
 
+	pci_info(parent, "%s link support %x enabled %x capable %x default %x disable %x\n",
+		 __func__, link->aspm_support, link->aspm_enabled,
+		 link->aspm_capable, link->aspm_default, link->aspm_disable);
+
 	/* Get and check endpoint acceptable latencies */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
 		u32 reg32, encoding;
@@ -744,6 +753,7 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
 
 static void pcie_config_aspm_dev(struct pci_dev *pdev, u32 val)
 {
+	pci_info(pdev, "%s set state %x\n", __func__, val);
 	pcie_capability_clear_and_set_word(pdev, PCI_EXP_LNKCTL,
 					   PCI_EXP_LNKCTL_ASPMC, val);
 }
@@ -754,6 +764,9 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
 	struct pci_bus *linkbus = parent->subordinate;
 
+	pci_info(parent, "%s requesting state %x (%s)\n", __func__,
+		 state, policy_str[state]);
+
 	/* Enable only the states that were not explicitly disabled */
 	state &= (link->aspm_capable & ~link->aspm_disable);
 
@@ -767,6 +780,10 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 		state |= (link->aspm_enabled & ASPM_STATE_L1_SS_PCIPM);
 	}
 
+	pci_info(parent, "%s link support %x enabled %x capable %x default %x disable %x\n",
+		 __func__, link->aspm_support, link->aspm_enabled,
+		 link->aspm_capable, link->aspm_default, link->aspm_disable);
+
 	/* Nothing to do if the link is already in the requested state */
 	if (link->aspm_enabled == state)
 		return;

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23 16:56 Bjorn Helgaas
2020-07-23 17:12 ` Bjorn Helgaas
2020-07-24  7:16   ` 吳昊澄 Ricky
2020-07-24 23:13     ` Bjorn Helgaas
2020-07-25 20:27       ` James Ettle
2020-07-27 14:14         ` Bjorn Helgaas
2020-07-27 19:52           ` James Ettle
2020-07-27 21:47             ` Bjorn Helgaas
2020-07-28  5:46               ` 吳昊澄 Ricky
2020-07-28 20:57               ` James Ettle
2020-07-28 23:06                 ` Bjorn Helgaas [this message]
2020-08-02 16:30                   ` James Ettle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200728230603.GA1870954@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=james@ettle.org.uk \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=puranjay12@gmail.com \
    --cc=ricky_wu@realtek.com \
    --cc=rui_feng@realsil.com.cn \
    --cc=wilderkde@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git