From: "Falkowski, Maciej" <maciej.falkowski@linux.intel.com>
To: Lizhi Hou <lizhi.hou@amd.com>,
ogabbay@kernel.org, quic_jhugo@quicinc.com,
dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, max.zhen@amd.com,
sonal.santan@amd.com, mario.limonciello@amd.com
Subject: Re: [PATCH V2] accel/amdxdna: Poll MPNPU_PWAITMODE after requesting firmware suspend
Date: Tue, 2 Dec 2025 18:29:06 +0100 [thread overview]
Message-ID: <da012e50-692b-4887-830f-d1884df2a4a9@linux.intel.com> (raw)
In-Reply-To: <20251202165427.507414-1-lizhi.hou@amd.com>
Reviewed-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
On 12/2/2025 5:54 PM, Lizhi Hou wrote:
> After issuing a firmware suspend request, the driver must ensure that the
> suspend operation has completed before proceeding. Add polling of the
> MPNPU_PWAITMODE register to confirm that the firmware has fully entered
> the suspended state. This prevents race conditions where subsequent
> operations assume the firmware is idle before it has actually completed
> its suspend sequence.
>
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> ---
> drivers/accel/amdxdna/aie2_message.c | 9 ++++++++-
> drivers/accel/amdxdna/aie2_pci.h | 2 ++
> drivers/accel/amdxdna/aie2_psp.c | 15 +++++++++++++++
> drivers/accel/amdxdna/npu1_regs.c | 2 ++
> drivers/accel/amdxdna/npu2_regs.c | 2 ++
> drivers/accel/amdxdna/npu4_regs.c | 2 ++
> drivers/accel/amdxdna/npu5_regs.c | 2 ++
> drivers/accel/amdxdna/npu6_regs.c | 2 ++
> 8 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
> index d493bb1c3360..fee3b0627aba 100644
> --- a/drivers/accel/amdxdna/aie2_message.c
> +++ b/drivers/accel/amdxdna/aie2_message.c
> @@ -59,8 +59,15 @@ static int aie2_send_mgmt_msg_wait(struct amdxdna_dev_hdl *ndev,
> int aie2_suspend_fw(struct amdxdna_dev_hdl *ndev)
> {
> DECLARE_AIE2_MSG(suspend, MSG_OP_SUSPEND);
> + int ret;
>
> - return aie2_send_mgmt_msg_wait(ndev, &msg);
> + ret = aie2_send_mgmt_msg_wait(ndev, &msg);
> + if (ret) {
> + XDNA_ERR(ndev->xdna, "Failed to suspend fw, ret %d", ret);
> + return ret;
> + }
> +
> + return aie2_psp_waitmode_poll(ndev->psp_hdl);
> }
>
> int aie2_resume_fw(struct amdxdna_dev_hdl *ndev)
> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
> index a5f9c42155d1..cc9f933f80b2 100644
> --- a/drivers/accel/amdxdna/aie2_pci.h
> +++ b/drivers/accel/amdxdna/aie2_pci.h
> @@ -70,6 +70,7 @@ enum psp_reg_idx {
> PSP_INTR_REG = PSP_NUM_IN_REGS,
> PSP_STATUS_REG,
> PSP_RESP_REG,
> + PSP_PWAITMODE_REG,
> PSP_MAX_REGS /* Keep this at the end */
> };
>
> @@ -290,6 +291,7 @@ int aie2_pm_set_mode(struct amdxdna_dev_hdl *ndev, enum amdxdna_power_mode_type
> struct psp_device *aie2m_psp_create(struct drm_device *ddev, struct psp_config *conf);
> int aie2_psp_start(struct psp_device *psp);
> void aie2_psp_stop(struct psp_device *psp);
> +int aie2_psp_waitmode_poll(struct psp_device *psp);
>
> /* aie2_error.c */
> int aie2_error_async_events_alloc(struct amdxdna_dev_hdl *ndev);
> diff --git a/drivers/accel/amdxdna/aie2_psp.c b/drivers/accel/amdxdna/aie2_psp.c
> index f28a060a8810..3a7130577e3e 100644
> --- a/drivers/accel/amdxdna/aie2_psp.c
> +++ b/drivers/accel/amdxdna/aie2_psp.c
> @@ -76,6 +76,21 @@ static int psp_exec(struct psp_device *psp, u32 *reg_vals)
> return 0;
> }
>
> +int aie2_psp_waitmode_poll(struct psp_device *psp)
> +{
> + struct amdxdna_dev *xdna = to_xdna_dev(psp->ddev);
> + u32 mode_reg;
> + int ret;
> +
> + ret = readx_poll_timeout(readl, PSP_REG(psp, PSP_PWAITMODE_REG), mode_reg,
> + (mode_reg & 0x1) == 1,
> + PSP_POLL_INTERVAL, PSP_POLL_TIMEOUT);
> + if (ret)
> + XDNA_ERR(xdna, "fw waitmode reg error, ret %d", ret);
> +
> + return ret;
> +}
> +
> void aie2_psp_stop(struct psp_device *psp)
> {
> u32 reg_vals[PSP_NUM_IN_REGS] = { PSP_RELEASE_TMR, };
> diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/amdxdna/npu1_regs.c
> index ec407f3b48fc..ebc6e2802297 100644
> --- a/drivers/accel/amdxdna/npu1_regs.c
> +++ b/drivers/accel/amdxdna/npu1_regs.c
> @@ -13,6 +13,7 @@
> #include "amdxdna_pci_drv.h"
>
> /* Address definition from NPU1 docs */
> +#define MPNPU_PWAITMODE 0x3010034
> #define MPNPU_PUB_SEC_INTR 0x3010090
> #define MPNPU_PUB_PWRMGMT_INTR 0x3010094
> #define MPNPU_PUB_SCRATCH2 0x30100A0
> @@ -92,6 +93,7 @@ static const struct amdxdna_dev_priv npu1_dev_priv = {
> DEFINE_BAR_OFFSET(PSP_INTR_REG, NPU1_PSP, MPNPU_PUB_SEC_INTR),
> DEFINE_BAR_OFFSET(PSP_STATUS_REG, NPU1_PSP, MPNPU_PUB_SCRATCH2),
> DEFINE_BAR_OFFSET(PSP_RESP_REG, NPU1_PSP, MPNPU_PUB_SCRATCH3),
> + DEFINE_BAR_OFFSET(PSP_PWAITMODE_REG, NPU1_PSP, MPNPU_PWAITMODE),
> },
> .smu_regs_off = {
> DEFINE_BAR_OFFSET(SMU_CMD_REG, NPU1_SMU, MPNPU_PUB_SCRATCH5),
> diff --git a/drivers/accel/amdxdna/npu2_regs.c b/drivers/accel/amdxdna/npu2_regs.c
> index 86f87d0d1354..ad0743fb06d5 100644
> --- a/drivers/accel/amdxdna/npu2_regs.c
> +++ b/drivers/accel/amdxdna/npu2_regs.c
> @@ -13,6 +13,7 @@
> #include "amdxdna_pci_drv.h"
>
> /* NPU Public Registers on MpNPUAxiXbar (refer to Diag npu_registers.h) */
> +#define MPNPU_PWAITMODE 0x301003C
> #define MPNPU_PUB_SEC_INTR 0x3010060
> #define MPNPU_PUB_PWRMGMT_INTR 0x3010064
> #define MPNPU_PUB_SCRATCH0 0x301006C
> @@ -85,6 +86,7 @@ static const struct amdxdna_dev_priv npu2_dev_priv = {
> DEFINE_BAR_OFFSET(PSP_INTR_REG, NPU2_PSP, MP0_C2PMSG_73),
> DEFINE_BAR_OFFSET(PSP_STATUS_REG, NPU2_PSP, MP0_C2PMSG_123),
> DEFINE_BAR_OFFSET(PSP_RESP_REG, NPU2_REG, MPNPU_PUB_SCRATCH3),
> + DEFINE_BAR_OFFSET(PSP_PWAITMODE_REG, NPU2_REG, MPNPU_PWAITMODE),
> },
> .smu_regs_off = {
> DEFINE_BAR_OFFSET(SMU_CMD_REG, NPU2_SMU, MP1_C2PMSG_0),
> diff --git a/drivers/accel/amdxdna/npu4_regs.c b/drivers/accel/amdxdna/npu4_regs.c
> index 986a5f28ba24..4ca21db70478 100644
> --- a/drivers/accel/amdxdna/npu4_regs.c
> +++ b/drivers/accel/amdxdna/npu4_regs.c
> @@ -13,6 +13,7 @@
> #include "amdxdna_pci_drv.h"
>
> /* NPU Public Registers on MpNPUAxiXbar (refer to Diag npu_registers.h) */
> +#define MPNPU_PWAITMODE 0x301003C
> #define MPNPU_PUB_SEC_INTR 0x3010060
> #define MPNPU_PUB_PWRMGMT_INTR 0x3010064
> #define MPNPU_PUB_SCRATCH0 0x301006C
> @@ -116,6 +117,7 @@ static const struct amdxdna_dev_priv npu4_dev_priv = {
> DEFINE_BAR_OFFSET(PSP_INTR_REG, NPU4_PSP, MP0_C2PMSG_73),
> DEFINE_BAR_OFFSET(PSP_STATUS_REG, NPU4_PSP, MP0_C2PMSG_123),
> DEFINE_BAR_OFFSET(PSP_RESP_REG, NPU4_REG, MPNPU_PUB_SCRATCH3),
> + DEFINE_BAR_OFFSET(PSP_PWAITMODE_REG, NPU4_REG, MPNPU_PWAITMODE),
> },
> .smu_regs_off = {
> DEFINE_BAR_OFFSET(SMU_CMD_REG, NPU4_SMU, MP1_C2PMSG_0),
> diff --git a/drivers/accel/amdxdna/npu5_regs.c b/drivers/accel/amdxdna/npu5_regs.c
> index 75ad97f0b937..131080652ef0 100644
> --- a/drivers/accel/amdxdna/npu5_regs.c
> +++ b/drivers/accel/amdxdna/npu5_regs.c
> @@ -13,6 +13,7 @@
> #include "amdxdna_pci_drv.h"
>
> /* NPU Public Registers on MpNPUAxiXbar (refer to Diag npu_registers.h) */
> +#define MPNPU_PWAITMODE 0x301003C
> #define MPNPU_PUB_SEC_INTR 0x3010060
> #define MPNPU_PUB_PWRMGMT_INTR 0x3010064
> #define MPNPU_PUB_SCRATCH0 0x301006C
> @@ -85,6 +86,7 @@ static const struct amdxdna_dev_priv npu5_dev_priv = {
> DEFINE_BAR_OFFSET(PSP_INTR_REG, NPU5_PSP, MP0_C2PMSG_73),
> DEFINE_BAR_OFFSET(PSP_STATUS_REG, NPU5_PSP, MP0_C2PMSG_123),
> DEFINE_BAR_OFFSET(PSP_RESP_REG, NPU5_REG, MPNPU_PUB_SCRATCH3),
> + DEFINE_BAR_OFFSET(PSP_PWAITMODE_REG, NPU5_REG, MPNPU_PWAITMODE),
> },
> .smu_regs_off = {
> DEFINE_BAR_OFFSET(SMU_CMD_REG, NPU5_SMU, MP1_C2PMSG_0),
> diff --git a/drivers/accel/amdxdna/npu6_regs.c b/drivers/accel/amdxdna/npu6_regs.c
> index 758dc013fe13..1f71285655b2 100644
> --- a/drivers/accel/amdxdna/npu6_regs.c
> +++ b/drivers/accel/amdxdna/npu6_regs.c
> @@ -13,6 +13,7 @@
> #include "amdxdna_pci_drv.h"
>
> /* NPU Public Registers on MpNPUAxiXbar (refer to Diag npu_registers.h) */
> +#define MPNPU_PWAITMODE 0x301003C
> #define MPNPU_PUB_SEC_INTR 0x3010060
> #define MPNPU_PUB_PWRMGMT_INTR 0x3010064
> #define MPNPU_PUB_SCRATCH0 0x301006C
> @@ -85,6 +86,7 @@ static const struct amdxdna_dev_priv npu6_dev_priv = {
> DEFINE_BAR_OFFSET(PSP_INTR_REG, NPU6_PSP, MP0_C2PMSG_73),
> DEFINE_BAR_OFFSET(PSP_STATUS_REG, NPU6_PSP, MP0_C2PMSG_123),
> DEFINE_BAR_OFFSET(PSP_RESP_REG, NPU6_REG, MPNPU_PUB_SCRATCH3),
> + DEFINE_BAR_OFFSET(PSP_PWAITMODE_REG, NPU6_REG, MPNPU_PWAITMODE),
> },
> .smu_regs_off = {
> DEFINE_BAR_OFFSET(SMU_CMD_REG, NPU6_SMU, MP1_C2PMSG_0),
next prev parent reply other threads:[~2025-12-02 17:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-02 16:54 [PATCH V2] accel/amdxdna: Poll MPNPU_PWAITMODE after requesting firmware suspend Lizhi Hou
2025-12-02 17:00 ` Mario Limonciello (AMD) (kernel.org)
2025-12-02 17:29 ` Falkowski, Maciej [this message]
2025-12-03 0:46 ` Lizhi Hou
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=da012e50-692b-4887-830f-d1884df2a4a9@linux.intel.com \
--to=maciej.falkowski@linux.intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizhi.hou@amd.com \
--cc=mario.limonciello@amd.com \
--cc=max.zhen@amd.com \
--cc=ogabbay@kernel.org \
--cc=quic_jhugo@quicinc.com \
--cc=sonal.santan@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.