Rename __qbuf_*() functions which are specific to a buffer type as __prepare_*() which matches with what they do. The naming was there for historical reasons; the purpose of the functions was changed without renaming them.

Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com Acked-by: Hans Verkuil hans.verkuil@cisco.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/v4l2-core/videobuf2-core.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 94afbbf9..8df680d 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -956,9 +956,9 @@ void vb2_discard_done(struct vb2_queue *q) EXPORT_SYMBOL_GPL(vb2_discard_done);

/** - * __qbuf_mmap() - handle qbuf of an MMAP buffer + * __prepare_mmap() - prepare an MMAP buffer */ -static int __qbuf_mmap(struct vb2_buffer *vb, const void *pb) +static int __prepare_mmap(struct vb2_buffer *vb, const void *pb) { int ret = 0;

@@ -969,9 +969,9 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const void *pb) }

/** - * __qbuf_userptr() - handle qbuf of a USERPTR buffer + * __prepare_userptr() - prepare a USERPTR buffer */ -static int __qbuf_userptr(struct vb2_buffer *vb, const void *pb) +static int __prepare_userptr(struct vb2_buffer *vb, const void *pb) { struct vb2_plane planes[VB2_MAX_PLANES]; struct vb2_queue *q = vb->vb2_queue; @@ -1087,9 +1087,9 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const void *pb) }

/** - * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer + * __prepare_dmabuf() - prepare a DMABUF buffer */ -static int __qbuf_dmabuf(struct vb2_buffer *vb, const void *pb) +static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb) { struct vb2_plane planes[VB2_MAX_PLANES]; struct vb2_queue *q = vb->vb2_queue; @@ -1255,13 +1255,13 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb)

switch (q->memory) { case VB2_MEMORY_MMAP: - ret = __qbuf_mmap(vb, pb); + ret = __prepare_mmap(vb, pb); break; case VB2_MEMORY_USERPTR: - ret = __qbuf_userptr(vb, pb); + ret = __prepare_userptr(vb, pb); break; case VB2_MEMORY_DMABUF: - ret = __qbuf_dmabuf(vb, pb); + ret = __prepare_dmabuf(vb, pb); break; default: WARN(1, "Invalid queue type\n");

The cache synchronisation may be a time consuming operation and thus not best performed in an interrupt which is a typical context for vb2_buffer_done() calls. This may consume up to tens of ms on some machines, depending on the buffer size.

Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com Acked-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 8bf3369..e866115 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -889,7 +889,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) { struct vb2_queue *q = vb->vb2_queue; unsigned long flags; - unsigned int plane;

if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE)) return; @@ -910,10 +909,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) dprintk(4, "done processing on buffer %d, state: %d\n", vb->index, state);

- /* sync buffers */ - for (plane = 0; plane < vb->num_planes; ++plane) - call_void_memop(vb, finish, vb->planes[plane].mem_priv); - spin_lock_irqsave(&q->done_lock, flags); if (state == VB2_BUF_STATE_QUEUED || state == VB2_BUF_STATE_REQUEUEING) { @@ -1573,6 +1568,10 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)

vb->state = VB2_BUF_STATE_DEQUEUED;

