From 57f6a7fe4102276b51a42b8eb16702a103ddf441 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sun, 1 Nov 2026 10:23:48 +1100 Subject: [PATCH 1/2] media: apple: avd: keep DMA blocked once the decoder is retired When the recovery budget runs out, or a reset fails, the decoder is retired: its jobs fail without touching the hardware until the module is reloaded. The frame that triggered it is still finished with an error, so its buffers go back to userspace while the engine that just timed out may still be running, which is the DMA-after-free pattern behind the J616s hangs. Retire through the same first steps as a recovery: disable the interrupt, stop the CM3 and attach the empty IOMMU domain, then leave it attached. A still-running engine can then only fault at the DART. A reset that fails after DMA was blocked keeps it blocked as well. Detach the empty domain at remove so a reload starts from the DMA domain. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/media/platform/apple/avd/avd-drv.c | 29 +++++++++++++++++++++- drivers/media/platform/apple/avd/avd.h | 2 ++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/apple/avd/avd-drv.c b/drivers/media/platform/apple/avd/avd-drv.c index 462df919265d5..7ffc93034e4b2 100644 --- a/drivers/media/platform/apple/avd/avd-drv.c +++ b/drivers/media/platform/apple/avd/avd-drv.c @@ -238,6 +238,27 @@ int avd_submit_job(struct avd_ctx *ctx) * to drain, and only then assert the pmgr reset. Recover sparingly, and * retire the decoder after a failed or a third recovery within a minute. */ +/* + * Retiring: stop the CM3 and leave the decoder's DMA blocked at its DART + * until the module is unloaded, so an engine that is still running (the + * frame timed out, after all) can only fault, never write memory whose + * buffers are about to go back to userspace. + */ +static void avd_park(struct avd_dev *avd) +{ + if (pm_runtime_resume_and_get(avd->dev) < 0) + return; + disable_irq(avd->irq); + avd_shutdown(avd); + if (avd->empty_domain && !avd->dma_parked) { + if (iommu_attach_device(avd->empty_domain, avd->dev)) + dev_err(avd->dev, "retire: could not block DMA\n"); + else + avd->dma_parked = true; + } + pm_runtime_put_autosuspend(avd->dev); +} + static int avd_reset(struct avd_dev *avd) { int ret = 0; @@ -251,6 +272,7 @@ static int avd_reset(struct avd_dev *avd) } if (++avd->recoveries > AVD_RECOVER_MAX) { ret = -EBUSY; + avd_park(avd); goto retire; } if (avd->last_recover && @@ -278,7 +300,10 @@ static int avd_reset(struct avd_dev *avd) ret = reset_control_reset(avd->rstc); if (ret) dev_err(avd->dev, "recover: reset failed: %d\n", ret); - if (avd->empty_domain) { + if (avd->empty_domain && ret) { + /* A decoder that failed its reset keeps its DMA blocked. */ + avd->dma_parked = true; + } else if (avd->empty_domain) { dev_warn(avd->dev, "recover: restore DMA\n"); iommu_detach_device(avd->empty_domain, avd->dev); if (avd->domain && iommu_get_domain_for_dev(avd->dev) != avd->domain) { @@ -1087,6 +1112,8 @@ static void avd_remove(struct platform_device *pdev) avd_v4l2_cleanup(avd); + if (avd->dma_parked) + iommu_detach_device(avd->empty_domain, avd->dev); if (avd->empty_domain) iommu_domain_free(avd->empty_domain); diff --git a/drivers/media/platform/apple/avd/avd.h b/drivers/media/platform/apple/avd/avd.h index 20411c991ae31..10cd73a90977b 100644 --- a/drivers/media/platform/apple/avd/avd.h +++ b/drivers/media/platform/apple/avd/avd.h @@ -241,6 +241,8 @@ struct avd_dev { unsigned int recoveries; unsigned long last_recover; bool retired; + /* Retired with the DART on the empty domain; detached at remove. */ + bool dma_parked; const struct avd_variant *variant; unsigned int capabilities; From fdb9ab00736e75cf3670f000fccd0c5109867d04 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sun, 1 Nov 2026 10:58:52 +1100 Subject: [PATCH 2/2] media: apple: avd: rest the decoder instead of retiring it on a reset storm A third recovery within a minute retired the decoder until the module was reloaded. That is too harsh for a desktop: on an M2 the H.264 conformance suite retired it within three seconds, because a few vectors use features the hardware rejects with a VP error, and every later stream failed until a reload. A user with a bad file would lose hardware decode until a reboot. Recoveries are now quiesced (DMA blocked at the DART before the reset) and have held up under forced errors, so the budget only needs to bound their rate. When it runs out, park the decoder (interrupt off, CM3 stopped, DMA blocked) and fail new frames without touching it until the budget window ends. The next frame after that resets it with DMA still blocked, restores the DMA domain and boots the firmware. Only a reset, restore or boot that fails retires the decoder. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/media/platform/apple/avd/avd-drv.c | 80 ++++++++++++++++++---- drivers/media/platform/apple/avd/avd.h | 5 +- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/drivers/media/platform/apple/avd/avd-drv.c b/drivers/media/platform/apple/avd/avd-drv.c index 7ffc93034e4b2..13f87523226d1 100644 --- a/drivers/media/platform/apple/avd/avd-drv.c +++ b/drivers/media/platform/apple/avd/avd-drv.c @@ -235,20 +235,22 @@ int avd_submit_job(struct avd_ctx *ctx) * frame), so: stop the interrupt handler and the CM3 first, block the * decoder's DMA at its DART so a transfer still in flight faults instead * of reaching memory that may already be reused, give such transfers time - * to drain, and only then assert the pmgr reset. Recover sparingly, and - * retire the decoder after a failed or a third recovery within a minute. + * to drain, and only then assert the pmgr reset. Recover sparingly: a + * third recovery within a minute parks the decoder (DMA blocked) until the + * minute is over, and only a failed reset retires it. */ /* - * Retiring: stop the CM3 and leave the decoder's DMA blocked at its DART - * until the module is unloaded, so an engine that is still running (the - * frame timed out, after all) can only fault, never write memory whose - * buffers are about to go back to userspace. + * Parking (resting or retired): stop the CM3 and leave the decoder's DMA + * blocked at its DART, so an engine that is still running (the frame timed + * out, after all) can only fault, never write memory whose buffers are about + * to go back to userspace. */ static void avd_park(struct avd_dev *avd) { if (pm_runtime_resume_and_get(avd->dev) < 0) return; disable_irq(avd->irq); + avd->irq_parked = true; avd_shutdown(avd); if (avd->empty_domain && !avd->dma_parked) { if (iommu_attach_device(avd->empty_domain, avd->dev)) @@ -271,9 +273,19 @@ static int avd_reset(struct avd_dev *avd) avd->recoveries = 0; } if (++avd->recoveries > AVD_RECOVER_MAX) { - ret = -EBUSY; + /* + * Out of budget: stop the decoder with its DMA blocked and let + * it rest until the window ends. Frames fail without touching + * the hardware meanwhile; the next frame after that recovers + * it. Bad streams (or conformance vectors using features the + * hardware lacks) must not cost the user the decoder until a + * reboot. + */ avd_park(avd); - goto retire; + avd->rest_until = avd->recover_window_start + AVD_RECOVER_WINDOW; + dev_warn(avd->dev, "decoder resting after repeated resets, new frames fail for up to %u s\n", + jiffies_to_msecs(avd->rest_until - jiffies) / 1000 + 1); + return -EBUSY; } if (avd->last_recover && time_before(jiffies, avd->last_recover + msecs_to_jiffies(AVD_RECOVER_GAP_MS))) @@ -328,8 +340,44 @@ static int avd_reset(struct avd_dev *avd) return 0; retire: avd->retired = true; - dev_err(avd->dev, "decoder retired after %s (%d): reload the module to use it again\n", - ret == -EBUSY ? "repeated resets" : "a failed reset", ret); + dev_err(avd->dev, "decoder retired after a failed reset (%d): reload the module to use it again\n", + ret); + return ret; +} + +/* Bring a rested decoder back: reset with DMA still blocked, restore, boot. */ +static int avd_wake(struct avd_dev *avd) +{ + int ret; + + ret = pm_runtime_resume_and_get(avd->dev); + if (ret < 0) + return ret; + dev_warn(avd->dev, "recover: wake after rest\n"); + avd_shutdown(avd); + ret = reset_control_reset(avd->rstc); + if (!ret && avd->dma_parked) { + iommu_detach_device(avd->empty_domain, avd->dev); + if (avd->domain && iommu_get_domain_for_dev(avd->dev) != avd->domain) + ret = -EIO; + else + avd->dma_parked = false; + } + if (!ret) + ret = avd_boot(avd); + if (!ret) { + avd->recoveries = 0; + avd->last_recover = jiffies ?: 1; + if (avd->irq_parked) + enable_irq(avd->irq); + avd->irq_parked = false; + dev_warn(avd->dev, "recover: done\n"); + } else { + avd->retired = true; + dev_err(avd->dev, "decoder retired after a failed wake (%d): reload the module to use it again\n", + ret); + } + pm_runtime_put_autosuspend(avd->dev); return ret; } @@ -485,14 +533,22 @@ static void avd_device_run(void *priv) enum vb2_buffer_state result = VB2_BUF_STATE_ERROR; const char *outcome = "already finished"; unsigned long flags; - bool finish = false; + bool finish = false, fail; u64 seq; int ret; if (WARN_ON(!desc)) return; - if (READ_ONCE(avd->retired) || ctx->broken) { + fail = READ_ONCE(avd->retired) || ctx->broken; + /* A rested decoder stays parked until its window ends, then wakes. */ + if (!fail && avd->rest_until) { + if (time_before(jiffies, avd->rest_until) || avd_wake(avd)) + fail = true; + else + avd->rest_until = 0; + } + if (fail) { struct vb2_v4l2_buffer *src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); /* Complete the request's controls too, or its fd never signals. */ diff --git a/drivers/media/platform/apple/avd/avd.h b/drivers/media/platform/apple/avd/avd.h index 10cd73a90977b..1f2fa8e312d92 100644 --- a/drivers/media/platform/apple/avd/avd.h +++ b/drivers/media/platform/apple/avd/avd.h @@ -241,8 +241,11 @@ struct avd_dev { unsigned int recoveries; unsigned long last_recover; bool retired; - /* Retired with the DART on the empty domain; detached at remove. */ + /* DMA blocked at the DART (retired or resting); detached at remove. */ bool dma_parked; + /* Out of recovery budget: frames fail without hardware access until then. */ + unsigned long rest_until; + bool irq_parked; const struct avd_variant *variant; unsigned int capabilities;