linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags
@ 2020-04-24  9:29 Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 01/13] videobuf2: use explicit unsigned int in vb2_queue Sergey Senozhatsky
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky

Hello,

	v5 with fixes and improvements.

I have a simple v4l-compliance patch now (will send it separately) which
tests cache and consistency hints. I ran compliance against the vivid
test driver, which was additionally extended with cache_hints module param:
Trimmed v4l-compliance output:

- vivid with disabled cache hints support

[..]
Buffer ioctls (Input 3):
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
[..]

- vivid with enabled cache hints (cache_hints=1,...)

[..]
Buffer ioctls (Input 3):
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
[..]


v4l-compliance revealed that we cannot reliably report
V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS, it's a bit special.
Let's look at fill_buf_caps()

	if (q->allow_cache_hints && q->io_modes & VB2_MMAP)
		*caps |= V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS;

There are several things here. First, if the queue is not setup yet
(we didn't call driver's ->queue_setup()) then ->allow_cache_hints
is expected to be 0. It's only in the ->queue_setup() that the driver
sets ->allow_cache_hints for those queues that can benefit from cache
management. What this means, is that if one does

	q.init(node->g_type(), memory);
	q.reqbufs(node);
	q.g_capabilities();

the cache hints cap will not be reported. We need to setup the queue

	q.init(node->g_type(), memory);
	q.reqbufs(node, 1);
	q.g_capabilities();

Second. Even if the queue is setup, we still can report wrong cache
hint values. Let's look at the following code

	fill_buf_caps(q, &p->capabilities);
	if (!vb2_queue_allows_cache_hints(q))
		p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
	ret = vb2_core_reqbufs(...);
	return ret;

The thing here is that vb2_core_reqbufs() and vb2_core_create_bufs()
can re-initialize the queue and invoke ->queue_setup(), possibly
changing its memory model, etc. so cache hints cap which we set or
clear before vb2_core_reqbufs() and vb2_core_create_bufs() can become
invalid after we call those functions. It's the same with
``req->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT``, we cannot clear
it before reqbufs()/create_bufs(). Therefore I added two simple
functions which fixup cache hint cap and non_consistent flag after
reqbufs()/create_bufs(). So the code looks like this now:

	fill_buf_caps(q, &p->capabilities);
	ret = vb2_core_reqbufs(...);
	fixup_consistency_attr(q, &p->flags);
	fixup_cache_hints_cap(q, &p->capabilities);
	return ret;


The rest is the pretty much the same.

Previous series:
v4 link: https://lore.kernel.org/lkml/20200302041213.27662-1-senozhatsky@chromium.org/
v3 link: https://lore.kernel.org/lkml/20200226111529.180197-1-senozhatsky@chromium.org
v2 link: https://lore.kernel.org/lkml/20200204025641.218376-1-senozhatsky@chromium.org/
v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@chromium.org/

Series Intro
========================================================================

	This is a reworked version of the vb2 cache hints
(V4L2_BUF_FLAG_NO_CACHE_INVALIDATE / V4L2_BUF_FLAG_NO_CACHE_CLEAN)
support patch series which previsouly was developed by Sakari and
Laurent [0].

The patch set attempts to preserve the existing behvaiour - cache
sync is performed in ->prepare() and ->finish() (unless the buffer
is DMA exported). User space can request “default behavior” override
with cache management hints, which are handled on a per-buffer basis
and should be supplied with v4l2_buffer ->flags during buffer
preparation. There are two possible hints:

- V4L2_BUF_FLAG_NO_CACHE_INVALIDATE
	No cache sync on ->finish()

- V4L2_BUF_FLAG_NO_CACHE_CLEAN
	No cache sync on ->prepare()

In order to keep things on the safe side, we also require driver
to explicitly state which of its queues (if any) support user space
cache management hints (such queues should have ->allow_cache_hints
bit set).

The patch set also (to some extent) simplifies allocators' ->prepare()
and ->finish() callbacks. Namely, we move cache management decision
making to the upper - core - layer. For example, if, previously, we
would have something like this

	vb2_buffer_done()
	  vb2_dc_finish()
	    if (buf->db_attach)
	      return;

where each allocators' ->finish() callback would either bail
out (DMA exported buffer, for instance) or sync, now that "bail
out or sync" decision is made before we call into the allocator.

Along with cache management hints, user space is also able to
adjust queue's memory consistency attributes. Memory consistency
attribute (dma_attrs) is per-queue, yet it plays its role on the
allocator level, when we allocate buffers’ private memory (planes).
For the time being, only one consistency attribute is supported:
DMA_ATTR_NON_CONSISTENT.

[0] https://www.mail-archive.com/linux-media@vger.kernel.org/msg112459.html

Sergey Senozhatsky (13):
  videobuf2: use explicit unsigned int in vb2_queue
  videobuf2: add cache management members
  videobuf2: handle V4L2 buffer cache flags
  videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
  videobuf2: add queue memory consistency parameter
  videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
  videobuf2: factor out planes prepare/finish functions
  videobuf2: do not sync caches when we are allowed not to
  videobuf2: check ->synced flag in prepare() and finish()
  videobuf2: add begin/end cpu_access callbacks to dma-contig
  videobuf2: add begin/end cpu_access callbacks to dma-sg
  videobuf2: don't test db_attach in dma-contig prepare and finish
  media: vivid: add cache_hints module param

 Documentation/admin-guide/media/vivid.rst     |   9 ++
 .../userspace-api/media/v4l/buffer.rst        |  41 +++++-
 .../media/v4l/vidioc-create-bufs.rst          |   7 +-
 .../media/v4l/vidioc-reqbufs.rst              |  20 ++-
 .../media/common/videobuf2/videobuf2-core.c   | 121 +++++++++++++-----
 .../common/videobuf2/videobuf2-dma-contig.c   |  41 +++++-
 .../media/common/videobuf2/videobuf2-dma-sg.c |  38 ++++--
 .../media/common/videobuf2/videobuf2-v4l2.c   |  93 +++++++++++++-
 drivers/media/dvb-core/dvb_vb2.c              |   2 +-
 drivers/media/test-drivers/vivid/vivid-core.c |   9 ++
 drivers/media/test-drivers/vivid/vivid-core.h |   1 +
 .../media/test-drivers/vivid/vivid-meta-cap.c |   5 +
 .../media/test-drivers/vivid/vivid-meta-out.c |   5 +
 .../media/test-drivers/vivid/vivid-sdr-cap.c  |   7 +
 .../test-drivers/vivid/vivid-touch-cap.c      |   5 +
 .../media/test-drivers/vivid/vivid-vbi-cap.c  |   5 +
 .../media/test-drivers/vivid/vivid-vbi-out.c  |   5 +
 .../media/test-drivers/vivid/vivid-vid-cap.c  |   5 +
 .../media/test-drivers/vivid/vivid-vid-out.c  |   5 +
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |   9 +-
 drivers/media/v4l2-core/v4l2-ioctl.c          |   5 +-
 include/media/videobuf2-core.h                |  47 +++++--
 include/uapi/linux/videodev2.h                |  13 +-
 23 files changed, 426 insertions(+), 72 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCHv5 01/13] videobuf2: use explicit unsigned int in vb2_queue
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 02/13] videobuf2: add cache management members Sergey Senozhatsky
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Switch from 'unsigned' to 'unsigned int' so that checkpatch doesn't
complain.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 include/media/videobuf2-core.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f11b96514cf7..9e522bd2acc7 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -558,15 +558,15 @@ struct vb2_queue {
 	unsigned int			io_modes;
 	struct device			*dev;
 	unsigned long			dma_attrs;
-	unsigned			bidirectional:1;
-	unsigned			fileio_read_once:1;
-	unsigned			fileio_write_immediately:1;
-	unsigned			allow_zero_bytesused:1;
-	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
-	unsigned			supports_requests:1;
-	unsigned			requires_requests:1;
-	unsigned			uses_qbuf:1;
-	unsigned			uses_requests:1;
+	unsigned int			bidirectional:1;
+	unsigned int			fileio_read_once:1;
+	unsigned int			fileio_write_immediately:1;
+	unsigned int			allow_zero_bytesused:1;
+	unsigned int		   quirk_poll_must_check_waiting_for_buffers:1;
+	unsigned int			supports_requests:1;
+	unsigned int			requires_requests:1;
+	unsigned int			uses_qbuf:1;
+	unsigned int			uses_requests:1;
 
 	struct mutex			*lock;
 	void				*owner;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 02/13] videobuf2: add cache management members
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 01/13] videobuf2: use explicit unsigned int in vb2_queue Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 03/13] videobuf2: handle V4L2 buffer cache flags Sergey Senozhatsky
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Extend vb2_buffer and vb2_queue structs with cache management
members.

V4L2 UAPI already contains two buffer flags which user-space,
supposedly, can use to control buffer cache sync:

- V4L2_BUF_FLAG_NO_CACHE_INVALIDATE
- V4L2_BUF_FLAG_NO_CACHE_CLEAN

None of these, however, do anything at the moment. This patch
set is intended to change it.