+ /* sync buffers */ + for (i = 0; i < vb->num_planes; ++i) + call_void_memop(vb, finish, vb->planes[i].mem_priv); + /* unmap DMABUF buffer */ if (q->memory == VB2_MEMORY_DMABUF) for (i = 0; i < vb->num_planes; ++i) {

The V4L2_BUF_FLAG_NO_CACHE_INVALIDATE and V4L2_BUF_FLAG_NO_CACHE_CLEAN buffer flags are currently not used by the kernel. Replace the definitions by a single V4L2_BUF_FLAG_NO_CACHE_SYNC flag to be used by further patches.

Different cache architectures should not be visible to the user space which can make no meaningful use of the differences anyway. In case a device can make use of non-coherent memory accesses, the necessary cache operations depend on the CPU architecture and the buffer type, not the requests of the user. The cache operation itself may be skipped on the user's request which was the purpose of the two flags.

On ARM the invalidate and clean are separate operations whereas on x86(-64) the two are a single operation (flush). Whether the hardware uses the buffer for reading (V4L2_BUF_TYPE_*_OUTPUT*) or writing (V4L2_BUF_TYPE_*CAPTURE*) already defines the required cache operation (clean and invalidate, respectively). No user input is required.

Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com Acked-by: Hans Verkuil hans.verkuil@cisco.com --- Documentation/media/uapi/v4l/buffer.rst | 24 ++++++++-------------- .../media/uapi/v4l/vidioc-prepare-buf.rst | 5 ++--- include/trace/events/v4l2.h | 3 +-- include/uapi/linux/videodev2.h | 7 +++++-- 4 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index ae6ee73..9eb42bd 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -559,23 +559,17 @@ Buffer Flags :ref:`VIDIOC_PREPARE_BUF <VIDIOC_QBUF>`, :ref:`VIDIOC_QBUF` or :ref:`VIDIOC_DQBUF <VIDIOC_QBUF>` ioctl is called. - * .. _`V4L2-BUF-FLAG-NO-CACHE-INVALIDATE`: + * .. _`V4L2-BUF-FLAG-NO-CACHE-SYNC`:

- - ``V4L2_BUF_FLAG_NO_CACHE_INVALIDATE`` + - ``V4L2_BUF_FLAG_NO_CACHE_SYNC`` - 0x00000800 - - Caches do not have to be invalidated for this buffer. Typically - applications shall use this flag if the data captured in the - buffer is not going to be touched by the CPU, instead the buffer - will, probably, be passed on to a DMA-capable hardware unit for - further processing or output. - * .. _`V4L2-BUF-FLAG-NO-CACHE-CLEAN`: - - - ``V4L2_BUF_FLAG_NO_CACHE_CLEAN`` - - 0x00001000 - - Caches do not have to be cleaned for this buffer. Typically - applications shall use this flag for output buffers if the data in - this buffer has not been created by the CPU but by some - DMA-capable unit, in which case caches have not been used. + - Do not perform CPU cache synchronisation operations when the buffer is + queued or dequeued. The user is responsible for the correct use of + this flag. It should be only used when the buffer is not accessed + using the CPU, e.g. the buffer is written to by a hardware block and + then read by another one, in which case the flag should be set in both + :ref:`VIDIOC_QBUF` and :ref:`VIDIOC_DQBUF` ioctls. This flag has no + effect on some devices / architectures. * .. _`V4L2-BUF-FLAG-LAST`:

- ``V4L2_BUF_FLAG_LAST`` diff --git a/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst b/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst index bdcfd9f..80aeb7e 100644 --- a/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst +++ b/Documentation/media/uapi/v4l/vidioc-prepare-buf.rst @@ -36,9 +36,8 @@ pass ownership of the buffer to the driver before actually enqueuing it, using the :ref:`VIDIOC_QBUF` ioctl, and to prepare it for future I/O. Such preparations may include cache invalidation or cleaning. Performing them in advance saves time during the actual I/O. In case such cache -operations are not required, the application can use one of -``V4L2_BUF_FLAG_NO_CACHE_INVALIDATE`` and -``V4L2_BUF_FLAG_NO_CACHE_CLEAN`` flags to skip the respective step. +operations are not required, the application can use the +``V4L2_BUF_FLAG_NO_CACHE_SYNC`` flag to skip the cache synchronization step.

The struct :c:type:`v4l2_buffer` structure is specified in :ref:`buffer`. diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h index b3a85b3..3d5897d 100644 --- a/include/trace/events/v4l2.h +++ b/include/trace/events/v4l2.h @@ -81,8 +81,7 @@ SHOW_FIELD { V4L2_BUF_FLAG_ERROR, "ERROR" }, \ { V4L2_BUF_FLAG_TIMECODE, "TIMECODE" }, \ { V4L2_BUF_FLAG_PREPARED, "PREPARED" }, \ - { V4L2_BUF_FLAG_NO_CACHE_INVALIDATE, "NO_CACHE_INVALIDATE" }, \ - { V4L2_BUF_FLAG_NO_CACHE_CLEAN, "NO_CACHE_CLEAN" }, \ + { V4L2_BUF_FLAG_NO_CACHE_SYNC, "NO_CACHE_SYNC" }, \ { V4L2_BUF_FLAG_TIMESTAMP_MASK, "TIMESTAMP_MASK" }, \ { V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN, "TIMESTAMP_UNKNOWN" }, \ { V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC, "TIMESTAMP_MONOTONIC" }, \ diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 2b8feb8..2e046bb 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -939,8 +939,11 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_TIMECODE 0x00000100 /* Buffer is prepared for queuing */ #define V4L2_BUF_FLAG_PREPARED 0x00000400 -/* Cache handling flags */ -#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x00000800 +/* Cache sync hint */ +#define V4L2_BUF_FLAG_NO_CACHE_SYNC 0x00000800 +/* DEPRECATED. THIS WILL BE REMOVED IN THE FUTURE! */ +#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE V4L2_BUF_FLAG_NO_CACHE_SYNC +/* DEPRECATED. THIS WILL BE REMOVED IN THE FUTURE! */ #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x00001000 /* Timestamp type */ #define V4L2_BUF_FLAG_TIMESTAMP_MASK 0x0000e000

attrs used to be a pointer and the caller of vb2_dc_alloc() could optionally provide it, or NULL. This was when struct dma_attrs was used to describe DMA attributes rather than an unsigned long value. There is no longer a need to maintain the condition, assign the value unconditionally. There is no functional difference because the memory was initialised to zero anyway.

Fixes: 00085f1e ("dma-mapping: use unsigned long for dma_attrs") Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com --- drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 30082a4..a8a46a8 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -149,8 +149,7 @@ static void *vb2_dc_alloc(struct device *dev, unsigned long attrs, if (!buf) return ERR_PTR(-ENOMEM);

- if (attrs) - buf->attrs = attrs; + buf->attrs = attrs; buf->cookie = dma_alloc_attrs(dev, size, &buf->dma_addr, GFP_KERNEL | gfp_flags, buf->attrs); if (!buf->cookie) {

The scatterlist table is needed in the vast majority of the cases. Allocate struct sg_table as part of the struct. This has the benefit of making managing the buffer data structure allocation, setup and release more simple.

Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com --- drivers/media/v4l2-core/videobuf2-dma-contig.c | 83 ++++++++++---------------- 1 file changed, 32 insertions(+), 51 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 22636cd..0afc3da 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -30,6 +30,7 @@ struct vb2_dc_buf { dma_addr_t dma_addr; unsigned long attrs; enum dma_data_direction dma_dir; + struct sg_table __dma_sgt; struct sg_table *dma_sgt;

/* MMAP related */ @@ -127,10 +128,9 @@ static void vb2_dc_put(void *buf_priv) if (!refcount_dec_and_test(&buf->refcount)) return;

- if (buf->dma_sgt) { + if (buf->dma_sgt) sg_free_table(buf->dma_sgt); - kfree(buf->dma_sgt); - } + dma_free_attrs(buf->dev, buf->size, buf->cookie, buf->dma_addr, buf->attrs); put_device(buf->dev); @@ -364,26 +364,6 @@ static struct dma_buf_ops vb2_dc_dmabuf_ops = { .release = vb2_dc_dmabuf_ops_release, };

-static struct sg_table *vb2_dc_get_base_sgt(struct vb2_dc_buf *buf) -{ - int ret; - struct sg_table *sgt; - - sgt = kmalloc(sizeof(*sgt), GFP_KERNEL); - if (!sgt) - return NULL; - - ret = dma_get_sgtable_attrs(buf->dev, sgt, buf->cookie, buf->dma_addr, - buf->size, buf->attrs); - if (ret < 0) { - dev_err(buf->dev, "failed to get scatterlist from DMA API\n"); - kfree(sgt); - return NULL; - } - - return sgt; -} - static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, unsigned long flags) { struct vb2_dc_buf *buf = buf_priv; @@ -395,11 +375,19 @@ static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv, unsigned long flags) exp_info.flags = flags; exp_info.priv = buf;

- if (!buf->dma_sgt) - buf->dma_sgt = vb2_dc_get_base_sgt(buf); + if (!buf->dma_sgt) { + int ret;

- if (!buf->dma_sgt) - return NULL; + ret = dma_get_sgtable_attrs(buf->dev, &buf->__dma_sgt, + buf->cookie, buf->dma_addr, + buf->size, buf->attrs); + if (ret < 0) { + dev_err(buf->dev, "failed to get scatterlist from DMA API\n"); + return NULL; + } + + buf->dma_sgt = &buf->__dma_sgt; + }

dbuf = dma_buf_export(&exp_info); if (IS_ERR(dbuf)) @@ -435,7 +423,6 @@ static void vb2_dc_put_userptr(void *buf_priv) for (i = 0; i < frame_vector_count(buf->vec); i++) set_page_dirty_lock(pages[i]); sg_free_table(sgt); - kfree(sgt); } vb2_destroy_framevec(buf->vec); kfree(buf); @@ -481,7 +468,6 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, unsigned long offset; int n_pages, i; int ret = 0; - struct sg_table *sgt; unsigned long contig_size; unsigned long dma_align = dma_get_cache_alignment();

@@ -529,33 +515,31 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, goto out; }

- sgt = kzalloc(sizeof(*sgt), GFP_KERNEL); - if (!sgt) { - pr_err("failed to allocate sg table\n"); - ret = -ENOMEM; - goto fail_pfnvec; - } - - ret = sg_alloc_table_from_pages(sgt, frame_vector_pages(vec), n_pages, - offset, size, GFP_KERNEL); + ret = sg_alloc_table_from_pages(&buf->__dma_sgt, + frame_vector_pages(vec), n_pages, + offset, size, GFP_KERNEL); if (ret) { pr_err("failed to initialize sg table\n"); - goto fail_sgt; + goto fail_pfnvec; }

+ buf->dma_sgt = &buf->__dma_sgt; + /* * No need to sync to the device, this will happen later when the * prepare() memop is called. */ - sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (sgt->nents <= 0) { + buf->dma_sgt->nents = dma_map_sg_attrs(buf->dev, buf->dma_sgt->sgl, + buf->dma_sgt->orig_nents, + buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); + if (buf->dma_sgt->nents <= 0) { pr_err("failed to map scatterlist\n"); ret = -EIO; goto fail_sgt_init; }

- contig_size = vb2_dc_get_contiguous_size(sgt); + contig_size = vb2_dc_get_contiguous_size(buf->dma_sgt); if (contig_size < size) { pr_err("contiguous mapping is too small %lu/%lu\n", contig_size, size); @@ -563,22 +547,19 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, goto fail_map_sg; }

- buf->dma_addr = sg_dma_address(sgt->sgl); - buf->dma_sgt = sgt; + buf->dma_addr = sg_dma_address(buf->dma_sgt->sgl); out: buf->size = size;

return buf;

fail_map_sg: - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sg_attrs(buf->dev, buf->dma_sgt->sgl, + buf->dma_sgt->orig_nents, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC);

fail_sgt_init: - sg_free_table(sgt); - -fail_sgt: - kfree(sgt); + sg_free_table(buf->dma_sgt);

fail_pfnvec: vb2_destroy_framevec(vec);

The scatterlist should always be present when the cache would need to be flushed. Each buffer type has its own means to provide that. Add WARN_ON_ONCE() to check the scatterist exists.

Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com Acked-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 8b0298a..f572911 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -101,7 +101,7 @@ static void vb2_dc_prepare(void *buf_priv) * DMABUF exporter will flush the cache for us; only USERPTR * and MMAP buffers with non-coherent memory will be flushed. */ - if (buf->attrs & DMA_ATTR_NON_CONSISTENT) + if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt)) dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); } @@ -115,7 +115,7 @@ static void vb2_dc_finish(void *buf_priv) * DMABUF exporter will flush the cache for us; only USERPTR * and MMAP buffers with non-coherent memory will be flushed. */ - if (buf->attrs & DMA_ATTR_NON_CONSISTENT) + if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt)) dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); } @@ -363,7 +363,7 @@ static int vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, * DMABUF exporter will flush the cache for us; only USERPTR * and MMAP buffers with non-coherent memory will be flushed. */ - if (buf->attrs & DMA_ATTR_NON_CONSISTENT) + if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt)) dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);

