[PATCH for v4.20 2/2] extended-controls.rst: add note to the MPEG2 state controls

[hverkuil-cisco]

From: Hans Verkuil 

Add a note mentioning that these two controls are not part of the
public API while they still stabilizing.

Signed-off-by: Hans Verkuil 
---
 Documentation/media/uapi/v4l/extended-controls.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
b/Documentation/media/uapi/v4l/extended-controls.rst
index 65a1d873196b..027358b91082 100644
--- a/Documentation/media/uapi/v4l/extended-controls.rst
+++ b/Documentation/media/uapi/v4l/extended-controls.rst
@@ -1505,6 +1505,11 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
 configuring a stateless hardware decoding pipeline for MPEG-2.
 The bitstream parameters are defined according to :ref:`mpeg2part2`.
 
+.. note::
+
+   This compound control is not yet part of the public kernel API and
+   it is expected to change.
+
 .. c:type:: v4l2_ctrl_mpeg2_slice_params
 
 .. cssclass:: longtable
@@ -1625,6 +1630,11 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
 Specifies quantization matrices (as extracted from the bitstream) for the
 associated MPEG-2 slice data.
 
+.. note::
+
+   This compound control is not yet part of the public kernel API and
+   it is expected to change.
+
 .. c:type:: v4l2_ctrl_mpeg2_quantization
 
 .. cssclass:: longtable
-- 
2.19.1

[PATCH for v4.20 0/2] cedrus: move MPEG controls out of the uAPI

[hverkuil-cisco]

From: Hans Verkuil 

The expectation was that the MPEG-2 state controls used by the staging
cedrus driver were stable, or would only require one final change. However,
it turns out that more changes are required, and that means that it is not
such a good idea to have these controls in the public kernel API.

This patch series moves all the MPEG-2 state control data to a new
media/mpeg2-ctrls.h header. So none of this is available from the public
API.

However, v4l2-ctrls.h includes it for now so the kAPI still knows about it
allowing the cedrus driver to use it without changes.

The second patch adds a note to these two controls, mentioning that they
are likely to change.

Moving forward, this allows us to take more time in getting the MPEG-2
(and later H264/5) state controls right.

Regards,

Hans

Hans Verkuil (2):
  mpeg2-ctrls.h: move MPEG2 state controls to non-public header
  extended-controls.rst: add note to the MPEG2 state controls

 .../media/uapi/v4l/extended-controls.rst  | 10 +++
 drivers/media/v4l2-core/v4l2-ctrls.c  |  4 +-
 include/media/mpeg2-ctrls.h   | 86 +++
 include/media/v4l2-ctrls.h|  6 ++
 include/uapi/linux/v4l2-controls.h| 68 ---
 include/uapi/linux/videodev2.h|  4 -
 6 files changed, 104 insertions(+), 74 deletions(-)
 create mode 100644 include/media/mpeg2-ctrls.h

-- 
2.19.1

[PATCH for v4.20 1/2] mpeg2-ctrls.h: move MPEG2 state controls to non-public header

[hverkuil-cisco]

From: Hans Verkuil 

The MPEG2 state controls for the cedrus stateless MPEG2 driver are
not yet stable. Move them out of the public headers into media/mpeg2-ctrls.h.

Eventually, once this has stabilized, they will be moved back to the
public headers.

Unfortunately I had to cast the control type to a u32 in two switch
statements to prevent a compiler warning about a control type define
not being part of the enum.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c |  4 +-
 include/media/mpeg2-ctrls.h  | 86 
 include/media/v4l2-ctrls.h   |  6 ++
 include/uapi/linux/v4l2-controls.h   | 68 --
 include/uapi/linux/videodev2.h   |  4 --
 5 files changed, 94 insertions(+), 74 deletions(-)
 create mode 100644 include/media/mpeg2-ctrls.h

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 5f2b033a7a42..10b8d94edbef 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1563,7 +1563,7 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 
idx,
u64 offset;
s64 val;
 