Since user-space cache management hints are supposed to be
implemented on a per-buffer basis we need to extend vb2_buffer
struct with two new members ->need_cache_sync_on_prepare and
->need_cache_sync_on_finish, which will store corresponding
user-space hints.

In order to preserve the existing behaviour, user-space cache
managements flags will be handled only by those drivers that
permit user-space cache hints. That's the purpose of vb2_queue
->allow_cache_hints member. Driver must set ->allow_cache_hints
during queue initialisation to enable cache management hints
mechanism.

Only drivers that set ->allow_cache_hints during queue initialisation
will handle user-space cache management hints. Otherwise hints
will be ignored.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 include/media/videobuf2-core.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 9e522bd2acc7..7f39d9fffc8c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -263,6 +263,10 @@ struct vb2_buffer {
 	 *			after the 'buf_finish' op is called.
 	 * copied_timestamp:	the timestamp of this capture buffer was copied
 	 *			from an output buffer.
+	 * need_cache_sync_on_prepare: when set buffer's ->prepare() function
+	 *			performs cache sync/invalidation.
+	 * need_cache_sync_on_finish: when set buffer's ->finish() function
+	 *			performs cache sync/invalidation.
 	 * queued_entry:	entry on the queued buffers list, which holds
 	 *			all buffers queued from userspace
 	 * done_entry:		entry on the list that stores all buffers ready
@@ -273,6 +277,8 @@ struct vb2_buffer {
 	unsigned int		synced:1;
 	unsigned int		prepared:1;
 	unsigned int		copied_timestamp:1;
+	unsigned int		need_cache_sync_on_prepare:1;
+	unsigned int		need_cache_sync_on_finish:1;
 
 	struct vb2_plane	planes[VB2_MAX_PLANES];
 	struct list_head	queued_entry;
@@ -491,6 +497,9 @@ struct vb2_buf_ops {
  * @uses_requests: requests are used for this queue. Set to 1 the first time
  *		a request is queued. Set to 0 when the queue is canceled.
  *		If this is 1, then you cannot queue buffers directly.
+ * @allow_cache_hints: when set user-space can pass cache management hints in
+ *		order to skip cache flush/invalidation on ->prepare() or/and
+ *		->finish().
  * @lock:	pointer to a mutex that protects the &struct vb2_queue. The
  *		driver can set this to a mutex to let the v4l2 core serialize
  *		the queuing ioctls. If the driver wants to handle locking
@@ -567,6 +576,7 @@ struct vb2_queue {
 	unsigned int			requires_requests:1;
 	unsigned int			uses_qbuf:1;
 	unsigned int			uses_requests:1;
+	unsigned int			allow_cache_hints:1;
 
 	struct mutex			*lock;
 	void				*owner;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 03/13] videobuf2: handle V4L2 buffer cache flags
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 01/13] videobuf2: use explicit unsigned int in vb2_queue Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 02/13] videobuf2: add cache management members Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 04/13] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Set video buffer cache management flags corresponding to V4L2 cache
flags.

Both ->prepare() and ->finish() cache management hints should be
passed during this stage (buffer preparation), because there is
no other way for user-space to tell V4L2 to avoid ->finish() cache
flush.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../media/common/videobuf2/videobuf2-v4l2.c   | 48 +++++++++++++++++++
 include/media/videobuf2-core.h                | 11 +++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index eb5d5db96552..f13851212cc8 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -337,6 +337,53 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b
 	return 0;
 }
 
+static void set_buffer_cache_hints(struct vb2_queue *q,
+				   struct vb2_buffer *vb,
+				   struct v4l2_buffer *b)
+{
+	/*
+	 * DMA exporter should take care of cache syncs, so we can avoid
+	 * explicit ->prepare()/->finish() syncs. For other ->memory types
+	 * we always need ->prepare() or/and ->finish() cache sync.
+	 */
+	if (q->memory == VB2_MEMORY_DMABUF) {
+		vb->need_cache_sync_on_finish = 0;
+		vb->need_cache_sync_on_prepare = 0;
+		return;
+	}
+
+	/*
+	 * Cache sync/invalidation flags are set by default in order to
+	 * preserve existing behaviour for old apps/drivers.
+	 */
+	vb->need_cache_sync_on_prepare = 1;
+	vb->need_cache_sync_on_finish = 1;
+
+	if (!vb2_queue_allows_cache_hints(q)) {
+		/*
+		 * Clear buffer cache flags if queue does not support user
+		 * space hints. That's to indicate to userspace that these
+		 * flags won't work.
+		 */
+		b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
+		b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_CLEAN;
+		return;
+	}
+
+	/*
+	 * ->finish() cache sync can be avoided when queue direction is
+	 * TO_DEVICE.
+	 */
+	if (q->dma_dir == DMA_TO_DEVICE)
+		vb->need_cache_sync_on_finish = 0;
+
+	if (b->flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)
+		vb->need_cache_sync_on_finish = 0;
+
+	if (b->flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN)
+		vb->need_cache_sync_on_prepare = 0;
+}
+
 static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev,
 				    struct v4l2_buffer *b, bool is_prepare,
 				    struct media_request **p_req)
