Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: "victording via sendgmr" <victording@victording.c.googlers.com>
Date:   Tue, 12 Jan 2021 04:02:05 +0000
In-Reply-To: <20210112040205.4117303-1-victording@google.com>
Message-Id: <20210112040146.2.Ic902bbd9f04e2d82ac578411e7fafc77b6c750e2@changeid>
Mime-Version: 1.0
References: <20210112040205.4117303-1-victording@google.com>
Subject: [PATCH 2/2] mmc: sdhci-pci-gli: Disable ASPM during a suspension
From:   Victor Ding <victording@google.com>
To:     Ulf Hansson <ulf.hansson@linaro.org>,
        Adrian Hunter <adrian.hunter@intel.com>
Cc:     Ben Chuang <ben.chuang@genesyslogic.com.tw>,
        Bjorn Helgaas <helgaas@kernel.org>,
        linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
        linux-mmc@vger.kernel.org, Victor Ding <victording@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

GL9750 has a 3100us PortTPowerOnTime; however, it enters L1.2 after
only ~4us inactivity per PCIe trace. During a suspend/resume process,
PCI access operations are frequently longer than 4us apart.
Therefore, the device frequently enters and leaves L1.2 during this
process, causing longer than desirable suspend/resume time. The total
time cost due to this L1.2 exit latency could add up to ~200ms.

Considering that PCI access operations are fairly close to each other
(though sometimes > 4us), the actual time the device could stay in
L1.2 is negligible. Therefore, the little power-saving benefit from
ASPM during suspend/resume does not overweight the performance
degradation caused by long L1.2 exit latency.

Therefore, this patch proposes to disable ASPM during a suspend/resume
process.

Signed-off-by: Victor Ding <victording@google.com>
---

 drivers/mmc/host/sdhci-pci-core.c |  2 +-
 drivers/mmc/host/sdhci-pci-gli.c  | 46 +++++++++++++++++++++++++++++--
 drivers/mmc/host/sdhci-pci.h      |  1 +
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 9552708846ca..fd7544a498c0 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -67,7 +67,7 @@ static int sdhci_pci_init_wakeup(struct sdhci_pci_chip *chip)
 	return 0;
 }
 
-static int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
+int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip)
 {
 	int i, ret;
 
diff --git a/drivers/mmc/host/sdhci-pci-gli.c b/drivers/mmc/host/sdhci-pci-gli.c
index 9887485a4134..c7b788b0e22e 100644
--- a/drivers/mmc/host/sdhci-pci-gli.c
+++ b/drivers/mmc/host/sdhci-pci-gli.c
@@ -109,6 +109,12 @@
 
 #define GLI_MAX_TUNING_LOOP 40
 
+#ifdef CONFIG_PM_SLEEP
+struct gli_host {
+	u16 linkctl_saved;
+};
+#endif
+
 /* Genesys Logic chipset */
 static inline void gl9750_wt_on(struct sdhci_host *host)
 {
@@ -577,14 +583,48 @@ static u32 sdhci_gl9750_readl(struct sdhci_host *host, int reg)
 }
 
 #ifdef CONFIG_PM_SLEEP
+static int sdhci_pci_gli_suspend(struct sdhci_pci_chip *chip)
+{
+	int ret;
+	struct sdhci_pci_slot *slot = chip->slots[0];
+	struct pci_dev *pdev = slot->chip->pdev;
+	struct gli_host *gli_host = sdhci_pci_priv(slot);
+
+	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKCTL,
+			&gli_host->linkctl_saved);
+	if (ret)
+		goto exit;
+
+	ret = pcie_capability_write_word(pdev, PCI_EXP_LNKCTL,
+			gli_host->linkctl_saved & ~PCI_EXP_LNKCTL_ASPMC);
+	if (ret)
+		goto exit;
+
+	ret = sdhci_pci_suspend_host(chip);
+
+exit:
+	return ret;
+}
+
 static int sdhci_pci_gli_resume(struct sdhci_pci_chip *chip)
 {
+	int ret;
 	struct sdhci_pci_slot *slot = chip->slots[0];
+	struct pci_dev *pdev = slot->chip->pdev;
+	struct gli_host *gli_host = sdhci_pci_priv(slot);
 
-	pci_free_irq_vectors(slot->chip->pdev);
+	pci_free_irq_vectors(pdev);
 	gli_pcie_enable_msi(slot);
 
-	return sdhci_pci_resume_host(chip);
+	ret = sdhci_pci_resume_host(chip);
+	if (ret)
+		goto exit;
+
+	ret = pcie_capability_clear_and_set_word(pdev, PCI_EXP_LNKCTL,
+			PCI_EXP_LNKCTL_ASPMC, gli_host->linkctl_saved);
+
+exit:
+	return ret;
 }
 
 static int sdhci_cqhci_gli_resume(struct sdhci_pci_chip *chip)
@@ -834,7 +874,9 @@ const struct sdhci_pci_fixes sdhci_gl9750 = {
 	.probe_slot	= gli_probe_slot_gl9750,
 	.ops            = &sdhci_gl9750_ops,
 #ifdef CONFIG_PM_SLEEP
+	.suspend        = sdhci_pci_gli_suspend,
 	.resume         = sdhci_pci_gli_resume,
+	.priv_size      = sizeof(struct gli_host),
 #endif
 };
 
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index d0ed232af0eb..16187a265e63 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -187,6 +187,7 @@ static inline void *sdhci_pci_priv(struct sdhci_pci_slot *slot)
 }
 
 #ifdef CONFIG_PM_SLEEP
+int sdhci_pci_suspend_host(struct sdhci_pci_chip *chip);
 int sdhci_pci_resume_host(struct sdhci_pci_chip *chip);
 #endif
 int sdhci_pci_enable_dma(struct sdhci_host *host);
-- 
2.30.0.284.gd98b1dd5eaa7-goog