On 10/14/26 11:36, Lizhi Hou wrote: > > On 10/9/26 15:53, David Zhang wrote: >> Add runtime suspend/resume for the AIE4 classic device by using its >> system suspend/resume handlers. >> >> Runtime suspend returns -EBUSY for a device without runtime_suspend, >> so the PM core keeps it active instead of entering an error state. >> >> A context suspended by runtime PM makes the command wait return >> -EAGAIN. Resume the device in the wait ioctl and wait again, so a >> command wait does not fail because of power management. >> >> Wrap .driver.pm and the AIE4 PM callbacks with pm_ptr(), so they are >> discarded when CONFIG_PM is disabled. >> >> Signed-off-by: David Zhang <[email protected]> >> --- >> drivers/accel/amdxdna/aie2_pci.c | 2 ++ >> drivers/accel/amdxdna/aie4_ctx.c | 10 +++++- >> drivers/accel/amdxdna/aie4_pci.c | 4 +++ >> drivers/accel/amdxdna/amdxdna_ctx.c | 47 ++++++++++++++++++------- >> drivers/accel/amdxdna/amdxdna_pci_drv.c | 4 +-- >> drivers/accel/amdxdna/amdxdna_pci_drv.h | 2 ++ >> drivers/accel/amdxdna/amdxdna_pm.c | 34 ++++++++++++++++++ >> drivers/accel/amdxdna/amdxdna_pm.h | 2 ++ >> 8 files changed, 90 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/accel/amdxdna/aie2_pci.c >> b/drivers/accel/amdxdna/aie2_pci.c >> index 690ad035463c..a3866e8933e6 100644 >> --- a/drivers/accel/amdxdna/aie2_pci.c >> +++ b/drivers/accel/amdxdna/aie2_pci.c >> @@ -1221,6 +1221,8 @@ const struct amdxdna_dev_ops aie2_ops = { >> .fini = aie2_fini, >> .resume = aie2_hw_resume, >> .suspend = aie2_hw_suspend, >> + .runtime_resume = aie2_hw_resume, >> + .runtime_suspend = aie2_hw_suspend, >> .get_aie_info = aie2_get_info, >> .set_aie_state = aie2_set_state, >> .hwctx_init = aie2_hwctx_init, >> diff --git a/drivers/accel/amdxdna/aie4_ctx.c >> b/drivers/accel/amdxdna/aie4_ctx.c >> index a2b6b0fa2934..a3f0a69b93b7 100644 >> --- a/drivers/accel/amdxdna/aie4_ctx.c >> +++ b/drivers/accel/amdxdna/aie4_ctx.c >> @@ -22,6 +22,7 @@ >> #include "amdxdna_mailbox.h" >> #include "amdxdna_mailbox_helper.h" >> #include "amdxdna_pci_drv.h" >> +#include "amdxdna_pm.h" >> #define CTX_INVALID_ID (~0U) >> #define CTX_INVALID_DOORBELL AMDXDNA_INVALID_DOORBELL_OFFSET >> @@ -377,10 +378,14 @@ int aie4_hwctx_init(struct amdxdna_hwctx *hwctx) >> if (ret) >> goto destroy_lock; >> - ret = aie4_hwctx_create(hwctx); >> + ret = amdxdna_pm_resume_get_locked(xdna); >> if (ret) >> goto umq_fini; >> + ret = aie4_hwctx_create(hwctx); >> + if (ret) >> + goto suspend_put; >> + >> priv->job_work_q = alloc_ordered_workqueue("aie4_job_%d_%d", 0, >> client->pid, hwctx->fw_ctx_id); >> if (!priv->job_work_q) { >> @@ -388,12 +393,15 @@ int aie4_hwctx_init(struct amdxdna_hwctx *hwctx) >> ret = -ENOMEM; >> goto destroy_ctx; >> } >> + amdxdna_pm_suspend_put(xdna); >> XDNA_DBG(xdna, "hwctx %d.%d init completed", client->pid, >> hwctx->fw_ctx_id); >> return 0; >> destroy_ctx: >> aie4_hwctx_destroy(hwctx, AIE4_HWCTX_NORMAL); >> +suspend_put: >> + amdxdna_pm_suspend_put(xdna); >> umq_fini: >> aie4_hwctx_umq_fini(hwctx); >> destroy_lock: >> diff --git a/drivers/accel/amdxdna/aie4_pci.c >> b/drivers/accel/amdxdna/aie4_pci.c >> index 70a052ab5f77..60140c3927a2 100644 >> --- a/drivers/accel/amdxdna/aie4_pci.c >> +++ b/drivers/accel/amdxdna/aie4_pci.c >> @@ -1041,6 +1041,7 @@ static int aie4_classic_init(struct amdxdna_dev *xdna) >> if (ret) >> goto free_work_buf; >> + amdxdna_pm_init(xdna); >> return 0; >> free_work_buf: >> @@ -1064,6 +1065,7 @@ static void aie4_vf_fini(struct amdxdna_dev *xdna) >> static void aie4_classic_fini(struct amdxdna_dev *xdna) >> { >> + amdxdna_pm_fini(xdna); >> aie4_classic_hw_stop(xdna->dev_handle); >> aie4_free_fw_log_buffer(xdna->dev_handle); >> aie4_free_work_buffer(xdna->dev_handle); >> @@ -1408,4 +1410,6 @@ const struct amdxdna_dev_ops aie4_classic_ops = { >> .set_aie_state = aie4_set_state, >> .resume = pm_ptr(aie4_classic_resume), >> .suspend = pm_ptr(aie4_classic_suspend), >> + .runtime_resume = pm_ptr(aie4_classic_resume), >> + .runtime_suspend = pm_ptr(aie4_classic_suspend), >> }; >> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c >> b/drivers/accel/amdxdna/amdxdna_ctx.c >> index 608b86fe8f6b..f4004630dc1b 100644 >> --- a/drivers/accel/amdxdna/amdxdna_ctx.c >> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c >> @@ -761,13 +761,32 @@ int amdxdna_drm_submit_cmd_ioctl(struct drm_device >> *dev, void *data, struct drm_ >> return -EINVAL; >> } >> +static int amdxdna_hwctx_wait_cmd(struct amdxdna_client *client, >> + struct amdxdna_drm_wait_cmd *args) >> +{ >> + struct amdxdna_dev *xdna = client->xdna; >> + struct amdxdna_hwctx *hwctx; >> + int ret, idx; >> + >> + idx = srcu_read_lock(&client->hwctx_srcu); >> + hwctx = xa_load(&client->hwctx_xa, args->hwctx); >> + if (!hwctx) { >> + XDNA_DBG(xdna, "PID %d failed to get ctx %d", client->pid, >> args->hwctx); >> + ret = -EINVAL; >> + } else { >> + ret = xdna->dev_info->ops->cmd_wait(hwctx, args->seq, >> args->timeout); >> + } >> + srcu_read_unlock(&client->hwctx_srcu, idx); >> + >> + return ret; >> +} >> + >> int amdxdna_drm_wait_cmd_ioctl(struct drm_device *dev, void *data, struct >> drm_file *filp) >> { >> struct amdxdna_client *client = filp->driver_priv; >> struct amdxdna_dev *xdna = to_xdna_dev(dev); >> struct amdxdna_drm_wait_cmd *args = data; >> - struct amdxdna_hwctx *hwctx; >> - int ret, idx; >> + int ret; >> XDNA_DBG(xdna, "PID %d ctx %d timeout set %d ms for cmd %llu", >> client->pid, args->hwctx, args->timeout, args->seq); >> @@ -775,22 +794,26 @@ int amdxdna_drm_wait_cmd_ioctl(struct drm_device *dev, >> void *data, struct drm_fi >> if (!xdna->dev_info->ops->cmd_wait) >> return -EOPNOTSUPP; >> - idx = srcu_read_lock(&client->hwctx_srcu); >> - hwctx = xa_load(&client->hwctx_xa, args->hwctx); >> - if (!hwctx) { >> - XDNA_DBG(xdna, "PID %d failed to get ctx %d", client->pid, >> args->hwctx); >> - ret = -EINVAL; >> - goto unlock_ctx_srcu; >> + ret = amdxdna_hwctx_wait_cmd(client, args); >> + if (ret == -EAGAIN) { >> + /* >> + * Suspended by power management before the command completed. >> + * Resume the device, which reconnects the context, and wait >> + * again. Take the PM reference outside hwctx_srcu: runtime >> + * resume takes dev_lock, which hwctx destroy holds across >> + * synchronize_srcu(). >> + */ >> + ret = amdxdna_pm_resume_get(xdna); >> + if (ret) >> + return ret; >> + ret = amdxdna_hwctx_wait_cmd(client, args); >> + amdxdna_pm_suspend_put(xdna); >> } >> - ret = xdna->dev_info->ops->cmd_wait(hwctx, args->seq, args->timeout); >> - >> XDNA_DBG(xdna, "PID %d ctx %d cmd %lld wait finished, ret %d", >> client->pid, args->hwctx, args->seq, ret); >> trace_amdxdna_debug_point(current->comm, args->seq, "job returned to >> user"); >> -unlock_ctx_srcu: >> - srcu_read_unlock(&client->hwctx_srcu, idx); >> return ret; >> } >> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c >> b/drivers/accel/amdxdna/amdxdna_pci_drv.c >> index f5f7831c4e80..cb78f145ffa1 100644 >> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c >> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c >> @@ -465,7 +465,7 @@ static void amdxdna_remove(struct pci_dev *pdev) >> static const struct dev_pm_ops amdxdna_pm_ops = { >> SYSTEM_SLEEP_PM_OPS(amdxdna_pm_suspend, amdxdna_pm_resume) >> - RUNTIME_PM_OPS(amdxdna_pm_suspend, amdxdna_pm_resume, NULL) >> + RUNTIME_PM_OPS(amdxdna_pm_runtime_suspend, amdxdna_pm_runtime_resume, >> NULL) >> }; >> static int amdxdna_sriov_configure(struct pci_dev *pdev, int num_vfs) >> @@ -484,7 +484,7 @@ static struct pci_driver amdxdna_pci_driver = { >> .id_table = pci_ids, >> .probe = amdxdna_probe, >> .remove = amdxdna_remove, >> - .driver.pm = &amdxdna_pm_ops, >> + .driver.pm = pm_ptr(&amdxdna_pm_ops), >> .sriov_configure = amdxdna_sriov_configure, >> }; >> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h >> b/drivers/accel/amdxdna/amdxdna_pci_drv.h >> index 505477b4a292..4d047cf3d981 100644 >> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.h >> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h >> @@ -57,6 +57,8 @@ struct amdxdna_dev_ops { >> void (*debugfs_init)(struct amdxdna_dev *xdna); >> int (*resume)(struct amdxdna_dev *xdna); >> int (*suspend)(struct amdxdna_dev *xdna); >> + int (*runtime_resume)(struct amdxdna_dev *xdna); >> + int (*runtime_suspend)(struct amdxdna_dev *xdna); >> int (*sriov_configure)(struct amdxdna_dev *xdna, int num_vfs); >> int (*hwctx_init)(struct amdxdna_hwctx *hwctx); >> void (*hwctx_stop)(struct amdxdna_hwctx *hwctx); >> diff --git a/drivers/accel/amdxdna/amdxdna_pm.c >> b/drivers/accel/amdxdna/amdxdna_pm.c >> index b1fafddd7ad5..7d7bc965342d 100644 >> --- a/drivers/accel/amdxdna/amdxdna_pm.c >> +++ b/drivers/accel/amdxdna/amdxdna_pm.c >> @@ -37,11 +37,40 @@ int amdxdna_pm_resume(struct device *dev) >> return ret; >> } >> +int amdxdna_pm_runtime_suspend(struct device *dev) >> +{ >> + struct amdxdna_dev *xdna = to_xdna_dev(dev_get_drvdata(dev)); >> + int ret = -EBUSY; >> + >> + guard(mutex)(&xdna->dev_lock); >> + if (xdna->dev_info->ops->runtime_suspend) >> + ret = xdna->dev_info->ops->runtime_suspend(xdna); >> + >> + XDNA_DBG(xdna, "Runtime suspend done ret %d", ret); >> + return ret; >> +} >> + >> +int amdxdna_pm_runtime_resume(struct device *dev) >> +{ >> + struct amdxdna_dev *xdna = to_xdna_dev(dev_get_drvdata(dev)); >> + int ret = -EOPNOTSUPP; >> + >> + guard(mutex)(&xdna->dev_lock); >> + if (xdna->dev_info->ops->runtime_resume) >> + ret = xdna->dev_info->ops->runtime_resume(xdna); >> + >> + XDNA_DBG(xdna, "Runtime resume done ret %d", ret); >> + return ret; >> +} >> + >> int amdxdna_pm_resume_get(struct amdxdna_dev *xdna) >> { >> struct device *dev = xdna->ddev.dev; >> int ret; >> + if (!pm_runtime_enabled(dev)) >> + return 0; >> + >> ret = pm_runtime_resume_and_get(dev); >> if (ret) { >> XDNA_ERR(xdna, "Resume failed: %d", ret); >> @@ -55,6 +84,10 @@ void amdxdna_pm_suspend_put(struct amdxdna_dev *xdna) >> { >> struct device *dev = xdna->ddev.dev; >> + if (!pm_runtime_enabled(dev)) >> + return; >> + >> + pm_runtime_mark_last_busy(dev); > This should be removed. mark_last_busy is called by put_autosuspend. >> pm_runtime_put_autosuspend(dev); >> } >> @@ -66,6 +99,7 @@ void amdxdna_pm_init(struct amdxdna_dev *xdna) >> pm_runtime_set_autosuspend_delay(dev, AMDXDNA_AUTOSUSPEND_DELAY); >> pm_runtime_use_autosuspend(dev); >> pm_runtime_allow(dev); >> + pm_runtime_mark_last_busy(dev); > > This should be removed.
Thank you. I will remove these 2 line of code in next version of the patch. /David > > Reviewed-by: Lizhi Hou <[email protected]> > >> pm_runtime_put_autosuspend(dev); >> } >> diff --git a/drivers/accel/amdxdna/amdxdna_pm.h >> b/drivers/accel/amdxdna/amdxdna_pm.h >> index 3d26b973e0e3..02beef6448f1 100644 >> --- a/drivers/accel/amdxdna/amdxdna_pm.h >> +++ b/drivers/accel/amdxdna/amdxdna_pm.h >> @@ -10,6 +10,8 @@ >> int amdxdna_pm_suspend(struct device *dev); >> int amdxdna_pm_resume(struct device *dev); >> +int amdxdna_pm_runtime_suspend(struct device *dev); >> +int amdxdna_pm_runtime_resume(struct device *dev); >> int amdxdna_pm_resume_get(struct amdxdna_dev *xdna); >> void amdxdna_pm_suspend_put(struct amdxdna_dev *xdna); >> void amdxdna_pm_init(struct amdxdna_dev *xdna);