@@ -381,6 +428,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
 	}
 
 	if (!vb->prepared) {
+		set_buffer_cache_hints(q, vb, b);
 		/* Copy relevant information provided by the userspace */
 		memset(vbuf->planes, 0,
 		       sizeof(vbuf->planes[0]) * vb->num_planes);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 7f39d9fffc8c..ccc5c498d3e3 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -635,6 +635,17 @@ struct vb2_queue {
 #endif
 };
 
+/**
+ * vb2_queue_allows_cache_hints() - Return true if the queue allows cache
+ * and memory consistency hints.
+ *
+ * @q:		pointer to &struct vb2_queue with videobuf2 queue
+ */
+static inline bool vb2_queue_allows_cache_hints(struct vb2_queue *q)
+{
+	return q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP;
+}
+
 /**
  * vb2_plane_vaddr() - Return a kernel virtual address of a given plane.
  * @vb:		pointer to &struct vb2_buffer to which the plane in
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 04/13] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (2 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 03/13] videobuf2: handle V4L2 buffer cache flags Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 05/13] videobuf2: add queue memory consistency parameter Sergey Senozhatsky
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

By setting or clearing V4L2_FLAG_MEMORY_NON_CONSISTENT flag
user-space should be able to set or clear queue's NON_CONSISTENT
->dma_attrs. Queue's ->dma_attrs are passed to the underlying
allocator in __vb2_buf_mem_alloc(), so thus user-space is able
to request vb2 buffer's memory to be either consistent (coherent)
or non-consistent.

The patch set also adds a corresponding capability flag:
fill_buf_caps() reports V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS when
queue supports user-space cache management hints. Note that
queue should be set up and initialized, since it's the driver
that sets ->allow_cache_hints.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../userspace-api/media/v4l/buffer.rst        | 41 ++++++++++++++++++-
 .../media/v4l/vidioc-reqbufs.rst              |  9 ++++
 .../media/common/videobuf2/videobuf2-v4l2.c   |  2 +
 include/uapi/linux/videodev2.h                |  3 ++
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
index 951ae1ed485f..66023658270c 100644
--- a/Documentation/userspace-api/media/v4l/buffer.rst
+++ b/Documentation/userspace-api/media/v4l/buffer.rst
@@ -577,7 +577,10 @@ Buffer Flags
 	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.
+	further processing or output. This flag is ignored unless the
+	queue reports
+	:ref:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS <V4L2-BUF-CAP-SUPPORTS-CACHE-HINTS>`
+	capability.
     * .. _`V4L2-BUF-FLAG-NO-CACHE-CLEAN`:
 
       - ``V4L2_BUF_FLAG_NO_CACHE_CLEAN``
@@ -585,7 +588,10 @@ Buffer Flags
       - 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.
+	DMA-capable unit, in which case caches have not been used. This flag
+	is ignored unless the queue reports
+	:ref:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS <V4L2-BUF-CAP-SUPPORTS-CACHE-HINTS>`
+	capability.
     * .. _`V4L2-BUF-FLAG-M2M-HOLD-CAPTURE-BUF`:
 
       - ``V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF``
@@ -681,6 +687,37 @@ Buffer Flags
 
     \normalsize
 
+.. _memory-flags:
+
+Memory Consistency Flags
+========================
+
+.. tabularcolumns:: |p{7.0cm}|p{2.2cm}|p{8.3cm}|
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       3 1 4
+
+    * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`:
+
+      - ``V4L2_FLAG_MEMORY_NON_CONSISTENT``
+      - 0x00000001
+      - A buffer is allocated either in consistent (it will be automatically
+	coherent between the CPU and the bus) or non-consistent memory. The
+	latter can provide performance gains, for instance the CPU cache
+	sync/flush operations can be avoided if the buffer is accessed by the
+	corresponding device only and the CPU does not read/write to/from that
+	buffer. However, this requires extra care from the driver -- it must
+	guarantee memory consistency by issuing a cache flush/sync when
+	consistency is needed. If this flag is set V4L2 will attempt to
+	allocate the buffer in non-consistent memory. The flag takes effect
+	only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the
+	queue reports the
+	:ref:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS <V4L2-BUF-CAP-SUPPORTS-CACHE-HINTS>`
+	capability.
 
 .. c:type:: v4l2_memory
 
diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
index 79884bb4893b..893570af617b 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
@@ -126,6 +126,7 @@ aborting or finishing any DMA in progress, an implicit
 .. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
 .. _V4L2-BUF-CAP-SUPPORTS-ORPHANED-BUFS:
 .. _V4L2-BUF-CAP-SUPPORTS-M2M-HOLD-CAPTURE-BUF:
+.. _V4L2-BUF-CAP-SUPPORTS-CACHE-HINTS:
 
 .. cssclass:: longtable
 
@@ -156,6 +157,14 @@ aborting or finishing any DMA in progress, an implicit
       - Only valid for stateless decoders. If set, then userspace can set the
         ``V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF`` flag to hold off on returning the
 	capture buffer until the OUTPUT timestamp changes.
+    * - ``V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS``
+      - 0x00000040
+      - Set for ``V4L2_MEMORY_MMAP`` queues which support memory consistency
+        and cache management hints. See
+        :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT <V4L2-FLAG-MEMORY-NON-CONSISTENT>`,
+        :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE <V4L2-BUF-FLAG-NO-CACHE-INVALIDATE>` and
+        :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN <V4L2-BUF-FLAG-NO-CACHE-CLEAN>`.
+
 
 Return Value
 ============
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index f13851212cc8..26716055d914 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -710,6 +710,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
 		*caps |= V4L2_BUF_CAP_SUPPORTS_DMABUF;
 	if (q->subsystem_flags & VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF)
 		*caps |= V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF;
+	if (q->allow_cache_hints && q->io_modes & VB2_MMAP)
+		*caps |= V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS;
 #ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API
 	if (q->supports_requests)
 		*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 9817b7e2c968..0d4deba8fe30 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -189,6 +189,8 @@ enum v4l2_memory {
 	V4L2_MEMORY_DMABUF           = 4,
 };
 
+#define V4L2_FLAG_MEMORY_NON_CONSISTENT		(1 << 0)
+
 /* see also http://vektor.theorem.ca/graphics/ycbcr/ */
 enum v4l2_colorspace {
 	/*
@@ -951,6 +953,7 @@ struct v4l2_requestbuffers {
 #define V4L2_BUF_CAP_SUPPORTS_REQUESTS			(1 << 3)
 #define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS		(1 << 4)
 #define V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF	(1 << 5)
+#define V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS		(1 << 6)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 05/13] videobuf2: add queue memory consistency parameter
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (3 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 04/13] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 06/13] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Preparations for future V4L2_FLAG_MEMORY_NON_CONSISTENT support.

Extend vb2_core_reqbufs() with queue memory consistency flag
that is applied to the newly allocated buffers.

An attempt to allocate a buffer with consistency requirements
which don't match queue's consistency model will fail.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../media/common/videobuf2/videobuf2-core.c   | 52 +++++++++++++++----
 .../media/common/videobuf2/videobuf2-v4l2.c   |  6 +--
 drivers/media/dvb-core/dvb_vb2.c              |  2 +-
 include/media/videobuf2-core.h                |  8 ++-
 4 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 44d65f5be845..f1a65925bc17 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -664,11 +664,34 @@ int vb2_verify_memory_type(struct vb2_queue *q,
 }
 EXPORT_SYMBOL(vb2_verify_memory_type);
 
+static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem)
+{
+	if (!vb2_queue_allows_cache_hints(q))
+		return;
+
+	if (consistent_mem)
+		q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT;
+	else
+		q->dma_attrs |= DMA_ATTR_NON_CONSISTENT;
+}
+
+static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem)
+{
+	bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT);
+
+	if (consistent_mem != queue_is_consistent) {
+		dprintk(1, "memory consistency model mismatch\n");
+		return false;
+	}
+	return true;
+}
+
 int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
-		unsigned int *count)
+		     unsigned int flags, unsigned int *count)
 {
 	unsigned int num_buffers, allocated_buffers, num_planes = 0;
 	unsigned plane_sizes[VB2_MAX_PLANES] = { };
+	bool consistent_mem = true;
 	unsigned int i;
 	int ret;
 
@@ -683,7 +706,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 	}
 
 	if (*count == 0 || q->num_buffers != 0 ||
-	    (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) {
+	    (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) ||
+	    !verify_consistency_attr(q, consistent_mem)) {
 		/*
 		 * We already have buffers allocated, so first check if they
 		 * are not in use and can be freed.
@@ -730,6 +754,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 	if (ret)
 		return ret;
 
+	set_queue_consistency(q, consistent_mem);
 	/* Check that driver has set sane values */
 	if (WARN_ON(!num_planes))
 		return -EINVAL;
@@ -804,11 +829,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 EXPORT_SYMBOL_GPL(vb2_core_reqbufs);
 
 int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
-		unsigned int *count, unsigned requested_planes,
-		const unsigned requested_sizes[])
+			 unsigned int flags, unsigned int *count,
+			 unsigned int requested_planes,
+			 const unsigned int requested_sizes[])
 {
 	unsigned int num_planes = 0, num_buffers, allocated_buffers;
 	unsigned plane_sizes[VB2_MAX_PLANES] = { };
+	bool consistent_mem = true;
 	int ret;
 
 	if (q->num_buffers == VB2_MAX_FRAME) {
@@ -824,9 +851,13 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
 		memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
 		q->memory = memory;
 		q->waiting_for_buffers = !q->is_output;
-	} else if (q->memory != memory) {
-		dprintk(1, "memory model mismatch\n");
-		return -EINVAL;
+	} else {
+		if (q->memory != memory) {
+			dprintk(1, "memory model mismatch\n");
+			return -EINVAL;
+		}
+		if (!verify_consistency_attr(q, consistent_mem))
+			return -EINVAL;
 	}
 
 	num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers);
@@ -845,6 +876,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
 	if (ret)
 		return ret;
 
+	set_queue_consistency(q, consistent_mem);
 	/* Finally, allocate buffers and video memory */
 	allocated_buffers = __vb2_queue_alloc(q, memory, num_buffers,
 				num_planes, plane_sizes);
@@ -2498,7 +2530,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
 	fileio->memory = VB2_MEMORY_MMAP;
 	fileio->type = q->type;
 	q->fileio = fileio;
-	ret = vb2_core_reqbufs(q, fileio->memory, &fileio->count);
+	ret = vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);
 	if (ret)
 		goto err_kfree;
 
@@ -2555,7 +2587,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
 
 err_reqbufs:
 	fileio->count = 0;
-	vb2_core_reqbufs(q, fileio->memory, &fileio->count);
+	vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);
 
 err_kfree:
 	q->fileio = NULL;
@@ -2575,7 +2607,7 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q)
 		vb2_core_streamoff(q, q->type);
 		q->fileio = NULL;
 		fileio->count = 0;
-		vb2_core_reqbufs(q, fileio->memory, &fileio->count);
+		vb2_core_reqbufs(q, fileio->memory, 0, &fileio->count);
 		kfree(fileio);
 		dprintk(3, "file io emulator closed\n");
 	}
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 26716055d914..04ea332567b1 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -723,7 +723,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	int ret = vb2_verify_memory_type(q, req->memory, req->type);
 
 	fill_buf_caps(q, &req->capabilities);
-	return ret ? ret : vb2_core_reqbufs(q, req->memory, &req->count);
+	return ret ? ret : vb2_core_reqbufs(q, req->memory, 0, &req->count);
 }
 EXPORT_SYMBOL_GPL(vb2_reqbufs);
 
@@ -797,7 +797,7 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
 	for (i = 0; i < requested_planes; i++)
 		if (requested_sizes[i] == 0)
 			return -EINVAL;
-	return ret ? ret : vb2_core_create_bufs(q, create->memory,
+	return ret ? ret : vb2_core_create_bufs(q, create->memory, true,
 		&create->count, requested_planes, requested_sizes);
 }
 EXPORT_SYMBOL_GPL(vb2_create_bufs);
@@ -973,7 +973,7 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
 		return res;
 	if (vb2_queue_is_busy(vdev, file))
 		return -EBUSY;
-	res = vb2_core_reqbufs(vdev->queue, p->memory, &p->count);
+	res = vb2_core_reqbufs(vdev->queue, p->memory, 0, &p->count);
 	/* If count == 0, then the owner has released all buffers and he
 	   is no longer owner of the queue. Otherwise we have a new owner. */
 	if (res == 0)
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index 6974f1731529..959d110407a4 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -342,7 +342,7 @@ int dvb_vb2_reqbufs(struct dvb_vb2_ctx *ctx, struct dmx_requestbuffers *req)
 
 	ctx->buf_siz = req->size;
 	ctx->buf_cnt = req->count;
