[PATCHv2 00/12] Add support for read-only requests

[PATCHv2 00/12] Add support for read-only requests

[Hans Verkuil]

Read-only requests do not contain any configuration values when queued
(only buffers can be part of a read-only request), but they do allow the
driver to associate additional information with the completed request.

This is useful to e.g. add per-frame metadata such as HDMI InfoFrames to
a captured buffer.

While working on this I discovered that the Request API is also missing a
feature: if userspace did not set any controls, then the request object
will not contain a control object (that's created only if the user sets a
control in the request).

This is fine for e.g. stateless codecs since they require that each request
contains controls, so this is always done. And this is also the reason that
this hasn't been a problem before, since the Request API is almost exclusively
used by stateless codecs.

But for e.g. vivid this means that the completed request does not contain
any controls in the request with the values of the time that the frame was
captured (or output).

In addition, if a driver needs to set a status control, then that control
won't be part of the request either.

This series adds a v4l2_ctrl_request_add_handler() function that can be
called in the req_validate() callback of the request. If the request
doesn't contain a control object, then it will add a new one.

This series adds read-only request support to vivid, vim2m and vicodec, and
adds new helper functions to vb2 (vb2_request_buffer_first) and v4l2-mem2mem.c
(v4l2_m2m_request_validate).

In addition, the first patch of this series adds debugfs support to the
media device to be able to keep track of outstanding requests and request
objects. Without this it is next to impossible to check if all requests and
request objects are properly released after all file handles are closed.

This series supersedes this old RFC series:

https://patchwork.linuxtv.org/project/linux-media/cover/20200728094851.121933-1-hverkuil-cisco@xs4all.nl/

I'll also post an RFC patch for v4l-utils that adds support for this to
v4l2-ctl and v4l2-compliance.

Regards,

	Hans


Hans Verkuil (12):
  media/mc: keep track of outstanding requests
  vivid: add read-only int32 control
  media: document read-only requests
  videodev2.h: add V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS
  videobuf2-core: add vb2_request_buffer_first()
  v4l2-ctrls.c: add v4l2_ctrl_request_add_handler
  vivid: call v4l2_ctrl_request_add_handler()
  vivid: add ro_requests module option
  v4l2-mem2mem.c: add v4l2_m2m_request_validate()
  vim2m: use v4l2_m2m_request_validate()
  vim2m: support read-only requests on the capture queue
  vicodec: add support for read-only requests

 Documentation/admin-guide/media/vivid.rst     | 10 +++
 .../mediactl/media-request-ioc-queue.rst      |  5 ++
 .../media/mediactl/request-api.rst            | 11 +++
 .../media/v4l/vidioc-reqbufs.rst              |  6 ++
 .../media/common/videobuf2/videobuf2-core.c   | 22 ++++++
 .../media/common/videobuf2/videobuf2-v4l2.c   |  4 +-
 drivers/media/mc/mc-device.c                  | 27 +++++++
 drivers/media/mc/mc-devnode.c                 | 13 ++++
 drivers/media/mc/mc-request.c                 |  8 ++-
 .../media/test-drivers/vicodec/vicodec-core.c | 70 +++++++++----------
 drivers/media/test-drivers/vim2m.c            | 13 +++-
 drivers/media/test-drivers/vivid/vivid-core.c | 52 ++++++++++++++
 drivers/media/test-drivers/vivid/vivid-core.h |  1 +
 .../media/test-drivers/vivid/vivid-ctrls.c    | 13 ++++
 .../test-drivers/vivid/vivid-kthread-cap.c    | 10 +--
 drivers/media/v4l2-core/v4l2-ctrls.c          | 35 ++++++++++
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 45 ++++++++++--
 include/media/media-device.h                  |  9 +++
 include/media/media-devnode.h                 |  4 ++
 include/media/media-request.h                 |  2 +
 include/media/v4l2-ctrls.h                    | 21 ++++++
 include/media/v4l2-mem2mem.h                  | 28 +++++++-
 include/media/videobuf2-core.h                |  8 +++
 include/uapi/linux/videodev2.h                |  1 +
 24 files changed, 368 insertions(+), 50 deletions(-)

-- 
2.27.0

[PATCHv2 01/12] media/mc: keep track of outstanding requests

[Hans Verkuil]

It can be hard to debug if all requests and request objects are correctly
cleaned up when all related filehandles are closed.

This patch adds a new 'requests' debugfs entry if CONFIG_VIDEO_ADV_DEBUG
is set to report the number of open requests and request objects for a
given media device node.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/mc/mc-device.c  | 27 +++++++++++++++++++++++++++
 drivers/media/mc/mc-devnode.c | 13 +++++++++++++
 drivers/media/mc/mc-request.c |  5 +++++
 include/media/media-device.h  |  9 +++++++++
 include/media/media-devnode.h |  4 ++++
 include/media/media-request.h |  2 ++
 6 files changed, 60 insertions(+)

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index da8088351135..6953e86b59bf 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -690,6 +690,23 @@ void media_device_unregister_entity(struct media_entity *entity)
 }
 EXPORT_SYMBOL_GPL(media_device_unregister_entity);
 
