Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   Rajvi Jingar <rajvi.jingar@linux.intel.com>
To:     rafael.j.wysocki@intel.com, bhelgaas@google.com
Cc:     rajvi.jingar@linux.intel.com, david.e.box@linux.intel.com,
        linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org
Subject: [PATCH v6 2/2] PCI/PM: disable PTM on all devices
Date:   Tue,  7 Jun 2022 14:08:52 -0700
Message-Id: <20220607210852.475863-2-rajvi.jingar@linux.intel.com>
In-Reply-To: <20220607210852.475863-1-rajvi.jingar@linux.intel.com>
References: <20220607210852.475863-1-rajvi.jingar@linux.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On receiving a PTM Request from a downstream device, if PTM is disabled
on the root port, as per PCIe specification, such request would cause
an Unsupported Request error. So disable PTM for any downstream devices.
PTM state needs to be saved before disabling it to be restored later.

set ptm_enabled from 'struct pci_dev' to 0 in pci_ptm_disable() and
it is used in pci_save_state() before saving PTM state to avoid
double save.

Fixes: a697f072f5da ("PCI: Disable PTM during suspend to save power")
Signed-off-by: Rajvi Jingar <rajvi.jingar@linux.intel.com>
Suggested-by: David E. Box <david.e.box@linux.intel.com>
---
 v1 -> v2: add Fixes tag in commit message
 v2 -> v3: move changelog after "---" marker
 v3 -> v4: add "---" marker after changelog
 v4 -> v5: move pci_disable_ptm() out of the pci_dev->state_saved check.
	   disable PTM for all devices, not just root ports.
 v5 -> v6: move pci_disable_ptm() to pci_pm_suspend()
	   set pci_dev->ptm_enabled to 0 in pci_ptm_disable() and it is
	   used in pci_save_state() before saving PTM state to avoid
	   double save.
---
 drivers/pci/pci-driver.c | 21 ++++++++++++++++++++-
 drivers/pci/pci.c        | 26 +++++++++++---------------
 drivers/pci/pcie/ptm.c   |  1 +
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 1f64de3e5280..db4d7835d7ae 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -803,14 +803,33 @@ static int pci_pm_suspend(struct device *dev)
 		pci_dev_adjust_pme(pci_dev);
 	}
 
+	/*
+	 * If a PTM Requester is put in a low-power state, a PTM Responder
+	 * upstream from it may also be put in a low-power state. Putting a
+	 * Port in D1, D2, or D3hot does not prohibit it from sending or
+	 * responding to PTM Requests. We want to disable PTM on Responders
+	 * when they are in a low-power state. Per 6.21.3, a PTM Requester
+	 * must not be enabled when the upstream PTM Responder is disabled.
+	 * Therefore, we must disable all PTM on all downstream PTM
+	 * Requesters before disabling it on the PTM Responder, e.g., a Root
+	 * Port.
+	 *
+	 * Also, to restore the PTM state, it needs to be saved before
+	 * disabling it for all devices.
+	 */
+	pci_save_ptm_state(pci_dev);
+	pci_disable_ptm(pci_dev);
+
 	if (pm->suspend) {
 		pci_power_t prev = pci_dev->current_state;
 		int error;
 
 		error = pm->suspend(dev);
 		suspend_report_result(dev, pm->suspend, error);
-		if (error)
+		if (error) {
+			pci_restore_ptm_state(pci_dev);
 			return error;
+		}
 
 		if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
 		    && pci_dev->current_state != PCI_UNKNOWN) {
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index cfaf40a540a8..0df9b783621e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1669,7 +1669,13 @@ int pci_save_state(struct pci_dev *dev)
 	pci_save_ltr_state(dev);
 	pci_save_dpc_state(dev);
 	pci_save_aer_state(dev);
-	pci_save_ptm_state(dev);
+	/*
+	 * PCI PM core disables PTM during suspend and saves PTM state before
+	 * that to be able to restore the ptm state restored later. So PCI core
+	 * needs this check to avoid double save.
+	 */
+	if (dev->ptm_enabled)
+		pci_save_ptm_state(dev);
 	return pci_save_vc_state(dev);
 }
 EXPORT_SYMBOL(pci_save_state);
@@ -2710,24 +2716,12 @@ int pci_prepare_to_sleep(struct pci_dev *dev)
 	if (target_state == PCI_POWER_ERROR)
 		return -EIO;
 
-	/*
-	 * There are systems (for example, Intel mobile chips since Coffee
-	 * Lake) where the power drawn while suspended can be significantly
-	 * reduced by disabling PTM on PCIe root ports as this allows the
-	 * port to enter a lower-power PM state and the SoC to reach a
-	 * lower-power idle state as a whole.
-	 */
-	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
-		pci_disable_ptm(dev);
-
 	pci_enable_wake(dev, target_state, wakeup);
 
 	error = pci_set_power_state(dev, target_state);
 
-	if (error) {
+	if (error)
 		pci_enable_wake(dev, target_state, false);
-		pci_restore_ptm_state(dev);
-	}
 
 	return error;
 }
@@ -2775,8 +2769,10 @@ int pci_finish_runtime_suspend(struct pci_dev *dev)
 	 * port to enter a lower-power PM state and the SoC to reach a
 	 * lower-power idle state as a whole.
 	 */
-	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT)
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
+		pci_save_ptm_state(dev);
 		pci_disable_ptm(dev);
+	}
 
 	__pci_enable_wake(dev, target_state, pci_dev_run_wake(dev));
 
diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 368a254e3124..746e29779c27 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -44,6 +44,7 @@ void pci_disable_ptm(struct pci_dev *dev)
 	pci_read_config_word(dev, ptm + PCI_PTM_CTRL, &ctrl);
 	ctrl &= ~(PCI_PTM_CTRL_ENABLE | PCI_PTM_CTRL_ROOT);
 	pci_write_config_word(dev, ptm + PCI_PTM_CTRL, ctrl);
+	dev->ptm_enabled = 0;
 }
 
 void pci_save_ptm_state(struct pci_dev *dev)
-- 
2.25.1