* [PATCHv3 01/11] videobuf2: add cache management members
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-27 11:55 ` Hans Verkuil
2020-02-26 11:15 ` [PATCHv3 02/11] videobuf2: handle V4L2 buffer cache flags Sergey Senozhatsky
` (10 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 memebers ->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 a2b2208b02da..4a19170672ac 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
@@ -564,6 +573,7 @@ struct vb2_queue {
unsigned requires_requests:1;
unsigned uses_qbuf:1;
unsigned uses_requests:1;
+ unsigned allow_cache_hints:1;
struct mutex *lock;
void *owner;
--
2.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCHv3 01/11] videobuf2: add cache management members
2020-02-26 11:15 ` [PATCHv3 01/11] videobuf2: add cache management members Sergey Senozhatsky
@ 2020-02-27 11:55 ` Hans Verkuil
2020-02-28 1:25 ` Sergey Senozhatsky
0 siblings, 1 reply; 25+ messages in thread
From: Hans Verkuil @ 2020-02-27 11:55 UTC (permalink / raw)
To: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On 2/26/20 12:15 PM, Sergey Senozhatsky wrote:
> 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 memebers ->need_cache_sync_on_prepare and
memebers -> members
Regards,
Hans
> ->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 a2b2208b02da..4a19170672ac 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
> @@ -564,6 +573,7 @@ struct vb2_queue {
> unsigned requires_requests:1;
> unsigned uses_qbuf:1;
> unsigned uses_requests:1;
> + unsigned allow_cache_hints:1;
>
> struct mutex *lock;
> void *owner;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 01/11] videobuf2: add cache management members
2020-02-27 11:55 ` Hans Verkuil
@ 2020-02-28 1:25 ` Sergey Senozhatsky
0 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-28 1:25 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa,
Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On (20/02/27 12:55), Hans Verkuil wrote:
> On 2/26/20 12:15 PM, Sergey Senozhatsky wrote:
> > 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 memebers ->need_cache_sync_on_prepare and
>
> memebers -> members
>
meme bears :)
-ss
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCHv3 02/11] videobuf2: handle V4L2 buffer cache flags
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 01/11] videobuf2: add cache management members Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-27 11:58 ` Hans Verkuil
2020-02-26 11:15 ` [PATCHv3 03/11] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
` (9 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 skip ->finish() cache flush.
There are two possible alternative approaches:
- The first one is to move cache sync from ->finish() to dqbuf().
But this breaks some drivers, that need to fix-up buffers before
dequeueing them.
- The second one is to move ->finish() call from ->done() to dqbuf.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
.../media/common/videobuf2/videobuf2-v4l2.c | 49 +++++++++++++++++++
include/media/videobuf2-core.h | 11 +++++
2 files changed, 60 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index eb5d5db96552..2a604bd7793a 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -199,6 +199,15 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b
vbuf->request_fd = -1;
vbuf->is_held = false;
+ /*
+ * Clear buffer cache flags if queue does not support user space hints.
+ * That's to indicate to userspace that these flags won't work.
+ */
+ if (!vb2_queue_allows_cache_hints(q)) {
+ b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
+ b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_CLEAN;
+ }
+
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
switch (b->memory) {
case VB2_MEMORY_USERPTR:
@@ -337,6 +346,45 @@ 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))
+ 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 +429,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 4a19170672ac..d307869bfb15 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -632,6 +632,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 != 0) && (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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCHv3 02/11] videobuf2: handle V4L2 buffer cache flags
2020-02-26 11:15 ` [PATCHv3 02/11] videobuf2: handle V4L2 buffer cache flags Sergey Senozhatsky
@ 2020-02-27 11:58 ` Hans Verkuil
2020-02-28 1:20 ` Sergey Senozhatsky
0 siblings, 1 reply; 25+ messages in thread
From: Hans Verkuil @ 2020-02-27 11:58 UTC (permalink / raw)
To: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On 2/26/20 12:15 PM, Sergey Senozhatsky wrote:
> 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 skip ->finish() cache flush.
>
> There are two possible alternative approaches:
> - The first one is to move cache sync from ->finish() to dqbuf().
> But this breaks some drivers, that need to fix-up buffers before
> dequeueing them.
>
> - The second one is to move ->finish() call from ->done() to dqbuf.
It's not clear what the purpose is of describing these alternative approaches.
I'd just drop that. It's a bit confusing.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
> .../media/common/videobuf2/videobuf2-v4l2.c | 49 +++++++++++++++++++
> include/media/videobuf2-core.h | 11 +++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index eb5d5db96552..2a604bd7793a 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -199,6 +199,15 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b
> vbuf->request_fd = -1;
> vbuf->is_held = false;
>
> + /*
> + * Clear buffer cache flags if queue does not support user space hints.
> + * That's to indicate to userspace that these flags won't work.
> + */
> + if (!vb2_queue_allows_cache_hints(q)) {
> + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_INVALIDATE;
> + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_CLEAN;
> + }
> +
> if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
> switch (b->memory) {
> case VB2_MEMORY_USERPTR:
> @@ -337,6 +346,45 @@ 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))
> + 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 +429,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 4a19170672ac..d307869bfb15 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -632,6 +632,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 != 0) && (q->memory == VB2_MEMORY_MMAP);
Simply to:
return q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP;
Regards,
Hans
> +}
> +
> /**
> * vb2_plane_vaddr() - Return a kernel virtual address of a given plane.
> * @vb: pointer to &struct vb2_buffer to which the plane in
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 02/11] videobuf2: handle V4L2 buffer cache flags
2020-02-27 11:58 ` Hans Verkuil
@ 2020-02-28 1:20 ` Sergey Senozhatsky
0 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-28 1:20 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa,
Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On (20/02/27 12:58), Hans Verkuil wrote:
[..]
> > There are two possible alternative approaches:
> > - The first one is to move cache sync from ->finish() to dqbuf().
> > But this breaks some drivers, that need to fix-up buffers before
> > dequeueing them.
> >
> > - The second one is to move ->finish() call from ->done() to dqbuf.
>
> It's not clear what the purpose is of describing these alternative approaches.
> I'd just drop that. It's a bit confusing.
OK.
> > +/**
> > + * 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 != 0) && (q->memory == VB2_MEMORY_MMAP);
>
> Simply to:
>
> return q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP;
>
OK. I saw vb2_is_busy()
static inline bool vb2_is_busy(struct vb2_queue *q)
{
return (q->num_buffers > 0);
}
in the very same header file and concluded that maybe this is the
preferred style.
-ss
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCHv3 03/11] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 01/11] videobuf2: add cache management members Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 02/11] videobuf2: handle V4L2 buffer cache flags Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-27 12:25 ` Hans Verkuil
2020-02-26 11:15 ` [PATCHv3 04/11] videobuf2: add queue memory consistency parameter Sergey Senozhatsky
` (8 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
Documentation/media/uapi/v4l/buffer.rst | 29 +++++++++++++++++++
.../media/uapi/v4l/vidioc-reqbufs.rst | 7 +++++
.../media/common/videobuf2/videobuf2-v4l2.c | 2 ++
include/uapi/linux/videodev2.h | 3 ++
4 files changed, 41 insertions(+)
diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
index 3112300c2fa0..6b629ac59bf2 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -681,6 +681,35 @@ 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
+ - vb2 buffer is allocated either in consistent (it will be automatically
+ coherent between CPU and bus) or non-consistent memory. The latter
+ can provide performance gains, for instance CPU cache sync/flush
+ operations can be avoided if the buffer is accessed by the corresponding
+ device only and CPU does not read/write to/from that buffer. However,
+ this requires extra care from the driver -- it must guarantee memory
+ consistency by issuing cache flush/sync when consistency is needed.
+ If this flag is set V4L2 will attempt to allocate vb2 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
+ :ref:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.
.. c:type:: v4l2_memory
diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index d0c643db477a..917df6fb6486 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -156,6 +156,13 @@ 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 when the queue/buffer support memory consistency and cache
+ management hints. See :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT`,
+ :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE` and
+ :ref:`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 2a604bd7793a..f91cba37e223 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -711,6 +711,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 != 0) && (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 5f9357dcb060..e92c29864730 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 {
/*
@@ -946,6 +948,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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCHv3 03/11] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-26 11:15 ` [PATCHv3 03/11] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
@ 2020-02-27 12:25 ` Hans Verkuil
2020-02-28 1:39 ` Sergey Senozhatsky
0 siblings, 1 reply; 25+ messages in thread
From: Hans Verkuil @ 2020-02-27 12:25 UTC (permalink / raw)
To: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On 2/26/20 12:15 PM, Sergey Senozhatsky wrote:
> 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.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
> Documentation/media/uapi/v4l/buffer.rst | 29 +++++++++++++++++++
> .../media/uapi/v4l/vidioc-reqbufs.rst | 7 +++++
> .../media/common/videobuf2/videobuf2-v4l2.c | 2 ++
> include/uapi/linux/videodev2.h | 3 ++
> 4 files changed, 41 insertions(+)
>
> diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
> index 3112300c2fa0..6b629ac59bf2 100644
> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -681,6 +681,35 @@ 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
> + - vb2 buffer is allocated either in consistent (it will be automatically
vb2 -> A
(vb2 is a kAPI term, and shouldn't be used in uAPI documentation)
> + coherent between CPU and bus) or non-consistent memory. The latter
CPU and bus -> the CPU and the bus
> + can provide performance gains, for instance CPU cache sync/flush
CPU -> the CPU
> + operations can be avoided if the buffer is accessed by the corresponding
> + device only and CPU does not read/write to/from that buffer. However,
CPU -> the CPU
> + this requires extra care from the driver -- it must guarantee memory
> + consistency by issuing cache flush/sync when consistency is needed.
cache -> a cache
> + If this flag is set V4L2 will attempt to allocate vb2 buffer in
vb2 -> the
> + non-consistent memory. The flag takes effect only if the buffer is
> + used for :ref:`memory mapping <mmap>` I/O and the queue reports
reports -> reports the
> + :ref:`V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS` capability.
>
> .. c:type:: v4l2_memory
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> index d0c643db477a..917df6fb6486 100644
> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> @@ -156,6 +156,13 @@ 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 when the queue/buffer support memory consistency and cache
support -> supports
> + management hints. See :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT`,
> + :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE` and
> + :ref:`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 2a604bd7793a..f91cba37e223 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -711,6 +711,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 != 0) && (q->io_modes & VB2_MMAP))
Just say:
if (q->allow_cache_hints && (q->io_modes & VB2_MMAP))
Regards,
Hans
> + *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 5f9357dcb060..e92c29864730 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 {
> /*
> @@ -946,6 +948,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
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 03/11] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-27 12:25 ` Hans Verkuil
@ 2020-02-28 1:39 ` Sergey Senozhatsky
0 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-28 1:39 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa,
Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On (20/02/27 13:25), Hans Verkuil wrote:
[..]
> > + - vb2 buffer is allocated either in consistent (it will be automatically
>
> vb2 -> A
>
> (vb2 is a kAPI term, and shouldn't be used in uAPI documentation)
>
> > + coherent between CPU and bus) or non-consistent memory. The latter
>
> CPU and bus -> the CPU and the bus
>
> > + can provide performance gains, for instance CPU cache sync/flush
>
> CPU -> the CPU
>
> > + operations can be avoided if the buffer is accessed by the corresponding
> > + device only and CPU does not read/write to/from that buffer. However,
>
> CPU -> the CPU
>
> > + this requires extra care from the driver -- it must guarantee memory
> > + consistency by issuing cache flush/sync when consistency is needed.
>
> cache -> a cache
>
> > + If this flag is set V4L2 will attempt to allocate vb2 buffer in
>
> vb2 -> the
>
> > + non-consistent memory. The flag takes effect only if the buffer is
> > + used for :ref:`memory mapping <mmap>` I/O and the queue reports
>
> reports -> reports the
OK.
> > +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> > @@ -156,6 +156,13 @@ 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 when the queue/buffer support memory consistency and cache
>
> support -> supports
OK.
> > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > @@ -711,6 +711,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 != 0) && (q->io_modes & VB2_MMAP))
>
> Just say:
>
> if (q->allow_cache_hints && (q->io_modes & VB2_MMAP))
OK.
-ss
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCHv3 04/11] videobuf2: add queue memory consistency parameter
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (2 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 03/11] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
` (7 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 | 47 +++++++++++++++----
.../media/common/videobuf2/videobuf2-v4l2.c | 6 +--
drivers/media/dvb-core/dvb_vb2.c | 2 +-
include/media/videobuf2-core.h | 7 ++-
4 files changed, 47 insertions(+), 15 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 4489744fbbd9..3ca0545db7ee 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -664,8 +664,19 @@ 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;
+}
+
int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
- unsigned int *count)
+ bool consistent_mem, unsigned int *count)
{
unsigned int num_buffers, allocated_buffers, num_planes = 0;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
@@ -720,6 +731,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME);
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
q->memory = memory;
+ set_queue_consistency(q, consistent_mem);
/*
* Ask the driver how many buffers and planes per buffer it requires.
@@ -803,9 +815,21 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
}
EXPORT_SYMBOL_GPL(vb2_core_reqbufs);
+static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem)
+{
+ bool queue_attr = q->dma_attrs & DMA_ATTR_NON_CONSISTENT;
+
+ if (consistent_mem != queue_attr) {
+ dprintk(1, "memory consistency model mismatch\n");
+ return false;
+ }
+ return true;
+}
+
int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
- unsigned int *count, unsigned requested_planes,
- const unsigned requested_sizes[])
+ bool consistent_mem, unsigned int *count,
+ unsigned requested_planes,
+ const unsigned requested_sizes[])
{
unsigned int num_planes = 0, num_buffers, allocated_buffers;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
@@ -823,10 +847,15 @@ 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;
+ set_queue_consistency(q, consistent_mem);
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);
@@ -2498,7 +2527,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, true, &fileio->count);
if (ret)
goto err_kfree;
@@ -2555,7 +2584,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, true, &fileio->count);
err_kfree:
q->fileio = NULL;
@@ -2575,7 +2604,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, true, &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 f91cba37e223..cc31629c06dc 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -724,7 +724,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, true, &req->count);
}
EXPORT_SYMBOL_GPL(vb2_reqbufs);
@@ -798,7 +798,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);
@@ -974,7 +974,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, true, &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..e60063652164 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, true, &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 d307869bfb15..d7fbf2ae9979 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -737,6 +737,7 @@ 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.
+ * @consistent_mem: memory consistency model.
* @count: requested buffer count.
*
* Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called
@@ -761,12 +762,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);
+ bool consistent_mem, 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.
+ * @consistent_mem: memory consistency model.
* @count: requested buffer count.
* @requested_planes: number of planes requested.
* @requested_sizes: array with the size of the planes.
@@ -784,7 +786,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,
+ bool consistent_mem, unsigned int *count,
+ unsigned int requested_planes,
const unsigned int requested_sizes[]);
/**
--
2.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (3 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 04/11] videobuf2: add queue memory consistency parameter Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-27 12:36 ` Hans Verkuil
2020-02-26 11:15 ` [PATCHv3 06/11] videobuf2: factor out planes prepare/finish functions Sergey Senozhatsky
` (6 subsequent siblings)
11 siblings, 1 reply; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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.
= 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/uapi/v4l/vidioc-create-bufs.rst | 8 ++++-
.../media/uapi/v4l/vidioc-reqbufs.rst | 15 +++++++--
.../media/common/videobuf2/videobuf2-v4l2.c | 31 +++++++++++++++++--
drivers/media/v4l2-core/v4l2-ioctl.c | 5 +--
include/uapi/linux/videodev2.h | 8 +++--
5 files changed, 54 insertions(+), 13 deletions(-)
diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
index bd08e4f77ae4..6a8a4d5de2f1 100644
--- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
@@ -121,7 +121,13 @@ 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`. Old drivers and applications must set it to
+ zero.
+
+ * - __u32
+ - ``reserved``\ [6]
- A place holder for future extensions. Drivers and applications
must set the array to zero.
diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index 917df6fb6486..e52cc4401fba 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -112,10 +112,19 @@ 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.
- * - __u32
+ * - union
+ - (anonymous)
+ * -
+ - __u32
+ - ``flags``\ [1]
+ - Specifies additional buffer management attributes.
+ See :ref:`memory-flags`. Old drivers and applications must set it to
+ zero.
+
+ * -
+ - __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-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index cc31629c06dc..4954c9fc678d 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -722,9 +722,18 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
{
int ret = vb2_verify_memory_type(q, req->memory, req->type);
+ bool consistent = true;
+
+ if (!vb2_queue_allows_cache_hints(q))
+ req->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
+
+ if (req->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
+ consistent = false;
fill_buf_caps(q, &req->capabilities);
- return ret ? ret : vb2_core_reqbufs(q, req->memory, true, &req->count);
+ if (ret)
+ return ret;
+ return vb2_core_reqbufs(q, req->memory, consistent, &req->count);
}
EXPORT_SYMBOL_GPL(vb2_reqbufs);
@@ -753,6 +762,7 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
unsigned requested_sizes[VIDEO_MAX_PLANES];
struct v4l2_format *f = &create->format;
int ret = vb2_verify_memory_type(q, create->memory, f->type);
+ bool consistent = true;
unsigned i;
fill_buf_caps(q, &create->capabilities);
@@ -798,7 +808,14 @@ 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,
+
+ if (!vb2_queue_allows_cache_hints(q))
+ create->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
+
+ if (create->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
+ consistent = false;
+
+ return ret ? ret : vb2_core_create_bufs(q, create->memory, consistent,
&create->count, requested_planes, requested_sizes);
}
EXPORT_SYMBOL_GPL(vb2_create_bufs);
@@ -968,13 +985,21 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
{
struct video_device *vdev = video_devdata(file);
int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type);
+ bool consistent = true;
fill_buf_caps(vdev->queue, &p->capabilities);
if (res)
return res;
if (vb2_queue_is_busy(vdev, file))
return -EBUSY;
- res = vb2_core_reqbufs(vdev->queue, p->memory, true, &p->count);
+
+ if (!vb2_queue_allows_cache_hints(vdev->queue))
+ p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
+
+ if (p->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
+ consistent = false;
+
+ res = vb2_core_reqbufs(vdev->queue, p->memory, consistent, &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/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index fbcc7a20eedf..53b87bfd50d7 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1973,9 +1973,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);
}
@@ -2015,7 +2012,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 e92c29864730..12b1bd220347 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -938,7 +938,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 */
@@ -2446,7 +2449,8 @@ struct v4l2_create_buffers {
__u32 memory;
struct v4l2_format format;
__u32 capabilities;
- __u32 reserved[7];
+ __u32 flags;
+ __u32 reserved[6];
};
/*
--
2.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-26 11:15 ` [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
@ 2020-02-27 12:36 ` Hans Verkuil
2020-02-28 1:38 ` Sergey Senozhatsky
2020-02-28 3:57 ` Sergey Senozhatsky
0 siblings, 2 replies; 25+ messages in thread
From: Hans Verkuil @ 2020-02-27 12:36 UTC (permalink / raw)
To: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On 2/26/20 12:15 PM, Sergey Senozhatsky wrote:
> 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.
>
> = 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/uapi/v4l/vidioc-create-bufs.rst | 8 ++++-
> .../media/uapi/v4l/vidioc-reqbufs.rst | 15 +++++++--
> .../media/common/videobuf2/videobuf2-v4l2.c | 31 +++++++++++++++++--
> drivers/media/v4l2-core/v4l2-ioctl.c | 5 +--
> include/uapi/linux/videodev2.h | 8 +++--
> 5 files changed, 54 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> index bd08e4f77ae4..6a8a4d5de2f1 100644
> --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> @@ -121,7 +121,13 @@ 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`. Old drivers and applications must set it to
> + zero.
Drop the last sentence, it's not relevant.
> +
> + * - __u32
> + - ``reserved``\ [6]
> - A place holder for future extensions. Drivers and applications
> must set the array to zero.
Old drivers and applications still think reserved is [7] and will zero this.
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> index 917df6fb6486..e52cc4401fba 100644
> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> @@ -112,10 +112,19 @@ 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.
> - * - __u32
> + * - union
> + - (anonymous)
Anonymous unions are formatted a bit differently (I made a very recent patch
that unified the union formatting in the v4l docs). See e.g.
Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst.
> + * -
> + - __u32
> + - ``flags``\ [1]
> + - Specifies additional buffer management attributes.
> + See :ref:`memory-flags`. Old drivers and applications must set it to
> + zero.
Ditto.
> +
> + * -
> + - __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-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index cc31629c06dc..4954c9fc678d 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -722,9 +722,18 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
> int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> {
> int ret = vb2_verify_memory_type(q, req->memory, req->type);
> + bool consistent = true;
> +
> + if (!vb2_queue_allows_cache_hints(q))
> + req->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
> +
> + if (req->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
> + consistent = false;
>
> fill_buf_caps(q, &req->capabilities);
> - return ret ? ret : vb2_core_reqbufs(q, req->memory, true, &req->count);
> + if (ret)
> + return ret;
> + return vb2_core_reqbufs(q, req->memory, consistent, &req->count);
> }
> EXPORT_SYMBOL_GPL(vb2_reqbufs);
>
> @@ -753,6 +762,7 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
> unsigned requested_sizes[VIDEO_MAX_PLANES];
> struct v4l2_format *f = &create->format;
> int ret = vb2_verify_memory_type(q, create->memory, f->type);
> + bool consistent = true;
> unsigned i;
>
> fill_buf_caps(q, &create->capabilities);
> @@ -798,7 +808,14 @@ 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,
> +
> + if (!vb2_queue_allows_cache_hints(q))
> + create->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
> +
> + if (create->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
> + consistent = false;
> +
> + return ret ? ret : vb2_core_create_bufs(q, create->memory, consistent,
> &create->count, requested_planes, requested_sizes);
> }
> EXPORT_SYMBOL_GPL(vb2_create_bufs);
> @@ -968,13 +985,21 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
> {
> struct video_device *vdev = video_devdata(file);
> int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type);
> + bool consistent = true;
>
> fill_buf_caps(vdev->queue, &p->capabilities);
> if (res)
> return res;
> if (vb2_queue_is_busy(vdev, file))
> return -EBUSY;
> - res = vb2_core_reqbufs(vdev->queue, p->memory, true, &p->count);
> +
> + if (!vb2_queue_allows_cache_hints(vdev->queue))
> + p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
> +
> + if (p->flags & V4L2_FLAG_MEMORY_NON_CONSISTENT)
> + consistent = false;
> +
> + res = vb2_core_reqbufs(vdev->queue, p->memory, consistent, &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/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index fbcc7a20eedf..53b87bfd50d7 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1973,9 +1973,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);
> }
>
> @@ -2015,7 +2012,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 e92c29864730..12b1bd220347 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -938,7 +938,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 */
> @@ -2446,7 +2449,8 @@ struct v4l2_create_buffers {
> __u32 memory;
> struct v4l2_format format;
> __u32 capabilities;
> - __u32 reserved[7];
> + __u32 flags;
> + __u32 reserved[6];
> };
>
> /*
>
Regards,
Hans
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-27 12:36 ` Hans Verkuil
@ 2020-02-28 1:38 ` Sergey Senozhatsky
2020-02-28 8:49 ` Hans Verkuil
2020-02-28 3:57 ` Sergey Senozhatsky
1 sibling, 1 reply; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-28 1:38 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa,
Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On (20/02/27 13:36), Hans Verkuil wrote:
> > diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> > index bd08e4f77ae4..6a8a4d5de2f1 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> > @@ -121,7 +121,13 @@ 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`. Old drivers and applications must set it to
> > + zero.
>
> Drop the last sentence, it's not relevant.
OK.
> > +
> > + * - __u32
> > + - ``reserved``\ [6]
> > - A place holder for future extensions. Drivers and applications
> > must set the array to zero.
>
> Old drivers and applications still think reserved is [7] and will zero this.
OK.
Hmm... If those apps use hard-coded size then we might have a problem.
If they use sizeof(reserved) then everything is OK. Shall we also have
a union here?
> > diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> > index 917df6fb6486..e52cc4401fba 100644
> > --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> > +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> > @@ -112,10 +112,19 @@ 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.
> > - * - __u32
> > + * - union
> > + - (anonymous)
>
> Anonymous unions are formatted a bit differently (I made a very recent patch
> that unified the union formatting in the v4l docs). See e.g.
> Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst.
OK, will take a look.
-ss
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-28 1:38 ` Sergey Senozhatsky
@ 2020-02-28 8:49 ` Hans Verkuil
0 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2020-02-28 8:49 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Hans Verkuil, Tomasz Figa, Mauro Carvalho Chehab, Kyungmin Park,
Marek Szyprowski, Sakari Ailus, Laurent Pinchart, Pawel Osciak,
linux-media, linux-kernel
On 2/28/20 2:38 AM, Sergey Senozhatsky wrote:
> On (20/02/27 13:36), Hans Verkuil wrote:
>>> diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>>> index bd08e4f77ae4..6a8a4d5de2f1 100644
>>> --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>>> +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>>> @@ -121,7 +121,13 @@ 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`. Old drivers and applications must set it to
>>> + zero.
>>
>> Drop the last sentence, it's not relevant.
>
> OK.
>
>>> +
>>> + * - __u32
>>> + - ``reserved``\ [6]
>>> - A place holder for future extensions. Drivers and applications
>>> must set the array to zero.
>>
>> Old drivers and applications still think reserved is [7] and will zero this.
>
> OK.
>
> Hmm... If those apps use hard-coded size then we might have a problem.
> If they use sizeof(reserved) then everything is OK. Shall we also have
> a union here?
No, apps will use sizeof(reserved).
Regards,
Hans
>
>>> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>>> index 917df6fb6486..e52cc4401fba 100644
>>> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>>> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>>> @@ -112,10 +112,19 @@ 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.
>>> - * - __u32
>>> + * - union
>>> + - (anonymous)
>>
>> Anonymous unions are formatted a bit differently (I made a very recent patch
>> that unified the union formatting in the v4l docs). See e.g.
>> Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst.
>
> OK, will take a look.
>
> -ss
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-27 12:36 ` Hans Verkuil
2020-02-28 1:38 ` Sergey Senozhatsky
@ 2020-02-28 3:57 ` Sergey Senozhatsky
2020-02-28 8:50 ` Hans Verkuil
1 sibling, 1 reply; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-28 3:57 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa,
Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On (20/02/27 13:36), Hans Verkuil wrote:
[..]
> > 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`. Old drivers and applications must set it to
> > + zero.
>
> Drop the last sentence, it's not relevant.
>
> > +
> > + * - __u32
> > + - ``reserved``\ [6]
> > - A place holder for future extensions. Drivers and applications
> > must set the array to zero.
>
> Old drivers and applications still think reserved is [7] and will zero this.
Just to make sure, does this mean that you also want me to drop the
"Drivers and applications must set the array to zero" sentence?
-ss
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-28 3:57 ` Sergey Senozhatsky
@ 2020-02-28 8:50 ` Hans Verkuil
2020-02-28 11:39 ` Sergey Senozhatsky
0 siblings, 1 reply; 25+ messages in thread
From: Hans Verkuil @ 2020-02-28 8:50 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Hans Verkuil, Tomasz Figa, Mauro Carvalho Chehab, Kyungmin Park,
Marek Szyprowski, Sakari Ailus, Laurent Pinchart, Pawel Osciak,
linux-media, linux-kernel
On 2/28/20 4:57 AM, Sergey Senozhatsky wrote:
> On (20/02/27 13:36), Hans Verkuil wrote:
> [..]
>>> 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`. Old drivers and applications must set it to
>>> + zero.
>>
>> Drop the last sentence, it's not relevant.
>>
>>> +
>>> + * - __u32
>>> + - ``reserved``\ [6]
>>> - A place holder for future extensions. Drivers and applications
>>> must set the array to zero.
>>
>> Old drivers and applications still think reserved is [7] and will zero this.
>
> Just to make sure, does this mean that you also want me to drop the
> "Drivers and applications must set the array to zero" sentence?
Not for the reserved field, only for the flags field.
Regards,
Hans
>
> -ss
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
2020-02-28 8:50 ` Hans Verkuil
@ 2020-02-28 11:39 ` Sergey Senozhatsky
0 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-28 11:39 UTC (permalink / raw)
To: Hans Verkuil
Cc: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa,
Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On (20/02/28 09:50), Hans Verkuil wrote:
> On 2/28/20 4:57 AM, Sergey Senozhatsky wrote:
> > On (20/02/27 13:36), Hans Verkuil wrote:
> > [..]
[..]
> >>> + * - __u32
> >>> + - ``reserved``\ [6]
> >>> - A place holder for future extensions. Drivers and applications
> >>> must set the array to zero.
> >>
> >> Old drivers and applications still think reserved is [7] and will zero this.
> >
> > Just to make sure, does this mean that you also want me to drop the
> > "Drivers and applications must set the array to zero" sentence?
>
> Not for the reserved field, only for the flags field.
Got it.
So V4 is ready. Do you think it'll make sense to send it
out now or next week?
-ss
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCHv3 06/11] videobuf2: factor out planes prepare/finish functions
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (4 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 05/11] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 07/11] videobuf2: do not sync caches when we are allowed not to Sergey Senozhatsky
` (5 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 3ca0545db7ee..c2a1eadb26cf 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.
@@ -951,7 +977,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;
@@ -971,12 +996,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) {
@@ -1301,7 +1322,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) {
@@ -1345,11 +1365,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;
@@ -1969,14 +1985,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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCHv3 07/11] videobuf2: do not sync caches when we are allowed not to
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (5 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 06/11] videobuf2: factor out planes prepare/finish functions Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 08/11] videobuf2: check ->synced flag in prepare() and finish() Sergey Senozhatsky
` (4 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 c2a1eadb26cf..988e8796de4f 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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCHv3 08/11] videobuf2: check ->synced flag in prepare() and finish()
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (6 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 07/11] videobuf2: do not sync caches when we are allowed not to Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 09/11] videobuf2: add begin/end cpu_access callbacks to dma-contig Sergey Senozhatsky
` (3 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 988e8796de4f..7f637e3a50ab 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,
@@ -1991,8 +1997,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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCHv3 09/11] videobuf2: add begin/end cpu_access callbacks to dma-contig
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (7 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 08/11] videobuf2: check ->synced flag in prepare() and finish() Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 10/11] videobuf2: add begin/end cpu_access callbacks to dma-sg Sergey Senozhatsky
` (2 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 | 33 +++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index d0c9dffe49e5..a387260fb321 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,32 @@ 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 +384,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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCHv3 10/11] videobuf2: add begin/end cpu_access callbacks to dma-sg
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (8 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 09/11] videobuf2: add begin/end cpu_access callbacks to dma-contig Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-26 11:15 ` [PATCHv3 11/11] videobuf2: don't test db_attach in dma-contig prepare and finish Sergey Senozhatsky
2020-02-27 12:39 ` [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Hans Verkuil
11 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 6db60e9d5183..ddc67c9aaedb 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)
@@ -470,6 +476,26 @@ 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;
@@ -488,6 +514,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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCHv3 11/11] videobuf2: don't test db_attach in dma-contig prepare and finish
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (9 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 10/11] videobuf2: add begin/end cpu_access callbacks to dma-sg Sergey Senozhatsky
@ 2020-02-26 11:15 ` Sergey Senozhatsky
2020-02-27 12:39 ` [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Hans Verkuil
11 siblings, 0 replies; 25+ messages in thread
From: Sergey Senozhatsky @ 2020-02-26 11:15 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel, Sergey Senozhatsky
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 a387260fb321..6ea0961149d7 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 ddc67c9aaedb..2a01bc567321 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.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags
2020-02-26 11:15 [PATCHv3 00/11] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Sergey Senozhatsky
` (10 preceding siblings ...)
2020-02-26 11:15 ` [PATCHv3 11/11] videobuf2: don't test db_attach in dma-contig prepare and finish Sergey Senozhatsky
@ 2020-02-27 12:39 ` Hans Verkuil
11 siblings, 0 replies; 25+ messages in thread
From: Hans Verkuil @ 2020-02-27 12:39 UTC (permalink / raw)
To: Sergey Senozhatsky, Hans Verkuil, Tomasz Figa
Cc: Mauro Carvalho Chehab, Kyungmin Park, Marek Szyprowski,
Sakari Ailus, Laurent Pinchart, Pawel Osciak, linux-media,
linux-kernel
On 2/26/20 12:15 PM, Sergey Senozhatsky wrote:
> Hello,
>
> V3 of the patchset, reshuffled and updated.
>
> - Most notable changes:
>
> a) Added a simple helper function vb2_queue_allows_cache_hints(),
> which return true if queue has ->allow_cache_hints and when
> ->memory is VB2_MEMORY_MMAP.
> That is - user space cache and memory consistency hints are now
> specifically for MMAP buffers and queues that support hints.
>
> b) Set V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS capability bit only when queue
> has ->allow_cache_hints and ->io_modes has VB2_MMAP bit set
>
> c) Clear "incompatible" request's flags when queue does not permit
> cache and memory consistency hints (IOW, when vb2_queue_allows_cache_hints()
> return false)
>
>
> Minor changes:
>
> - Squashed V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS and V4L2_FLAG_MEMORY_NON_CONSISTENT
> patches.
>
> - Added more documentation and code comments.
>
>
> Previous series:
> 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.
v3 looks very good, I only found some minor issues.
I think v4 should be ready to be merged, unless others have more comments.
Regards,
Hans
>
> [0] https://www.mail-archive.com/linux-media@vger.kernel.org/msg112459.html
>
> Sergey Senozhatsky (11):
> 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
>
> Documentation/media/uapi/v4l/buffer.rst | 29 +++++
> .../media/uapi/v4l/vidioc-create-bufs.rst | 8 +-
> .../media/uapi/v4l/vidioc-reqbufs.rst | 22 +++-
> .../media/common/videobuf2/videobuf2-core.c | 110 +++++++++++++-----
> .../common/videobuf2/videobuf2-dma-contig.c | 39 ++++++-
> .../media/common/videobuf2/videobuf2-dma-sg.c | 36 ++++--
> .../media/common/videobuf2/videobuf2-v4l2.c | 82 ++++++++++++-
> drivers/media/dvb-core/dvb_vb2.c | 2 +-
> drivers/media/v4l2-core/v4l2-ioctl.c | 5 +-
> include/media/videobuf2-core.h | 28 ++++-
> include/uapi/linux/videodev2.h | 11 +-
> 11 files changed, 314 insertions(+), 58 deletions(-)
>
^ permalink raw reply [flat|nested] 25+ messages in thread