+#if IS_ENABLED(CONFIG_DEBUG_FS) && IS_ENABLED(CONFIG_VIDEO_ADV_DEBUG)
+/*
+ * Log the state of media requests.
+ * Very useful for debugging.
+ */
+static int media_device_requests(struct seq_file *file, void *priv)
+{
+	struct media_device *dev = dev_get_drvdata(file->private);
+
+	seq_printf(file, "number of requests: %d\n",
+		   atomic_read(&dev->num_requests));
+	seq_printf(file, "number of request objects: %d\n",
+		   atomic_read(&dev->num_request_objects));
+	return 0;
+}
+#endif
+
 /**
  * media_device_init() - initialize a media device
  * @mdev:	The media device
@@ -713,6 +730,8 @@ void media_device_init(struct media_device *mdev)
 	ida_init(&mdev->entity_internal_idx);
 
 	atomic_set(&mdev->request_id, 0);
+	atomic_set(&mdev->num_requests, 0);
+	atomic_set(&mdev->num_request_objects, 0);
 
 	dev_dbg(mdev->dev, "Media device initialized\n");
 }
@@ -765,6 +784,13 @@ int __must_check __media_device_register(struct media_device *mdev,
 
 	dev_dbg(mdev->dev, "Media device registered\n");
 
+#if IS_ENABLED(CONFIG_DEBUG_FS) && IS_ENABLED(CONFIG_VIDEO_ADV_DEBUG)
+	mdev->media_dir = debugfs_create_dir(dev_name(&devnode->dev),
+					     media_top_dir);
+	debugfs_create_devm_seqfile(&devnode->dev, "requests",
+				    mdev->media_dir, media_device_requests);
+#endif
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__media_device_register);
@@ -842,6 +868,7 @@ void media_device_unregister(struct media_device *mdev)
 
 	dev_dbg(mdev->dev, "Media device unregistered\n");
 
+	debugfs_remove_recursive(mdev->media_dir);
 	device_remove_file(&mdev->devnode->dev, &dev_attr_model);
 	media_devnode_unregister(mdev->devnode);
 	/* devnode free is handled in media_devnode_*() */
diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
index f11382afe23b..cb229556c989 100644
--- a/drivers/media/mc/mc-devnode.c
+++ b/drivers/media/mc/mc-devnode.c
@@ -45,6 +45,12 @@ static dev_t media_dev_t;
 static DEFINE_MUTEX(media_devnode_lock);
 static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
 
+/*
+ * debugfs
+ */
+struct dentry *media_top_dir;
+
+
 /* Called when the last user of the media device exits. */
 static void media_devnode_release(struct device *cd)
 {
@@ -252,6 +258,8 @@ int __must_check media_devnode_register(struct media_device *mdev,
 		goto cdev_add_error;
 	}
 
+	dev_set_drvdata(&devnode->dev, mdev);
+
 	/* Part 4: Activate this minor. The char device can now be used. */
 	set_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
 
@@ -304,6 +312,10 @@ static int __init media_devnode_init(void)
 		return ret;
 	}
 
+#if IS_ENABLED(CONFIG_DEBUG_FS) && IS_ENABLED(CONFIG_VIDEO_ADV_DEBUG)
+	media_top_dir = debugfs_create_dir("media", NULL);
+#endif
+
 	ret = bus_register(&media_bus_type);
 	if (ret < 0) {
 		unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
@@ -316,6 +328,7 @@ static int __init media_devnode_init(void)
 
 static void __exit media_devnode_exit(void)
 {
+	debugfs_remove_recursive(media_top_dir);
 	bus_unregister(&media_bus_type);
 	unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
 }
diff --git a/drivers/media/mc/mc-request.c b/drivers/media/mc/mc-request.c
index c0782fd96c59..3b013deaeb06 100644
--- a/drivers/media/mc/mc-request.c
+++ b/drivers/media/mc/mc-request.c
@@ -74,6 +74,7 @@ static void media_request_release(struct kref *kref)
 		mdev->ops->req_free(req);
 	else
 		kfree(req);
+	atomic_dec(&mdev->num_requests);
 }
 
 void media_request_put(struct media_request *req)
@@ -330,6 +331,7 @@ int media_request_alloc(struct media_device *mdev, int *alloc_fd)
 
 	snprintf(req->debug_str, sizeof(req->debug_str), "%u:%d",
 		 atomic_inc_return(&mdev->request_id), fd);
+	atomic_inc(&mdev->num_requests);
 	dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str);
 
 	fd_install(fd, filp);
@@ -357,6 +359,7 @@ static void media_request_object_release(struct kref *kref)
 	if (WARN_ON(req))
 		media_request_object_unbind(obj);
 	obj->ops->release(obj);
+	atomic_dec(&obj->mdev->num_request_objects);
 }
 
 struct media_request_object *
