[PATCH] accel/amdxdna: Keep PCI power state in sync with runtime PM

reza.jelveh@gmail.com posted 1 patch an hour ago
drivers/accel/amdxdna/aie2_pci.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
[PATCH] accel/amdxdna: Keep PCI power state in sync with runtime PM
Posted by reza.jelveh@gmail.com an hour ago
From: Reza Jelveh <reza.jelveh+git@gmail.com>

The platform powers the NPU down as soon as the driver stops its
firmware, but aie2_hw_stop() never recorded that transition. The PCI
core kept tracking the device as D0, so on the next system suspend the
noirq phase attempted a D0-to-D3hot transition on a device whose config
space returns 0xffffffff:

  Unable to change power state from D0 to D3hot, device inaccessible

The platform's power state was then out of sync with the kernel's, the
SMU idle condition was never met, and suspend-to-idle did not reach the
deepest state. On resume, aie2_hw_start() re-enabled the device without
requesting D0 first and failed reading the SMU mailbox (0xffffffff,
-EINVAL), leaving the NPU dead until reboot.

Record D3hot at the end of aie2_hw_stop(), while the device is still
reachable, and request D0 at the beginning of aie2_hw_start(), so the
kernel's PCI power state always matches the platform state. Skip the
firmware stop in aie2_hw_suspend() when runtime PM has already stopped
the device, and demote the "device is already stopped" message to debug
level.

Signed-off-by: Reza Jelveh <reza.jelveh+git@gmail.com>
---
 drivers/accel/amdxdna/aie2_pci.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index a7b923005..ad14e4fc6 100644
--- a/drivers/accel/amdxdna/aie2_pci.c
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -288,7 +288,7 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna)
 	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
 
 	if (ndev->dev_status <= AIE2_DEV_INIT) {
-		XDNA_ERR(xdna, "device is already stopped");
+		XDNA_DBG(xdna, "device is already stopped");
 		return;
 	}
 
@@ -301,6 +301,12 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna)
 	aie2_smu_fini(ndev);
 	aie2_error_async_events_free(ndev);
 	pci_disable_device(pdev);
+	/*
+	 * The platform powers the NPU down once the firmware is stopped.
+	 * Record D3hot while the device is still reachable, so the noirq
+	 * suspend path does not attempt a stale D0 transition.
+	 */
+	pci_set_power_state(pdev, PCI_D3hot);
 
 	ndev->dev_status = AIE2_DEV_INIT;
 }
@@ -318,6 +324,13 @@ static int aie2_hw_start(struct amdxdna_dev *xdna)
 		return 0;
 	}
 
+	/* The platform powers the device down when the firmware is stopped. */
+	ret = pci_set_power_state(pdev, PCI_D0);
+	if (ret) {
+		XDNA_ERR(xdna, "failed to power up device, ret %d", ret);
+		return ret;
+	}
+
 	ret = pci_enable_device(pdev);
 	if (ret) {
 		XDNA_ERR(xdna, "failed to enable device, ret %d", ret);
@@ -427,12 +440,15 @@ static int aie2_hw_start(struct amdxdna_dev *xdna)
 
 static int aie2_hw_suspend(struct amdxdna_dev *xdna)
 {
+	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
 	struct amdxdna_client *client;
 
 	list_for_each_entry(client, &xdna->client_list, node)
 		aie2_hwctx_suspend(client);
 
-	aie2_hw_stop(xdna);
+	/* Runtime PM may already have stopped the device. */
+	if (ndev->dev_status > AIE2_DEV_INIT)
+		aie2_hw_stop(xdna);
 
 	return 0;
 }
-- 
2.55.0