[PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[Karl Mehltretter]

UVC and vivid access imported DMA-BUFs with the CPU without cache
synchronization. On a Pi 400, captured data remained in the CPU cache
while physical RAM was stale.

Add an opt-in queue flag to bracket CPU access from buf_prepare() through
buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
drivers pass buffers to hardware. Patch 1 balances finish operations in
VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.

This follows the per-queue flag proposed here:
https://lists.openwall.net/linux-kernel/2024/06/20/236

Based on Linux 7.3. Tests cover the same code on these earlier bases:

- Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
  imported CMA DMA-BUFs with a DRM device mapping:

  Path                          Runs   Stale frames
  UVC, vb2-vmalloc               12        0/96
  vivid, vb2-vmalloc             12        0/96
  vivid, vb2-dma-contig          12        0/96

  All six lifecycle cases passed with both vivid allocators. Read-only
  output USERPTR passed. The test added no warnings or failed CPU-access
  callbacks. A control without the DRM mapping found 4/16 stale frames.

- SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
  PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
  cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
  begin/end calls. The baseline was already coherent.

- BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
  six traced lifecycle cases and 60 repeats passed with balanced callbacks
  and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
  captured 96 UVC frames without stale data, with 100/100 begin/end calls.
  Its baseline was already coherent.

Karl Mehltretter (4):
  media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
  media: videobuf2: add opt-in DMA-BUF CPU access
  media: uvcvideo: synchronize imported DMA-BUFs for CPU access
  media: vivid: synchronize imported DMA-BUFs for CPU access

 .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
 .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
 .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
 .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
 drivers/media/test-drivers/vivid/vivid-core.c |  1 +
 drivers/media/usb/uvc/uvc_queue.c             |  1 +
 include/media/videobuf2-core.h                | 12 ++-
 7 files changed, 116 insertions(+), 9 deletions(-)


base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.47.3

[PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS

[Karl Mehltretter]

VIDIOC_PREPARE_BUF leaves a buffer DEQUEUED after calling buf_prepare()
and the prepare() memory operation. VIDIOC_REMOVE_BUFS accepts that state
but frees the buffer without calling the matching finish operations.

With VIDEO_ADV_DEBUG, vivid reports:

  unbalanced counters for queue 000000009fa716f8, buffer 0:
       buf_prepare: 1 buf_finish: 0
       prepare: 1 finish: 0

Finish prepared buffers before freeing them, as queue cancellation does.

Fixes: a3293a85381e ("media: v4l2: Add REMOVE_BUFS ioctl")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index b0a6084f17573..460eaf4e82f78 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1746,6 +1746,19 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
 			goto unlock;
 		}
 	}
+
+	/* A buffer can be prepared without being queued. Finish it first. */
+	for (i = start; i < start + count; i++) {
+		struct vb2_buffer *vb = vb2_get_buffer(q, i);
+
+		if (!vb->prepared)
+			continue;
+
+		__vb2_buf_mem_finish(vb);
+		call_void_vb_qop(vb, buf_finish, vb);
+		vb->prepared = 0;
+	}
+
 	__vb2_queue_free(q, start, count);
 	dprintk(q, 2, "%u buffers removed\n", count);
 
-- 
2.47.3

[PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

[Karl Mehltretter]

Drivers must bracket CPU access to imported DMA-BUFs with
dma_buf_begin_cpu_access() and dma_buf_end_cpu_access().

Add a queue flag to do so from buf_prepare() through buf_finish(). Use
read access for output and write access for capture. Map imports
bidirectionally because CPU and device access directions are opposite.
Pass the mapping direction to attach_dmabuf() so USERPTR and MMAP keep
the queue's normal direction.

Keep this opt-in because some vb2-vmalloc drivers pass buffers to
hardware. End access in process context because DMA-BUF callbacks may
sleep.

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 .../media/common/videobuf2/videobuf2-core.c   | 83 ++++++++++++++++++-
 .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
 .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
 .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
 include/media/videobuf2-core.h                | 12 ++-
 5 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 460eaf4e82f78..d2a78cd0a86bb 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -371,6 +371,65 @@ static void __vb2_buf_mem_finish(struct vb2_buffer *vb)
 		call_void_memop(vb, finish, vb->planes[plane].mem_priv);
 }
 