-	ret = vb2_core_reqbufs(&ctx->vb_q, VB2_MEMORY_MMAP, &req->count);
+	ret = vb2_core_reqbufs(&ctx->vb_q, VB2_MEMORY_MMAP, 0, &req->count);
 	if (ret) {
 		ctx->state = DVB_VB2_STATE_NONE;
 		dprintk(1, "[%s] count=%d size=%d errno=%d\n", ctx->name,
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index ccc5c498d3e3..9e68fe043a6c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -740,6 +740,8 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb);
  * vb2_core_reqbufs() - Initiate streaming.
  * @q:		pointer to &struct vb2_queue with videobuf2 queue.
  * @memory:	memory type, as defined by &enum vb2_memory.
+ * @flags:	auxiliary queue/buffer management flags. Currently, the only
+ *		used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT.
  * @count:	requested buffer count.
  *
  * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called
@@ -764,12 +766,13 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb);
  * Return: returns zero on success; an error code otherwise.
  */
 int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
-		unsigned int *count);
+		    unsigned int flags, unsigned int *count);
 
 /**
  * vb2_core_create_bufs() - Allocate buffers and any required auxiliary structs
  * @q: pointer to &struct vb2_queue with videobuf2 queue.
  * @memory: memory type, as defined by &enum vb2_memory.
+ * @flags: auxiliary queue/buffer management flags.
  * @count: requested buffer count.
  * @requested_planes: number of planes requested.
  * @requested_sizes: array with the size of the planes.
@@ -787,7 +790,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
  * Return: returns zero on success; an error code otherwise.
  */
 int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
-			 unsigned int *count, unsigned int requested_planes,
+			 unsigned int flags, unsigned int *count,
+			 unsigned int requested_planes,
 			 const unsigned int requested_sizes[]);
 
 /**
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 06/13] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (4 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 05/13] videobuf2: add queue memory consistency parameter Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 07/13] videobuf2: factor out planes prepare/finish functions Sergey Senozhatsky
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

This patch lets user-space to request a non-consistent memory
allocation during CREATE_BUFS and REQBUFS ioctl calls.

= CREATE_BUFS

  struct v4l2_create_buffers has seven 4-byte reserved areas,
  so reserved[0] is renamed to ->flags. The struct, thus, now
  has six reserved 4-byte regions.

= CREATE_BUFS32

  struct v4l2_create_buffers32 has seven 4-byte reserved areas,
  so reserved[0] is renamed to ->flags. The struct, thus, now
  has six reserved 4-byte regions.

= REQBUFS

 We use one bit of a ->reserved[1] member of struct v4l2_requestbuffers,
 which is now renamed to ->flags. Unlike v4l2_create_buffers, struct
 v4l2_requestbuffers does not have enough reserved room. Therefore for
 backward compatibility  ->reserved and ->flags were put into anonymous
 union.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../media/v4l/vidioc-create-bufs.rst          |  7 ++-
 .../media/v4l/vidioc-reqbufs.rst              | 11 ++++-
 .../media/common/videobuf2/videobuf2-core.c   |  6 +++
 .../media/common/videobuf2/videobuf2-v4l2.c   | 43 +++++++++++++++++--
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  9 +++-
 drivers/media/v4l2-core/v4l2-ioctl.c          |  5 +--
 include/uapi/linux/videodev2.h                | 10 ++++-
 7 files changed, 76 insertions(+), 15 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
index bd580232be10..4ece2551b355 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst
@@ -121,7 +121,12 @@ than the number requested.
 	other changes, then set ``count`` to 0, ``memory`` to
 	``V4L2_MEMORY_MMAP`` and ``format.type`` to the buffer type.
     * - __u32
-      - ``reserved``\ [7]
+      - ``flags``
+      - Specifies additional buffer management attributes.
+	See :ref:`memory-flags`.
+
+    * - __u32
+      - ``reserved``\ [6]
       - A place holder for future extensions. Drivers and applications
 	must set the array to zero.
 
diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
index 893570af617b..a933a72dab1f 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
@@ -112,10 +112,17 @@ aborting or finishing any DMA in progress, an implicit
 	``V4L2_MEMORY_MMAP`` and ``type`` set to the buffer type. This will
 	free any previously allocated buffers, so this is typically something
 	that will be done at the start of the application.
+    * - union {
+      - (anonymous)
+    * - __u32
+      - ``flags``
+      - Specifies additional buffer management attributes.
+	See :ref:`memory-flags`.
     * - __u32
       - ``reserved``\ [1]
-      - A place holder for future extensions. Drivers and applications
-	must set the array to zero.
+      - Kept for backwards compatibility. Use ``flags`` instead.
+    * - }
+      -
 
 .. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
 
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index f1a65925bc17..d16f47022552 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -695,6 +695,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 	unsigned int i;
 	int ret;
 
+	if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
+		consistent_mem = false;
+
 	if (q->streaming) {
 		dprintk(1, "streaming active\n");
 		return -EBUSY;
@@ -838,6 +841,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
 	bool consistent_mem = true;
 	int ret;
 
+	if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
+		consistent_mem = false;
+
 	if (q->num_buffers == VB2_MAX_FRAME) {
 		dprintk(1, "maximum number of buffers already allocated\n");
 		return -ENOBUFS;
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 04ea332567b1..aeb29c33984e 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -718,12 +718,36 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
 #endif
 }
 
+/*
+ * vb2_core_reqbufs()/vb2_core_create_bufs() can re-initialize the queue,
+ * change its memory, type and invoke driver's ->queue_setup(), so we need
+ * to adjust consistency flag and caps accordingly.
+ */
+static void fixup_consistency_attr(struct vb2_queue *q, unsigned int *flags)
+{
+	if (!vb2_queue_allows_cache_hints(q))
+		*flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
+}
+
+static void fixup_cache_hints_cap(struct vb2_queue *q, unsigned int *caps)
+{
+	if (!vb2_queue_allows_cache_hints(q))
+		*caps &= ~V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS;
+	else
+		*caps |= V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS;
+}
+
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 {
 	int ret = vb2_verify_memory_type(q, req->memory, req->type);
 
 	fill_buf_caps(q, &req->capabilities);
-	return ret ? ret : vb2_core_reqbufs(q, req->memory, 0, &req->count);
+	if (ret)
+		return ret;
+	ret = vb2_core_reqbufs(q, req->memory, req->flags, &req->count);
+	fixup_consistency_attr(q, &req->flags);
+	fixup_cache_hints_cap(q, &req->capabilities);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(vb2_reqbufs);
 
@@ -797,8 +821,16 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
 	for (i = 0; i < requested_planes; i++)
 		if (requested_sizes[i] == 0)
 			return -EINVAL;
-	return ret ? ret : vb2_core_create_bufs(q, create->memory, true,
-		&create->count, requested_planes, requested_sizes);
+
+	if (ret)
+		return ret;
+
+	ret = vb2_core_create_bufs(q, create->memory,
+				   create->flags, &create->count,
+				   requested_planes, requested_sizes);
+	fixup_consistency_attr(q, &create->flags);
+	fixup_cache_hints_cap(q, &create->capabilities);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(vb2_create_bufs);
 
@@ -973,7 +1005,10 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
 		return res;
 	if (vb2_queue_is_busy(vdev, file))
 		return -EBUSY;
-	res = vb2_core_reqbufs(vdev->queue, p->memory, 0, &p->count);
+
+	res = vb2_core_reqbufs(vdev->queue, p->memory, p->flags, &p->count);
+	fixup_consistency_attr(vdev->queue, &p->flags);
+	fixup_cache_hints_cap(vdev->queue, &p->capabilities);
 	/* If count == 0, then the owner has released all buffers and he
 	   is no longer owner of the queue. Otherwise we have a new owner. */
 	if (res == 0)
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index a99e82ec9ab6..aa162ed6d01c 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -246,6 +246,8 @@ struct v4l2_format32 {
  * @memory:	buffer memory type
  * @format:	frame format, for which buffers are requested
  * @capabilities: capabilities of this buffer type.
+ * @flags:	additional buffer management attributes (ignored if queue
+ *		does not have V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS capability).
  * @reserved:	future extensions
  */
 struct v4l2_create_buffers32 {
@@ -254,7 +256,8 @@ struct v4l2_create_buffers32 {
 	__u32			memory;	/* enum v4l2_memory */
 	struct v4l2_format32	format;
 	__u32			capabilities;
-	__u32			reserved[7];
+	__u32			flags;
+	__u32			reserved[6];
 };
 
 static int __bufsize_v4l2_format(struct v4l2_format32 __user *p32, u32 *size)
@@ -355,7 +358,8 @@ static int get_v4l2_create32(struct v4l2_create_buffers __user *p64,
 {
 	if (!access_ok(p32, sizeof(*p32)) ||
 	    copy_in_user(p64, p32,
-			 offsetof(struct v4l2_create_buffers32, format)))
+			 offsetof(struct v4l2_create_buffers32, format)) ||
+	    assign_in_user(&p64->flags, &p32->flags))
 		return -EFAULT;
 	return __get_v4l2_format32(&p64->format, &p32->format,
 				   aux_buf, aux_space);
@@ -417,6 +421,7 @@ static int put_v4l2_create32(struct v4l2_create_buffers __user *p64,
 	    copy_in_user(p32, p64,
 			 offsetof(struct v4l2_create_buffers32, format)) ||
 	    assign_in_user(&p32->capabilities, &p64->capabilities) ||
+	    assign_in_user(&p32->flags, &p64->flags) ||
 	    copy_in_user(p32->reserved, p64->reserved, sizeof(p64->reserved)))
 		return -EFAULT;
 	return __put_v4l2_format32(&p64->format, &p32->format);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 833e79a2cb98..4c2dadbeefdc 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1978,9 +1978,6 @@ static int v4l_reqbufs(const struct v4l2_ioctl_ops *ops,
 
 	if (ret)
 		return ret;
-
-	CLEAR_AFTER_FIELD(p, capabilities);
-
 	return ops->vidioc_reqbufs(file, fh, p);
 }
 
@@ -2020,7 +2017,7 @@ static int v4l_create_bufs(const struct v4l2_ioctl_ops *ops,
 	if (ret)
 		return ret;
 
-	CLEAR_AFTER_FIELD(create, capabilities);
+	CLEAR_AFTER_FIELD(create, flags);
 
 	v4l_sanitize_format(&create->format);
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 0d4deba8fe30..f0a4694dd7c6 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -943,7 +943,10 @@ struct v4l2_requestbuffers {
 	__u32			type;		/* enum v4l2_buf_type */
 	__u32			memory;		/* enum v4l2_memory */
 	__u32			capabilities;
-	__u32			reserved[1];
+	union {
+		__u32		flags;
+		__u32		reserved[1];
+	};
 };
 
 /* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
@@ -2447,6 +2450,8 @@ struct v4l2_dbg_chip_info {
  * @memory:	enum v4l2_memory; buffer memory type
  * @format:	frame format, for which buffers are requested
  * @capabilities: capabilities of this buffer type.
+ * @flags:	additional buffer management attributes (ignored if queue
+ *		does not have V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS capability).
  * @reserved:	future extensions
  */
 struct v4l2_create_buffers {
@@ -2455,7 +2460,8 @@ struct v4l2_create_buffers {
 	__u32			memory;
 	struct v4l2_format	format;
 	__u32			capabilities;
-	__u32			reserved[7];
+	__u32			flags;
+	__u32			reserved[6];
 };
 
 /*
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 07/13] videobuf2: factor out planes prepare/finish functions
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (5 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 06/13] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 08/13] videobuf2: do not sync caches when we are allowed not to Sergey Senozhatsky
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Factor out the code, no functional changes.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../media/common/videobuf2/videobuf2-core.c   | 52 +++++++++++--------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index d16f47022552..0be31a11c913 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -296,6 +296,32 @@ static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb)
 		__vb2_plane_dmabuf_put(vb, &vb->planes[plane]);
 }
 
+/*
+ * __vb2_buf_mem_prepare() - call ->prepare() on buffer's private memory
+ * to sync caches
+ */
+static void __vb2_buf_mem_prepare(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);
+	vb->synced = 1;
+}
+
+/*
+ * __vb2_buf_mem_finish() - call ->finish on buffer's private memory
+ * to sync caches
+ */
+static void __vb2_buf_mem_finish(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);
+	vb->synced = 0;
+}
+
 /*
  * __setup_offsets() - setup unique offsets ("cookies") for every plane in
  * the buffer.
@@ -960,7 +986,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;
@@ -980,12 +1005,8 @@ 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);
 
-	if (state != VB2_BUF_STATE_QUEUED) {
-		/* sync buffers */
-		for (plane = 0; plane < vb->num_planes; ++plane)
-			call_void_memop(vb, finish, vb->planes[plane].mem_priv);
-		vb->synced = 0;
-	}
+	if (state != VB2_BUF_STATE_QUEUED)
+		__vb2_buf_mem_finish(vb);
 
 	spin_lock_irqsave(&q->done_lock, flags);
 	if (state == VB2_BUF_STATE_QUEUED) {
@@ -1310,7 +1331,6 @@ static int __buf_prepare(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	enum vb2_buffer_state orig_state = vb->state;
-	unsigned int plane;
 	int ret;
 
 	if (q->error) {
@@ -1354,11 +1374,7 @@ static int __buf_prepare(struct vb2_buffer *vb)
 		return ret;
 	}
 
-	/* sync buffers */
-	for (plane = 0; plane < vb->num_planes; ++plane)
-		call_void_memop(vb, prepare, vb->planes[plane].mem_priv);
-
-	vb->synced = 1;
+	__vb2_buf_mem_prepare(vb);
 	vb->prepared = 1;
 	vb->state = orig_state;
 
@@ -1978,14 +1994,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 				call_void_vb_qop(vb, buf_request_complete, vb);
 		}
 
-		if (vb->synced) {
-			unsigned int plane;
-
-			for (plane = 0; plane < vb->num_planes; ++plane)
-				call_void_memop(vb, finish,
-						vb->planes[plane].mem_priv);
-			vb->synced = 0;
-		}
+		if (vb->synced)
+			__vb2_buf_mem_finish(vb);
 
 		if (vb->prepared) {
 			call_void_vb_qop(vb, buf_finish, vb);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 08/13] videobuf2: do not sync caches when we are allowed not to
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (6 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 07/13] videobuf2: factor out planes prepare/finish functions Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 09/13] videobuf2: check ->synced flag in prepare() and finish() Sergey Senozhatsky
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Skip ->prepare() or/and ->finish() cache synchronisation if
user-space requested us to do so (or when queue dma direction
permits us to skip cache syncs).

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 0be31a11c913..5409e15b6af4 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -304,8 +304,11 @@ static void __vb2_buf_mem_prepare(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);
+	if (vb->need_cache_sync_on_prepare) {
+		for (plane = 0; plane < vb->num_planes; ++plane)
+			call_void_memop(vb, prepare,
+					vb->planes[plane].mem_priv);
+	}
 	vb->synced = 1;
 }
 
@@ -317,8 +320,11 @@ static void __vb2_buf_mem_finish(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);
+	if (vb->need_cache_sync_on_finish) {
+		for (plane = 0; plane < vb->num_planes; ++plane)
+			call_void_memop(vb, finish,
+					vb->planes[plane].mem_priv);
+	}
 	vb->synced = 0;
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 09/13] videobuf2: check ->synced flag in prepare() and finish()
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (7 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 08/13] videobuf2: do not sync caches when we are allowed not to Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 10/13] videobuf2: add begin/end cpu_access callbacks to dma-contig Sergey Senozhatsky
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