@@ -420,6 +423,7 @@ int media_request_object_bind(struct media_request *req,
 	obj->req = req;
 	obj->ops = ops;
 	obj->priv = priv;
+	obj->mdev = req->mdev;
 
 	if (is_buffer)
 		list_add_tail(&obj->list, &req->objects);
@@ -427,6 +431,7 @@ int media_request_object_bind(struct media_request *req,
 		list_add(&obj->list, &req->objects);
 	req->num_incomplete_objects++;
 	ret = 0;
+	atomic_inc(&obj->mdev->num_request_objects);
 
 unlock:
 	spin_unlock_irqrestore(&req->lock, flags);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 1345e6da688a..d95d2b2ef619 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -13,6 +13,7 @@
 
 #include <linux/list.h>
 #include <linux/mutex.h>
+#include <linux/atomic.h>
 
 #include <media/media-devnode.h>
 #include <media/media-entity.h>
@@ -106,6 +107,9 @@ struct media_device_ops {
  * @req_queue_mutex: Serialise the MEDIA_REQUEST_IOC_QUEUE ioctl w.r.t.
  *		     other operations that stop or start streaming.
  * @request_id: Used to generate unique request IDs
+ * @num_requests: number of associated requests
+ * @num_request_objects: number of associated request objects
+ * @media_dir:	DebugFS media directory
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
@@ -179,6 +183,11 @@ struct media_device {
 
 	struct mutex req_queue_mutex;
 	atomic_t request_id;
+	atomic_t num_requests;
+	atomic_t num_request_objects;
+
+	/* debugfs */
+	struct dentry *media_dir;
 };
 
 /* We don't need to include pci.h or usb.h here */
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index d27c1c646c28..ec47216420c7 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -20,9 +20,13 @@
 #include <linux/fs.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/debugfs.h>
 
 struct media_device;
 
+/* DebugFS top-level media directory */
+extern struct dentry *media_top_dir;
+
 /*
  * Flag to mark the media_devnode struct as registered. Drivers must not touch
  * this flag directly, it will be set and cleared by media_devnode_register and
diff --git a/include/media/media-request.h b/include/media/media-request.h
index 3cd25a2717ce..580594c9398d 100644
--- a/include/media/media-request.h
+++ b/include/media/media-request.h
@@ -256,6 +256,7 @@ struct media_request_object_ops {
  * struct media_request_object - An opaque object that belongs to a media
  *				 request
  *
+ * @mdev: Media device this object belongs to
  * @ops: object's operations
  * @priv: object's priv pointer
  * @req: the request this object belongs to (can be NULL)
@@ -267,6 +268,7 @@ struct media_request_object_ops {
  * another struct that contains the actual data for this request object.
  */
 struct media_request_object {
+	struct media_device *mdev;
 	const struct media_request_object_ops *ops;
 	void *priv;
 	struct media_request *req;
-- 
2.27.0

[PATCHv2 02/12] vivid: add read-only int32 control

[Hans Verkuil]

This read-only int32 control is used to test read-only controls in
combination with requests. It is set by the driver to the buffer sequence
counter module 256.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/test-drivers/vivid/vivid-core.h       |  1 +
 drivers/media/test-drivers/vivid/vivid-ctrls.c      | 13 +++++++++++++
 .../media/test-drivers/vivid/vivid-kthread-cap.c    | 10 ++++++----
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h
index 99e69b8f770f..058a12c6890e 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.h
+++ b/drivers/media/test-drivers/vivid/vivid-core.h
@@ -230,6 +230,7 @@ struct vivid_dev {
 	struct v4l2_ctrl		*string;
 	struct v4l2_ctrl		*bitmask;
 	struct v4l2_ctrl		*int_menu;
+	struct v4l2_ctrl		*ro_int32;
 	struct v4l2_ctrl		*test_pattern;
 	struct v4l2_ctrl		*colorspace;
 	struct v4l2_ctrl		*rgb_range_cap;
diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c
index 334130568dcb..f16910ceaa77 100644
--- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
+++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
@@ -33,6 +33,7 @@
 #define VIVID_CID_U16_MATRIX		(VIVID_CID_CUSTOM_BASE + 9)
 #define VIVID_CID_U8_4D_ARRAY		(VIVID_CID_CUSTOM_BASE + 10)
 #define VIVID_CID_AREA			(VIVID_CID_CUSTOM_BASE + 11)
+#define VIVID_CID_RO_INTEGER		(VIVID_CID_CUSTOM_BASE + 12)
 
 #define VIVID_CID_VIVID_BASE		(0x00f00000 | 0xf000)
 #define VIVID_CID_VIVID_CLASS		(0x00f00000 | 1)
@@ -280,6 +281,17 @@ static const struct v4l2_ctrl_config vivid_ctrl_area = {
 	.p_def.p_const = &area,
 };
 
+static const struct v4l2_ctrl_config vivid_ctrl_ro_int32 = {
+	.ops = &vivid_user_gen_ctrl_ops,
+	.id = VIVID_CID_RO_INTEGER,
+	.name = "Read-Only Integer 32 Bits",
+	.type = V4L2_CTRL_TYPE_INTEGER,
+	.flags = V4L2_CTRL_FLAG_READ_ONLY,
+	.min = 0,
+	.max = 255,
+	.step = 1,
+};
+
 /* Framebuffer Controls */
 
 static int vivid_fb_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -1590,6 +1602,7 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap,
 	dev->string = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_string, NULL);
 	dev->bitmask = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_bitmask, NULL);
 	dev->int_menu = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_int_menu, NULL);
+	dev->ro_int32 = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_ro_int32, NULL);
 	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_area, NULL);
 	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u32_array, NULL);
 	v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u16_matrix, NULL);
diff --git a/drivers/media/test-drivers/vivid/vivid-kthread-cap.c b/drivers/media/test-drivers/vivid/vivid-kthread-cap.c
index 01a9d671b947..cf7f62bfcf2f 100644
--- a/drivers/media/test-drivers/vivid/vivid-kthread-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-kthread-cap.c
@@ -426,6 +426,7 @@ static void vivid_fillbuff(struct vivid_dev *dev, struct vivid_buffer *buf)
 		is_loop = true;
 
 	buf->vb.sequence = dev->vid_cap_seq_count;