+/* Output buffers are read by the driver; capture buffers are written. */
+static enum dma_data_direction vb2_cpu_access_dir(const struct vb2_queue *q)
+{
+	return q->is_output ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+}
+
+static bool __vb2_dmabuf_is_duplicate(const struct vb2_buffer *vb,
+				      unsigned int plane)
+{
+	unsigned int i;
+
+	for (i = 0; i < plane; i++)
+		if (vb->planes[i].dbuf == vb->planes[plane].dbuf)
+			return true;
+
+	return false;
+}
+
+static void __vb2_buf_end_cpu_access(struct vb2_buffer *vb,
+				     unsigned int num_planes)
+{
+	struct vb2_queue *q = vb->vb2_queue;
+	enum dma_data_direction dir = vb2_cpu_access_dir(q);
+	unsigned int plane;
+
+	for (plane = 0; plane < num_planes; plane++) {
+		int ret;
+
+		if (__vb2_dmabuf_is_duplicate(vb, plane))
+			continue;
+
+		ret = dma_buf_end_cpu_access(vb->planes[plane].dbuf, dir);
+		if (ret)
+			pr_warn_ratelimited("[%s] failed to end CPU access to dmabuf plane %u: %d\n",
+					    q->name, plane, ret);
+	}
+}
+
+static int __vb2_buf_begin_cpu_access(struct vb2_buffer *vb)
+{
+	struct vb2_queue *q = vb->vb2_queue;
+	enum dma_data_direction dir = vb2_cpu_access_dir(q);
+	unsigned int plane;
+	int ret;
+
+	for (plane = 0; plane < vb->num_planes; plane++) {
+		if (__vb2_dmabuf_is_duplicate(vb, plane))
+			continue;
+
+		ret = dma_buf_begin_cpu_access(vb->planes[plane].dbuf, dir);
+		if (ret) {
+			__vb2_buf_end_cpu_access(vb, plane);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 /*
  * __setup_offsets() - setup unique offsets ("cookies") for every plane in
  * the buffer.
@@ -1423,6 +1482,9 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 	}
 
 	if (reacquired) {
+		enum dma_data_direction attach_dir = q->uses_cpu_access ?
+			DMA_BIDIRECTIONAL : q->dma_dir;
+
 		if (vb->planes[0].mem_priv) {
 			vb->copied_timestamp = 0;
 			call_void_vb_qop(vb, buf_cleanup, vb);
@@ -1453,7 +1515,8 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 						  vb,
 						  q->alloc_devs[plane] ? : q->dev,
 						  planes[plane].dbuf,
-						  planes[plane].length);
+						  planes[plane].length,
+						  attach_dir);
 			if (IS_ERR(mem_priv)) {
 				dprintk(q, 1, "failed to attach dmabuf\n");
 				ret = PTR_ERR(mem_priv);
@@ -1504,9 +1567,21 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 		}
 	}
 
+	if (q->uses_cpu_access) {
+		ret = __vb2_buf_begin_cpu_access(vb);
+		if (ret) {
+			dprintk(q, 1, "failed to begin CPU access to dmabuf: %d\n",
+				ret);
+			call_void_vb_qop(vb, buf_cleanup, vb);
+			goto err_put_vb2_buf;
+		}
+	}
+
 	ret = call_vb_qop(vb, buf_prepare, vb);
 	if (ret) {
 		dprintk(q, 1, "buffer preparation failed\n");
+		if (q->uses_cpu_access)
+			__vb2_buf_end_cpu_access(vb, vb->num_planes);
 		call_void_vb_qop(vb, buf_cleanup, vb);
 		goto err_put_vb2_buf;
 	}
@@ -1756,6 +1831,8 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
 
 		__vb2_buf_mem_finish(vb);
 		call_void_vb_qop(vb, buf_finish, vb);
+		if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
+			__vb2_buf_end_cpu_access(vb, vb->num_planes);
 		vb->prepared = 0;
 	}
 
@@ -2160,6 +2237,8 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	}
 
 	call_void_vb_qop(vb, buf_finish, vb);
+	if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
+		__vb2_buf_end_cpu_access(vb, vb->num_planes);
 	vb->prepared = 0;
 
 	if (pindex)
@@ -2296,6 +2375,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 
 		if (vb->prepared) {
 			call_void_vb_qop(vb, buf_finish, vb);
+			if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
+				__vb2_buf_end_cpu_access(vb, vb->num_planes);
 			vb->prepared = 0;
 		}
 		__vb2_dqbuf(vb);
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index 9ce6284cd5f26..68c146c043fb1 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -768,7 +768,8 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
-				  struct dma_buf *dbuf, unsigned long size)
+				  struct dma_buf *dbuf, unsigned long size,
+				  enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_buf *buf;
 	struct dma_buf_attachment *dba;
@@ -794,7 +795,7 @@ static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
 		return dba;
 	}
 
-	buf->dma_dir = vb->vb2_queue->dma_dir;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 	buf->db_attach = dba;
 
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index b1d0695cda260..ab03f9d31391a 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -616,7 +616,8 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
-				      struct dma_buf *dbuf, unsigned long size)
+				      struct dma_buf *dbuf, unsigned long size,
+				      enum dma_data_direction dma_dir)
 {
 	struct vb2_dma_sg_buf *buf;
 	struct dma_buf_attachment *dba;
@@ -640,7 +641,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
 		return dba;
 	}
 
-	buf->dma_dir = vb->vb2_queue->dma_dir;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 	buf->db_attach = dba;
 	buf->vb = vb;
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
index 63f023696f3ee..c38faa0188bc9 100644
--- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
+++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
@@ -402,7 +402,8 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
 static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
 				       struct device *dev,
 				       struct dma_buf *dbuf,
-				       unsigned long size)
+				       unsigned long size,
+				       enum dma_data_direction dma_dir)
 {
 	struct vb2_vmalloc_buf *buf;
 
@@ -414,7 +415,7 @@ static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
 		return ERR_PTR(-ENOMEM);
 
 	buf->dbuf = dbuf;
-	buf->dma_dir = vb->vb2_queue->dma_dir;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 
 	return buf;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 4b4f4c15c53a7..8dfe22fa4423c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -78,7 +78,8 @@ struct vb2_buffer;
  *		to the userspace, also optional.
  * @attach_dmabuf: attach a shared &struct dma_buf for a hardware operation;
  *		   used for DMABUF memory types; dev is the alloc device
- *		   dbuf is the shared dma_buf; returns ERR_PTR() on failure;
+ *		   dbuf is the shared dma_buf; dma_dir is the DMA mapping
+ *		   direction; returns ERR_PTR() on failure;
  *		   allocator private per-buffer structure on success;
  *		   this needs to be used for further accesses to the buffer.
  * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF
@@ -136,7 +137,8 @@ struct vb2_mem_ops {
 	void		*(*attach_dmabuf)(struct vb2_buffer *vb,
 					  struct device *dev,
 					  struct dma_buf *dbuf,
-					  unsigned long size);
+					  unsigned long size,
+					  enum dma_data_direction dma_dir);
 	void		(*detach_dmabuf)(void *buf_priv);
 	int		(*map_dmabuf)(void *buf_priv);
 	void		(*unmap_dmabuf)(void *buf_priv);
@@ -510,6 +512,11 @@ struct vb2_buf_ops {
  *		->finish().
  * @non_coherent_mem: when set queue will attempt to allocate buffers using
  *		non-coherent memory.
+ * @uses_cpu_access: the driver transfers buffer data with the CPU instead of
+ *		DMA. For imported dma-bufs, videobuf2 brackets the interval from
+ *		buf_prepare through buf_finish with dma_buf_begin_cpu_access()
+ *		and dma_buf_end_cpu_access(). Imported attachments use
+ *		bidirectional DMA mappings.
  * @lock:	pointer to a mutex that protects the &struct vb2_queue. The
  *		driver must set this to a mutex to let the v4l2 core serialize
  *		the queuing ioctls. This lock is used when waiting for a new
@@ -614,6 +621,7 @@ struct vb2_queue {
 	unsigned int			uses_requests:1;
 	unsigned int			allow_cache_hints:1;
 	unsigned int			non_coherent_mem:1;
+	unsigned int			uses_cpu_access:1;
 
 	struct mutex			*lock;
 	void				*owner;
-- 
2.47.3

[PATCH 3/4] media: uvcvideo: synchronize imported DMA-BUFs for CPU access

[Karl Mehltretter]

uvcvideo writes USB payloads into imported DMA-BUFs with the CPU without
cache synchronization, leaving physical RAM stale.

Set uses_cpu_access to bracket the driver's CPU access.

On a Pi 400, 12 eight-frame captures each had stale data before this
change. All 96 frames matched RAM with it.

Fixes: ccc4c539c05f ("[media] v4l: uvc: add support for DMABUF importing")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/media/usb/uvc/uvc_queue.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 3c002c8f442fb..1f92ee96e0710 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -247,6 +247,7 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
 	queue->queue.drv_priv = queue;
 	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
 	queue->queue.mem_ops = &vb2_vmalloc_memops;
+	queue->queue.uses_cpu_access = true;
 	queue->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
 		| V4L2_BUF_FLAG_TSTAMP_SRC_SOE;
 	queue->queue.lock = &queue->mutex;
-- 
2.47.3

[PATCH 4/4] media: vivid: synchronize imported DMA-BUFs for CPU access

[Karl Mehltretter]

vivid accesses imported DMA-BUFs with the CPU without cache
synchronization, leaving physical RAM stale after capture.

Set uses_cpu_access to bracket the driver's CPU access.

On a Pi 400, stale frames fell from 25/96 to 0/96 with this change.

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/media/test-drivers/vivid/vivid-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index 62cfb5feb2cf5..e3aa1fc3bcf75 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -925,6 +925,7 @@ static int vivid_create_queue(struct vivid_dev *dev,
 	q->ops = ops;
 	q->mem_ops = allocators[dev->inst] == 1 ? &vb2_dma_contig_memops :
 						  &vb2_vmalloc_memops;
+	q->uses_cpu_access = true;
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	q->min_reqbufs_allocation = min_reqbufs_allocation;
 	q->lock = &dev->mutex;
-- 
2.47.3

Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[Laurent Pinchart]

On Mon, Oct 19, 2026 at 09:14:07AM +0200, Karl Mehltretter wrote:
> UVC and vivid access imported DMA-BUFs with the CPU without cache
> synchronization. On a Pi 400, captured data remained in the CPU cache
> while physical RAM was stale.
> 
> Add an opt-in queue flag to bracket CPU access from buf_prepare() through
> buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
> drivers pass buffers to hardware. Patch 1 balances finish operations in
> VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.

This seems completely wrong.

> This follows the per-queue flag proposed here:
> https://lists.openwall.net/linux-kernel/2024/06/20/236
> 
> Based on Linux 7.3. Tests cover the same code on these earlier bases:
> 
> - Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
>   imported CMA DMA-BUFs with a DRM device mapping:
> 
>   Path                          Runs   Stale frames
>   UVC, vb2-vmalloc               12        0/96
>   vivid, vb2-vmalloc             12        0/96
>   vivid, vb2-dma-contig          12        0/96
> 
>   All six lifecycle cases passed with both vivid allocators. Read-only
>   output USERPTR passed. The test added no warnings or failed CPU-access
>   callbacks. A control without the DRM mapping found 4/16 stale frames.
> 
> - SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
>   PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
>   cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
>   begin/end calls. The baseline was already coherent.
> 
> - BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
>   six traced lifecycle cases and 60 repeats passed with balanced callbacks
>   and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
>   captured 96 UVC frames without stale data, with 100/100 begin/end calls.
>   Its baseline was already coherent.
> 
> Karl Mehltretter (4):
>   media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
>   media: videobuf2: add opt-in DMA-BUF CPU access
>   media: uvcvideo: synchronize imported DMA-BUFs for CPU access
>   media: vivid: synchronize imported DMA-BUFs for CPU access
> 
>  .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
>  drivers/media/test-drivers/vivid/vivid-core.c |  1 +
>  drivers/media/usb/uvc/uvc_queue.c             |  1 +
>  include/media/videobuf2-core.h                | 12 ++-
>  7 files changed, 116 insertions(+), 9 deletions(-)
> 
> 
> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[Christian König]

On 10/19/26 14:44, Laurent Pinchart wrote:
> On Mon, Oct 19, 2026 at 09:14:07AM +0200, Karl Mehltretter wrote:
>> UVC and vivid access imported DMA-BUFs with the CPU without cache
>> synchronization. On a Pi 400, captured data remained in the CPU cache
>> while physical RAM was stale.
>>
>> Add an opt-in queue flag to bracket CPU access from buf_prepare() through
>> buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
>> drivers pass buffers to hardware. Patch 1 balances finish operations in
>> VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.
> 
> This seems completely wrong.

It looks a bit questionable from 1000 miles above, but I might be wrong.

Let me iterate the rules here:

Accessing imported DMA-buf with the CPU from userspace should only be possible by using the dma_buf_mmap() interface and that includes the DMA_BUF_IOCTL_SYNC IOCTL to flush buffers dirtied by the CPU. If that is not done this way then that would be a *major* security issue, but I guess that this isn't the issue here.

What the drivers do more likely is that they use the vmap interface to access the DMA-buf with the CPU inside the kernel. Correct?

In this case dma_buf_begin_cpu_access() and dma_buf_end_cpu_access() should be called to properly frame that access.

That seems to be added in patch #2, but I absolutely can't judge if that is correct or not since at the same time you speak about userptr access and the code really looks a bit like spaghetti. 

Regards,
Christian.

> 
>> This follows the per-queue flag proposed here:
>> https://lists.openwall.net/linux-kernel/2024/06/20/236
>>
>> Based on Linux 7.3. Tests cover the same code on these earlier bases:
>>
>> - Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
>>   imported CMA DMA-BUFs with a DRM device mapping:
>>
>>   Path                          Runs   Stale frames
>>   UVC, vb2-vmalloc               12        0/96
>>   vivid, vb2-vmalloc             12        0/96
>>   vivid, vb2-dma-contig          12        0/96
>>
>>   All six lifecycle cases passed with both vivid allocators. Read-only
>>   output USERPTR passed. The test added no warnings or failed CPU-access
>>   callbacks. A control without the DRM mapping found 4/16 stale frames.
>>
>> - SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
>>   PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
>>   cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
>>   begin/end calls. The baseline was already coherent.
>>
>> - BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
>>   six traced lifecycle cases and 60 repeats passed with balanced callbacks
>>   and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
>>   captured 96 UVC frames without stale data, with 100/100 begin/end calls.
>>   Its baseline was already coherent.
>>
>> Karl Mehltretter (4):
>>   media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
>>   media: videobuf2: add opt-in DMA-BUF CPU access
>>   media: uvcvideo: synchronize imported DMA-BUFs for CPU access
>>   media: vivid: synchronize imported DMA-BUFs for CPU access
>>
>>  .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
>>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
>>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
>>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
>>  drivers/media/test-drivers/vivid/vivid-core.c |  1 +
>>  drivers/media/usb/uvc/uvc_queue.c             |  1 +
>>  include/media/videobuf2-core.h                | 12 ++-
>>  7 files changed, 116 insertions(+), 9 deletions(-)
>>
>>
>> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
> 

Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[Laurent Pinchart]

On Mon, Oct 19, 2026 at 04:48:13PM +0200, Christian König wrote:
> On 10/19/26 14:44, Laurent Pinchart wrote:
> > On Mon, Oct 19, 2026 at 09:14:07AM +0200, Karl Mehltretter wrote:
> >> UVC and vivid access imported DMA-BUFs with the CPU without cache
> >> synchronization. On a Pi 400, captured data remained in the CPU cache
> >> while physical RAM was stale.
> >>
> >> Add an opt-in queue flag to bracket CPU access from buf_prepare() through
> >> buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
> >> drivers pass buffers to hardware. Patch 1 balances finish operations in
> >> VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.
> > 
> > This seems completely wrong.
> 
> It looks a bit questionable from 1000 miles above, but I might be wrong.
> 
> Let me iterate the rules here:
> 
> Accessing imported DMA-buf with the CPU from userspace should only be
> possible by using the dma_buf_mmap() interface and that includes the
> DMA_BUF_IOCTL_SYNC IOCTL to flush buffers dirtied by the CPU. If that
> is not done this way then that would be a *major* security issue, but
> I guess that this isn't the issue here.
> 
> What the drivers do more likely is that they use the vmap interface to
> access the DMA-buf with the CPU inside the kernel. Correct?

This patch series introduces a big penalty in the common case where a
userspace application consumes the frame with the CPU. There should be
no cache invalidation in that case.

> In this case dma_buf_begin_cpu_access() and dma_buf_end_cpu_access()
> should be called to properly frame that access.
> 
> That seems to be added in patch #2, but I absolutely can't judge if
> that is correct or not since at the same time you speak about userptr
> access and the code really looks a bit like spaghetti. 
> 
> >> This follows the per-queue flag proposed here:
> >> https://lists.openwall.net/linux-kernel/2024/06/20/236
> >>
> >> Based on Linux 7.3. Tests cover the same code on these earlier bases:
> >>
> >> - Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
> >>   imported CMA DMA-BUFs with a DRM device mapping:
> >>
> >>   Path                          Runs   Stale frames
> >>   UVC, vb2-vmalloc               12        0/96
> >>   vivid, vb2-vmalloc             12        0/96
> >>   vivid, vb2-dma-contig          12        0/96
> >>
> >>   All six lifecycle cases passed with both vivid allocators. Read-only
> >>   output USERPTR passed. The test added no warnings or failed CPU-access
> >>   callbacks. A control without the DRM mapping found 4/16 stale frames.
> >>
> >> - SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
> >>   PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
> >>   cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
> >>   begin/end calls. The baseline was already coherent.
> >>
> >> - BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
> >>   six traced lifecycle cases and 60 repeats passed with balanced callbacks
> >>   and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
> >>   captured 96 UVC frames without stale data, with 100/100 begin/end calls.
> >>   Its baseline was already coherent.
> >>
> >> Karl Mehltretter (4):
> >>   media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
> >>   media: videobuf2: add opt-in DMA-BUF CPU access
> >>   media: uvcvideo: synchronize imported DMA-BUFs for CPU access
> >>   media: vivid: synchronize imported DMA-BUFs for CPU access
> >>
> >>  .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
> >>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
> >>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
> >>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
> >>  drivers/media/test-drivers/vivid/vivid-core.c |  1 +
> >>  drivers/media/usb/uvc/uvc_queue.c             |  1 +
> >>  include/media/videobuf2-core.h                | 12 ++-
> >>  7 files changed, 116 insertions(+), 9 deletions(-)
> >>
> >>
> >> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[Karl Mehltretter]

Hi Christian,

On Mon, Oct 19, 2026 at 04:48:13PM +0100, Christian König wrote:
> 
> What the drivers do more likely is that they use the vmap interface to access the DMA-buf with the CPU inside the kernel. Correct?
> 

Yes. UVC and the tested vivid vb2-vmalloc path write the imported
DMA-BUF through a kernel vmap. Userspace mmaps the DMA-BUF and brackets
each read with DMA_BUF_IOCTL_SYNC.

> In this case dma_buf_begin_cpu_access() and dma_buf_end_cpu_access() should be called to properly frame that access.
> 
> That seems to be added in patch #2, but I absolutely can't judge if that is correct or not since at the same time you speak about userptr access and the code really looks a bit like spaghetti. 


USERPTR is not part of the CPU-access bracketing. The cover letter
mentions it only as a regression test.

An earlier version changed q->dma_dir to DMA_BIDIRECTIONAL and made
read-only output USERPTR fail PREPARE_BUF with EFAULT. Patch 2 leaves
q->dma_dir unchanged and passes DMA_BIDIRECTIONAL only to
attach_dmabuf() for imported DMA-BUFs.

Thanks,
Karl

Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs

[Karl Mehltretter]

Hi Laurent,

On Mon, Oct 19, 2026 at 06:01:16PM +0100, Laurent Pinchart wrote:
> 
> This patch series introduces a big penalty in the common case where a
> userspace application consumes the frame with the CPU. There should be
> no cache invalidation in that case.
> 

I measured this on my Pi 500+. The cost appears when the DMA-BUF has a
V3D attachment that stays mapped but idle. I compared patch 1 alone with
patches 1-4 on the same v7.3-rc4 base at 640x480 YUYV. For the DMA-BUF
runs, userspace read every frame between DMA_BUF_IOCTL_SYNC READ calls.

Median process CPU time (us/frame), five 1800-frame runs:

                                      patch 1      patches 1-4
  DMA-BUF, no DRM import              129.2        130.6
  DMA-BUF, idle V3D map               130.0        153.1
  MMAP                                126.5        125.4

The no-import and MMAP ranges overlapped. The V3D-mapped ranges did not.
Pi 400 reproduced the mapped cost with vivid (+8.7%) and UVC (+7.5%).

The task-clock profile covered only the patched mapped path:

  Operation                              Sample share
  DQBUF producer END|WRITE clean         26%
  userspace START|READ invalidation      13%
  userspace END|READ clean               8%

I tried a CMA experiment that kept the DMA-BUF callbacks but skipped the
read-only END|READ attachment sync. The skip variant saved a median 12.3
us/frame (8.0%) across three alternating same-boot pairs. With private
DMA-BUFs, the full-sync and skip variants both passed 64 byte-checked
V3D read and overwrite cycles after END|READ.

That experiment still performs END|WRITE and START|READ. In a negative
control without END|WRITE, all eight V3D reads returned stale data.

Patch 1 is independent, so I can send it separately. 

For patches 2-4: How should vb2 avoid the END|WRITE/START|READ
round trip when userspace consumes the frame with the CPU, while still
publishing the frame if the next consumer is a device?

Thanks,
Karl

Re: [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS

[Tomasz Figa]

Hi Karl,

On Mon, Oct 19, 2026 at 4:14 PM Karl Mehltretter <kmehltretter@gmail.com> wrote:
>
> VIDIOC_PREPARE_BUF leaves a buffer DEQUEUED after calling buf_prepare()
> and the prepare() memory operation. VIDIOC_REMOVE_BUFS accepts that state
> but frees the buffer without calling the matching finish operations.
>
> With VIDEO_ADV_DEBUG, vivid reports:
>
>   unbalanced counters for queue 000000009fa716f8, buffer 0:
>        buf_prepare: 1 buf_finish: 0
>        prepare: 1 finish: 0
>
> Finish prepared buffers before freeing them, as queue cancellation does.
>
> Fixes: a3293a85381e ("media: v4l2: Add REMOVE_BUFS ioctl")
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Thanks for the patch. Please take a look at my comments inline.

>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index b0a6084f17573..460eaf4e82f78 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1746,6 +1746,19 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
>                         goto unlock;
>                 }
>         }
> +
> +       /* A buffer can be prepared without being queued. Finish it first. */
> +       for (i = start; i < start + count; i++) {
> +               struct vb2_buffer *vb = vb2_get_buffer(q, i);
> +
> +               if (!vb->prepared)
> +                       continue;
> +
> +               __vb2_buf_mem_finish(vb);
> +               call_void_vb_qop(vb, buf_finish, vb);
> +               vb->prepared = 0;
> +       }
> +

__vb2_queue_cancel() does much more than that, especially all the
media request handling. In addition, this open coded handling just
causes code duplication.

Would it make more sense to refactor the relevant part of the buffer
cancelation loop from __vb2_queue_cancel() into a helper function and
call it here instead?

Best,
Tomasz

>         __vb2_queue_free(q, start, count);
>         dprintk(q, 2, "%u buffers removed\n", count);
>
> --
> 2.47.3
>

Re: [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

[Tomasz Figa]

On Mon, Oct 19, 2026 at 4:14 PM Karl Mehltretter <kmehltretter@gmail.com> wrote:
>
> Drivers must bracket CPU access to imported DMA-BUFs with
> dma_buf_begin_cpu_access() and dma_buf_end_cpu_access().
>
> Add a queue flag to do so from buf_prepare() through buf_finish(). Use
> read access for output and write access for capture. Map imports
> bidirectionally because CPU and device access directions are opposite.
> Pass the mapping direction to attach_dmabuf() so USERPTR and MMAP keep
> the queue's normal direction.

This isn't right. For the regular vb2 mem ops (dma-sg, dma-contig), a
buffer in the prepared state is expected to be ready for hardware
usage, meaning only the hardware can access it until the buffer is
finished. The vmalloc ops are special in a way that CPU is the
"hardware".

>
> Keep this opt-in because some vb2-vmalloc drivers pass buffers to
> hardware. End access in process context because DMA-BUF callbacks may
> sleep.

That is at least unexpected, but also probably wrong (I haven't
checked all the drivers) - the vmalloc mem ops are expected to be used
by drivers that only access the buffer contents from the CPU.

Putting all the above together, what I think we need is:
 1) prepare/finish ops in vb2_vmalloc_memops to invoke the
dma_buf_begin/end_cpu_access() API if the buffer is backed by a
DMA-buf,
 2) some vb2 API for drivers using the regular (dma-) memops to invoke
the dma_buf_begin/end_cpu_access() API for them when they need to
perform some CPU accesses to the buffers - typically they would do
that in their vb2_ops::buf_prepare/finish callbacks, but definitelly
must not after the buffer is already prepared.

It sounds like this series focuses on uvcvideo and vivid, which likely
means that 1) alone would be enough to fix the original problem. WDYT?

Best,
Tomasz

>
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
>  .../media/common/videobuf2/videobuf2-core.c   | 83 ++++++++++++++++++-
>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
>  include/media/videobuf2-core.h                | 12 ++-
>  5 files changed, 101 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 460eaf4e82f78..d2a78cd0a86bb 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -371,6 +371,65 @@ static void __vb2_buf_mem_finish(struct vb2_buffer *vb)
>                 call_void_memop(vb, finish, vb->planes[plane].mem_priv);
>  }
>
> +/* Output buffers are read by the driver; capture buffers are written. */
> +static enum dma_data_direction vb2_cpu_access_dir(const struct vb2_queue *q)
> +{
> +       return q->is_output ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +}
> +
> +static bool __vb2_dmabuf_is_duplicate(const struct vb2_buffer *vb,
> +                                     unsigned int plane)
> +{
> +       unsigned int i;
> +
> +       for (i = 0; i < plane; i++)
> +               if (vb->planes[i].dbuf == vb->planes[plane].dbuf)
> +                       return true;
> +
> +       return false;
> +}
> +
> +static void __vb2_buf_end_cpu_access(struct vb2_buffer *vb,
> +                                    unsigned int num_planes)
> +{
> +       struct vb2_queue *q = vb->vb2_queue;
> +       enum dma_data_direction dir = vb2_cpu_access_dir(q);
> +       unsigned int plane;
> +
> +       for (plane = 0; plane < num_planes; plane++) {
> +               int ret;
> +
> +               if (__vb2_dmabuf_is_duplicate(vb, plane))
> +                       continue;
> +
> +               ret = dma_buf_end_cpu_access(vb->planes[plane].dbuf, dir);
> +               if (ret)
> +                       pr_warn_ratelimited("[%s] failed to end CPU access to dmabuf plane %u: %d\n",
> +                                           q->name, plane, ret);
> +       }
> +}
> +
> +static int __vb2_buf_begin_cpu_access(struct vb2_buffer *vb)
> +{
> +       struct vb2_queue *q = vb->vb2_queue;
> +       enum dma_data_direction dir = vb2_cpu_access_dir(q);
> +       unsigned int plane;
> +       int ret;
> +
> +       for (plane = 0; plane < vb->num_planes; plane++) {
> +               if (__vb2_dmabuf_is_duplicate(vb, plane))
> +                       continue;
> +
> +               ret = dma_buf_begin_cpu_access(vb->planes[plane].dbuf, dir);
> +               if (ret) {
> +                       __vb2_buf_end_cpu_access(vb, plane);
> +                       return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  /*
>   * __setup_offsets() - setup unique offsets ("cookies") for every plane in
>   * the buffer.
> @@ -1423,6 +1482,9 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>         }
>
>         if (reacquired) {
> +               enum dma_data_direction attach_dir = q->uses_cpu_access ?
> +                       DMA_BIDIRECTIONAL : q->dma_dir;
> +
>                 if (vb->planes[0].mem_priv) {
>                         vb->copied_timestamp = 0;
>                         call_void_vb_qop(vb, buf_cleanup, vb);
> @@ -1453,7 +1515,8 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>                                                   vb,
>                                                   q->alloc_devs[plane] ? : q->dev,
>                                                   planes[plane].dbuf,
> -                                                 planes[plane].length);
> +                                                 planes[plane].length,
> +                                                 attach_dir);
>                         if (IS_ERR(mem_priv)) {
>                                 dprintk(q, 1, "failed to attach dmabuf\n");
>                                 ret = PTR_ERR(mem_priv);
> @@ -1504,9 +1567,21 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
>                 }
>         }
>
> +       if (q->uses_cpu_access) {
> +               ret = __vb2_buf_begin_cpu_access(vb);
> +               if (ret) {
> +                       dprintk(q, 1, "failed to begin CPU access to dmabuf: %d\n",
> +                               ret);
> +                       call_void_vb_qop(vb, buf_cleanup, vb);
> +                       goto err_put_vb2_buf;
> +               }
> +       }
> +
>         ret = call_vb_qop(vb, buf_prepare, vb);
>         if (ret) {
>                 dprintk(q, 1, "buffer preparation failed\n");
> +               if (q->uses_cpu_access)
> +                       __vb2_buf_end_cpu_access(vb, vb->num_planes);
>                 call_void_vb_qop(vb, buf_cleanup, vb);
>                 goto err_put_vb2_buf;
>         }
> @@ -1756,6 +1831,8 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
>
>                 __vb2_buf_mem_finish(vb);
>                 call_void_vb_qop(vb, buf_finish, vb);
> +               if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
> +                       __vb2_buf_end_cpu_access(vb, vb->num_planes);
>                 vb->prepared = 0;
>         }
>
> @@ -2160,6 +2237,8 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
>         }
>
>         call_void_vb_qop(vb, buf_finish, vb);
> +       if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
> +               __vb2_buf_end_cpu_access(vb, vb->num_planes);
>         vb->prepared = 0;
>
>         if (pindex)
> @@ -2296,6 +2375,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>
>                 if (vb->prepared) {
>                         call_void_vb_qop(vb, buf_finish, vb);
> +                       if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
> +                               __vb2_buf_end_cpu_access(vb, vb->num_planes);
>                         vb->prepared = 0;
>                 }
>                 __vb2_dqbuf(vb);
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index 9ce6284cd5f26..68c146c043fb1 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -768,7 +768,8 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
>  }
>
>  static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> -                                 struct dma_buf *dbuf, unsigned long size)
> +                                 struct dma_buf *dbuf, unsigned long size,
> +                                 enum dma_data_direction dma_dir)
>  {
>         struct vb2_dc_buf *buf;
>         struct dma_buf_attachment *dba;
> @@ -794,7 +795,7 @@ static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
>                 return dba;
>         }
>
> -       buf->dma_dir = vb->vb2_queue->dma_dir;
> +       buf->dma_dir = dma_dir;
>         buf->size = size;
>         buf->db_attach = dba;
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index b1d0695cda260..ab03f9d31391a 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -616,7 +616,8 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
>  }
>
>  static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> -                                     struct dma_buf *dbuf, unsigned long size)
> +                                     struct dma_buf *dbuf, unsigned long size,
> +                                     enum dma_data_direction dma_dir)
>  {
>         struct vb2_dma_sg_buf *buf;
>         struct dma_buf_attachment *dba;
> @@ -640,7 +641,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
>                 return dba;
>         }
>
> -       buf->dma_dir = vb->vb2_queue->dma_dir;
> +       buf->dma_dir = dma_dir;
>         buf->size = size;
>         buf->db_attach = dba;
>         buf->vb = vb;
> diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> index 63f023696f3ee..c38faa0188bc9 100644
> --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> @@ -402,7 +402,8 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
>  static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
>                                        struct device *dev,
>                                        struct dma_buf *dbuf,
> -                                      unsigned long size)
> +                                      unsigned long size,
> +                                      enum dma_data_direction dma_dir)
>  {
>         struct vb2_vmalloc_buf *buf;
>
> @@ -414,7 +415,7 @@ static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
>                 return ERR_PTR(-ENOMEM);
>
>         buf->dbuf = dbuf;
> -       buf->dma_dir = vb->vb2_queue->dma_dir;
> +       buf->dma_dir = dma_dir;
>         buf->size = size;
>
>         return buf;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 4b4f4c15c53a7..8dfe22fa4423c 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -78,7 +78,8 @@ struct vb2_buffer;
>   *             to the userspace, also optional.
>   * @attach_dmabuf: attach a shared &struct dma_buf for a hardware operation;
>   *                used for DMABUF memory types; dev is the alloc device
> - *                dbuf is the shared dma_buf; returns ERR_PTR() on failure;
> + *                dbuf is the shared dma_buf; dma_dir is the DMA mapping
> + *                direction; returns ERR_PTR() on failure;
>   *                allocator private per-buffer structure on success;
>   *                this needs to be used for further accesses to the buffer.
>   * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF
> @@ -136,7 +137,8 @@ struct vb2_mem_ops {
>         void            *(*attach_dmabuf)(struct vb2_buffer *vb,
>                                           struct device *dev,
>                                           struct dma_buf *dbuf,
> -                                         unsigned long size);
> +                                         unsigned long size,
> +                                         enum dma_data_direction dma_dir);
>         void            (*detach_dmabuf)(void *buf_priv);
>         int             (*map_dmabuf)(void *buf_priv);
>         void            (*unmap_dmabuf)(void *buf_priv);
> @@ -510,6 +512,11 @@ struct vb2_buf_ops {
>   *             ->finish().
>   * @non_coherent_mem: when set queue will attempt to allocate buffers using
>   *             non-coherent memory.
> + * @uses_cpu_access: the driver transfers buffer data with the CPU instead of
> + *             DMA. For imported dma-bufs, videobuf2 brackets the interval from
> + *             buf_prepare through buf_finish with dma_buf_begin_cpu_access()
> + *             and dma_buf_end_cpu_access(). Imported attachments use
> + *             bidirectional DMA mappings.
>   * @lock:      pointer to a mutex that protects the &struct vb2_queue. The
>   *             driver must set this to a mutex to let the v4l2 core serialize
>   *             the queuing ioctls. This lock is used when waiting for a new
> @@ -614,6 +621,7 @@ struct vb2_queue {
>         unsigned int                    uses_requests:1;
>         unsigned int                    allow_cache_hints:1;
>         unsigned int                    non_coherent_mem:1;
> +       unsigned int                    uses_cpu_access:1;
>
>         struct mutex                    *lock;
>         void                            *owner;
> --
> 2.47.3
>

Re: [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

[Laurent Pinchart]

On Thu, Oct 22, 2026 at 01:20:45AM +0900, Tomasz Figa wrote:
> On Mon, Oct 19, 2026 at 4:14 PM Karl Mehltretter <kmehltretter@gmail.com> wrote:
> >
> > Drivers must bracket CPU access to imported DMA-BUFs with
> > dma_buf_begin_cpu_access() and dma_buf_end_cpu_access().
> >
> > Add a queue flag to do so from buf_prepare() through buf_finish(). Use
> > read access for output and write access for capture. Map imports
> > bidirectionally because CPU and device access directions are opposite.
> > Pass the mapping direction to attach_dmabuf() so USERPTR and MMAP keep
> > the queue's normal direction.
> 
> This isn't right. For the regular vb2 mem ops (dma-sg, dma-contig), a
> buffer in the prepared state is expected to be ready for hardware
> usage, meaning only the hardware can access it until the buffer is
> finished. The vmalloc ops are special in a way that CPU is the
> "hardware".
> 
> > Keep this opt-in because some vb2-vmalloc drivers pass buffers to
> > hardware. End access in process context because DMA-BUF callbacks may
> > sleep.
> 
> That is at least unexpected, but also probably wrong (I haven't
> checked all the drivers) - the vmalloc mem ops are expected to be used
> by drivers that only access the buffer contents from the CPU.
> 
> Putting all the above together, what I think we need is:
>  1) prepare/finish ops in vb2_vmalloc_memops to invoke the
> dma_buf_begin/end_cpu_access() API if the buffer is backed by a
> DMA-buf,
>  2) some vb2 API for drivers using the regular (dma-) memops to invoke
> the dma_buf_begin/end_cpu_access() API for them when they need to
> perform some CPU accesses to the buffers - typically they would do
> that in their vb2_ops::buf_prepare/finish callbacks, but definitelly
> must not after the buffer is already prepared.
> 
> It sounds like this series focuses on uvcvideo and vivid, which likely
> means that 1) alone would be enough to fix the original problem. WDYT?

I think the dma-buf API doesn't give us what we need. Cleaning the CPU
cache through end_cpu_access() when the producer is done writting to the
buffer is right in some use cases, but not in all of them. It assumes
that the next consumer will access the data through DMA, which isn't
always the case. What I think we need is to track the state of the
cache(s) in the dma_buf, and perform cache management operations
accordingly when needed. In the uvcvideo -> display use case for
instance, the dma_buf should track that the CPU cache is dirty when
finishing CPU access, and should be cleaned on the display side before
performing the DMA. In a use case where the producer would DMA to the
buffer, cache management would be skipped as the dma_buf would see that
the cache is not dirty.

> > Assisted-by: LLM
> > Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> > ---
> >  .../media/common/videobuf2/videobuf2-core.c   | 83 ++++++++++++++++++-
> >  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
> >  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
> >  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
> >  include/media/videobuf2-core.h                | 12 ++-
> >  5 files changed, 101 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> > index 460eaf4e82f78..d2a78cd0a86bb 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-core.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> > @@ -371,6 +371,65 @@ static void __vb2_buf_mem_finish(struct vb2_buffer *vb)
> >                 call_void_memop(vb, finish, vb->planes[plane].mem_priv);
> >  }
> >
> > +/* Output buffers are read by the driver; capture buffers are written. */
> > +static enum dma_data_direction vb2_cpu_access_dir(const struct vb2_queue *q)
> > +{
> > +       return q->is_output ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> > +}
> > +
> > +static bool __vb2_dmabuf_is_duplicate(const struct vb2_buffer *vb,
> > +                                     unsigned int plane)
> > +{
> > +       unsigned int i;
> > +
> > +       for (i = 0; i < plane; i++)
> > +               if (vb->planes[i].dbuf == vb->planes[plane].dbuf)
> > +                       return true;
> > +
> > +       return false;
> > +}
> > +
> > +static void __vb2_buf_end_cpu_access(struct vb2_buffer *vb,
> > +                                    unsigned int num_planes)
> > +{
> > +       struct vb2_queue *q = vb->vb2_queue;
> > +       enum dma_data_direction dir = vb2_cpu_access_dir(q);
> > +       unsigned int plane;
> > +
> > +       for (plane = 0; plane < num_planes; plane++) {
> > +               int ret;
> > +
> > +               if (__vb2_dmabuf_is_duplicate(vb, plane))
> > +                       continue;
> > +
> > +               ret = dma_buf_end_cpu_access(vb->planes[plane].dbuf, dir);
> > +               if (ret)
> > +                       pr_warn_ratelimited("[%s] failed to end CPU access to dmabuf plane %u: %d\n",
> > +                                           q->name, plane, ret);
> > +       }
> > +}
> > +
> > +static int __vb2_buf_begin_cpu_access(struct vb2_buffer *vb)
> > +{
> > +       struct vb2_queue *q = vb->vb2_queue;
> > +       enum dma_data_direction dir = vb2_cpu_access_dir(q);
> > +       unsigned int plane;
> > +       int ret;
> > +
> > +       for (plane = 0; plane < vb->num_planes; plane++) {
> > +               if (__vb2_dmabuf_is_duplicate(vb, plane))
> > +                       continue;
> > +
> > +               ret = dma_buf_begin_cpu_access(vb->planes[plane].dbuf, dir);
> > +               if (ret) {
> > +                       __vb2_buf_end_cpu_access(vb, plane);
> > +                       return ret;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  /*
> >   * __setup_offsets() - setup unique offsets ("cookies") for every plane in
> >   * the buffer.
> > @@ -1423,6 +1482,9 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
> >         }
> >
> >         if (reacquired) {
> > +               enum dma_data_direction attach_dir = q->uses_cpu_access ?
> > +                       DMA_BIDIRECTIONAL : q->dma_dir;
> > +
> >                 if (vb->planes[0].mem_priv) {
> >                         vb->copied_timestamp = 0;
> >                         call_void_vb_qop(vb, buf_cleanup, vb);
> > @@ -1453,7 +1515,8 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
> >                                                   vb,
> >                                                   q->alloc_devs[plane] ? : q->dev,
> >                                                   planes[plane].dbuf,
> > -                                                 planes[plane].length);
> > +                                                 planes[plane].length,
> > +                                                 attach_dir);
> >                         if (IS_ERR(mem_priv)) {
> >                                 dprintk(q, 1, "failed to attach dmabuf\n");
> >                                 ret = PTR_ERR(mem_priv);
> > @@ -1504,9 +1567,21 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
> >                 }
> >         }
> >
> > +       if (q->uses_cpu_access) {
> > +               ret = __vb2_buf_begin_cpu_access(vb);
> > +               if (ret) {
> > +                       dprintk(q, 1, "failed to begin CPU access to dmabuf: %d\n",
> > +                               ret);
> > +                       call_void_vb_qop(vb, buf_cleanup, vb);
> > +                       goto err_put_vb2_buf;
> > +               }
> > +       }
> > +
> >         ret = call_vb_qop(vb, buf_prepare, vb);
> >         if (ret) {
> >                 dprintk(q, 1, "buffer preparation failed\n");
> > +               if (q->uses_cpu_access)
> > +                       __vb2_buf_end_cpu_access(vb, vb->num_planes);
> >                 call_void_vb_qop(vb, buf_cleanup, vb);
> >                 goto err_put_vb2_buf;
> >         }
> > @@ -1756,6 +1831,8 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
> >
> >                 __vb2_buf_mem_finish(vb);
> >                 call_void_vb_qop(vb, buf_finish, vb);
> > +               if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
> > +                       __vb2_buf_end_cpu_access(vb, vb->num_planes);
> >                 vb->prepared = 0;
> >         }
> >
> > @@ -2160,6 +2237,8 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
> >         }
> >
> >         call_void_vb_qop(vb, buf_finish, vb);
> > +       if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
> > +               __vb2_buf_end_cpu_access(vb, vb->num_planes);
> >         vb->prepared = 0;
> >
> >         if (pindex)
> > @@ -2296,6 +2375,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> >
> >                 if (vb->prepared) {
> >                         call_void_vb_qop(vb, buf_finish, vb);
> > +                       if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
> > +                               __vb2_buf_end_cpu_access(vb, vb->num_planes);
> >                         vb->prepared = 0;
> >                 }
> >                 __vb2_dqbuf(vb);
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > index 9ce6284cd5f26..68c146c043fb1 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > @@ -768,7 +768,8 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
> >  }
> >
> >  static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> > -                                 struct dma_buf *dbuf, unsigned long size)
> > +                                 struct dma_buf *dbuf, unsigned long size,
> > +                                 enum dma_data_direction dma_dir)
> >  {
> >         struct vb2_dc_buf *buf;
> >         struct dma_buf_attachment *dba;
> > @@ -794,7 +795,7 @@ static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> >                 return dba;
> >         }
> >
> > -       buf->dma_dir = vb->vb2_queue->dma_dir;
> > +       buf->dma_dir = dma_dir;
> >         buf->size = size;
> >         buf->db_attach = dba;
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > index b1d0695cda260..ab03f9d31391a 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > @@ -616,7 +616,8 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
> >  }
> >
> >  static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> > -                                     struct dma_buf *dbuf, unsigned long size)
> > +                                     struct dma_buf *dbuf, unsigned long size,
> > +                                     enum dma_data_direction dma_dir)
> >  {
> >         struct vb2_dma_sg_buf *buf;
> >         struct dma_buf_attachment *dba;
> > @@ -640,7 +641,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> >                 return dba;
> >         }
> >
> > -       buf->dma_dir = vb->vb2_queue->dma_dir;
> > +       buf->dma_dir = dma_dir;
> >         buf->size = size;
> >         buf->db_attach = dba;
> >         buf->vb = vb;
> > diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> > index 63f023696f3ee..c38faa0188bc9 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> > @@ -402,7 +402,8 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
> >  static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
> >                                        struct device *dev,
> >                                        struct dma_buf *dbuf,
> > -                                      unsigned long size)
> > +                                      unsigned long size,
> > +                                      enum dma_data_direction dma_dir)
> >  {
> >         struct vb2_vmalloc_buf *buf;
> >
> > @@ -414,7 +415,7 @@ static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
> >                 return ERR_PTR(-ENOMEM);
> >
> >         buf->dbuf = dbuf;
> > -       buf->dma_dir = vb->vb2_queue->dma_dir;
> > +       buf->dma_dir = dma_dir;
> >         buf->size = size;
> >
> >         return buf;
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index 4b4f4c15c53a7..8dfe22fa4423c 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -78,7 +78,8 @@ struct vb2_buffer;
> >   *             to the userspace, also optional.
> >   * @attach_dmabuf: attach a shared &struct dma_buf for a hardware operation;
> >   *                used for DMABUF memory types; dev is the alloc device
> > - *                dbuf is the shared dma_buf; returns ERR_PTR() on failure;
> > + *                dbuf is the shared dma_buf; dma_dir is the DMA mapping
> > + *                direction; returns ERR_PTR() on failure;
> >   *                allocator private per-buffer structure on success;
> >   *                this needs to be used for further accesses to the buffer.
> >   * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF
> > @@ -136,7 +137,8 @@ struct vb2_mem_ops {
> >         void            *(*attach_dmabuf)(struct vb2_buffer *vb,
> >                                           struct device *dev,
> >                                           struct dma_buf *dbuf,
> > -                                         unsigned long size);
> > +                                         unsigned long size,
> > +                                         enum dma_data_direction dma_dir);
> >         void            (*detach_dmabuf)(void *buf_priv);
> >         int             (*map_dmabuf)(void *buf_priv);
> >         void            (*unmap_dmabuf)(void *buf_priv);
> > @@ -510,6 +512,11 @@ struct vb2_buf_ops {
> >   *             ->finish().
> >   * @non_coherent_mem: when set queue will attempt to allocate buffers using
> >   *             non-coherent memory.
> > + * @uses_cpu_access: the driver transfers buffer data with the CPU instead of
> > + *             DMA. For imported dma-bufs, videobuf2 brackets the interval from
> > + *             buf_prepare through buf_finish with dma_buf_begin_cpu_access()
> > + *             and dma_buf_end_cpu_access(). Imported attachments use
> > + *             bidirectional DMA mappings.
> >   * @lock:      pointer to a mutex that protects the &struct vb2_queue. The
> >   *             driver must set this to a mutex to let the v4l2 core serialize
> >   *             the queuing ioctls. This lock is used when waiting for a new
> > @@ -614,6 +621,7 @@ struct vb2_queue {
> >         unsigned int                    uses_requests:1;
> >         unsigned int                    allow_cache_hints:1;
> >         unsigned int                    non_coherent_mem:1;
> > +       unsigned int                    uses_cpu_access:1;
> >
> >         struct mutex                    *lock;
> >         void                            *owner;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]

Hi,

Le mercredi 21 octobre 2026 à 20:59 +0300, Laurent Pinchart a écrit :
> > It sounds like this series focuses on uvcvideo and vivid, which likely
> > means that 1) alone would be enough to fix the original problem. WDYT?
> 
> I think the dma-buf API doesn't give us what we need. Cleaning the CPU
> cache through end_cpu_access() when the producer is done writting to the
> buffer is right in some use cases, but not in all of them. It assumes
> that the next consumer will access the data through DMA, which isn't
> always the case. What I think we need is to track the state of the
> cache(s) in the dma_buf, and perform cache management operations
> accordingly when needed. In the uvcvideo -> display use case for
> instance, the dma_buf should track that the CPU cache is dirty when
> finishing CPU access, and should be cleaned on the display side before
> performing the DMA. In a use case where the producer would DMA to the
> buffer, cache management would be skipped as the dma_buf would see that
> the cache is not dirty.

I also second this as the only viable option. Its not only about uvcvideo, but
any memory imported by a driver doing DMA (which is not also the exporter). That
includes udmabuf usage. These days, cache artifacts are visible with Intel
display controllers, and Intel mobile GPU drivers. Note that GStreamer properly
signal CPU read/write using DMABuf sync ioctl.

For most Intel laptop user with GStreamer 1.28, these example produce artifacts
on movement. To me, the solution is to track the cache state according to the
access signalling (either userspace or driver), and let DMA user/consumer decide
if a cache flush is needed. There is a obvious challenge in keeping this state
coherent whenever there is multiple consumer, of if some broken application do
concurrent write. Result for the last will always be undefined, but the dmabuf
CPU cache state should not be undefined.

UVC reproducer:
  gst-launch-1.0 v4l2src ! waylandsink

udmabuf reproducer:
  gst-launch-1.0 videotestsrc pattern=ball ! waylandsink

Nicolas

p.s. since Intel GPU these days is also affected, you can swap waylandsink with
glimagesink and you will also see artifacts. One may blame Intel, I believe they
made the choice of performance over correctness in absence of capable HW or
dmabuf signalling.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS

[Karl Mehltretter]

Hello Tomasz,

On Thu, Oct 22, 2026 at 01:00:19AM +0100, Tomasz Figa wrote:
> > +               __vb2_buf_mem_finish(vb);
> > +               call_void_vb_qop(vb, buf_finish, vb);
> > +               vb->prepared = 0;
> > +       }
> > +
> 
> __vb2_queue_cancel() does much more than that, especially all the
> media request handling. In addition, this open coded handling just
> causes code duplication.
> 
> Would it make more sense to refactor the relevant part of the buffer
> cancelation loop from __vb2_queue_cancel() into a helper function and
> call it here instead?
> 

Yes that makes sense. The complete per-buffer cancellation tail can be
moved into a shared helper, while keeping the REMOVE_BUFS range check
before any buffer is changed.

I can then send patch 1 separately from the DMA-BUF access work.

Thanks,
Karl

Re: [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

[Karl Mehltretter]

On Thu, Oct 22, 2026 at 01:20:45AM +0100, Tomasz Figa wrote:
> On Mon, Oct 19, 2026 at 4:14 PM Karl Mehltretter <kmehltretter@gmail.com> wrote:
> > Add a queue flag to do so from buf_prepare() through buf_finish(). Use
> > read access for output and write access for capture. Map imports
> > bidirectionally because CPU and device access directions are opposite.
> > Pass the mapping direction to attach_dmabuf() so USERPTR and MMAP keep
> > the queue's normal direction.
> 
> This isn't right. For the regular vb2 mem ops (dma-sg, dma-contig), a
> buffer in the prepared state is expected to be ready for hardware
> usage, meaning only the hardware can access it until the buffer is
> finished. The vmalloc ops are special in a way that CPU is the
> "hardware".

Agreed. vivid can select vb2_dma_contig_memops while patch 4 enables
uses_cpu_access unconditionally.

Automatic handling belongs in vb2_vmalloc_memops. Regular DMA memops
need separate handling.

> > Keep this opt-in because some vb2-vmalloc drivers pass buffers to
> > hardware. End access in process context because DMA-BUF callbacks may
> > sleep.
> 
> That is at least unexpected, but also probably wrong (I haven't
> checked all the drivers) - the vmalloc mem ops are expected to be used
> by drivers that only access the buffer contents from the CPU.

One current exception is AMD ISP4. It combines VB2_DMABUF with
vb2_vmalloc_memops, imports and pins the DMA-BUF through amdgpu, and
passes the resulting GPU address to ISP firmware.

> Putting all the above together, what I think we need is:
>  1) prepare/finish ops in vb2_vmalloc_memops to invoke the
> dma_buf_begin/end_cpu_access() API if the buffer is backed by a
> DMA-buf,
>  2) some vb2 API for drivers using the regular (dma-) memops to invoke
> the dma_buf_begin/end_cpu_access() API for them when they need to
> perform some CPU accesses to the buffers - typically they would do
> that in their vb2_ops::buf_prepare/finish callbacks, but definitelly
> must not after the buffer is already prepared.
> 
> It sounds like this series focuses on uvcvideo and vivid, which likely
> means that 1) alone would be enough to fix the original problem. WDYT?

1) alone would cover the original vmalloc problem.

But today it would also mark the ISP4 hardware interval as CPU access.

The memops finish() callback may also run from vb2_buffer_done() in
atomic context, while dma_buf_end_cpu_access() may sleep.

The callbacks are void, so errors cannot be returned.

Should ISP4 move to different memops, or should CPU access remain
opt-in for vb2-vmalloc queues?

Thanks,
Karl

Re: [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access

[Karl Mehltretter]

On Wed, Oct 21, 2026 at 05:03:19PM +0100, Nicolas Dufresne wrote:
> > I think the dma-buf API doesn't give us what we need. Cleaning the CPU
> > cache through end_cpu_access() when the producer is done writting to the
> > buffer is right in some use cases, but not in all of them. It assumes
> > that the next consumer will access the data through DMA, which isn't
> > always the case. What I think we need is to track the state of the
> > cache(s) in the dma_buf, and perform cache management operations
> > accordingly when needed. In the uvcvideo -> display use case for
> > instance, the dma_buf should track that the CPU cache is dirty when
> > finishing CPU access, and should be cleaned on the display side before
> > performing the DMA. In a use case where the producer would DMA to the
> > buffer, cache management would be skipped as the dma_buf would see that
> > the cache is not dirty.
> 
> I also second this as the only viable option. Its not only about uvcvideo, but
> any memory imported by a driver doing DMA (which is not also the exporter). That
> includes udmabuf usage. These days, cache artifacts are visible with Intel
> display controllers, and Intel mobile GPU drivers. Note that GStreamer properly
> signal CPU read/write using DMABuf sync ioctl.
> 
> For most Intel laptop user with GStreamer 1.28, these example produce artifacts
> on movement. To me, the solution is to track the cache state according to the
> access signalling (either userspace or driver), and let DMA user/consumer decide
> if a cache flush is needed. There is a obvious challenge in keeping this state
> coherent whenever there is multiple consumer, of if some broken application do
> concurrent write. Result for the last will always be undefined, but the dmabuf
> CPU cache state should not be undefined.
> 

For a persistently mapped attachment, where would the transition to
device access happen? 

dma_buf_map_attachment() may only run once, while the importer can
submit the buffer many times. Would this need a new per-attachment
operation before each submission, or can an existing event be used?

I tested that distinction with one persistent V3D attachment on my Pi
400 and Pi 500+:

- If end_cpu_access() only recorded dirty state, with no sync before
  submission, V3D read stale data in all 64 frames on both boards.

- Synchronizing the attachment immediately before submission produced
  64/64 matching frames on both boards.

This only covers V3D, but it seems that a dirty bit still needs a
device-side use event. Is that the model you had in mind?

Thanks,
Karl