This simplifies the code a tiny bit and let's us to avoid
unneeded ->prepare()/->finish() calls.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 5409e15b6af4..5a69005b7d59 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -304,6 +304,9 @@ static void __vb2_buf_mem_prepare(struct vb2_buffer *vb)
 {
 	unsigned int plane;
 
+	if (vb->synced)
+		return;
+
 	if (vb->need_cache_sync_on_prepare) {
 		for (plane = 0; plane < vb->num_planes; ++plane)
 			call_void_memop(vb, prepare,
@@ -320,6 +323,9 @@ static void __vb2_buf_mem_finish(struct vb2_buffer *vb)
 {
 	unsigned int plane;
 
+	if (!vb->synced)
+		return;
+
 	if (vb->need_cache_sync_on_finish) {
 		for (plane = 0; plane < vb->num_planes; ++plane)
 			call_void_memop(vb, finish,
@@ -2000,8 +2006,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 				call_void_vb_qop(vb, buf_request_complete, vb);
 		}
 
-		if (vb->synced)
-			__vb2_buf_mem_finish(vb);
+		__vb2_buf_mem_finish(vb);
 
 		if (vb->prepared) {
 			call_void_vb_qop(vb, buf_finish, vb);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 10/13] videobuf2: add begin/end cpu_access callbacks to dma-contig
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (8 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 09/13] videobuf2: check ->synced flag in prepare() and finish() Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 11/13] videobuf2: add begin/end cpu_access callbacks to dma-sg Sergey Senozhatsky
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Provide begin_cpu_access() and end_cpu_access() callbacks for
cache synchronisation on exported buffers.

The patch also adds a new helper function - vb2_dc_buffer_consistent(),
which returns true is if the buffer is consistent (DMA_ATTR_NON_CONSISTENT
bit cleared), so then we don't need to sync anything.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../common/videobuf2/videobuf2-dma-contig.c   | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index d3a3ee5b597b..6787e2cb905e 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -42,6 +42,11 @@ struct vb2_dc_buf {
 	struct dma_buf_attachment	*db_attach;
 };
 
+static inline bool vb2_dc_buffer_consistent(unsigned long attr)
+{
+	return !(attr & DMA_ATTR_NON_CONSISTENT);
+}
+
 /*********************************************/
 /*        scatterlist table functions        */
 /*********************************************/
@@ -335,6 +340,34 @@ static void vb2_dc_dmabuf_ops_release(struct dma_buf *dbuf)
 	vb2_dc_put(dbuf->priv);
 }
 
+static int
+vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf,
+				   enum dma_data_direction direction)
+{
+	struct vb2_dc_buf *buf = dbuf->priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	if (vb2_dc_buffer_consistent(buf->attrs))
+		return 0;
+
+	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+	return 0;
+}
+
+static int
+vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf,
+				 enum dma_data_direction direction)
+{
+	struct vb2_dc_buf *buf = dbuf->priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	if (vb2_dc_buffer_consistent(buf->attrs))
+		return 0;
+
+	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+	return 0;
+}
+
 static void *vb2_dc_dmabuf_ops_vmap(struct dma_buf *dbuf)
 {
 	struct vb2_dc_buf *buf = dbuf->priv;
@@ -353,6 +386,8 @@ static const struct dma_buf_ops vb2_dc_dmabuf_ops = {
 	.detach = vb2_dc_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dc_dmabuf_ops_map,
 	.unmap_dma_buf = vb2_dc_dmabuf_ops_unmap,
+	.begin_cpu_access = vb2_dc_dmabuf_ops_begin_cpu_access,
+	.end_cpu_access = vb2_dc_dmabuf_ops_end_cpu_access,
 	.vmap = vb2_dc_dmabuf_ops_vmap,
 	.mmap = vb2_dc_dmabuf_ops_mmap,
 	.release = vb2_dc_dmabuf_ops_release,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 11/13] videobuf2: add begin/end cpu_access callbacks to dma-sg
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (9 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 10/13] videobuf2: add begin/end cpu_access callbacks to dma-contig Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 12/13] videobuf2: don't test db_attach in dma-contig prepare and finish Sergey Senozhatsky
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