+	v4l2_ctrl_s_ctrl(dev->ro_int32, buf->vb.sequence & 0xff);
 	if (dev->field_cap == V4L2_FIELD_ALTERNATE) {
 		/*
 		 * 60 Hz standards start with the bottom field, 50 Hz standards
@@ -515,10 +516,11 @@ static void vivid_fillbuff(struct vivid_dev *dev, struct vivid_buffer *buf)
 		mutex_unlock(dev->ctrl_hdl_user_aud.lock);
 		tpg_gen_text(tpg, basep, line++ * line_height, 16, str);
 		mutex_lock(dev->ctrl_hdl_user_gen.lock);
-		snprintf(str, sizeof(str), " int32 %d, int64 %lld, bitmask %08x ",
-			dev->int32->cur.val,
-			*dev->int64->p_cur.p_s64,
-			dev->bitmask->cur.val);
+		snprintf(str, sizeof(str), " int32 %d, ro_int32 %d, int64 %lld, bitmask %08x ",
+			 dev->int32->cur.val,
+			 dev->ro_int32->cur.val,
+			 *dev->int64->p_cur.p_s64,
+			 dev->bitmask->cur.val);
 		tpg_gen_text(tpg, basep, line++ * line_height, 16, str);
 		snprintf(str, sizeof(str), " boolean %d, menu %s, string \"%s\" ",
 			dev->boolean->cur.val,
-- 
2.27.0

[PATCHv2 03/12] media: document read-only requests

[Hans Verkuil]

This patch documents reaed-only requests, including the new
V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS capability flag.

Based in part on a patch from Yunfei Dong <yunfei.dong@mediatek.com>.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../media/mediactl/media-request-ioc-queue.rst        |  5 +++++
 .../userspace-api/media/mediactl/request-api.rst      | 11 +++++++++++
 .../userspace-api/media/v4l/vidioc-reqbufs.rst        |  6 ++++++
 3 files changed, 22 insertions(+)

diff --git a/Documentation/userspace-api/media/mediactl/media-request-ioc-queue.rst b/Documentation/userspace-api/media/mediactl/media-request-ioc-queue.rst
index ad55b6b32616..4ed1b3585ce1 100644
--- a/Documentation/userspace-api/media/mediactl/media-request-ioc-queue.rst
+++ b/Documentation/userspace-api/media/mediactl/media-request-ioc-queue.rst
@@ -76,6 +76,11 @@ queued directly and you next try to queue a request, or vice versa.
 A request must contain at least one buffer, otherwise this ioctl will
 return an ``ENOENT`` error.
 
+If the :ref:`buffer capabilities <v4l2-buf-capabilities>` indicate that
+only read-only requests are supported, then the request can only contain
+buffers. If anything else is present then this ioctl will return an
+``EINVAL`` error.
+
 Return Value
 ============
 
diff --git a/Documentation/userspace-api/media/mediactl/request-api.rst b/Documentation/userspace-api/media/mediactl/request-api.rst
index 37d9442a541e..88bfe8b735e8 100644
--- a/Documentation/userspace-api/media/mediactl/request-api.rst
+++ b/Documentation/userspace-api/media/mediactl/request-api.rst
@@ -53,6 +53,13 @@ with different configurations in advance, knowing that the configuration will be
 applied when needed to get the expected result. Configuration values at the time
 of request completion are also available for reading.
 
+In some cases it does not make sense for user-space to associate configuration
+values with a frame, instead it only makes sense to retrieving configuration
+values at the time of request completion. In that case read-only requests can be
+used that only allow buffers to be queued and not configuration values. Which
+type of requests (regular or read-only) are supported is signalled through
+:ref:`buffer capabilities <v4l2-buf-capabilities>`.
+
 General Usage
 -------------
 
@@ -95,6 +102,10 @@ A queued request cannot be modified anymore.
    output buffers, not for capture buffers. Attempting to add a capture buffer
    to a request will result in an ``EBADR`` error.
 
+If the buffer type supports only read-only requests, and the request contains
+configuration values such as controls, then ``EINVAL`` is returned since no
+configuration values are allowed when submitting a read-only request.
+
 If the request contains configurations for multiple entities, individual drivers
 may synchronize so the requested pipeline's topology is applied before the
 buffers are processed. Media controller drivers do a best effort implementation
diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
index 75d894d9c36c..13454146d5fc 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst
@@ -134,6 +134,7 @@ aborting or finishing any DMA in progress, an implicit
 .. _V4L2-BUF-CAP-SUPPORTS-ORPHANED-BUFS:
 .. _V4L2-BUF-CAP-SUPPORTS-M2M-HOLD-CAPTURE-BUF:
 .. _V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS:
+.. _V4L2-BUF-CAP-SUPPORTS-RO-REQUESTS:
 
 .. cssclass:: longtable
 
@@ -154,6 +155,7 @@ aborting or finishing any DMA in progress, an implicit
     * - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
       - 0x00000008
       - This buffer type supports :ref:`requests <media-request-api>`.
+        This flag is mutually exclusive with ``V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS``.
     * - ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS``
       - 0x00000010
       - The kernel allows calling :ref:`VIDIOC_REQBUFS` while buffers are still
@@ -172,6 +174,10 @@ aborting or finishing any DMA in progress, an implicit
         :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT <V4L2-FLAG-MEMORY-NON-CONSISTENT>`,
         :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE <V4L2-BUF-FLAG-NO-CACHE-INVALIDATE>` and
         :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN <V4L2-BUF-FLAG-NO-CACHE-CLEAN>`.
+    * - ``V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS``
+      - 0x00000080
+      - This buffer type supports read-only :ref:`requests <media-request-api>`.
+        This flag is mutually exclusive with ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``.
 
 
 Return Value
-- 
2.27.0

[PATCHv2 04/12] videodev2.h: add V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS

[Hans Verkuil]

This patch adds support for the V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS
flag. This flag is used for Read-Only Requests.

Based on a patch from Yunfei Dong <yunfei.dong@mediatek.com>.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 3 +++
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 +++-
 drivers/media/v4l2-core/v4l2-mem2mem.c          | 3 ++-
 include/media/videobuf2-core.h                  | 1 +
 include/uapi/linux/videodev2.h                  | 1 +
 5 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index f544d3393e9d..5194056129de 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -2354,6 +2354,9 @@ int vb2_core_queue_init(struct vb2_queue *q)
 	if (WARN_ON(q->requires_requests && !q->supports_requests))
 		return -EINVAL;
 
+	if (WARN_ON(q->supports_ro_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 30caad27281e..9471320caaa0 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -717,7 +717,9 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
 	if (q->allow_cache_hints && q->io_modes & VB2_MMAP)
 		*caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS;
 #ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API
-	if (q->supports_requests)
+	if (q->supports_ro_requests)
+		*caps |= V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS;
+	else if (q->supports_requests)
 		*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
 #endif
 }
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 95a8f2dc5341..48f87cfe2f63 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -713,7 +713,8 @@ int v4l2_m2m_qbuf(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 
 	vq = v4l2_m2m_get_vq(m2m_ctx, buf->type);
 	if (V4L2_TYPE_IS_CAPTURE(vq->type) &&
-	    (buf->flags & V4L2_BUF_FLAG_REQUEST_FD)) {
+	    (buf->flags & V4L2_BUF_FLAG_REQUEST_FD) &&
+	    !vq->supports_ro_requests) {
 		dprintk("%s: requests cannot be used with capture buffers\n",
 			__func__);
 		return -EPERM;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 52ef92049073..af7ecff725a5 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -575,6 +575,7 @@ struct vb2_queue {
 	unsigned int			allow_zero_bytesused:1;
 	unsigned int		   quirk_poll_must_check_waiting_for_buffers:1;
 	unsigned int			supports_requests:1;
+	unsigned int			supports_ro_requests:1;
 	unsigned int			requires_requests:1;
 	unsigned int			uses_qbuf:1;
 	unsigned int			uses_requests:1;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c7b70ff53bc1..7226d6d3c1e4 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -963,6 +963,7 @@ struct v4l2_requestbuffers {
 #define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS		(1 << 4)
 #define V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF	(1 << 5)
 #define V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS		(1 << 6)
+#define V4L2_BUF_CAP_SUPPORTS_RO_REQUESTS		(1 << 7)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
-- 
2.27.0

[PATCHv2 05/12] videobuf2-core: add vb2_request_buffer_first()

[Hans Verkuil]

Add the vb2_request_buffer_first() helper function to obtain the
first vb2 buffer in the request.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../media/common/videobuf2/videobuf2-core.c   | 19 +++++++++++++++++++
 include/media/videobuf2-core.h                |  7 +++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 5194056129de..01ca0add39c2 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1500,6 +1500,25 @@ unsigned int vb2_request_buffer_cnt(struct media_request *req)
 }
 EXPORT_SYMBOL_GPL(vb2_request_buffer_cnt);
 
+struct vb2_buffer *vb2_request_buffer_first(struct media_request *req)
+{
+	struct media_request_object *obj;
+	struct vb2_buffer *vb = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&req->lock, flags);
+	list_for_each_entry(obj, &req->objects, list) {
+		if (vb2_request_object_is_buffer(obj)) {
+			vb = container_of(obj, struct vb2_buffer, req_obj);
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&req->lock, flags);
+
+	return vb;
+}
+EXPORT_SYMBOL_GPL(vb2_request_buffer_first);
+
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 {
 	struct vb2_buffer *vb;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index af7ecff725a5..58fbd9ef506a 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -1258,4 +1258,11 @@ bool vb2_request_object_is_buffer(struct media_request_object *obj);
  */
 unsigned int vb2_request_buffer_cnt(struct media_request *req);
 
+/**
+ * vb2_request_buffer_first() - return the first buffer in the request
+ *
+ * @req:	the request.
+ */
+struct vb2_buffer *vb2_request_buffer_first(struct media_request *req);
+
 #endif /* _MEDIA_VIDEOBUF2_CORE_H */
-- 
2.27.0

[PATCHv2 06/12] v4l2-ctrls.c: add v4l2_ctrl_request_add_handler

[Hans Verkuil]

Add a new v4l2_ctrl_request_add_handler() function that can be called in
req_validate() to add a control handler request object if needed.

This is needed if the driver needs to set controls in the request,

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/mc/mc-request.c        |  3 ++-
 drivers/media/v4l2-core/v4l2-ctrls.c | 35 ++++++++++++++++++++++++++++
 include/media/v4l2-ctrls.h           | 21 +++++++++++++++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/media/mc/mc-request.c b/drivers/media/mc/mc-request.c
index 3b013deaeb06..d66711964429 100644
--- a/drivers/media/mc/mc-request.c
+++ b/drivers/media/mc/mc-request.c
@@ -417,7 +417,8 @@ int media_request_object_bind(struct media_request *req,
 
 	spin_lock_irqsave(&req->lock, flags);
 
-	if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING))
+	if (WARN_ON(req->state != MEDIA_REQUEST_STATE_UPDATING &&
+		    req->state != MEDIA_REQUEST_STATE_VALIDATING))
 		goto unlock;
 
 	obj->req = req;
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 3f3fbcd60cc6..8a3f2b2027e4 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -4345,6 +4345,41 @@ void v4l2_ctrl_request_complete(struct media_request *req,
 }
 EXPORT_SYMBOL(v4l2_ctrl_request_complete);
 
