[PATCH v5 00/23] support for stateless decoder

[PATCH v5 00/23] support for stateless decoder

[Dafna Hirschfeld]

Main Changes from v4:
1. fixes according to the review
2. two new patches:
  0015-media-vicodec-Handle-the-case-that-the-reference-buf.patch
  0023-media-vicodec-set-pixelformat-to-V4L2_PIX_FMT_FWHT_S.patch

Dafna Hirschfeld (20):
  media: vicodec: selection api should only check single buffer types
  media: vicodec: upon release, call m2m release before freeing ctrl
    handler
  media: v4l2-ctrl: v4l2_ctrl_request_setup returns with error upon
    failure
  media: vicodec: change variable name for the return value of
    v4l2_fwht_encode
  media: vicodec: bugfix - call v4l2_m2m_buf_copy_metadata also if
    decoding fails
  media: vicodec: bugfix: free compressed_frame upon device release
  media: vicodec: Move raw frame preparation code to a function
  media: vicodec: add field 'buf' to fwht_raw_frame
  media: vicodec: keep the ref frame according to the format in decoder
  media: vicodec: Validate version dependent header values in a separate
    function
  media: vicodec: rename v4l2_fwht_default_fmt to v4l2_fwht_find_nth_fmt
  media: vicodec: Handle the case that the reference buffer is NULL
  media: vicodec: add struct for encoder/decoder instance
  media: vicodec: add documentation to V4L2_CID_FWHT_I/P_FRAME_QP
  media: vicodec: add documentation to V4L2_CID_MPEG_VIDEO_FWHT_PARAMS
  media: vicodec: add documentation to V4L2_PIX_FMT_FWHT_STATELESS
  media: vicodec: Introducing stateless fwht defs and structs
  media: vicodec: Register another node for stateless decoder
  media: vicodec: Add support for stateless decoder.
  media: vicodec: set pixelformat to V4L2_PIX_FMT_FWHT_STATELESS for
    stateless decoder

Hans Verkuil (3):
  vb2: add requires_requests bit for stateless codecs
  videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS
  cedrus: set requires_requests

 .../media/uapi/v4l/ext-ctrls-codec.rst        | 130 ++++
 .../media/uapi/v4l/pixfmt-compressed.rst      |   6 +
 .../media/uapi/v4l/vidioc-reqbufs.rst         |   4 +
 .../media/common/videobuf2/videobuf2-core.c   |   5 +-
 .../media/common/videobuf2/videobuf2-v4l2.c   |   6 +
 drivers/media/platform/vicodec/codec-fwht.c   |  92 ++-
 drivers/media/platform/vicodec/codec-fwht.h   |  12 +-
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 431 ++++-------
 .../media/platform/vicodec/codec-v4l2-fwht.h  |   7 +-
 drivers/media/platform/vicodec/vicodec-core.c | 731 +++++++++++++-----
 drivers/media/v4l2-core/v4l2-ctrls.c          |  30 +-
 .../staging/media/sunxi/cedrus/cedrus_video.c |   1 +
 include/media/fwht-ctrls.h                    |  31 +
 include/media/v4l2-ctrls.h                    |   7 +-
 include/media/videobuf2-core.h                |   3 +
 include/uapi/linux/v4l2-controls.h            |   4 +
 include/uapi/linux/videodev2.h                |   2 +
 17 files changed, 940 insertions(+), 562 deletions(-)
 create mode 100644 include/media/fwht-ctrls.h

-- 
2.17.1

[PATCH v5 01/23] vb2: add requires_requests bit for stateless codecs

[Dafna Hirschfeld]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Stateless codecs require the use of the Request API as opposed of it
being optional.

So add a bit to indicate this and let vb2 check for this.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 5 ++++-
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 6 ++++++
 include/media/videobuf2-core.h                  | 3 +++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 15b6b9c0a2e4..d8cf9d3ec54d 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1518,7 +1518,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
 
 	if ((req && q->uses_qbuf) ||
 	    (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
-	     q->uses_requests)) {
+	     (q->uses_requests || q->requires_requests))) {
 		dprintk(1, "queue in wrong mode (qbuf vs requests)\n");
 		return -EBUSY;
 	}
@@ -2247,6 +2247,9 @@ int vb2_core_queue_init(struct vb2_queue *q)
 	    WARN_ON(!q->ops->buf_queue))
 		return -EINVAL;
 
+	if (WARN_ON(q->requires_requests && !q->supports_requests))
+		return -EINVAL;
+
 	INIT_LIST_HEAD(&q->queued_list);
 	INIT_LIST_HEAD(&q->done_list);
 	spin_lock_init(&q->done_lock);
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index d09dee20e421..4dc4855056f1 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -385,6 +385,10 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
 			dprintk(1, "%s: queue uses requests\n", opname);
 			return -EBUSY;
 		}