Provide begin_cpu_access() and end_cpu_access() dma_buf_ops
callbacks for cache synchronisation on exported buffers.

V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers.
dma-sg allocates memory using the page allocator directly, so
there is no memory consistency guarantee.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 .../media/common/videobuf2/videobuf2-dma-sg.c | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 92072a08af25..595137e358e7 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -120,6 +120,12 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs,
 	buf->num_pages = size >> PAGE_SHIFT;
 	buf->dma_sgt = &buf->sg_table;
 
+	/*
+	 * NOTE: dma-sg allocates memory using the page allocator directly, so
+	 * there is no memory consistency guarantee, hence dma-sg ignores DMA
+	 * attributes passed from the upper layer. That means that
+	 * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers.
+	 */
 	buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *),
 				    GFP_KERNEL | __GFP_ZERO);
 	if (!buf->pages)
@@ -469,6 +475,28 @@ static void vb2_dma_sg_dmabuf_ops_release(struct dma_buf *dbuf)
 	vb2_dma_sg_put(dbuf->priv);
 }
 
+static int
+vb2_dma_sg_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf,
+				       enum dma_data_direction direction)
+{
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+	return 0;
+}
+
+static int
+vb2_dma_sg_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf,
+				     enum dma_data_direction direction)
+{
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+	return 0;
+}
+
 static void *vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf)
 {
 	struct vb2_dma_sg_buf *buf = dbuf->priv;
@@ -487,6 +515,8 @@ static const struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
 	.detach = vb2_dma_sg_dmabuf_ops_detach,
 	.map_dma_buf = vb2_dma_sg_dmabuf_ops_map,
 	.unmap_dma_buf = vb2_dma_sg_dmabuf_ops_unmap,
+	.begin_cpu_access = vb2_dma_sg_dmabuf_ops_begin_cpu_access,
+	.end_cpu_access = vb2_dma_sg_dmabuf_ops_end_cpu_access,
 	.vmap = vb2_dma_sg_dmabuf_ops_vmap,
 	.mmap = vb2_dma_sg_dmabuf_ops_mmap,
 	.release = vb2_dma_sg_dmabuf_ops_release,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 12/13] videobuf2: don't test db_attach in dma-contig prepare and finish
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (10 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 11/13] videobuf2: add begin/end cpu_access callbacks to dma-sg Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-24  9:29 ` [PATCHv5 13/13] media: vivid: add cache_hints module param Sergey Senozhatsky
  2020-04-28 14:47 ` [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Hans Verkuil
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky, Sergey Senozhatsky

From: Sergey Senozhatsky <senozhatsky@chromium.org>

We moved cache management decision making to the upper layer and
rely on buffer's need_cache_sync flags and videobuf2 core. If the
upper layer (core) has decided to invoke ->prepare() or ->finish()
then we must sync.

For DMABUF ->need_cache_sync_on_prepare and ->need_cache_sync_on_flush
are always false so videobuf core does not call ->prepare() and
->finish() on such buffers.

Additionally, scratch the DMABUF comment.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/media/common/videobuf2/videobuf2-dma-contig.c | 6 ++----
 drivers/media/common/videobuf2/videobuf2-dma-sg.c     | 8 --------
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index 6787e2cb905e..5a3e1c3b556f 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -100,8 +100,7 @@ static void vb2_dc_prepare(void *buf_priv)
 	struct vb2_dc_buf *buf = buf_priv;
 	struct sg_table *sgt = buf->dma_sgt;
 
-	/* DMABUF exporter will flush the cache for us */
-	if (!sgt || buf->db_attach)
+	if (!sgt)
 		return;
 
 	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
@@ -113,8 +112,7 @@ static void vb2_dc_finish(void *buf_priv)
 	struct vb2_dc_buf *buf = buf_priv;
 	struct sg_table *sgt = buf->dma_sgt;
 
-	/* DMABUF exporter will flush the cache for us */
-	if (!sgt || buf->db_attach)
+	if (!sgt)
 		return;
 
 	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 595137e358e7..0a40e00f0d7e 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -204,10 +204,6 @@ static void vb2_dma_sg_prepare(void *buf_priv)
 	struct vb2_dma_sg_buf *buf = buf_priv;
 	struct sg_table *sgt = buf->dma_sgt;
 
-	/* DMABUF exporter will flush the cache for us */
-	if (buf->db_attach)
-		return;
-
 	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
 			       buf->dma_dir);
 }
@@ -217,10 +213,6 @@ static void vb2_dma_sg_finish(void *buf_priv)
 	struct vb2_dma_sg_buf *buf = buf_priv;
 	struct sg_table *sgt = buf->dma_sgt;
 
-	/* DMABUF exporter will flush the cache for us */
-	if (buf->db_attach)
-		return;
-
 	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
 }
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCHv5 13/13] media: vivid: add cache_hints module param
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (11 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 12/13] videobuf2: don't test db_attach in dma-contig prepare and finish Sergey Senozhatsky
@ 2020-04-24  9:29 ` Sergey Senozhatsky
  2020-04-28 14:47 ` [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Hans Verkuil
  13 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel,
	Sergey Senozhatsky

Add a cache_hints module param to control per-queue user space cache
hints support.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/admin-guide/media/vivid.rst          | 9 +++++++++
 drivers/media/test-drivers/vivid/vivid-core.c      | 9 +++++++++
 drivers/media/test-drivers/vivid/vivid-core.h      | 1 +
 drivers/media/test-drivers/vivid/vivid-meta-cap.c  | 5 +++++
 drivers/media/test-drivers/vivid/vivid-meta-out.c  | 5 +++++
 drivers/media/test-drivers/vivid/vivid-sdr-cap.c   | 7 +++++++
 drivers/media/test-drivers/vivid/vivid-touch-cap.c | 5 +++++
 drivers/media/test-drivers/vivid/vivid-vbi-cap.c   | 5 +++++
 drivers/media/test-drivers/vivid/vivid-vbi-out.c   | 5 +++++
 drivers/media/test-drivers/vivid/vivid-vid-cap.c   | 5 +++++
 drivers/media/test-drivers/vivid/vivid-vid-out.c   | 5 +++++
 11 files changed, 61 insertions(+)

diff --git a/Documentation/admin-guide/media/vivid.rst b/Documentation/admin-guide/media/vivid.rst
index 52e57b773f07..a41fa7129dec 100644
--- a/Documentation/admin-guide/media/vivid.rst
+++ b/Documentation/admin-guide/media/vivid.rst
@@ -293,6 +293,15 @@ all configurable using the following module options:
 		- 0: vmalloc
 		- 1: dma-contig
 
+- cache_hints:
+
+	user-space cache hints selection, default is 0. It specifies if the
+	device supports user-space cache (for MMAP queues only) and memory
+	consistency hints.
+
+		- 0: forbid hints
+		- 1: allow hints
+
 Taken together, all these module options allow you to precisely customize
 the driver behavior and test your application with all sorts of permutations.
 It is also very suitable to emulate hardware that is not yet available, e.g.
diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index 6c740e3e6999..b1bea405f382 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -169,6 +169,14 @@ MODULE_PARM_DESC(allocators, " memory allocator selection, default is 0.\n"
 			     "\t\t    0 == vmalloc\n"
 			     "\t\t    1 == dma-contig");
 
+static unsigned int cache_hints[VIVID_MAX_DEVS] = {
+	[0 ... (VIVID_MAX_DEVS - 1)] = 0
+};
+module_param_array(cache_hints, uint, NULL, 0444);
+MODULE_PARM_DESC(cache_hints, " user-space cache hints, default is 0.\n"
+			     "\t\t    0 == forbid\n"
+			     "\t\t    1 == allow");
+
 static struct vivid_dev *vivid_devs[VIVID_MAX_DEVS];
 
 const struct v4l2_rect vivid_min_rect = {
@@ -819,6 +827,7 @@ static int vivid_create_queue(struct vivid_dev *dev,
 	q->lock = &dev->mutex;
 	q->dev = dev->v4l2_dev.dev;
 	q->supports_requests = true;
+	dev->allow_cache_hints = (cache_hints[dev->inst] == 1);
 
 	return vb2_queue_init(q);
 }
diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h
index 99e69b8f770f..75ab627ba91f 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.h
+++ b/drivers/media/test-drivers/vivid/vivid-core.h
@@ -572,6 +572,7 @@ struct vivid_dev {
 
 	bool				meta_pts;
 	bool				meta_scr;
+	bool				allow_cache_hints;
 };
 
 static inline bool vivid_is_webcam(const struct vivid_dev *dev)
diff --git a/drivers/media/test-drivers/vivid/vivid-meta-cap.c b/drivers/media/test-drivers/vivid/vivid-meta-cap.c
index 780f96860a6d..0ba8fff47106 100644
--- a/drivers/media/test-drivers/vivid/vivid-meta-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-meta-cap.c
@@ -34,6 +34,11 @@ static int meta_cap_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
 		*nbuffers = 2 - vq->num_buffers;
 
 	*nplanes = 1;
+
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
 	return 0;
 }
 
diff --git a/drivers/media/test-drivers/vivid/vivid-meta-out.c b/drivers/media/test-drivers/vivid/vivid-meta-out.c
index ff8a039aba72..6be56f1230d4 100644
--- a/drivers/media/test-drivers/vivid/vivid-meta-out.c
+++ b/drivers/media/test-drivers/vivid/vivid-meta-out.c
@@ -34,6 +34,11 @@ static int meta_out_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
 		*nbuffers = 2 - vq->num_buffers;
 
 	*nplanes = 1;
+
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
 	return 0;
 }
 
