mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Lizhi Hou <lizhi.hou@amd.com>
To: David Zhang <yidong.zhang@amd.com>, <quic_jhugo@quicinc.com>,
	<karol.wachowski@linux.intel.com>, <max.zhen@amd.com>,
	<ogabbay@kernel.org>, <dri-devel@lists.freedesktop.org>,
	<linux-kernel@vger.kernel.org>
Cc: <sonal.santan@amd.com>, <mario.limonciello@amd.com>
Subject: Re: [PATCH V4 19/19] accel/amdxdna: Implement runtime suspend and resume support
Date: Wed, 14 Oct 2026 11:36:45 -0700	[thread overview]
Message-ID: <ebcf212e-fadf-8661-565e-bb7b63afc46a@amd.com> (raw)
In-Reply-To: <20261009225343.3079074-20-yidong.zhang@amd.com>


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 <yidong.zhang@amd.com>
> ---
>   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.

Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>

>   	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);

      reply	other threads:[~2026-10-14 18:37 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09 22:53 [PATCH V4 00/19] accel/amdxdna: Kernel submission and PM for AIE4 David Zhang
2026-10-09 22:53 ` [PATCH V4 01/19] accel/amdxdna: Rename NPU3 firmware files David Zhang
2026-10-09 22:53 ` [PATCH V4 02/19] accel/amdxdna: Remove mmap for doorbell David Zhang
2026-10-12 20:37   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 03/19] accel/amdxdna: Add CERT firmware version support David Zhang
2026-10-12 20:39   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 04/19] accel/amdxdna: Support firmware 6.0 David Zhang
2026-10-12 20:41   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 05/19] accel/amdxdna: Add NPU3 classic device support David Zhang
2026-10-12 20:41   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 06/19] accel/amdxdna: Add AIE version query to aie4_get_info David Zhang
2026-10-12 20:55   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 07/19] accel/amdxdna: Add get and set power_mode for AIE4 David Zhang
2026-10-12 21:29   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 08/19] accel/amdxdna: Add clock, DPM frequency, and resource info queries " David Zhang
2026-10-12 22:58   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 09/19] accel/amdxdna: Add context switch hysteresis with debugfs control David Zhang
2026-10-13 18:29   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 10/19] accel/amdxdna: Refactor AIE4 hardware initialization sequence David Zhang
2026-10-13 18:33   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 11/19] accel/amdxdna: Decouple AIE4 doorbell and MSI-X notify transport hooks David Zhang
2026-10-13 18:38   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 12/19] accel/amdxdna: Implement AIE4 kernel queue lifecycle and memory layout David Zhang
2026-10-13 18:41   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 13/19] accel/amdxdna: Prepare for AIE4 command submission David Zhang
2026-10-13 18:42   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 14/19] accel/amdxdna: Move HMM invalidate wait into common GEM code David Zhang
2026-10-13 18:45   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 15/19] accel/amdxdna: Make populate_range common for AIE2 and AIE4 David Zhang
2026-10-14 18:10   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 16/19] accel/amdxdna: Implement AIE4 command packet building and submission David Zhang
2026-10-14 18:15   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 17/19] accel/amdxdna: Enable AIE4 firmware logging to DRAM David Zhang
2026-10-14 18:24   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 18/19] accel/amdxdna: Implement AIE4 suspend and resume David Zhang
2026-10-14 18:28   ` Lizhi Hou
2026-10-09 22:53 ` [PATCH V4 19/19] accel/amdxdna: Implement runtime suspend and resume support David Zhang
2026-10-14 18:36   ` Lizhi Hou [this message]

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=ebcf212e-fadf-8661-565e-bb7b63afc46a@amd.com \
    --to=lizhi.hou@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=karol.wachowski@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=max.zhen@amd.com \
    --cc=ogabbay@kernel.org \
    --cc=quic_jhugo@quicinc.com \
    --cc=sonal.santan@amd.com \
    --cc=yidong.zhang@amd.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®