[PATCH 0/5] Post-v18: Request API updates

[PATCH 0/5] Post-v18: Request API updates

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Hi all,

This patch series sits on top of my v18 series for the Request API.
It makes some final (?) changes as discussed in:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg134419.html

and:

https://www.spinics.net/lists/linux-media/msg138596.html

The combined v18 patches + this series is available here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=reqv18-1

Updated v4l-utils for this is available here:

https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=request

Userspace visible changes:

- Invalid request_fd values now return -EINVAL instead of -ENOENT.
- It is no longer possible to use VIDIOC_G_EXT_CTRLS for requests
  that are not completed. -EACCES is returned in that case.

Driver visible changes (important for the cedrus driver!):

Drivers should set the new vb2_queue 'supports_request' bitfield to 1
if a vb2_queue can support requests. Otherwise the queue cannot be
used with requests.

This bitfield is also used to fill in the new capabilities field
in struct v4l2_requestbuffers and v4l2_create_buffers.

There will be a follow-up documentation patch incorporating
Laurent's comments, but that doesn't change any APIs.

Regards,

	Hans

Hans Verkuil (5):
  media-request: return -EINVAL for invalid request_fds
  v4l2-ctrls: return -EACCES if request wasn't completed
  buffer.rst: only set V4L2_BUF_FLAG_REQUEST_FD for QBUF
  videodev2.h: add new capabilities for buffer types
  vb2: set reqbufs/create_bufs capabilities

 Documentation/media/uapi/v4l/buffer.rst       |  6 ++--
 .../media/uapi/v4l/vidioc-create-bufs.rst     | 10 +++++-
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst     | 36 +++++++++----------
 Documentation/media/uapi/v4l/vidioc-qbuf.rst  | 12 +++----
 .../media/uapi/v4l/vidioc-reqbufs.rst         | 36 ++++++++++++++++++-
 .../media/common/videobuf2/videobuf2-v4l2.c   | 19 +++++++++-
 drivers/media/media-request.c                 |  6 ++--
 drivers/media/platform/vim2m.c                |  1 +
 drivers/media/platform/vivid/vivid-core.c     |  5 +++
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  4 ++-
 drivers/media/v4l2-core/v4l2-ctrls.c          |  5 ++-
 drivers/media/v4l2-core/v4l2-ioctl.c          |  4 +--
 include/media/videobuf2-core.h                |  2 ++
 include/uapi/linux/videodev2.h                | 13 +++++--
 14 files changed, 118 insertions(+), 41 deletions(-)

-- 
2.18.0

[PATCH 1/5] media-request: return -EINVAL for invalid request_fds

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Instead of returning -ENOENT when a request_fd was not found (VIDIOC_QBUF
and VIDIOC_G/S/TRY_EXT_CTRLS), we now return -EINVAL. This is in line
with what we do when invalid dmabuf fds are passed to e.g. VIDIOC_QBUF.

Also document that EINVAL is returned for invalid m.fd values, we never
documented that.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 Documentation/media/uapi/v4l/buffer.rst        |  4 ++--
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst      | 18 ++++++++----------
 Documentation/media/uapi/v4l/vidioc-qbuf.rst   | 12 +++++-------
 drivers/media/media-request.c                  |  6 ++++--
 4 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
index dd0065a95ea0..35c2fadd10de 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -313,8 +313,8 @@ struct v4l2_buffer
 	queued to that request. This is set by the user when calling
 	:ref:`ioctl VIDIOC_QBUF <VIDIOC_QBUF>` and ignored by other ioctls.
 	If the device does not support requests, then ``EPERM`` will be returned.
-	If requests are supported but an invalid request FD is given, then
-	``ENOENT`` will be returned.
+	If requests are supported but an invalid request file descriptor is
+	given, then ``EINVAL`` will be returned.
 
 
 
diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
index 771fd1161277..9c56a9b6e98a 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
@@ -101,8 +101,8 @@ then the controls are not applied immediately when calling
 :ref:`VIDIOC_S_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>`, but instead are applied by
 the driver for the buffer associated with the same request.
 If the device does not support requests, then ``EPERM`` will be returned.
-If requests are supported but an invalid request FD is given, then
-``ENOENT`` will be returned.
+If requests are supported but an invalid request file descriptor is given,
+then ``EINVAL`` will be returned.
 
 An attempt to call :ref:`VIDIOC_S_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` for a
 request that has already been queued will result in an ``EBUSY`` error.
