Differences

This shows you the differences between two versions of the page.

--- fourier:vb2_dma_resv_rfc [2026/04/28 22:06] – fourier: vb2-dma-resv kernel RFC page markus_fritsche
+++ fourier:vb2_dma_resv_rfc [2026/04/30 08:56] (current) – v1 sent + reviews from Nicolas/Christian + v2 sketch backlogged + lore-watch caveat markus_fritsche
@@ Line 1: / Line 1: @@
 ====== fourier — vb2-dma-resv kernel RFC ======
--patch kernel RFC drafted as part of the fourier campaign. Targets the
+-patch kernel RFC. Sent to **linux-media** + dri-devel + linaro-mm-sig +
-**linux-media** mailing list. Index: [[fourier:start]].
+linux-rockchip + linux-arm-kernel + linux-kernel on 2026-04-29 19:53 UTC.
+Index: [[fourier:start]].
-===== Why this exists =====
+===== Status (2026-04-30) =====
-The architectural correction that makes the kwin-fourier wait premise
+  * **v1 sent:** Cover Msg-Id ''<20260429195306.239666-1-mfritsche@reauktion.de>''. Thread on lore: [[https://lore.kernel.org/linux-media/20260429195306.239666-1-mfritsche@reauktion.de/]]
-(//&quot;the wait will signal correctly&quot;//) actually true.
+  * **Two reviewer replies received** the same day. Both substantive, neither a NACK.
+  * **v2 sketch backlogged.** Drafted but not yet rewritten in user voice; awaiting review before patches get touched.
+===== Why this exists =====
 vb2 currently does **not** propagate V4L2 producer state into the dmabuf
-&#039;&#039;dma_resv&#039;&#039; exclusive fence. That means a downstream consumer (e.g. the
+''dma_resv'' exclusive fence. A downstream consumer (e.g. the KWin
-KWin compositor polling a dmabuf for completion) cannot rely on the
+compositor polling a dmabuf for completion) cannot rely on the fence
-fence to indicate &quot;decode finished, frame ready&quot;. Without this fix, the
+to indicate "decode finished, frame ready". The compositor instead
-compositor either spins on stub fences (the bug kwin-fourier 0001
+either spins on stub fences (the bug kwin-fourier 0001 works around)
-identified) or has to do the wait itself the hard way (which 0002 does
+or has to do the wait the hard way at compositor level.
-as a workaround).
+===== Patches (v1) =====
+  - vb2 helper API ''vb2_buffer_attach_release_fence()''
+  - hantro opt-in (called from ''buf_queue'' — see review notes for why this is wrong)
+  - rockchip-rga opt-in (same shape, same problem)
+===== v1 reviewer replies =====
+==== Nicolas Dufresne (V4L2 maintainer side) ====
+''<3d8deeb15581b754e4c061d4c4a13657aa08bc3c.camel@ndufresne.ca>'',
+-04-29 21:22 UTC.
+  * Why implicit fencing instead of explicit?
+  * **Critical contract violation:** attaching the fence at ''buf_queue'' (CAPTURE side) publishes it //before// OUTPUT bitstream + parameters + request are guaranteed populated. A user that starves the OUTPUT queue leaves the fence un-armed forever. Violates the dma_fence finite-time-signal contract. Same wall Padovan's earlier explicit-sync series hit.
+  * **Right attach point for hantro:** right before the DEC bit on the control register, after bitstream + params + request are queued.
+  * Suggests basing the design on ''DMA_BUF_IOCTL_EXPORT_SYNC_FILE'' as the kick IOCTL, or attaching an out-fence to ''media_request''. Cites DW100's m2m-to-request conversion as a precedent for that path being approachable.
+  * Mentions Freedreno + Etnaviv "back flash" issue with implicit fences on imported V4L2 buffers as higher-priority work — the GL drivers attach their submit fence into the V4L2 dmabuf's resv on the consumer side, causing the next compositor frame to wait on the GL job rather than the camera.
+==== Christian König (dma-buf / dma-resv maintainer, AMD) ====
+''<4c63c81b-14f7-4e6b-a733-2f98e6055ca8@amd.com>'', 2026-04-30 06:51 UTC,
+in reply to Nicolas.
+  * "+1" on Nicolas's contract critique.
+  * "Implicit fencing is basically just a workaround how the GPU HW used to work ~20 years ago and only rarely makes sense today."
+  * **But:** "putting it behind a flag might be acceptable when you want to interact with implicit synced HW, e.g. a GPU." — exactly the fourier compositor case.
+  * Maintainer-hat ask: annotate fence-publish path with Sima Vetter's ''dma_fence_begin_signalling()'' / ''dma_fence_end_signalling()'' primitives, run with lockdep enabled.
+===== v2 plan (sketch backlogged) =====
+Net of the two reviews: not a NACK. A directed redirect with a permission
+slip ("behind a flag") for the fourier use case. v2 sketch + prior-art
+reference saved in the gitea repo at:
+  * ''upstream-submissions/vb2-dma-resv/v2-cover-letter-sketch.md''
+  * ''upstream-submissions/vb2-dma-resv/v2-prior-art-references.md''
+==== v2 decisions taken ====
+  * **Attach point moves** from ''buf_queue'' to m2m ''device_run''. Hantro: insert at ''drivers/media/platform/verisilicon/hantro_drv.c'' line 187 (between ''v4l2_m2m_buf_copy_metadata'' and ''ctx->codec_ops->run''). RGA: lift ''v4l2_m2m_next_*_buf'' + the fence-attach above the ''ctrl_lock'' spinlock at ''drivers/media/platform/rockchip/rga/rga.c'' line 41 — the helper's ''dma_resv_lock'' is sleepable.
+  * **Per-driver opt-in:** new ''vb2_queue::supports_release_fences'' bool, set in queue init.
+  * **Kconfig:** ''CONFIG_VIDEOBUF2_RELEASE_FENCES'', default n. Bool, not tristate.
+  * **Lockdep:** ''dma_fence_begin/end_signalling'' pairs around publish + signal paths. Validated by full bbb 1080p30 chromium playback under ''CONFIG_PROVE_LOCKING=y CONFIG_LOCKDEP=y''.
+  * **Tag:** keep ''[PATCH RFC v2]''. Attach-point move is non-trivial; one more design round at RFC is cheap.
+  * **CC:** Daniel/Sima Vetter (''daniel.vetter@ffwll.ch'') added; ''--in-reply-to'' the v1 cover so v2 threads under v1.
+==== Open for v3 / future ====
+  * ''media_request'' out-fence path (Nicolas's preferred long-term direction). Bigger redesign; runs in parallel with this opt-in producer-fence series and does not conflict.
+  * Auto-on per vb2-CAPTURE queue. Probably not — opt-in is the safer story given the unfixed freedreno/etnaviv back-flash on the consumer side.
+===== Relation to the rest of the fourier campaign =====
-===== Patches =====
+  * **kwin-fourier 0001-skip-wait** (currently shipped, ''kwin-fourier 1:6.6.4-3'') skips the watchDmaBuf wait entirely. Pragmatic fix; works because no real fence ever lands today.
+  * **kwin-fourier 0002-upstream-shape** was an attempt to write the dmabuf-fd poll dance the way upstream would accept it. Reviewer on KDE MR 9157 (now closed) showed the technical claims were wrong — sync_file is a fence snapshot at export, dmabuf-fd polling tracks later-added fences, Mutter explicitly moved off the polling path on amdgpu. Not for re-submission.
+  * **vb2-dma-resv RFC v2** fixes the gap at the producer level so kwin-fourier 0001's bypass is no longer needed: KWin's stock ''watchDmaBuf'' wait completes correctly because the producer fence now signals when hantro completes.
+  * Both are shippable independently. The kwin patch helps on kernels that never get the RFC; the RFC helps on compositors that never take the kwin patch. With both in tree the kwin patch becomes a no-op and can be retired.
-  - vb2 helper API for &#039;&#039;vb2_buffer_attach_release_fence()&#039;&#039;
+===== How to fetch reply traffic on this thread =====
-  - hantro opt-in
-  - rockchip-rga opt-in
-All kernel APIs verified against &#039;&#039;include/linux/dma-{fence,resv}.h&#039;&#039;.
+''lore-watch.sh'' only matches messages where ''mfritsche@reauktion.de'' appears in
-Patches are drafted, regenerated via &#039;&#039;git format-patch&#039;&#039; in the
+headers. Mid-thread replies that drop the address (the common case for
-shape linux-media expects.
+review threads) are NOT caught. Both 2026-04-30 replies were missed.
+Manual walk:
-===== Submission =====
+<code bash>
+ssh noether
+git clone --bare --depth 500 \
+  https://lore.kernel.org/linux-media/1 \
+  ~/.local/state/lore-watch/linux-media-pi-1.git
+cd ~/.local/state/lore-watch/linux-media-pi-1.git
+git fetch --depth 1000 origin
+git log --all --format='%H %ci %s' | \
+  grep -i 'videobuf2.*dma_resv\|dma_resv.*videobuf2'
+git show <sha>:m
+</code>
-  * **Target list:** linux-media
+linux-media's live public-inbox epoch is **1** as of 2026-04-30; probe
-  * **Recipe + CC list:** &#039;&#039;upstream-submissions/vb2-dma-resv/lkml-submission-notes.md&#039;&#039; in &#039;&#039;[[https://git.reauktion.de/marfrit/marfrit-packages|gitea/marfrit-packages]]&#039;&#039;
+higher numbers if no hits. Anubis blocks lore HTTP; git pack-protocol
-  * **Source path:** &#039;&#039;kernel/vb2-dma-resv-rfc/&#039;&#039; in the same repo
+bypasses it cleanly.
-===== Relation to the rest of the campaign =====
+===== Source paths =====
-  * **kwin-fourier 0002** does the dmabuf-fd poll dance as a workaround at the compositor level.
+  * **Patches (v1):** ''kernel/vb2-dma-resv-rfc/'' in [[https://git.reauktion.de/marfrit/marfrit-packages|gitea/marfrit-packages]]
-  * **vb2-dma-resv RFC** fixes it at the producer level so the workaround stops being necessary.
+  * **v1 submission notes (CC list, send recipe):** ''upstream-submissions/vb2-dma-resv/lkml-submission-notes.md''
-  * Both are shippable independently — different upstreams, different review cadences. The kwin patch is useful even on kernels that never get the RFC; the RFC is useful even on compositors that never take the kwin patch.
+  * **v2 sketch + prior art:** ''upstream-submissions/vb2-dma-resv/v2-cover-letter-sketch.md'', ''v2-prior-art-references.md''