* [PATCHv2 0/9] vb2/cedrus: add tag support
@ 2018-11-14 13:47 Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 1/9] videodev2.h: " Hans Verkuil
` (11 more replies)
0 siblings, 12 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne
From: Hans Verkuil <hansverk@cisco.com>
As was discussed here (among other places):
https://lkml.org/lkml/2018/10/19/440
using capture queue buffer indices to refer to reference frames is
not a good idea. A better idea is to use a 'tag' where the
application can assign a u64 tag to an output buffer, which is then
copied to the capture buffer(s) derived from the output buffer.
A u64 is chosen since this allows userspace to also use pointers to
internal structures as 'tag'.
The first three patches add core tag support, the next patch document
the tag support, then a new helper function is added to v4l2-mem2mem.c
to easily copy data from a source to a destination buffer that drivers
can use.
Next a new supports_tags vb2_queue flag is added to indicate that
the driver supports tags. Ideally this should not be necessary, but
that would require that all m2m drivers are converted to using the
new helper function introduced in the previous patch. That takes more
time then I have now since we want to get this in for 4.20.
Finally the vim2m, vicodec and cedrus drivers are converted to support
tags.
I also removed the 'pad' fields from the mpeg2 control structs (it
should never been added in the first place) and aligned the structs
to a u32 boundary (u64 for the tag values).
Note that this might change further (Paul suggested using bitfields).
Also note that the cedrus code doesn't set the sequence counter, that's
something that should still be added before this driver can be moved
out of staging.
Note: if no buffer is found for a certain tag, then the dma address
is just set to 0. That happened before as well with invalid buffer
indices. This should be checked in the driver!
The previous RFC series was tested successfully with the cedrus driver.
Regards,
Hans
Changes since v1:
- changed to a u32 tag. Using a 64 bit tag was overly complicated due
to the bad layout of the v4l2_buffer struct, and there is no real
need for it by applications.
Main changes since the RFC:
- Added new buffer capability flag
- Added m2m helper to copy data between buffers
- Added documentation
- Added tag logging in v4l2-ioctl.c
Hans Verkuil (9):
videodev2.h: add tag support
vb2: add tag support
v4l2-ioctl.c: log v4l2_buffer tag
buffer.rst: document the new buffer tag feature.
v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
vb2: add new supports_tags queue flag
vim2m: add tag support
vicodec: add tag support
cedrus: add tag support
Documentation/media/uapi/v4l/buffer.rst | 32 +++++++++----
.../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++
.../media/common/videobuf2/videobuf2-v4l2.c | 45 ++++++++++++++++---
drivers/media/platform/vicodec/vicodec-core.c | 14 ++----
drivers/media/platform/vim2m.c | 14 ++----
drivers/media/v4l2-core/v4l2-ctrls.c | 9 ----
drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++--
drivers/media/v4l2-core/v4l2-mem2mem.c | 23 ++++++++++
drivers/staging/media/sunxi/cedrus/cedrus.h | 9 ++--
.../staging/media/sunxi/cedrus/cedrus_dec.c | 2 +
.../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ++++-----
.../staging/media/sunxi/cedrus/cedrus_video.c | 2 +
include/media/v4l2-mem2mem.h | 21 +++++++++
include/media/videobuf2-core.h | 2 +
include/media/videobuf2-v4l2.h | 21 ++++++++-
include/uapi/linux/v4l2-controls.h | 14 +++---
include/uapi/linux/videodev2.h | 9 +++-
17 files changed, 178 insertions(+), 73 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv2 1/9] videodev2.h: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 2/9] vb2: " Hans Verkuil
` (10 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Add support for 'tags' to struct v4l2_buffer. These can be used
by m2m devices so userspace can set a tag for an output buffer and
this value will then be copied to the capture buffer(s).
This tag can be used to refer to capture buffers, something that
is needed by stateless HW codecs.
The new V4L2_BUF_CAP_SUPPORTS_TAGS capability indicates whether
or not tags are supported.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
include/uapi/linux/videodev2.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c8e8ff810190..173a94d2cbef 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -879,6 +879,7 @@ struct v4l2_requestbuffers {
#define V4L2_BUF_CAP_SUPPORTS_USERPTR (1 << 1)
#define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2)
#define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3)
+#define V4L2_BUF_CAP_SUPPORTS_TAGS (1 << 4)
/**
* struct v4l2_plane - plane info for multi-planar buffers
@@ -923,6 +924,7 @@ struct v4l2_plane {
* @field: enum v4l2_field; field order of the image in the buffer
* @timestamp: frame timestamp
* @timecode: frame timecode
+ * @tag: buffer tag
* @sequence: sequence count of this frame
* @memory: enum v4l2_memory; the method, in which the actual video data is
* passed
@@ -950,7 +952,10 @@ struct v4l2_buffer {
__u32 flags;
__u32 field;
struct timeval timestamp;
- struct v4l2_timecode timecode;
+ union {
+ struct v4l2_timecode timecode;
+ __u32 tag;
+ };
__u32 sequence;
/* memory location */
@@ -988,6 +993,8 @@ struct v4l2_buffer {
#define V4L2_BUF_FLAG_IN_REQUEST 0x00000080
/* timecode field is valid */
#define V4L2_BUF_FLAG_TIMECODE 0x00000100
+/* tag field is valid */
+#define V4L2_BUF_FLAG_TAG 0x00000200
/* Buffer is prepared for queuing */
#define V4L2_BUF_FLAG_PREPARED 0x00000400
/* Cache handling flags */
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 2/9] vb2: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 1/9] videodev2.h: " Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 3/9] v4l2-ioctl.c: log v4l2_buffer tag Hans Verkuil
` (9 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Add support for tags to vb2. Besides just storing and setting
the tag this patch also adds the vb2_find_tag() function that
can be used to find a buffer with the given tag.
This function will only look at DEQUEUED and DONE buffers, i.e.
buffers that are already processed.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
.../media/common/videobuf2/videobuf2-v4l2.c | 43 ++++++++++++++++---
include/media/videobuf2-v4l2.h | 21 ++++++++-
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index a17033ab2c22..115f2868223a 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -50,7 +50,8 @@ module_param(debug, int, 0644);
V4L2_BUF_FLAG_TIMESTAMP_MASK)
/* Output buffer flags that should be passed on to the driver */
#define V4L2_BUFFER_OUT_FLAGS (V4L2_BUF_FLAG_PFRAME | V4L2_BUF_FLAG_BFRAME | \
- V4L2_BUF_FLAG_KEYFRAME | V4L2_BUF_FLAG_TIMECODE)
+ V4L2_BUF_FLAG_KEYFRAME | \
+ V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG)
/*
* __verify_planes_array() - verify that the planes array passed in struct
@@ -144,8 +145,11 @@ static void __copy_timestamp(struct vb2_buffer *vb, const void *pb)
*/
if (q->copy_timestamp)
vb->timestamp = timeval_to_ns(&b->timestamp);
- vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE;
- if (b->flags & V4L2_BUF_FLAG_TIMECODE)
+ vbuf->flags |= b->flags &
+ (V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG);
+ if (b->flags & V4L2_BUF_FLAG_TAG)
+ vbuf->tag = b->tag;
+ else if (b->flags & V4L2_BUF_FLAG_TIMECODE)
vbuf->timecode = b->timecode;
}
};
@@ -195,6 +199,7 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b
}
vbuf->sequence = 0;
vbuf->request_fd = -1;
+ vbuf->tag = 0;
if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
switch (b->memory) {
@@ -314,13 +319,19 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b
}
if (V4L2_TYPE_IS_OUTPUT(b->type)) {
+ if ((b->flags & V4L2_BUF_FLAG_TIMECODE) &&
+ (b->flags & V4L2_BUF_FLAG_TAG)) {
+ dprintk(1, "buffer flag TIMECODE cannot be combined with flag TAG\n");
+ return -EINVAL;
+ }
+
/*
* For output buffers mask out the timecode flag:
* this will be handled later in vb2_qbuf().
* The 'field' is valid metadata for this output buffer
* and so that needs to be copied here.
*/
- vbuf->flags &= ~V4L2_BUF_FLAG_TIMECODE;
+ vbuf->flags &= ~(V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG);
vbuf->field = b->field;
} else {
/* Zero any output buffer flags as this is a capture buffer */
@@ -461,7 +472,10 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb)
b->flags = vbuf->flags;
b->field = vbuf->field;
b->timestamp = ns_to_timeval(vb->timestamp);
- b->timecode = vbuf->timecode;
+ if (b->flags & V4L2_BUF_FLAG_TAG)
+ b->tag = vbuf->tag;
+ else if (b->flags & V4L2_BUF_FLAG_TIMECODE)
+ b->timecode = vbuf->timecode;
b->sequence = vbuf->sequence;
b->reserved2 = 0;
b->request_fd = 0;
@@ -587,6 +601,25 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
.copy_timestamp = __copy_timestamp,
};
+int vb2_find_tag(const struct vb2_queue *q, u32 tag,
+ unsigned int start_idx)
+{
+ unsigned int i;
+
+ for (i = start_idx; i < q->num_buffers; i++) {
+ struct vb2_buffer *vb = q->bufs[i];
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+ if ((vb->state == VB2_BUF_STATE_DEQUEUED ||
+ vb->state == VB2_BUF_STATE_DONE) &&
+ (vbuf->flags & V4L2_BUF_FLAG_TAG) &&
+ vbuf->tag == tag)
+ return i;
+ }
+ return -1;
+}
+EXPORT_SYMBOL_GPL(vb2_find_tag);
+
/*
* vb2_querybuf() - query video buffer information
* @q: videobuf queue
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 727855463838..c2a541af6b2c 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -31,8 +31,9 @@
* @field: field order of the image in the buffer, as defined by
* &enum v4l2_field.
* @timecode: frame timecode.
+ * @tag: user specified buffer tag value.
* @sequence: sequence count of this frame.
- * @request_fd: the request_fd associated with this buffer
+ * @request_fd: the request_fd associated with this buffer.
* @planes: plane information (userptr/fd, length, bytesused, data_offset).
*
* Should contain enough information to be able to cover all the fields
@@ -44,6 +45,7 @@ struct vb2_v4l2_buffer {
__u32 flags;
__u32 field;
struct v4l2_timecode timecode;
+ __u32 tag;
__u32 sequence;
__s32 request_fd;
struct vb2_plane planes[VB2_MAX_PLANES];
@@ -55,6 +57,23 @@ struct vb2_v4l2_buffer {
#define to_vb2_v4l2_buffer(vb) \
container_of(vb, struct vb2_v4l2_buffer, vb2_buf)
+/**
+ * vb2_find_tag() - Find buffer with given tag in the queue
+ *
+ * @q: pointer to &struct vb2_queue with videobuf2 queue.
+ * @tag: the tag to find. Only buffers in state DEQUEUED or DONE
+ * are considered.
+ * @start_idx: the start index (usually 0) in the buffer array to start
+ * searching from. Note that there may be multiple buffers
+ * with the same tag value, so you can restart the search
+ * by setting @start_idx to the previously found index + 1.
+ *
+ * Returns the buffer index of the buffer with the given @tag, or
+ * -1 if no buffer with @tag was found.
+ */
+int vb2_find_tag(const struct vb2_queue *q, u32 tag,
+ unsigned int start_idx);
+
int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
/**
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 3/9] v4l2-ioctl.c: log v4l2_buffer tag
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 1/9] videodev2.h: " Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 2/9] vb2: " Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 4/9] buffer.rst: document the new buffer tag feature Hans Verkuil
` (8 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
When debugging is on, log the new tag field of struct v4l2_buffer
as well.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index c63746968fa3..49103787d19a 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -498,9 +498,12 @@ static void v4l_print_buffer(const void *arg, bool write_only)
p->bytesused, p->m.userptr, p->length);
}
- printk(KERN_DEBUG "timecode=%02d:%02d:%02d type=%d, flags=0x%08x, frames=%d, userbits=0x%08x\n",
- tc->hours, tc->minutes, tc->seconds,
- tc->type, tc->flags, tc->frames, *(__u32 *)tc->userbits);
+ if (p->flags & V4L2_BUF_FLAG_TAG)
+ printk(KERN_DEBUG "tag=%x\n", p->tag);
+ else if (p->flags & V4L2_BUF_FLAG_TIMECODE)
+ printk(KERN_DEBUG "timecode=%02d:%02d:%02d type=%d, flags=0x%08x, frames=%d, userbits=0x%08x\n",
+ tc->hours, tc->minutes, tc->seconds,
+ tc->type, tc->flags, tc->frames, *(__u32 *)tc->userbits);
}
static void v4l_print_exportbuffer(const void *arg, bool write_only)
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 4/9] buffer.rst: document the new buffer tag feature.
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (2 preceding siblings ...)
2018-11-14 13:47 ` [PATCHv2 3/9] v4l2-ioctl.c: log v4l2_buffer tag Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 5/9] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function Hans Verkuil
` (7 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
Document V4L2_BUF_FLAG_TAG and struct v4l2_buffer_tag.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
Documentation/media/uapi/v4l/buffer.rst | 32 ++++++++++++++-----
.../media/uapi/v4l/vidioc-reqbufs.rst | 4 +++
2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
index 2e266d32470a..0d4aa83d2bce 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -220,17 +220,25 @@ struct v4l2_buffer
use ``V4L2_BUF_FLAG_TIMESTAMP_COPY`` the application has to fill
in the timestamp which will be copied by the driver to the capture
stream.
- * - struct :c:type:`v4l2_timecode`
+ * - union
+ * -
+ - struct :c:type:`v4l2_timecode`
- ``timecode``
- -
- - When ``type`` is ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` and the
- ``V4L2_BUF_FLAG_TIMECODE`` flag is set in ``flags``, this
+ - When the ``V4L2_BUF_FLAG_TIMECODE`` flag is set in ``flags``, this
structure contains a frame timecode. In
:c:type:`V4L2_FIELD_ALTERNATE <v4l2_field>` mode the top and
bottom field contain the same timecode. Timecodes are intended to
help video editing and are typically recorded on video tapes, but
also embedded in compressed formats like MPEG. This field is
independent of the ``timestamp`` and ``sequence`` fields.
+ * -
+ - __u32
+ - ``tag``
+ - When the ``V4L2_BUF_FLAG_TAG`` flag is set in ``flags``, this
+ structure contains a user-specified tag value.
+
+ It is used by stateless codecs where this tag can be used to
+ refer to buffers that contain reference frames.
* - __u32
- ``sequence``
-
@@ -567,6 +575,14 @@ Buffer Flags
when the ``VIDIOC_DQBUF`` ioctl is called. Applications can set
this bit and the corresponding ``timecode`` structure when
``type`` refers to an output stream.
+ * .. _`V4L2-BUF-FLAG-TAG`:
+
+ - ``V4L2_BUF_FLAG_TAG``
+ - 0x00000200
+ - The ``tag`` field is valid. Applications can set
+ this bit and the corresponding ``tag`` structure. If tags are
+ supported then the ``V4L2_BUF_CAP_SUPPORTS_TAGS`` capability
+ is also set.
* .. _`V4L2-BUF-FLAG-PREPARED`:
- ``V4L2_BUF_FLAG_PREPARED``
@@ -704,10 +720,10 @@ enum v4l2_memory
Timecodes
=========
-The struct :c:type:`v4l2_timecode` structure is designed to hold a
-:ref:`smpte12m` or similar timecode. (struct
-struct :c:type:`timeval` timestamps are stored in struct
-:c:type:`v4l2_buffer` field ``timestamp``.)
+The :c:type:`v4l2_buffer_timecode` structure is designed to hold a
+:ref:`smpte12m` or similar timecode.
+(struct :c:type:`timeval` timestamps are stored in the struct
+:c:type:`v4l2_buffer` ``timestamp`` field.)
.. c:type:: v4l2_timecode
diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index d4bbbb0c60e8..5090a62f324c 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -112,6 +112,7 @@ any DMA in progress, an implicit
.. _V4L2-BUF-CAP-SUPPORTS-USERPTR:
.. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
.. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
+.. _V4L2-BUF-CAP-SUPPORTS-TAGS:
.. cssclass:: longtable
@@ -132,6 +133,9 @@ any DMA in progress, an implicit
* - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
- 0x00000008
- This buffer type supports :ref:`requests <media-request-api>`.
+ * - ``V4L2_BUF_CAP_SUPPORTS_TAGS``
+ - 0x00000010
+ - This buffer type supports ``V4L2_BUF_FLAG_TAG``.
Return Value
============
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 5/9] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (3 preceding siblings ...)
2018-11-14 13:47 ` [PATCHv2 4/9] buffer.rst: document the new buffer tag feature Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 6/9] vb2: add new supports_tags queue flag Hans Verkuil
` (6 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
Memory-to-memory devices should copy various parts of
struct v4l2_buffer from the output buffer to the capture buffer.
Add a helper function that does that to simplify the driver code.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 23 +++++++++++++++++++++++
include/media/v4l2-mem2mem.h | 21 +++++++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 1ed2465972ac..bfd5321fda1c 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -953,6 +953,29 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx,
}
EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
+void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb,
+ struct vb2_v4l2_buffer *cap_vb,
+ bool copy_frame_flags)
+{
+ u32 mask = V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG |
+ V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
+
+ if (copy_frame_flags)
+ mask |= V4L2_BUF_FLAG_KEYFRAME | V4L2_BUF_FLAG_PFRAME |
+ V4L2_BUF_FLAG_BFRAME;
+
+ cap_vb->vb2_buf.timestamp = out_vb->vb2_buf.timestamp;
+
+ if (out_vb->flags & V4L2_BUF_FLAG_TAG)
+ cap_vb->tag = out_vb->tag;
+ if (out_vb->flags & V4L2_BUF_FLAG_TIMECODE)
+ cap_vb->timecode = out_vb->timecode;
+ cap_vb->field = out_vb->field;
+ cap_vb->flags &= ~mask;
+ cap_vb->flags |= out_vb->flags & mask;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_buf_copy_data);
+
void v4l2_m2m_request_queue(struct media_request *req)
{
struct media_request_object *obj, *obj_safe;
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 5467264771ec..bb4feb6969d2 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -622,6 +622,27 @@ v4l2_m2m_dst_buf_remove_by_idx(struct v4l2_m2m_ctx *m2m_ctx, unsigned int idx)
return v4l2_m2m_buf_remove_by_idx(&m2m_ctx->cap_q_ctx, idx);
}
+/**
+ * v4l2_m2m_buf_copy_data() - copy buffer data from the output buffer to the
+ * capture buffer
+ *
+ * @out_vb: the output buffer that is the source of the data.
+ * @cap_vb: the capture buffer that will receive the data.
+ * @copy_frame_flags: copy the KEY/B/PFRAME flags as well.
+ *
+ * This helper function copies the timestamp, timecode (if the TIMECODE
+ * buffer flag was set), tag (if the TAG buffer flag was set), field
+ * and the TIMECODE, TAG, KEYFRAME, BFRAME, PFRAME and TSTAMP_SRC_MASK
+ * flags from @out_vb to @cap_vb.
+ *
+ * If @copy_frame_flags is false, then the KEYFRAME, BFRAME and PFRAME
+ * flags are not copied. This is typically needed for encoders that
+ * set this bits explicitly.
+ */
+void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb,
+ struct vb2_v4l2_buffer *cap_vb,
+ bool copy_frame_flags);
+
/* v4l2 request helper */
void v4l2_m2m_request_queue(struct media_request *req);
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 6/9] vb2: add new supports_tags queue flag
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (4 preceding siblings ...)
2018-11-14 13:47 ` [PATCHv2 5/9] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 7/9] vim2m: add tag support Hans Verkuil
` (5 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
Add new flag to indicate that buffer tags are supported.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 ++
include/media/videobuf2-core.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 115f2868223a..d2d4985fb352 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -666,6 +666,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
*caps |= V4L2_BUF_CAP_SUPPORTS_DMABUF;
if (q->supports_requests)
*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
+ if (q->supports_tags)
+ *caps |= V4L2_BUF_CAP_SUPPORTS_TAGS;
}
int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index e86981d615ae..81f2dbfd0094 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -473,6 +473,7 @@ struct vb2_buf_ops {
* has not been called. This is a vb1 idiom that has been adopted
* also by vb2.
* @supports_requests: this queue supports the Request API.
+ * @supports_tags: this queue supports tags in struct v4l2_buffer.
* @uses_qbuf: qbuf was used directly for this queue. Set to 1 the first
* time this is called. Set to 0 when the queue is canceled.
* If this is 1, then you cannot queue buffers from a request.
@@ -547,6 +548,7 @@ struct vb2_queue {
unsigned allow_zero_bytesused:1;
unsigned quirk_poll_must_check_waiting_for_buffers:1;
unsigned supports_requests:1;
+ unsigned supports_tags:1;
unsigned uses_qbuf:1;
unsigned uses_requests:1;
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 7/9] vim2m: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (5 preceding siblings ...)
2018-11-14 13:47 ` [PATCHv2 6/9] vb2: add new supports_tags queue flag Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 8/9] vicodec: " Hans Verkuil
` (4 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Copy tags in vim2m.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/platform/vim2m.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index d82db738f174..9d1222f489b8 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -241,17 +241,7 @@ static int device_process(struct vim2m_ctx *ctx,
out_vb->sequence =
get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE)->sequence++;
in_vb->sequence = q_data->sequence++;
- out_vb->vb2_buf.timestamp = in_vb->vb2_buf.timestamp;
-
- if (in_vb->flags & V4L2_BUF_FLAG_TIMECODE)
- out_vb->timecode = in_vb->timecode;
- out_vb->field = in_vb->field;
- out_vb->flags = in_vb->flags &
- (V4L2_BUF_FLAG_TIMECODE |
- V4L2_BUF_FLAG_KEYFRAME |
- V4L2_BUF_FLAG_PFRAME |
- V4L2_BUF_FLAG_BFRAME |
- V4L2_BUF_FLAG_TSTAMP_SRC_MASK);
+ v4l2_m2m_buf_copy_data(out_vb, in_vb, true);
switch (ctx->mode) {
case MEM2MEM_HFLIP | MEM2MEM_VFLIP:
@@ -856,6 +846,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
src_vq->lock = &ctx->dev->dev_mutex;
src_vq->supports_requests = true;
+ src_vq->supports_tags = true;
ret = vb2_queue_init(src_vq);
if (ret)
@@ -869,6 +860,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
dst_vq->mem_ops = &vb2_vmalloc_memops;
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = &ctx->dev->dev_mutex;
+ dst_vq->supports_tags = true;
return vb2_queue_init(dst_vq);
}
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 8/9] vicodec: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (6 preceding siblings ...)
2018-11-14 13:47 ` [PATCHv2 7/9] vim2m: add tag support Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 9/9] cedrus: " Hans Verkuil
` (3 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Copy tags in vicodec.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/platform/vicodec/vicodec-core.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index b292cff26c86..72245183b077 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -190,18 +190,8 @@ static int device_process(struct vicodec_ctx *ctx,
}
out_vb->sequence = q_cap->sequence++;
- out_vb->vb2_buf.timestamp = in_vb->vb2_buf.timestamp;
-
- if (in_vb->flags & V4L2_BUF_FLAG_TIMECODE)
- out_vb->timecode = in_vb->timecode;
- out_vb->field = in_vb->field;
out_vb->flags &= ~V4L2_BUF_FLAG_LAST;
- out_vb->flags |= in_vb->flags &
- (V4L2_BUF_FLAG_TIMECODE |
- V4L2_BUF_FLAG_KEYFRAME |
- V4L2_BUF_FLAG_PFRAME |
- V4L2_BUF_FLAG_BFRAME |
- V4L2_BUF_FLAG_TSTAMP_SRC_MASK);
+ v4l2_m2m_buf_copy_data(in_vb, out_vb, !ctx->is_enc);
return 0;
}
@@ -1066,6 +1056,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
src_vq->lock = ctx->is_enc ? &ctx->dev->enc_mutex :
&ctx->dev->dec_mutex;
+ src_vq->supports_tags = true;
ret = vb2_queue_init(src_vq);
if (ret)
@@ -1081,6 +1072,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
dst_vq->mem_ops = &vb2_vmalloc_memops;
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = src_vq->lock;
+ dst_vq->supports_tags = true;
return vb2_queue_init(dst_vq);
}
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 9/9] cedrus: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (7 preceding siblings ...)
2018-11-14 13:47 ` [PATCHv2 8/9] vicodec: " Hans Verkuil
@ 2018-11-14 13:47 ` Hans Verkuil
2018-11-16 9:00 ` [PATCHv2 0/9] vb2/cedrus: " Tomasz Figa
` (2 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-14 13:47 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne, Hans Verkuil
Replace old reference frame indices by new tag method.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
drivers/media/v4l2-core/v4l2-ctrls.c | 9 --------
drivers/staging/media/sunxi/cedrus/cedrus.h | 9 +++++---
.../staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
.../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ++++++++-----------
.../staging/media/sunxi/cedrus/cedrus_video.c | 2 ++
include/uapi/linux/v4l2-controls.h | 14 +++++--------
6 files changed, 24 insertions(+), 33 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 5f2b033a7a42..b854cceb19dc 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1660,15 +1660,6 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
return -EINVAL;
}
- if (p_mpeg2_slice_params->backward_ref_index >= VIDEO_MAX_FRAME ||
- p_mpeg2_slice_params->forward_ref_index >= VIDEO_MAX_FRAME)
- return -EINVAL;
-
- if (p_mpeg2_slice_params->pad ||
- p_mpeg2_slice_params->picture.pad ||
- p_mpeg2_slice_params->sequence.pad)
- return -EINVAL;
-
return 0;
case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 3f61248c57ac..781676b55a1b 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -142,11 +142,14 @@ static inline dma_addr_t cedrus_buf_addr(struct vb2_buffer *buf,
}
static inline dma_addr_t cedrus_dst_buf_addr(struct cedrus_ctx *ctx,
- unsigned int index,
- unsigned int plane)
+ int index, unsigned int plane)
{
- struct vb2_buffer *buf = ctx->dst_bufs[index];
+ struct vb2_buffer *buf;
+ if (index < 0)
+ return 0;
+
+ buf = ctx->dst_bufs[index];
return buf ? cedrus_buf_addr(buf, &ctx->dst_fmt, plane) : 0;
}
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index e40180a33951..0cfd6036d0cd 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -53,6 +53,8 @@ void cedrus_device_run(void *priv)
break;
}
+ v4l2_m2m_buf_copy_data(run.src, run.dst, true);
+
dev->dec_ops[ctx->current_codec]->setup(ctx, &run);
spin_unlock_irqrestore(&ctx->dev->irq_lock, flags);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 9abd39cae38c..fdde9a099153 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -82,7 +82,10 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
dma_addr_t fwd_luma_addr, fwd_chroma_addr;
dma_addr_t bwd_luma_addr, bwd_chroma_addr;
struct cedrus_dev *dev = ctx->dev;
+ struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
const u8 *matrix;
+ int forward_idx;
+ int backward_idx;
unsigned int i;
u32 reg;
@@ -156,23 +159,17 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
/* Forward and backward prediction reference buffers. */
+ forward_idx = vb2_find_tag(cap_q, slice_params->forward_ref_tag, 0);
- fwd_luma_addr = cedrus_dst_buf_addr(ctx,
- slice_params->forward_ref_index,
- 0);
- fwd_chroma_addr = cedrus_dst_buf_addr(ctx,
- slice_params->forward_ref_index,
- 1);
+ fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0);
+ fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1);
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr);
cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, fwd_chroma_addr);
- bwd_luma_addr = cedrus_dst_buf_addr(ctx,
- slice_params->backward_ref_index,
- 0);
- bwd_chroma_addr = cedrus_dst_buf_addr(ctx,
- slice_params->backward_ref_index,
- 1);
+ backward_idx = vb2_find_tag(cap_q, slice_params->backward_ref_tag, 0);
+ bwd_luma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 0);
+ bwd_chroma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 1);
cedrus_write(dev, VE_DEC_MPEG_BWD_REF_LUMA_ADDR, bwd_luma_addr);
cedrus_write(dev, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR, bwd_chroma_addr);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 5c5fce678b93..293df48326cc 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -522,6 +522,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq,
src_vq->lock = &ctx->dev->dev_mutex;
src_vq->dev = ctx->dev->dev;
src_vq->supports_requests = true;
+ src_vq->supports_tags = true;
ret = vb2_queue_init(src_vq);
if (ret)
@@ -537,6 +538,7 @@ int cedrus_queue_init(void *priv, struct vb2_queue *src_vq,
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = &ctx->dev->dev_mutex;
dst_vq->dev = ctx->dev->dev;
+ dst_vq->supports_tags = true;
return vb2_queue_init(dst_vq);
}
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 998983a6e6b7..45a55bb27e5a 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1109,10 +1109,9 @@ struct v4l2_mpeg2_sequence {
__u32 vbv_buffer_size;
/* ISO/IEC 13818-2, ITU-T Rec. H.262: Sequence extension */
- __u8 profile_and_level_indication;
+ __u16 profile_and_level_indication;
__u8 progressive_sequence;
__u8 chroma_format;
- __u8 pad;
};
struct v4l2_mpeg2_picture {
@@ -1130,23 +1129,20 @@ struct v4l2_mpeg2_picture {
__u8 intra_vlc_format;
__u8 alternate_scan;
__u8 repeat_first_field;
- __u8 progressive_frame;
- __u8 pad;
+ __u16 progressive_frame;
};
struct v4l2_ctrl_mpeg2_slice_params {
__u32 bit_size;
__u32 data_bit_offset;
+ __u32 backward_ref_tag;
+ __u32 forward_ref_tag;
struct v4l2_mpeg2_sequence sequence;
struct v4l2_mpeg2_picture picture;
/* ISO/IEC 13818-2, ITU-T Rec. H.262: Slice */
- __u8 quantiser_scale_code;
-
- __u8 backward_ref_index;
- __u8 forward_ref_index;
- __u8 pad;
+ __u32 quantiser_scale_code;
};
struct v4l2_ctrl_mpeg2_quantization {
--
2.19.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCHv2 0/9] vb2/cedrus: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (8 preceding siblings ...)
2018-11-14 13:47 ` [PATCHv2 9/9] cedrus: " Hans Verkuil
@ 2018-11-16 9:00 ` Tomasz Figa
2018-11-16 11:35 ` Hans Verkuil
2018-11-19 11:18 ` Hans Verkuil
2018-11-22 3:35 ` Alexandre Courbot
11 siblings, 1 reply; 16+ messages in thread
From: Tomasz Figa @ 2018-11-16 9:00 UTC (permalink / raw)
To: Hans Verkuil
Cc: Linux Media Mailing List, Alexandre Courbot, Maxime Ripard,
Paul Kocialkowski, nicolas
Hi Hans,
On Wed, Nov 14, 2018 at 10:47 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hansverk@cisco.com>
>
> As was discussed here (among other places):
>
> https://lkml.org/lkml/2018/10/19/440
>
> using capture queue buffer indices to refer to reference frames is
> not a good idea. A better idea is to use a 'tag' where the
> application can assign a u64 tag to an output buffer, which is then
> copied to the capture buffer(s) derived from the output buffer.
>
Thanks for the patches. Please see my comments below.
> A u64 is chosen since this allows userspace to also use pointers to
> internal structures as 'tag'.
>
I think this is not true anymore in this version.
> The first three patches add core tag support, the next patch document
> the tag support, then a new helper function is added to v4l2-mem2mem.c
> to easily copy data from a source to a destination buffer that drivers
> can use.
>
> Next a new supports_tags vb2_queue flag is added to indicate that
> the driver supports tags. Ideally this should not be necessary, but
> that would require that all m2m drivers are converted to using the
> new helper function introduced in the previous patch. That takes more
> time then I have now since we want to get this in for 4.20.
>
> Finally the vim2m, vicodec and cedrus drivers are converted to support
> tags.
>
> I also removed the 'pad' fields from the mpeg2 control structs (it
> should never been added in the first place) and aligned the structs
> to a u32 boundary (u64 for the tag values).
u32 in this version
>
> Note that this might change further (Paul suggested using bitfields).
>
> Also note that the cedrus code doesn't set the sequence counter, that's
> something that should still be added before this driver can be moved
> out of staging.
>
> Note: if no buffer is found for a certain tag, then the dma address
> is just set to 0. That happened before as well with invalid buffer
> indices. This should be checked in the driver!
Note that DMA address 0 may as well be a valid address. Should we have
another way of signaling that?
>
> The previous RFC series was tested successfully with the cedrus driver.
>
> Regards,
>
> Hans
>
> Changes since v1:
>
> - changed to a u32 tag. Using a 64 bit tag was overly complicated due
> to the bad layout of the v4l2_buffer struct, and there is no real
> need for it by applications.
I hope these won't become famous last words. :) For Chromium it should
be okay indeed.
Since we've been thinking about a redesign of the buffer struct,
perhaps we can live with u32 for now and we can design the new struct
to handle u64 nicely?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv2 0/9] vb2/cedrus: add tag support
2018-11-16 9:00 ` [PATCHv2 0/9] vb2/cedrus: " Tomasz Figa
@ 2018-11-16 11:35 ` Hans Verkuil
0 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-16 11:35 UTC (permalink / raw)
To: Tomasz Figa
Cc: Linux Media Mailing List, Alexandre Courbot, Maxime Ripard,
Paul Kocialkowski, nicolas
On 11/16/2018 10:00 AM, Tomasz Figa wrote:
> Hi Hans,
>
> On Wed, Nov 14, 2018 at 10:47 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> From: Hans Verkuil <hansverk@cisco.com>
>>
>> As was discussed here (among other places):
>>
>> https://lkml.org/lkml/2018/10/19/440
>>
>> using capture queue buffer indices to refer to reference frames is
>> not a good idea. A better idea is to use a 'tag' where the
>> application can assign a u64 tag to an output buffer, which is then
>> copied to the capture buffer(s) derived from the output buffer.
>>
>
> Thanks for the patches. Please see my comments below.
>
>> A u64 is chosen since this allows userspace to also use pointers to
>> internal structures as 'tag'.
>>
>
> I think this is not true anymore in this version.
Ah, forgot to update the commit message.
>
>> The first three patches add core tag support, the next patch document
>> the tag support, then a new helper function is added to v4l2-mem2mem.c
>> to easily copy data from a source to a destination buffer that drivers
>> can use.
>>
>> Next a new supports_tags vb2_queue flag is added to indicate that
>> the driver supports tags. Ideally this should not be necessary, but
>> that would require that all m2m drivers are converted to using the
>> new helper function introduced in the previous patch. That takes more
>> time then I have now since we want to get this in for 4.20.
>>
>> Finally the vim2m, vicodec and cedrus drivers are converted to support
>> tags.
>>
>> I also removed the 'pad' fields from the mpeg2 control structs (it
>> should never been added in the first place) and aligned the structs
>> to a u32 boundary (u64 for the tag values).
>
> u32 in this version
>
>>
>> Note that this might change further (Paul suggested using bitfields).
>>
>> Also note that the cedrus code doesn't set the sequence counter, that's
>> something that should still be added before this driver can be moved
>> out of staging.
>>
>> Note: if no buffer is found for a certain tag, then the dma address
>> is just set to 0. That happened before as well with invalid buffer
>> indices. This should be checked in the driver!
>
> Note that DMA address 0 may as well be a valid address. Should we have
> another way of signaling that?
See this patch:
https://patchwork.linuxtv.org/patch/52975/
The memory of the reference buffer should be checked and the refcount
incremented in the request validate function.
Once that's done the dma address will be guaranteed to be OK.
This is missing functionality that is important to add.
>
>>
>> The previous RFC series was tested successfully with the cedrus driver.
>>
>> Regards,
>>
>> Hans
>>
>> Changes since v1:
>>
>> - changed to a u32 tag. Using a 64 bit tag was overly complicated due
>> to the bad layout of the v4l2_buffer struct, and there is no real
>> need for it by applications.
>
> I hope these won't become famous last words. :) For Chromium it should
> be okay indeed.
The only 'feature' that is now missing is the ability to store pointers
as the tag. So worst case you need to do an extra lookup in the application.
>
> Since we've been thinking about a redesign of the buffer struct,
> perhaps we can live with u32 for now and we can design the new struct
> to handle u64 nicely?
It could be done, but I'm not sure if it is a good idea to have
different behavior between v4l2_buffer and v4l2_ext_buffer.
Something to discuss at that time.
Regards,
Hans
>
> Best regards,
> Tomasz
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv2 0/9] vb2/cedrus: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (9 preceding siblings ...)
2018-11-16 9:00 ` [PATCHv2 0/9] vb2/cedrus: " Tomasz Figa
@ 2018-11-19 11:18 ` Hans Verkuil
2018-11-19 13:53 ` Paul Kocialkowski
2018-11-22 3:35 ` Alexandre Courbot
11 siblings, 1 reply; 16+ messages in thread
From: Hans Verkuil @ 2018-11-19 11:18 UTC (permalink / raw)
To: linux-media
Cc: Alexandre Courbot, maxime.ripard, paul.kocialkowski, tfiga,
Nicolas Dufresne
On 11/14/2018 02:47 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hansverk@cisco.com>
>
> As was discussed here (among other places):
>
> https://lkml.org/lkml/2018/10/19/440
>
> using capture queue buffer indices to refer to reference frames is
> not a good idea. A better idea is to use a 'tag' where the
> application can assign a u64 tag to an output buffer, which is then
> copied to the capture buffer(s) derived from the output buffer.
>
> A u64 is chosen since this allows userspace to also use pointers to
> internal structures as 'tag'.
>
> The first three patches add core tag support, the next patch document
> the tag support, then a new helper function is added to v4l2-mem2mem.c
> to easily copy data from a source to a destination buffer that drivers
> can use.
>
> Next a new supports_tags vb2_queue flag is added to indicate that
> the driver supports tags. Ideally this should not be necessary, but
> that would require that all m2m drivers are converted to using the
> new helper function introduced in the previous patch. That takes more
> time then I have now since we want to get this in for 4.20.
>
> Finally the vim2m, vicodec and cedrus drivers are converted to support
> tags.
>
> I also removed the 'pad' fields from the mpeg2 control structs (it
> should never been added in the first place) and aligned the structs
> to a u32 boundary (u64 for the tag values).
>
> Note that this might change further (Paul suggested using bitfields).
>
> Also note that the cedrus code doesn't set the sequence counter, that's
> something that should still be added before this driver can be moved
> out of staging.
>
> Note: if no buffer is found for a certain tag, then the dma address
> is just set to 0. That happened before as well with invalid buffer
> indices. This should be checked in the driver!
>
> The previous RFC series was tested successfully with the cedrus driver.
>
> Regards,
>
> Hans
I'd like to get some Acked-by or Reviewed-by replies for this series.
Or comments if you don't like something.
I would really like to get this in for 4.20 so the cedrus API is stable
(hopefully), since this is a last outstanding API item.
Tomasz: you commented that the text still referred to the tag as a u64,
but that was only in the cover letter, not the patches themselves. So
I don't plan to post a v3 just for that.
Regards,
Hans
>
> Changes since v1:
>
> - changed to a u32 tag. Using a 64 bit tag was overly complicated due
> to the bad layout of the v4l2_buffer struct, and there is no real
> need for it by applications.
>
> Main changes since the RFC:
>
> - Added new buffer capability flag
> - Added m2m helper to copy data between buffers
> - Added documentation
> - Added tag logging in v4l2-ioctl.c
>
> Hans Verkuil (9):
> videodev2.h: add tag support
> vb2: add tag support
> v4l2-ioctl.c: log v4l2_buffer tag
> buffer.rst: document the new buffer tag feature.
> v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
> vb2: add new supports_tags queue flag
> vim2m: add tag support
> vicodec: add tag support
> cedrus: add tag support
>
> Documentation/media/uapi/v4l/buffer.rst | 32 +++++++++----
> .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++
> .../media/common/videobuf2/videobuf2-v4l2.c | 45 ++++++++++++++++---
> drivers/media/platform/vicodec/vicodec-core.c | 14 ++----
> drivers/media/platform/vim2m.c | 14 ++----
> drivers/media/v4l2-core/v4l2-ctrls.c | 9 ----
> drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++--
> drivers/media/v4l2-core/v4l2-mem2mem.c | 23 ++++++++++
> drivers/staging/media/sunxi/cedrus/cedrus.h | 9 ++--
> .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 +
> .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ++++-----
> .../staging/media/sunxi/cedrus/cedrus_video.c | 2 +
> include/media/v4l2-mem2mem.h | 21 +++++++++
> include/media/videobuf2-core.h | 2 +
> include/media/videobuf2-v4l2.h | 21 ++++++++-
> include/uapi/linux/v4l2-controls.h | 14 +++---
> include/uapi/linux/videodev2.h | 9 +++-
> 17 files changed, 178 insertions(+), 73 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv2 0/9] vb2/cedrus: add tag support
2018-11-19 11:18 ` Hans Verkuil
@ 2018-11-19 13:53 ` Paul Kocialkowski
2018-11-19 14:01 ` Hans Verkuil
0 siblings, 1 reply; 16+ messages in thread
From: Paul Kocialkowski @ 2018-11-19 13:53 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: Alexandre Courbot, maxime.ripard, tfiga, Nicolas Dufresne
[-- Attachment #1: Type: text/plain, Size: 6149 bytes --]
Hi,
On Mon, 2018-11-19 at 12:18 +0100, Hans Verkuil wrote:
> On 11/14/2018 02:47 PM, Hans Verkuil wrote:
> > From: Hans Verkuil <hansverk@cisco.com>
> >
> > As was discussed here (among other places):
> >
> > https://lkml.org/lkml/2018/10/19/440
> >
> > using capture queue buffer indices to refer to reference frames is
> > not a good idea. A better idea is to use a 'tag' where the
> > application can assign a u64 tag to an output buffer, which is then
> > copied to the capture buffer(s) derived from the output buffer.
> >
> > A u64 is chosen since this allows userspace to also use pointers to
> > internal structures as 'tag'.
> >
> > The first three patches add core tag support, the next patch document
> > the tag support, then a new helper function is added to v4l2-mem2mem.c
> > to easily copy data from a source to a destination buffer that drivers
> > can use.
> >
> > Next a new supports_tags vb2_queue flag is added to indicate that
> > the driver supports tags. Ideally this should not be necessary, but
> > that would require that all m2m drivers are converted to using the
> > new helper function introduced in the previous patch. That takes more
> > time then I have now since we want to get this in for 4.20.
> >
> > Finally the vim2m, vicodec and cedrus drivers are converted to support
> > tags.
> >
> > I also removed the 'pad' fields from the mpeg2 control structs (it
> > should never been added in the first place) and aligned the structs
> > to a u32 boundary (u64 for the tag values).
> >
> > Note that this might change further (Paul suggested using bitfields).
> >
> > Also note that the cedrus code doesn't set the sequence counter, that's
> > something that should still be added before this driver can be moved
> > out of staging.
> >
> > Note: if no buffer is found for a certain tag, then the dma address
> > is just set to 0. That happened before as well with invalid buffer
> > indices. This should be checked in the driver!
> >
> > The previous RFC series was tested successfully with the cedrus driver.
> >
> > Regards,
> >
> > Hans
>
> I'd like to get some Acked-by or Reviewed-by replies for this series.
> Or comments if you don't like something.
The series looks good to me, so consider it:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Also, I'm glad you made the v4l2_m2m_buf_copy_data helper function,
since all drivers will need to do these same operations anyway.
> I would really like to get this in for 4.20 so the cedrus API is stable
> (hopefully), since this is a last outstanding API item.
I see a few more items that we need to tackle before we can consider the
MPEG-2 stateless API as stable.
* I mentionned using flags in the mpeg2 structures to solve the
alignment issues, but I failed to provide a patch implementing it at
this point. This would make the MPEG-2 controls more similar to the
proposed H.264 ones and generally makes more sense than using a u8 for a
binary value.
* Some time ago, we also discussed adding extra fields to the structures
for later use. Do you think that is still relevant? Adding a flags
elements (with unused bits) would certainly help in this direction
anyway.
* During the discussions on the spec, the concensus was also to use one
slice_params per-slice (instead of per-picture) which requires splitting
the per-picture and the per-slice parameters. On this as well, I fell
behind and didn't send a proposal yet.
So my question is: does it need to be done for 4.20 or can we affoard
going through another version cycle for these changes?
It was my impression that we wanted to wait for another driver to use
the API to declare it stable (and move the driver out of staging).
Cheers,
Paul
> Tomasz: you commented that the text still referred to the tag as a u64,
> but that was only in the cover letter, not the patches themselves. So
> I don't plan to post a v3 just for that.
>
> Regards,
>
> Hans
>
> > Changes since v1:
> >
> > - changed to a u32 tag. Using a 64 bit tag was overly complicated due
> > to the bad layout of the v4l2_buffer struct, and there is no real
> > need for it by applications.
> >
> > Main changes since the RFC:
> >
> > - Added new buffer capability flag
> > - Added m2m helper to copy data between buffers
> > - Added documentation
> > - Added tag logging in v4l2-ioctl.c
> >
> > Hans Verkuil (9):
> > videodev2.h: add tag support
> > vb2: add tag support
> > v4l2-ioctl.c: log v4l2_buffer tag
> > buffer.rst: document the new buffer tag feature.
> > v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
> > vb2: add new supports_tags queue flag
> > vim2m: add tag support
> > vicodec: add tag support
> > cedrus: add tag support
> >
> > Documentation/media/uapi/v4l/buffer.rst | 32 +++++++++----
> > .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++
> > .../media/common/videobuf2/videobuf2-v4l2.c | 45 ++++++++++++++++---
> > drivers/media/platform/vicodec/vicodec-core.c | 14 ++----
> > drivers/media/platform/vim2m.c | 14 ++----
> > drivers/media/v4l2-core/v4l2-ctrls.c | 9 ----
> > drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++--
> > drivers/media/v4l2-core/v4l2-mem2mem.c | 23 ++++++++++
> > drivers/staging/media/sunxi/cedrus/cedrus.h | 9 ++--
> > .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 +
> > .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ++++-----
> > .../staging/media/sunxi/cedrus/cedrus_video.c | 2 +
> > include/media/v4l2-mem2mem.h | 21 +++++++++
> > include/media/videobuf2-core.h | 2 +
> > include/media/videobuf2-v4l2.h | 21 ++++++++-
> > include/uapi/linux/v4l2-controls.h | 14 +++---
> > include/uapi/linux/videodev2.h | 9 +++-
> > 17 files changed, 178 insertions(+), 73 deletions(-)
> >
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv2 0/9] vb2/cedrus: add tag support
2018-11-19 13:53 ` Paul Kocialkowski
@ 2018-11-19 14:01 ` Hans Verkuil
0 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2018-11-19 14:01 UTC (permalink / raw)
To: Paul Kocialkowski, linux-media
Cc: Mauro Carvalho Chehab, maxime.ripard, tfiga, Nicolas Dufresne
On 11/19/2018 02:53 PM, Paul Kocialkowski wrote:
> Hi,
>
> On Mon, 2018-11-19 at 12:18 +0100, Hans Verkuil wrote:
>> On 11/14/2018 02:47 PM, Hans Verkuil wrote:
>>> From: Hans Verkuil <hansverk@cisco.com>
>>>
>>> As was discussed here (among other places):
>>>
>>> https://lkml.org/lkml/2018/10/19/440
>>>
>>> using capture queue buffer indices to refer to reference frames is
>>> not a good idea. A better idea is to use a 'tag' where the
>>> application can assign a u64 tag to an output buffer, which is then
>>> copied to the capture buffer(s) derived from the output buffer.
>>>
>>> A u64 is chosen since this allows userspace to also use pointers to
>>> internal structures as 'tag'.
>>>
>>> The first three patches add core tag support, the next patch document
>>> the tag support, then a new helper function is added to v4l2-mem2mem.c
>>> to easily copy data from a source to a destination buffer that drivers
>>> can use.
>>>
>>> Next a new supports_tags vb2_queue flag is added to indicate that
>>> the driver supports tags. Ideally this should not be necessary, but
>>> that would require that all m2m drivers are converted to using the
>>> new helper function introduced in the previous patch. That takes more
>>> time then I have now since we want to get this in for 4.20.
>>>
>>> Finally the vim2m, vicodec and cedrus drivers are converted to support
>>> tags.
>>>
>>> I also removed the 'pad' fields from the mpeg2 control structs (it
>>> should never been added in the first place) and aligned the structs
>>> to a u32 boundary (u64 for the tag values).
>>>
>>> Note that this might change further (Paul suggested using bitfields).
>>>
>>> Also note that the cedrus code doesn't set the sequence counter, that's
>>> something that should still be added before this driver can be moved
>>> out of staging.
>>>
>>> Note: if no buffer is found for a certain tag, then the dma address
>>> is just set to 0. That happened before as well with invalid buffer
>>> indices. This should be checked in the driver!
>>>
>>> The previous RFC series was tested successfully with the cedrus driver.
>>>
>>> Regards,
>>>
>>> Hans
>>
>> I'd like to get some Acked-by or Reviewed-by replies for this series.
>> Or comments if you don't like something.
>
> The series looks good to me, so consider it:
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>
> Also, I'm glad you made the v4l2_m2m_buf_copy_data helper function,
> since all drivers will need to do these same operations anyway.
>
>> I would really like to get this in for 4.20 so the cedrus API is stable
>> (hopefully), since this is a last outstanding API item.
>
> I see a few more items that we need to tackle before we can consider the
> MPEG-2 stateless API as stable.
>
> * I mentionned using flags in the mpeg2 structures to solve the
> alignment issues, but I failed to provide a patch implementing it at
> this point. This would make the MPEG-2 controls more similar to the
> proposed H.264 ones and generally makes more sense than using a u8 for a
> binary value.
I think it will be good to have this done. If you can make a patch that I can
rebase my tag series on, then that would be best.
>
> * Some time ago, we also discussed adding extra fields to the structures
> for later use. Do you think that is still relevant? Adding a flags
> elements (with unused bits) would certainly help in this direction
> anyway.
If you switch to bitfields, then there might be enough room anyway for
addition flags in the future.
>
> * During the discussions on the spec, the concensus was also to use one
> slice_params per-slice (instead of per-picture) which requires splitting
> the per-picture and the per-slice parameters. On this as well, I fell
> behind and didn't send a proposal yet.
>
> So my question is: does it need to be done for 4.20 or can we affoard
> going through another version cycle for these changes?
>
> It was my impression that we wanted to wait for another driver to use
> the API to declare it stable (and move the driver out of staging).
Good question, I would like to hear what Mauro says.
I would prefer to postpone this to 4.21, since I don't feel comfortable
with such large changes for an rcX (esp. splitting the per-picture/slice
parameters).
Regards,
Hans
>
> Cheers,
>
> Paul
>
>> Tomasz: you commented that the text still referred to the tag as a u64,
>> but that was only in the cover letter, not the patches themselves. So
>> I don't plan to post a v3 just for that.
>>
>> Regards,
>>
>> Hans
>>
>>> Changes since v1:
>>>
>>> - changed to a u32 tag. Using a 64 bit tag was overly complicated due
>>> to the bad layout of the v4l2_buffer struct, and there is no real
>>> need for it by applications.
>>>
>>> Main changes since the RFC:
>>>
>>> - Added new buffer capability flag
>>> - Added m2m helper to copy data between buffers
>>> - Added documentation
>>> - Added tag logging in v4l2-ioctl.c
>>>
>>> Hans Verkuil (9):
>>> videodev2.h: add tag support
>>> vb2: add tag support
>>> v4l2-ioctl.c: log v4l2_buffer tag
>>> buffer.rst: document the new buffer tag feature.
>>> v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
>>> vb2: add new supports_tags queue flag
>>> vim2m: add tag support
>>> vicodec: add tag support
>>> cedrus: add tag support
>>>
>>> Documentation/media/uapi/v4l/buffer.rst | 32 +++++++++----
>>> .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++
>>> .../media/common/videobuf2/videobuf2-v4l2.c | 45 ++++++++++++++++---
>>> drivers/media/platform/vicodec/vicodec-core.c | 14 ++----
>>> drivers/media/platform/vim2m.c | 14 ++----
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 9 ----
>>> drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++--
>>> drivers/media/v4l2-core/v4l2-mem2mem.c | 23 ++++++++++
>>> drivers/staging/media/sunxi/cedrus/cedrus.h | 9 ++--
>>> .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 +
>>> .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ++++-----
>>> .../staging/media/sunxi/cedrus/cedrus_video.c | 2 +
>>> include/media/v4l2-mem2mem.h | 21 +++++++++
>>> include/media/videobuf2-core.h | 2 +
>>> include/media/videobuf2-v4l2.h | 21 ++++++++-
>>> include/uapi/linux/v4l2-controls.h | 14 +++---
>>> include/uapi/linux/videodev2.h | 9 +++-
>>> 17 files changed, 178 insertions(+), 73 deletions(-)
>>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv2 0/9] vb2/cedrus: add tag support
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
` (10 preceding siblings ...)
2018-11-19 11:18 ` Hans Verkuil
@ 2018-11-22 3:35 ` Alexandre Courbot
11 siblings, 0 replies; 16+ messages in thread
From: Alexandre Courbot @ 2018-11-22 3:35 UTC (permalink / raw)
To: Hans Verkuil
Cc: Linux Media Mailing List, Maxime Ripard, Paul Kocialkowski,
Tomasz Figa, Nicolas Dufresne
On Wed, Nov 14, 2018 at 10:47 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hansverk@cisco.com>
>
> As was discussed here (among other places):
>
> https://lkml.org/lkml/2018/10/19/440
>
> using capture queue buffer indices to refer to reference frames is
> not a good idea. A better idea is to use a 'tag' where the
> application can assign a u64 tag to an output buffer, which is then
> copied to the capture buffer(s) derived from the output buffer.
>
> A u64 is chosen since this allows userspace to also use pointers to
> internal structures as 'tag'.
>
> The first three patches add core tag support, the next patch document
> the tag support, then a new helper function is added to v4l2-mem2mem.c
> to easily copy data from a source to a destination buffer that drivers
> can use.
>
> Next a new supports_tags vb2_queue flag is added to indicate that
> the driver supports tags. Ideally this should not be necessary, but
> that would require that all m2m drivers are converted to using the
> new helper function introduced in the previous patch. That takes more
> time then I have now since we want to get this in for 4.20.
>
> Finally the vim2m, vicodec and cedrus drivers are converted to support
> tags.
>
> I also removed the 'pad' fields from the mpeg2 control structs (it
> should never been added in the first place) and aligned the structs
> to a u32 boundary (u64 for the tag values).
>
> Note that this might change further (Paul suggested using bitfields).
>
> Also note that the cedrus code doesn't set the sequence counter, that's
> something that should still be added before this driver can be moved
> out of staging.
>
> Note: if no buffer is found for a certain tag, then the dma address
> is just set to 0. That happened before as well with invalid buffer
> indices. This should be checked in the driver!
>
> The previous RFC series was tested successfully with the cedrus driver.
>
> Regards,
>
> Hans
>
> Changes since v1:
>
> - changed to a u32 tag. Using a 64 bit tag was overly complicated due
> to the bad layout of the v4l2_buffer struct, and there is no real
> need for it by applications.
>
> Main changes since the RFC:
>
> - Added new buffer capability flag
> - Added m2m helper to copy data between buffers
> - Added documentation
> - Added tag logging in v4l2-ioctl.c
>
> Hans Verkuil (9):
> videodev2.h: add tag support
> vb2: add tag support
> v4l2-ioctl.c: log v4l2_buffer tag
> buffer.rst: document the new buffer tag feature.
> v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
> vb2: add new supports_tags queue flag
> vim2m: add tag support
> vicodec: add tag support
> cedrus: add tag support
Good call on the v4l2_m2m_buf_copy_data() function!
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
>
> Documentation/media/uapi/v4l/buffer.rst | 32 +++++++++----
> .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++
> .../media/common/videobuf2/videobuf2-v4l2.c | 45 ++++++++++++++++---
> drivers/media/platform/vicodec/vicodec-core.c | 14 ++----
> drivers/media/platform/vim2m.c | 14 ++----
> drivers/media/v4l2-core/v4l2-ctrls.c | 9 ----
> drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++--
> drivers/media/v4l2-core/v4l2-mem2mem.c | 23 ++++++++++
> drivers/staging/media/sunxi/cedrus/cedrus.h | 9 ++--
> .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 +
> .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ++++-----
> .../staging/media/sunxi/cedrus/cedrus_video.c | 2 +
> include/media/v4l2-mem2mem.h | 21 +++++++++
> include/media/videobuf2-core.h | 2 +
> include/media/videobuf2-v4l2.h | 21 ++++++++-
> include/uapi/linux/v4l2-controls.h | 14 +++---
> include/uapi/linux/videodev2.h | 9 +++-
> 17 files changed, 178 insertions(+), 73 deletions(-)
>
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-11-22 14:13 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-14 13:47 [PATCHv2 0/9] vb2/cedrus: add tag support Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 1/9] videodev2.h: " Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 2/9] vb2: " Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 3/9] v4l2-ioctl.c: log v4l2_buffer tag Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 4/9] buffer.rst: document the new buffer tag feature Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 5/9] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 6/9] vb2: add new supports_tags queue flag Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 7/9] vim2m: add tag support Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 8/9] vicodec: " Hans Verkuil
2018-11-14 13:47 ` [PATCHv2 9/9] cedrus: " Hans Verkuil
2018-11-16 9:00 ` [PATCHv2 0/9] vb2/cedrus: " Tomasz Figa
2018-11-16 11:35 ` Hans Verkuil
2018-11-19 11:18 ` Hans Verkuil
2018-11-19 13:53 ` Paul Kocialkowski
2018-11-19 14:01 ` Hans Verkuil
2018-11-22 3:35 ` Alexandre Courbot
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).