+int v4l2_ctrl_request_add_handler(struct media_request *req,
+				  struct v4l2_ctrl_handler *main_hdl,
+				  bool is_ro_request)
+{
+	struct media_request_object *obj;
+	struct v4l2_ctrl_handler *new_hdl;
+	int ret = 0;
+
+	if (!req || !main_hdl)
+		return 0;
+
+	if (WARN_ON(req->state != MEDIA_REQUEST_STATE_VALIDATING))
+		return -EBUSY;
+
+	/* If a request object is found, then do nothing. */
+	obj = media_request_object_find(req, &req_ops, main_hdl);
+	if (obj) {
+		media_request_object_put(obj);
+		return is_ro_request ? -EINVAL : 0;
+	}
+
+	/* Create a new request so the driver can return controls */
+	new_hdl = kzalloc(sizeof(*new_hdl), GFP_KERNEL);
+	if (!new_hdl)
+		return -ENOMEM;
+
+	ret = v4l2_ctrl_handler_init(new_hdl, (main_hdl->nr_of_buckets - 1) * 8);
+	if (!ret)
+		ret = v4l2_ctrl_request_bind(req, new_hdl, main_hdl);
+	if (ret)
+		kfree(new_hdl);
+	return ret;
+}
+EXPORT_SYMBOL(v4l2_ctrl_request_add_handler);
+
 int v4l2_ctrl_request_setup(struct media_request *req,
 			     struct v4l2_ctrl_handler *main_hdl)
 {
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index f40e2cbb21d3..f0ee6f860798 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -1254,6 +1254,27 @@ int v4l2_ctrl_request_setup(struct media_request *req,
 void v4l2_ctrl_request_complete(struct media_request *req,
 				struct v4l2_ctrl_handler *parent);
 
+/**
+ * v4l2_ctrl_request_add_handler - Add a control handler request object
+ *
+ * @req: The request
+ * @parent: The parent control handler ('priv' in media_request_object_find())
+ * @is_ro_request: this is a read-only request
+ *
+ * If the user created a request without controls, but the driver wants to
+ * set controls for the request, then this function can be called in the
+ * req_validate function. If there is no control handler object in the
+ * request, then this will add one. Now the driver can set controls
+ * and when v4l2_ctrl_request_complete() is called they will be automatically
+ * copied into the request.
+ *
+ * If @is_ro_request is true, then if there *is* a control handler object
+ * in the request, then this function will return -EINVAL.
+ */
+int v4l2_ctrl_request_add_handler(struct media_request *req,
+				  struct v4l2_ctrl_handler *parent,
+				  bool is_ro_request);
+
 /**
  * v4l2_ctrl_request_hdl_find - Find the control handler in the request
  *
-- 
2.27.0

[PATCHv2 07/12] vivid: call v4l2_ctrl_request_add_handler()

[Hans Verkuil]

Call v4l2_ctrl_request_add_handler() from req_validate() to add the
control handler request object if needed.

Without this the returned request object would not have a copy of the
controls used for the captured frame.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/test-drivers/vivid/vivid-core.c | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index f7ee37e9508d..c21bc27bbfeb 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -783,6 +783,48 @@ static void vivid_dev_release(struct v4l2_device *v4l2_dev)
 static int vivid_req_validate(struct media_request *req)
 {
 	struct vivid_dev *dev = container_of(req->mdev, struct vivid_dev, mdev);
+	struct vb2_buffer *vb = vb2_request_buffer_first(req);
+	struct v4l2_ctrl_handler *hdl = NULL;
+	bool ro_req;
+	int ret;
+
+	if (!vb)
+		return -ENOENT;
+
+	switch (vb->vb2_queue->type) {
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		if (vb->vb2_queue == &dev->vb_touch_cap_q)
+			hdl = &dev->ctrl_hdl_touch_cap;
+		else
+			hdl = &dev->ctrl_hdl_vid_cap;
+		break;
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+		hdl = &dev->ctrl_hdl_vid_out;
+		break;
+	case V4L2_BUF_TYPE_VBI_CAPTURE:
+	case V4L2_BUF_TYPE_SLICED_VBI_CAPTURE:
+		hdl = &dev->ctrl_hdl_vbi_cap;
+		break;
+	case V4L2_BUF_TYPE_VBI_OUTPUT:
+	case V4L2_BUF_TYPE_SLICED_VBI_OUTPUT:
+		hdl = &dev->ctrl_hdl_vbi_out;
+		break;
+	case V4L2_BUF_TYPE_META_CAPTURE:
+		hdl = &dev->ctrl_hdl_meta_cap;
+		break;
+	case V4L2_BUF_TYPE_META_OUTPUT:
+		hdl = &dev->ctrl_hdl_meta_out;
+		break;
+	case V4L2_BUF_TYPE_SDR_CAPTURE:
+		hdl = &dev->ctrl_hdl_sdr_cap;
+		break;
+	}
+	ro_req = vb->vb2_queue->supports_ro_requests;
+	ret = v4l2_ctrl_request_add_handler(req, hdl, ro_req);
+	if (ret)
+		return ret;
 
 	if (dev->req_validate_error) {
 		dev->req_validate_error = false;
-- 
2.27.0

[PATCHv2 08/12] vivid: add ro_requests module option

[Hans Verkuil]

Add the ro_requests module option to test Read-Only Requests with vivid.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/admin-guide/media/vivid.rst     | 10 ++++++++++
 drivers/media/test-drivers/vivid/vivid-core.c | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/admin-guide/media/vivid.rst b/Documentation/admin-guide/media/vivid.rst
index 6d7175f96f74..423a61797a1d 100644
--- a/Documentation/admin-guide/media/vivid.rst
+++ b/Documentation/admin-guide/media/vivid.rst
@@ -302,6 +302,16 @@ all configurable using the following module options:
 		- 0: forbid hints
 		- 1: allow hints
 
+- ro_requests:
+
+	specifies if the capture device supports the standard Request API (i.e.
+	userspace can set controls in a request before queueing it), or
+	the Read-Only Request API (userspace can only read back controls after
+	the request was completed). Default is 0.
+
+		- 0: regular requests
+		- 1: read-only requests
+
 Taken together, all these module options allow you to precisely customize
 the driver behavior and test your application with all sorts of permutations.
 It is also very suitable to emulate hardware that is not yet available, e.g.
diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index c21bc27bbfeb..cc1510024b68 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -177,6 +177,14 @@ MODULE_PARM_DESC(cache_hints, " user-space cache hints, default is 0.\n"
 			     "\t\t    0 == forbid\n"
 			     "\t\t    1 == allow");
 
+static unsigned int ro_requests[VIVID_MAX_DEVS] = {
+	[0 ... (VIVID_MAX_DEVS - 1)] = 0
+};
+module_param_array(ro_requests, uint, NULL, 0444);
+MODULE_PARM_DESC(ro_requests, " use read-only requests instead of regular requests, default is 0.\n"
+			     "\t\t    0 == regular requests\n"
+			     "\t\t    1 == read-only requests");
+
 static struct vivid_dev *vivid_devs[VIVID_MAX_DEVS];
 
 const struct v4l2_rect vivid_min_rect = {
@@ -869,6 +877,8 @@ static int vivid_create_queue(struct vivid_dev *dev,
 	q->lock = &dev->mutex;
 	q->dev = dev->v4l2_dev.dev;
 	q->supports_requests = true;
+	if (V4L2_TYPE_IS_CAPTURE(buf_type))
+		q->supports_ro_requests = (ro_requests[dev->inst] == 1);
 	q->allow_cache_hints = (cache_hints[dev->inst] == 1);
 
 	return vb2_queue_init(q);
-- 
2.27.0

[PATCHv2 09/12] v4l2-mem2mem.c: add v4l2_m2m_request_validate()

[Hans Verkuil]

Add the new helper function v4l2_m2m_request_validate().

This function adds a control handler object if it is missing
in the request.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 42 +++++++++++++++++++++++---
 include/media/v4l2-mem2mem.h           | 28 ++++++++++++++++-
 2 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 48f87cfe2f63..b8d7746a0c21 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -20,6 +20,7 @@
 #include <media/v4l2-device.h>
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
+#include <media/v4l2-ctrls.h>
 
 MODULE_DESCRIPTION("Mem to mem device framework for videobuf");
 MODULE_AUTHOR("Pawel Osciak, <pawel@osciak.com>");
@@ -1230,6 +1231,35 @@ void v4l2_m2m_buf_copy_metadata(const struct vb2_v4l2_buffer *out_vb,
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_buf_copy_metadata);
 
+int v4l2_m2m_request_validate(struct media_request *req)
+{
+	struct vb2_buffer *vb = vb2_request_buffer_first(req);
+	struct v4l2_m2m_ctx *m2m_ctx;
+	int ret;
+
+	if (!vb)
+		return -ENOENT;
+
+	if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type))
+		m2m_ctx = container_of(vb->vb2_queue,
+				       struct v4l2_m2m_ctx,
+				       out_q_ctx.q);
+	else
+		m2m_ctx = container_of(vb->vb2_queue,
+				       struct v4l2_m2m_ctx,
+				       cap_q_ctx.q);
+
+	WARN_ON_ONCE(!m2m_ctx->req_ctrl_handler);
+
+	/*
+	 * Add a control handler object if it was missing in the request.
+	 */
+	ret = v4l2_ctrl_request_add_handler(req, m2m_ctx->req_ctrl_handler,
+					    vb->vb2_queue->supports_ro_requests);
+	return ret ? ret : vb2_request_validate(req);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_request_validate);
+
 void v4l2_m2m_request_queue(struct media_request *req)
 {
 	struct media_request_object *obj, *obj_safe;
@@ -1253,10 +1283,14 @@ void v4l2_m2m_request_queue(struct media_request *req)
 		if (vb2_request_object_is_buffer(obj)) {
 			/* Sanity checks */
 			vb = container_of(obj, struct vb2_buffer, req_obj);
-			WARN_ON(!V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type));
-			m2m_ctx_obj = container_of(vb->vb2_queue,
-						   struct v4l2_m2m_ctx,
-						   out_q_ctx.q);
+			if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type))
+				m2m_ctx_obj = container_of(vb->vb2_queue,
+							   struct v4l2_m2m_ctx,
+							   out_q_ctx.q);
+			else
+				m2m_ctx_obj = container_of(vb->vb2_queue,
+							   struct v4l2_m2m_ctx,
+							   cap_q_ctx.q);
 			WARN_ON(m2m_ctx && m2m_ctx_obj != m2m_ctx);
 			m2m_ctx = m2m_ctx_obj;
 		}
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 98753f00df7e..e1274d0550d0 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -84,6 +84,8 @@ struct v4l2_m2m_queue_ctx {
  * @last_src_buf: indicate the last source buffer for draining
  * @next_buf_last: next capture queud buffer will be tagged as last
  * @has_stopped: indicate the device has been stopped
+ * @req_ctrl_handler: the control handler to use with requests.
+ *	Must be set when using v4l2_m2m_request_validate().
  * @m2m_dev: opaque pointer to the internal data to handle M2M context
  * @cap_q_ctx: Capture (output to memory) queue context
  * @out_q_ctx: Output (input from memory) queue context
@@ -106,6 +108,7 @@ struct v4l2_m2m_ctx {
 	struct vb2_v4l2_buffer		*last_src_buf;
 	bool				next_buf_last;
 	bool				has_stopped;
+	struct v4l2_ctrl_handler	*req_ctrl_handler;
 
 	/* internal use only */
 	struct v4l2_m2m_dev		*m2m_dev;
@@ -809,8 +812,31 @@ void v4l2_m2m_buf_copy_metadata(const struct vb2_v4l2_buffer *out_vb,
 				struct vb2_v4l2_buffer *cap_vb,
 				bool copy_frame_flags);
 
-/* v4l2 request helper */
+/* v4l2 request helpers */
 
+/**
+ * v4l2_m2m_request_validate() - validate the request
+ *
+ * @req: the request
+ *
+ * This validates the request. If the request does not contain a
+ * control handler object, then this object is created. This function
+ * can be set as the media_device req_validate op. If the driver
+ * already requires (and checks) that one or more controls must be present
+ * in the request, then there is no need to use this function.
+ *
+ * If this helper is called, then the driver must set the @req_ctrl_handler
+ * field of struct v4l2_m2m_ctx.
+ */
+int v4l2_m2m_request_validate(struct media_request *req);
+
+/**
+ * v4l2_m2m_request_queue() - queue the request
+ *
+ * @req: the request
+ *
+ * Set this function as the media_device req_queue op.
+ */
 void v4l2_m2m_request_queue(struct media_request *req);
 
 /* v4l2 ioctl helpers */
-- 
2.27.0

[PATCHv2 10/12] vim2m: use v4l2_m2m_request_validate()

[Hans Verkuil]

Use v4l2_m2m_request_validate() instead of vb2_request_validate.
The v4l2_m2m_request_validate() ensures that a control handler
request object is added if it was missing.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/test-drivers/vim2m.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-drivers/vim2m.c
index a776bb8e0e09..b73de65c0006 100644
--- a/drivers/media/test-drivers/vim2m.c
+++ b/drivers/media/test-drivers/vim2m.c
@@ -1230,6 +1230,7 @@ static int vim2m_open(struct file *file)
 		goto open_unlock;
 	}
 
+	ctx->fh.m2m_ctx->req_ctrl_handler = hdl;
 	v4l2_fh_add(&ctx->fh);
 	atomic_inc(&dev->num_inst);
 
@@ -1299,7 +1300,7 @@ static const struct v4l2_m2m_ops m2m_ops = {
 };
 
 static const struct media_device_ops m2m_media_ops = {
-	.req_validate = vb2_request_validate,
+	.req_validate = v4l2_m2m_request_validate,
 	.req_queue = v4l2_m2m_request_queue,
 };
 
-- 
2.27.0

[PATCHv2 11/12] vim2m: support read-only requests on the capture queue

[Hans Verkuil]

Added support for read-only requests on the capture queue
in order to test this feature.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/test-drivers/vim2m.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-drivers/vim2m.c
index b73de65c0006..7c738b9fd637 100644
--- a/drivers/media/test-drivers/vim2m.c
+++ b/drivers/media/test-drivers/vim2m.c
@@ -602,7 +602,11 @@ static void device_run(void *priv)
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
-	/* Apply request controls if any */
+	/*
+	 * Apply request controls if any.
+	 * The dst_buf queue has read-only requests, so no need to
+	 * setup any controls for that buffer.
+	 */
 	v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
 				&ctx->hdl);
 