+		if (q->requires_requests) {
+			dprintk(1, "%s: queue requires requests\n", opname);
+			return -EACCES;
+		}
 		return 0;
 	} else if (!q->supports_requests) {
 		dprintk(1, "%s: queue does not support requests\n", opname);
@@ -658,6 +662,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
 #ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API
 	if (q->supports_requests)
 		*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
+	if (q->requires_requests)
+		*caps |= V4L2_BUF_CAP_REQUIRES_REQUESTS;
 #endif
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 910f3d469005..fbf8dbbcbc09 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -484,6 +484,8 @@ 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.
+ * @requires_requests: this queue requires the Request API. If this is set to 1,
+ *		then supports_requests must be set to 1 as well.
  * @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.
@@ -558,6 +560,7 @@ struct vb2_queue {
 	unsigned			allow_zero_bytesused:1;
 	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
 	unsigned			supports_requests:1;
+	unsigned			requires_requests:1;
 	unsigned			uses_qbuf:1;
 	unsigned			uses_requests:1;
 
-- 
2.17.1

[PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Dafna Hirschfeld]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Add capability to indicate that requests are required instead of
merely supported.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 4 ++++
 include/uapi/linux/videodev2.h                  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index d7faef10e39b..d42a3d9a7db3 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -125,6 +125,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-REQUIRES-REQUESTS:
 
 .. cssclass:: longtable
 
@@ -150,6 +151,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_REQUIRES_REQUESTS``
+      - 0x00000020
+      - This buffer type requires the use of :ref:`requests <media-request-api>`.
 
 Return Value
 ============
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 1db220da3bcc..97e6a6a968ba 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
-- 
2.17.1

[PATCH v5 03/23] cedrus: set requires_requests

[Dafna Hirschfeld]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

The cedrus stateless decoder requires the use of request, so
indicate this by setting requires_requests to 1.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/staging/media/sunxi/cedrus/cedrus_video.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index b47854b3bce4..9673874ece10 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -536,6 +536,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->requires_requests = true;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
-- 
2.17.1

[PATCH v5 04/23] media: vicodec: selection api should only check single buffer types

[Dafna Hirschfeld]

The selection api should check only single buffer types
because multiplanar types are converted to
single in drivers/media/v4l2-core/v4l2-ioctl.c

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 20 +++----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index d7636fe9e174..b92a91e06e18 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -945,16 +945,6 @@ static int vidioc_g_selection(struct file *file, void *priv,
 {
 	struct vicodec_ctx *ctx = file2ctx(file);
 	struct vicodec_q_data *q_data;
-	enum v4l2_buf_type valid_cap_type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	enum v4l2_buf_type valid_out_type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
-
-	if (multiplanar) {
-		valid_cap_type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
-		valid_out_type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
-	}
-
-	if (s->type != valid_cap_type && s->type != valid_out_type)
-		return -EINVAL;
 
 	q_data = get_q_data(ctx, s->type);
 	if (!q_data)
@@ -963,8 +953,8 @@ static int vidioc_g_selection(struct file *file, void *priv,
 	 * encoder supports only cropping on the OUTPUT buffer
 	 * decoder supports only composing on the CAPTURE buffer
 	 */
-	if ((ctx->is_enc && s->type == valid_out_type) ||
-	    (!ctx->is_enc && s->type == valid_cap_type)) {
+	if ((ctx->is_enc && s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) ||
+	    (!ctx->is_enc && s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)) {
 		switch (s->target) {
 		case V4L2_SEL_TGT_COMPOSE:
 		case V4L2_SEL_TGT_CROP:
@@ -992,12 +982,8 @@ static int vidioc_s_selection(struct file *file, void *priv,
 {
 	struct vicodec_ctx *ctx = file2ctx(file);
 	struct vicodec_q_data *q_data;
-	enum v4l2_buf_type out_type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
-
-	if (multiplanar)
-		out_type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
 
-	if (s->type != out_type)
+	if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
 	q_data = get_q_data(ctx, s->type);
-- 
2.17.1

[PATCH v5 05/23] media: vicodec: upon release, call m2m release before freeing ctrl handler

[Dafna Hirschfeld]

'v4l2_m2m_ctx_release' calls request complete
so it should be called before 'v4l2_ctrl_handler_free'.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index b92a91e06e18..0909f86547f1 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1606,12 +1606,12 @@ static int vicodec_release(struct file *file)
 	struct video_device *vfd = video_devdata(file);
 	struct vicodec_ctx *ctx = file2ctx(file);
 
-	v4l2_fh_del(&ctx->fh);
-	v4l2_fh_exit(&ctx->fh);
-	v4l2_ctrl_handler_free(&ctx->hdl);
 	mutex_lock(vfd->lock);
 	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 	mutex_unlock(vfd->lock);
+	v4l2_fh_del(&ctx->fh);
+	v4l2_fh_exit(&ctx->fh);
+	v4l2_ctrl_handler_free(&ctx->hdl);
 	kfree(ctx);
 
 	return 0;
-- 
2.17.1

[PATCH v5 06/23] media: v4l2-ctrl: v4l2_ctrl_request_setup returns with error upon failure

[Dafna Hirschfeld]

If one of the controls fails to set,
then 'v4l2_ctrl_request_setup'
immediately returns with the error code.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 18 +++++++++++-------
 include/media/v4l2-ctrls.h           |  2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index b79d3bbd8350..54d66dbc2a31 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3899,18 +3899,19 @@ void v4l2_ctrl_request_complete(struct media_request *req,
 }
 EXPORT_SYMBOL(v4l2_ctrl_request_complete);
 
-void v4l2_ctrl_request_setup(struct media_request *req,
+int v4l2_ctrl_request_setup(struct media_request *req,
 			     struct v4l2_ctrl_handler *main_hdl)
 {
 	struct media_request_object *obj;
 	struct v4l2_ctrl_handler *hdl;
 	struct v4l2_ctrl_ref *ref;
+	int ret = 0;
 
 	if (!req || !main_hdl)
-		return;
+		return 0;
 
 	if (WARN_ON(req->state != MEDIA_REQUEST_STATE_QUEUED))
-		return;
+		return -EBUSY;
 
 	/*
 	 * Note that it is valid if nothing was found. It means
@@ -3919,10 +3920,10 @@ void v4l2_ctrl_request_setup(struct media_request *req,
 	 */
 	obj = media_request_object_find(req, &req_ops, main_hdl);
 	if (!obj)
-		return;
+		return 0;
 	if (obj->completed) {
 		media_request_object_put(obj);
-		return;
+		return -EBUSY;
 	}
 	hdl = container_of(obj, struct v4l2_ctrl_handler, req_obj);
 
@@ -3990,12 +3991,15 @@ void v4l2_ctrl_request_setup(struct media_request *req,
 				update_from_auto_cluster(master);
 		}
 
-		try_or_set_cluster(NULL, master, true, 0);
-
+		ret = try_or_set_cluster(NULL, master, true, 0);
 		v4l2_ctrl_unlock(master);
+
+		if (ret)
+			break;
 	}
 
 	media_request_object_put(obj);
+	return ret;
 }
 EXPORT_SYMBOL(v4l2_ctrl_request_setup);
 
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index e5cae37ced2d..200f8a66ecaa 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -1127,7 +1127,7 @@ __poll_t v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait);
  * applying control values in a request is only applicable to memory-to-memory
  * devices.
  */
-void v4l2_ctrl_request_setup(struct media_request *req,
+int v4l2_ctrl_request_setup(struct media_request *req,
 			     struct v4l2_ctrl_handler *parent);
 
 /**
-- 
2.17.1

[PATCH v5 07/23] media: vicodec: change variable name for the return value of v4l2_fwht_encode

[Dafna Hirschfeld]

v4l2_fwht_encode returns either an error code on
failure or the size of the compressed frame on
success. So change the var assigned to it from
'ret' to 'comp_sz_or_errcode' to clarify that.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 0909f86547f1..eec31b144d56 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -178,13 +178,14 @@ static int device_process(struct vicodec_ctx *ctx,
 
 	if (ctx->is_enc) {
 		struct vicodec_q_data *q_src;
+		int comp_sz_or_errcode;
 
 		q_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
 		state->info = q_src->info;
-		ret = v4l2_fwht_encode(state, p_src, p_dst);
-		if (ret < 0)
-			return ret;
-		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, ret);
+		comp_sz_or_errcode = v4l2_fwht_encode(state, p_src, p_dst);
+		if (comp_sz_or_errcode < 0)
+			return comp_sz_or_errcode;
+		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, comp_sz_or_errcode);
 	} else {
 		unsigned int comp_frame_size = ntohl(ctx->state.header.size);
 
-- 
2.17.1

[PATCH v5 08/23] media: vicodec: bugfix - call v4l2_m2m_buf_copy_metadata also if decoding fails

[Dafna Hirschfeld]

The function 'v4l2_m2m_buf_copy_metadata' should
be called even if decoding/encoding ends with
status VB2_BUF_STATE_ERROR, so that the metadata
is copied from the source buffer to the dest buffer.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index eec31b144d56..8205a602bb38 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -159,12 +159,10 @@ static int device_process(struct vicodec_ctx *ctx,
 			  struct vb2_v4l2_buffer *dst_vb)
 {
 	struct vicodec_dev *dev = ctx->dev;
-	struct vicodec_q_data *q_dst;
 	struct v4l2_fwht_state *state = &ctx->state;
 	u8 *p_src, *p_dst;
 	int ret;
 
-	q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 	if (ctx->is_enc)
 		p_src = vb2_plane_vaddr(&src_vb->vb2_buf, 0);
 	else
@@ -187,8 +185,10 @@ static int device_process(struct vicodec_ctx *ctx,
 			return comp_sz_or_errcode;
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, comp_sz_or_errcode);
 	} else {
+		struct vicodec_q_data *q_dst;
 		unsigned int comp_frame_size = ntohl(ctx->state.header.size);
 
+		q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 		if (comp_frame_size > ctx->comp_max_size)
 			return -EINVAL;
 		state->info = q_dst->info;
@@ -197,11 +197,6 @@ static int device_process(struct vicodec_ctx *ctx,
 			return ret;
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
 	}
-
-	dst_vb->sequence = q_dst->sequence++;
-	dst_vb->flags &= ~V4L2_BUF_FLAG_LAST;
-	v4l2_m2m_buf_copy_metadata(src_vb, dst_vb, !ctx->is_enc);
-
 	return 0;
 }
 
@@ -275,16 +270,22 @@ static void device_run(void *priv)
 	struct vicodec_ctx *ctx = priv;
 	struct vicodec_dev *dev = ctx->dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
-	struct vicodec_q_data *q_src;
+	struct vicodec_q_data *q_src, *q_dst;
 	u32 state;
 
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 	q_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+	q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
 	state = VB2_BUF_STATE_DONE;
 	if (device_process(ctx, src_buf, dst_buf))
 		state = VB2_BUF_STATE_ERROR;
+	else
+		dst_buf->sequence = q_dst->sequence++;
+	dst_buf->flags &= ~V4L2_BUF_FLAG_LAST;
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, !ctx->is_enc);
+
 	ctx->last_dst_buf = dst_buf;
 
 	spin_lock(ctx->lock);
-- 
2.17.1

[PATCH v5 09/23] media: vicodec: bugfix: free compressed_frame upon device release

[Dafna Hirschfeld]

Free compressed_frame buffer upon device release.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 8205a602bb38..8128ea6d1948 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1614,6 +1614,7 @@ static int vicodec_release(struct file *file)
 	v4l2_fh_del(&ctx->fh);
 	v4l2_fh_exit(&ctx->fh);
 	v4l2_ctrl_handler_free(&ctx->hdl);
+	kvfree(ctx->state.compressed_frame);
 	kfree(ctx);
 
 	return 0;
-- 
2.17.1

[PATCH v5 10/23] media: vicodec: Move raw frame preparation code to a function

[Dafna Hirschfeld]

Introduce 'prepare_raw_frame' function that fills the values
of a raw frame struct according to the format.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 143 ++++++++++--------
 1 file changed, 78 insertions(+), 65 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index 6573a471c5ca..515b3115b3c6 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -75,117 +75,130 @@ const struct v4l2_fwht_pixfmt_info *v4l2_fwht_get_pixfmt(u32 idx)
 	return v4l2_fwht_pixfmts + idx;
 }
 
-int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
+static int prepare_raw_frame(struct fwht_raw_frame *rf,
+			 const struct v4l2_fwht_pixfmt_info *info, u8 *buf,
+			 unsigned int size)
 {
-	unsigned int size = state->stride * state->coded_height;
-	unsigned int chroma_stride = state->stride;
-	const struct v4l2_fwht_pixfmt_info *info = state->info;
-	struct fwht_cframe_hdr *p_hdr;
-	struct fwht_cframe cf;
-	struct fwht_raw_frame rf;
-	u32 encoding;
-	u32 flags = 0;
-
-	if (!info)
-		return -EINVAL;
-
-	rf.luma = p_in;
-	rf.width_div = info->width_div;
-	rf.height_div = info->height_div;
-	rf.luma_alpha_step = info->luma_alpha_step;
-	rf.chroma_step = info->chroma_step;
-	rf.alpha = NULL;
-	rf.components_num = info->components_num;
+	rf->luma = buf;
+	rf->width_div = info->width_div;
+	rf->height_div = info->height_div;
+	rf->luma_alpha_step = info->luma_alpha_step;
+	rf->chroma_step = info->chroma_step;
+	rf->alpha = NULL;
+	rf->components_num = info->components_num;
 
 	switch (info->id) {
 	case V4L2_PIX_FMT_GREY:
-		rf.cb = NULL;
-		rf.cr = NULL;
+		rf->cb = NULL;
+		rf->cr = NULL;
 		break;
 	case V4L2_PIX_FMT_YUV420:
-		rf.cb = rf.luma + size;
-		rf.cr = rf.cb + size / 4;
-		chroma_stride /= 2;
+		rf->cb = rf->luma + size;
+		rf->cr = rf->cb + size / 4;
 		break;
 	case V4L2_PIX_FMT_YVU420:
-		rf.cr = rf.luma + size;
-		rf.cb = rf.cr + size / 4;
-		chroma_stride /= 2;
+		rf->cr = rf->luma + size;
+		rf->cb = rf->cr + size / 4;
 		break;
 	case V4L2_PIX_FMT_YUV422P:
-		rf.cb = rf.luma + size;
-		rf.cr = rf.cb + size / 2;
-		chroma_stride /= 2;
+		rf->cb = rf->luma + size;
+		rf->cr = rf->cb + size / 2;
 		break;
 	case V4L2_PIX_FMT_NV12:
 	case V4L2_PIX_FMT_NV16:
 	case V4L2_PIX_FMT_NV24:
-		rf.cb = rf.luma + size;
-		rf.cr = rf.cb + 1;
+		rf->cb = rf->luma + size;
+		rf->cr = rf->cb + 1;
 		break;
 	case V4L2_PIX_FMT_NV21:
 	case V4L2_PIX_FMT_NV61:
 	case V4L2_PIX_FMT_NV42:
-		rf.cr = rf.luma + size;
-		rf.cb = rf.cr + 1;
+		rf->cr = rf->luma + size;
+		rf->cb = rf->cr + 1;
 		break;
 	case V4L2_PIX_FMT_YUYV:
-		rf.cb = rf.luma + 1;
-		rf.cr = rf.cb + 2;
+		rf->cb = rf->luma + 1;
+		rf->cr = rf->cb + 2;
 		break;
 	case V4L2_PIX_FMT_YVYU:
-		rf.cr = rf.luma + 1;
-		rf.cb = rf.cr + 2;
+		rf->cr = rf->luma + 1;
+		rf->cb = rf->cr + 2;
 		break;
 	case V4L2_PIX_FMT_UYVY:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_VYUY:
-		rf.cr = rf.luma;
-		rf.cb = rf.cr + 2;
-		rf.luma++;
+		rf->cr = rf->luma;
+		rf->cb = rf->cr + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_RGB24:
 	case V4L2_PIX_FMT_HSV24:
-		rf.cr = rf.luma;
-		rf.cb = rf.cr + 2;
-		rf.luma++;
+		rf->cr = rf->luma;
+		rf->cb = rf->cr + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_BGR24:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_RGB32:
 	case V4L2_PIX_FMT_XRGB32:
 	case V4L2_PIX_FMT_HSV32:
-		rf.cr = rf.luma + 1;
-		rf.cb = rf.cr + 2;
-		rf.luma += 2;
+		rf->cr = rf->luma + 1;
+		rf->cb = rf->cr + 2;
+		rf->luma += 2;
 		break;
 	case V4L2_PIX_FMT_BGR32:
 	case V4L2_PIX_FMT_XBGR32:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_ARGB32:
-		rf.alpha = rf.luma;
-		rf.cr = rf.luma + 1;
-		rf.cb = rf.cr + 2;
-		rf.luma += 2;
+		rf->alpha = rf->luma;
+		rf->cr = rf->luma + 1;
+		rf->cb = rf->cr + 2;
+		rf->luma += 2;
 		break;
 	case V4L2_PIX_FMT_ABGR32:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
-		rf.alpha = rf.cr + 1;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
+		rf->alpha = rf->cr + 1;
 		break;
 	default:
 		return -EINVAL;
 	}
+	return 0;
+}
+
+int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
+{
+	unsigned int size = state->stride * state->coded_height;
+	unsigned int chroma_stride = state->stride;
+	const struct v4l2_fwht_pixfmt_info *info = state->info;
+	struct fwht_cframe_hdr *p_hdr;
+	struct fwht_cframe cf;
+	struct fwht_raw_frame rf;
+	u32 encoding;
+	u32 flags = 0;
+
+	if (!info)
+		return -EINVAL;
+
+	if (prepare_raw_frame(&rf, info, p_in, size))
+		return -EINVAL;
+
+	if (info->planes_num == 3)
+		chroma_stride /= 2;
+
+	if (info->id == V4L2_PIX_FMT_NV24 ||
+	    info->id == V4L2_PIX_FMT_NV42)
+		chroma_stride *= 2;
 
 	cf.i_frame_qp = state->i_frame_qp;
 	cf.p_frame_qp = state->p_frame_qp;
-- 
2.17.1

[PATCH v5 11/23] media: vicodec: add field 'buf' to fwht_raw_frame

[Dafna Hirschfeld]

Add the field 'buf' to fwht_raw_frame to indicate
the start of the raw frame buffer.
This field will be used to copy the capture buffer
to the reference buffer in the next patch.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/codec-fwht.h   | 1 +
 drivers/media/platform/vicodec/vicodec-core.c | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
index c410512d47c5..8f0b790839f8 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -124,6 +124,7 @@ struct fwht_raw_frame {
 	unsigned int luma_alpha_step;
 	unsigned int chroma_step;
 	unsigned int components_num;
+	u8 *buf;
 	u8 *luma, *cb, *cr, *alpha;
 };
 
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 8128ea6d1948..42af0e922249 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1352,7 +1352,8 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	state->stride = q_data->coded_width *
 				info->bytesperline_mult;
 
-	state->ref_frame.luma = kvmalloc(total_planes_size, GFP_KERNEL);
+	state->ref_frame.buf = kvmalloc(total_planes_size, GFP_KERNEL);
+	state->ref_frame.luma = state->ref_frame.buf;
 	ctx->comp_max_size = total_planes_size;
 	new_comp_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
 
@@ -1401,7 +1402,9 @@ static void vicodec_stop_streaming(struct vb2_queue *q)
 
 	if ((!V4L2_TYPE_IS_OUTPUT(q->type) && !ctx->is_enc) ||
 	    (V4L2_TYPE_IS_OUTPUT(q->type) && ctx->is_enc)) {
-		kvfree(ctx->state.ref_frame.luma);
+		kvfree(ctx->state.ref_frame.buf);
+		ctx->state.ref_frame.buf = NULL;
+		ctx->state.ref_frame.luma = NULL;
 		ctx->comp_max_size = 0;
 		ctx->source_changed = false;
 	}
-- 
2.17.1

[PATCH v5 12/23] media: vicodec: keep the ref frame according to the format in decoder

[Dafna Hirschfeld]

In the decoder, save the inner reference frame in the same
format as the capture buffer.
The decoder writes directly to the capture buffer and then
the capture buffer is copied to the reference buffer.
This will simplify the stateless decoder.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/codec-fwht.c   |  83 +++---
 drivers/media/platform/vicodec/codec-fwht.h   |  11 +-
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 255 ++----------------
 .../media/platform/vicodec/codec-v4l2-fwht.h  |   1 +
 drivers/media/platform/vicodec/vicodec-core.c |  40 +++
 5 files changed, 122 insertions(+), 268 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.c b/drivers/media/platform/vicodec/codec-fwht.c
index d1d6085da9f1..9a0dc739c58c 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -632,12 +632,13 @@ static int decide_blocktype(const u8 *cur, const u8 *reference,
 	return vari <= vard ? IBLOCK : PBLOCK;
 }
 
-static void fill_decoder_block(u8 *dst, const s16 *input, int stride)
+static void fill_decoder_block(u8 *dst, const s16 *input, int stride,
+			       unsigned int dst_step)
 {
 	int i, j;
 
 	for (i = 0; i < 8; i++) {
-		for (j = 0; j < 8; j++, input++, dst++) {
+		for (j = 0; j < 8; j++, input++, dst += dst_step) {
 			if (*input < 0)
 				*dst = 0;
 			else if (*input > 255)
@@ -645,17 +646,19 @@ static void fill_decoder_block(u8 *dst, const s16 *input, int stride)
 			else
 				*dst = *input;
 		}
-		dst += stride - 8;
+		dst += stride - (8 * dst_step);
 	}
 }
 
-static void add_deltas(s16 *deltas, const u8 *ref, int stride)
+static void add_deltas(s16 *deltas, const u8 *ref, int stride,
+		       unsigned int ref_step)
 {
 	int k, l;
 
 	for (k = 0; k < 8; k++) {
 		for (l = 0; l < 8; l++) {
-			*deltas += *ref++;
+			*deltas += *ref;
+			ref += ref_step;
 			/*
 			 * Due to quantizing, it might possible that the
 			 * decoded coefficients are slightly out of range
@@ -666,7 +669,7 @@ static void add_deltas(s16 *deltas, const u8 *ref, int stride)
 				*deltas = 255;
 			deltas++;
 		}
-		ref += stride - 8;
+		ref += stride - (8 * ref_step);
 	}
 }
 
@@ -711,8 +714,8 @@ static u32 encode_plane(u8 *input, u8 *refp, __be16 **rlco, __be16 *rlco_max,
 				ifwht(cf->de_coeffs, cf->de_fwht, blocktype);
 
 				if (blocktype == PBLOCK)
-					add_deltas(cf->de_fwht, refp, 8);
-				fill_decoder_block(refp, cf->de_fwht, 8);
+					add_deltas(cf->de_fwht, refp, 8, 1);
+				fill_decoder_block(refp, cf->de_fwht, 8, 1);
 			}
 
 			input += 8 * input_step;
@@ -821,8 +824,10 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
 	return encoding;
 }
 
-static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
-			 u32 height, u32 width, u32 coded_width,
+static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco,
+			 u32 height, u32 width, const u8 *ref, u32 ref_stride,
+			 unsigned int ref_step, u8 *dst,
+			 unsigned int dst_stride, unsigned int dst_step,
 			 bool uncompressed, const __be16 *end_of_rlco_buf)
 {
 	unsigned int copies = 0;
@@ -834,10 +839,15 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
 	height = round_up(height, 8);
 
 	if (uncompressed) {
+		int i;
+
 		if (end_of_rlco_buf + 1 < *rlco + width * height / 2)
 			return false;
-		memcpy(ref, *rlco, width * height);
-		*rlco += width * height / 2;
+		for (i = 0; i < height; i++) {
+			memcpy(dst, *rlco, width);
+			dst += dst_stride;
+			*rlco += width / 2;
+		}
 		return true;
 	}
 
@@ -849,15 +859,17 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
 	 */
 	for (j = 0; j < height / 8; j++) {
 		for (i = 0; i < width / 8; i++) {
-			u8 *refp = ref + j * 8 * coded_width + i * 8;
+			const u8 *refp = ref + j * 8 * ref_stride +
+				i * 8 * ref_step;
+			u8 *dstp = dst + j * 8 * dst_stride + i * 8 * dst_step;
 
 			if (copies) {
 				memcpy(cf->de_fwht, copy, sizeof(copy));
 				if (stat & PFRAME_BIT)
 					add_deltas(cf->de_fwht, refp,
-						   coded_width);
-				fill_decoder_block(refp, cf->de_fwht,
-						   coded_width);
+						   ref_stride, ref_step);
+				fill_decoder_block(dstp, cf->de_fwht,
+						   dst_stride, dst_step);
 				copies--;
 				continue;
 			}
@@ -877,23 +889,29 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
 			if (copies)
 				memcpy(copy, cf->de_fwht, sizeof(copy));
 			if (stat & PFRAME_BIT)
-				add_deltas(cf->de_fwht, refp, coded_width);
-			fill_decoder_block(refp, cf->de_fwht, coded_width);
+				add_deltas(cf->de_fwht, refp,
+					   ref_stride, ref_step);
+			fill_decoder_block(dstp, cf->de_fwht, dst_stride,
+					   dst_step);
 		}
 	}
 	return true;
 }
 
-bool fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
-		       u32 hdr_flags, unsigned int components_num,
-		       unsigned int width, unsigned int height,
-		       unsigned int coded_width)
+bool fwht_decode_frame(struct fwht_cframe *cf, u32 hdr_flags,
+		       unsigned int components_num, unsigned int width,
+		       unsigned int height, const struct fwht_raw_frame *ref,
+		       unsigned int ref_stride, unsigned int ref_chroma_stride,
+		       struct fwht_raw_frame *dst, unsigned int dst_stride,
+		       unsigned int dst_chroma_stride)
 {
 	const __be16 *rlco = cf->rlc_data;
 	const __be16 *end_of_rlco_buf = cf->rlc_data +
 			(cf->size / sizeof(*rlco)) - 1;
 
-	if (!decode_plane(cf, &rlco, ref->luma, height, width, coded_width,
+	if (!decode_plane(cf, &rlco, height, width, ref->luma, ref_stride,
+			  ref->luma_alpha_step, dst->luma, dst_stride,
+			  dst->luma_alpha_step,
 			  hdr_flags & FWHT_FL_LUMA_IS_UNCOMPRESSED,
 			  end_of_rlco_buf))
 		return false;
@@ -901,27 +919,30 @@ bool fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
 	if (components_num >= 3) {
 		u32 h = height;
 		u32 w = width;
-		u32 c = coded_width;
 
 		if (!(hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT))
 			h /= 2;
-		if (!(hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH)) {
+		if (!(hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH))
 			w /= 2;
-			c /= 2;
-		}
-		if (!decode_plane(cf, &rlco, ref->cb, h, w, c,
+
+		if (!decode_plane(cf, &rlco, h, w, ref->cb, ref_chroma_stride,
+				  ref->chroma_step, dst->cb, dst_chroma_stride,
+				  dst->chroma_step,
 				  hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED,
 				  end_of_rlco_buf))
 			return false;
-		if (!decode_plane(cf, &rlco, ref->cr, h, w, c,
+		if (!decode_plane(cf, &rlco, h, w, ref->cr, ref_chroma_stride,
+				  ref->chroma_step, dst->cr, dst_chroma_stride,
+				  dst->chroma_step,
 				  hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED,
 				  end_of_rlco_buf))
 			return false;
 	}
 
 	if (components_num == 4)
-		if (!decode_plane(cf, &rlco, ref->alpha, height, width,
-				  coded_width,
+		if (!decode_plane(cf, &rlco, height, width, ref->alpha, ref_stride,
+				  ref->luma_alpha_step, dst->alpha, dst_stride,
+				  dst->luma_alpha_step,
 				  hdr_flags & FWHT_FL_ALPHA_IS_UNCOMPRESSED,
 				  end_of_rlco_buf))
 			return false;
diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
index 8f0b790839f8..b6fec2b1cbca 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -141,9 +141,10 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
 		      bool is_intra, bool next_is_intra,
 		      unsigned int width, unsigned int height,
 		      unsigned int stride, unsigned int chroma_stride);
-bool fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
-		       u32 hdr_flags, unsigned int components_num,
-		       unsigned int width, unsigned int height,
-		       unsigned int coded_width);
-
+bool fwht_decode_frame(struct fwht_cframe *cf, u32 hdr_flags,
+		unsigned int components_num, unsigned int width,
+		unsigned int height, const struct fwht_raw_frame *ref,
+		unsigned int ref_stride, unsigned int ref_chroma_stride,
+		struct fwht_raw_frame *dst, unsigned int dst_stride,
+		unsigned int dst_chroma_stride);
 #endif
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index 515b3115b3c6..f15d76fae45c 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -248,14 +248,17 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
 
 int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
 {
-	unsigned int i, j, k;
 	u32 flags;
 	struct fwht_cframe cf;
-	u8 *p, *ref_p;
 	unsigned int components_num = 3;
 	unsigned int version;
 	const struct v4l2_fwht_pixfmt_info *info;
 	unsigned int hdr_width_div, hdr_height_div;
+	struct fwht_raw_frame dst_rf;
+	unsigned int dst_chroma_stride = state->stride;
+	unsigned int ref_chroma_stride = state->ref_stride;
+	unsigned int dst_size = state->stride * state->coded_height;
+	unsigned int ref_size;
 
 	if (!state->info)
 		return -EINVAL;
@@ -303,241 +306,29 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
 	    hdr_height_div != info->height_div)
 		return -EINVAL;
 
-	if (!fwht_decode_frame(&cf, &state->ref_frame, flags, components_num,
-			       state->visible_width, state->visible_height,
-			       state->coded_width))
+	if (prepare_raw_frame(&dst_rf, info, p_out, dst_size))
 		return -EINVAL;
+	if (info->planes_num == 3) {
+		dst_chroma_stride /= 2;
+		ref_chroma_stride /= 2;
+	}
+	if (info->id == V4L2_PIX_FMT_NV24 ||
+	    info->id == V4L2_PIX_FMT_NV42) {
+		dst_chroma_stride *= 2;
+		ref_chroma_stride *= 2;
+	}
 
-	/*
-	 * TODO - handle the case where the compressed stream encodes a
-	 * different format than the requested decoded format.
-	 */
-	switch (state->info->id) {
-	case V4L2_PIX_FMT_GREY:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
-		break;
-	case V4L2_PIX_FMT_YUV420:
-	case V4L2_PIX_FMT_YUV422P:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
-
-		ref_p = state->ref_frame.cb;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		ref_p = state->ref_frame.cr;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		break;
-	case V4L2_PIX_FMT_YVU420:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
 
-		ref_p = state->ref_frame.cr;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		ref_p = state->ref_frame.cb;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		break;
-	case V4L2_PIX_FMT_NV12:
-	case V4L2_PIX_FMT_NV16:
-	case V4L2_PIX_FMT_NV24:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
+	ref_size = state->ref_stride * state->coded_height;
 
-		k = 0;
-		for (i = 0; i < state->coded_height / 2; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.cr[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_NV21:
-	case V4L2_PIX_FMT_NV61:
-	case V4L2_PIX_FMT_NV42:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
+	if (prepare_raw_frame(&state->ref_frame, info, state->ref_frame.buf,
+			      ref_size))
+		return -EINVAL;
 
-		k = 0;
-		for (i = 0; i < state->coded_height / 2; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_YUYV:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				*p++ = state->ref_frame.cr[k / 2];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_YVYU:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				*p++ = state->ref_frame.cb[k / 2];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_UYVY:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cb[k / 2];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_VYUY:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cr[k / 2];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_RGB24:
-	case V4L2_PIX_FMT_HSV24:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_BGR24:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_RGB32:
-	case V4L2_PIX_FMT_XRGB32:
-	case V4L2_PIX_FMT_HSV32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = 0;
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_BGR32:
-	case V4L2_PIX_FMT_XBGR32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k];
-				*p++ = 0;
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_ARGB32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.alpha[k];
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_ABGR32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.alpha[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	default:
+	if (!fwht_decode_frame(&cf, flags, components_num,
+			state->visible_width, state->visible_height,
+			&state->ref_frame, state->ref_stride, ref_chroma_stride,
+			&dst_rf, state->stride, dst_chroma_stride))
 		return -EINVAL;
-	}
 	return 0;
 }
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
index aa6fa90a48be..53eba97ebc83 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
@@ -30,6 +30,7 @@ struct v4l2_fwht_state {
 	unsigned int coded_width;
 	unsigned int coded_height;
 	unsigned int stride;
+	unsigned int ref_stride;
 	unsigned int gop_size;
 	unsigned int gop_cnt;
 	u16 i_frame_qp;
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 42af0e922249..4b97ba30fec3 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -154,6 +154,43 @@ static struct vicodec_q_data *get_q_data(struct vicodec_ctx *ctx,
 	return NULL;
 }
 
+static void copy_cap_to_ref(const u8 *cap, const struct v4l2_fwht_pixfmt_info *info,
+		struct v4l2_fwht_state *state)
+{
+	int plane_idx;
+	u8 *p_ref = state->ref_frame.buf;
+	unsigned int cap_stride = state->stride;
+	unsigned int ref_stride = state->ref_stride;
+
+	for (plane_idx = 0; plane_idx < info->planes_num; plane_idx++) {
+		int i;
+		unsigned int h_div = (plane_idx == 1 || plane_idx == 2) ?
+			info->height_div : 1;
+		const u8 *row_cap = cap;
+		u8 *row_ref = p_ref;
+
+		if (info->planes_num == 3 && plane_idx == 1) {
+			cap_stride /= 2;
+			ref_stride /= 2;
+		}
+
+		if (plane_idx == 1 &&
+		    (info->id == V4L2_PIX_FMT_NV24 ||
+		     info->id == V4L2_PIX_FMT_NV42)) {
+			cap_stride *= 2;
+			ref_stride *= 2;
+		}
+
+		for (i = 0; i < state->visible_height / h_div; i++) {
+			memcpy(row_ref, row_cap, ref_stride);
+			row_ref += ref_stride;
+			row_cap += cap_stride;
+		}
+		cap += cap_stride * (state->coded_height / h_div);
+		p_ref += ref_stride * (state->coded_height / h_div);
+	}
+}
+
 static int device_process(struct vicodec_ctx *ctx,
 			  struct vb2_v4l2_buffer *src_vb,
 			  struct vb2_v4l2_buffer *dst_vb)
@@ -195,6 +232,8 @@ static int device_process(struct vicodec_ctx *ctx,
 		ret = v4l2_fwht_decode(state, p_src, p_dst);
 		if (ret < 0)
 			return ret;
+		copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
+
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
 	}
 	return 0;
@@ -1352,6 +1391,7 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	state->stride = q_data->coded_width *
 				info->bytesperline_mult;
 
+	state->ref_stride = q_data->coded_width * info->luma_alpha_step;
 	state->ref_frame.buf = kvmalloc(total_planes_size, GFP_KERNEL);
 	state->ref_frame.luma = state->ref_frame.buf;
 	ctx->comp_max_size = total_planes_size;
-- 
2.17.1

[PATCH v5 13/23] media: vicodec: Validate version dependent header values in a separate function

[Dafna Hirschfeld]

Move the code that validates version dependent header
values to a separate function 'validate_by_version'

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 31 ++++++++++++-------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 4b97ba30fec3..d051f9901409 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -191,6 +191,23 @@ static void copy_cap_to_ref(const u8 *cap, const struct v4l2_fwht_pixfmt_info *i
 	}
 }
 
+static bool validate_by_version(unsigned int flags, unsigned int version)
+{
+	if (!version || version > FWHT_VERSION)
+		return false;
+
+	if (version >= 2) {
+		unsigned int components_num = 1 +
+			((flags & FWHT_FL_COMPONENTS_NUM_MSK) >>
+			 FWHT_FL_COMPONENTS_NUM_OFFSET);
+		unsigned int pixenc = flags & FWHT_FL_PIXENC_MSK;
+
+		if (components_num == 0 || components_num > 4 || !pixenc)
+			return false;
+	}
+	return true;
+}
+
 static int device_process(struct vicodec_ctx *ctx,
 			  struct vb2_v4l2_buffer *src_vb,
 			  struct vb2_v4l2_buffer *dst_vb)
@@ -397,21 +414,11 @@ static bool is_header_valid(const struct fwht_cframe_hdr *p_hdr)
 	unsigned int version = ntohl(p_hdr->version);
 	unsigned int flags = ntohl(p_hdr->flags);
 
-	if (!version || version > FWHT_VERSION)
-		return false;
-
 	if (w < MIN_WIDTH || w > MAX_WIDTH || h < MIN_HEIGHT || h > MAX_HEIGHT)
 		return false;
 
-	if (version >= 2) {
-		unsigned int components_num = 1 +
-			((flags & FWHT_FL_COMPONENTS_NUM_MSK) >>
-			FWHT_FL_COMPONENTS_NUM_OFFSET);
-		unsigned int pixenc = flags & FWHT_FL_PIXENC_MSK;
-
-		if (components_num == 0 || components_num > 4 || !pixenc)
-			return false;
-	}
+	if (!validate_by_version(flags, version))
+		return false;
 
 	info = info_from_header(p_hdr);
 	if (!info)
-- 
2.17.1

[PATCH v5 14/23] media: vicodec: rename v4l2_fwht_default_fmt to v4l2_fwht_find_nth_fmt

[Dafna Hirschfeld]

Rename 'v4l2_fwht_default_fmt' to 'v4l2_fwht_find_nth_fmt'
and add a function 'v4l2_fwht_validate_fmt' to check if
a format info matches the parameters.
This function will also be used to validate the stateless
params when adding support for stateless codecs.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 22 ++++++++++++++-----
 .../media/platform/vicodec/codec-v4l2-fwht.h  |  5 ++++-
 drivers/media/platform/vicodec/vicodec-core.c |  4 ++--
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index f15d76fae45c..372ed95e1a1f 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -37,7 +37,19 @@ static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
 	{ V4L2_PIX_FMT_GREY,    1, 1, 1, 1, 0, 1, 1, 1, 1, FWHT_FL_PIXENC_RGB},
 };
 
-const struct v4l2_fwht_pixfmt_info *v4l2_fwht_default_fmt(u32 width_div,
+bool v4l2_fwht_validate_fmt(const struct v4l2_fwht_pixfmt_info *info,
+			    u32 width_div, u32 height_div, u32 components_num,
+			    u32 pixenc)
+{
+	if (info->width_div == width_div &&
+	    info->height_div == height_div &&
+	    (!pixenc || info->pixenc == pixenc) &&
+	    info->components_num == components_num)
+		return true;
+	return false;
+}
+
+const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_nth_fmt(u32 width_div,
 							  u32 height_div,
 							  u32 components_num,
 							  u32 pixenc,
@@ -46,10 +58,10 @@ const struct v4l2_fwht_pixfmt_info *v4l2_fwht_default_fmt(u32 width_div,
 	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(v4l2_fwht_pixfmts); i++) {
-		if (v4l2_fwht_pixfmts[i].width_div == width_div &&
-		    v4l2_fwht_pixfmts[i].height_div == height_div &&
-		    (!pixenc || v4l2_fwht_pixfmts[i].pixenc == pixenc) &&
-		    v4l2_fwht_pixfmts[i].components_num == components_num) {
+		bool is_valid = v4l2_fwht_validate_fmt(&v4l2_fwht_pixfmts[i],
+						       width_div, height_div,
+						       components_num, pixenc);
+		if (is_valid) {
 			if (start_idx == 0)
 				return v4l2_fwht_pixfmts + i;
 			start_idx--;
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
index 53eba97ebc83..b59503d4049a 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
@@ -48,7 +48,10 @@ struct v4l2_fwht_state {
 
 const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat);
 const struct v4l2_fwht_pixfmt_info *v4l2_fwht_get_pixfmt(u32 idx);
-const struct v4l2_fwht_pixfmt_info *v4l2_fwht_default_fmt(u32 width_div,
+bool v4l2_fwht_validate_fmt(const struct v4l2_fwht_pixfmt_info *info,
+			    u32 width_div, u32 height_div, u32 components_num,
+			    u32 pixenc);
+const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_nth_fmt(u32 width_div,
 							  u32 height_div,
 							  u32 components_num,
 							  u32 pixenc,
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index d051f9901409..15dfdd99be3a 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -402,7 +402,7 @@ info_from_header(const struct fwht_cframe_hdr *p_hdr)
 				FWHT_FL_COMPONENTS_NUM_OFFSET);
 		pixenc = (flags & FWHT_FL_PIXENC_MSK);
 	}
-	return v4l2_fwht_default_fmt(width_div, height_div,
+	return v4l2_fwht_find_nth_fmt(width_div, height_div,
 				     components_num, pixenc, 0);
 }
 
@@ -623,7 +623,7 @@ static int enum_fmt(struct v4l2_fmtdesc *f, struct vicodec_ctx *ctx,
 		if (!info || ctx->is_enc)
 			info = v4l2_fwht_get_pixfmt(f->index);
 		else
-			info = v4l2_fwht_default_fmt(info->width_div,
+			info = v4l2_fwht_find_nth_fmt(info->width_div,
 						     info->height_div,
 						     info->components_num,
 						     info->pixenc,
-- 
2.17.1

[PATCH v5 15/23] media: vicodec: Handle the case that the reference buffer is NULL

[Dafna Hirschfeld]

In the stateless decoder the reference buffer is null if the
frame is an I-frame (flagged with FWHT_FL_I_FRAME).
Make sure not to dereference it in that case.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/codec-fwht.c      |  9 +++++----
 drivers/media/platform/vicodec/codec-v4l2-fwht.c | 11 +++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.c b/drivers/media/platform/vicodec/codec-fwht.c
index 9a0dc739c58c..25992817f45b 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -834,6 +834,7 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco,
 	s16 copy[8 * 8];
 	u16 stat;
 	unsigned int i, j;
+	bool is_intra = !ref;
 
 	width = round_up(width, 8);
 	height = round_up(height, 8);
@@ -865,7 +866,7 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco,
 
 			if (copies) {
 				memcpy(cf->de_fwht, copy, sizeof(copy));
-				if (stat & PFRAME_BIT)
+				if ((stat & PFRAME_BIT) && !is_intra)
 					add_deltas(cf->de_fwht, refp,
 						   ref_stride, ref_step);
 				fill_decoder_block(dstp, cf->de_fwht,
@@ -877,18 +878,18 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco,
 			stat = derlc(rlco, cf->coeffs, end_of_rlco_buf);
 			if (stat & OVERFLOW_BIT)
 				return false;
-			if (stat & PFRAME_BIT)
+			if ((stat & PFRAME_BIT) && !is_intra)
 				dequantize_inter(cf->coeffs);
 			else
 				dequantize_intra(cf->coeffs);
 
 			ifwht(cf->coeffs, cf->de_fwht,
-			      (stat & PFRAME_BIT) ? 0 : 1);
+			      ((stat & PFRAME_BIT) && !is_intra) ? 0 : 1);
 
 			copies = (stat & DUPS_MASK) >> 1;
 			if (copies)
 				memcpy(copy, cf->de_fwht, sizeof(copy));
-			if (stat & PFRAME_BIT)
+			if ((stat & PFRAME_BIT) && !is_intra)
 				add_deltas(cf->de_fwht, refp,
 					   ref_stride, ref_step);
 			fill_decoder_block(dstp, cf->de_fwht, dst_stride,
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index 372ed95e1a1f..01e7f09efc4e 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -99,6 +99,17 @@ static int prepare_raw_frame(struct fwht_raw_frame *rf,
 	rf->alpha = NULL;
 	rf->components_num = info->components_num;
 
+	/*
+	 * The buffer is NULL if it is the reference
+	 * frame of an I-frame in the stateless decoder
+	 */
+	if (!buf) {
+		rf->luma = NULL;
+		rf->cb = NULL;
+		rf->cr = NULL;
+		rf->alpha = NULL;
+		return 0;
+	}
 	switch (info->id) {
 	case V4L2_PIX_FMT_GREY:
 		rf->cb = NULL;
-- 
2.17.1

[PATCH v5 16/23] media: vicodec: add struct for encoder/decoder instance

[Dafna Hirschfeld]

Add struct 'vicodec_dev_instance' for the fields in vicodec_dev
that have have both decoder and encoder versions.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 194 +++++++++---------
 1 file changed, 92 insertions(+), 102 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 15dfdd99be3a..5998b9e86cda 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -89,21 +89,21 @@ enum {
 	V4L2_M2M_DST = 1,
 };
 
+struct vicodec_dev_instance {
+	struct video_device     vfd;
+	struct mutex            mutex;
+	spinlock_t              lock;
+	struct v4l2_m2m_dev     *m2m_dev;
+};
+
 struct vicodec_dev {
 	struct v4l2_device	v4l2_dev;
-	struct video_device	enc_vfd;
-	struct video_device	dec_vfd;
+	struct vicodec_dev_instance stateful_enc;
+	struct vicodec_dev_instance stateful_dec;
 #ifdef CONFIG_MEDIA_CONTROLLER
 	struct media_device	mdev;
 #endif
 
-	struct mutex		enc_mutex;
-	struct mutex		dec_mutex;
-	spinlock_t		enc_lock;
-	spinlock_t		dec_lock;
-
-	struct v4l2_m2m_dev	*enc_dev;
-	struct v4l2_m2m_dev	*dec_dev;
 };
 
 struct vicodec_ctx {
@@ -368,9 +368,9 @@ static void device_run(void *priv)
 	spin_unlock(ctx->lock);
 
 	if (ctx->is_enc)
-		v4l2_m2m_job_finish(dev->enc_dev, ctx->fh.m2m_ctx);
+		v4l2_m2m_job_finish(dev->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
 	else
-		v4l2_m2m_job_finish(dev->dec_dev, ctx->fh.m2m_ctx);
+		v4l2_m2m_job_finish(dev->stateful_dec.m2m_dev, ctx->fh.m2m_ctx);
 }
 
 static void job_remove_src_buf(struct vicodec_ctx *ctx, u32 state)
@@ -1490,9 +1490,8 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->ops = &vicodec_qops;
 	src_vq->mem_ops = &vb2_vmalloc_memops;
 	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->lock = ctx->is_enc ? &ctx->dev->stateful_enc.mutex :
+		&ctx->dev->stateful_dec.mutex;
 	ret = vb2_queue_init(src_vq);
 	if (ret)
 		return ret;
@@ -1580,7 +1579,7 @@ static int vicodec_open(struct file *file)
 		goto open_unlock;
 	}
 
-	if (vfd == &dev->enc_vfd)
+	if (vfd == &dev->stateful_enc.vfd)
 		ctx->is_enc = true;
 
 	v4l2_fh_init(&ctx->fh, video_devdata(file));
@@ -1628,13 +1627,13 @@ static int vicodec_open(struct file *file)
 	ctx->state.colorspace = V4L2_COLORSPACE_REC709;
 
 	if (ctx->is_enc) {
-		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->enc_dev, ctx,
-						    &queue_init);
-		ctx->lock = &dev->enc_lock;
+		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_enc.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->stateful_enc.lock;
 	} else {
-		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->dec_dev, ctx,
-						    &queue_init);
-		ctx->lock = &dev->dec_lock;
+		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_dec.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->stateful_dec.lock;
 	}
 
 	if (IS_ERR(ctx->fh.m2m_ctx)) {
@@ -1693,19 +1692,57 @@ static const struct v4l2_m2m_ops m2m_ops = {
 	.job_ready	= job_ready,
 };
 
+static int register_instance(struct vicodec_dev *dev,
+			     struct vicodec_dev_instance *dev_instance,
+			     const char *name, bool is_enc)
+{
+	struct video_device *vfd;
+	int ret;
+
+	spin_lock_init(&dev_instance->lock);
+	mutex_init(&dev_instance->mutex);
+	dev_instance->m2m_dev = v4l2_m2m_init(&m2m_ops);
+	if (IS_ERR(dev_instance->m2m_dev)) {
+		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec enc device\n");
+		return PTR_ERR(dev_instance->m2m_dev);
+	}
+
+	dev_instance->vfd = vicodec_videodev;
+	vfd = &dev_instance->vfd;
+	vfd->lock = &dev_instance->mutex;
+	vfd->v4l2_dev = &dev->v4l2_dev;
+	strscpy(vfd->name, name, sizeof(vfd->name));
+	vfd->device_caps = V4L2_CAP_STREAMING |
+		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
+	if (is_enc) {
+		v4l2_disable_ioctl(vfd, VIDIOC_DECODER_CMD);
+		v4l2_disable_ioctl(vfd, VIDIOC_TRY_DECODER_CMD);
+	} else {
+		v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
+		v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
+	}
+	video_set_drvdata(vfd, dev);
+
+	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
+	if (ret) {
+		v4l2_err(&dev->v4l2_dev, "Failed to register video device '%s'\n", name);
+		v4l2_m2m_release(dev_instance->m2m_dev);
+		return ret;
+	}
+	v4l2_info(&dev->v4l2_dev, "Device '%s' registered as /dev/video%d\n",
+		  name, vfd->num);
+	return 0;
+}
+
 static int vicodec_probe(struct platform_device *pdev)
 {
 	struct vicodec_dev *dev;
-	struct video_device *vfd;
 	int ret;
 
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
-	spin_lock_init(&dev->enc_lock);
-	spin_lock_init(&dev->dec_lock);
-
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
 	if (ret)
 		return ret;
@@ -1719,100 +1756,53 @@ static int vicodec_probe(struct platform_device *pdev)
 	dev->v4l2_dev.mdev = &dev->mdev;
 #endif
 
-	mutex_init(&dev->enc_mutex);
-	mutex_init(&dev->dec_mutex);
-
 	platform_set_drvdata(pdev, dev);
 
-	dev->enc_dev = v4l2_m2m_init(&m2m_ops);
-	if (IS_ERR(dev->enc_dev)) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec device\n");
-		ret = PTR_ERR(dev->enc_dev);
+	if (register_instance(dev, &dev->stateful_enc,
+			      "stateful-encoder", true))
 		goto unreg_dev;
-	}
-
-	dev->dec_dev = v4l2_m2m_init(&m2m_ops);
-	if (IS_ERR(dev->dec_dev)) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec device\n");
-		ret = PTR_ERR(dev->dec_dev);
-		goto err_enc_m2m;
-	}
 
-	dev->enc_vfd = vicodec_videodev;
-	vfd = &dev->enc_vfd;
-	vfd->lock = &dev->enc_mutex;
-	vfd->v4l2_dev = &dev->v4l2_dev;
-	strscpy(vfd->name, "vicodec-enc", sizeof(vfd->name));
-	vfd->device_caps = V4L2_CAP_STREAMING |
-		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
-	v4l2_disable_ioctl(vfd, VIDIOC_DECODER_CMD);
-	v4l2_disable_ioctl(vfd, VIDIOC_TRY_DECODER_CMD);
-	video_set_drvdata(vfd, dev);
-
-	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
-	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
-		goto err_dec_m2m;
-	}
-	v4l2_info(&dev->v4l2_dev,
-			"Device registered as /dev/video%d\n", vfd->num);
-
-	dev->dec_vfd = vicodec_videodev;
-	vfd = &dev->dec_vfd;
-	vfd->lock = &dev->dec_mutex;
-	vfd->v4l2_dev = &dev->v4l2_dev;
-	vfd->device_caps = V4L2_CAP_STREAMING |
-		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
-	strscpy(vfd->name, "vicodec-dec", sizeof(vfd->name));
-	v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
-	v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
-	video_set_drvdata(vfd, dev);
-
-	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
-	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
-		goto unreg_enc;
-	}
-	v4l2_info(&dev->v4l2_dev,
-			"Device registered as /dev/video%d\n", vfd->num);
+	if (register_instance(dev, &dev->stateful_dec,
+			      "stateful-decoder", false))
+		goto unreg_sf_enc;
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-	ret = v4l2_m2m_register_media_controller(dev->enc_dev,
-			&dev->enc_vfd, MEDIA_ENT_F_PROC_VIDEO_ENCODER);
+	ret = v4l2_m2m_register_media_controller(dev->stateful_enc.m2m_dev,
+						 &dev->stateful_enc.vfd,
+						 MEDIA_ENT_F_PROC_VIDEO_ENCODER);
 	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
+		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for enc\n");
 		goto unreg_m2m;
 	}
 
-	ret = v4l2_m2m_register_media_controller(dev->dec_dev,
-			&dev->dec_vfd, MEDIA_ENT_F_PROC_VIDEO_DECODER);
+	ret = v4l2_m2m_register_media_controller(dev->stateful_dec.m2m_dev,
+						 &dev->stateful_dec.vfd,
+						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
 	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
-		goto unreg_m2m_enc_mc;
+		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for dec\n");
+		goto unreg_m2m_sf_enc_mc;
 	}
 
 	ret = media_device_register(&dev->mdev);
 	if (ret) {
 		v4l2_err(&dev->v4l2_dev, "Failed to register mem2mem media device\n");
-		goto unreg_m2m_dec_mc;
+		goto unreg_m2m_sf_dec_mc;
 	}
 #endif
 	return 0;
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-unreg_m2m_dec_mc:
-	v4l2_m2m_unregister_media_controller(dev->dec_dev);
-unreg_m2m_enc_mc:
-	v4l2_m2m_unregister_media_controller(dev->enc_dev);
+unreg_m2m_sf_dec_mc:
+	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
+unreg_m2m_sf_enc_mc:
+	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
 unreg_m2m:
-	video_unregister_device(&dev->dec_vfd);
+	video_unregister_device(&dev->stateful_dec.vfd);
+	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
 #endif
-unreg_enc:
-	video_unregister_device(&dev->enc_vfd);
-err_dec_m2m:
-	v4l2_m2m_release(dev->dec_dev);
-err_enc_m2m:
-	v4l2_m2m_release(dev->enc_dev);
+unreg_sf_enc:
+	video_unregister_device(&dev->stateful_enc.vfd);
+	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
 unreg_dev:
 	v4l2_device_unregister(&dev->v4l2_dev);
 
@@ -1827,15 +1817,15 @@ static int vicodec_remove(struct platform_device *pdev)
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 	media_device_unregister(&dev->mdev);
-	v4l2_m2m_unregister_media_controller(dev->enc_dev);
-	v4l2_m2m_unregister_media_controller(dev->dec_dev);
+	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
+	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
 	media_device_cleanup(&dev->mdev);
 #endif
 
-	v4l2_m2m_release(dev->enc_dev);
-	v4l2_m2m_release(dev->dec_dev);
-	video_unregister_device(&dev->enc_vfd);
-	video_unregister_device(&dev->dec_vfd);
+	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
+	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
+	video_unregister_device(&dev->stateful_enc.vfd);
+	video_unregister_device(&dev->stateful_dec.vfd);
 	v4l2_device_unregister(&dev->v4l2_dev);
 
 	return 0;
-- 
2.17.1

[PATCH v5 17/23] media: vicodec: add documentation to V4L2_CID_FWHT_I/P_FRAME_QP

[Dafna Hirschfeld]

add documentation to V4L2_CID_FWHT_I/P_FRAME_QP
controls in ext-ctrls-codec.rst

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
index c97fb7923be5..301a4dfbbbde 100644
--- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
+++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
@@ -1537,6 +1537,17 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
 	non-intra-coded frames, in zigzag scanning order. Only relevant for
 	non-4:2:0 YUV formats.
 
+
+
+
+``V4L2_CID_FWHT_I_FRAME_QP (integer)``
+    Quantization parameter for an I frame for FWHT. Valid range: from 1
+    to 31.
+
+``V4L2_CID_FWHT_P_FRAME_QP (integer)``
+    Quantization parameter for a P frame for FWHT. Valid range: from 1
+    to 31.
+
 MFC 5.1 MPEG Controls
 =====================
 
-- 
2.17.1

[PATCH v5 18/23] media: vicodec: add documentation to V4L2_CID_MPEG_VIDEO_FWHT_PARAMS

[Dafna Hirschfeld]

add documentation to V4L2_CID_MPEG_VIDEO_FWHT_PARAMS
control and its related 'v4l2_ctrl_fwht_params' struct

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 .../media/uapi/v4l/ext-ctrls-codec.rst        | 119 ++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
index 301a4dfbbbde..45ce592279c3 100644
--- a/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
+++ b/Documentation/media/uapi/v4l/ext-ctrls-codec.rst
@@ -1538,6 +1538,125 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
 	non-4:2:0 YUV formats.
 
 
+.. _v4l2-mpeg-fwht:
+
+``V4L2_CID_MPEG_VIDEO_FWHT_PARAMS (struct)``
+    Specifies the fwht parameters (as extracted from the bitstream) for the
+    associated FWHT data. This includes the necessary parameters for
+    configuring a stateless hardware decoding pipeline for FWHT.
+
+    .. note::
+
+       This compound control is not yet part of the public kernel API and
+       it is expected to change.
+
+.. c:type:: v4l2_ctrl_fwht_params
+
+.. cssclass:: longtable
+
+.. flat-table:: struct v4l2_ctrl_fwht_params
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u64
+      - ``backward_ref_ts``
+      - Timestamp of the V4L2 capture buffer to use as backward reference, used
+        with P-coded frames. The timestamp refers to the
+	``timestamp`` field in struct :c:type:`v4l2_buffer`. Use the
+	:c:func:`v4l2_timeval_to_ns()` function to convert the struct
+	:c:type:`timeval` in struct :c:type:`v4l2_buffer` to a __u64.
+    * - __u32
+      - ``version``
+      - The version of the codec
+    * - __u32
+      - ``width``
+      - The width of the frame
+    * - __u32
+      - ``height``
+      - The height of the frame
+    * - __u32
+      - ``flags``
+      - The flags of the frame, see :ref:`fwht-flags`.
+    * - __u32
+      - ``colorspace``
+      - The colorspace of the frame, from enum :c:type:`v4l2_colorspace`.
+    * - __u32
+      - ``xfer_func``
+      - The transfer function, from enum :c:type:`v4l2_xfer_func`.
+    * - __u32
+      - ``ycbcr_enc``
+      - The Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`.
+    * - __u32
+      - ``quantization``
+      - The quantization range, from enum :c:type:`v4l2_quantization`.
+
+
+
+.. _fwht-flags:
+
+FWHT Flags
+============
+.. tabularcolumns:: |p{7.0cm}|p{2.2cm}|p{8.3cm}|
+
+.. cssclass:: longtable
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       3 1 4
+
+
+    * - ``FWHT_FL_IS_INTERLACED``
+      - 0x00000001
+      - Set if this is an interlaced format
+    * - ``FWHT_FL_IS_BOTTOM_FIRST``
+      - 0x00000002
+      - Set if this is a bottom-first (NTSC) interlaced format
+    * - ``FWHT_FL_IS_ALTERNATE``
+      - 0x00000004
+      - Set if each 'frame' contains just one field
+    * - ``FWHT_FL_IS_BOTTOM_FIELD``
+      - 0x00000008
+      - If FWHT_FL_IS_ALTERNATE was set, then this is set if this 'frame' is the
+	bottom field, else it is the top field.
+    * - ``FWHT_FL_LUMA_IS_UNCOMPRESSED``
+      - 0x00000010
+      - Set if the luma plane is uncompressed
+    * - ``FWHT_FL_CB_IS_UNCOMPRESSED``
+      - 0x00000020
+      - Set if the cb plane is uncompressed
+    * - ``FWHT_FL_CR_IS_UNCOMPRESSED``
+      - 0x00000040
+      - Set if the cr plane is uncompressed
+    * - ``FWHT_FL_CHROMA_FULL_HEIGHT``
+      - 0x00000080
+      - Set if the chroma plane has the same height as the luma plane,
+	else the chroma plane is half the height of the luma plane
+    * - ``FWHT_FL_CHROMA_FULL_WIDTH``
+      - 0x00000100
+      - Set if the chroma plane has the same width as the luma plane,
+	else the chroma plane is half the width of the luma plane
+    * - ``FWHT_FL_ALPHA_IS_UNCOMPRESSED``
+      - 0x00000200
+      - Set if the alpha plane is uncompressed
+    * - ``FWHT_FL_I_FRAME``
+      - 0x00000400
+      - Set if this is an I-frame
+    * - ``FWHT_FL_COMPONENTS_NUM_MSK``
+      - 0x00070000
+      - A 4-values flag - the number of components - 1
+    * - ``FWHT_FL_PIXENC_YUV``
+      - 0x00080000
+      - Set if the pixel encoding is YUV
+    * - ``FWHT_FL_PIXENC_RGB``
+      - 0x00100000
+      - Set if the pixel encoding is RGB
+    * - ``FWHT_FL_PIXENC_HSV``
+      - 0x00180000
+      - Set if the pixel encoding is HSV
+
+
 
 
 ``V4L2_CID_FWHT_I_FRAME_QP (integer)``
-- 
2.17.1

[PATCH v5 19/23] media: vicodec: add documentation to V4L2_PIX_FMT_FWHT_STATELESS

[Dafna Hirschfeld]

add documentation to V4L2_PIX_FMT_FWHT_STATELESS
in pixfmt-compressed.rst

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 Documentation/media/uapi/v4l/pixfmt-compressed.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
index 2675bef3eefe..6c961cfb74da 100644
--- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst
+++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
@@ -125,3 +125,9 @@ Compressed Formats
       - Video elementary stream using a codec based on the Fast Walsh Hadamard
         Transform. This codec is implemented by the vicodec ('Virtual Codec')
 	driver. See the codec-fwht.h header for more details.
+    * .. _V4L2-PIX-FMT-FWHT-STATELESS:
+
+      - ``V4L2_PIX_FMT_FWHT_STATELESS``
+      - 'SFWH'
+      - Same format as V4L2_PIX_FMT_FWHT but requires stateless codec implementation.
+	See the :ref:`associated Codec Control IDs <v4l2-mpeg-fwht>`.
-- 
2.17.1

[PATCH v5 20/23] media: vicodec: Introducing stateless fwht defs and structs

[Dafna Hirschfeld]

Add structs and definitions needed to implement stateless
decoder for fwht and add I/P-frames QP controls to the
public api.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 41 ++++++-------------
 drivers/media/v4l2-core/v4l2-ctrls.c          | 12 ++++++
 include/media/fwht-ctrls.h                    | 31 ++++++++++++++
 include/media/v4l2-ctrls.h                    |  5 ++-
 include/uapi/linux/v4l2-controls.h            |  4 ++
 include/uapi/linux/videodev2.h                |  1 +
 6 files changed, 65 insertions(+), 29 deletions(-)
 create mode 100644 include/media/fwht-ctrls.h

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 5998b9e86cda..6c9a41838d31 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -64,6 +64,10 @@ static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
 	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
 };
 
+static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {
+	V4L2_PIX_FMT_FWHT_STATELESS, 0, 3, 1, 1, 1, 1, 1, 0, 1
+};
+
 static void vicodec_dev_release(struct device *dev)
 {
 }
@@ -1510,10 +1514,6 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	return vb2_queue_init(dst_vq);
 }
 
-#define VICODEC_CID_CUSTOM_BASE		(V4L2_CID_MPEG_BASE | 0xf000)
-#define VICODEC_CID_I_FRAME_QP		(VICODEC_CID_CUSTOM_BASE + 0)
-#define VICODEC_CID_P_FRAME_QP		(VICODEC_CID_CUSTOM_BASE + 1)
-
 static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct vicodec_ctx *ctx = container_of(ctrl->handler,
@@ -1523,10 +1523,10 @@ static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
 		ctx->state.gop_size = ctrl->val;
 		return 0;
-	case VICODEC_CID_I_FRAME_QP:
+	case V4L2_CID_FWHT_I_FRAME_QP:
 		ctx->state.i_frame_qp = ctrl->val;
 		return 0;
-	case VICODEC_CID_P_FRAME_QP:
+	case V4L2_CID_FWHT_P_FRAME_QP:
 		ctx->state.p_frame_qp = ctrl->val;
 		return 0;
 	}
@@ -1537,26 +1537,9 @@ static const struct v4l2_ctrl_ops vicodec_ctrl_ops = {
 	.s_ctrl = vicodec_s_ctrl,
 };
 
-static const struct v4l2_ctrl_config vicodec_ctrl_i_frame = {
-	.ops = &vicodec_ctrl_ops,
-	.id = VICODEC_CID_I_FRAME_QP,
-	.name = "FWHT I-Frame QP Value",
-	.type = V4L2_CTRL_TYPE_INTEGER,
-	.min = 1,
-	.max = 31,
-	.def = 20,
-	.step = 1,
-};
-
-static const struct v4l2_ctrl_config vicodec_ctrl_p_frame = {
-	.ops = &vicodec_ctrl_ops,
-	.id = VICODEC_CID_P_FRAME_QP,
-	.name = "FWHT P-Frame QP Value",
-	.type = V4L2_CTRL_TYPE_INTEGER,
-	.min = 1,
-	.max = 31,
-	.def = 20,
-	.step = 1,
+static const struct v4l2_ctrl_config vicodec_ctrl_stateless_state = {
+	.id		= V4L2_CID_MPEG_VIDEO_FWHT_PARAMS,
+	.elem_size      = sizeof(struct v4l2_ctrl_fwht_params),
 };
 
 /*
@@ -1589,8 +1572,10 @@ static int vicodec_open(struct file *file)
 	v4l2_ctrl_handler_init(hdl, 4);
 	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_MPEG_VIDEO_GOP_SIZE,
 			  1, 16, 1, 10);
-	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_i_frame, NULL);
-	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_p_frame, NULL);
+	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_FWHT_I_FRAME_QP,
+			  1, 31, 1, 20);
+	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_FWHT_P_FRAME_QP,
+			  1, 31, 1, 20);
 	if (hdl->error) {
 		rc = hdl->error;
 		v4l2_ctrl_handler_free(hdl);
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 54d66dbc2a31..aed1c3a06500 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -849,6 +849,9 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:		return "Force Key Frame";
 	case V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS:		return "MPEG-2 Slice Parameters";
 	case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION:		return "MPEG-2 Quantization Matrices";
+	case V4L2_CID_MPEG_VIDEO_FWHT_PARAMS:			return "FWHT Stateless Parameters";
+	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
+	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
 
 	/* VPX controls */
 	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
@@ -1303,6 +1306,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION:
 		*type = V4L2_CTRL_TYPE_MPEG2_QUANTIZATION;
 		break;
+	case V4L2_CID_MPEG_VIDEO_FWHT_PARAMS:
+		*type = V4L2_CTRL_TYPE_FWHT_PARAMS;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
@@ -1669,6 +1675,9 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
 	case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
 		return 0;
 
+	case V4L2_CTRL_TYPE_FWHT_PARAMS:
+		return 0;
+
 	default:
 		return -EINVAL;
 	}
@@ -2249,6 +2258,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
 		elem_size = sizeof(struct v4l2_ctrl_mpeg2_quantization);
 		break;
+	case V4L2_CTRL_TYPE_FWHT_PARAMS:
+		elem_size = sizeof(struct v4l2_ctrl_fwht_params);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);
diff --git a/include/media/fwht-ctrls.h b/include/media/fwht-ctrls.h
new file mode 100644
index 000000000000..615027410e47
--- /dev/null
+++ b/include/media/fwht-ctrls.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * These are the FWHT state controls for use with stateless FWHT
+ * 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 _FWHT_CTRLS_H_
+#define _FWHT_CTRLS_H_
+
+#define V4L2_CTRL_TYPE_FWHT_PARAMS 0x0105
+
+#define V4L2_CID_MPEG_VIDEO_FWHT_PARAMS	(V4L2_CID_MPEG_BASE + 292)
+
+struct v4l2_ctrl_fwht_params {
+	__u64 backward_ref_ts;
+	__u32 version;
+	__u32 width;
+	__u32 height;
+	__u32 flags;
+	__u32 colorspace;
+	__u32 xfer_func;
+	__u32 ycbcr_enc;
+	__u32 quantization;
+};
+
+
+#endif
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 200f8a66ecaa..bd621cec65a5 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -23,10 +23,11 @@
 #include <media/media-request.h>
 
 /*
- * Include the mpeg2 stateless codec compound control definitions.
+ * Include the mpeg2 and fwht stateless codec compound control definitions.
  * This will move to the public headers once this API is fully stable.
  */
 #include <media/mpeg2-ctrls.h>
+#include <media/fwht-ctrls.h>
 
 /* forward references */
 struct file;
@@ -49,6 +50,7 @@ struct poll_table_struct;
  * @p_char:			Pointer to a string.
  * @p_mpeg2_slice_params:	Pointer to a MPEG2 slice parameters structure.
  * @p_mpeg2_quantization:	Pointer to a MPEG2 quantization data structure.
+ * @p_fwht_params:		Pointer to a FWHT stateless parameters structure.
  * @p:				Pointer to a compound value.
  */
 union v4l2_ctrl_ptr {
@@ -60,6 +62,7 @@ union v4l2_ctrl_ptr {
 	char *p_char;
 	struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
 	struct v4l2_ctrl_mpeg2_quantization *p_mpeg2_quantization;
+	struct v4l2_ctrl_fwht_params *p_fwht_params;
 	void *p;
 };
 
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 06479f2fb3ae..78816ec88751 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -404,6 +404,10 @@ enum v4l2_mpeg_video_multi_slice_mode {
 #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_MPEG_BASE+228)
 #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME		(V4L2_CID_MPEG_BASE+229)
 
+/* CIDs for the FWHT codec as used by the vicodec driver. */
+#define V4L2_CID_FWHT_I_FRAME_QP             (V4L2_CID_MPEG_BASE + 290)
+#define V4L2_CID_FWHT_P_FRAME_QP             (V4L2_CID_MPEG_BASE + 291)
+
 #define V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP		(V4L2_CID_MPEG_BASE+300)
 #define V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP		(V4L2_CID_MPEG_BASE+301)
 #define V4L2_CID_MPEG_VIDEO_H263_B_FRAME_QP		(V4L2_CID_MPEG_BASE+302)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 97e6a6a968ba..1ac3c22d883a 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -669,6 +669,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
+#define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
 
 /*  Vendor-specific formats   */
 #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
-- 
2.17.1

[PATCH v5 21/23] media: vicodec: Register another node for stateless decoder

[Dafna Hirschfeld]

Add stateless decoder instance field to the dev struct and
register another node for the statelsess decoder.
The stateless API for the node will be implemented in further patches.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 46 +++++++++++++++++--
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 6c9a41838d31..7733b22247b6 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -104,6 +104,7 @@ struct vicodec_dev {
 	struct v4l2_device	v4l2_dev;
 	struct vicodec_dev_instance stateful_enc;
 	struct vicodec_dev_instance stateful_dec;
+	struct vicodec_dev_instance stateless_dec;
 #ifdef CONFIG_MEDIA_CONTROLLER
 	struct media_device	mdev;
 #endif
@@ -114,6 +115,7 @@ struct vicodec_ctx {
 	struct v4l2_fh		fh;
 	struct vicodec_dev	*dev;
 	bool			is_enc;
+	bool			is_stateless;
 	spinlock_t		*lock;
 
 	struct v4l2_ctrl_handler hdl;
@@ -373,6 +375,9 @@ static void device_run(void *priv)
 
 	if (ctx->is_enc)
 		v4l2_m2m_job_finish(dev->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
+	else if (ctx->is_stateless)
+		v4l2_m2m_job_finish(dev->stateless_dec.m2m_dev,
+				    ctx->fh.m2m_ctx);
 	else
 		v4l2_m2m_job_finish(dev->stateful_dec.m2m_dev, ctx->fh.m2m_ctx);
 }
@@ -1494,8 +1499,14 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->ops = &vicodec_qops;
 	src_vq->mem_ops = &vb2_vmalloc_memops;
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
-	src_vq->lock = ctx->is_enc ? &ctx->dev->stateful_enc.mutex :
-		&ctx->dev->stateful_dec.mutex;
+	if (ctx->is_enc)
+		src_vq->lock = &ctx->dev->stateful_enc.mutex;
+	else if (ctx->is_stateless)
+		src_vq->lock = &ctx->dev->stateless_dec.mutex;
+	else
+		src_vq->lock = &ctx->dev->stateful_dec.mutex;
+	src_vq->supports_requests = ctx->is_stateless;
+	src_vq->requires_requests = ctx->is_stateless;
 	ret = vb2_queue_init(src_vq);
 	if (ret)
 		return ret;
@@ -1564,6 +1575,8 @@ static int vicodec_open(struct file *file)
 
 	if (vfd == &dev->stateful_enc.vfd)
 		ctx->is_enc = true;
+	else if (vfd == &dev->stateless_dec.vfd)
+		ctx->is_stateless = true;
 
 	v4l2_fh_init(&ctx->fh, video_devdata(file));
 	file->private_data = &ctx->fh;
@@ -1576,6 +1589,8 @@ static int vicodec_open(struct file *file)
 			  1, 31, 1, 20);
 	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_FWHT_P_FRAME_QP,
 			  1, 31, 1, 20);
+	if (ctx->is_stateless)
+		v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_stateless_state, NULL);
 	if (hdl->error) {
 		rc = hdl->error;
 		v4l2_ctrl_handler_free(hdl);
@@ -1615,6 +1630,10 @@ static int vicodec_open(struct file *file)
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_enc.m2m_dev,
 						    ctx, &queue_init);
 		ctx->lock = &dev->stateful_enc.lock;
+	} else if (ctx->is_stateless) {
+		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateless_dec.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->stateless_dec.lock;
 	} else {
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_dec.m2m_dev,
 						    ctx, &queue_init);
@@ -1751,6 +1770,10 @@ static int vicodec_probe(struct platform_device *pdev)
 			      "stateful-decoder", false))
 		goto unreg_sf_enc;
 
+	if (register_instance(dev, &dev->stateless_dec,
+			      "videdev-stateless-dec", false))
+		goto unreg_sf_dec;
+
 #ifdef CONFIG_MEDIA_CONTROLLER
 	ret = v4l2_m2m_register_media_controller(dev->stateful_enc.m2m_dev,
 						 &dev->stateful_enc.vfd,
@@ -1768,23 +1791,36 @@ static int vicodec_probe(struct platform_device *pdev)
 		goto unreg_m2m_sf_enc_mc;
 	}
 
+	ret = v4l2_m2m_register_media_controller(dev->stateless_dec.m2m_dev,
+						 &dev->stateless_dec.vfd,
+						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
+	if (ret) {
+		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for stateless dec\n");
+		goto unreg_m2m_sf_dec_mc;
+	}
+
 	ret = media_device_register(&dev->mdev);
 	if (ret) {
 		v4l2_err(&dev->v4l2_dev, "Failed to register mem2mem media device\n");
-		goto unreg_m2m_sf_dec_mc;
+		goto unreg_m2m_sl_dec_mc;
 	}
 #endif
 	return 0;
 
 #ifdef CONFIG_MEDIA_CONTROLLER
+unreg_m2m_sl_dec_mc:
+	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
 unreg_m2m_sf_dec_mc:
 	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
 unreg_m2m_sf_enc_mc:
 	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
 unreg_m2m:
+	video_unregister_device(&dev->stateless_dec.vfd);
+	v4l2_m2m_release(dev->stateless_dec.m2m_dev);
+#endif
+unreg_sf_dec:
 	video_unregister_device(&dev->stateful_dec.vfd);
 	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
-#endif
 unreg_sf_enc:
 	video_unregister_device(&dev->stateful_enc.vfd);
 	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
@@ -1804,6 +1840,7 @@ static int vicodec_remove(struct platform_device *pdev)
 	media_device_unregister(&dev->mdev);
 	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
 	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
+	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
 	media_device_cleanup(&dev->mdev);
 #endif
 
@@ -1811,6 +1848,7 @@ static int vicodec_remove(struct platform_device *pdev)
 	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
 	video_unregister_device(&dev->stateful_enc.vfd);
 	video_unregister_device(&dev->stateful_dec.vfd);
+	video_unregister_device(&dev->stateless_dec.vfd);
 	v4l2_device_unregister(&dev->v4l2_dev);
 
 	return 0;
-- 
2.17.1

[PATCH v5 22/23] media: vicodec: Add support for stateless decoder.

[Dafna Hirschfeld]

Implement a stateless decoder for the new node.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 .../media/platform/vicodec/codec-v4l2-fwht.h  |   1 +
 drivers/media/platform/vicodec/vicodec-core.c | 286 ++++++++++++++++--
 2 files changed, 263 insertions(+), 24 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
index b59503d4049a..1a0d2a9f931a 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
@@ -44,6 +44,7 @@ struct v4l2_fwht_state {
 	struct fwht_raw_frame ref_frame;
 	struct fwht_cframe_hdr header;
 	u8 *compressed_frame;
+	u64 ref_frame_ts;
 };
 
 const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat);
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 7733b22247b6..d2633a6135c8 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -214,6 +214,41 @@ static bool validate_by_version(unsigned int flags, unsigned int version)
 	return true;
 }
 
+static bool validate_stateless_params_flags(const struct v4l2_ctrl_fwht_params *params,
+					    const struct v4l2_fwht_pixfmt_info *cur_info)
+{
+	unsigned int width_div =
+		(params->flags & FWHT_FL_CHROMA_FULL_WIDTH) ? 1 : 2;
+	unsigned int height_div =
+		(params->flags & FWHT_FL_CHROMA_FULL_HEIGHT) ? 1 : 2;
+	unsigned int components_num = 3;
+	unsigned int pixenc = 0;
+
+	if (params->version < 3)
+		return false;
+
+	components_num = 1 + ((params->flags & FWHT_FL_COMPONENTS_NUM_MSK) >>
+			      FWHT_FL_COMPONENTS_NUM_OFFSET);
+	pixenc = (params->flags & FWHT_FL_PIXENC_MSK);
+	if (v4l2_fwht_validate_fmt(cur_info, width_div, height_div,
+				    components_num, pixenc))
+		return true;
+	return false;
+}
+
+
+static void update_state_from_header(struct vicodec_ctx *ctx)
+{
+	const struct fwht_cframe_hdr *p_hdr = &ctx->state.header;
+
+	ctx->state.visible_width = ntohl(p_hdr->width);
+	ctx->state.visible_height = ntohl(p_hdr->height);
+	ctx->state.colorspace = ntohl(p_hdr->colorspace);
+	ctx->state.xfer_func = ntohl(p_hdr->xfer_func);
+	ctx->state.ycbcr_enc = ntohl(p_hdr->ycbcr_enc);
+	ctx->state.quantization = ntohl(p_hdr->quantization);
+}
+
 static int device_process(struct vicodec_ctx *ctx,
 			  struct vb2_v4l2_buffer *src_vb,
 			  struct vb2_v4l2_buffer *dst_vb)
@@ -221,12 +256,48 @@ static int device_process(struct vicodec_ctx *ctx,
 	struct vicodec_dev *dev = ctx->dev;
 	struct v4l2_fwht_state *state = &ctx->state;
 	u8 *p_src, *p_dst;
-	int ret;
+	int ret = 0;
 
-	if (ctx->is_enc)
+	if (ctx->is_enc || ctx->is_stateless)
 		p_src = vb2_plane_vaddr(&src_vb->vb2_buf, 0);
 	else
 		p_src = state->compressed_frame;
+
+	if (ctx->is_stateless) {
+		struct media_request *src_req = src_vb->vb2_buf.req_obj.req;
+
+		ret = v4l2_ctrl_request_setup(src_req, &ctx->hdl);
+		if (ret)
+			return ret;
+		update_state_from_header(ctx);
+
+		ctx->state.header.size =
+			htonl(vb2_get_plane_payload(&src_vb->vb2_buf, 0));
+		/*
+		 * set the reference buffer from the reference timestamp
+		 * only if this is a P-frame
+		 */
+		if (!(ntohl(ctx->state.header.flags) & FWHT_FL_I_FRAME)) {
+			struct vb2_buffer *ref_vb2_buf;
+			int ref_buf_idx;
+			struct vb2_queue *vq_cap =
+				v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+						V4L2_BUF_TYPE_VIDEO_CAPTURE);
+
+			ref_buf_idx = vb2_find_timestamp(vq_cap,
+							 ctx->state.ref_frame_ts, 0);
+			if (ref_buf_idx < 0)
+				return -EINVAL;
+
+			ref_vb2_buf = vq_cap->bufs[ref_buf_idx];
+			if (ref_vb2_buf->state == VB2_BUF_STATE_ERROR)
+				ret = -EINVAL;
+			ctx->state.ref_frame.buf =
+				vb2_plane_vaddr(ref_vb2_buf, 0);
+		} else {
+			ctx->state.ref_frame.buf = NULL;
+		}
+	}
 	p_dst = vb2_plane_vaddr(&dst_vb->vb2_buf, 0);
 	if (!p_src || !p_dst) {
 		v4l2_err(&dev->v4l2_dev,
@@ -255,11 +326,12 @@ static int device_process(struct vicodec_ctx *ctx,
 		ret = v4l2_fwht_decode(state, p_src, p_dst);
 		if (ret < 0)
 			return ret;
-		copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
+		if (!ctx->is_stateless)
+			copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
 
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
 	}
-	return 0;
+	return ret;
 }
 
 /*
@@ -334,9 +406,13 @@ static void device_run(void *priv)
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 	struct vicodec_q_data *q_src, *q_dst;
 	u32 state;
+	struct media_request *src_req;
+
 
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+	src_req = src_buf->vb2_buf.req_obj.req;
+
 	q_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
 	q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
@@ -355,7 +431,7 @@ static void device_run(void *priv)
 		dst_buf->flags |= V4L2_BUF_FLAG_LAST;
 		v4l2_event_queue_fh(&ctx->fh, &eos_event);
 	}
-	if (ctx->is_enc) {
+	if (ctx->is_enc || ctx->is_stateless) {
 		src_buf->sequence = q_src->sequence++;
 		src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 		v4l2_m2m_buf_done(src_buf, state);
@@ -367,6 +443,9 @@ static void device_run(void *priv)
 		ctx->comp_has_next_frame = false;
 	}
 	v4l2_m2m_buf_done(dst_buf, state);
+	if (ctx->is_stateless && src_req)
+		v4l2_ctrl_request_complete(src_req, &ctx->hdl);
+
 	ctx->comp_size = 0;
 	ctx->header_size = 0;
 	ctx->comp_magic_cnt = 0;
@@ -445,6 +524,12 @@ static void update_capture_data_from_header(struct vicodec_ctx *ctx)
 	unsigned int hdr_width_div = (flags & FWHT_FL_CHROMA_FULL_WIDTH) ? 1 : 2;
 	unsigned int hdr_height_div = (flags & FWHT_FL_CHROMA_FULL_HEIGHT) ? 1 : 2;
 
+	/*
+	 * This function should not be used by a stateless codec since
+	 * it changes values in q_data that are not request specific
+	 */
+	WARN_ON(ctx->is_stateless);
+
 	q_dst->info = info;
 	q_dst->visible_width = ntohl(p_hdr->width);
 	q_dst->visible_height = ntohl(p_hdr->height);
@@ -497,7 +582,7 @@ static int job_ready(void *priv)
 
 	if (ctx->source_changed)
 		return 0;
-	if (ctx->is_enc || ctx->comp_has_frame)
+	if (ctx->is_stateless || ctx->is_enc || ctx->comp_has_frame)
 		return 1;
 
 restart:
@@ -1243,6 +1328,14 @@ static int vicodec_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
 	return 0;
 }
 
+static int vicodec_buf_out_validate(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+	vbuf->field = V4L2_FIELD_NONE;
+	return 0;
+}
+
 static int vicodec_buf_prepare(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -1306,10 +1399,11 @@ static void vicodec_buf_queue(struct vb2_buffer *vb)
 	}
 
 	/*
-	 * source change event is relevant only for the decoder
+	 * source change event is relevant only for the stateful decoder
 	 * in the compressed stream
 	 */
-	if (ctx->is_enc || !V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
+	if (ctx->is_stateless || ctx->is_enc ||
+	    !V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
 		return;
 	}
@@ -1357,12 +1451,33 @@ static void vicodec_return_bufs(struct vb2_queue *q, u32 state)
 			vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 		if (vbuf == NULL)
 			return;
+		v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req,
+					   &ctx->hdl);
 		spin_lock(ctx->lock);
 		v4l2_m2m_buf_done(vbuf, state);
 		spin_unlock(ctx->lock);
 	}
 }
 
+static unsigned int total_frame_size(struct vicodec_q_data *q_data)
+{
+	unsigned int size;
+	unsigned int chroma_div;
+
+	if (!q_data->info) {
+		WARN_ON(1);
+		return 0;
+	}
+	size = q_data->coded_width * q_data->coded_height;
+	chroma_div = q_data->info->width_div * q_data->info->height_div;
+
+	if (q_data->info->components_num == 4)
+		return 2 * size + 2 * (size / chroma_div);
+	else if (q_data->info->components_num == 3)
+		return size + 2 * (size / chroma_div);
+	return size;
+}
+
 static int vicodec_start_streaming(struct vb2_queue *q,
 				   unsigned int count)
 {
@@ -1373,7 +1488,7 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	unsigned int size = q_data->coded_width * q_data->coded_height;
 	unsigned int chroma_div;
 	unsigned int total_planes_size;
-	u8 *new_comp_frame;
+	u8 *new_comp_frame = NULL;
 
 	if (!info)
 		return -EINVAL;
@@ -1393,12 +1508,8 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 		vicodec_return_bufs(q, VB2_BUF_STATE_QUEUED);
 		return -EINVAL;
 	}
-	if (info->components_num == 4)
-		total_planes_size = 2 * size + 2 * (size / chroma_div);
-	else if (info->components_num == 3)
-		total_planes_size = size + 2 * (size / chroma_div);
-	else
-		total_planes_size = size;
+	total_planes_size = total_frame_size(q_data);
+	ctx->comp_max_size = total_planes_size;
 
 	state->visible_width = q_data->visible_width;
 	state->visible_height = q_data->visible_height;
@@ -1407,10 +1518,14 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	state->stride = q_data->coded_width *
 				info->bytesperline_mult;
 
+	if (ctx->is_stateless) {
+		state->ref_stride = state->stride;
+		return 0;
+	}
 	state->ref_stride = q_data->coded_width * info->luma_alpha_step;
+
 	state->ref_frame.buf = kvmalloc(total_planes_size, GFP_KERNEL);
 	state->ref_frame.luma = state->ref_frame.buf;
-	ctx->comp_max_size = total_planes_size;
 	new_comp_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
 
 	if (!state->ref_frame.luma || !new_comp_frame) {
@@ -1458,7 +1573,8 @@ static void vicodec_stop_streaming(struct vb2_queue *q)
 
 	if ((!V4L2_TYPE_IS_OUTPUT(q->type) && !ctx->is_enc) ||
 	    (V4L2_TYPE_IS_OUTPUT(q->type) && ctx->is_enc)) {
-		kvfree(ctx->state.ref_frame.buf);
+		if (!ctx->is_stateless)
+			kvfree(ctx->state.ref_frame.buf);
 		ctx->state.ref_frame.buf = NULL;
 		ctx->state.ref_frame.luma = NULL;
 		ctx->comp_max_size = 0;
@@ -1474,14 +1590,24 @@ static void vicodec_stop_streaming(struct vb2_queue *q)
 	}
 }
 
+static void vicodec_buf_request_complete(struct vb2_buffer *vb)
+{
+	struct vicodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+	v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
+}
+
+
 static const struct vb2_ops vicodec_qops = {
-	.queue_setup	 = vicodec_queue_setup,
-	.buf_prepare	 = vicodec_buf_prepare,
-	.buf_queue	 = vicodec_buf_queue,
-	.start_streaming = vicodec_start_streaming,
-	.stop_streaming  = vicodec_stop_streaming,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
+	.queue_setup		= vicodec_queue_setup,
+	.buf_out_validate	= vicodec_buf_out_validate,
+	.buf_prepare		= vicodec_buf_prepare,
+	.buf_queue		= vicodec_buf_queue,
+	.buf_request_complete	= vicodec_buf_request_complete,
+	.start_streaming	= vicodec_start_streaming,
+	.stop_streaming		= vicodec_stop_streaming,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
@@ -1525,10 +1651,56 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	return vb2_queue_init(dst_vq);
 }
 
+static int vicodec_try_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct vicodec_ctx *ctx = container_of(ctrl->handler,
+			struct vicodec_ctx, hdl);
+	const struct v4l2_ctrl_fwht_params *params;
+	struct vicodec_q_data *q_dst = get_q_data(ctx,
+			V4L2_BUF_TYPE_VIDEO_CAPTURE);
+
+	switch (ctrl->id) {
+	case V4L2_CID_MPEG_VIDEO_FWHT_PARAMS:
+		if (!q_dst->info)
+			return -EINVAL;
+		params = ctrl->p_new.p_fwht_params;
+		if (params->width > q_dst->coded_width ||
+		    params->width < MIN_WIDTH ||
+		    params->height > q_dst->coded_height ||
+		    params->height < MIN_HEIGHT)
+			return -EINVAL;
+		if (!validate_by_version(params->flags, params->version))
+			return -EINVAL;
+		if (!validate_stateless_params_flags(params, q_dst->info))
+			return -EINVAL;
+	default:
+		return 0;
+	}
+	return 0;
+}
+
+static void update_header_from_stateless_params(struct vicodec_ctx *ctx,
+						const struct v4l2_ctrl_fwht_params *params)
+{
+	struct fwht_cframe_hdr *p_hdr = &ctx->state.header;
+
+	p_hdr->magic1 = FWHT_MAGIC1;
+	p_hdr->magic2 = FWHT_MAGIC2;
+	p_hdr->version = htonl(params->version);
+	p_hdr->width = htonl(params->width);
+	p_hdr->height = htonl(params->height);
+	p_hdr->flags = htonl(params->flags);
+	p_hdr->colorspace = htonl(params->colorspace);
+	p_hdr->xfer_func = htonl(params->xfer_func);
+	p_hdr->ycbcr_enc = htonl(params->ycbcr_enc);
+	p_hdr->quantization = htonl(params->quantization);
+}
+
 static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct vicodec_ctx *ctx = container_of(ctrl->handler,
 					       struct vicodec_ctx, hdl);
+	const struct v4l2_ctrl_fwht_params *params;
 
 	switch (ctrl->id) {
 	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
@@ -1540,15 +1712,22 @@ static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_FWHT_P_FRAME_QP:
 		ctx->state.p_frame_qp = ctrl->val;
 		return 0;
+	case V4L2_CID_MPEG_VIDEO_FWHT_PARAMS:
+		params = ctrl->p_new.p_fwht_params;
+		update_header_from_stateless_params(ctx, params);
+		ctx->state.ref_frame_ts = params->backward_ref_ts;
+		return 0;
 	}
 	return -EINVAL;
 }
 
 static const struct v4l2_ctrl_ops vicodec_ctrl_ops = {
 	.s_ctrl = vicodec_s_ctrl,
+	.try_ctrl = vicodec_try_ctrl,
 };
 
 static const struct v4l2_ctrl_config vicodec_ctrl_stateless_state = {
+	.ops		= &vicodec_ctrl_ops,
 	.id		= V4L2_CID_MPEG_VIDEO_FWHT_PARAMS,
 	.elem_size      = sizeof(struct v4l2_ctrl_fwht_params),
 };
@@ -1673,6 +1852,59 @@ static int vicodec_release(struct file *file)
 	return 0;
 }
 
+static int vicodec_request_validate(struct media_request *req)
+{
+	struct media_request_object *obj;
+	struct v4l2_ctrl_handler *parent_hdl, *hdl;
+	struct vicodec_ctx *ctx = NULL;
+	struct v4l2_ctrl *ctrl;
+	unsigned int count;
+
+	list_for_each_entry(obj, &req->objects, list) {
+		struct vb2_buffer *vb;
+
+		if (vb2_request_object_is_buffer(obj)) {
+			vb = container_of(obj, struct vb2_buffer, req_obj);
+			ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+			break;
+		}
+	}
+
+	if (!ctx) {
+		pr_err("No buffer was provided with the request\n");
+		return -ENOENT;
+	}
+
+	count = vb2_request_buffer_cnt(req);
+	if (!count) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "No buffer was provided with the request\n");
+		return -ENOENT;
+	} else if (count > 1) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "More than one buffer was provided with the request\n");
+		return -EINVAL;
+	}
+
+	parent_hdl = &ctx->hdl;
+
+	hdl = v4l2_ctrl_request_hdl_find(req, parent_hdl);
+	if (!hdl) {
+		v4l2_info(&ctx->dev->v4l2_dev, "Missing codec control\n");
+		return -ENOENT;
+	}
+	ctrl = v4l2_ctrl_request_hdl_ctrl_find(hdl,
+					       vicodec_ctrl_stateless_state.id);
+	if (!ctrl) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "Missing required codec control\n");
+		return -ENOENT;
+	}
+
+	return vb2_request_validate(req);
+}
+
 static const struct v4l2_file_operations vicodec_fops = {
 	.owner		= THIS_MODULE,
 	.open		= vicodec_open,
@@ -1691,6 +1923,11 @@ static const struct video_device vicodec_videodev = {
 	.release	= video_device_release_empty,
 };
 
+static const struct media_device_ops vicodec_m2m_media_ops = {
+	.req_validate	= vicodec_request_validate,
+	.req_queue	= v4l2_m2m_request_queue,
+};
+
 static const struct v4l2_m2m_ops m2m_ops = {
 	.device_run	= device_run,
 	.job_ready	= job_ready,
@@ -1757,6 +1994,7 @@ static int vicodec_probe(struct platform_device *pdev)
 	strscpy(dev->mdev.bus_info, "platform:vicodec",
 		sizeof(dev->mdev.bus_info));
 	media_device_init(&dev->mdev);
+	dev->mdev.ops = &vicodec_m2m_media_ops;
 	dev->v4l2_dev.mdev = &dev->mdev;
 #endif
 
-- 
2.17.1

[PATCH v5 23/23] media: vicodec: set pixelformat to V4L2_PIX_FMT_FWHT_STATELESS for stateless decoder

[Dafna Hirschfeld]

for stateless decoder, set the output pixelformat
to V4L2_PIX_FMT_FWHT_STATELESS and the pix info to
pixfmt_stateless_fwht

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 47 ++++++++++++++-----
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index d2633a6135c8..2f7419b39452 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -728,7 +728,8 @@ static int enum_fmt(struct v4l2_fmtdesc *f, struct vicodec_ctx *ctx,
 	} else {
 		if (f->index)
 			return -EINVAL;
-		f->pixelformat = V4L2_PIX_FMT_FWHT;
+		f->pixelformat = ctx->is_stateless ?
+			V4L2_PIX_FMT_FWHT_STATELESS : V4L2_PIX_FMT_FWHT;
 	}
 	return 0;
 }
@@ -830,13 +831,15 @@ static int vidioc_try_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 	struct v4l2_pix_format_mplane *pix_mp;
 	struct v4l2_pix_format *pix;
 	struct v4l2_plane_pix_format *plane;
-	const struct v4l2_fwht_pixfmt_info *info = &pixfmt_fwht;
+	const struct v4l2_fwht_pixfmt_info *info = ctx->is_stateless ?
+		&pixfmt_stateless_fwht : &pixfmt_fwht;
 
 	switch (f->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 		pix = &f->fmt.pix;
-		if (pix->pixelformat != V4L2_PIX_FMT_FWHT)
+		if (pix->pixelformat != V4L2_PIX_FMT_FWHT &&
+		    pix->pixelformat != V4L2_PIX_FMT_FWHT_STATELESS)
 			info = find_fmt(pix->pixelformat);
 
 		pix->width = clamp(pix->width, MIN_WIDTH, MAX_WIDTH);
@@ -857,7 +860,8 @@ static int vidioc_try_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		pix_mp = &f->fmt.pix_mp;
 		plane = pix_mp->plane_fmt;
-		if (pix_mp->pixelformat != V4L2_PIX_FMT_FWHT)
+		if (pix_mp->pixelformat != V4L2_PIX_FMT_FWHT &&
+		    pix_mp->pixelformat != V4L2_PIX_FMT_FWHT_STATELESS)
 			info = find_fmt(pix_mp->pixelformat);
 		pix_mp->num_planes = 1;
 
@@ -934,8 +938,12 @@ static int vidioc_try_fmt_vid_out(struct file *file, void *priv,
 		if (multiplanar)
 			return -EINVAL;
 		pix = &f->fmt.pix;
-		pix->pixelformat = !ctx->is_enc ? V4L2_PIX_FMT_FWHT :
-				   find_fmt(pix->pixelformat)->id;
+		if (ctx->is_enc)
+			pix->pixelformat = find_fmt(pix->pixelformat)->id;
+		else if (ctx->is_stateless)
+			pix->pixelformat = V4L2_PIX_FMT_FWHT_STATELESS;
+		else
+			pix->pixelformat = V4L2_PIX_FMT_FWHT;
 		if (!pix->colorspace)
 			pix->colorspace = V4L2_COLORSPACE_REC709;
 		break;
@@ -943,8 +951,12 @@ static int vidioc_try_fmt_vid_out(struct file *file, void *priv,
 		if (!multiplanar)
 			return -EINVAL;
 		pix_mp = &f->fmt.pix_mp;
-		pix_mp->pixelformat = !ctx->is_enc ? V4L2_PIX_FMT_FWHT :
-				      find_fmt(pix_mp->pixelformat)->id;
+		if (ctx->is_enc)
+			pix_mp->pixelformat = find_fmt(pix_mp->pixelformat)->id;
+		else if (ctx->is_stateless)
+			pix_mp->pixelformat = V4L2_PIX_FMT_FWHT_STATELESS;
+		else
+			pix_mp->pixelformat = V4L2_PIX_FMT_FWHT;
 		if (!pix_mp->colorspace)
 			pix_mp->colorspace = V4L2_COLORSPACE_REC709;
 		break;
@@ -987,6 +999,8 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 
 		if (pix->pixelformat == V4L2_PIX_FMT_FWHT)
 			q_data->info = &pixfmt_fwht;
+		else if (pix->pixelformat == V4L2_PIX_FMT_FWHT_STATELESS)
+			q_data->info = &pixfmt_stateless_fwht;
 		else
 			q_data->info = find_fmt(pix->pixelformat);
 		q_data->coded_width = pix->width;
@@ -1008,6 +1022,8 @@ static int vidioc_s_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
 
 		if (pix_mp->pixelformat == V4L2_PIX_FMT_FWHT)
 			q_data->info = &pixfmt_fwht;
+		else if (pix_mp->pixelformat == V4L2_PIX_FMT_FWHT_STATELESS)
+			q_data->info = &pixfmt_stateless_fwht;
 		else
 			q_data->info = find_fmt(pix_mp->pixelformat);
 		q_data->coded_width = pix_mp->width;
@@ -1220,6 +1236,8 @@ static int vicodec_enum_framesizes(struct file *file, void *fh,
 				   struct v4l2_frmsizeenum *fsize)
 {
 	switch (fsize->pixel_format) {
+	case V4L2_PIX_FMT_FWHT_STATELESS:
+		break;
 	case V4L2_PIX_FMT_FWHT:
 		break;
 	default:
@@ -1504,7 +1522,8 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	    (!V4L2_TYPE_IS_OUTPUT(q->type) && ctx->is_enc))
 		return 0;
 
-	if (info->id == V4L2_PIX_FMT_FWHT) {
+	if (info->id == V4L2_PIX_FMT_FWHT ||
+	    info->id == V4L2_PIX_FMT_FWHT_STATELESS) {
 		vicodec_return_bufs(q, VB2_BUF_STATE_QUEUED);
 		return -EINVAL;
 	}
@@ -1779,15 +1798,19 @@ static int vicodec_open(struct file *file)
 	ctx->fh.ctrl_handler = hdl;
 	v4l2_ctrl_handler_setup(hdl);
 
-	ctx->q_data[V4L2_M2M_SRC].info =
-		ctx->is_enc ? v4l2_fwht_get_pixfmt(0) : &pixfmt_fwht;
+	if (ctx->is_enc)
+		ctx->q_data[V4L2_M2M_SRC].info = v4l2_fwht_get_pixfmt(0);
+	else if (ctx->is_stateless)
+		ctx->q_data[V4L2_M2M_SRC].info = &pixfmt_stateless_fwht;
+	else
+		ctx->q_data[V4L2_M2M_SRC].info = &pixfmt_fwht;
 	ctx->q_data[V4L2_M2M_SRC].coded_width = 1280;
 	ctx->q_data[V4L2_M2M_SRC].coded_height = 720;
 	ctx->q_data[V4L2_M2M_SRC].visible_width = 1280;
 	ctx->q_data[V4L2_M2M_SRC].visible_height = 720;
 	size = 1280 * 720 * ctx->q_data[V4L2_M2M_SRC].info->sizeimage_mult /
 		ctx->q_data[V4L2_M2M_SRC].info->sizeimage_div;
-	if (ctx->is_enc)
+	if (ctx->is_enc || ctx->is_stateless)
 		ctx->q_data[V4L2_M2M_SRC].sizeimage = size;
 	else
 		ctx->q_data[V4L2_M2M_SRC].sizeimage =
-- 
2.17.1

Re: [PATCH v5 01/23] vb2: add requires_requests bit for stateless codecs

[Paul Kocialkowski]

Hi,

On Wed, 2019-03-06 at 13:13 -0800, Dafna Hirschfeld wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Stateless codecs require the use of the Request API as opposed of it
> being optional.
> 
> So add a bit to indicate this and let vb2 check for this.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 5 ++++-
>  drivers/media/common/videobuf2/videobuf2-v4l2.c | 6 ++++++
>  include/media/videobuf2-core.h                  | 3 +++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 15b6b9c0a2e4..d8cf9d3ec54d 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1518,7 +1518,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
>  
>  	if ((req && q->uses_qbuf) ||
>  	    (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
> -	     q->uses_requests)) {
> +	     (q->uses_requests || q->requires_requests))) {
>  		dprintk(1, "queue in wrong mode (qbuf vs requests)\n");
>  		return -EBUSY;
>  	}
> @@ -2247,6 +2247,9 @@ int vb2_core_queue_init(struct vb2_queue *q)
>  	    WARN_ON(!q->ops->buf_queue))
>  		return -EINVAL;
>  
> +	if (WARN_ON(q->requires_requests && !q->supports_requests))
> +		return -EINVAL;
> +
>  	INIT_LIST_HEAD(&q->queued_list);
>  	INIT_LIST_HEAD(&q->done_list);
>  	spin_lock_init(&q->done_lock);
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index d09dee20e421..4dc4855056f1 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -385,6 +385,10 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
>  			dprintk(1, "%s: queue uses requests\n", opname);
>  			return -EBUSY;
>  		}
> +		if (q->requires_requests) {
> +			dprintk(1, "%s: queue requires requests\n", opname);
> +			return -EACCES;
> +		}
>  		return 0;
>  	} else if (!q->supports_requests) {
>  		dprintk(1, "%s: queue does not support requests\n", opname);
> @@ -658,6 +662,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
>  #ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API
>  	if (q->supports_requests)
>  		*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
> +	if (q->requires_requests)
> +		*caps |= V4L2_BUF_CAP_REQUIRES_REQUESTS;
>  #endif
>  }
>  
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 910f3d469005..fbf8dbbcbc09 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -484,6 +484,8 @@ 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.
> + * @requires_requests: this queue requires the Request API. If this is set to 1,
> + *		then supports_requests must be set to 1 as well.
>   * @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.
> @@ -558,6 +560,7 @@ struct vb2_queue {
>  	unsigned			allow_zero_bytesused:1;
>  	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
>  	unsigned			supports_requests:1;
> +	unsigned			requires_requests:1;
>  	unsigned			uses_qbuf:1;
>  	unsigned			uses_requests:1;
>  
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Paul Kocialkowski]

Hi,

On Wed, 2019-03-06 at 13:13 -0800, Dafna Hirschfeld wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Add capability to indicate that requests are required instead of
> merely supported.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 4 ++++
>  include/uapi/linux/videodev2.h                  | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> index d7faef10e39b..d42a3d9a7db3 100644
> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> @@ -125,6 +125,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-REQUIRES-REQUESTS:
>  
>  .. cssclass:: longtable
>  
> @@ -150,6 +151,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_REQUIRES_REQUESTS``
> +      - 0x00000020
> +      - This buffer type requires the use of :ref:`requests <media-request-api>`.
>  
>  Return Value
>  ============
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 1db220da3bcc..97e6a6a968ba 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
>  
>  /**
>   * struct v4l2_plane - plane info for multi-planar buffers
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH v5 03/23] cedrus: set requires_requests

[Paul Kocialkowski]

Hi,

On Wed, 2019-03-06 at 13:13 -0800, Dafna Hirschfeld wrote:
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> The cedrus stateless decoder requires the use of request, so
> indicate this by setting requires_requests to 1.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Note that this is true for now, but we might need to get rid of the
flag when adding support for decoding JPEG, which may not require the
request API.

Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_video.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index b47854b3bce4..9673874ece10 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -536,6 +536,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->requires_requests = true;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH v5 06/23] media: v4l2-ctrl: v4l2_ctrl_request_setup returns with error upon failure

[Paul Kocialkowski]

Hi Dafna,

On Wed, 2019-03-06 at 13:13 -0800, Dafna Hirschfeld wrote:
> If one of the controls fails to set,
> then 'v4l2_ctrl_request_setup'
> immediately returns with the error code.

Very neat idea, the error should definitely be propagated!

We should update the cedrus driver to take this into account, marking
both src/dst buffers as error (as suggested by Hans on IRC).

> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 18 +++++++++++-------
>  include/media/v4l2-ctrls.h           |  2 +-
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index b79d3bbd8350..54d66dbc2a31 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -3899,18 +3899,19 @@ void v4l2_ctrl_request_complete(struct media_request *req,
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_request_complete);
>  
> -void v4l2_ctrl_request_setup(struct media_request *req,
> +int v4l2_ctrl_request_setup(struct media_request *req,
>  			     struct v4l2_ctrl_handler *main_hdl)
>  {
>  	struct media_request_object *obj;
>  	struct v4l2_ctrl_handler *hdl;
>  	struct v4l2_ctrl_ref *ref;
> +	int ret = 0;
>  
>  	if (!req || !main_hdl)
> -		return;
> +		return 0;
>  
>  	if (WARN_ON(req->state != MEDIA_REQUEST_STATE_QUEUED))
> -		return;
> +		return -EBUSY;
>  
>  	/*
>  	 * Note that it is valid if nothing was found. It means
> @@ -3919,10 +3920,10 @@ void v4l2_ctrl_request_setup(struct media_request *req,
>  	 */
>  	obj = media_request_object_find(req, &req_ops, main_hdl);
>  	if (!obj)
> -		return;
> +		return 0;
>  	if (obj->completed) {
>  		media_request_object_put(obj);
> -		return;
> +		return -EBUSY;
>  	}
>  	hdl = container_of(obj, struct v4l2_ctrl_handler, req_obj);
>  
> @@ -3990,12 +3991,15 @@ void v4l2_ctrl_request_setup(struct media_request *req,
>  				update_from_auto_cluster(master);
>  		}
>  
> -		try_or_set_cluster(NULL, master, true, 0);
> -
> +		ret = try_or_set_cluster(NULL, master, true, 0);
>  		v4l2_ctrl_unlock(master);
> +
> +		if (ret)
> +			break;
>  	}
>  
>  	media_request_object_put(obj);
> +	return ret;
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_request_setup);
>  
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index e5cae37ced2d..200f8a66ecaa 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -1127,7 +1127,7 @@ __poll_t v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait);
>   * applying control values in a request is only applicable to memory-to-memory
>   * devices.
>   */
> -void v4l2_ctrl_request_setup(struct media_request *req,
> +int v4l2_ctrl_request_setup(struct media_request *req,
>  			     struct v4l2_ctrl_handler *parent);
>  
>  /**
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH v5 22/23] media: vicodec: Add support for stateless decoder.

[Hans Verkuil]

On 3/6/19 10:13 PM, Dafna Hirschfeld wrote:
> Implement a stateless decoder for the new node.
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---

<snip>

> +static int vicodec_try_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct vicodec_ctx *ctx = container_of(ctrl->handler,
> +			struct vicodec_ctx, hdl);
> +	const struct v4l2_ctrl_fwht_params *params;
> +	struct vicodec_q_data *q_dst = get_q_data(ctx,
> +			V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_MPEG_VIDEO_FWHT_PARAMS:
> +		if (!q_dst->info)
> +			return -EINVAL;
> +		params = ctrl->p_new.p_fwht_params;
> +		if (params->width > q_dst->coded_width ||
> +		    params->width < MIN_WIDTH ||
> +		    params->height > q_dst->coded_height ||
> +		    params->height < MIN_HEIGHT)
> +			return -EINVAL;
> +		if (!validate_by_version(params->flags, params->version))
> +			return -EINVAL;
> +		if (!validate_stateless_params_flags(params, q_dst->info))
> +			return -EINVAL;

There should be a return 0; statement here rather than fall through.

I'll make this trivial change when I prepare the pull request for this
patch series, so you don't need to do anything Dafna.

Regards,

	Hans

Re: [PATCH v5 21/23] media: vicodec: Register another node for stateless decoder

[Hans Verkuil]

On 3/6/19 10:13 PM, Dafna Hirschfeld wrote:
> Add stateless decoder instance field to the dev struct and
> register another node for the statelsess decoder.
> The stateless API for the node will be implemented in further patches.
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
>  drivers/media/platform/vicodec/vicodec-core.c | 46 +++++++++++++++++--
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 6c9a41838d31..7733b22247b6 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -104,6 +104,7 @@ struct vicodec_dev {
>  	struct v4l2_device	v4l2_dev;
>  	struct vicodec_dev_instance stateful_enc;
>  	struct vicodec_dev_instance stateful_dec;
> +	struct vicodec_dev_instance stateless_dec;
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	struct media_device	mdev;
>  #endif
> @@ -114,6 +115,7 @@ struct vicodec_ctx {
>  	struct v4l2_fh		fh;
>  	struct vicodec_dev	*dev;
>  	bool			is_enc;
> +	bool			is_stateless;
>  	spinlock_t		*lock;
>  
>  	struct v4l2_ctrl_handler hdl;
> @@ -373,6 +375,9 @@ static void device_run(void *priv)
>  
>  	if (ctx->is_enc)
>  		v4l2_m2m_job_finish(dev->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
> +	else if (ctx->is_stateless)
> +		v4l2_m2m_job_finish(dev->stateless_dec.m2m_dev,
> +				    ctx->fh.m2m_ctx);
>  	else
>  		v4l2_m2m_job_finish(dev->stateful_dec.m2m_dev, ctx->fh.m2m_ctx);
>  }
> @@ -1494,8 +1499,14 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->ops = &vicodec_qops;
>  	src_vq->mem_ops = &vb2_vmalloc_memops;
>  	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> -	src_vq->lock = ctx->is_enc ? &ctx->dev->stateful_enc.mutex :
> -		&ctx->dev->stateful_dec.mutex;
> +	if (ctx->is_enc)
> +		src_vq->lock = &ctx->dev->stateful_enc.mutex;
> +	else if (ctx->is_stateless)
> +		src_vq->lock = &ctx->dev->stateless_dec.mutex;
> +	else
> +		src_vq->lock = &ctx->dev->stateful_dec.mutex;
> +	src_vq->supports_requests = ctx->is_stateless;
> +	src_vq->requires_requests = ctx->is_stateless;
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
>  		return ret;
> @@ -1564,6 +1575,8 @@ static int vicodec_open(struct file *file)
>  
>  	if (vfd == &dev->stateful_enc.vfd)
>  		ctx->is_enc = true;
> +	else if (vfd == &dev->stateless_dec.vfd)
> +		ctx->is_stateless = true;
>  
>  	v4l2_fh_init(&ctx->fh, video_devdata(file));
>  	file->private_data = &ctx->fh;
> @@ -1576,6 +1589,8 @@ static int vicodec_open(struct file *file)
>  			  1, 31, 1, 20);
>  	v4l2_ctrl_new_std(hdl, &vicodec_ctrl_ops, V4L2_CID_FWHT_P_FRAME_QP,
>  			  1, 31, 1, 20);
> +	if (ctx->is_stateless)
> +		v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_stateless_state, NULL);
>  	if (hdl->error) {
>  		rc = hdl->error;
>  		v4l2_ctrl_handler_free(hdl);
> @@ -1615,6 +1630,10 @@ static int vicodec_open(struct file *file)
>  		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_enc.m2m_dev,
>  						    ctx, &queue_init);
>  		ctx->lock = &dev->stateful_enc.lock;
> +	} else if (ctx->is_stateless) {
> +		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateless_dec.m2m_dev,
> +						    ctx, &queue_init);
> +		ctx->lock = &dev->stateless_dec.lock;
>  	} else {
>  		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_dec.m2m_dev,
>  						    ctx, &queue_init);
> @@ -1751,6 +1770,10 @@ static int vicodec_probe(struct platform_device *pdev)
>  			      "stateful-decoder", false))
>  		goto unreg_sf_enc;
>  
> +	if (register_instance(dev, &dev->stateless_dec,
> +			      "videdev-stateless-dec", false))

Typo: this should be "stateless-decoder".

I'll make this change when I prepare the pull request.

Regards,

	Hans

> +		goto unreg_sf_dec;
> +
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	ret = v4l2_m2m_register_media_controller(dev->stateful_enc.m2m_dev,
>  						 &dev->stateful_enc.vfd,
> @@ -1768,23 +1791,36 @@ static int vicodec_probe(struct platform_device *pdev)
>  		goto unreg_m2m_sf_enc_mc;
>  	}
>  
> +	ret = v4l2_m2m_register_media_controller(dev->stateless_dec.m2m_dev,
> +						 &dev->stateless_dec.vfd,
> +						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
> +	if (ret) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for stateless dec\n");
> +		goto unreg_m2m_sf_dec_mc;
> +	}
> +
>  	ret = media_device_register(&dev->mdev);
>  	if (ret) {
>  		v4l2_err(&dev->v4l2_dev, "Failed to register mem2mem media device\n");
> -		goto unreg_m2m_sf_dec_mc;
> +		goto unreg_m2m_sl_dec_mc;
>  	}
>  #endif
>  	return 0;
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
> +unreg_m2m_sl_dec_mc:
> +	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
>  unreg_m2m_sf_dec_mc:
>  	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
>  unreg_m2m_sf_enc_mc:
>  	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
>  unreg_m2m:
> +	video_unregister_device(&dev->stateless_dec.vfd);
> +	v4l2_m2m_release(dev->stateless_dec.m2m_dev);
> +#endif
> +unreg_sf_dec:
>  	video_unregister_device(&dev->stateful_dec.vfd);
>  	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
> -#endif
>  unreg_sf_enc:
>  	video_unregister_device(&dev->stateful_enc.vfd);
>  	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
> @@ -1804,6 +1840,7 @@ static int vicodec_remove(struct platform_device *pdev)
>  	media_device_unregister(&dev->mdev);
>  	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
>  	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
> +	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
>  	media_device_cleanup(&dev->mdev);
>  #endif
>  
> @@ -1811,6 +1848,7 @@ static int vicodec_remove(struct platform_device *pdev)
>  	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
>  	video_unregister_device(&dev->stateful_enc.vfd);
>  	video_unregister_device(&dev->stateful_dec.vfd);
> +	video_unregister_device(&dev->stateless_dec.vfd);
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
>  	return 0;
> 

[PATCH v5 24/23] v4l2-ioctl.c: add V4L2_PIX_FMT_FWHT_STATELESS to v4l_fill_fmtdesc

[Hans Verkuil]

Add V4L2_PIX_FMT_FWHT_STATELESS to the list of pixelformats that
v4l_fill_fmtdesc() understands.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
Note: this patch should come after patch 20/23.
---
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index f6d663934648..10133f9e27c3 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1337,6 +1337,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 		case V4L2_PIX_FMT_VP9:		descr = "VP9"; break;
 		case V4L2_PIX_FMT_HEVC:		descr = "HEVC"; break; /* aka H.265 */
 		case V4L2_PIX_FMT_FWHT:		descr = "FWHT"; break; /* used in vicodec */
+		case V4L2_PIX_FMT_FWHT_STATELESS:	descr = "FWHT Stateless"; break; /* used in vicodec */
 		case V4L2_PIX_FMT_CPIA1:	descr = "GSPCA CPiA YUV"; break;
 		case V4L2_PIX_FMT_WNVA:		descr = "WNVA"; break;
 		case V4L2_PIX_FMT_SN9C10X:	descr = "GSPCA SN9C10X"; break;

Re: [PATCH v5 03/23] cedrus: set requires_requests

[Hans Verkuil]

On 3/12/19 4:32 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Wed, 2019-03-06 at 13:13 -0800, Dafna Hirschfeld wrote:
>> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>
>> The cedrus stateless decoder requires the use of request, so
>> indicate this by setting requires_requests to 1.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Note that this is true for now, but we might need to get rid of the
> flag when adding support for decoding JPEG, which may not require the
> request API.

I thought about this some more, and the flag can just be set or cleared
whenever a new format is set. I.e. when JPEG is selected, then both the
supports_requests and requires_requests flags can be set to false, and
set to true again when a non-JPEG format is set.

Regards,

	Hans

> 
> Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> 
> Cheers,
> 
> Paul
> 
>> ---
>>  drivers/staging/media/sunxi/cedrus/cedrus_video.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> index b47854b3bce4..9673874ece10 100644
>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> @@ -536,6 +536,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->requires_requests = true;
>>  
>>  	ret = vb2_queue_init(src_vq);
>>  	if (ret)

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Mauro Carvalho Chehab]

Em Wed,  6 Mar 2019 13:13:22 -0800
Dafna Hirschfeld <dafna3@gmail.com> escreveu:

> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Add capability to indicate that requests are required instead of
> merely supported.

Not sure if I liked this patch, and for sure it lacks a lot of documentation:

First of all, the patch description doesn't help. For example, it doesn't
explain or mention any use case example that would require (instead of
merely support) a request.

> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 4 ++++
>  include/uapi/linux/videodev2.h                  | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> index d7faef10e39b..d42a3d9a7db3 100644
> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> @@ -125,6 +125,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-REQUIRES-REQUESTS:
>  
>  .. cssclass:: longtable
>  
> @@ -150,6 +151,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_REQUIRES_REQUESTS``
> +      - 0x00000020
> +      - This buffer type requires the use of :ref:`requests <media-request-api>`.

And the documentation here is really poor, as it doesn't explain what's
the API and drivers expected behavior with regards to this flag.

I mean, if, on a new driver, requests are mandatory, what happens if a
non-request-API aware application tries to use it? 

Another thing that concerns me a lot is that people might want to add it
to existing drivers. Well, if an application was written before the
addition of this driver, and request API become mandatory, such app
will stop working, if it doesn't use request API.

At very least, it should be mentioned somewhere that existing drivers
should never set this flag, as this would break it for existing
userspace apps.

Still, I would prefer to not have to add something like that.


>  
>  Return Value
>  ============
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 1db220da3bcc..97e6a6a968ba 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
>  
>  /**
>   * struct v4l2_plane - plane info for multi-planar buffers



Thanks,
Mauro

Re: [PATCH v5 01/23] vb2: add requires_requests bit for stateless codecs

[Mauro Carvalho Chehab]

Em Wed,  6 Mar 2019 13:13:21 -0800
Dafna Hirschfeld <dafna3@gmail.com> escreveu:

> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Stateless codecs require the use of the Request API as opposed of it
> being optional.
> 
> So add a bit to indicate this and let vb2 check for this.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 5 ++++-
>  drivers/media/common/videobuf2/videobuf2-v4l2.c | 6 ++++++
>  include/media/videobuf2-core.h                  | 3 +++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 15b6b9c0a2e4..d8cf9d3ec54d 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1518,7 +1518,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
>  
>  	if ((req && q->uses_qbuf) ||
>  	    (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
> -	     q->uses_requests)) {
> +	     (q->uses_requests || q->requires_requests))) {
>  		dprintk(1, "queue in wrong mode (qbuf vs requests)\n");
>  		return -EBUSY;

Huh? -EBUSY doesn't seem the right error code to be issued if a driver
ignores V4L2_BUF_CAP_REQUIRES_REQUESTS.

>  	}
> @@ -2247,6 +2247,9 @@ int vb2_core_queue_init(struct vb2_queue *q)
>  	    WARN_ON(!q->ops->buf_queue))
>  		return -EINVAL;
>  
> +	if (WARN_ON(q->requires_requests && !q->supports_requests))
> +		return -EINVAL;
> +
>  	INIT_LIST_HEAD(&q->queued_list);
>  	INIT_LIST_HEAD(&q->done_list);
>  	spin_lock_init(&q->done_lock);
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index d09dee20e421..4dc4855056f1 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -385,6 +385,10 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
>  			dprintk(1, "%s: queue uses requests\n", opname);
>  			return -EBUSY;
>  		}
> +		if (q->requires_requests) {
> +			dprintk(1, "%s: queue requires requests\n", opname);
> +			return -EACCES;

I also don't think that -EACCES is the right error. This is not a matter of
wrong permissions. Running the app as root won't make it work.

> +		}
>  		return 0;
>  	} else if (!q->supports_requests) {
>  		dprintk(1, "%s: queue does not support requests\n", opname);
> @@ -658,6 +662,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
>  #ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API
>  	if (q->supports_requests)
>  		*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
> +	if (q->requires_requests)
> +		*caps |= V4L2_BUF_CAP_REQUIRES_REQUESTS;
>  #endif
>  }
>  
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 910f3d469005..fbf8dbbcbc09 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -484,6 +484,8 @@ 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.
> + * @requires_requests: this queue requires the Request API. If this is set to 1,
> + *		then supports_requests must be set to 1 as well.
>   * @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.
> @@ -558,6 +560,7 @@ struct vb2_queue {
>  	unsigned			allow_zero_bytesused:1;
>  	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
>  	unsigned			supports_requests:1;
> +	unsigned			requires_requests:1;
>  	unsigned			uses_qbuf:1;
>  	unsigned			uses_requests:1;
>  



Thanks,
Mauro

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Hans Verkuil]

On 3/20/19 11:11 AM, Mauro Carvalho Chehab wrote:
> Em Wed,  6 Mar 2019 13:13:22 -0800
> Dafna Hirschfeld <dafna3@gmail.com> escreveu:
> 
>> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>
>> Add capability to indicate that requests are required instead of
>> merely supported.
> 
> Not sure if I liked this patch, and for sure it lacks a lot of documentation:
> 
> First of all, the patch description doesn't help. For example, it doesn't
> explain or mention any use case example that would require (instead of
> merely support) a request.

Stateless codecs require the use of requests (i.e. they can't function without
this).

However, right now every driver has to check for this and return an error when
an attempt is made to stream without requests.

And userspace has no way of knowing whether requests are required by the driver
as opposed to being optional.

That's what this attempts to do: show to userspace that requests are required,
and add a vb2 flag that will force the core to check this so drivers do not need
to test for it.

Currently the only drivers that would need this are cedrus and vicodec.

> 
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 4 ++++
>>  include/uapi/linux/videodev2.h                  | 1 +
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> index d7faef10e39b..d42a3d9a7db3 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> @@ -125,6 +125,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-REQUIRES-REQUESTS:
>>  
>>  .. cssclass:: longtable
>>  
>> @@ -150,6 +151,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_REQUIRES_REQUESTS``
>> +      - 0x00000020
>> +      - This buffer type requires the use of :ref:`requests <media-request-api>`.
> 
> And the documentation here is really poor, as it doesn't explain what's
> the API and drivers expected behavior with regards to this flag.
> 
> I mean, if, on a new driver, requests are mandatory, what happens if a
> non-request-API aware application tries to use it? 

An error will be returned. And that error needs to be documented, I agree.

All this does is shift the check from the driver to the v4l2 core. It doesn't
change anything for userspace, except that with this capability flag userspace
can detect beforehand that requests are required.

> 
> Another thing that concerns me a lot is that people might want to add it
> to existing drivers. Well, if an application was written before the
> addition of this driver, and request API become mandatory, such app
> will stop working, if it doesn't use request API.
> At very least, it should be mentioned somewhere that existing drivers
> should never set this flag, as this would break it for existing
> userspace apps.
> 
> Still, I would prefer to not have to add something like that.

The only affected driver is the staging cedrus driver. And that will
actually crash if you try to use it without requests.

If you look at patch 3 you'll see that it just sets the flag and doesn't
remove any code that was supposed to check for use-without-requests.
That's because there never was a check and the driver would just crash.

So we're safe here.

I believe patches 1-3 make sense, but I also agree that the documentation
and commit logs can be improved.

I can either respin with updated patches 1-3, or, if you still have concerns,
drop 1-3 and repost the remainder of the series. But then I'll need to add
checks against the use of the stateless vicodec decoder without requests in
patch 21/23.

But this really doesn't belong in a driver. These checks should be done in the
vb2 core.

Regards,

	Hans

> 
> 
>>  
>>  Return Value
>>  ============
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 1db220da3bcc..97e6a6a968ba 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
>>  
>>  /**
>>   * struct v4l2_plane - plane info for multi-planar buffers
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH v5 01/23] vb2: add requires_requests bit for stateless codecs

[Hans Verkuil]

On 3/20/19 11:21 AM, Mauro Carvalho Chehab wrote:
> Em Wed,  6 Mar 2019 13:13:21 -0800
> Dafna Hirschfeld <dafna3@gmail.com> escreveu:
> 
>> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>
>> Stateless codecs require the use of the Request API as opposed of it
>> being optional.
>>
>> So add a bit to indicate this and let vb2 check for this.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  drivers/media/common/videobuf2/videobuf2-core.c | 5 ++++-
>>  drivers/media/common/videobuf2/videobuf2-v4l2.c | 6 ++++++
>>  include/media/videobuf2-core.h                  | 3 +++
>>  3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
>> index 15b6b9c0a2e4..d8cf9d3ec54d 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-core.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
>> @@ -1518,7 +1518,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
>>  
>>  	if ((req && q->uses_qbuf) ||
>>  	    (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
>> -	     q->uses_requests)) {
>> +	     (q->uses_requests || q->requires_requests))) {
>>  		dprintk(1, "queue in wrong mode (qbuf vs requests)\n");
>>  		return -EBUSY;
> 
> Huh? -EBUSY doesn't seem the right error code to be issued if a driver
> ignores V4L2_BUF_CAP_REQUIRES_REQUESTS.

I agree. See my next comment below.

> 
>>  	}
>> @@ -2247,6 +2247,9 @@ int vb2_core_queue_init(struct vb2_queue *q)
>>  	    WARN_ON(!q->ops->buf_queue))
>>  		return -EINVAL;
>>  
>> +	if (WARN_ON(q->requires_requests && !q->supports_requests))
>> +		return -EINVAL;
>> +
>>  	INIT_LIST_HEAD(&q->queued_list);
>>  	INIT_LIST_HEAD(&q->done_list);
>>  	spin_lock_init(&q->done_lock);
>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> index d09dee20e421..4dc4855056f1 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> @@ -385,6 +385,10 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
>>  			dprintk(1, "%s: queue uses requests\n", opname);
>>  			return -EBUSY;
>>  		}
>> +		if (q->requires_requests) {
>> +			dprintk(1, "%s: queue requires requests\n", opname);
>> +			return -EACCES;
> 
> I also don't think that -EACCES is the right error. This is not a matter of
> wrong permissions. Running the app as root won't make it work.

I believe EPERM is returned if you have the wrong permissions.

EACCES is returned when you are in the wrong mode, e.g. when attempting
to set a read-only control, or read from a write-only control.

So I believe this is indeed the right error code. And I also agree that the
core code above should return EACCES as well in this particular case instead
of EBUSY.

Regards,

	Hans

> 
>> +		}
>>  		return 0;
>>  	} else if (!q->supports_requests) {
>>  		dprintk(1, "%s: queue does not support requests\n", opname);
>> @@ -658,6 +662,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
>>  #ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API
>>  	if (q->supports_requests)
>>  		*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
>> +	if (q->requires_requests)
>> +		*caps |= V4L2_BUF_CAP_REQUIRES_REQUESTS;
>>  #endif
>>  }
>>  
>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>> index 910f3d469005..fbf8dbbcbc09 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -484,6 +484,8 @@ 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.
>> + * @requires_requests: this queue requires the Request API. If this is set to 1,
>> + *		then supports_requests must be set to 1 as well.
>>   * @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.
>> @@ -558,6 +560,7 @@ struct vb2_queue {
>>  	unsigned			allow_zero_bytesused:1;
>>  	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
>>  	unsigned			supports_requests:1;
>> +	unsigned			requires_requests:1;
>>  	unsigned			uses_qbuf:1;
>>  	unsigned			uses_requests:1;
>>  
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Mauro Carvalho Chehab]

Em Wed, 20 Mar 2019 11:39:47 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 3/20/19 11:11 AM, Mauro Carvalho Chehab wrote:
> > Em Wed,  6 Mar 2019 13:13:22 -0800
> > Dafna Hirschfeld <dafna3@gmail.com> escreveu:
> >   
> >> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> >>
> >> Add capability to indicate that requests are required instead of
> >> merely supported.  
> > 
> > Not sure if I liked this patch, and for sure it lacks a lot of documentation:
> > 
> > First of all, the patch description doesn't help. For example, it doesn't
> > explain or mention any use case example that would require (instead of
> > merely support) a request.  
> 
> Stateless codecs require the use of requests (i.e. they can't function without
> this).
> 
> However, right now every driver has to check for this and return an error when
> an attempt is made to stream without requests.
> 
> And userspace has no way of knowing whether requests are required by the driver
> as opposed to being optional.
> 
> That's what this attempts to do: show to userspace that requests are required,
> and add a vb2 flag that will force the core to check this so drivers do not need
> to test for it.
> 
> Currently the only drivers that would need this are cedrus and vicodec.

I see. Please add a comment like that at this patch's description.

> 
> >   
> >>
> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> >> ---
> >>  Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 4 ++++
> >>  include/uapi/linux/videodev2.h                  | 1 +
> >>  2 files changed, 5 insertions(+)
> >>
> >> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> >> index d7faef10e39b..d42a3d9a7db3 100644
> >> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> >> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> >> @@ -125,6 +125,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-REQUIRES-REQUESTS:
> >>  
> >>  .. cssclass:: longtable
> >>  
> >> @@ -150,6 +151,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_REQUIRES_REQUESTS``
> >> +      - 0x00000020
> >> +      - This buffer type requires the use of :ref:`requests <media-request-api>`.  
> > 
> > And the documentation here is really poor, as it doesn't explain what's
> > the API and drivers expected behavior with regards to this flag.
> > 
> > I mean, if, on a new driver, requests are mandatory, what happens if a
> > non-request-API aware application tries to use it?   
> 
> An error will be returned. And that error needs to be documented, I agree.

As discussed at the #v4l channel, EBADR error code seems to be an
appropriate error code for it. Please document it.

> 
> All this does is shift the check from the driver to the v4l2 core. It doesn't
> change anything for userspace, except that with this capability flag userspace
> can detect beforehand that requests are required.

Yeah, checking at the core makes sense.

> 
> > 
> > Another thing that concerns me a lot is that people might want to add it
> > to existing drivers. Well, if an application was written before the
> > addition of this driver, and request API become mandatory, such app
> > will stop working, if it doesn't use request API.
> > At very least, it should be mentioned somewhere that existing drivers
> > should never set this flag, as this would break it for existing
> > userspace apps.
> > 
> > Still, I would prefer to not have to add something like that.  
> 
> The only affected driver is the staging cedrus driver. And that will
> actually crash if you try to use it without requests.
> 
> If you look at patch 3 you'll see that it just sets the flag and doesn't
> remove any code that was supposed to check for use-without-requests.
> That's because there never was a check and the driver would just crash.
> 
> So we're safe here.

Making it mandatory for the cedrus driver makes sense, but no other
current driver should ever use it. 

The problem I see is that, as we advance on improving the requests API,
non-stateless-codec drivers may end supporting the request API. 
That's perfectly fine, but such other drivers should *never* be
changed to use V4L2_BUF_CAP_REQUIRES_REQUESTS. This also applies to any
new driver that it is not implementing a stateless codec.

Btw, as this seems to be a requirement only for stateless codec drivers,
perhaps we should (at least in Kernelspace) to use, instead, a
V4L2_BUF_CAP_STATELESS_CODEC_ONLY flag, with the V4L2 core would
translate it to V4L2_BUF_CAP_REQUIRES_REQUESTS before returning it to
userspace, and have a special #ifdef at the userspace header, in order
to prevent this flag to be set directly by a random driver.

> 
> I believe patches 1-3 make sense, but I also agree that the documentation
> and commit logs can be improved.
> 
> I can either respin with updated patches 1-3, or, if you still have concerns,
> drop 1-3 and repost the remainder of the series. But then I'll need to add
> checks against the use of the stateless vicodec decoder without requests in
> patch 21/23.

Whatever you prefer. If the remaining patches don't require it, you could
just tag the pull request as new and ping me on IRC. I'll review the remaining
ones, skipping the V4L2_BUF_CAP_REQUIRES_REQUESTS specific patches.

> 
> But this really doesn't belong in a driver. These checks should be done in the
> vb2 core.

Yeah, I agree.

> 
> Regards,
> 
> 	Hans
> 
> > 
> >   
> >>  
> >>  Return Value
> >>  ============
> >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> >> index 1db220da3bcc..97e6a6a968ba 100644
> >> --- a/include/uapi/linux/videodev2.h
> >> +++ b/include/uapi/linux/videodev2.h
> >> @@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
> >>  
> >>  /**
> >>   * struct v4l2_plane - plane info for multi-planar buffers  
> > 
> > 
> > 
> > Thanks,
> > Mauro
> >   
> 



Thanks,
Mauro

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Hans Verkuil]

On 3/20/19 12:42 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 20 Mar 2019 11:39:47 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 3/20/19 11:11 AM, Mauro Carvalho Chehab wrote:
>>> Em Wed,  6 Mar 2019 13:13:22 -0800
>>> Dafna Hirschfeld <dafna3@gmail.com> escreveu:
>>>   
>>>> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>>>
>>>> Add capability to indicate that requests are required instead of
>>>> merely supported.  
>>>
>>> Not sure if I liked this patch, and for sure it lacks a lot of documentation:
>>>
>>> First of all, the patch description doesn't help. For example, it doesn't
>>> explain or mention any use case example that would require (instead of
>>> merely support) a request.  
>>
>> Stateless codecs require the use of requests (i.e. they can't function without
>> this).
>>
>> However, right now every driver has to check for this and return an error when
>> an attempt is made to stream without requests.
>>
>> And userspace has no way of knowing whether requests are required by the driver
>> as opposed to being optional.
>>
>> That's what this attempts to do: show to userspace that requests are required,
>> and add a vb2 flag that will force the core to check this so drivers do not need
>> to test for it.
>>
>> Currently the only drivers that would need this are cedrus and vicodec.
> 
> I see. Please add a comment like that at this patch's description.
> 
>>
>>>   
>>>>
>>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>>>> ---
>>>>  Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 4 ++++
>>>>  include/uapi/linux/videodev2.h                  | 1 +
>>>>  2 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>>>> index d7faef10e39b..d42a3d9a7db3 100644
>>>> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>>>> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>>>> @@ -125,6 +125,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-REQUIRES-REQUESTS:
>>>>  
>>>>  .. cssclass:: longtable
>>>>  
>>>> @@ -150,6 +151,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_REQUIRES_REQUESTS``
>>>> +      - 0x00000020
>>>> +      - This buffer type requires the use of :ref:`requests <media-request-api>`.  
>>>
>>> And the documentation here is really poor, as it doesn't explain what's
>>> the API and drivers expected behavior with regards to this flag.
>>>
>>> I mean, if, on a new driver, requests are mandatory, what happens if a
>>> non-request-API aware application tries to use it?   
>>
>> An error will be returned. And that error needs to be documented, I agree.
> 
> As discussed at the #v4l channel, EBADR error code seems to be an
> appropriate error code for it. Please document it.
> 
>>
>> All this does is shift the check from the driver to the v4l2 core. It doesn't
>> change anything for userspace, except that with this capability flag userspace
>> can detect beforehand that requests are required.
> 
> Yeah, checking at the core makes sense.
> 
>>
>>>
>>> Another thing that concerns me a lot is that people might want to add it
>>> to existing drivers. Well, if an application was written before the
>>> addition of this driver, and request API become mandatory, such app
>>> will stop working, if it doesn't use request API.
>>> At very least, it should be mentioned somewhere that existing drivers
>>> should never set this flag, as this would break it for existing
>>> userspace apps.
>>>
>>> Still, I would prefer to not have to add something like that.  
>>
>> The only affected driver is the staging cedrus driver. And that will
>> actually crash if you try to use it without requests.
>>
>> If you look at patch 3 you'll see that it just sets the flag and doesn't
>> remove any code that was supposed to check for use-without-requests.
>> That's because there never was a check and the driver would just crash.
>>
>> So we're safe here.
> 
> Making it mandatory for the cedrus driver makes sense, but no other
> current driver should ever use it. 

The only other drivers that implement the request API are vivid and vim2m.

For both the request API is optional.

And of course this patch series that adds the stateless decoder support to
vicodec, so vicodec is the only other driver besides the cedrus driver that
sets this flag.

> The problem I see is that, as we advance on improving the requests API,
> non-stateless-codec drivers may end supporting the request API. 
> That's perfectly fine, but such other drivers should *never* be
> changed to use V4L2_BUF_CAP_REQUIRES_REQUESTS. This also applies to any
> new driver that it is not implementing a stateless codec.
> 
> Btw, as this seems to be a requirement only for stateless codec drivers,
> perhaps we should (at least in Kernelspace) to use, instead, a
> V4L2_BUF_CAP_STATELESS_CODEC_ONLY flag, with the V4L2 core would
> translate it to V4L2_BUF_CAP_REQUIRES_REQUESTS before returning it to
> userspace, and have a special #ifdef at the userspace header, in order
> to prevent this flag to be set directly by a random driver.

I don't think this makes sense. Requiring requests is not something you
can miss since you have to code for it.

However, there is something else that we need to think about and that is
that V4L2_BUF_CAP_REQUIRES_REQUESTS can be format specific. E.g. a stateless
codec driver can also support a JPEG codec, and for that format requests
are most likely not required at all. So this capability might actually be
format-specific.

I've decided to drop the patch adding this capability flag. The vb2
requires_requests flag remains, as does the EBADR error code + updated
documentation for that error code, since that is still needed. But signaling
to userspace that it is required is something we can add later when we have
a bit more time to think about it.

I'll respin and repost the series.

Regards,

	Hans

> 
>>
>> I believe patches 1-3 make sense, but I also agree that the documentation
>> and commit logs can be improved.
>>
>> I can either respin with updated patches 1-3, or, if you still have concerns,
>> drop 1-3 and repost the remainder of the series. But then I'll need to add
>> checks against the use of the stateless vicodec decoder without requests in
>> patch 21/23.
> 
> Whatever you prefer. If the remaining patches don't require it, you could
> just tag the pull request as new and ping me on IRC. I'll review the remaining
> ones, skipping the V4L2_BUF_CAP_REQUIRES_REQUESTS specific patches.
> 
>>
>> But this really doesn't belong in a driver. These checks should be done in the
>> vb2 core.
> 
> Yeah, I agree.
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>>   
>>>>  
>>>>  Return Value
>>>>  ============
>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>> index 1db220da3bcc..97e6a6a968ba 100644
>>>> --- a/include/uapi/linux/videodev2.h
>>>> +++ b/include/uapi/linux/videodev2.h
>>>> @@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
>>>>  
>>>>  /**
>>>>   * struct v4l2_plane - plane info for multi-planar buffers  
>>>
>>>
>>>
>>> Thanks,
>>> Mauro
>>>   
>>
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Mauro Carvalho Chehab]

Em Wed, 20 Mar 2019 13:20:07 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> >> The only affected driver is the staging cedrus driver. And that will
> >> actually crash if you try to use it without requests.
> >>
> >> If you look at patch 3 you'll see that it just sets the flag and doesn't
> >> remove any code that was supposed to check for use-without-requests.
> >> That's because there never was a check and the driver would just crash.
> >>
> >> So we're safe here.  
> > 
> > Making it mandatory for the cedrus driver makes sense, but no other
> > current driver should ever use it.   
> 
> The only other drivers that implement the request API are vivid and vim2m.
> 
> For both the request API is optional.
> 
> And of course this patch series that adds the stateless decoder support to
> vicodec, so vicodec is the only other driver besides the cedrus driver that
> sets this flag.

The current vicodec implementation is only stateless?

> > The problem I see is that, as we advance on improving the requests API,
> > non-stateless-codec drivers may end supporting the request API. 
> > That's perfectly fine, but such other drivers should *never* be
> > changed to use V4L2_BUF_CAP_REQUIRES_REQUESTS. This also applies to any
> > new driver that it is not implementing a stateless codec.
> > 
> > Btw, as this seems to be a requirement only for stateless codec drivers,
> > perhaps we should (at least in Kernelspace) to use, instead, a
> > V4L2_BUF_CAP_STATELESS_CODEC_ONLY flag, with the V4L2 core would
> > translate it to V4L2_BUF_CAP_REQUIRES_REQUESTS before returning it to
> > userspace, and have a special #ifdef at the userspace header, in order
> > to prevent this flag to be set directly by a random driver.  
> 
> I don't think this makes sense. Requiring requests is not something you
> can miss since you have to code for it.
> 
> However, there is something else that we need to think about and that is
> that V4L2_BUF_CAP_REQUIRES_REQUESTS can be format specific. E.g. a stateless
> codec driver can also support a JPEG codec, and for that format requests
> are most likely not required at all. So this capability might actually be
> format-specific.

Yes, on formats that don't have temporal compression, there's no sense
to make request API mandatory.

For formats that have temporal compression, the codec driver can either 
be stateless or stateful (or even support both modes).

It sounds to me that a flag like that should be returned by S_FMT and
TRY_FMT or on a separate ioctl.

It also seems to make sense if userspace could select between stateless
and stateful modes, if the driver supports both modes for the same
fourcc.

> I've decided to drop the patch adding this capability flag. The vb2
> requires_requests flag remains, as does the EBADR error code + updated
> documentation for that error code, since that is still needed. But signaling
> to userspace that it is required is something we can add later when we have
> a bit more time to think about it.

Ok.

> 
> I'll respin and repost the series.
> 
> Regards,
> 
> 	Hans
> 
> >   
> >>
> >> I believe patches 1-3 make sense, but I also agree that the documentation
> >> and commit logs can be improved.
> >>
> >> I can either respin with updated patches 1-3, or, if you still have concerns,
> >> drop 1-3 and repost the remainder of the series. But then I'll need to add
> >> checks against the use of the stateless vicodec decoder without requests in
> >> patch 21/23.  
> > 
> > Whatever you prefer. If the remaining patches don't require it, you could
> > just tag the pull request as new and ping me on IRC. I'll review the remaining
> > ones, skipping the V4L2_BUF_CAP_REQUIRES_REQUESTS specific patches.
> >   
> >>
> >> But this really doesn't belong in a driver. These checks should be done in the
> >> vb2 core.  
> > 
> > Yeah, I agree.
> >   
> >>
> >> Regards,
> >>
> >> 	Hans
> >>  
> >>>
> >>>     
> >>>>  
> >>>>  Return Value
> >>>>  ============
> >>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> >>>> index 1db220da3bcc..97e6a6a968ba 100644
> >>>> --- a/include/uapi/linux/videodev2.h
> >>>> +++ b/include/uapi/linux/videodev2.h
> >>>> @@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
> >>>>  
> >>>>  /**
> >>>>   * struct v4l2_plane - plane info for multi-planar buffers    
> >>>
> >>>
> >>>
> >>> Thanks,
> >>> Mauro
> >>>     
> >>  
> > 
> > 
> > 
> > Thanks,
> > Mauro
> >   
> 



Thanks,
Mauro

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Hans Verkuil]

On 3/20/19 1:37 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 20 Mar 2019 13:20:07 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>>>> The only affected driver is the staging cedrus driver. And that will
>>>> actually crash if you try to use it without requests.
>>>>
>>>> If you look at patch 3 you'll see that it just sets the flag and doesn't
>>>> remove any code that was supposed to check for use-without-requests.
>>>> That's because there never was a check and the driver would just crash.
>>>>
>>>> So we're safe here.  
>>>
>>> Making it mandatory for the cedrus driver makes sense, but no other
>>> current driver should ever use it.   
>>
>> The only other drivers that implement the request API are vivid and vim2m.
>>
>> For both the request API is optional.
>>
>> And of course this patch series that adds the stateless decoder support to
>> vicodec, so vicodec is the only other driver besides the cedrus driver that
>> sets this flag.
> 
> The current vicodec implementation is only stateless?

vicodec before this series is only stateful. After this series a new video node
is added which is for the stateless decoder. And that device will require
requests.

> 
>>> The problem I see is that, as we advance on improving the requests API,
>>> non-stateless-codec drivers may end supporting the request API. 
>>> That's perfectly fine, but such other drivers should *never* be
>>> changed to use V4L2_BUF_CAP_REQUIRES_REQUESTS. This also applies to any
>>> new driver that it is not implementing a stateless codec.
>>>
>>> Btw, as this seems to be a requirement only for stateless codec drivers,
>>> perhaps we should (at least in Kernelspace) to use, instead, a
>>> V4L2_BUF_CAP_STATELESS_CODEC_ONLY flag, with the V4L2 core would
>>> translate it to V4L2_BUF_CAP_REQUIRES_REQUESTS before returning it to
>>> userspace, and have a special #ifdef at the userspace header, in order
>>> to prevent this flag to be set directly by a random driver.  
>>
>> I don't think this makes sense. Requiring requests is not something you
>> can miss since you have to code for it.
>>
>> However, there is something else that we need to think about and that is
>> that V4L2_BUF_CAP_REQUIRES_REQUESTS can be format specific. E.g. a stateless
>> codec driver can also support a JPEG codec, and for that format requests
>> are most likely not required at all. So this capability might actually be
>> format-specific.
> 
> Yes, on formats that don't have temporal compression, there's no sense
> to make request API mandatory.
> 
> For formats that have temporal compression, the codec driver can either 
> be stateless or stateful (or even support both modes).
> 
> It sounds to me that a flag like that should be returned by S_FMT and
> TRY_FMT or on a separate ioctl.
> 
> It also seems to make sense if userspace could select between stateless
> and stateful modes, if the driver supports both modes for the same
> fourcc.

That can't happen. The stateless formats have their own fourcc. It really
is a different format.

Regards,

	Hans

> 
>> I've decided to drop the patch adding this capability flag. The vb2
>> requires_requests flag remains, as does the EBADR error code + updated
>> documentation for that error code, since that is still needed. But signaling
>> to userspace that it is required is something we can add later when we have
>> a bit more time to think about it.
> 
> Ok.
> 
>>
>> I'll respin and repost the series.
>>
>> Regards,
>>
>> 	Hans
>>
>>>   
>>>>
>>>> I believe patches 1-3 make sense, but I also agree that the documentation
>>>> and commit logs can be improved.
>>>>
>>>> I can either respin with updated patches 1-3, or, if you still have concerns,
>>>> drop 1-3 and repost the remainder of the series. But then I'll need to add
>>>> checks against the use of the stateless vicodec decoder without requests in
>>>> patch 21/23.  
>>>
>>> Whatever you prefer. If the remaining patches don't require it, you could
>>> just tag the pull request as new and ping me on IRC. I'll review the remaining
>>> ones, skipping the V4L2_BUF_CAP_REQUIRES_REQUESTS specific patches.
>>>   
>>>>
>>>> But this really doesn't belong in a driver. These checks should be done in the
>>>> vb2 core.  
>>>
>>> Yeah, I agree.
>>>   
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>  
>>>>>
>>>>>     
>>>>>>  
>>>>>>  Return Value
>>>>>>  ============
>>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>>>> index 1db220da3bcc..97e6a6a968ba 100644
>>>>>> --- a/include/uapi/linux/videodev2.h
>>>>>> +++ b/include/uapi/linux/videodev2.h
>>>>>> @@ -895,6 +895,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_REQUIRES_REQUESTS	(1 << 5)
>>>>>>  
>>>>>>  /**
>>>>>>   * struct v4l2_plane - plane info for multi-planar buffers    
>>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Mauro
>>>>>     
>>>>  
>>>
>>>
>>>
>>> Thanks,
>>> Mauro
>>>   
>>
> 
> 
> 
> Thanks,
> Mauro
> 

Re: [PATCH v5 02/23] videodev2.h: add V4L2_BUF_CAP_REQUIRES_REQUESTS

[Mauro Carvalho Chehab]

Em Wed, 20 Mar 2019 13:41:42 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 3/20/19 1:37 PM, Mauro Carvalho Chehab wrote:
> > Em Wed, 20 Mar 2019 13:20:07 +0100
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >   
> >>>> The only affected driver is the staging cedrus driver. And that will
> >>>> actually crash if you try to use it without requests.
> >>>>
> >>>> If you look at patch 3 you'll see that it just sets the flag and doesn't
> >>>> remove any code that was supposed to check for use-without-requests.
> >>>> That's because there never was a check and the driver would just crash.
> >>>>
> >>>> So we're safe here.    
> >>>
> >>> Making it mandatory for the cedrus driver makes sense, but no other
> >>> current driver should ever use it.     
> >>
> >> The only other drivers that implement the request API are vivid and vim2m.
> >>
> >> For both the request API is optional.
> >>
> >> And of course this patch series that adds the stateless decoder support to
> >> vicodec, so vicodec is the only other driver besides the cedrus driver that
> >> sets this flag.  
> > 
> > The current vicodec implementation is only stateless?  
> 
> vicodec before this series is only stateful. After this series a new video node
> is added which is for the stateless decoder. And that device will require
> requests.

I see. see below.

> 
> >   
> >>> The problem I see is that, as we advance on improving the requests API,
> >>> non-stateless-codec drivers may end supporting the request API. 
> >>> That's perfectly fine, but such other drivers should *never* be
> >>> changed to use V4L2_BUF_CAP_REQUIRES_REQUESTS. This also applies to any
> >>> new driver that it is not implementing a stateless codec.
> >>>
> >>> Btw, as this seems to be a requirement only for stateless codec drivers,
> >>> perhaps we should (at least in Kernelspace) to use, instead, a
> >>> V4L2_BUF_CAP_STATELESS_CODEC_ONLY flag, with the V4L2 core would
> >>> translate it to V4L2_BUF_CAP_REQUIRES_REQUESTS before returning it to
> >>> userspace, and have a special #ifdef at the userspace header, in order
> >>> to prevent this flag to be set directly by a random driver.    
> >>
> >> I don't think this makes sense. Requiring requests is not something you
> >> can miss since you have to code for it.
> >>
> >> However, there is something else that we need to think about and that is
> >> that V4L2_BUF_CAP_REQUIRES_REQUESTS can be format specific. E.g. a stateless
> >> codec driver can also support a JPEG codec, and for that format requests
> >> are most likely not required at all. So this capability might actually be
> >> format-specific.  
> > 
> > Yes, on formats that don't have temporal compression, there's no sense
> > to make request API mandatory.
> > 
> > For formats that have temporal compression, the codec driver can either 
> > be stateless or stateful (or even support both modes).
> > 
> > It sounds to me that a flag like that should be returned by S_FMT and
> > TRY_FMT or on a separate ioctl.
> > 
> > It also seems to make sense if userspace could select between stateless
> > and stateful modes, if the driver supports both modes for the same
> > fourcc.  
> 
> That can't happen. The stateless formats have their own fourcc. It really
> is a different format.

Well, if we're already defining different fourcc for the stateless
codecs, then I think that your proposed patch:

	[PATCH v5.1 2/2] cedrus: set requires_requests

Is not the best way to implement it.

	Instead, I would have a helper function like:

static int v4l2_format_requires_request_api(uint32 fourcc)
{
	switch(fourcc) {
	case V4L2_PIX_FMT_MPEG2_SLICE:
		return 1;
	default:
		return 0;
	}
}

called by V4L2 core at S_FMT handler in order to set vq->requires_requests.

Also, in this case, I would add a V4L2_FMT_FLAG_REQUIRE_REQUEST_API or
V4L2_FMT_FLAG_STATELESS_CODEC flag at VIDIOC_ENUM_FMT in order to
indicate it.

Thanks,
Mauro