-   switch (ctrl->type) {
+   switch ((u32)ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER:
return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl);
case V4L2_CTRL_TYPE_INTEGER64:
@@ -2232,7 +2232,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
is_array = nr_of_dims > 0;
 
/* Prefill elem_size for all types handled by std_type_ops */
-   switch (type) {
+   switch ((u32)type) {
case V4L2_CTRL_TYPE_INTEGER64:
elem_size = sizeof(s64);
break;
diff --git a/include/media/mpeg2-ctrls.h b/include/media/mpeg2-ctrls.h
new file mode 100644
index ..d21f40edc09e
--- /dev/null
+++ b/include/media/mpeg2-ctrls.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * These are the MPEG2 state controls for use with stateless MPEG-2
+ * codec drivers.
+ *
+ * It turns out that these structs are not stable yet and will undergo
+ * more changes. So keep them private until they are stable and ready to
+ * become part of the official public API.
+ */
+
+#ifndef _MPEG2_CTRLS_H_
+#define _MPEG2_CTRLS_H_
+
+#define V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (V4L2_CID_MPEG_BASE+250)
+#define V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION (V4L2_CID_MPEG_BASE+251)
+
+/* enum v4l2_ctrl_type type values */
+#define V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS 0x0103
+#defineV4L2_CTRL_TYPE_MPEG2_QUANTIZATION 0x0104
+
+#define V4L2_MPEG2_PICTURE_CODING_TYPE_I   1
+#define V4L2_MPEG2_PICTURE_CODING_TYPE_P   2
+#define V4L2_MPEG2_PICTURE_CODING_TYPE_B   3
+#define V4L2_MPEG2_PICTURE_CODING_TYPE_D   4
+
+struct v4l2_mpeg2_sequence {
+   /* ISO/IEC 13818-2, ITU-T Rec. H.262: Sequence header */
+   __u16   horizontal_size;
+   __u16   vertical_size;
+   __u32   vbv_buffer_size;
+
+   /* ISO/IEC 13818-2, ITU-T Rec. H.262: Sequence extension */
+   __u8profile_and_level_indication;
+   __u8progressive_sequence;
+   __u8chroma_format;
+   __u8pad;
+};
+
+struct v4l2_mpeg2_picture {
+   /* ISO/IEC 13818-2, ITU-T Rec. H.262: Picture header */
+   __u8picture_coding_type;
+
+   /* ISO/IEC 13818-2, ITU-T Rec. H.262: Picture coding extension */
+   __u8f_code[2][2];
+   __u8intra_dc_precision;
+   __u8picture_structure;
+   __u8top_field_first;
+   __u8frame_pred_frame_dct;
+   __u8concealment_motion_vectors;
+   __u8q_scale_type;
+   __u8intra_vlc_format;
+   __u8alternate_scan;
+   __u8repeat_first_field;
+   __u8progressive_frame;
+   __u8pad;
+};
+
+struct v4l2_ctrl_mpeg2_slice_params {
+   __u32   bit_size;
+   __u32   data_bit_offset;
+
+   struct v4l2_mpeg2_sequence sequence;
+   struct v4l2_mpeg2_picture picture;
+
+   /* ISO/IEC 13818-2, ITU-T Rec. H.262: Slice */
+   __u8quantiser_scale_code;
+
+   __u8backward_ref_index;
+   __u8forward_ref_index;
+   __u8pad;
+};
+
+struct v4l2_ctrl_mpeg2_quantization {
+   /* ISO/IEC 13818-2, ITU-T Rec. H.262: Quant matrix extension */
+   __u8load_intra_quantiser_matrix;
+   __u8load_non_intra_quantiser_matrix;
+   __u8load_chroma_intra_quantiser_matrix;
+   __u8load_chroma_non_intra_quantiser_matrix;
+
+   __u8intra_quantiser_matrix[64];
+   __u8non_intra_quantiser_matrix[64];
+   __u8chroma_intra_quantiser_matrix[64];
+   __u8chroma_non_intra_quantiser_matrix[64];
+};
+
+#endif
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 83ce0593b275..d63cf227b0ab 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -22,6 +22,12 @@
 #include 
 #include 
 
+/*
+ * Include the mpeg2 

[PATCHv4 05/10] buffer.rst: clean up timecode documentation

[hverkuil-cisco]

From: Hans Verkuil 

V4L2_BUF_FLAG_TIMECODE is not video capture specific, so drop that
part.

The 'Timecodes' section was a bit messy, so that's cleaned up.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 Documentation/media/uapi/v4l/buffer.rst | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index f83ee00cb30b..359b131a212d 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -223,8 +223,7 @@ struct v4l2_buffer
 * - 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 ` mode the top and
bottom field contain the same timecode. Timecodes are intended to
@@ -715,10 +714,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
-- 
2.19.1

[PATCHv4 02/10] vb2: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

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 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 .../media/common/videobuf2/videobuf2-v4l2.c   | 39 ---
 include/media/videobuf2-v4l2.h| 21 +-
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c 
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 1244c246d0c4..e0e31e1c67c9 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,7 +145,10 @@ static void __copy_timestamp(struct vb2_buffer *vb, const 
void *pb)
 */
if (q->copy_timestamp)
vb->timestamp = timeval_to_ns(>timestamp);
-   vbuf->flags |= 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;
if (b->flags & V4L2_BUF_FLAG_TIMECODE)
vbuf->timecode = b->timecode;
}
@@ -194,6 +198,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,12 +319,12 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer 
*vb, struct v4l2_buffer *b
 
if (V4L2_TYPE_IS_OUTPUT(b->type)) {
/*
-* For output buffers mask out the timecode flag:
-* this will be handled later in vb2_qbuf().
+* For output buffers mask out the timecode and tag flags:
+* these 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 */
@@ -460,7 +465,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;
+   if (b->flags & V4L2_BUF_FLAG_TIMECODE)
+   b->timecode = vbuf->timecode;
b->sequence = vbuf->sequence;
b->reserved2 = 0;
b->request_fd = 0;
@@ -586,6 +594,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
  *  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:   

[PATCHv4 01/10] videodev2.h: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

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 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 2db1635de956..9095d7abe10d 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -881,6 +881,7 @@ struct v4l2_requestbuffers {
 #define V4L2_BUF_CAP_SUPPORTS_DMABUF   (1 << 2)
 #define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3)
 #define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS (1 << 4)
+#define V4L2_BUF_CAP_SUPPORTS_TAGS (1 << 5)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
@@ -940,6 +941,7 @@ struct v4l2_plane {
  * @length:size in bytes of the buffer (NOT its payload) for single-plane
  * buffers (when type != *_MPLANE); number of elements in the
  * planes array for multi-plane buffers
+ * @tag:   buffer tag
  * @request_fd: fd of the request that this buffer should use
  *
  * Contains data exchanged by application and driver using one of the Streaming
@@ -964,7 +966,10 @@ struct v4l2_buffer {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   union {
+   __u32   reserved2;
+   __u32   tag;
+   };
union {
__s32   request_fd;
__u32   reserved;
@@ -990,6 +995,8 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_IN_REQUEST   0x0080
 /* timecode field is valid */
 #define V4L2_BUF_FLAG_TIMECODE 0x0100
+/* tag field is valid */
+#define V4L2_BUF_FLAG_TAG  0x0200
 /* Buffer is prepared for queuing */
 #define V4L2_BUF_FLAG_PREPARED 0x0400
 /* Cache handling flags */
-- 
2.19.1

[PATCHv4 08/10] vim2m: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

Copy tags in vim2m.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 d01821a6906a..be328483a53a 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:
@@ -855,6 +845,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 = >dev->dev_mutex;
src_vq->supports_requests = true;
+   src_vq->supports_tags = true;
 
ret = vb2_queue_init(src_vq);
if (ret)
@@ -868,6 +859,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, 
struct vb2_queue *ds
dst_vq->mem_ops = _vmalloc_memops;
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = >dev->dev_mutex;
+   dst_vq->supports_tags = true;
 
return vb2_queue_init(dst_vq);
 }
-- 
2.19.1

[PATCHv4 10/10] cedrus: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

Replace old reference frame indices by new tag method.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 129a986fa7e1..e859496e4e95 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1661,15 +1661,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, >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, );
 
spin_unlock_irqrestore(>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 = >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 = 

[PATCHv4 00/10] As was discussed here (among other places):

[hverkuil-cisco]

From: Hans Verkuil 

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 u32 tag to an output buffer, which is then 
copied to the capture buffer(s) derived from the output buffer.

It has been suggested that the timestamp can be used for this. But
there are a number of reasons why this is a bad idea:

1) the struct timeval is converted to a u64 in vb2. So there can be
   all sorts of unexpected conversion issues. In particular, the
   output of ns_to_timeval(timeval_to_ns(tv)) does not necessarily
   match the input.

2) it gets worse with the y2038 code where userspace either deals
   with a 32 bit tv_sec value or a 64 bit value.

In other words, using timestamp for this is not a good idea.

This implementation adds a new tag field in a union with the reserved2
field. The interpretation of that union depends on the flags field, so
it still can be used for other things as well. In addition, in the previous
patches the tag was in a union with the timecode field (again determined
by the flags field), so if we need to cram additional information in this
struct we can always put it in a union with the timecode field as well.
It worked for the tag, it should work for other things.

But we really need to start looking at a struct v4l2_ext_buffer.

The first three patches add core tag support, the next two patches 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.

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.

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!

Regards,

Hans

Changes since v3:

- use reserved2 for the tag
- split the documentation in two: one documenting the tag, one
  cleaning up the timecode documentation.

Changes since v2:

- rebased
- added Reviewed-by tags
- fixed a few remaining references in the documentation to the old
  v4l2_buffer_tag struct that was used in early versions of this
  series.

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 (10):
  videodev2.h: add tag support
  vb2: add tag support
  v4l2-ioctl.c: log v4l2_buffer tag
  buffer.rst: document the new buffer tag feature.
  buffer.rst: clean up timecode documentation
  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   | 28 +
 .../media/uapi/v4l/vidioc-reqbufs.rst |  4 ++
 .../media/common/videobuf2/videobuf2-v4l2.c   | 41 ---
 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, 168 insertions(+), 75 deletions(-)

-- 
2.19.1

[PATCHv4 06/10] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function

[hverkuil-cisco]

From: 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 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 5bbdec55b7d7..a9cb1ac33dc0 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -975,6 +975,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(_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

[PATCHv4 03/10] v4l2-ioctl.c: log v4l2_buffer tag

[hverkuil-cisco]

From: Hans Verkuil 

When debugging is on, log the new tag field of struct v4l2_buffer
as well.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 b9616b1f227b..07c6c939a23c 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);
+   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

[PATCHv4 07/10] vb2: add new supports_tags queue flag

[hverkuil-cisco]

From: Hans Verkuil 

Add new flag to indicate that buffer tags are supported.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 e0e31e1c67c9..5aa5b1ea90a8 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -659,6 +659,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 {
unsignedallow_zero_bytesused:1;
unsigned   quirk_poll_must_check_waiting_for_buffers:1;
unsignedsupports_requests:1;
+   unsignedsupports_tags:1;
unsigneduses_qbuf:1;
unsigneduses_requests:1;
 
-- 
2.19.1

[PATCHv4 04/10] buffer.rst: document the new buffer tag feature.

[hverkuil-cisco]

From: Hans Verkuil 

Document V4L2_BUF_FLAG_TAG.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 Documentation/media/uapi/v4l/buffer.rst | 17 ++---
 Documentation/media/uapi/v4l/vidioc-reqbufs.rst |  4 
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index 2e266d32470a..f83ee00cb30b 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -301,10 +301,13 @@ struct v4l2_buffer
elements in the ``planes`` array. The driver will fill in the
actual number of valid elements in that array.
 * - __u32
-  - ``reserved2``
+  - ``tag``
   -
-  - A place holder for future extensions. Drivers and applications
-   must set this to 0.
+  - When the ``V4L2_BUF_FLAG_TAG`` flag is set in ``flags``, this
+   field 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
   - ``request_fd``
   -
@@ -567,6 +570,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``
+  - 0x0200
+  - The ``tag`` field is valid. Applications can set
+   this bit and the corresponding ``tag`` field. If tags are
+   supported then the ``V4L2_BUF_CAP_SUPPORTS_TAGS`` capability
+   is also set.
 * .. _`V4L2-BUF-FLAG-PREPARED`:
 
   - ``V4L2_BUF_FLAG_PREPARED``
diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst 
b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index e62a15782790..38a7d0aee483 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -118,6 +118,7 @@ aborting or finishing any DMA in progress, an implicit
 .. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
 .. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
 .. _V4L2-BUF-CAP-SUPPORTS-ORPHANED-BUFS:
+.. _V4L2-BUF-CAP-SUPPORTS-TAGS:
 
 .. cssclass:: longtable
 
@@ -143,6 +144,9 @@ aborting or finishing any DMA in progress, an implicit
   - The kernel allows calling :ref:`VIDIOC_REQBUFS` while buffers are still
 mapped or exported via DMABUF. These orphaned buffers will be freed
 when they are unmapped or when the exported DMABUF fds are closed.
+* - ``V4L2_BUF_CAP_SUPPORTS_TAGS``
+  - 0x0020
+  - This buffer type supports ``V4L2_BUF_FLAG_TAG``.
 
 Return Value
 
-- 
2.19.1

[PATCHv4 09/10] vicodec: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

Copy tags in vicodec.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 b7bdfe97215b..4d39ea033653 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;
 }
@@ -1083,6 +1073,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 ? >dev->enc_mutex :
>dev->dec_mutex;
+   src_vq->supports_tags = true;
 
ret = vb2_queue_init(src_vq);
if (ret)
@@ -1098,6 +1089,7 @@ static int queue_init(void *priv, struct vb2_queue 
*src_vq,
dst_vq->mem_ops = _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

[PATCHv3 3/9] v4l2-ioctl.c: log v4l2_buffer tag

[hverkuil-cisco]

From: Hans Verkuil 

When debugging is on, log the new tag field of struct v4l2_buffer
as well.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 e384142d2826..653497f31104 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

[PATCHv3 0/9] vb2/cedrus: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

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 u32 tag to an output buffer, which is then 
copied to the capture buffer(s) derived from the output buffer.

It has been suggested that the timestamp can be used for this. But
there are a number of reasons why this is a bad idea:

1) the struct timeval is converted to a u64 in vb2. So there can be
   all sorts of unexpected conversion issues. In particular, the
   output of ns_to_timeval(timeval_to_ns(tv)) does not necessarily
   match the input.

2) it gets worse with the y2038 code where userspace either deals
   with a 32 bit tv_sec value or a 64 bit value.

In other words, using timestamp for this is not a good idea.

This implementation adds a new tag field in a union with the timecode
field. Right now timecode is not actually used in any driver, except
that m2m drivers are supposed to copy it from the OUTPUT queue to the
CAPTURE queue (it can be set by userspace, so it should be copied by
the driver). An alternative would be to use the reserved2 field in
struct v4l2_buffer for the tag. But then we have no more room for
a fence fd. But if we decide on a struct v4l2_ext_buffer API going
forward, then we might just use that for fences as well and we can
just take the reserved2 field for the tag instead.

I have no strong opinion on this.

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.

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.

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!

Regards,

Hans

Changes since v2:

- rebased
- added Reviewed-by tags
- fixed a few remaining references in the documentation to the old
  v4l2_buffer_tag struct that was used in early versions of this
  series.

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

[PATCHv3 1/9] videodev2.h: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

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 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 2a223835214c..99b2b13a5757 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -880,6 +880,7 @@ struct v4l2_requestbuffers {
 #define V4L2_BUF_CAP_SUPPORTS_DMABUF   (1 << 2)
 #define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3)
 #define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS (1 << 4)
+#define V4L2_BUF_CAP_SUPPORTS_TAGS (1 << 5)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
@@ -924,6 +925,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
@@ -951,7 +953,10 @@ struct v4l2_buffer {
__u32   flags;
__u32   field;
struct timeval  timestamp;
-   struct v4l2_timecodetimecode;
+   union {
+   struct v4l2_timecodetimecode;
+   __u32   tag;
+   };
__u32   sequence;
 
/* memory location */
@@ -989,6 +994,8 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_IN_REQUEST   0x0080
 /* timecode field is valid */
 #define V4L2_BUF_FLAG_TIMECODE 0x0100
+/* tag field is valid */
+#define V4L2_BUF_FLAG_TAG  0x0200
 /* Buffer is prepared for queuing */
 #define V4L2_BUF_FLAG_PREPARED 0x0400
 /* Cache handling flags */
-- 
2.19.1

[PATCHv3 2/9] vb2: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

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 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 .../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 1244c246d0c4..ecbf4f0755cb 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(>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;
}
 };
@@ -194,6 +198,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) {
@@ -313,13 +318,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 */
@@ -460,7 +471,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;
@@ -586,6 +600,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
  *  v4l2_field.
  

[PATCHv3 7/9] vim2m: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

Copy tags in vim2m.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 d01821a6906a..be328483a53a 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:
@@ -855,6 +845,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 = >dev->dev_mutex;
src_vq->supports_requests = true;
+   src_vq->supports_tags = true;
 
ret = vb2_queue_init(src_vq);
if (ret)
@@ -868,6 +859,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, 
struct vb2_queue *ds
dst_vq->mem_ops = _vmalloc_memops;
dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
dst_vq->lock = >dev->dev_mutex;
+   dst_vq->supports_tags = true;
 
return vb2_queue_init(dst_vq);
 }
-- 
2.19.1

[PATCHv3 5/9] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function

[hverkuil-cisco]

From: 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 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 5bbdec55b7d7..a9cb1ac33dc0 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -975,6 +975,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(_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

[PATCHv3 4/9] buffer.rst: document the new buffer tag feature.

[hverkuil-cisco]

From: Hans Verkuil 

Document V4L2_BUF_FLAG_TAG.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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..3c09a94c5a10 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 ` 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
+   field 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``
+  - 0x0200
+  - The ``tag`` field is valid. Applications can set
+   this bit and the corresponding ``tag`` field. 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 e62a15782790..38a7d0aee483 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -118,6 +118,7 @@ aborting or finishing any DMA in progress, an implicit
 .. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
 .. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
 .. _V4L2-BUF-CAP-SUPPORTS-ORPHANED-BUFS:
+.. _V4L2-BUF-CAP-SUPPORTS-TAGS:
 
 .. cssclass:: longtable
 
@@ -143,6 +144,9 @@ aborting or finishing any DMA in progress, an implicit
   - The kernel allows calling :ref:`VIDIOC_REQBUFS` while buffers are still
 mapped or exported via DMABUF. These orphaned buffers will be freed
 when they are unmapped or when the exported DMABUF fds are closed.
+* - ``V4L2_BUF_CAP_SUPPORTS_TAGS``
+  - 0x0020
+  - This buffer type supports ``V4L2_BUF_FLAG_TAG``.
 
 Return Value
 
-- 
2.19.1

[PATCHv3 6/9] vb2: add new supports_tags queue flag

[hverkuil-cisco]

From: Hans Verkuil 

Add new flag to indicate that buffer tags are supported.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 ecbf4f0755cb..189dd7fb12c0 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -665,6 +665,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 {
unsignedallow_zero_bytesused:1;
unsigned   quirk_poll_must_check_waiting_for_buffers:1;
unsignedsupports_requests:1;
+   unsignedsupports_tags:1;
unsigneduses_qbuf:1;
unsigneduses_requests:1;
 
-- 
2.19.1

[PATCHv3 8/9] vicodec: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

Copy tags in vicodec.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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 b7bdfe97215b..4d39ea033653 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;
 }
@@ -1083,6 +1073,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 ? >dev->enc_mutex :
>dev->dec_mutex;
+   src_vq->supports_tags = true;
 
ret = vb2_queue_init(src_vq);
if (ret)
@@ -1098,6 +1089,7 @@ static int queue_init(void *priv, struct vb2_queue 
*src_vq,
dst_vq->mem_ops = _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

[PATCHv3 9/9] cedrus: add tag support

[hverkuil-cisco]

From: Hans Verkuil 

Replace old reference frame indices by new tag method.

Signed-off-by: Hans Verkuil 
Reviewed-by: Paul Kocialkowski 
Reviewed-by: Alexandre Courbot 
---
 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, >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, );
 
spin_unlock_irqrestore(>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 = >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 = 

[PATCH 2/3] vim2m: add buf_validate callback

[hverkuil-cisco]

From: Hans Verkuil 

Split off the field validation from buf_prepare into a new
buf_validate function. Field validation for output buffers should
be done there since buf_prepare is not guaranteed to be called at
QBUF time.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/vim2m.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index d01821a6906a..9559be91daca 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -753,15 +753,13 @@ static int vim2m_queue_setup(struct vb2_queue *vq,
return 0;
 }
 
-static int vim2m_buf_prepare(struct vb2_buffer *vb)
+static int vim2m_buf_validate(struct vb2_buffer *vb)
 {
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct vim2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
-   struct vim2m_q_data *q_data;
 
dprintk(ctx->dev, "type: %d\n", vb->vb2_queue->type);
 
-   q_data = get_q_data(ctx, vb->vb2_queue->type);
if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
if (vbuf->field == V4L2_FIELD_ANY)
vbuf->field = V4L2_FIELD_NONE;
@@ -772,6 +770,17 @@ static int vim2m_buf_prepare(struct vb2_buffer *vb)
}
}
 
+   return 0;
+}
+
+static int vim2m_buf_prepare(struct vb2_buffer *vb)
+{
+   struct vim2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+   struct vim2m_q_data *q_data;
+
+   dprintk(ctx->dev, "type: %d\n", vb->vb2_queue->type);
+
+   q_data = get_q_data(ctx, vb->vb2_queue->type);
if (vb2_plane_size(vb, 0) < q_data->sizeimage) {
dprintk(ctx->dev, "%s data will not fit into plane (%lu < 
%lu)\n",
__func__, vb2_plane_size(vb, 0), 
(long)q_data->sizeimage);
@@ -832,6 +841,7 @@ static void vim2m_buf_request_complete(struct vb2_buffer 
*vb)
 
 static const struct vb2_ops vim2m_qops = {
.queue_setup = vim2m_queue_setup,
+   .buf_validate= vim2m_buf_validate,
.buf_prepare = vim2m_buf_prepare,
.buf_queue   = vim2m_buf_queue,
.start_streaming = vim2m_start_streaming,
-- 
2.19.1

[PATCH 3/3] vivid: add buf_validate callback

[hverkuil-cisco]

From: Hans Verkuil 

Split off the field validation from buf_prepare into a new
buf_validate function. Field validation for output buffers should
be done there since buf_prepare is not guaranteed to be called at
QBUF time.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/vivid/vivid-vid-out.c | 23 ++--
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-vid-out.c 
b/drivers/media/platform/vivid/vivid-vid-out.c
index 7642cbdb0e14..3e93dbbb4ffe 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -81,10 +81,24 @@ static int vid_out_queue_setup(struct vb2_queue *vq,
return 0;
 }
 
-static int vid_out_buf_prepare(struct vb2_buffer *vb)
+static int vid_out_buf_validate(struct vb2_buffer *vb)
 {
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
struct vivid_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
+
+   dprintk(dev, 1, "%s\n", __func__);
+
+   if (dev->field_out != V4L2_FIELD_ALTERNATE)
+   vbuf->field = dev->field_out;
+   else if (vbuf->field != V4L2_FIELD_TOP &&
+vbuf->field != V4L2_FIELD_BOTTOM)
+   return -EINVAL;
+   return 0;
+}
+
+static int vid_out_buf_prepare(struct vb2_buffer *vb)
+{
+   struct vivid_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
unsigned long size;
unsigned planes;
unsigned p;
@@ -105,12 +119,6 @@ static int vid_out_buf_prepare(struct vb2_buffer *vb)
return -EINVAL;
}
 
-   if (dev->field_out != V4L2_FIELD_ALTERNATE)
-   vbuf->field = dev->field_out;
-   else if (vbuf->field != V4L2_FIELD_TOP &&
-vbuf->field != V4L2_FIELD_BOTTOM)
-   return -EINVAL;
-
for (p = 0; p < planes; p++) {
size = dev->bytesperline_out[p] * dev->fmt_out_rect.height +
vb->planes[p].data_offset;
@@ -190,6 +198,7 @@ static void vid_out_buf_request_complete(struct vb2_buffer 
*vb)
 
 const struct vb2_ops vivid_vid_out_qops = {
.queue_setup= vid_out_queue_setup,
+   .buf_validate   = vid_out_buf_validate,
.buf_prepare= vid_out_buf_prepare,
.buf_queue  = vid_out_buf_queue,
.start_streaming= vid_out_start_streaming,
-- 
2.19.1

[PATCH 1/3] vb2: add buf_validate callback

[hverkuil-cisco]

From: Hans Verkuil 

Adding the request API uncovered a pre-existing problem with
validating output buffers.

The problem is that for output buffers the driver has to validate
the 'field' field of struct v4l2_buffer. This is critical when
encoding or deinterlacing interlaced video.

Drivers always did this in the buf_prepare callback, but that is
not called from VIDIOC_QBUF in two situations: when queueing a
buffer to a request and if VIDIOC_PREPARE_BUF has been called
earlier for that buffer.

As a result of this the 'field' value is not validated.

This patch adds a new buf_validate callback to validate the
output buffer at QBUF time.

Note that PREPARE_BUF doesn't need to validate this: it just
locks the buffer memory and doesn't need nor want to know about
how this buffer is actually going to be used. It's the QBUF ioctl
that determines this.

This issue was found by v4l2-compliance since it failed to replace
V4L2_FIELD_ANY by a proper field value when testing the vivid video
output in combination with requests.

There never was a test before for the PREPARE_BUF/QBUF case, so even
though this bug has been present for quite some time, it was never
noticed.

Signed-off-by: Hans Verkuil 
---
 drivers/media/common/videobuf2/videobuf2-core.c | 12 +---
 include/media/videobuf2-core.h  |  5 +
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 0ca81d495bda..42eb7716f8a9 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -499,9 +499,9 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned 
int buffers)
pr_info(" buf_init: %u buf_cleanup: %u buf_prepare: 
%u buf_finish: %u\n",
vb->cnt_buf_init, vb->cnt_buf_cleanup,
vb->cnt_buf_prepare, vb->cnt_buf_finish);
-   pr_info(" buf_queue: %u buf_done: %u 
buf_request_complete: %u\n",
-   vb->cnt_buf_queue, vb->cnt_buf_done,
-   vb->cnt_buf_request_complete);
+   pr_info(" buf_validate: %u buf_queue: %u buf_done: 
%u buf_request_complete: %u\n",
+   vb->cnt_buf_validate, vb->cnt_buf_queue,
+   vb->cnt_buf_done, vb->cnt_buf_request_complete);
pr_info(" alloc: %u put: %u prepare: %u finish: %u 
mmap: %u\n",
vb->cnt_mem_alloc, vb->cnt_mem_put,
vb->cnt_mem_prepare, vb->cnt_mem_finish,
@@ -1506,6 +1506,12 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb,
return -EBUSY;
}
 
+   ret = call_vb_qop(vb, buf_validate, vb);
+   if (ret) {
+   dprintk(1, "buffer validation failed\n");
+   return ret;
+   }
+
if (req) {
int ret;
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index e86981d615ae..c9f0f3f4ef9a 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -294,6 +294,7 @@ struct vb2_buffer {
u32 cnt_mem_num_users;
u32 cnt_mem_mmap;
 
+   u32 cnt_buf_validate;
u32 cnt_buf_init;
u32 cnt_buf_prepare;
u32 cnt_buf_finish;
@@ -340,6 +341,9 @@ struct vb2_buffer {
  * @wait_finish:   reacquire all locks released in the previous callback;
  * required to continue operation after sleeping while
  * waiting for a new buffer to arrive.
+ * @buf_validate:  called every time the buffer is queued from userspace;
+ * drivers can use this to validate userspace-provided
+ * information; optional.
  * @buf_init:  called once after allocating a buffer (in MMAP case)
  * or after acquiring a new USERPTR buffer; drivers may
  * perform additional buffer-related initialization;
@@ -407,6 +411,7 @@ struct vb2_ops {
void (*wait_prepare)(struct vb2_queue *q);
void (*wait_finish)(struct vb2_queue *q);
 
+   int (*buf_validate)(struct vb2_buffer *vb);
int (*buf_init)(struct vb2_buffer *vb);
int (*buf_prepare)(struct vb2_buffer *vb);
void (*buf_finish)(struct vb2_buffer *vb);
-- 
2.19.1

[PATCH for v4.20 3/5] vb2: keep a reference to the request until dqbuf

[hverkuil-cisco]

From: Hans Verkuil 

When vb2_buffer_done is called the buffer is unbound from the
request and put. The media_request_object_put also 'put's the
request reference. If the application has already closed the
request fd, then that means that the request reference at that
point goes to 0 and the whole request is released.

This means that the control handler associated with the request is
also freed and that causes this kernel oops:

[174705.995401] BUG: sleeping function called from invalid context at 
kernel/locking/mutex.c:908
[174705.995411] in_atomic(): 1, irqs_disabled(): 1, pid: 28071, name: 
vivid-000-vid-o
[174705.995416] 2 locks held by vivid-000-vid-o/28071:
[174705.995420]  #0: 1ea3a232 (>mutex#3){}, at: 
vivid_thread_vid_out+0x3f5/0x550 [vivid]
[174705.995447]  #1: e30a0d1e (&(>done_lock)->rlock){}, at: 
vb2_buffer_done+0x92/0x1d0 [videobuf2_common]
[174705.995460] Preemption disabled at:
[174705.995461] [<>]   (null)
[174705.995472] CPU: 11 PID: 28071 Comm: vivid-000-vid-o Tainted: GW
 4.20.0-rc1-test-no #88
[174705.995476] Hardware name: VMware, Inc. VMware Virtual Platform/440BX 
Desktop Reference Platform, BIOS 6.00 05/19/2017
[174705.995481] Call Trace:
[174705.995500]  dump_stack+0x46/0x60
[174705.995512]  ___might_sleep.cold.79+0xe1/0xf1
[174705.995523]  __mutex_lock+0x50/0x8f0
[174705.995531]  ? find_held_lock+0x2d/0x90
[174705.995536]  ? find_held_lock+0x2d/0x90
[174705.995542]  ? find_held_lock+0x2d/0x90
[174705.995564]  ? v4l2_ctrl_handler_free.part.13+0x44/0x1d0 [videodev]
[174705.995576]  v4l2_ctrl_handler_free.part.13+0x44/0x1d0 [videodev]
[174705.995590]  v4l2_ctrl_request_release+0x1c/0x30 [videodev]
[174705.995600]  media_request_clean+0x64/0xe0 [media]
[174705.995609]  media_request_release+0x19/0x40 [media]
[174705.995617]  vb2_buffer_done+0xef/0x1d0 [videobuf2_common]
[174705.995630]  vivid_thread_vid_out+0x2c1/0x550 [vivid]
[174705.995645]  ? vivid_stop_generating_vid_cap+0x1c0/0x1c0 [vivid]
[174705.995653]  kthread+0x113/0x130
[174705.995659]  ? kthread_park+0x80/0x80
[174705.995667]  ret_from_fork+0x35/0x40

The vb2_buffer_done function can be called from interrupt context, so
anything that sleeps is not allowed.

The solution is to increment the request refcount when the buffer is
queued and decrement it when the buffer is dequeued. Releasing the
request is fine if that happens from VIDIOC_DQBUF.

Signed-off-by: Hans Verkuil 
---
 .../media/common/videobuf2/videobuf2-core.c   | 38 ---
 include/media/videobuf2-core.h|  2 +
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 77e2bfe5e722..86a12b335aac 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1360,8 +1360,12 @@ static void vb2_req_release(struct media_request_object 
*obj)
 {
struct vb2_buffer *vb = container_of(obj, struct vb2_buffer, req_obj);
 
-   if (vb->state == VB2_BUF_STATE_IN_REQUEST)
+   if (vb->state == VB2_BUF_STATE_IN_REQUEST) {
vb->state = VB2_BUF_STATE_DEQUEUED;
+   if (vb->request)
+   media_request_put(vb->request);
+   vb->request = NULL;
+   }
 }
 
 static const struct media_request_object_ops vb2_core_req_ops = {
@@ -1529,6 +1533,18 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb,
return ret;
 
vb->state = VB2_BUF_STATE_IN_REQUEST;
+
+   /*
+* Increment the refcount and store the request.
+* The request refcount is decremented again when the
+* buffer is dequeued. This is to prevent vb2_buffer_done()
+* from freeing the request from interrupt context, which can
+* happen if the application closed the request fd after
+* queueing the request.
+*/
+   media_request_get(req);
+   vb->request = req;
+
/* Fill buffer information for the userspace */
if (pb) {
call_void_bufop(q, copy_timestamp, vb, pb);
@@ -1750,10 +1766,6 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
call_void_memop(vb, unmap_dmabuf, 
vb->planes[i].mem_priv);
vb->planes[i].dbuf_mapped = 0;
}
-   if (vb->req_obj.req) {
-   media_request_object_unbind(>req_obj);
-   media_request_object_put(>req_obj);
-   }
call_void_bufop(q, init_buffer, vb);
 }
 
@@ -1798,6 +1810,14 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int 
*pindex, void *pb,
/* go back to dequeued state */
__vb2_dqbuf(vb);
 
+   if (WARN_ON(vb->req_obj.req)) {
+   media_request_object_unbind(>req_obj);
+   

[PATCH for v4.20 4/5] vb2: don't unbind/put the object when going to state QUEUED

[hverkuil-cisco]

From: Hans Verkuil 

When a buffer is returned to state QUEUED (that happens when
start_streaming fails), then do not unbind and put the object
from the request. Nothing has changed yet, so just keep it as
is.

Signed-off-by: Hans Verkuil 
---
 drivers/media/common/videobuf2/videobuf2-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 86a12b335aac..70e8c3366f9c 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -948,7 +948,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
}
atomic_dec(>owned_by_drv_count);
 
-   if (vb->req_obj.req) {
+   if (state != VB2_BUF_STATE_QUEUED && vb->req_obj.req) {
/* This is not supported at the moment */
WARN_ON(state == VB2_BUF_STATE_REQUEUEING);
media_request_object_unbind(>req_obj);
-- 
2.19.1

[PATCH for v4.20 1/5] vb2: don't call __vb2_queue_cancel if vb2_start_streaming failed

[hverkuil-cisco]

From: Hans Verkuil 

vb2_start_streaming() already rolls back the buffers, so there is no
need to call __vb2_queue_cancel(). Especially since __vb2_queue_cancel()
does too much, such as zeroing the q->queued_count value, causing vb2
to think that no buffers have been queued.

It appears that this call to __vb2_queue_cancel() is a left-over from
before commit b3379c6201bb3.

Signed-off-by: Hans Verkuil 
Fixes: b3379c6201bb3 ('vb2: only call start_streaming if sufficient buffers are 
queued')
Cc:   # for v4.16 and up
---
 drivers/media/common/videobuf2/videobuf2-core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 0ca81d495bda..77e2bfe5e722 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1941,10 +1941,8 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int 
type)
if (ret)
return ret;
ret = vb2_start_streaming(q);
-   if (ret) {
-   __vb2_queue_cancel(q);
+   if (ret)
return ret;
-   }
}
 
q->streaming = 1;
-- 
2.19.1

[PATCH for v4.20 0/5] vb2 fixes (mostly request API related)

[hverkuil-cisco]

From: Hans Verkuil 

While improving the v4l2-compliance tests I came across several vb2
problems.

After modifying v4l2-compliance I was now able to use the vivid error
injection feature to test what happens if VIDIOC_STREAMON fails and
a following STREAMON succeeds.

This generated patches 1/5 and 4/5+5/5.

Patch 1/5 fixes an issue that was never noticed before since it didn't
result in kernel oopses or warnings, but after the second STREAMON it
won't call start_streaming anymore until you call REQBUFS(0) or close
the device node.

Patches 4 and 5 are request specific fixes for the same corner case:
the code would unbind (in vb2) or complete (in vivid) a request if
start_streaming fails, but it should just leave things as-is. The
buffer is just put back in the queue, ready for the next attempt at
STREAMON.

Patch 2/5 was also discovered by v4l2-compliance: the request fd in
v4l2_buffer should be ignored by VIDIOC_PREPARE_BUF, but it wasn't.

Patch 3/5 fixes a nasty corner case: a buffer with associated request
is queued, and then the request fd is closed by v4l2-compliance.

When the driver calls vb2_buffer_done, the buffer request object is
unbound, the object is put, and indirectly the associated request
is put as well, and since that was the last references to the request
the whole request is released, which requires the ability to call
mutex_lock. But vb2_buffer_done is atomic (it can be called
from interrupt context), so this shouldn't happen.

vb2 now takes an extra refcount to the request on qbuf and releases
it on dqbuf and at two other places where an internal dqbuf is done.

Note that 'skip request checks for VIDIOC_PREPARE_BUF' is a duplicate
of https://patchwork.linuxtv.org/patch/53171/, which is now marked as
superseded.

I've marked all these patches for 4.20, but I think it is also possible
to apply them for 4.21 since the request API is only used by virtual
drivers and a staging driver.

Regards,

Hans

Hans Verkuil (5):
  vb2: don't call __vb2_queue_cancel if vb2_start_streaming failed
  vb2: skip request checks for VIDIOC_PREPARE_BUF
  vb2: keep a reference to the request until dqbuf
  vb2: don't unbind/put the object when going to state QUEUED
  vivid: drop v4l2_ctrl_request_complete() from start_streaming

 .../media/common/videobuf2/videobuf2-core.c   | 44 +++
 .../media/common/videobuf2/videobuf2-v4l2.c   | 11 +++--
 drivers/media/platform/vivid/vivid-sdr-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vbi-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vbi-out.c  |  2 -
 drivers/media/platform/vivid/vivid-vid-cap.c  |  2 -
 drivers/media/platform/vivid/vivid-vid-out.c  |  2 -
 include/media/videobuf2-core.h|  2 +
 8 files changed, 44 insertions(+), 23 deletions(-)

-- 
2.19.1

[PATCH for v4.20 5/5] vivid: drop v4l2_ctrl_request_complete() from start_streaming

[hverkuil-cisco]

From: Hans Verkuil 

If start_streaming() fails and all queued buffers are returned to
vb2, then do not call v4l2_ctrl_request_complete(). Nothing happened
to the request and the state should remain as it was before
start_streaming was called.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/vivid/vivid-sdr-cap.c | 2 --
 drivers/media/platform/vivid/vivid-vbi-cap.c | 2 --
 drivers/media/platform/vivid/vivid-vbi-out.c | 2 --
 drivers/media/platform/vivid/vivid-vid-cap.c | 2 --
 drivers/media/platform/vivid/vivid-vid-out.c | 2 --
 5 files changed, 10 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c 
b/drivers/media/platform/vivid/vivid-sdr-cap.c
index dcdc80e272c2..9acc709b0740 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -276,8 +276,6 @@ static int sdr_cap_start_streaming(struct vb2_queue *vq, 
unsigned count)
 
list_for_each_entry_safe(buf, tmp, >sdr_cap_active, list) {
list_del(>list);
-   v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-  >ctrl_hdl_sdr_cap);
vb2_buffer_done(>vb.vb2_buf,
VB2_BUF_STATE_QUEUED);
}
diff --git a/drivers/media/platform/vivid/vivid-vbi-cap.c 
b/drivers/media/platform/vivid/vivid-vbi-cap.c
index 903cebeb5ce5..d666271bdaed 100644
--- a/drivers/media/platform/vivid/vivid-vbi-cap.c
+++ b/drivers/media/platform/vivid/vivid-vbi-cap.c
@@ -204,8 +204,6 @@ static int vbi_cap_start_streaming(struct vb2_queue *vq, 
unsigned count)
 
list_for_each_entry_safe(buf, tmp, >vbi_cap_active, list) {
list_del(>list);
-   v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-  >ctrl_hdl_vbi_cap);
vb2_buffer_done(>vb.vb2_buf,
VB2_BUF_STATE_QUEUED);
}
diff --git a/drivers/media/platform/vivid/vivid-vbi-out.c 
b/drivers/media/platform/vivid/vivid-vbi-out.c
index 9357c07e30d6..cd56476902a2 100644
--- a/drivers/media/platform/vivid/vivid-vbi-out.c
+++ b/drivers/media/platform/vivid/vivid-vbi-out.c
@@ -96,8 +96,6 @@ static int vbi_out_start_streaming(struct vb2_queue *vq, 
unsigned count)
 
list_for_each_entry_safe(buf, tmp, >vbi_out_active, list) {
list_del(>list);
-   v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-  >ctrl_hdl_vbi_out);
vb2_buffer_done(>vb.vb2_buf,
VB2_BUF_STATE_QUEUED);
}
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c 
b/drivers/media/platform/vivid/vivid-vid-cap.c
index a1ed5fdabc75..c059fc12668a 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -243,8 +243,6 @@ static int vid_cap_start_streaming(struct vb2_queue *vq, 
unsigned count)
 
list_for_each_entry_safe(buf, tmp, >vid_cap_active, list) {
list_del(>list);
-   v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-  >ctrl_hdl_vid_cap);
vb2_buffer_done(>vb.vb2_buf,
VB2_BUF_STATE_QUEUED);
}
diff --git a/drivers/media/platform/vivid/vivid-vid-out.c 
b/drivers/media/platform/vivid/vivid-vid-out.c
index 7642cbdb0e14..ea250aee2b2e 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -162,8 +162,6 @@ static int vid_out_start_streaming(struct vb2_queue *vq, 
unsigned count)
 
list_for_each_entry_safe(buf, tmp, >vid_out_active, list) {
list_del(>list);
-   v4l2_ctrl_request_complete(buf->vb.vb2_buf.req_obj.req,
-  >ctrl_hdl_vid_out);
vb2_buffer_done(>vb.vb2_buf,
VB2_BUF_STATE_QUEUED);
}
-- 
2.19.1

[PATCH for v4.20 2/5] vb2: skip request checks for VIDIOC_PREPARE_BUF

[hverkuil-cisco]

From: Hans Verkuil 

VIDIOC_PREPARE_BUF should ignore V4L2_BUF_FLAG_REQUEST_FD since it isn't
doing anything with requests. So inform vb2_queue_or_prepare_buf whether
it is called from vb2_prepare_buf or vb2_qbuf and just return 0 in the
first case.

This was found when adding new v4l2-compliance checks.

Signed-off-by: Hans Verkuil 
---
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c 
b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 1244c246d0c4..1ac1b3f334f6 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -332,10 +332,10 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer 
*vb, struct v4l2_buffer *b
 }
 
 static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device 
*mdev,
-   struct v4l2_buffer *b,
-   const char *opname,
+   struct v4l2_buffer *b, bool is_prepare,
struct media_request **p_req)
 {
+   const char *opname = is_prepare ? "prepare_buf" : "qbuf";
struct media_request *req;
struct vb2_v4l2_buffer *vbuf;
struct vb2_buffer *vb;
@@ -377,6 +377,9 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, 
struct media_device *md
return ret;
}
 
+   if (is_prepare)
+   return 0;
+
if (!(b->flags & V4L2_BUF_FLAG_REQUEST_FD)) {
if (q->uses_requests) {
dprintk(1, "%s: queue uses requests\n", opname);
@@ -656,7 +659,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct 
media_device *mdev,
if (b->flags & V4L2_BUF_FLAG_REQUEST_FD)
return -EINVAL;
 
-   ret = vb2_queue_or_prepare_buf(q, mdev, b, "prepare_buf", NULL);
+   ret = vb2_queue_or_prepare_buf(q, mdev, b, true, NULL);
 
return ret ? ret : vb2_core_prepare_buf(q, b->index, b);
 }
@@ -728,7 +731,7 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev,
return -EBUSY;
}
 
-   ret = vb2_queue_or_prepare_buf(q, mdev, b, "qbuf", );
+   ret = vb2_queue_or_prepare_buf(q, mdev, b, false, );
if (ret)
return ret;
ret = vb2_core_qbuf(q, b->index, b, req);
-- 
2.19.1