@@ -611,6 +615,8 @@ static void device_run(void *priv)
 	/* Complete request controls if any */
 	v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
 				   &ctx->hdl);
+	v4l2_ctrl_request_complete(dst_buf->vb2_buf.req_obj.req,
+				   &ctx->hdl);
 
 	/* Run delayed work, which simulates a hardware irq  */
 	schedule_delayed_work(&ctx->work_run, msecs_to_jiffies(ctx->transtime));
@@ -1143,6 +1149,8 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->mem_ops = &vb2_vmalloc_memops;
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	dst_vq->lock = &ctx->vb_mutex;
+	dst_vq->supports_requests = true;
+	dst_vq->supports_ro_requests = true;
 
 	return vb2_queue_init(dst_vq);
 }
-- 
2.27.0

[PATCHv2 12/12] vicodec: add support for read-only requests

[Hans Verkuil]

Add support for read-only requests for the stateless decoder capture
queue in order to test this feature.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 .../media/test-drivers/vicodec/vicodec-core.c | 70 +++++++++----------
 1 file changed, 34 insertions(+), 36 deletions(-)

diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
index 71928e30dae8..c377d5fdfd07 100644
--- a/drivers/media/test-drivers/vicodec/vicodec-core.c
+++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
@@ -267,6 +267,11 @@ static int device_process(struct vicodec_ctx *ctx,
 	if (ctx->is_stateless) {
 		struct media_request *src_req = src_vb->vb2_buf.req_obj.req;
 
+		/*
+		 * Apply request controls if any.
+		 * The dst_vb queue has read-only requests, so no need to
+		 * setup any controls for that buffer.
+		 */
 		ret = v4l2_ctrl_request_setup(src_req, &ctx->hdl);
 		if (ret)
 			return ret;
@@ -408,11 +413,12 @@ 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;
+	struct media_request *src_req, *dst_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;
+	dst_req = dst_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);
@@ -452,6 +458,8 @@ static void device_run(void *priv)
 	spin_unlock(ctx->lock);
 	if (ctx->is_stateless && src_req)
 		v4l2_ctrl_request_complete(src_req, &ctx->hdl);
+	if (ctx->is_stateless && dst_req)
+		v4l2_ctrl_request_complete(dst_req, &ctx->hdl);
 
 	if (ctx->is_enc)
 		v4l2_m2m_job_finish(dev->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
@@ -1733,6 +1741,8 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->mem_ops = &vb2_vmalloc_memops;
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	dst_vq->lock = src_vq->lock;
+	dst_vq->supports_requests = ctx->is_stateless;
+	dst_vq->supports_ro_requests = ctx->is_stateless;
 
 	return vb2_queue_init(dst_vq);
 }
@@ -1910,6 +1920,7 @@ static int vicodec_open(struct file *file)
 	} else if (ctx->is_stateless) {
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateless_dec.m2m_dev,
 						    ctx, &queue_init);
