| Message ID | 20241118152152.2921611-1-karunika.choo@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/panthor: Simplify FW fast reset path | expand |
On Mon, Nov 18, 2024 at 03:21:52PM +0000, Karunika Choo wrote: > Stop checking the FW halt_status as MCU_STATUS should be sufficient. > This should make the check for successful FW halt and subsequently > setting fast_reset to true more robust. > > We should also clear GLB_REQ.GLB_HALT bit only on post-reset prior > to starting the FW. > > Signed-off-by: Karunika Choo <karunika.choo@arm.com> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com> Best regards, Liviu > --- > drivers/gpu/drm/panthor/panthor_fw.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c > index ecca5565ce41..6d41089b84ab 100644 > --- a/drivers/gpu/drm/panthor/panthor_fw.c > +++ b/drivers/gpu/drm/panthor/panthor_fw.c > @@ -1098,17 +1098,11 @@ void panthor_fw_pre_reset(struct panthor_device *ptdev, bool on_hang) > panthor_fw_update_reqs(glb_iface, req, GLB_HALT, GLB_HALT); > gpu_write(ptdev, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1); > if (!readl_poll_timeout(ptdev->iomem + MCU_STATUS, status, > - status == MCU_STATUS_HALT, 10, 100000) && > - glb_iface->output->halt_status == PANTHOR_FW_HALT_OK) { > + status == MCU_STATUS_HALT, 10, 100000)) { > ptdev->fw->fast_reset = true; > } else { > drm_warn(&ptdev->base, "Failed to cleanly suspend MCU"); > } > - > - /* The FW detects 0 -> 1 transitions. Make sure we reset > - * the HALT bit before the FW is rebooted. > - */ > - panthor_fw_update_reqs(glb_iface, req, 0, GLB_HALT); > } > > panthor_job_irq_suspend(&ptdev->fw->irq); > @@ -1134,6 +1128,11 @@ int panthor_fw_post_reset(struct panthor_device *ptdev) > * the FW sections. If it fails, go for a full reset. > */ > if (ptdev->fw->fast_reset) { > + /* The FW detects 0 -> 1 transitions. Make sure we reset > + * the HALT bit before the FW is rebooted. > + */ > + panthor_fw_update_reqs(glb_iface, req, 0, GLB_HALT); > + > ret = panthor_fw_start(ptdev); > if (!ret) > goto out; > -- > 2.25.1 >
On Mon, 18 Nov 2024 15:21:52 +0000 Karunika Choo <karunika.choo@arm.com> wrote: > Stop checking the FW halt_status as MCU_STATUS should be sufficient. > This should make the check for successful FW halt and subsequently > setting fast_reset to true more robust. > > We should also clear GLB_REQ.GLB_HALT bit only on post-reset prior > to starting the FW. And maybe add 'only if we're doing a fast reset, because the slow reset will re-initialize all FW sections, including the global interface.' > > Signed-off-by: Karunika Choo <karunika.choo@arm.com> > --- > drivers/gpu/drm/panthor/panthor_fw.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c > index ecca5565ce41..6d41089b84ab 100644 > --- a/drivers/gpu/drm/panthor/panthor_fw.c > +++ b/drivers/gpu/drm/panthor/panthor_fw.c > @@ -1098,17 +1098,11 @@ void panthor_fw_pre_reset(struct panthor_device *ptdev, bool on_hang) > panthor_fw_update_reqs(glb_iface, req, GLB_HALT, GLB_HALT); > gpu_write(ptdev, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1); > if (!readl_poll_timeout(ptdev->iomem + MCU_STATUS, status, > - status == MCU_STATUS_HALT, 10, 100000) && > - glb_iface->output->halt_status == PANTHOR_FW_HALT_OK) { > + status == MCU_STATUS_HALT, 10, 100000)) { > ptdev->fw->fast_reset = true; > } else { > drm_warn(&ptdev->base, "Failed to cleanly suspend MCU"); > } > - > - /* The FW detects 0 -> 1 transitions. Make sure we reset > - * the HALT bit before the FW is rebooted. > - */ > - panthor_fw_update_reqs(glb_iface, req, 0, GLB_HALT); > } > > panthor_job_irq_suspend(&ptdev->fw->irq); > @@ -1134,6 +1128,11 @@ int panthor_fw_post_reset(struct panthor_device *ptdev) > * the FW sections. If it fails, go for a full reset. > */ > if (ptdev->fw->fast_reset) { > + /* The FW detects 0 -> 1 transitions. Make sure we reset > + * the HALT bit before the FW is rebooted. Can you add * This is not needed on a slow reset, because FW * sections are re-initialized. ? > + */ > + panthor_fw_update_reqs(glb_iface, req, 0, GLB_HALT); > + > ret = panthor_fw_start(ptdev); > if (!ret) > goto out; This these 2 minor nits addressed, the patch is `Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>`
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c index ecca5565ce41..6d41089b84ab 100644 --- a/drivers/gpu/drm/panthor/panthor_fw.c +++ b/drivers/gpu/drm/panthor/panthor_fw.c @@ -1098,17 +1098,11 @@ void panthor_fw_pre_reset(struct panthor_device *ptdev, bool on_hang) panthor_fw_update_reqs(glb_iface, req, GLB_HALT, GLB_HALT); gpu_write(ptdev, CSF_DOORBELL(CSF_GLB_DOORBELL_ID), 1); if (!readl_poll_timeout(ptdev->iomem + MCU_STATUS, status, - status == MCU_STATUS_HALT, 10, 100000) && - glb_iface->output->halt_status == PANTHOR_FW_HALT_OK) { + status == MCU_STATUS_HALT, 10, 100000)) { ptdev->fw->fast_reset = true; } else { drm_warn(&ptdev->base, "Failed to cleanly suspend MCU"); } - - /* The FW detects 0 -> 1 transitions. Make sure we reset - * the HALT bit before the FW is rebooted. - */ - panthor_fw_update_reqs(glb_iface, req, 0, GLB_HALT); } panthor_job_irq_suspend(&ptdev->fw->irq); @@ -1134,6 +1128,11 @@ int panthor_fw_post_reset(struct panthor_device *ptdev) * the FW sections. If it fails, go for a full reset. */ if (ptdev->fw->fast_reset) { + /* The FW detects 0 -> 1 transitions. Make sure we reset + * the HALT bit before the FW is rebooted. + */ + panthor_fw_update_reqs(glb_iface, req, 0, GLB_HALT); + ret = panthor_fw_start(ptdev); if (!ret) goto out;
Stop checking the FW halt_status as MCU_STATUS should be sufficient. This should make the check for successful FW halt and subsequently setting fast_reset to true more robust. We should also clear GLB_REQ.GLB_HALT bit only on post-reset prior to starting the FW. Signed-off-by: Karunika Choo <karunika.choo@arm.com> --- drivers/gpu/drm/panthor/panthor_fw.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)