@@ -380,7 +380,7 @@ static int vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, * DMABUF exporter will flush the cache for us; only USERPTR * and MMAP buffers with non-coherent memory will be flushed. */ - if (buf->attrs & DMA_ATTR_NON_CONSISTENT) + if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt)) dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);

From: Samu Onkalo samu.onkalo@intel.com

The user may request to the driver (vb2) to skip the cache maintenance operations in case the buffer does not need cache synchronisation, e.g. in cases where the buffer is passed between hardware blocks without it being touched by the CPU.

Also document that the prepare and finish vb2_mem_ops might not get called every time the buffer ownership changes between the kernel and the user space.

Signed-off-by: Samu Onkalo samu.onkalo@intel.com Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com Acked-by: Hans Verkuil hans.verkuil@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 101 +++++++++++++++++++++---------- drivers/media/v4l2-core/videobuf2-v4l2.c | 14 ++++- include/media/videobuf2-core.h | 23 ++++--- 3 files changed, 97 insertions(+), 41 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index c659b64..fbe0db1 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -189,6 +189,28 @@ static void __vb2_queue_cancel(struct vb2_queue *q); static void __enqueue_in_driver(struct vb2_buffer *vb);

/** + * __mem_prepare_planes() - call finish mem op for all planes of the buffer + */ +static void __mem_prepare_planes(struct vb2_buffer *vb) +{ + unsigned int plane; + + for (plane = 0; plane < vb->num_planes; ++plane) + call_void_memop(vb, prepare, vb->planes[plane].mem_priv); +} + +/** + * __mem_finish_planes() - call finish mem op for all planes of the buffer + */ +static void __mem_finish_planes(struct vb2_buffer *vb) +{ + unsigned int plane; + + for (plane = 0; plane < vb->num_planes; ++plane) + call_void_memop(vb, finish, vb->planes[plane].mem_priv); +} + +/** * __vb2_buf_mem_alloc() - allocate video memory for the given buffer */ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb) @@ -953,20 +975,29 @@ EXPORT_SYMBOL_GPL(vb2_discard_done); /** * __prepare_mmap() - prepare an MMAP buffer */ -static int __prepare_mmap(struct vb2_buffer *vb, const void *pb) +static int __prepare_mmap(struct vb2_buffer *vb, const void *pb, + bool no_cache_sync) { - int ret = 0; + int ret;

- if (pb) + if (pb) { ret = call_bufop(vb->vb2_queue, fill_vb2_buffer, vb, pb, vb->planes); - return ret ? ret : call_vb_qop(vb, buf_prepare, vb); + if (ret) + return ret; + } + + if (!no_cache_sync) + __mem_prepare_planes(vb); + + return call_vb_qop(vb, buf_prepare, vb); }

