Re: [PATCH V4 18/19] accel/amdxdna: Implement AIE4 suspend and resume

[Lizhi Hou <lizhi.hou@amd.com>]

On 10/9/26 15:53, David Zhang wrote:
> Implement suspend and resume callbacks for AIE4 Physical Function (PF),
> Virtual Function (VF), and Classic device types:
> - Implement aie4_hwctx_suspend_all() to destroy or drain contexts across
>    all registered clients and wait for in-flight jobs.
> - Implement aie4_hwctx_resume_all() to recreate firmware contexts and
>    kick doorbells via aie4_hwctx_resume_jobs() to resume hardware queue
>    consumption.
> - Guard firmware destroy message in aie4_hwctx_destroy() when context
>    ID is invalid.
> - Restore SR-IOV virtual functions on PF resume via aie4_restore_sriov().
> - Link each VF to the PF with a stateless device link when SR-IOV is
>    enabled, so system suspend handles VFs before the PF and resume
>    handles the PF before VFs. The link survives VF driver rebind and is
>    removed before SR-IOV is disabled.
>
> Signed-off-by: David Zhang <yidong.zhang@amd.com>
> ---
>   drivers/accel/amdxdna/aie4_ctx.c        |  18 +-
>   drivers/accel/amdxdna/aie4_pci.c        | 251 ++++++++++++++++++++++++
>   drivers/accel/amdxdna/aie4_pci.h        |  11 ++
>   drivers/accel/amdxdna/aie4_sriov.c      |  66 ++++++-
>   drivers/accel/amdxdna/amdxdna_pci_drv.h |   3 +
>   5 files changed, 347 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie4_ctx.c b/drivers/accel/amdxdna/aie4_ctx.c
> index 43d26454be6c..a2b6b0fa2934 100644
> --- a/drivers/accel/amdxdna/aie4_ctx.c
> +++ b/drivers/accel/amdxdna/aie4_ctx.c
> @@ -255,7 +255,7 @@ void aie4_hwctx_destroy(struct amdxdna_hwctx *hwctx, enum aie4_hwctx_flags flags
>   	if (has_error)
>   		wake_up_all(&priv->job_list_wq);
>   
> -	if (flags != AIE4_HWCTX_DISCONNECT)
> +	if (flags != AIE4_HWCTX_DISCONNECT && priv->hw_ctx_id != CTX_INVALID_ID)
>   		aie4_msg_destroy_context(ndev, priv->hw_ctx_id);
>   
>   	if (cert_comp)
> @@ -363,6 +363,7 @@ int aie4_hwctx_init(struct amdxdna_hwctx *hwctx)
>   		return -ENOMEM;
>   	hwctx->priv = priv;
>   	priv->hwctx = hwctx;
> +	priv->hw_ctx_id = CTX_INVALID_ID;
>   
>   	/* Initialize io_lock guarding cert_comp binding. */
>   	mutex_init(&priv->io_lock);
> @@ -951,6 +952,21 @@ int aie4_hwctx_wait_for_running(struct amdxdna_hwctx *hwctx)
>   	return ret;
>   }
>   
> +void aie4_hwctx_resume_jobs(struct amdxdna_hwctx *hwctx)
> +{
> +	struct amdxdna_hwctx_priv *priv = hwctx->priv;
> +
> +	mutex_lock(&priv->io_lock);
> +	if (list_empty(&priv->running_job_list)) {
> +		mutex_unlock(&priv->io_lock);
> +		return;
> +	}
> +	aie4_doorbell_ring(hwctx);
> +	mutex_unlock(&priv->io_lock);
> +
> +	queue_work(priv->job_work_q, &priv->job_work);
> +}
> +
>   static void put_cmd_bos(struct amdxdna_sched_job *job)
>   {
>   	u32 i;
> diff --git a/drivers/accel/amdxdna/aie4_pci.c b/drivers/accel/amdxdna/aie4_pci.c
> index e09d24d85cdf..70a052ab5f77 100644
> --- a/drivers/accel/amdxdna/aie4_pci.c
> +++ b/drivers/accel/amdxdna/aie4_pci.c
> @@ -1126,11 +1126,258 @@ static void aie4_debugfs_init(struct amdxdna_dev *xdna)
>   					   &aie4_ctx_hysteresis_fops);
>   }
>   
> +void aie4_hwctx_suspend_all(struct amdxdna_dev_hdl *ndev, int clean_jobs)
> +{
> +	struct amdxdna_dev *xdna = ndev->aie.xdna;
> +	struct amdxdna_client *client;
> +	struct amdxdna_hwctx *hwctx;
> +	unsigned long hwctx_id;
> +	int idx;
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +
> +	amdxdna_for_each_client(xdna, client) {
> +		idx = srcu_read_lock(&client->hwctx_srcu);
> +		amdxdna_for_each_hwctx(client, hwctx_id, hwctx) {
> +			/* clean up workers and drain running jobs */
> +			if (clean_jobs) {
> +				int ret;
> +
> +				aie4_hwctx_destroy(hwctx, AIE4_HWCTX_ERROR);
> +				ret = aie4_hwctx_wait_for_running(hwctx);
> +				if (ret)
> +					XDNA_WARN(xdna, "hwctx %s wait for running failed %d",
> +						  hwctx->name, ret);
> +			} else {
> +				aie4_hwctx_destroy(hwctx, AIE4_HWCTX_NORMAL);
> +			}
> +		}
> +		srcu_read_unlock(&client->hwctx_srcu, idx);
> +	}
> +
> +	XDNA_DBG(xdna, "Finished hwctx suspend");
> +}
> +
> +int aie4_hwctx_resume_all(struct amdxdna_dev_hdl *ndev)
> +{
> +	struct amdxdna_dev *xdna = ndev->aie.xdna;
> +	struct amdxdna_client *client;
> +	struct amdxdna_hwctx *hwctx;
> +	unsigned long hwctx_id;
> +	int ret, idx;
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +
> +	amdxdna_for_each_client(xdna, client) {
> +		idx = srcu_read_lock(&client->hwctx_srcu);
> +		amdxdna_for_each_hwctx(client, hwctx_id, hwctx) {
> +			ret = aie4_hwctx_create(hwctx);
> +			if (ret)
> +				goto error;
> +			aie4_hwctx_resume_jobs(hwctx);
> +		}
> +		srcu_read_unlock(&client->hwctx_srcu, idx);
> +	}
> +
> +	XDNA_DBG(xdna, "Finished hwctx resume");
> +	return 0;
> +error:
> +	srcu_read_unlock(&client->hwctx_srcu, idx);
> +	XDNA_DBG(xdna, "Failed hwctx resume");
> +	return ret;
> +}
> +
> +static int aie4_restore_sriov(struct amdxdna_dev_hdl *ndev)
> +{
> +	struct amdxdna_dev *xdna = ndev->aie.xdna;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	int ret;
> +
> +	if (ndev->num_vfs) {
> +		if (pci_num_vf(pdev) != ndev->num_vfs) {
> +			XDNA_ERR(xdna, "inconsistent vf number");
> +			return -EINVAL;
> +		}
> +		ret = aie4_create_vfs(ndev, ndev->num_vfs);
> +		if (ret) {
> +			XDNA_ERR(xdna, "create vfs failed, %d", ret);
> +			return ret;
> +		}
> +		XDNA_DBG(xdna, "restored num_vfs %d", ndev->num_vfs);
> +	}
> +
> +	return 0;
> +}
> +
> +static int aie4_pf_suspend(struct amdxdna_dev *xdna)
> +{
> +	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +	aie4_pf_hw_stop(ndev);
> +	pci_disable_device(pdev);
> +
> +	XDNA_DBG(xdna, "pf suspend done");
> +	return 0;
> +}
> +
> +static int aie4_pf_resume(struct amdxdna_dev *xdna)
> +{
> +	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	int ret;
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "enable pci device failed %d", ret);
> +		return ret;
> +	}
> +	pci_set_master(pdev);
> +
> +	ret = aie4_pf_hw_start(ndev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "hw_start failed %d", ret);
> +		goto pci_disable;
> +	}
> +
> +	ret = aie4_restore_sriov(ndev);
> +	if (ret)
> +		goto hw_stop;
> +
> +	XDNA_DBG(xdna, "pf resume done");
> +	return 0;
> +hw_stop:
> +	aie4_pf_hw_stop(ndev);
> +pci_disable:
> +	pci_disable_device(pdev);
> +	return ret;
> +}
> +
> +static int aie4_vf_suspend(struct amdxdna_dev *xdna)
> +{
> +	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +	aie4_hwctx_suspend_all(ndev, false);
> +	/*
> +	 * partition_fini and mailbox messages should not be called here
> +	 * because PF suspend will do the cleanup for all VFs.
> +	 */
> +	aie4_mailbox_fini(ndev);
> +	pci_disable_device(pdev);
> +
> +	XDNA_DBG(xdna, "vf suspend done");
> +	return 0;
> +}
> +
> +static int aie4_vf_resume(struct amdxdna_dev *xdna)
> +{
> +	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	int ret;
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "enable pci device failed %d", ret);
> +		/* Contexts cannot reconnect; fail waiters and abort running jobs. */
> +		aie4_hwctx_suspend_all(ndev, true);
> +		return ret;
> +	}
> +	pci_set_master(pdev);
> +
> +	ret = aie4_vf_hw_start(ndev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "hw_start failed %d", ret);
> +		/* Contexts cannot reconnect; fail waiters and abort running jobs. */
> +		aie4_hwctx_suspend_all(ndev, true);
> +		goto pci_disable;
> +	}
> +
> +	ret = aie4_hwctx_resume_all(ndev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "hwctx_resume failed %d", ret);
> +		goto hw_clear;
> +	}
> +
> +	XDNA_DBG(xdna, "vf resume done");
> +	return 0;
> +
> +hw_clear:
> +	aie4_hwctx_suspend_all(ndev, true);
> +	aie4_vf_hw_stop(ndev);
> +pci_disable:
> +	pci_disable_device(pdev);
> +	return ret;
> +}
> +
> +static int aie4_classic_suspend(struct amdxdna_dev *xdna)
> +{
> +	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +	aie4_hwctx_suspend_all(ndev, false);
> +	aie4_classic_hw_stop(ndev);
> +	pci_disable_device(pdev);
> +
> +	XDNA_DBG(xdna, "classic suspend done");
> +	return 0;
> +}
> +
> +static int aie4_classic_resume(struct amdxdna_dev *xdna)
> +{
> +	struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
> +	struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev);
> +	int ret;
> +
> +	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "enable pci device failed %d", ret);
> +		/* Contexts cannot reconnect; fail waiters and abort running jobs. */
> +		aie4_hwctx_suspend_all(ndev, true);
> +		return ret;
> +	}
> +	pci_set_master(pdev);
> +
> +	ret = aie4_classic_hw_start(ndev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "hw_start failed %d", ret);
> +		/* Contexts cannot reconnect; fail waiters and abort running jobs. */
> +		aie4_hwctx_suspend_all(ndev, true);
> +		goto pci_disable;
> +	}
> +
> +	ret = aie4_hwctx_resume_all(ndev);
> +	if (ret) {
> +		XDNA_ERR(xdna, "hwctx_resume failed %d", ret);
> +		goto hw_clear;
> +	}
> +
> +	XDNA_DBG(xdna, "classic resume done");
> +	return 0;
> +hw_clear:
> +	aie4_hwctx_suspend_all(ndev, true);
> +	aie4_classic_hw_stop(ndev);
> +pci_disable:
> +	pci_disable_device(pdev);
> +	return ret;
> +}
> +
>   const struct amdxdna_dev_ops aie4_pf_ops = {
>   	.init			= aie4_pf_init,
>   	.fini			= aie4_pf_fini,
>   	.debugfs_init		= aie4_debugfs_init,
>   	.sriov_configure        = aie4_sriov_configure,
> +	.resume			= pm_ptr(aie4_pf_resume),
> +	.suspend		= pm_ptr(aie4_pf_suspend),
>   };
>   
>   const struct amdxdna_dev_ops aie4_vf_ops = {
> @@ -1144,6 +1391,8 @@ const struct amdxdna_dev_ops aie4_vf_ops = {
>   	.cmd_wait		= aie4_cmd_wait,
>   	.get_aie_info		= aie4_get_info,
>   	.set_aie_state		= aie4_set_state,
> +	.resume			= pm_ptr(aie4_vf_resume),
> +	.suspend		= pm_ptr(aie4_vf_suspend),
>   };
>   
>   const struct amdxdna_dev_ops aie4_classic_ops = {
> @@ -1157,4 +1406,6 @@ const struct amdxdna_dev_ops aie4_classic_ops = {
>   	.cmd_wait		= aie4_cmd_wait,
>   	.get_aie_info		= aie4_get_info,
>   	.set_aie_state		= aie4_set_state,
> +	.resume			= pm_ptr(aie4_classic_resume),
> +	.suspend		= pm_ptr(aie4_classic_suspend),
>   };
> diff --git a/drivers/accel/amdxdna/aie4_pci.h b/drivers/accel/amdxdna/aie4_pci.h
> index 4b7367af7cf1..fdc55e498246 100644
> --- a/drivers/accel/amdxdna/aie4_pci.h
> +++ b/drivers/accel/amdxdna/aie4_pci.h
> @@ -95,6 +95,7 @@ struct amdxdna_dev_hdl {
>   	u32				total_col;
>   	u32				max_aieclk_level;
>   	u32				max_npuhclk_level;
> +	u32				num_vfs;
>   
>   	struct dpm_clk_freq		dpm_clk_tbl[AIE4_MAX_DPM_LEVEL_COUNT];
>   
> @@ -159,8 +160,11 @@ int aie4_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job,
>   int aie4_hwctx_create(struct amdxdna_hwctx *hwctx);
>   void aie4_hwctx_destroy(struct amdxdna_hwctx *hwctx, enum aie4_hwctx_flags);
>   int aie4_hwctx_wait_for_running(struct amdxdna_hwctx *hwctx);
> +void aie4_hwctx_resume_jobs(struct amdxdna_hwctx *hwctx);
>   
>   /* aie4_pci.c */
> +void aie4_hwctx_suspend_all(struct amdxdna_dev_hdl *ndev, int clean_jobs);
> +int aie4_hwctx_resume_all(struct amdxdna_dev_hdl *ndev);
>   int aie4_restore_power_mode(struct amdxdna_dev_hdl *ndev);
>   
>   /*
> @@ -176,10 +180,17 @@ void aie4_free_notification(struct cert_comp *comp);
>   
>   /* aie4_sriov.c */
>   #if IS_ENABLED(CONFIG_PCI_IOV)
> +int aie4_create_vfs(struct amdxdna_dev_hdl *ndev, int num_vfs);
>   int aie4_sriov_configure(struct amdxdna_dev *xdna, int num_vfs);
>   int aie4_sriov_stop(struct amdxdna_dev_hdl *ndev);
>   #else
> +static inline int aie4_create_vfs(struct amdxdna_dev_hdl *ndev, int num_vfs)
> +{
> +	return 0;
> +}
> +
>   #define aie4_sriov_configure NULL
> +
>   static inline int aie4_sriov_stop(struct amdxdna_dev_hdl *ndev)
>   {
>   	return 0;
> diff --git a/drivers/accel/amdxdna/aie4_sriov.c b/drivers/accel/amdxdna/aie4_sriov.c
> index e1ce633768a5..d3df36644914 100644
> --- a/drivers/accel/amdxdna/aie4_sriov.c
> +++ b/drivers/accel/amdxdna/aie4_sriov.c
> @@ -26,7 +26,7 @@ static int aie4_destroy_vfs(struct amdxdna_dev_hdl *ndev)
>   	return ret;
>   }
>   
> -static int aie4_create_vfs(struct amdxdna_dev_hdl *ndev, int num_vfs)
> +int aie4_create_vfs(struct amdxdna_dev_hdl *ndev, int num_vfs)
>   {
>   	DECLARE_AIE_MSG(aie4_msg_create_vfs, AIE4_MSG_OP_CREATE_VFS);
>   	int ret;
> @@ -39,6 +39,59 @@ static int aie4_create_vfs(struct amdxdna_dev_hdl *ndev, int num_vfs)
>   	return ret;
>   }
>   
> +static int aie4_for_each_vfs(struct amdxdna_dev *xdna,
> +			     int (*cb)(struct amdxdna_dev *, struct pci_dev *))
> +{
> +	struct pci_dev *pdev_pf = to_pci_dev(xdna->ddev.dev);
> +	struct pci_dev *pdev_vf;
> +	int pos, ret;
> +	u16 vf_did;
> +
> +	pos = pci_find_ext_capability(pdev_pf, PCI_EXT_CAP_ID_SRIOV);
> +	if (!pos)
> +		return 0;
> +	ret = pci_read_config_word(pdev_pf, pos + PCI_SRIOV_VF_DID, &vf_did);
> +	if (ret) {
> +		XDNA_ERR(xdna, "read VF Device ID failed %d", ret);
> +		return -ENODEV;
> +	}
> +
> +	for (pdev_vf = pci_get_device(pdev_pf->vendor, vf_did, NULL);
> +	     pdev_vf;
> +	     pdev_vf = pci_get_device(pdev_pf->vendor, vf_did, pdev_vf)) {
> +		if (!pdev_vf->is_virtfn || pdev_vf->physfn != pdev_pf)
> +			continue;
> +
> +		ret = cb(xdna, pdev_vf);
> +		if (ret) {
> +			/* Release reference on early return. */
> +			pci_dev_put(pdev_vf);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int aie4_link_vf(struct amdxdna_dev *xdna, struct pci_dev *pdev_vf)
> +{
> +	struct pci_dev *pdev_pf = to_pci_dev(xdna->ddev.dev);
> +
> +	/* Stateless so the link survives VF driver rebind. */
> +	if (!device_link_add(&pdev_vf->dev, &pdev_pf->dev, DL_FLAG_STATELESS)) {
> +		XDNA_ERR(xdna, "Failed to link VF %s", pci_name(pdev_vf));
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int aie4_unlink_vf(struct amdxdna_dev *xdna, struct pci_dev *pdev_vf)
> +{
> +	device_link_remove(&pdev_vf->dev, xdna->ddev.dev);
> +	return 0;
> +}
> +
>   int aie4_sriov_stop(struct amdxdna_dev_hdl *ndev)
>   {
>   	struct amdxdna_dev *xdna = ndev->aie.xdna;
> @@ -54,7 +107,9 @@ int aie4_sriov_stop(struct amdxdna_dev_hdl *ndev)
>   		return -EPERM;
>   	}
>   
> +	aie4_for_each_vfs(xdna, aie4_unlink_vf);
>   	pci_disable_sriov(pdev);
> +	ndev->num_vfs = 0;
>   	return aie4_destroy_vfs(ndev);
>   }
>   
> @@ -75,6 +130,15 @@ static int aie4_sriov_start(struct amdxdna_dev_hdl *ndev, int num_vfs)
>   		return ret;
>   	}
>   
> +	ret = aie4_for_each_vfs(xdna, aie4_link_vf);
> +	if (ret) {
> +		aie4_for_each_vfs(xdna, aie4_unlink_vf);
> +		pci_disable_sriov(pdev);
> +		aie4_destroy_vfs(ndev);
> +		return ret;
> +	}
> +
> +	ndev->num_vfs = num_vfs;
>   	return num_vfs;
>   }
>   
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h b/drivers/accel/amdxdna/amdxdna_pci_drv.h
> index eccb461d32bc..505477b4a292 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.h
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h
> @@ -168,6 +168,9 @@ struct amdxdna_client {
>   #define amdxdna_for_each_hwctx(client, hwctx_id, entry)		\
>   	xa_for_each(&(client)->hwctx_xa, hwctx_id, entry)
>   
> +#define amdxdna_for_each_client(xdna, client)			\
> +	list_for_each_entry(client, &(xdna)->client_list, node)
> +
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>   /* Add device info below */
>   extern const struct amdxdna_dev_info dev_npu1_info;
>   extern const struct amdxdna_dev_info dev_npu3_classic_info;