@@ -301,8 +301,8 @@ still cause this situation.
       - File descriptor of the request to be used by this operation. Only
 	valid if ``which`` is set to ``V4L2_CTRL_WHICH_REQUEST_VAL``.
 	If the device does not support requests, then ``EPERM`` will be returned.
-	If requests are supported but an invalid request FD is given, then
-	``ENOENT`` will be returned.
+	If requests are supported but an invalid request file descriptor is
+	given, then ``EINVAL`` will be returned.
     * - __u32
       - ``reserved``\ [1]
       - Reserved for future extensions.
@@ -378,11 +378,13 @@ appropriately. The generic error codes are described at the
 
 EINVAL
     The struct :c:type:`v4l2_ext_control` ``id`` is
-    invalid, the struct :c:type:`v4l2_ext_controls`
+    invalid, or the struct :c:type:`v4l2_ext_controls`
     ``which`` is invalid, or the struct
     :c:type:`v4l2_ext_control` ``value`` was
     inappropriate (e.g. the given menu index is not supported by the
-    driver). This error code is also returned by the
+    driver), or the ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL``
+    but the given ``request_fd`` was invalid.
+    This error code is also returned by the
     :ref:`VIDIOC_S_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` and :ref:`VIDIOC_TRY_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` ioctls if two or
     more control values are in conflict.
 
@@ -409,7 +411,3 @@ EACCES
 EPERM
     The ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL`` but the
     device does not support requests.
-
-ENOENT
-    The ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL`` but the
-    the given ``request_fd`` was invalid.
diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
index 0e415f2551b2..7bff69c15452 100644
--- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst
+++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
@@ -105,8 +105,8 @@ until the request itself is queued. Also, the driver will apply any
 settings associated with the request for this buffer. This field will
 be ignored unless the ``V4L2_BUF_FLAG_REQUEST_FD`` flag is set.
 If the device does not support requests, then ``EPERM`` will be returned.
-If requests are supported but an invalid request FD is given, then
-``ENOENT`` will be returned.
+If requests are supported but an invalid request file descriptor is given,
+then ``EINVAL`` will be returned.
 
 .. caution::
    It is not allowed to mix queuing requests with queuing buffers directly.
@@ -152,7 +152,9 @@ EAGAIN
 EINVAL
     The buffer ``type`` is not supported, or the ``index`` is out of
     bounds, or no buffers have been allocated yet, or the ``userptr`` or
-    ``length`` are invalid.
+    ``length`` are invalid, or the ``V4L2_BUF_FLAG_REQUEST_FD`` flag was
+    set but the the given ``request_fd`` was invalid, or ``m.fd`` was
+    an invalid DMABUF file descriptor.
 
 EIO
     ``VIDIOC_DQBUF`` failed due to an internal error. Can also indicate
@@ -179,7 +181,3 @@ EPERM
     the application now tries to queue it directly, or vice versa (it is
     not permitted to mix the two APIs). Or an attempt is made to queue a
     CAPTURE buffer to a request for a :ref:`memory-to-memory device <codec>`.
-
-ENOENT
-    The ``V4L2_BUF_FLAG_REQUEST_FD`` flag was set but the the given
-    ``request_fd`` was invalid.
diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
index 4b0ce8fde7c9..4cee67e6657e 100644
--- a/drivers/media/media-request.c
+++ b/drivers/media/media-request.c
@@ -244,7 +244,7 @@ media_request_get_by_fd(struct media_device *mdev, int request_fd)
 
 	filp = fget(request_fd);
 	if (!filp)
-		return ERR_PTR(-ENOENT);
+		goto err_no_req_fd;
 
 	if (filp->f_op != &request_fops)
 		goto err_fput;
@@ -268,7 +268,9 @@ media_request_get_by_fd(struct media_device *mdev, int request_fd)
 err_fput:
 	fput(filp);
 
-	return ERR_PTR(-ENOENT);
+err_no_req_fd:
+	dev_dbg(mdev->dev, "cannot find request_fd %d\n", request_fd);
+	return ERR_PTR(-EINVAL);
 }
 EXPORT_SYMBOL_GPL(media_request_get_by_fd);
 
-- 
2.18.0