/** * __prepare_userptr() - prepare a USERPTR buffer */ -static int __prepare_userptr(struct vb2_buffer *vb, const void *pb) +static int __prepare_userptr(struct vb2_buffer *vb, const void *pb, + bool no_cache_sync) { struct vb2_plane planes[VB2_MAX_PLANES]; struct vb2_queue *q = vb->vb2_queue; @@ -1057,6 +1088,11 @@ static int __prepare_userptr(struct vb2_buffer *vb, const void *pb) dprintk(1, "buffer initialization failed\n"); goto err; } + + /* This is new buffer memory --- always synchronise cache. */ + __mem_prepare_planes(vb); + } else if (!no_cache_sync) { + __mem_prepare_planes(vb); }

ret = call_vb_qop(vb, buf_prepare, vb); @@ -1084,7 +1120,8 @@ static int __prepare_userptr(struct vb2_buffer *vb, const void *pb) /** * __prepare_dmabuf() - prepare a DMABUF buffer */ -static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb) +static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb, + bool no_cache_sync) { struct vb2_plane planes[VB2_MAX_PLANES]; struct vb2_queue *q = vb->vb2_queue; @@ -1199,6 +1236,11 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, const void *pb) dprintk(1, "buffer initialization failed\n"); goto err; } + + /* This is new buffer memory --- always synchronise cache. */ + __mem_prepare_planes(vb); + } else if (!no_cache_sync) { + __mem_prepare_planes(vb); }