diff --git a/drivers/media/test-drivers/vivid/vivid-sdr-cap.c b/drivers/media/test-drivers/vivid/vivid-sdr-cap.c
index 2b7522e16efc..f87cd7d0fada 100644
--- a/drivers/media/test-drivers/vivid/vivid-sdr-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-sdr-cap.c
@@ -211,9 +211,16 @@ static int sdr_cap_queue_setup(struct vb2_queue *vq,
 		       unsigned *nbuffers, unsigned *nplanes,
 		       unsigned sizes[], struct device *alloc_devs[])
 {
+	struct vivid_dev *dev = vb2_get_drv_priv(vq);
+
 	/* 2 = max 16-bit sample returned */
 	sizes[0] = SDR_CAP_SAMPLES_PER_BUF * 2;
 	*nplanes = 1;
+
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
 	return 0;
 }
 
diff --git a/drivers/media/test-drivers/vivid/vivid-touch-cap.c b/drivers/media/test-drivers/vivid/vivid-touch-cap.c
index ebb00b128030..af43a5f69ea0 100644
--- a/drivers/media/test-drivers/vivid/vivid-touch-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-touch-cap.c
@@ -27,6 +27,11 @@ static int touch_cap_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
 		*nbuffers = 2 - vq->num_buffers;
 
 	*nplanes = 1;
+
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
 	return 0;
 }
 
diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c
index 1a9348eea781..126754c0bee4 100644
--- a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c
@@ -138,6 +138,11 @@ static int vbi_cap_queue_setup(struct vb2_queue *vq,
 		*nbuffers = 2 - vq->num_buffers;
 
 	*nplanes = 1;
+
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
 	return 0;
 }
 
diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-out.c b/drivers/media/test-drivers/vivid/vivid-vbi-out.c
index cd56476902a2..d35b458c01d2 100644
--- a/drivers/media/test-drivers/vivid/vivid-vbi-out.c
+++ b/drivers/media/test-drivers/vivid/vivid-vbi-out.c
@@ -34,6 +34,11 @@ static int vbi_out_queue_setup(struct vb2_queue *vq,
 		*nbuffers = 2 - vq->num_buffers;
 
 	*nplanes = 1;
+
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
 	return 0;
 }
 
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index e94beef008c8..d0dfd3632ba9 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -129,6 +129,11 @@ static int vid_cap_queue_setup(struct vb2_queue *vq,
 
 	*nplanes = buffers;
 
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
+
 	dprintk(dev, 1, "%s: count=%d\n", __func__, *nbuffers);
 	for (p = 0; p < buffers; p++)
 		dprintk(dev, 1, "%s: size[%u]=%u\n", __func__, p, sizes[p]);
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-out.c b/drivers/media/test-drivers/vivid/vivid-vid-out.c
index ee3446e3217c..4af8aaa13ffa 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-out.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-out.c
@@ -78,6 +78,11 @@ static int vid_out_queue_setup(struct vb2_queue *vq,
 
 	*nplanes = planes;
 
+	if (dev->allow_cache_hints && vq->memory == VB2_MEMORY_MMAP)
+		vq->allow_cache_hints = 1;
+	else
+		vq->allow_cache_hints = 0;
+
 	dprintk(dev, 1, "%s: count=%d\n", __func__, *nbuffers);
 	for (p = 0; p < planes; p++)
 		dprintk(dev, 1, "%s: size[%u]=%u\n", __func__, p, sizes[p]);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags
  2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
                   ` (12 preceding siblings ...)
  2020-04-24  9:29 ` [PATCHv5 13/13] media: vivid: add cache_hints module param Sergey Senozhatsky
@ 2020-04-28 14:47 ` Hans Verkuil
  2020-05-14 15:36   ` Sergey Senozhatsky
  13 siblings, 1 reply; 16+ messages in thread
From: Hans Verkuil @ 2020-04-28 14:47 UTC (permalink / raw)
  To: Sergey Senozhatsky, Hans Verkuil
  Cc: Mauro Carvalho Chehab, Tomasz Figa, linux-media, linux-kernel

Hi Sergey,

On 24/04/2020 11:29, Sergey Senozhatsky wrote:
> Hello,
> 
> 	v5 with fixes and improvements.
> 
> I have a simple v4l-compliance patch now (will send it separately) which
> tests cache and consistency hints. I ran compliance against the vivid
> test driver, which was additionally extended with cache_hints module param:
> Trimmed v4l-compliance output:
> 
> - vivid with disabled cache hints support
> 
> [..]
> Buffer ioctls (Input 3):
> 	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
> 	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
> 	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
> 	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> 	test VIDIOC_EXPBUF: OK
> [..]
> 
> - vivid with enabled cache hints (cache_hints=1,...)
> 
> [..]
> Buffer ioctls (Input 3):
> 	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
> 	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
> 	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
> 	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> 	test VIDIOC_EXPBUF: OK
> [..]
> 
> 
> v4l-compliance revealed that we cannot reliably report
> V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS, it's a bit special.
> Let's look at fill_buf_caps()
> 
> 	if (q->allow_cache_hints && q->io_modes & VB2_MMAP)
> 		*caps |= V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS;
> 
> There are several things here. First, if the queue is not setup yet
> (we didn't call driver's ->queue_setup()) then ->allow_cache_hints
> is expected to be 0. It's only in the ->queue_setup() that the driver
> sets ->allow_cache_hints for those queues that can benefit from cache
> management. What this means, is that if one does
> 
> 	q.init(node->g_type(), memory);
> 	q.reqbufs(node);
> 	q.g_capabilities();
> 
> the cache hints cap will not be reported. We need to setup the queue
> 
> 	q.init(node->g_type(), memory);
> 	q.reqbufs(node, 1);
> 	q.g_capabilities();

I missed that. What should happen is that q->allow_cache_hints is set by the
driver before vb2_queue_init is called. And the documentation should be updated
to say that the V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS flag is only valid when using the
MMAP streaming I/O model.

Perhaps the flag should be renamed to V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS to
make this explicit? Other opinions are welcome.

> 
> Second. Even if the queue is setup, we still can report wrong cache
> hint values. Let's look at the following code
> 
> 	fill_buf_caps(q, &p->capabilities);
> 	if (!vb2_queue_allows_cache_hints(q))
> 		p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;

The problem here is that vb2_queue_allows_cache_hints(q) uses stale information:
the current streaming mode instead of the requested streaming mode.

This should read:

	if (!q->allow_cache_hints || p->memory != V4L2_MEMORY_MMAP)
		p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;

And V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS is always set regardless of the
memory model. It just needs to be documented that this capability applies
to MMAP mode only.

> 	ret = vb2_core_reqbufs(...);
> 	return ret;
> 
> The thing here is that vb2_core_reqbufs() and vb2_core_create_bufs()
> can re-initialize the queue and invoke ->queue_setup(), possibly
> changing its memory model, etc. so cache hints cap which we set or
> clear before vb2_core_reqbufs() and vb2_core_create_bufs() can become
> invalid after we call those functions. It's the same with
> ``req->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT``, we cannot clear
> it before reqbufs()/create_bufs(). Therefore I added two simple
> functions which fixup cache hint cap and non_consistent flag after
> reqbufs()/create_bufs(). So the code looks like this now:
> 
> 	fill_buf_caps(q, &p->capabilities);
> 	ret = vb2_core_reqbufs(...);
> 	fixup_consistency_attr(q, &p->flags);
> 	fixup_cache_hints_cap(q, &p->capabilities);

These fixup functions are ugly, unless I missed something I think the
approach described above works just fine.

With these changes I think it is ready to go in.

Regards,

	Hans

> 	return ret;
> 
> 
> The rest is the pretty much the same.
> 
> Previous series:
> v4 link: https://lore.kernel.org/lkml/20200302041213.27662-1-senozhatsky@chromium.org/
> v3 link: https://lore.kernel.org/lkml/20200226111529.180197-1-senozhatsky@chromium.org
> v2 link: https://lore.kernel.org/lkml/20200204025641.218376-1-senozhatsky@chromium.org/
> v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@chromium.org/
> 
> Series Intro
> ========================================================================
> 
> 	This is a reworked version of the vb2 cache hints
> (V4L2_BUF_FLAG_NO_CACHE_INVALIDATE / V4L2_BUF_FLAG_NO_CACHE_CLEAN)
> support patch series which previsouly was developed by Sakari and
> Laurent [0].
> 
> The patch set attempts to preserve the existing behvaiour - cache
> sync is performed in ->prepare() and ->finish() (unless the buffer
> is DMA exported). User space can request “default behavior” override
> with cache management hints, which are handled on a per-buffer basis
> and should be supplied with v4l2_buffer ->flags during buffer
> preparation. There are two possible hints:
> 
> - V4L2_BUF_FLAG_NO_CACHE_INVALIDATE
> 	No cache sync on ->finish()
> 
> - V4L2_BUF_FLAG_NO_CACHE_CLEAN
> 	No cache sync on ->prepare()
> 
> In order to keep things on the safe side, we also require driver
> to explicitly state which of its queues (if any) support user space
> cache management hints (such queues should have ->allow_cache_hints
> bit set).
> 
> The patch set also (to some extent) simplifies allocators' ->prepare()
> and ->finish() callbacks. Namely, we move cache management decision
> making to the upper - core - layer. For example, if, previously, we
> would have something like this
> 
> 	vb2_buffer_done()
> 	  vb2_dc_finish()
> 	    if (buf->db_attach)
> 	      return;
> 
> where each allocators' ->finish() callback would either bail
> out (DMA exported buffer, for instance) or sync, now that "bail
> out or sync" decision is made before we call into the allocator.
> 
> Along with cache management hints, user space is also able to
> adjust queue's memory consistency attributes. Memory consistency
> attribute (dma_attrs) is per-queue, yet it plays its role on the
> allocator level, when we allocate buffers’ private memory (planes).
> For the time being, only one consistency attribute is supported:
> DMA_ATTR_NON_CONSISTENT.
> 
> [0] https://www.mail-archive.com/linux-media@vger.kernel.org/msg112459.html
> 
> Sergey Senozhatsky (13):
>   videobuf2: use explicit unsigned int in vb2_queue
>   videobuf2: add cache management members
>   videobuf2: handle V4L2 buffer cache flags
>   videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
>   videobuf2: add queue memory consistency parameter
>   videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
>   videobuf2: factor out planes prepare/finish functions
>   videobuf2: do not sync caches when we are allowed not to
>   videobuf2: check ->synced flag in prepare() and finish()
>   videobuf2: add begin/end cpu_access callbacks to dma-contig
>   videobuf2: add begin/end cpu_access callbacks to dma-sg
>   videobuf2: don't test db_attach in dma-contig prepare and finish
>   media: vivid: add cache_hints module param
> 
>  Documentation/admin-guide/media/vivid.rst     |   9 ++
>  .../userspace-api/media/v4l/buffer.rst        |  41 +++++-
>  .../media/v4l/vidioc-create-bufs.rst          |   7 +-
>  .../media/v4l/vidioc-reqbufs.rst              |  20 ++-
>  .../media/common/videobuf2/videobuf2-core.c   | 121 +++++++++++++-----
>  .../common/videobuf2/videobuf2-dma-contig.c   |  41 +++++-
>  .../media/common/videobuf2/videobuf2-dma-sg.c |  38 ++++--
>  .../media/common/videobuf2/videobuf2-v4l2.c   |  93 +++++++++++++-
>  drivers/media/dvb-core/dvb_vb2.c              |   2 +-
>  drivers/media/test-drivers/vivid/vivid-core.c |   9 ++
>  drivers/media/test-drivers/vivid/vivid-core.h |   1 +
>  .../media/test-drivers/vivid/vivid-meta-cap.c |   5 +
>  .../media/test-drivers/vivid/vivid-meta-out.c |   5 +
>  .../media/test-drivers/vivid/vivid-sdr-cap.c  |   7 +
>  .../test-drivers/vivid/vivid-touch-cap.c      |   5 +
>  .../media/test-drivers/vivid/vivid-vbi-cap.c  |   5 +
>  .../media/test-drivers/vivid/vivid-vbi-out.c  |   5 +
>  .../media/test-drivers/vivid/vivid-vid-cap.c  |   5 +
>  .../media/test-drivers/vivid/vivid-vid-out.c  |   5 +
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |   9 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c          |   5 +-
>  include/media/videobuf2-core.h                |  47 +++++--
>  include/uapi/linux/videodev2.h                |  13 +-
>  23 files changed, 426 insertions(+), 72 deletions(-)
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags
  2020-04-28 14:47 ` [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Hans Verkuil
@ 2020-05-14 15:36   ` Sergey Senozhatsky
  0 siblings, 0 replies; 16+ messages in thread
From: Sergey Senozhatsky @ 2020-05-14 15:36 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sergey Senozhatsky, Hans Verkuil, Mauro Carvalho Chehab,
	Tomasz Figa, linux-media, linux-kernel

On (20/04/28 16:47), Hans Verkuil wrote:
> Hi Sergey,
> 
> On 24/04/2020 11:29, Sergey Senozhatsky wrote:

[..]

> I missed that. What should happen is that q->allow_cache_hints is set by the
> driver before vb2_queue_init is called. And the documentation should be updated
> to say that the V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS flag is only valid when using the
> MMAP streaming I/O model.
> 
> Perhaps the flag should be renamed to V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS to
> make this explicit? Other opinions are welcome.
> 
> > 
> > Second. Even if the queue is setup, we still can report wrong cache
> > hint values. Let's look at the following code
> > 
> > 	fill_buf_caps(q, &p->capabilities);
> > 	if (!vb2_queue_allows_cache_hints(q))
> > 		p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
> 
> The problem here is that vb2_queue_allows_cache_hints(q) uses stale information:
> the current streaming mode instead of the requested streaming mode.
> 
> This should read:
> 
> 	if (!q->allow_cache_hints || p->memory != V4L2_MEMORY_MMAP)
> 		p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
> 
> And V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS is always set regardless of the
> memory model. It just needs to be documented that this capability applies
> to MMAP mode only.
> 
> > 	ret = vb2_core_reqbufs(...);
> > 	return ret;
> > 
> > The thing here is that vb2_core_reqbufs() and vb2_core_create_bufs()
> > can re-initialize the queue and invoke ->queue_setup(), possibly
> > changing its memory model, etc. so cache hints cap which we set or
> > clear before vb2_core_reqbufs() and vb2_core_create_bufs() can become
> > invalid after we call those functions. It's the same with
> > ``req->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT``, we cannot clear
> > it before reqbufs()/create_bufs(). Therefore I added two simple
> > functions which fixup cache hint cap and non_consistent flag after
> > reqbufs()/create_bufs(). So the code looks like this now:
> > 
> > 	fill_buf_caps(q, &p->capabilities);
> > 	ret = vb2_core_reqbufs(...);
> > 	fixup_consistency_attr(q, &p->flags);
> > 	fixup_cache_hints_cap(q, &p->capabilities);
> 
> These fixup functions are ugly, unless I missed something I think the
> approach described above works just fine.
> 
> With these changes I think it is ready to go in.

ACK to all of these.
Will send the updated patch set shortly.

	-ss

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-05-14 15:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-24  9:29 [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 01/13] videobuf2: use explicit unsigned int in vb2_queue Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 02/13] videobuf2: add cache management members Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 03/13] videobuf2: handle V4L2 buffer cache flags Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 04/13] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 05/13] videobuf2: add queue memory consistency parameter Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 06/13] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 07/13] videobuf2: factor out planes prepare/finish functions Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 08/13] videobuf2: do not sync caches when we are allowed not to Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 09/13] videobuf2: check ->synced flag in prepare() and finish() Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 10/13] videobuf2: add begin/end cpu_access callbacks to dma-contig Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 11/13] videobuf2: add begin/end cpu_access callbacks to dma-sg Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 12/13] videobuf2: don't test db_attach in dma-contig prepare and finish Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 13/13] media: vivid: add cache_hints module param Sergey Senozhatsky
2020-04-28 14:47 ` [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Hans Verkuil
2020-05-14 15:36   ` Sergey Senozhatsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).