[PATCH 2/5] v4l2-ctrls: return -EACCES if request wasn't completed

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

For now (this might be relaxed in the future) we do not allow getting
controls from a request that isn't completed. In that case we return
-EACCES. Update the documentation accordingly.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst      | 18 +++++++++---------
 drivers/media/v4l2-core/v4l2-ctrls.c           |  5 ++---
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
index 9c56a9b6e98a..ad8908ce3095 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
@@ -107,13 +107,12 @@ then ``EINVAL`` will be returned.
 An attempt to call :ref:`VIDIOC_S_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` for a
 request that has already been queued will result in an ``EBUSY`` error.
 
-If ``request_fd`` is specified and ``which`` is set to ``V4L2_CTRL_WHICH_REQUEST_VAL``
-during a call to :ref:`VIDIOC_G_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>`, then the
-returned values will be the values currently set for the request (or the
-hardware value if none is set) if the request has not yet been queued, or the
-values of the controls at the time of request completion if it has already
-completed. Attempting to get controls while the request has been queued but
-not yet completed will result in an ``EBUSY`` error.
+If ``request_fd`` is specified and ``which`` is set to
+``V4L2_CTRL_WHICH_REQUEST_VAL`` during a call to
+:ref:`VIDIOC_G_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>`, then it will return the
+values of the controls at the time of request completion.
+If the request is not yet completed, then this will result in an
+``EACCES`` error.
 
 The driver will only set/get these controls if all control values are
 correct. This prevents the situation where only some of the controls
@@ -405,8 +404,9 @@ ENOSPC
     and this error code is returned.
 
 EACCES
-    Attempt to try or set a read-only control or to get a write-only
-    control.
+    Attempt to try or set a read-only control, or to get a write-only
+    control, or to get a control from a request that has not yet been
+    completed.
 
 EPERM
     The ``which`` field was set to ``V4L2_CTRL_WHICH_REQUEST_VAL`` but the
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index a197b60183f5..ccaf3068de6d 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3301,10 +3301,9 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct media_device *mdev,
 		if (IS_ERR(req))
 			return PTR_ERR(req);
 