ret = call_vb_qop(vb, buf_prepare, vb); @@ -1231,10 +1273,10 @@ static void __enqueue_in_driver(struct vb2_buffer *vb) call_void_vb_qop(vb, buf_queue, vb); }

-static int __buf_prepare(struct vb2_buffer *vb, const void *pb) +static int __buf_prepare(struct vb2_buffer *vb, const void *pb, + bool no_cache_sync) { struct vb2_queue *q = vb->vb2_queue; - unsigned int plane; int ret;

if (q->error) { @@ -1246,13 +1288,13 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb)

switch (q->memory) { case VB2_MEMORY_MMAP: - ret = __prepare_mmap(vb, pb); + ret = __prepare_mmap(vb, pb, no_cache_sync); break; case VB2_MEMORY_USERPTR: - ret = __prepare_userptr(vb, pb); + ret = __prepare_userptr(vb, pb, no_cache_sync); break; case VB2_MEMORY_DMABUF: - ret = __prepare_dmabuf(vb, pb); + ret = __prepare_dmabuf(vb, pb, no_cache_sync); break; default: WARN(1, "Invalid queue type\n"); @@ -1265,16 +1307,13 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb) return ret; }

- /* sync buffers */ - for (plane = 0; plane < vb->num_planes; ++plane) - call_void_memop(vb, prepare, vb->planes[plane].mem_priv); - vb->state = VB2_BUF_STATE_PREPARED;

