* [PATCH 0/2] drm/vc4: Fix async page flip completion
@ 2026-11-12 12:56 Maíra Canal
2026-11-12 12:56 ` [PATCH 1/2] drm/vc4: Drop BO usecnt before releasing the old FB Maíra Canal
2026-11-12 12:56 ` [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue Maíra Canal
0 siblings, 2 replies; 4+ messages in thread
From: Maíra Canal @ 2026-11-12 12:56 UTC (permalink / raw)
To: Iago Toral, Melissa Wen, Maxime Ripard, Dave Stevenson,
Raspberry Pi Kernel Maintenance, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: kernel-dev, dri-devel, stable, Maíra Canal
VC4 implements async page flips with a custom code path that waits for
the BO fence before completing the flip. On VC4_GEN_4 (RPi 0-3), the
completion also decrements the usecnt of the old FB's BO. This series
fixes two bugs in that path.
Since commit e8941ac97f28 ("drm/vc4: Remove BOs seqnos"), the completion
runs from a dma_fence callback. VC4 signals that fence from the render
job IRQ handler. As vc4_bo_dec_usecnt() takes a mutex, running an SDL
application with SDL_VIDEODRIVER=kmsdrm on the RPi 3 triggers:
[ 2511.636319] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:623
[ 2511.636667] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 0, name: swapper/0
[ 2511.636681] preempt_count: 1000002, expected: 0
[ 2511.636689] RCU nest depth: 0, expected: 0
[ 2511.636698] locks held by swapper/0/0: 1, on CPU#0:
[ 2511.636711] #0: ffff000037183198 (&fence->inline_lock){-...}-{3:3}, at: dma_fence_signal+0x88/0xec
<snip>
[ 2511.636892] Hardware name: Raspberry Pi 3 Model B Plus Rev 1.3 (DT)
[ 2511.636899] Call trace:
<snip>
[ 2511.636991] mutex_lock_nested+0x24/0x30
[ 2511.637002] vc4_bo_dec_usecnt+0x40/0x124 [vc4]
[ 2511.637070] vc4_async_page_flip_complete_with_cleanup+0xa0/0xc4 [vc4]
[ 2511.637102] dma_fence_signal_timestamp_locked+0x118/0x290
[ 2511.637116] dma_fence_signal+0x9c/0xec
[ 2511.637126] vc4_irq_render_job_done+0x24/0x114 [vc4]
[ 2511.637158] vc4_irq+0x16c/0x274 [vc4]
While looking into this issue, I also noticed that, since commit
4d12c36fb73b ("drm/vc4: crtc: Move the BO handling out of common page-flip
callback"), the old FB is released before decrementing its BO usecnt. If
userspace removes the old FB and closes its handle while the flip is
pending, vc4_bo_dec_usecnt() runs on a freed or recycled BO.
Patch 1 fixes the use-after-free and patch 2 moves the completion to a
worker. The UAF fix comes first, as it affects older stable kernels that
don't have the dma_fence-based completion.
Best regards,
- Maíra
---
Maíra Canal (2):
drm/vc4: Drop BO usecnt before releasing the old FB
drm/vc4: Complete async page flips from an ordered workqueue
drivers/gpu/drm/vc4/vc4_crtc.c | 108 ++++++++++++++++++++---------------------
drivers/gpu/drm/vc4/vc4_drv.h | 6 +++
2 files changed, 58 insertions(+), 56 deletions(-)
---
base-commit: 8bf8cda54c08ee3693f4dd1e2b7607fe5e466c79
change-id: 20261111-vc4-async-flip-fixes-1b1ab17dc795
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] drm/vc4: Drop BO usecnt before releasing the old FB
2026-11-12 12:56 [PATCH 0/2] drm/vc4: Fix async page flip completion Maíra Canal
@ 2026-11-12 12:56 ` Maíra Canal
2026-11-12 12:56 ` [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue Maíra Canal
1 sibling, 0 replies; 4+ messages in thread
From: Maíra Canal @ 2026-11-12 12:56 UTC (permalink / raw)
To: Iago Toral, Melissa Wen, Maxime Ripard, Dave Stevenson,
Raspberry Pi Kernel Maintenance, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: kernel-dev, dri-devel, stable, Maíra Canal
The async page flip completion drops the old FB reference before
decrementing its BO usecnt. If that was the last reference, the BO is
already freed or recycled into the BO cache when vc4_bo_dec_usecnt()
runs, leading to a use-after-free or a refcount underflow.
This could happen when userspace removes the old FB and closes its GEM
handle while the flip is still pending.
Decrement the BO usecnt before releasing the old FB reference. Do this
by unifying the two fence callbacks and only decrementing the BO usecnt
in VC4_GEN_4.
Cc: stable@vger.kernel.org
Fixes: 4d12c36fb73b ("drm/vc4: crtc: Move the BO handling out of common page-flip callback")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 62 +++++++++++++++---------------------------
1 file changed, 22 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f42cd5353230..c558b527c7e6 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -895,6 +895,7 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
{
struct drm_crtc *crtc = flip_state->crtc;
struct drm_device *dev = crtc->dev;
+ struct vc4_dev *vc4 = to_vc4_dev(dev);
struct drm_plane *plane = crtc->primary;
vc4_plane_async_set_fb(plane, flip_state->fb);
@@ -909,39 +910,27 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
drm_crtc_vblank_put(crtc);
drm_framebuffer_put(flip_state->fb);
- if (flip_state->old_fb)
- drm_framebuffer_put(flip_state->old_fb);
-
- kfree(flip_state);
-}
-
-static void vc4_async_page_flip_complete_with_cleanup(struct dma_fence *fence,
- struct dma_fence_cb *cb)
-{
- struct vc4_async_flip_state *flip_state =
- container_of(cb, struct vc4_async_flip_state, cb);
- struct vc4_bo *bo = NULL;
-
if (flip_state->old_fb) {
- struct drm_gem_dma_object *dma_bo =
- drm_fb_dma_get_gem_obj(flip_state->old_fb, 0);
- bo = to_vc4_bo(&dma_bo->base);
+ /*
+ * Decrement the BO usecnt in order to keep the inc/dec
+ * calls balanced when the planes are updated through
+ * the async update path.
+ *
+ * FIXME: we should move to generic async-page-flip when
+ * it's available, so that we can get rid of this
+ * hand-made cleanup_fb() logic.
+ */
+ if (vc4->gen == VC4_GEN_4) {
+ struct drm_gem_dma_object *dma_bo =
+ drm_fb_dma_get_gem_obj(flip_state->old_fb, 0);
+
+ vc4_bo_dec_usecnt(to_vc4_bo(&dma_bo->base));
+ }
+
+ drm_framebuffer_put(flip_state->old_fb);
}
- vc4_async_page_flip_complete(flip_state);
- dma_fence_put(fence);
-
- /*
- * Decrement the BO usecnt in order to keep the inc/dec
- * calls balanced when the planes are updated through
- * the async update path.
- *
- * FIXME: we should move to generic async-page-flip when
- * it's available, so that we can get rid of this
- * hand-made cleanup_fb() logic.
- */
- if (bo)
- vc4_bo_dec_usecnt(bo);
+ kfree(flip_state);
}
static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
@@ -959,30 +948,23 @@ static int vc4_async_set_fence_cb(struct drm_device *dev,
{
struct drm_framebuffer *fb = flip_state->fb;
struct drm_gem_dma_object *dma_bo = drm_fb_dma_get_gem_obj(fb, 0);
- dma_fence_func_t async_page_flip_complete_function;
- struct vc4_dev *vc4 = to_vc4_dev(dev);
struct dma_fence *fence;
int ret;
- if (vc4->gen == VC4_GEN_4)
- async_page_flip_complete_function = vc4_async_page_flip_complete_with_cleanup;
- else
- async_page_flip_complete_function = vc4_async_page_flip_fence_complete;
-
ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ, &fence);
if (ret)
return ret;
/* If there's no fence, complete the page flip immediately */
if (!fence) {
- async_page_flip_complete_function(fence, &flip_state->cb);
+ vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
return 0;
}
/* If the fence has already been completed, complete the page flip */
if (dma_fence_add_callback(fence, &flip_state->cb,
- async_page_flip_complete_function))
- async_page_flip_complete_function(fence, &flip_state->cb);
+ vc4_async_page_flip_fence_complete))
+ vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
return 0;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue
2026-11-12 12:56 [PATCH 0/2] drm/vc4: Fix async page flip completion Maíra Canal
2026-11-12 12:56 ` [PATCH 1/2] drm/vc4: Drop BO usecnt before releasing the old FB Maíra Canal
@ 2026-11-12 12:56 ` Maíra Canal
2026-11-12 13:17 ` sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Maíra Canal @ 2026-11-12 12:56 UTC (permalink / raw)
To: Iago Toral, Melissa Wen, Maxime Ripard, Dave Stevenson,
Raspberry Pi Kernel Maintenance, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: kernel-dev, dri-devel, stable, Maíra Canal
Async page flips complete from a dma_fence callback, which runs in the
fence signaling context: the VC4 render job IRQ handler on RPi 0-3 since
commit e8941ac97f28 ("drm/vc4: Remove BOs seqnos"), and the V3D IRQ
handler on RPi 4-5 since commit d19e00ee06a9 ("drm/vc4: crtc: Fix out of
order frames during asynchronous page flips").
Completing the flip may sleep: on RPi 0-3, vc4_bo_dec_usecnt() takes
bo->madv_lock, and on all platforms, dropping the last framebuffer
reference takes fb_lock and can free the BO. When running the kernel
with CONFIG_DEBUG_ATOMIC_SLEEP=y on RPi 3, this triggers the following
warning:
[ 2511.636319] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:623
[ 2511.636667] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 0, name: swapper/0
[ 2511.636681] preempt_count: 1000002, expected: 0
[ 2511.636689] RCU nest depth: 0, expected: 0
[ 2511.636698] locks held by swapper/0/0: 1, on CPU#0:
[ 2511.636711] #0: ffff000037183198 (&fence->inline_lock){-...}-{3:3}, at: dma_fence_signal+0x88/0xec
<snip>
[ 2511.636892] Hardware name: Raspberry Pi 3 Model B Plus Rev 1.3 (DT)
[ 2511.636899] Call trace:
<snip>
[ 2511.636991] mutex_lock_nested+0x24/0x30
[ 2511.637002] vc4_bo_dec_usecnt+0x40/0x124 [vc4]
[ 2511.637070] vc4_async_page_flip_complete_with_cleanup+0xa0/0xc4 [vc4]
[ 2511.637102] dma_fence_signal_timestamp_locked+0x118/0x290
[ 2511.637116] dma_fence_signal+0x9c/0xec
[ 2511.637126] vc4_irq_render_job_done+0x24/0x114 [vc4]
[ 2511.637158] vc4_irq+0x16c/0x274 [vc4]
Make the fence callback only queue a work item on a per-CRTC ordered
workqueue and complete the flip from it. Flips with no fence or an already
signaled one still complete inline, but only after flushing that workqueue,
so they can't complete before earlier pending flips on the same CRTC.
Cc: stable@vger.kernel.org
Fixes: d19e00ee06a9 ("drm/vc4: crtc: Fix out of order frames during asynchronous page flips")
Fixes: e8941ac97f28 ("drm/vc4: Remove BOs seqnos")
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 52 +++++++++++++++++++++++++++---------------
drivers/gpu/drm/vc4/vc4_drv.h | 6 +++++
2 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index c558b527c7e6..623b636a0365 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -884,15 +884,18 @@ struct vc4_async_flip_state {
struct drm_framebuffer *fb;
struct drm_framebuffer *old_fb;
struct drm_pending_vblank_event *event;
+ struct dma_fence *fence;
struct dma_fence_cb cb;
+ struct work_struct work;
};
-/* Called when the V3D execution for the BO being flipped to is done, so that
- * we can actually update the plane's address to point to it.
+/* Executed when the V3D execution for the BO being flipped to is done, so
+ * that we can actually update the plane's address to point to it.
*/
-static void
-vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
+static void vc4_async_page_flip_work(struct work_struct *work)
{
+ struct vc4_async_flip_state *flip_state =
+ container_of(work, struct vc4_async_flip_state, work);
struct drm_crtc *crtc = flip_state->crtc;
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -930,17 +933,19 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
drm_framebuffer_put(flip_state->old_fb);
}
+ dma_fence_put(flip_state->fence);
kfree(flip_state);
}
-static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
- struct dma_fence_cb *cb)
+static void vc4_async_page_flip_fence_cb(struct dma_fence *fence,
+ struct dma_fence_cb *cb)
{
struct vc4_async_flip_state *flip_state =
container_of(cb, struct vc4_async_flip_state, cb);
+ struct vc4_crtc *vc4_crtc = to_vc4_crtc(flip_state->crtc);
- vc4_async_page_flip_complete(flip_state);
- dma_fence_put(fence);
+ /* Completing the flip may sleep, so defer it to a worker. */
+ queue_work(vc4_crtc->async_flip_wq, &flip_state->work);
}
static int vc4_async_set_fence_cb(struct drm_device *dev,
@@ -948,23 +953,26 @@ static int vc4_async_set_fence_cb(struct drm_device *dev,
{
struct drm_framebuffer *fb = flip_state->fb;
struct drm_gem_dma_object *dma_bo = drm_fb_dma_get_gem_obj(fb, 0);
- struct dma_fence *fence;
+ struct vc4_crtc *vc4_crtc = to_vc4_crtc(flip_state->crtc);
int ret;
- ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ, &fence);
+ INIT_WORK(&flip_state->work, vc4_async_page_flip_work);
+
+ ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ,
+ &flip_state->fence);
if (ret)
return ret;
- /* If there's no fence, complete the page flip immediately */
- if (!fence) {
- vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
+ if (flip_state->fence &&
+ !dma_fence_add_callback(flip_state->fence, &flip_state->cb,
+ vc4_async_page_flip_fence_cb))
return 0;
- }
- /* If the fence has already been completed, complete the page flip */
- if (dma_fence_add_callback(fence, &flip_state->cb,
- vc4_async_page_flip_fence_complete))
- vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
+ /* No fence or the fence has already signaled. Complete the flip
+ * immediately, after earlier queued flips.
+ */
+ flush_workqueue(vc4_crtc->async_flip_wq);
+ vc4_async_page_flip_work(&flip_state->work);
return 0;
}
@@ -990,7 +998,7 @@ vc4_async_page_flip_common(struct drm_crtc *crtc,
/* Save the current FB before it's replaced by the new one in
* drm_atomic_set_fb_for_plane(). We'll need the old FB in
- * vc4_async_page_flip_complete() to decrement the BO usecnt and keep
+ * vc4_async_page_flip_work() to decrement the BO usecnt and keep
* it consistent.
* FIXME: we should move to generic async-page-flip when it's
* available, so that we can get rid of this hand-made cleanup_fb()
@@ -1387,7 +1395,13 @@ int __vc4_crtc_init(struct drm_device *drm,
vc4_crtc->data = data;
vc4_crtc->pdev = pdev;
vc4_crtc->feeds_txp = feeds_txp;
+ vc4_crtc->async_flip_wq = drmm_alloc_ordered_workqueue(drm, "%s-async-flip",
+ 0, data->name);
+ if (IS_ERR(vc4_crtc->async_flip_wq))
+ return PTR_ERR(vc4_crtc->async_flip_wq);
+
spin_lock_init(&vc4_crtc->irq_lock);
+
ret = drmm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
crtc_funcs, data->name);
if (ret)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 09134809bf15..f68486be08a6 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -596,6 +596,12 @@ struct vc4_crtc {
* access to that value.
*/
unsigned int current_hvs_channel;
+
+ /**
+ * @async_flip_wq: Ordered workqueue completing the CRTC's async
+ * page flips.
+ */
+ struct workqueue_struct *async_flip_wq;
};
#define to_vc4_crtc(_crtc) \
--
2.56.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue
2026-11-12 12:56 ` [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue Maíra Canal
@ 2026-11-12 13:17 ` sashiko-bot
0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-12 13:17 UTC (permalink / raw)
To: Maíra Canal; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] drm/vc4: inline flip in vc4_async_set_fence_cb() overtakes
pending flips
- [Medium] drm/vc4: NULL data->name and inverted drmm order in
__vc4_crtc_init()
--
commit 350614eb18304f31f6b7fcd5f4826c9bda20f32b
Author: Maíra Canal <mcanal@igalia.com>
drm/vc4: Complete async page flips from an ordered workqueue
This commit defers async page flip completion from the dma_fence callback to
a per-CRTC ordered workqueue so that sleeping operations do not run in
atomic context.
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index c558b527c7e60..623b636a03652 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -884,15 +884,18 @@ struct vc4_async_flip_state {
> struct drm_framebuffer *fb;
> struct drm_framebuffer *old_fb;
> struct drm_pending_vblank_event *event;
> + struct dma_fence *fence;
> struct dma_fence_cb cb;
> + struct work_struct work;
> };
>
> -/* Called when the V3D execution for the BO being flipped to is done, so that
> - * we can actually update the plane's address to point to it.
> +/* Executed when the V3D execution for the BO being flipped to is done, so
> + * that we can actually update the plane's address to point to it.
> */
> -static void
> -vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
> +static void vc4_async_page_flip_work(struct work_struct *work)
> {
> + struct vc4_async_flip_state *flip_state =
> + container_of(work, struct vc4_async_flip_state, work);
> struct drm_crtc *crtc = flip_state->crtc;
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> @@ -930,17 +933,19 @@ vc4_async_page_flip_complete(struct vc4_async_flip_state *flip_state)
> drm_framebuffer_put(flip_state->old_fb);
> }
>
> + dma_fence_put(flip_state->fence);
> kfree(flip_state);
> }
>
> -static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
> - struct dma_fence_cb *cb)
> +static void vc4_async_page_flip_fence_cb(struct dma_fence *fence,
> + struct dma_fence_cb *cb)
> {
> struct vc4_async_flip_state *flip_state =
> container_of(cb, struct vc4_async_flip_state, cb);
> + struct vc4_crtc *vc4_crtc = to_vc4_crtc(flip_state->crtc);
>
> - vc4_async_page_flip_complete(flip_state);
> - dma_fence_put(fence);
> + /* Completing the flip may sleep, so defer it to a worker. */
> + queue_work(vc4_crtc->async_flip_wq, &flip_state->work);
> }
>
> static int vc4_async_set_fence_cb(struct drm_device *dev,
> @@ -948,23 +953,26 @@ static int vc4_async_set_fence_cb(struct drm_device *dev,
> {
> struct drm_framebuffer *fb = flip_state->fb;
> struct drm_gem_dma_object *dma_bo = drm_fb_dma_get_gem_obj(fb, 0);
> - struct dma_fence *fence;
> + struct vc4_crtc *vc4_crtc = to_vc4_crtc(flip_state->crtc);
> int ret;
>
> - ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ, &fence);
> + INIT_WORK(&flip_state->work, vc4_async_page_flip_work);
> +
> + ret = dma_resv_get_singleton(dma_bo->base.resv, DMA_RESV_USAGE_READ,
> + &flip_state->fence);
> if (ret)
> return ret;
>
> - /* If there's no fence, complete the page flip immediately */
> - if (!fence) {
> - vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
> + if (flip_state->fence &&
> + !dma_fence_add_callback(flip_state->fence, &flip_state->cb,
> + vc4_async_page_flip_fence_cb))
> return 0;
> - }
>
> - /* If the fence has already been completed, complete the page flip */
> - if (dma_fence_add_callback(fence, &flip_state->cb,
> - vc4_async_page_flip_fence_complete))
> - vc4_async_page_flip_fence_complete(fence, &flip_state->cb);
> + /* No fence or the fence has already signaled. Complete the flip
> + * immediately, after earlier queued flips.
> + */
> + flush_workqueue(vc4_crtc->async_flip_wq);
> + vc4_async_page_flip_work(&flip_state->work);
[Severity: High]
Can an inline flip in vc4_async_set_fence_cb() overtake or race with an
earlier flip whose fence has not yet queued its work item?
flush_workqueue() only waits for work items already on async_flip_wq. If
flip 1 (to fb1) is still waiting on an unsignaled fence when flip 2 (to fb2,
with no fence or an already-signaled fence) runs, flip 2 completes inline
first:
CPU0 (ioctl) CPU1 (kworker)
vc4_async_page_flip_common() [flip 1, fb1]
plane->state->fb = fb1
dma_fence_add_callback(fence1, ...)
vc4_async_page_flip_common() [flip 2, fb2]
flip2->old_fb = fb1
plane->state->fb = fb2
flush_workqueue(async_flip_wq)
vc4_async_page_flip_work(flip2)
vc4_plane_async_set_fb(plane, fb2)
vc4_bo_dec_usecnt(fb1)
drm_framebuffer_put(fb1)
<fence1 signals, queues work>
vc4_async_page_flip_work(flip1)
vc4_plane_async_set_fb(plane, fb1)
drm_framebuffer_put(fb1)
<-- HVS scans out unreferenced fb1
If fence1 signals around the time of flip 2, both vc4_async_page_flip_work()
calls can also run concurrently and race in vc4_plane_async_set_fb() on
vc4_state->dlist and vc4_state->hw_dlist without locking. When flip 1
finishes after flip 2, it overwrites the display list with the older fb1 and
drops flip 1's reference to fb1 after flip 2 already dropped fb1's old_fb
reference and BO usecnt, leaving the HVS scanning out a framebuffer whose
references and usecnt have been released.
[ ... ]
> @@ -1387,7 +1395,13 @@ int __vc4_crtc_init(struct drm_device *drm,
> vc4_crtc->data = data;
> vc4_crtc->pdev = pdev;
> vc4_crtc->feeds_txp = feeds_txp;
> + vc4_crtc->async_flip_wq = drmm_alloc_ordered_workqueue(drm, "%s-async-flip",
> + 0, data->name);
> + if (IS_ERR(vc4_crtc->async_flip_wq))
> + return PTR_ERR(vc4_crtc->async_flip_wq);
> +
> spin_lock_init(&vc4_crtc->irq_lock);
> +
> ret = drmm_crtc_init_with_planes(drm, crtc, primary_plane, NULL,
> crtc_funcs, data->name);
[Severity: Medium]
Does allocating async_flip_wq before drmm_crtc_init_with_planes() pass a
NULL data->name on BCM2712 and invert the drmm teardown order?
bcm2712_pv0_data and bcm2712_pv1_data leave .base.name as NULL and rely on
drmm_crtc_init_with_planes() to assign a default crtc->name, so data->name
formats the workqueue name as "(null)-async-flip" on Raspberry Pi 5.
In addition, drmm actions run in reverse registration order, so
drmm_crtc_init_with_planes_cleanup() calls drm_crtc_cleanup() and zeroes
*crtc (including crtc->dev and crtc->primary) before
__drmm_workqueue_release() drains and destroys async_flip_wq. Allocating
the workqueue after drmm_crtc_init_with_planes() using crtc->name avoids
both issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-vc4-async-flip-fixes-v1-0-42449a703840@igalia.com?part=2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-12 13:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12 12:56 [PATCH 0/2] drm/vc4: Fix async page flip completion Maíra Canal
2026-11-12 12:56 ` [PATCH 1/2] drm/vc4: Drop BO usecnt before releasing the old FB Maíra Canal
2026-11-12 12:56 ` [PATCH 2/2] drm/vc4: Complete async page flips from an ordered workqueue Maíra Canal
2026-11-12 13:17 ` sashiko-bot
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.