-		if (req->state != MEDIA_REQUEST_STATE_IDLE &&
-		    req->state != MEDIA_REQUEST_STATE_COMPLETE) {
+		if (req->state != MEDIA_REQUEST_STATE_COMPLETE) {
 			media_request_put(req);
-			return -EBUSY;
+			return -EACCES;
 		}
 
 		obj = v4l2_ctrls_find_req_obj(hdl, req, false);
-- 
2.18.0

[PATCH 3/5] buffer.rst: only set V4L2_BUF_FLAG_REQUEST_FD for QBUF

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Document that V4L2_BUF_FLAG_REQUEST_FD should only be used with
VIDIOC_QBUF and cleared otherwise.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 Documentation/media/uapi/v4l/buffer.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst
index 35c2fadd10de..1865cd5b9d3c 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -312,6 +312,8 @@ struct v4l2_buffer
         and flag ``V4L2_BUF_FLAG_REQUEST_FD`` is set, then the buffer will be
 	queued to that request. This is set by the user when calling
 	:ref:`ioctl VIDIOC_QBUF <VIDIOC_QBUF>` and ignored by other ioctls.
+	Applications should not set ``V4L2_BUF_FLAG_REQUEST_FD`` for any ioctls
+	other than :ref:`VIDIOC_QBUF <VIDIOC_QBUF>`.
 	If the device does not support requests, then ``EPERM`` will be returned.
 	If requests are supported but an invalid request file descriptor is
 	given, then ``EINVAL`` will be returned.
-- 
2.18.0

[PATCH 4/5] videodev2.h: add new capabilities for buffer types

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

VIDIOC_REQBUFS and VIDIOC_CREATE_BUFFERS will return capabilities
telling userspace what the given buffer type is capable of.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 .../media/uapi/v4l/vidioc-create-bufs.rst     | 10 +++++-
 .../media/uapi/v4l/vidioc-reqbufs.rst         | 36 ++++++++++++++++++-
 include/uapi/linux/videodev2.h                | 13 +++++--
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
index a39e18d69511..fd34d3f236c9 100644
--- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
@@ -102,7 +102,15 @@ than the number requested.
       - ``format``
       - Filled in by the application, preserved by the driver.
     * - __u32
-      - ``reserved``\ [8]
+      - ``capabilities``
+      - Set by the driver. If 0, then the driver doesn't support
+        capabilities. In that case all you know is that the driver is
+	guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
+	other :c:type:`v4l2_memory` types. It will not support any others
+	capabilities. See :ref:`here <v4l2-buf-capabilities>` for a list of the
+	capabilities.
+    * - __u32
+      - ``reserved``\ [7]
       - A place holder for future extensions. Drivers and applications
 	must set the array to zero.
 
diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index 316f52c8a310..20a4bd4f2b74 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -88,10 +88,44 @@ any DMA in progress, an implicit
 	``V4L2_MEMORY_DMABUF`` or ``V4L2_MEMORY_USERPTR``. See
 	:c:type:`v4l2_memory`.
     * - __u32
-      - ``reserved``\ [2]
+      - ``capabilities``
+      - Set by the driver. If 0, then the driver doesn't support
+        capabilities. In that case all you know is that the driver is
+	guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
+	other :c:type:`v4l2_memory` types. It will not support any others
+	capabilities.
+    * - __u32
+      - ``reserved``\ [1]
       - A place holder for future extensions. Drivers and applications
 	must set the array to zero.
 
+.. tabularcolumns:: |p{6.1cm}|p{2.2cm}|p{8.7cm}|
+
+.. _v4l2-buf-capabilities:
+.. _V4L2-BUF-CAP-SUPPORTS-MMAP:
+.. _V4L2-BUF-CAP-SUPPORTS-USERPTR:
+.. _V4L2-BUF-CAP-SUPPORTS-DMABUF:
+.. _V4L2-BUF-CAP-SUPPORTS-REQUESTS:
+
+.. cssclass:: longtable
+
+.. flat-table:: V4L2 Buffer Capabilities Flags
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       3 1 4
+
+    * - ``V4L2_BUF_CAP_SUPPORTS_MMAP``
+      - 0x00000001
+      - This buffer type supports the ``V4L2_MEMORY_MMAP`` streaming mode.
+    * - ``V4L2_BUF_CAP_SUPPORTS_USERPTR``
+      - 0x00000002
+      - This buffer type supports the ``V4L2_MEMORY_USERPTR`` streaming mode.
+    * - ``V4L2_BUF_CAP_SUPPORTS_DMABUF``
+      - 0x00000004
+      - This buffer type supports the ``V4L2_MEMORY_DMABUF`` streaming mode.
+    * - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
+      - 0x00000008
+      - This buffer type supports :ref:`requests <media-request-api>`.
 
 Return Value
 ============
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 91126b7312f8..490fc9964d97 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -856,9 +856,16 @@ struct v4l2_requestbuffers {
 	__u32			count;
 	__u32			type;		/* enum v4l2_buf_type */
 	__u32			memory;		/* enum v4l2_memory */
-	__u32			reserved[2];
+	__u32			capabilities;
+	__u32			reserved[1];
 };
 
+/* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
+#define V4L2_BUF_CAP_SUPPORTS_MMAP	(1 << 0)
+#define V4L2_BUF_CAP_SUPPORTS_USERPTR	(1 << 1)
+#define V4L2_BUF_CAP_SUPPORTS_DMABUF	(1 << 2)
+#define V4L2_BUF_CAP_SUPPORTS_REQUESTS	(1 << 3)
+
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
  * @bytesused:		number of bytes occupied by data in the plane (payload)
@@ -2312,6 +2319,7 @@ struct v4l2_dbg_chip_info {
  *		return: number of created buffers
  * @memory:	enum v4l2_memory; buffer memory type
  * @format:	frame format, for which buffers are requested
+ * @capabilities: capabilities of this buffer type.
  * @reserved:	future extensions
  */
 struct v4l2_create_buffers {
@@ -2319,7 +2327,8 @@ struct v4l2_create_buffers {
 	__u32			count;
 	__u32			memory;
 	struct v4l2_format	format;
-	__u32			reserved[8];
+	__u32			capabilities;
+	__u32			reserved[7];
 };
 
 /*
-- 
2.18.0

[PATCH 5/5] vb2: set reqbufs/create_bufs capabilities

[Hans Verkuil]

From: Hans Verkuil <hansverk@cisco.com>

Set the capabilities field of v4l2_requestbuffers and v4l2_create_buffers.

The various mapping modes were easy, but for signaling the request capability
a new 'supports_requests' bitfield was added to videobuf2-core.h (and set in
vim2m and vivid). Drivers have to set this bitfield for any queue where
requests are supported.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 .../media/common/videobuf2/videobuf2-v4l2.c   | 19 ++++++++++++++++++-
 drivers/media/platform/vim2m.c                |  1 +
 drivers/media/platform/vivid/vivid-core.c     |  5 +++++
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  4 +++-
 drivers/media/v4l2-core/v4l2-ioctl.c          |  4 ++--
 include/media/videobuf2-core.h                |  2 ++
 6 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index a70df16d68f1..2caaabd50532 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -384,7 +384,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
 			return -EPERM;
 		}
 		return 0;
-	} else if (q->uses_qbuf) {
+	} else if (q->uses_qbuf || !q->supports_requests) {
 		dprintk(1, "%s: queue does not use requests\n", opname);
 		return -EPERM;
 	}
@@ -619,10 +619,24 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
 }
 EXPORT_SYMBOL(vb2_querybuf);
 
+static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
+{
+	*caps = 0;
+	if (q->io_modes & VB2_MMAP)
+		*caps |= V4L2_BUF_CAP_SUPPORTS_MMAP;
+	if (q->io_modes & VB2_USERPTR)
+		*caps |= V4L2_BUF_CAP_SUPPORTS_USERPTR;
+	if (q->io_modes & VB2_DMABUF)
+		*caps |= V4L2_BUF_CAP_SUPPORTS_DMABUF;
+	if (q->supports_requests)
+		*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
+}
+
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 {
 	int ret = vb2_verify_memory_type(q, req->memory, req->type);
 
+	fill_buf_caps(q, &req->capabilities);
 	return ret ? ret : vb2_core_reqbufs(q, req->memory, &req->count);
 }
 EXPORT_SYMBOL_GPL(vb2_reqbufs);
@@ -654,6 +668,7 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
 	int ret = vb2_verify_memory_type(q, create->memory, f->type);
 	unsigned i;
 
+	fill_buf_caps(q, &create->capabilities);
 	create->index = q->num_buffers;
 	if (create->count == 0)
 		return ret != -EBUSY ? ret : 0;
@@ -861,6 +876,7 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
 	struct video_device *vdev = video_devdata(file);
 	int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type);
 
+	fill_buf_caps(vdev->queue, &p->capabilities);
 	if (res)
 		return res;
 	if (vb2_queue_is_busy(vdev, file))
@@ -882,6 +898,7 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
 			p->format.type);
 
 	p->index = vdev->queue->num_buffers;
+	fill_buf_caps(vdev->queue, &p->capabilities);
 	/*
 	 * If count == 0, then just check if memory and type are valid.
 	 * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0.
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 5423f0dd0821..40fbb1e429af 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -855,6 +855,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
 	src_vq->mem_ops = &vb2_vmalloc_memops;
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	src_vq->lock = &ctx->dev->dev_mutex;
+	src_vq->supports_requests = true;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index 3f6f5cbe1b60..e7f1394832fe 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -1077,6 +1077,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
 		q->dev = dev->v4l2_dev.dev;
+		q->supports_requests = true;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1097,6 +1098,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
 		q->dev = dev->v4l2_dev.dev;
+		q->supports_requests = true;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1117,6 +1119,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
 		q->dev = dev->v4l2_dev.dev;
+		q->supports_requests = true;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1137,6 +1140,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
 		q->dev = dev->v4l2_dev.dev;
+		q->supports_requests = true;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1156,6 +1160,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->min_buffers_needed = 8;
 		q->lock = &dev->mutex;
 		q->dev = dev->v4l2_dev.dev;
+		q->supports_requests = true;
 
 		ret = vb2_queue_init(q);
 		if (ret)
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 633465d21d04..0028e0be6b5b 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -251,7 +251,8 @@ struct v4l2_create_buffers32 {
 	__u32			count;
 	__u32			memory;	/* enum v4l2_memory */
 	struct v4l2_format32	format;
-	__u32			reserved[8];
+	__u32			capabilities;
+	__u32			reserved[7];
 };
 
 static int __bufsize_v4l2_format(struct v4l2_format32 __user *p32, u32 *size)
@@ -411,6 +412,7 @@ static int put_v4l2_create32(struct v4l2_create_buffers __user *p64,
 	if (!access_ok(VERIFY_WRITE, p32, sizeof(*p32)) ||
 	    copy_in_user(p32, p64,
 			 offsetof(struct v4l2_create_buffers32, format)) ||
+	    assign_in_user(&p32->capabilities, &p64->capabilities) ||
 	    copy_in_user(p32->reserved, p64->reserved, sizeof(p64->reserved)))
 		return -EFAULT;
 	return __put_v4l2_format32(&p64->format, &p32->format);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 2a84ca9e328a..87dba0b9c0a7 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1877,7 +1877,7 @@ static int v4l_reqbufs(const struct v4l2_ioctl_ops *ops,
 	if (ret)
 		return ret;
 
-	CLEAR_AFTER_FIELD(p, memory);
+	CLEAR_AFTER_FIELD(p, capabilities);
 
 	return ops->vidioc_reqbufs(file, fh, p);
 }
@@ -1918,7 +1918,7 @@ static int v4l_create_bufs(const struct v4l2_ioctl_ops *ops,
 	if (ret)
 		return ret;
 
-	CLEAR_AFTER_FIELD(create, format);
+	CLEAR_AFTER_FIELD(create, capabilities);
 
 	v4l_sanitize_format(&create->format);
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 881f53b38b26..6c76b9802589 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -472,6 +472,7 @@ struct vb2_buf_ops {
  * @quirk_poll_must_check_waiting_for_buffers: Return %EPOLLERR at poll when QBUF
  *              has not been called. This is a vb1 idiom that has been adopted
  *              also by vb2.
+ * @supports_requests: this queue supports the Request API.
  * @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.
@@ -545,6 +546,7 @@ struct vb2_queue {
 	unsigned			fileio_write_immediately:1;
 	unsigned			allow_zero_bytesused:1;
 	unsigned		   quirk_poll_must_check_waiting_for_buffers:1;
+	unsigned			supports_requests:1;
 	unsigned			uses_qbuf:1;
 	unsigned			uses_requests:1;
 
-- 
2.18.0

Re: [PATCH 4/5] videodev2.h: add new capabilities for buffer types

[Tomasz Figa]

Hi Hans,

On Fri, Aug 24, 2018 at 5:22 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hansverk@cisco.com>
>
> VIDIOC_REQBUFS and VIDIOC_CREATE_BUFFERS will return capabilities
> telling userspace what the given buffer type is capable of.
>

Please see my comments below.

> Signed-off-by: Hans Verkuil <hansverk@cisco.com>
> ---
>  .../media/uapi/v4l/vidioc-create-bufs.rst     | 10 +++++-
>  .../media/uapi/v4l/vidioc-reqbufs.rst         | 36 ++++++++++++++++++-
>  include/uapi/linux/videodev2.h                | 13 +++++--
>  3 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> index a39e18d69511..fd34d3f236c9 100644
> --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> @@ -102,7 +102,15 @@ than the number requested.
>        - ``format``
>        - Filled in by the application, preserved by the driver.
>      * - __u32
> -      - ``reserved``\ [8]
> +      - ``capabilities``
> +      - Set by the driver. If 0, then the driver doesn't support
> +        capabilities. In that case all you know is that the driver is
> +       guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
> +       other :c:type:`v4l2_memory` types. It will not support any others
> +       capabilities. See :ref:`here <v4l2-buf-capabilities>` for a list of the
> +       capabilities.

Perhaps it would make sense to document how the application is
expected to query for these capabilities? Right now, the application
is expected to fill in the "memory" field in this struct (and reqbufs
counterpart), but it sounds a bit strange that one needs to know what
"memory" value to write there to query what set of "memory" values is
supported. In theory, MMAP is expected to be always supported, but it
sounds strange anyway. Also, is there a way to call REQBUFS without
altering the buffer allocation?

Best regards,
Tomasz

Re: [PATCH 0/5] Post-v18: Request API updates

[Tomasz Figa]

Hi Hans,

On Fri, Aug 24, 2018 at 5:22 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Hi all,
>
> This patch series sits on top of my v18 series for the Request API.
> It makes some final (?) changes as discussed in:
>
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg134419.html
>
> and:
>
> https://www.spinics.net/lists/linux-media/msg138596.html
>

For all patches except "[PATCH 4/5] videodev2.h: add new capabilities
for buffer types":

Reviewed-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz

Re: [PATCH 4/5] videodev2.h: add new capabilities for buffer types

[Hans Verkuil]

On 24/08/18 16:36, Tomasz Figa wrote:
> Hi Hans,
> 
> On Fri, Aug 24, 2018 at 5:22 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> From: Hans Verkuil <hansverk@cisco.com>
>>
>> VIDIOC_REQBUFS and VIDIOC_CREATE_BUFFERS will return capabilities
>> telling userspace what the given buffer type is capable of.
>>
> 
> Please see my comments below.
> 
>> Signed-off-by: Hans Verkuil <hansverk@cisco.com>
>> ---
>>  .../media/uapi/v4l/vidioc-create-bufs.rst     | 10 +++++-
>>  .../media/uapi/v4l/vidioc-reqbufs.rst         | 36 ++++++++++++++++++-
>>  include/uapi/linux/videodev2.h                | 13 +++++--
>>  3 files changed, 55 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>> index a39e18d69511..fd34d3f236c9 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>> @@ -102,7 +102,15 @@ than the number requested.
>>        - ``format``
>>        - Filled in by the application, preserved by the driver.
>>      * - __u32
>> -      - ``reserved``\ [8]
>> +      - ``capabilities``
>> +      - Set by the driver. If 0, then the driver doesn't support
>> +        capabilities. In that case all you know is that the driver is
>> +       guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
>> +       other :c:type:`v4l2_memory` types. It will not support any others
>> +       capabilities. See :ref:`here <v4l2-buf-capabilities>` for a list of the
>> +       capabilities.
> 
> Perhaps it would make sense to document how the application is
> expected to query for these capabilities? Right now, the application
> is expected to fill in the "memory" field in this struct (and reqbufs
> counterpart), but it sounds a bit strange that one needs to know what
> "memory" value to write there to query what set of "memory" values is
> supported. In theory, MMAP is expected to be always supported, but it
> sounds strange anyway. Also, is there a way to call REQBUFS without
> altering the buffer allocation?

No, this is only possible with CREATE_BUFS.

But it is reasonable to call REQBUFS with a count of 0, since you want to
start with a clean slate anyway.

The only option I see would be to introduce a new memory type (e.g.
V4L2_MEMORY_CAPS) to just return the capabilities. But that's more ugly
then just requiring that you use MMAP when calling this.

I'm inclined to just document that you need to set memory to MMAP if
you want to get the capabilities since MMAP is always guaranteed to
exist.

Regards,

	Hans

Re: [PATCH 4/5] videodev2.h: add new capabilities for buffer types

[Tomasz Figa]

On Tue, Aug 28, 2018 at 9:37 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 24/08/18 16:36, Tomasz Figa wrote:
> > Hi Hans,
> >
> > On Fri, Aug 24, 2018 at 5:22 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> >>
> >> From: Hans Verkuil <hansverk@cisco.com>
> >>
> >> VIDIOC_REQBUFS and VIDIOC_CREATE_BUFFERS will return capabilities
> >> telling userspace what the given buffer type is capable of.
> >>
> >
> > Please see my comments below.
> >
> >> Signed-off-by: Hans Verkuil <hansverk@cisco.com>
> >> ---
> >>  .../media/uapi/v4l/vidioc-create-bufs.rst     | 10 +++++-
> >>  .../media/uapi/v4l/vidioc-reqbufs.rst         | 36 ++++++++++++++++++-
> >>  include/uapi/linux/videodev2.h                | 13 +++++--
> >>  3 files changed, 55 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> >> index a39e18d69511..fd34d3f236c9 100644
> >> --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> >> +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
> >> @@ -102,7 +102,15 @@ than the number requested.
> >>        - ``format``
> >>        - Filled in by the application, preserved by the driver.
> >>      * - __u32
> >> -      - ``reserved``\ [8]
> >> +      - ``capabilities``
> >> +      - Set by the driver. If 0, then the driver doesn't support
> >> +        capabilities. In that case all you know is that the driver is
> >> +       guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
> >> +       other :c:type:`v4l2_memory` types. It will not support any others
> >> +       capabilities. See :ref:`here <v4l2-buf-capabilities>` for a list of the
> >> +       capabilities.
> >
> > Perhaps it would make sense to document how the application is
> > expected to query for these capabilities? Right now, the application
> > is expected to fill in the "memory" field in this struct (and reqbufs
> > counterpart), but it sounds a bit strange that one needs to know what
> > "memory" value to write there to query what set of "memory" values is
> > supported. In theory, MMAP is expected to be always supported, but it
> > sounds strange anyway. Also, is there a way to call REQBUFS without
> > altering the buffer allocation?
>
> No, this is only possible with CREATE_BUFS.
>
> But it is reasonable to call REQBUFS with a count of 0, since you want to
> start with a clean slate anyway.
>
> The only option I see would be to introduce a new memory type (e.g.
> V4L2_MEMORY_CAPS) to just return the capabilities. But that's more ugly
> then just requiring that you use MMAP when calling this.
>
> I'm inclined to just document that you need to set memory to MMAP if
> you want to get the capabilities since MMAP is always guaranteed to
> exist.

I guess it's the least evil we can get without changing the API too
much. Fair enough.

Best regards,
Tomasz

Re: [PATCH 4/5] videodev2.h: add new capabilities for buffer types

[Hans Verkuil]

On 08/29/2018 10:49 AM, Tomasz Figa wrote:
> On Tue, Aug 28, 2018 at 9:37 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> On 24/08/18 16:36, Tomasz Figa wrote:
>>> Hi Hans,
>>>
>>> On Fri, Aug 24, 2018 at 5:22 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>>>
>>>> From: Hans Verkuil <hansverk@cisco.com>
>>>>
>>>> VIDIOC_REQBUFS and VIDIOC_CREATE_BUFFERS will return capabilities
>>>> telling userspace what the given buffer type is capable of.
>>>>
>>>
>>> Please see my comments below.
>>>
>>>> Signed-off-by: Hans Verkuil <hansverk@cisco.com>
>>>> ---
>>>>  .../media/uapi/v4l/vidioc-create-bufs.rst     | 10 +++++-
>>>>  .../media/uapi/v4l/vidioc-reqbufs.rst         | 36 ++++++++++++++++++-
>>>>  include/uapi/linux/videodev2.h                | 13 +++++--
>>>>  3 files changed, 55 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>>>> index a39e18d69511..fd34d3f236c9 100644
>>>> --- a/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>>>> +++ b/Documentation/media/uapi/v4l/vidioc-create-bufs.rst
>>>> @@ -102,7 +102,15 @@ than the number requested.
>>>>        - ``format``
>>>>        - Filled in by the application, preserved by the driver.
>>>>      * - __u32
>>>> -      - ``reserved``\ [8]
>>>> +      - ``capabilities``
>>>> +      - Set by the driver. If 0, then the driver doesn't support
>>>> +        capabilities. In that case all you know is that the driver is
>>>> +       guaranteed to support ``V4L2_MEMORY_MMAP`` and *might* support
>>>> +       other :c:type:`v4l2_memory` types. It will not support any others
>>>> +       capabilities. See :ref:`here <v4l2-buf-capabilities>` for a list of the
>>>> +       capabilities.
>>>
>>> Perhaps it would make sense to document how the application is
>>> expected to query for these capabilities? Right now, the application
>>> is expected to fill in the "memory" field in this struct (and reqbufs
>>> counterpart), but it sounds a bit strange that one needs to know what
>>> "memory" value to write there to query what set of "memory" values is
>>> supported. In theory, MMAP is expected to be always supported, but it
>>> sounds strange anyway. Also, is there a way to call REQBUFS without
>>> altering the buffer allocation?
>>
>> No, this is only possible with CREATE_BUFS.
>>
>> But it is reasonable to call REQBUFS with a count of 0, since you want to
>> start with a clean slate anyway.
>>
>> The only option I see would be to introduce a new memory type (e.g.
>> V4L2_MEMORY_CAPS) to just return the capabilities. But that's more ugly
>> then just requiring that you use MMAP when calling this.
>>
>> I'm inclined to just document that you need to set memory to MMAP if
>> you want to get the capabilities since MMAP is always guaranteed to
>> exist.
> 
> I guess it's the least evil we can get without changing the API too
> much. Fair enough.
> 

Can you ack the updated patch:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg134624.html

And take a look at patches 6-10 as well, if you have time.

I'd like to get this merged in a request topic branch asap.

Regards,

	Hans

> Best regards,
> Tomasz
>