return 0; }

-int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb) +int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb, + bool no_cache_sync) { struct vb2_buffer *vb; int ret; @@ -1286,7 +1325,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb) return -EINVAL; }

- ret = __buf_prepare(vb, pb); + ret = __buf_prepare(vb, pb, no_cache_sync); if (ret) return ret;

@@ -1362,7 +1401,8 @@ static int vb2_start_streaming(struct vb2_queue *q) return ret; }

-int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb) +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, + bool no_cache_sync) { struct vb2_buffer *vb; int ret; @@ -1371,7 +1411,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)

switch (vb->state) { case VB2_BUF_STATE_DEQUEUED: - ret = __buf_prepare(vb, pb); + ret = __buf_prepare(vb, pb, no_cache_sync); if (ret) return ret; break; @@ -1557,7 +1597,7 @@ EXPORT_SYMBOL_GPL(vb2_wait_for_all_buffers); /** * __vb2_dqbuf() - bring back the buffer to the DEQUEUED state */ -static void __vb2_dqbuf(struct vb2_buffer *vb) +static void __vb2_dqbuf(struct vb2_buffer *vb, bool no_cache_sync) { struct vb2_queue *q = vb->vb2_queue; unsigned int i; @@ -1568,9 +1608,8 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)

vb->state = VB2_BUF_STATE_DEQUEUED;

- /* sync buffers */ - for (i = 0; i < vb->num_planes; ++i) - call_void_memop(vb, finish, vb->planes[i].mem_priv); + if (!no_cache_sync) + __mem_finish_planes(vb);

/* unmap DMABUF buffer */ if (q->memory == VB2_MEMORY_DMABUF) @@ -1583,7 +1622,7 @@ static void __vb2_dqbuf(struct vb2_buffer *vb) }

int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb, - bool nonblocking) + bool nonblocking, bool no_cache_sync) { struct vb2_buffer *vb = NULL; int ret; @@ -1620,7 +1659,7 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb, trace_vb2_dqbuf(q, vb);

/* go back to dequeued state */ - __vb2_dqbuf(vb); + __vb2_dqbuf(vb, no_cache_sync);

dprintk(1, "dqbuf of buffer %d, with state %d\n", vb->index, vb->state); @@ -1694,7 +1733,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) vb->state = VB2_BUF_STATE_PREPARED; call_void_vb_qop(vb, buf_finish, vb); } - __vb2_dqbuf(vb); + __vb2_dqbuf(vb, false); } }

