| Message ID | 20241204213729.3113941-2-lizhi.hou@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | AMD NPU driver improvements | expand |
On 12/4/2024 15:37, Lizhi Hou wrote: > Add device status to track if aie2_hw_start() or aie2_hw_stop() is > re-entered. In aie2_hw_stop(), call drmm_kfree to free mbox. > > Signed-off-by: Lizhi Hou <lizhi.hou@amd.com> > --- > drivers/accel/amdxdna/aie2_pci.c | 17 +++++++++++++++++ > drivers/accel/amdxdna/aie2_pci.h | 7 +++++++ > drivers/accel/amdxdna/amdxdna_mailbox.c | 6 ------ > 3 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c > index 349ada697e48..19c76b2b204b 100644 > --- a/drivers/accel/amdxdna/aie2_pci.c > +++ b/drivers/accel/amdxdna/aie2_pci.c > @@ -267,12 +267,22 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna) > struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev); > struct amdxdna_dev_hdl *ndev = xdna->dev_handle; > > + if (ndev->dev_status <= AIE2_DEV_INIT) { > + XDNA_ERR(xdna, "device is already stopped"); > + return; > + } > + > aie2_mgmt_fw_fini(ndev); > xdna_mailbox_stop_channel(ndev->mgmt_chann); > xdna_mailbox_destroy_channel(ndev->mgmt_chann); > + ndev->mgmt_chann = NULL; > + drmm_kfree(&xdna->ddev, ndev->mbox); > + ndev->mbox = NULL; > aie2_psp_stop(ndev->psp_hdl); > aie2_smu_fini(ndev); > pci_disable_device(pdev); > + > + ndev->dev_status = AIE2_DEV_INIT; > } > > static int aie2_hw_start(struct amdxdna_dev *xdna) > @@ -283,6 +293,11 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) > u32 xdna_mailbox_intr_reg; > int mgmt_mb_irq, ret; > > + if (ndev->dev_status >= AIE2_DEV_START) { > + XDNA_INFO(xdna, "device is already started"); > + return 0; > + } > + > ret = pci_enable_device(pdev); > if (ret) { > XDNA_ERR(xdna, "failed to enable device, ret %d", ret); > @@ -345,6 +360,8 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) > goto destroy_mgmt_chann; > } > > + ndev->dev_status = AIE2_DEV_START; > + > return 0; > > destroy_mgmt_chann: > diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h > index 6a2686255c9c..5d262ae5c9bb 100644 > --- a/drivers/accel/amdxdna/aie2_pci.h > +++ b/drivers/accel/amdxdna/aie2_pci.h > @@ -149,6 +149,11 @@ struct amdxdna_hwctx_priv { > struct drm_syncobj *syncobj; > }; > > +enum aie2_dev_status { > + AIE2_DEV_INIT, > + AIE2_DEV_START, > +}; > + > struct amdxdna_dev_hdl { > struct amdxdna_dev *xdna; > const struct amdxdna_dev_priv *priv; > @@ -171,6 +176,8 @@ struct amdxdna_dev_hdl { > struct mailbox *mbox; > struct mailbox_channel *mgmt_chann; > struct async_events *async_events; > + > + u32 dev_status; Shouldn't this be "enum aie2_dev_status" not "u32"? > }; > > #define DEFINE_BAR_OFFSET(reg_name, bar, reg_addr) \ > diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c > index 415d99abaaa3..eab79dbb8b25 100644 > --- a/drivers/accel/amdxdna/amdxdna_mailbox.c > +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c > @@ -530,9 +530,6 @@ xdna_mailbox_create_channel(struct mailbox *mb, > > int xdna_mailbox_destroy_channel(struct mailbox_channel *mb_chann) > { > - if (!mb_chann) > - return 0; > - > MB_DBG(mb_chann, "IRQ disabled and RX work cancelled"); > free_irq(mb_chann->msix_irq, mb_chann); > destroy_workqueue(mb_chann->work_q); > @@ -548,9 +545,6 @@ int xdna_mailbox_destroy_channel(struct mailbox_channel *mb_chann) > > void xdna_mailbox_stop_channel(struct mailbox_channel *mb_chann) > { > - if (!mb_chann) > - return; > - > /* Disable an irq and wait. This might sleep. */ > disable_irq(mb_chann->msix_irq); >
On 12/4/24 14:26, Mario Limonciello wrote: > On 12/4/2024 15:37, Lizhi Hou wrote: >> Add device status to track if aie2_hw_start() or aie2_hw_stop() is >> re-entered. In aie2_hw_stop(), call drmm_kfree to free mbox. >> >> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com> >> --- >> drivers/accel/amdxdna/aie2_pci.c | 17 +++++++++++++++++ >> drivers/accel/amdxdna/aie2_pci.h | 7 +++++++ >> drivers/accel/amdxdna/amdxdna_mailbox.c | 6 ------ >> 3 files changed, 24 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/accel/amdxdna/aie2_pci.c >> b/drivers/accel/amdxdna/aie2_pci.c >> index 349ada697e48..19c76b2b204b 100644 >> --- a/drivers/accel/amdxdna/aie2_pci.c >> +++ b/drivers/accel/amdxdna/aie2_pci.c >> @@ -267,12 +267,22 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna) >> struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev); >> struct amdxdna_dev_hdl *ndev = xdna->dev_handle; >> + if (ndev->dev_status <= AIE2_DEV_INIT) { >> + XDNA_ERR(xdna, "device is already stopped"); >> + return; >> + } >> + >> aie2_mgmt_fw_fini(ndev); >> xdna_mailbox_stop_channel(ndev->mgmt_chann); >> xdna_mailbox_destroy_channel(ndev->mgmt_chann); >> + ndev->mgmt_chann = NULL; >> + drmm_kfree(&xdna->ddev, ndev->mbox); >> + ndev->mbox = NULL; >> aie2_psp_stop(ndev->psp_hdl); >> aie2_smu_fini(ndev); >> pci_disable_device(pdev); >> + >> + ndev->dev_status = AIE2_DEV_INIT; >> } >> static int aie2_hw_start(struct amdxdna_dev *xdna) >> @@ -283,6 +293,11 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) >> u32 xdna_mailbox_intr_reg; >> int mgmt_mb_irq, ret; >> + if (ndev->dev_status >= AIE2_DEV_START) { >> + XDNA_INFO(xdna, "device is already started"); >> + return 0; >> + } >> + >> ret = pci_enable_device(pdev); >> if (ret) { >> XDNA_ERR(xdna, "failed to enable device, ret %d", ret); >> @@ -345,6 +360,8 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) >> goto destroy_mgmt_chann; >> } >> + ndev->dev_status = AIE2_DEV_START; >> + >> return 0; >> destroy_mgmt_chann: >> diff --git a/drivers/accel/amdxdna/aie2_pci.h >> b/drivers/accel/amdxdna/aie2_pci.h >> index 6a2686255c9c..5d262ae5c9bb 100644 >> --- a/drivers/accel/amdxdna/aie2_pci.h >> +++ b/drivers/accel/amdxdna/aie2_pci.h >> @@ -149,6 +149,11 @@ struct amdxdna_hwctx_priv { >> struct drm_syncobj *syncobj; >> }; >> +enum aie2_dev_status { >> + AIE2_DEV_INIT, >> + AIE2_DEV_START, >> +}; >> + >> struct amdxdna_dev_hdl { >> struct amdxdna_dev *xdna; >> const struct amdxdna_dev_priv *priv; >> @@ -171,6 +176,8 @@ struct amdxdna_dev_hdl { >> struct mailbox *mbox; >> struct mailbox_channel *mgmt_chann; >> struct async_events *async_events; >> + >> + u32 dev_status; > > Shouldn't this be "enum aie2_dev_status" not "u32"? You are right. I will fix it. Thanks, Lizhi > >> }; >> #define DEFINE_BAR_OFFSET(reg_name, bar, reg_addr) \ >> diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c >> b/drivers/accel/amdxdna/amdxdna_mailbox.c >> index 415d99abaaa3..eab79dbb8b25 100644 >> --- a/drivers/accel/amdxdna/amdxdna_mailbox.c >> +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c >> @@ -530,9 +530,6 @@ xdna_mailbox_create_channel(struct mailbox *mb, >> int xdna_mailbox_destroy_channel(struct mailbox_channel *mb_chann) >> { >> - if (!mb_chann) >> - return 0; >> - >> MB_DBG(mb_chann, "IRQ disabled and RX work cancelled"); >> free_irq(mb_chann->msix_irq, mb_chann); >> destroy_workqueue(mb_chann->work_q); >> @@ -548,9 +545,6 @@ int xdna_mailbox_destroy_channel(struct >> mailbox_channel *mb_chann) >> void xdna_mailbox_stop_channel(struct mailbox_channel *mb_chann) >> { >> - if (!mb_chann) >> - return; >> - >> /* Disable an irq and wait. This might sleep. */ >> disable_irq(mb_chann->msix_irq); >
diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c index 349ada697e48..19c76b2b204b 100644 --- a/drivers/accel/amdxdna/aie2_pci.c +++ b/drivers/accel/amdxdna/aie2_pci.c @@ -267,12 +267,22 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna) struct pci_dev *pdev = to_pci_dev(xdna->ddev.dev); struct amdxdna_dev_hdl *ndev = xdna->dev_handle; + if (ndev->dev_status <= AIE2_DEV_INIT) { + XDNA_ERR(xdna, "device is already stopped"); + return; + } + aie2_mgmt_fw_fini(ndev); xdna_mailbox_stop_channel(ndev->mgmt_chann); xdna_mailbox_destroy_channel(ndev->mgmt_chann); + ndev->mgmt_chann = NULL; + drmm_kfree(&xdna->ddev, ndev->mbox); + ndev->mbox = NULL; aie2_psp_stop(ndev->psp_hdl); aie2_smu_fini(ndev); pci_disable_device(pdev); + + ndev->dev_status = AIE2_DEV_INIT; } static int aie2_hw_start(struct amdxdna_dev *xdna) @@ -283,6 +293,11 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) u32 xdna_mailbox_intr_reg; int mgmt_mb_irq, ret; + if (ndev->dev_status >= AIE2_DEV_START) { + XDNA_INFO(xdna, "device is already started"); + return 0; + } + ret = pci_enable_device(pdev); if (ret) { XDNA_ERR(xdna, "failed to enable device, ret %d", ret); @@ -345,6 +360,8 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) goto destroy_mgmt_chann; } + ndev->dev_status = AIE2_DEV_START; + return 0; destroy_mgmt_chann: diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h index 6a2686255c9c..5d262ae5c9bb 100644 --- a/drivers/accel/amdxdna/aie2_pci.h +++ b/drivers/accel/amdxdna/aie2_pci.h @@ -149,6 +149,11 @@ struct amdxdna_hwctx_priv { struct drm_syncobj *syncobj; }; +enum aie2_dev_status { + AIE2_DEV_INIT, + AIE2_DEV_START, +}; + struct amdxdna_dev_hdl { struct amdxdna_dev *xdna; const struct amdxdna_dev_priv *priv; @@ -171,6 +176,8 @@ struct amdxdna_dev_hdl { struct mailbox *mbox; struct mailbox_channel *mgmt_chann; struct async_events *async_events; + + u32 dev_status; }; #define DEFINE_BAR_OFFSET(reg_name, bar, reg_addr) \ diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c index 415d99abaaa3..eab79dbb8b25 100644 --- a/drivers/accel/amdxdna/amdxdna_mailbox.c +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c @@ -530,9 +530,6 @@ xdna_mailbox_create_channel(struct mailbox *mb, int xdna_mailbox_destroy_channel(struct mailbox_channel *mb_chann) { - if (!mb_chann) - return 0; - MB_DBG(mb_chann, "IRQ disabled and RX work cancelled"); free_irq(mb_chann->msix_irq, mb_chann); destroy_workqueue(mb_chann->work_q); @@ -548,9 +545,6 @@ int xdna_mailbox_destroy_channel(struct mailbox_channel *mb_chann) void xdna_mailbox_stop_channel(struct mailbox_channel *mb_chann) { - if (!mb_chann) - return; - /* Disable an irq and wait. This might sleep. */ disable_irq(mb_chann->msix_irq);
Add device status to track if aie2_hw_start() or aie2_hw_stop() is re-entered. In aie2_hw_stop(), call drmm_kfree to free mbox. Signed-off-by: Lizhi Hou <lizhi.hou@amd.com> --- drivers/accel/amdxdna/aie2_pci.c | 17 +++++++++++++++++ drivers/accel/amdxdna/aie2_pci.h | 7 +++++++ drivers/accel/amdxdna/amdxdna_mailbox.c | 6 ------ 3 files changed, 24 insertions(+), 6 deletions(-)