+		ctx->fh.m2m_ctx->req_ctrl_handler = hdl;
 		ctx->lock = &dev->stateless_dec.lock;
 	} else {
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_dec.m2m_dev,
@@ -1952,34 +1963,19 @@ static int vicodec_release(struct file *file)
 
 static int vicodec_request_validate(struct media_request *req)
 {
-	struct media_request_object *obj;
+	struct vb2_buffer *vb = vb2_request_buffer_first(req);
 	struct v4l2_ctrl_handler *parent_hdl, *hdl;
-	struct vicodec_ctx *ctx = NULL;
+	struct vicodec_ctx *ctx;
 	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");
+	if (!vb) {
+		dev_info(req->mdev->dev,
+			 "No buffer was provided with the request\n");
 		return -ENOENT;
 	}
+	ctx = vb2_get_drv_priv(vb->vb2_queue);
 
-	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) {
+	if (vb2_request_buffer_cnt(req) > 1) {
 		v4l2_info(&ctx->dev->v4l2_dev,
 			  "More than one buffer was provided with the request\n");
 		return -EINVAL;
@@ -1987,21 +1983,23 @@ static int vicodec_request_validate(struct media_request *req)
 
 	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);
-	v4l2_ctrl_request_hdl_put(hdl);
-	if (!ctrl) {
-		v4l2_info(&ctx->dev->v4l2_dev,
-			  "Missing required codec control\n");
-		return -ENOENT;
+	if (vb->vb2_queue->is_output) {
+		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);
+		v4l2_ctrl_request_hdl_put(hdl);
+		if (!ctrl) {
+			v4l2_info(&ctx->dev->v4l2_dev,
+				  "Missing required codec control\n");
+			return -ENOENT;
+		}
 	}
 
-	return vb2_request_validate(req);
+	return v4l2_m2m_request_validate(req);
 }
 
 static const struct v4l2_file_operations vicodec_fops = {
-- 
2.27.0