@@ -2242,7 +2281,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) * Queue all buffers. */ for (i = 0; i < q->num_buffers; i++) { - ret = vb2_core_qbuf(q, i, NULL); + ret = vb2_core_qbuf(q, i, NULL, false); if (ret) goto err_reqbufs; fileio->bufs[i].queued = 1; @@ -2345,7 +2384,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ /* * Call vb2_dqbuf to get buffer back. */ - ret = vb2_core_dqbuf(q, &index, NULL, nonblock); + ret = vb2_core_dqbuf(q, &index, NULL, nonblock, false); dprintk(5, "vb2_dqbuf result: %d\n", ret); if (ret) return ret; @@ -2421,7 +2460,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_

if (copy_timestamp) b->timestamp = ktime_get_ns(); - ret = vb2_core_qbuf(q, index, NULL); + ret = vb2_core_qbuf(q, index, NULL, false); dprintk(5, "vb2_dbuf result: %d\n", ret); if (ret) return ret; @@ -2507,7 +2546,7 @@ static int vb2_thread(void *data) } else { call_void_qop(q, wait_finish, q); if (!threadio->stop) - ret = vb2_core_dqbuf(q, &index, NULL, 0); + ret = vb2_core_dqbuf(q, &index, NULL, 0, false); call_void_qop(q, wait_prepare, q); dprintk(5, "file io: vb2_dqbuf result: %d\n", ret); if (!ret) @@ -2524,7 +2563,7 @@ static int vb2_thread(void *data) if (copy_timestamp) vb->timestamp = ktime_get_ns();; if (!threadio->stop) - ret = vb2_core_qbuf(q, vb->index, NULL); + ret = vb2_core_qbuf(q, vb->index, NULL, false); call_void_qop(q, wait_prepare, q); if (ret || threadio->stop) break; diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c index 0c06699..a0c4511 100644 --- a/drivers/media/v4l2-core/videobuf2-v4l2.c +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c @@ -499,8 +499,11 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b) }

ret = vb2_queue_or_prepare_buf(q, b, "prepare_buf"); + if (ret) + return ret;

- return ret ? ret : vb2_core_prepare_buf(q, b->index, b); + return vb2_core_prepare_buf(q, b->index, b, + b->flags & V4L2_BUF_FLAG_NO_CACHE_SYNC); } EXPORT_SYMBOL_GPL(vb2_prepare_buf);

@@ -568,7 +571,11 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) }

ret = vb2_queue_or_prepare_buf(q, b, "qbuf"); - return ret ? ret : vb2_core_qbuf(q, b->index, b); + if (ret) + return ret; + + return vb2_core_qbuf(q, b->index, b, + b->flags & V4L2_BUF_FLAG_NO_CACHE_SYNC); } EXPORT_SYMBOL_GPL(vb2_qbuf);

@@ -586,7 +593,8 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking) return -EINVAL; }

- ret = vb2_core_dqbuf(q, NULL, b, nonblocking); + ret = vb2_core_dqbuf(q, NULL, b, nonblocking, + b->flags & V4L2_BUF_FLAG_NO_CACHE_SYNC);

/* * After calling the VIDIOC_DQBUF V4L2_BUF_FLAG_DONE must be diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 4172f6e..08f1d0e 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -83,10 +83,14 @@ struct vb2_threadio_data; * dmabuf. * @unmap_dmabuf: releases access control to the dmabuf - allocator is notified * that this driver is done using the dmabuf for now. - * @prepare: called every time the buffer is passed from userspace to the - * driver, useful for cache synchronisation, optional. - * @finish: called every time the buffer is passed back from the driver - * to the userspace, also optional. + * @prepare: Called on the plane when the buffer ownership is passed from + * the user space to the kernel and the plane must be cache + * syncronised. The V4L2_BUF_FLAG_NO_CACHE_SYNC buffer flag may + * be used to skip this call. Optional. + * @finish: Called on the plane when the buffer ownership is passed from + * the kernel to the user space and the plane must be cache + * syncronised. The V4L2_BUF_FLAG_NO_CACHE_SYNC buffer flag may + * be used to skip this call. Optional. * @vaddr: return a kernel virtual address to a given memory buffer * associated with the passed private structure or NULL if no * such mapping exists. @@ -696,6 +700,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, * @index: id number of the buffer * @pb: buffer structure passed from userspace to vidioc_prepare_buf * handler in driver + * @no_cache_sync if true, skip cache synchronization * * Should be called from vidioc_prepare_buf ioctl handler of a driver. * The passed buffer should have been verified. @@ -705,7 +710,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, * The return values from this function are intended to be directly returned * from vidioc_prepare_buf handler in driver. */ -int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb); +int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb, + bool no_cache_sync);

/** * vb2_core_qbuf() - Queue a buffer from userspace @@ -714,6 +720,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb); * @index: id number of the buffer * @pb: buffer structure passed from userspace to vidioc_qbuf handler * in driver + * @no_cache_sync if true, skip cache synchronization * * Should be called from vidioc_qbuf ioctl handler of a driver. * The passed buffer should have been verified. @@ -728,7 +735,8 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb); * The return values from this function are intended to be directly returned * from vidioc_qbuf handler in driver. */ -int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb); +int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, + bool no_cache_sync);

/** * vb2_core_dqbuf() - Dequeue a buffer to the userspace @@ -739,6 +747,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb); * @nonblocking: if true, this call will not sleep waiting for a buffer if no * buffers ready for dequeuing are present. Normally the driver * would be passing (file->f_flags & O_NONBLOCK) here + * @no_cache_sync if true, skip cache synchronization * * Should be called from vidioc_dqbuf ioctl handler of a driver. * The passed buffer should have been verified. @@ -755,7 +764,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb); * from vidioc_dqbuf handler in driver. */ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb, - bool nonblocking); + bool nonblocking, bool no_cache_sync);

int vb2_core_streamon(struct vb2_queue *q, unsigned int type); int vb2_core_streamoff(struct vb2_queue *q, unsigned int type);

The patch changes the DMA direction from DMA_FROM_DEVICE to DMA_TO_DEVICE for capture buffers.

The DMA API does not require that any synchronisation is done to the buffer when it is passed to hardware for writing _but_ there's a caveat: the user *must not* have written to the buffer.

The V4L2 API does however not require this. Instead, it requires that the user does not access the buffer since it is queued to the device using VIDIOC_QBUF IOCTL until the buffer is dequeued again using VIDIOC_DQBUF IOCTL.

So in this case we want to ensure there will be no dirty cache lines that could end up to memory possibly after the device has written to the same memory area. What data gets written to the system memory from the cache is not extremely important. Still, an for debugging purposes an application capturing images could fill the buffer with a known pattern which will be overwritten by the device, hence DMA_TO_DEVICE.

If an application can guarantee that it has not written to the buffer, it can specify the V4L2_BUF_FLAG_NO_CACHE_SYNC flag to omit the sync operation.

Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com --- drivers/media/v4l2-core/videobuf2-dma-contig.c | 2 +- drivers/media/v4l2-core/videobuf2-dma-sg.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index f572911..320e53a 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -103,7 +103,7 @@ static void vb2_dc_prepare(void *buf_priv) */ if (buf->attrs & DMA_ATTR_NON_CONSISTENT && !WARN_ON_ONCE(!sgt)) dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir); + DMA_TO_DEVICE); }

static void vb2_dc_finish(void *buf_priv) diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 5662f00..88b2530 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c @@ -206,7 +206,7 @@ static void vb2_dma_sg_prepare(void *buf_priv) */ if (buf->dma_attrs & DMA_ATTR_NON_CONSISTENT) dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir); + DMA_TO_DEVICE); }

static void vb2_dma_sg_finish(void *buf_priv)

Document when should the user specify V4L2_BUF_FLAG_NO_CACHE_SYNC flag.

Signed-off-by: Sakari Ailus sakari.ailus@linux.intel.com --- Documentation/media/uapi/v4l/buffer.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)

diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index 9eb42bd..e1f93dd 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -570,6 +570,27 @@ Buffer Flags then read by another one, in which case the flag should be set in both :ref:`VIDIOC_QBUF` and :ref:`VIDIOC_DQBUF` ioctls. This flag has no effect on some devices / architectures. + + More specifically, this flag causes cache synchronisation to + be skipped for OUTPUT buffers when the buffer is queued. The + flag can be used if the buffer has been previously written to + by hardware but has not been written to by the CPU. + + Additionally, if this flag is specified for a CAPTURE buffer + when it is queued, cache synchronisation is skipped. This + signals that the application can guarantee that it has not + written to the buffer memory since it was last dequeued from + the device. + + Specifying this flag for a CAPTURE buffer when + dequeueing a buffer will skip cache maintenance for the buffer + memory. An application may not access the buffer memory in + that case but it may well be passed onwards to another device + in the system. + + Specifying this flag has no effect when dequeuing an OUTPUT + buffer. + * .. _`V4L2-BUF-FLAG-LAST`:

- ``V4L2_BUF_FLAG_LAST``