diff mbox series

[v3] media: v4l2-subdev: Add new ioctl for client capabilities

Message ID 20230323135835.206970-1-tomi.valkeinen@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series [v3] media: v4l2-subdev: Add new ioctl for client capabilities | expand

Commit Message

Tomi Valkeinen March 23, 2023, 1:58 p.m. UTC
Add new ioctls to set and get subdev client capabilities. Client in this
context means the userspace application which opens the subdev device
node. The client capabilities are stored in the file handle of the
opened subdev device node, and the client must set the capabilities for
each opened subdev.

For now we only add a single flag, V4L2_SUBDEV_CLIENT_CAP_STREAMS, which
indicates that the client is streams-aware.

The reason for needing such a flag is as follows:

Many structs passed via ioctls, e.g. struct v4l2_subdev_format, contain
reserved fields (usually a single array field). These reserved fields
can be used to extend the ioctl. The userspace is required to zero the
reserved fields.

We recently added a new 'stream' field to many of these structs, and the
space for the field was taken from these reserved arrays. The assumption
was that these new 'stream' fields are always initialized to zero if the
userspace does not use them. This was a mistake, as, as mentioned above,
the userspace is required to zero the _reserved_ fields. In other words,
there is no requirement to zero this new stream field, and if the
userspace doesn't use the field (which is the case for all userspace
applications at the moment), the field may contain random data.

This shows that the way the reserved fields are defined in v4l2 is, in
my opinion, somewhat broken, but there is nothing to do about that.

To fix this issue we need a way for the userspace to tell the kernel
that the userspace has indeed set the 'stream' field, and it's fine for
the kernel to access it. This is achieved with the new ioctl, which the
userspace should usually use right after opening the subdev device node.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Interdiff against v2:
  diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
  index 0c4ff938f63d..20f12a1cc0f7 100644
  --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
  +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
  @@ -37,20 +37,29 @@ Description
   ===========
   
   These ioctls are used to get and set the client (the application using the
  -subdevice ioctls) capabilities. By default no client capabilities are set.
  +subdevice ioctls) capabilities. The client capabilities are stored in the file
  +handle of the opened subdev device node, and the client must set the
  +capabilities for each opened subdev separately.
  +
  +By default no client capabilities are set when a subdev device node is opened.
   
   The purpose of the client capabilities are to inform the kernel of the behavior
   of the client, mainly related to maintaining compatibility with different
   kernel and userspace versions.
   
  -The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` will modify the struct
  -:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that were
  -accepted. A common case for the kernel not accepting a capability is that the
  -kernel is older than the headers the userspace uses, and thus the capability is
  -unknown to the kernel.
  +The ``VIDIOC_SUBDEV_G_CLIENT_CAP`` ioctl returns the current client capabilities
  +associated with the file handle ``fd``.
   
  -The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` will replace all the previously set
  -capabilities.
  +The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` ioctl sets client capabilities for the file
  +handle ``fd``. The new capabilities fully replace the current capabilities, the
  +ioctl can therefore also be used to remove capabilities that have previously
  +been set.
  +
  +``VIDIOC_SUBDEV_S_CLIENT_CAP`` modifies the struct
  +:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that have
  +been accepted. A common case for the kernel not accepting a capability is that
  +the kernel is older than the headers the userspace uses, and thus the capability
  +is unknown to the kernel.
   
   .. flat-table:: Client Capabilities
       :header-rows:  1

 .../userspace-api/media/v4l/user-func.rst     |  1 +
 .../media/v4l/vidioc-subdev-g-client-cap.rst  | 83 +++++++++++++++++++
 drivers/media/v4l2-core/v4l2-subdev.c         | 63 ++++++++++++++
 include/media/v4l2-subdev.h                   |  1 +
 include/uapi/linux/v4l2-subdev.h              | 21 +++++
 5 files changed, 169 insertions(+)
 create mode 100644 Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst

Comments

Laurent Pinchart April 12, 2023, 12:26 p.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Thu, Mar 23, 2023 at 03:58:35PM +0200, Tomi Valkeinen wrote:
> Add new ioctls to set and get subdev client capabilities. Client in this
> context means the userspace application which opens the subdev device
> node. The client capabilities are stored in the file handle of the
> opened subdev device node, and the client must set the capabilities for
> each opened subdev.
> 
> For now we only add a single flag, V4L2_SUBDEV_CLIENT_CAP_STREAMS, which
> indicates that the client is streams-aware.
> 
> The reason for needing such a flag is as follows:
> 
> Many structs passed via ioctls, e.g. struct v4l2_subdev_format, contain
> reserved fields (usually a single array field). These reserved fields
> can be used to extend the ioctl. The userspace is required to zero the
> reserved fields.
> 
> We recently added a new 'stream' field to many of these structs, and the
> space for the field was taken from these reserved arrays. The assumption
> was that these new 'stream' fields are always initialized to zero if the
> userspace does not use them. This was a mistake, as, as mentioned above,
> the userspace is required to zero the _reserved_ fields. In other words,
> there is no requirement to zero this new stream field, and if the
> userspace doesn't use the field (which is the case for all userspace
> applications at the moment), the field may contain random data.
> 
> This shows that the way the reserved fields are defined in v4l2 is, in
> my opinion, somewhat broken, but there is nothing to do about that.
> 
> To fix this issue we need a way for the userspace to tell the kernel
> that the userspace has indeed set the 'stream' field, and it's fine for
> the kernel to access it. This is achieved with the new ioctl, which the
> userspace should usually use right after opening the subdev device node.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Interdiff against v2:
>   diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>   index 0c4ff938f63d..20f12a1cc0f7 100644
>   --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>   +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>   @@ -37,20 +37,29 @@ Description
>    ===========
>    
>    These ioctls are used to get and set the client (the application using the
>   -subdevice ioctls) capabilities. By default no client capabilities are set.
>   +subdevice ioctls) capabilities. The client capabilities are stored in the file
>   +handle of the opened subdev device node, and the client must set the
>   +capabilities for each opened subdev separately.
>   +
>   +By default no client capabilities are set when a subdev device node is opened.
>    
>    The purpose of the client capabilities are to inform the kernel of the behavior
>    of the client, mainly related to maintaining compatibility with different
>    kernel and userspace versions.
>    
>   -The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` will modify the struct
>   -:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that were
>   -accepted. A common case for the kernel not accepting a capability is that the
>   -kernel is older than the headers the userspace uses, and thus the capability is
>   -unknown to the kernel.
>   +The ``VIDIOC_SUBDEV_G_CLIENT_CAP`` ioctl returns the current client capabilities
>   +associated with the file handle ``fd``.
>    
>   -The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` will replace all the previously set
>   -capabilities.
>   +The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` ioctl sets client capabilities for the file
>   +handle ``fd``. The new capabilities fully replace the current capabilities, the
>   +ioctl can therefore also be used to remove capabilities that have previously
>   +been set.
>   +
>   +``VIDIOC_SUBDEV_S_CLIENT_CAP`` modifies the struct
>   +:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that have
>   +been accepted. A common case for the kernel not accepting a capability is that
>   +the kernel is older than the headers the userspace uses, and thus the capability
>   +is unknown to the kernel.
>    
>    .. flat-table:: Client Capabilities
>        :header-rows:  1
> 
>  .../userspace-api/media/v4l/user-func.rst     |  1 +
>  .../media/v4l/vidioc-subdev-g-client-cap.rst  | 83 +++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-subdev.c         | 63 ++++++++++++++
>  include/media/v4l2-subdev.h                   |  1 +
>  include/uapi/linux/v4l2-subdev.h              | 21 +++++
>  5 files changed, 169 insertions(+)
>  create mode 100644 Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
> 
> diff --git a/Documentation/userspace-api/media/v4l/user-func.rst b/Documentation/userspace-api/media/v4l/user-func.rst
> index 228c1521f190..15ff0bf7bbe6 100644
> --- a/Documentation/userspace-api/media/v4l/user-func.rst
> +++ b/Documentation/userspace-api/media/v4l/user-func.rst
> @@ -72,6 +72,7 @@ Function Reference
>      vidioc-subdev-g-frame-interval
>      vidioc-subdev-g-routing
>      vidioc-subdev-g-selection
> +    vidioc-subdev-g-client-cap
>      vidioc-subdev-querycap
>      vidioc-subscribe-event
>      func-mmap
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
> new file mode 100644
> index 000000000000..20f12a1cc0f7
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
> @@ -0,0 +1,83 @@
> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> +.. c:namespace:: V4L
> +
> +.. _VIDIOC_SUBDEV_G_CLIENT_CAP:
> +
> +************************************************************
> +ioctl VIDIOC_SUBDEV_G_CLIENT_CAP, VIDIOC_SUBDEV_S_CLIENT_CAP
> +************************************************************
> +
> +Name
> +====
> +
> +VIDIOC_SUBDEV_G_CLIENT_CAP - VIDIOC_SUBDEV_S_CLIENT_CAP - Get or set client
> +capabilities.
> +
> +Synopsis
> +========
> +
> +.. c:macro:: VIDIOC_SUBDEV_G_CLIENT_CAP
> +
> +``int ioctl(int fd, VIDIOC_SUBDEV_G_CLIENT_CAP, struct v4l2_subdev_client_capability *argp)``
> +
> +.. c:macro:: VIDIOC_SUBDEV_S_CLIENT_CAP
> +
> +``int ioctl(int fd, VIDIOC_SUBDEV_S_CLIENT_CAP, struct v4l2_subdev_client_capability *argp)``
> +
> +Arguments
> +=========
> +
> +``fd``
> +    File descriptor returned by :ref:`open() <func-open>`.
> +
> +``argp``
> +    Pointer to struct :c:type:`v4l2_subdev_client_capability`.
> +
> +Description
> +===========
> +
> +These ioctls are used to get and set the client (the application using the
> +subdevice ioctls) capabilities. The client capabilities are stored in the file
> +handle of the opened subdev device node, and the client must set the
> +capabilities for each opened subdev separately.
> +
> +By default no client capabilities are set when a subdev device node is opened.
> +
> +The purpose of the client capabilities are to inform the kernel of the behavior
> +of the client, mainly related to maintaining compatibility with different
> +kernel and userspace versions.
> +
> +The ``VIDIOC_SUBDEV_G_CLIENT_CAP`` ioctl returns the current client capabilities
> +associated with the file handle ``fd``.
> +
> +The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` ioctl sets client capabilities for the file
> +handle ``fd``. The new capabilities fully replace the current capabilities, the
> +ioctl can therefore also be used to remove capabilities that have previously
> +been set.
> +
> +``VIDIOC_SUBDEV_S_CLIENT_CAP`` modifies the struct
> +:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that have
> +been accepted. A common case for the kernel not accepting a capability is that
> +the kernel is older than the headers the userspace uses, and thus the capability
> +is unknown to the kernel.
> +
> +.. flat-table:: Client Capabilities
> +    :header-rows:  1
> +
> +    * - Capability
> +      - Description
> +    * - ``V4L2_SUBDEV_CLIENT_CAP_STREAMS``
> +      - The client is aware of streams. Setting this flag enables the use
> +        of 'stream' fields (referring to the stream number) with various
> +        ioctls. If this is not set (which is the default), the 'stream' fields
> +        will be forced to 0 by the kernel.
> +
> +Return Value
> +============
> +
> +On success 0 is returned, on error -1 and the ``errno`` variable is set
> +appropriately. The generic error codes are described at the
> +:ref:`Generic Error Codes <gen-errors>` chapter.
> +
> +ENOIOCTLCMD
> +   The kernel does not support this ioctl.
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index dff1d9be7841..9d47f4800ea8 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -498,8 +498,11 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  	struct video_device *vdev = video_devdata(file);
>  	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
>  	struct v4l2_fh *vfh = file->private_data;
> +	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
>  	bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags);
>  	bool streams_subdev = sd->flags & V4L2_SUBDEV_FL_STREAMS;
> +	bool client_supports_streams = subdev_fh->client_caps &
> +				       V4L2_SUBDEV_CLIENT_CAP_STREAMS;
>  	int rval;
>  
>  	switch (cmd) {
> @@ -624,6 +627,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  	case VIDIOC_SUBDEV_G_FMT: {
>  		struct v4l2_subdev_format *format = arg;
>  
> +		if (!client_supports_streams)
> +			format->stream = 0;
> +
>  		memset(format->reserved, 0, sizeof(format->reserved));
>  		memset(format->format.reserved, 0, sizeof(format->format.reserved));
>  		return v4l2_subdev_call(sd, pad, get_fmt, state, format);
> @@ -635,6 +641,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		if (format->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
>  			return -EPERM;
>  
> +		if (!client_supports_streams)
> +			format->stream = 0;
> +
>  		memset(format->reserved, 0, sizeof(format->reserved));
>  		memset(format->format.reserved, 0, sizeof(format->format.reserved));
>  		return v4l2_subdev_call(sd, pad, set_fmt, state, format);
> @@ -644,6 +653,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		struct v4l2_subdev_crop *crop = arg;
>  		struct v4l2_subdev_selection sel;
>  
> +		if (!client_supports_streams)
> +			crop->stream = 0;
> +
>  		memset(crop->reserved, 0, sizeof(crop->reserved));
>  		memset(&sel, 0, sizeof(sel));
>  		sel.which = crop->which;
> @@ -665,6 +677,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		if (crop->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
>  			return -EPERM;
>  
> +		if (!client_supports_streams)
> +			crop->stream = 0;
> +
>  		memset(crop->reserved, 0, sizeof(crop->reserved));
>  		memset(&sel, 0, sizeof(sel));
>  		sel.which = crop->which;
> @@ -683,6 +698,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  	case VIDIOC_SUBDEV_ENUM_MBUS_CODE: {
>  		struct v4l2_subdev_mbus_code_enum *code = arg;
>  
> +		if (!client_supports_streams)
> +			code->stream = 0;
> +
>  		memset(code->reserved, 0, sizeof(code->reserved));
>  		return v4l2_subdev_call(sd, pad, enum_mbus_code, state,
>  					code);
> @@ -691,6 +709,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  	case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: {
>  		struct v4l2_subdev_frame_size_enum *fse = arg;
>  
> +		if (!client_supports_streams)
> +			fse->stream = 0;
> +
>  		memset(fse->reserved, 0, sizeof(fse->reserved));
>  		return v4l2_subdev_call(sd, pad, enum_frame_size, state,
>  					fse);
> @@ -699,6 +720,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  	case VIDIOC_SUBDEV_G_FRAME_INTERVAL: {
>  		struct v4l2_subdev_frame_interval *fi = arg;
>  
> +		if (!client_supports_streams)
> +			fi->stream = 0;
> +
>  		memset(fi->reserved, 0, sizeof(fi->reserved));
>  		return v4l2_subdev_call(sd, video, g_frame_interval, arg);
>  	}
> @@ -709,6 +733,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		if (ro_subdev)
>  			return -EPERM;
>  
> +		if (!client_supports_streams)
> +			fi->stream = 0;
> +
>  		memset(fi->reserved, 0, sizeof(fi->reserved));
>  		return v4l2_subdev_call(sd, video, s_frame_interval, arg);
>  	}
> @@ -716,6 +743,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  	case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL: {
>  		struct v4l2_subdev_frame_interval_enum *fie = arg;
>  
> +		if (!client_supports_streams)
> +			fie->stream = 0;
> +
>  		memset(fie->reserved, 0, sizeof(fie->reserved));
>  		return v4l2_subdev_call(sd, pad, enum_frame_interval, state,
>  					fie);
> @@ -724,6 +754,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  	case VIDIOC_SUBDEV_G_SELECTION: {
>  		struct v4l2_subdev_selection *sel = arg;
>  
> +		if (!client_supports_streams)
> +			sel->stream = 0;
> +
>  		memset(sel->reserved, 0, sizeof(sel->reserved));
>  		return v4l2_subdev_call(
>  			sd, pad, get_selection, state, sel);
> @@ -735,6 +768,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		if (sel->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
>  			return -EPERM;
>  
> +		if (!client_supports_streams)
> +			sel->stream = 0;
> +
>  		memset(sel->reserved, 0, sizeof(sel->reserved));
>  		return v4l2_subdev_call(
>  			sd, pad, set_selection, state, sel);
> @@ -876,6 +912,33 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  					routing->which, &krouting);
>  	}
>  
> +	case VIDIOC_SUBDEV_G_CLIENT_CAP: {
> +		struct v4l2_subdev_client_capability *client_cap = arg;
> +
> +		client_cap->capabilities = subdev_fh->client_caps;
> +
> +		return 0;
> +	}
> +
> +	case VIDIOC_SUBDEV_S_CLIENT_CAP: {
> +		struct v4l2_subdev_client_capability *client_cap = arg;
> +
> +		/*
> +		 * Clear V4L2_SUBDEV_CLIENT_CAP_STREAMS if streams API is not
> +		 * enabled. Remove this when streams API is no longer
> +		 * experimental.
> +		 */
> +		if (!v4l2_subdev_enable_streams_api)
> +			client_cap->capabilities &= ~V4L2_SUBDEV_CLIENT_CAP_STREAMS;
> +
> +		/* Filter out unsupported capabilities */
> +		client_cap->capabilities &= V4L2_SUBDEV_CLIENT_CAP_STREAMS;
> +
> +		subdev_fh->client_caps = client_cap->capabilities;
> +
> +		return 0;
> +	}
> +
>  	default:
>  		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
>  	}
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 17773be4a4ee..b5bb5b802929 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1121,6 +1121,7 @@ struct v4l2_subdev_fh {
>  	struct module *owner;
>  #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>  	struct v4l2_subdev_state *state;
> +	u64 client_caps;
>  #endif
>  };
>  
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index 654d659de835..4a195b68f28f 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -233,6 +233,24 @@ struct v4l2_subdev_routing {
>  	__u32 reserved[6];
>  };
>  
> +/*
> + * The client is aware of streams. Setting this flag enables the use of 'stream'
> + * fields (referring to the stream number) with various ioctls. If this is not
> + * set (which is the default), the 'stream' fields will be forced to 0 by the
> + * kernel.
> + */
> + #define V4L2_SUBDEV_CLIENT_CAP_STREAMS		(1U << 0)
> +
> +/**
> + * struct v4l2_subdev_client_capability - Capabilities of the client accessing
> + *					  the subdev
> + *
> + * @capabilities: A bitmask of V4L2_SUBDEV_CLIENT_CAP_* flags.
> + */
> +struct v4l2_subdev_client_capability {
> +	__u64 capabilities;
> +};
> +
>  /* Backwards compatibility define --- to be removed */
>  #define v4l2_subdev_edid v4l2_edid
>  
> @@ -250,6 +268,9 @@ struct v4l2_subdev_routing {
>  #define VIDIOC_SUBDEV_S_SELECTION		_IOWR('V', 62, struct v4l2_subdev_selection)
>  #define VIDIOC_SUBDEV_G_ROUTING			_IOWR('V', 38, struct v4l2_subdev_routing)
>  #define VIDIOC_SUBDEV_S_ROUTING			_IOWR('V', 39, struct v4l2_subdev_routing)
> +#define VIDIOC_SUBDEV_G_CLIENT_CAP		_IOR('V',  101, struct v4l2_subdev_client_capability)
> +#define VIDIOC_SUBDEV_S_CLIENT_CAP		_IOWR('V',  102, struct v4l2_subdev_client_capability)
> +
>  /* The following ioctls are identical to the ioctls in videodev2.h */
>  #define VIDIOC_SUBDEV_G_STD			_IOR('V', 23, v4l2_std_id)
>  #define VIDIOC_SUBDEV_S_STD			_IOW('V', 24, v4l2_std_id)
Tomi Valkeinen April 12, 2023, 1:04 p.m. UTC | #2
On 12/04/2023 15:26, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Thu, Mar 23, 2023 at 03:58:35PM +0200, Tomi Valkeinen wrote:
>> Add new ioctls to set and get subdev client capabilities. Client in this
>> context means the userspace application which opens the subdev device
>> node. The client capabilities are stored in the file handle of the
>> opened subdev device node, and the client must set the capabilities for
>> each opened subdev.
>>
>> For now we only add a single flag, V4L2_SUBDEV_CLIENT_CAP_STREAMS, which
>> indicates that the client is streams-aware.
>>
>> The reason for needing such a flag is as follows:
>>
>> Many structs passed via ioctls, e.g. struct v4l2_subdev_format, contain
>> reserved fields (usually a single array field). These reserved fields
>> can be used to extend the ioctl. The userspace is required to zero the
>> reserved fields.
>>
>> We recently added a new 'stream' field to many of these structs, and the
>> space for the field was taken from these reserved arrays. The assumption
>> was that these new 'stream' fields are always initialized to zero if the
>> userspace does not use them. This was a mistake, as, as mentioned above,
>> the userspace is required to zero the _reserved_ fields. In other words,
>> there is no requirement to zero this new stream field, and if the
>> userspace doesn't use the field (which is the case for all userspace
>> applications at the moment), the field may contain random data.
>>
>> This shows that the way the reserved fields are defined in v4l2 is, in
>> my opinion, somewhat broken, but there is nothing to do about that.
>>
>> To fix this issue we need a way for the userspace to tell the kernel
>> that the userspace has indeed set the 'stream' field, and it's fine for
>> the kernel to access it. This is achieved with the new ioctl, which the
>> userspace should usually use right after opening the subdev device node.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

Hans, Mauro, Sakari, it's a bit late, but any chance to get this to the 
next merge window?

  Tomi

> 
>> ---
>> Interdiff against v2:
>>    diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>>    index 0c4ff938f63d..20f12a1cc0f7 100644
>>    --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>>    +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>>    @@ -37,20 +37,29 @@ Description
>>     ===========
>>     
>>     These ioctls are used to get and set the client (the application using the
>>    -subdevice ioctls) capabilities. By default no client capabilities are set.
>>    +subdevice ioctls) capabilities. The client capabilities are stored in the file
>>    +handle of the opened subdev device node, and the client must set the
>>    +capabilities for each opened subdev separately.
>>    +
>>    +By default no client capabilities are set when a subdev device node is opened.
>>     
>>     The purpose of the client capabilities are to inform the kernel of the behavior
>>     of the client, mainly related to maintaining compatibility with different
>>     kernel and userspace versions.
>>     
>>    -The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` will modify the struct
>>    -:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that were
>>    -accepted. A common case for the kernel not accepting a capability is that the
>>    -kernel is older than the headers the userspace uses, and thus the capability is
>>    -unknown to the kernel.
>>    +The ``VIDIOC_SUBDEV_G_CLIENT_CAP`` ioctl returns the current client capabilities
>>    +associated with the file handle ``fd``.
>>     
>>    -The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` will replace all the previously set
>>    -capabilities.
>>    +The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` ioctl sets client capabilities for the file
>>    +handle ``fd``. The new capabilities fully replace the current capabilities, the
>>    +ioctl can therefore also be used to remove capabilities that have previously
>>    +been set.
>>    +
>>    +``VIDIOC_SUBDEV_S_CLIENT_CAP`` modifies the struct
>>    +:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that have
>>    +been accepted. A common case for the kernel not accepting a capability is that
>>    +the kernel is older than the headers the userspace uses, and thus the capability
>>    +is unknown to the kernel.
>>     
>>     .. flat-table:: Client Capabilities
>>         :header-rows:  1
>>
>>   .../userspace-api/media/v4l/user-func.rst     |  1 +
>>   .../media/v4l/vidioc-subdev-g-client-cap.rst  | 83 +++++++++++++++++++
>>   drivers/media/v4l2-core/v4l2-subdev.c         | 63 ++++++++++++++
>>   include/media/v4l2-subdev.h                   |  1 +
>>   include/uapi/linux/v4l2-subdev.h              | 21 +++++
>>   5 files changed, 169 insertions(+)
>>   create mode 100644 Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>>
>> diff --git a/Documentation/userspace-api/media/v4l/user-func.rst b/Documentation/userspace-api/media/v4l/user-func.rst
>> index 228c1521f190..15ff0bf7bbe6 100644
>> --- a/Documentation/userspace-api/media/v4l/user-func.rst
>> +++ b/Documentation/userspace-api/media/v4l/user-func.rst
>> @@ -72,6 +72,7 @@ Function Reference
>>       vidioc-subdev-g-frame-interval
>>       vidioc-subdev-g-routing
>>       vidioc-subdev-g-selection
>> +    vidioc-subdev-g-client-cap
>>       vidioc-subdev-querycap
>>       vidioc-subscribe-event
>>       func-mmap
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>> new file mode 100644
>> index 000000000000..20f12a1cc0f7
>> --- /dev/null
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
>> @@ -0,0 +1,83 @@
>> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
>> +.. c:namespace:: V4L
>> +
>> +.. _VIDIOC_SUBDEV_G_CLIENT_CAP:
>> +
>> +************************************************************
>> +ioctl VIDIOC_SUBDEV_G_CLIENT_CAP, VIDIOC_SUBDEV_S_CLIENT_CAP
>> +************************************************************
>> +
>> +Name
>> +====
>> +
>> +VIDIOC_SUBDEV_G_CLIENT_CAP - VIDIOC_SUBDEV_S_CLIENT_CAP - Get or set client
>> +capabilities.
>> +
>> +Synopsis
>> +========
>> +
>> +.. c:macro:: VIDIOC_SUBDEV_G_CLIENT_CAP
>> +
>> +``int ioctl(int fd, VIDIOC_SUBDEV_G_CLIENT_CAP, struct v4l2_subdev_client_capability *argp)``
>> +
>> +.. c:macro:: VIDIOC_SUBDEV_S_CLIENT_CAP
>> +
>> +``int ioctl(int fd, VIDIOC_SUBDEV_S_CLIENT_CAP, struct v4l2_subdev_client_capability *argp)``
>> +
>> +Arguments
>> +=========
>> +
>> +``fd``
>> +    File descriptor returned by :ref:`open() <func-open>`.
>> +
>> +``argp``
>> +    Pointer to struct :c:type:`v4l2_subdev_client_capability`.
>> +
>> +Description
>> +===========
>> +
>> +These ioctls are used to get and set the client (the application using the
>> +subdevice ioctls) capabilities. The client capabilities are stored in the file
>> +handle of the opened subdev device node, and the client must set the
>> +capabilities for each opened subdev separately.
>> +
>> +By default no client capabilities are set when a subdev device node is opened.
>> +
>> +The purpose of the client capabilities are to inform the kernel of the behavior
>> +of the client, mainly related to maintaining compatibility with different
>> +kernel and userspace versions.
>> +
>> +The ``VIDIOC_SUBDEV_G_CLIENT_CAP`` ioctl returns the current client capabilities
>> +associated with the file handle ``fd``.
>> +
>> +The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` ioctl sets client capabilities for the file
>> +handle ``fd``. The new capabilities fully replace the current capabilities, the
>> +ioctl can therefore also be used to remove capabilities that have previously
>> +been set.
>> +
>> +``VIDIOC_SUBDEV_S_CLIENT_CAP`` modifies the struct
>> +:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that have
>> +been accepted. A common case for the kernel not accepting a capability is that
>> +the kernel is older than the headers the userspace uses, and thus the capability
>> +is unknown to the kernel.
>> +
>> +.. flat-table:: Client Capabilities
>> +    :header-rows:  1
>> +
>> +    * - Capability
>> +      - Description
>> +    * - ``V4L2_SUBDEV_CLIENT_CAP_STREAMS``
>> +      - The client is aware of streams. Setting this flag enables the use
>> +        of 'stream' fields (referring to the stream number) with various
>> +        ioctls. If this is not set (which is the default), the 'stream' fields
>> +        will be forced to 0 by the kernel.
>> +
>> +Return Value
>> +============
>> +
>> +On success 0 is returned, on error -1 and the ``errno`` variable is set
>> +appropriately. The generic error codes are described at the
>> +:ref:`Generic Error Codes <gen-errors>` chapter.
>> +
>> +ENOIOCTLCMD
>> +   The kernel does not support this ioctl.
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
>> index dff1d9be7841..9d47f4800ea8 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -498,8 +498,11 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   	struct video_device *vdev = video_devdata(file);
>>   	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
>>   	struct v4l2_fh *vfh = file->private_data;
>> +	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
>>   	bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags);
>>   	bool streams_subdev = sd->flags & V4L2_SUBDEV_FL_STREAMS;
>> +	bool client_supports_streams = subdev_fh->client_caps &
>> +				       V4L2_SUBDEV_CLIENT_CAP_STREAMS;
>>   	int rval;
>>   
>>   	switch (cmd) {
>> @@ -624,6 +627,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   	case VIDIOC_SUBDEV_G_FMT: {
>>   		struct v4l2_subdev_format *format = arg;
>>   
>> +		if (!client_supports_streams)
>> +			format->stream = 0;
>> +
>>   		memset(format->reserved, 0, sizeof(format->reserved));
>>   		memset(format->format.reserved, 0, sizeof(format->format.reserved));
>>   		return v4l2_subdev_call(sd, pad, get_fmt, state, format);
>> @@ -635,6 +641,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   		if (format->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
>>   			return -EPERM;
>>   
>> +		if (!client_supports_streams)
>> +			format->stream = 0;
>> +
>>   		memset(format->reserved, 0, sizeof(format->reserved));
>>   		memset(format->format.reserved, 0, sizeof(format->format.reserved));
>>   		return v4l2_subdev_call(sd, pad, set_fmt, state, format);
>> @@ -644,6 +653,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   		struct v4l2_subdev_crop *crop = arg;
>>   		struct v4l2_subdev_selection sel;
>>   
>> +		if (!client_supports_streams)
>> +			crop->stream = 0;
>> +
>>   		memset(crop->reserved, 0, sizeof(crop->reserved));
>>   		memset(&sel, 0, sizeof(sel));
>>   		sel.which = crop->which;
>> @@ -665,6 +677,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   		if (crop->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
>>   			return -EPERM;
>>   
>> +		if (!client_supports_streams)
>> +			crop->stream = 0;
>> +
>>   		memset(crop->reserved, 0, sizeof(crop->reserved));
>>   		memset(&sel, 0, sizeof(sel));
>>   		sel.which = crop->which;
>> @@ -683,6 +698,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   	case VIDIOC_SUBDEV_ENUM_MBUS_CODE: {
>>   		struct v4l2_subdev_mbus_code_enum *code = arg;
>>   
>> +		if (!client_supports_streams)
>> +			code->stream = 0;
>> +
>>   		memset(code->reserved, 0, sizeof(code->reserved));
>>   		return v4l2_subdev_call(sd, pad, enum_mbus_code, state,
>>   					code);
>> @@ -691,6 +709,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   	case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: {
>>   		struct v4l2_subdev_frame_size_enum *fse = arg;
>>   
>> +		if (!client_supports_streams)
>> +			fse->stream = 0;
>> +
>>   		memset(fse->reserved, 0, sizeof(fse->reserved));
>>   		return v4l2_subdev_call(sd, pad, enum_frame_size, state,
>>   					fse);
>> @@ -699,6 +720,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   	case VIDIOC_SUBDEV_G_FRAME_INTERVAL: {
>>   		struct v4l2_subdev_frame_interval *fi = arg;
>>   
>> +		if (!client_supports_streams)
>> +			fi->stream = 0;
>> +
>>   		memset(fi->reserved, 0, sizeof(fi->reserved));
>>   		return v4l2_subdev_call(sd, video, g_frame_interval, arg);
>>   	}
>> @@ -709,6 +733,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   		if (ro_subdev)
>>   			return -EPERM;
>>   
>> +		if (!client_supports_streams)
>> +			fi->stream = 0;
>> +
>>   		memset(fi->reserved, 0, sizeof(fi->reserved));
>>   		return v4l2_subdev_call(sd, video, s_frame_interval, arg);
>>   	}
>> @@ -716,6 +743,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   	case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL: {
>>   		struct v4l2_subdev_frame_interval_enum *fie = arg;
>>   
>> +		if (!client_supports_streams)
>> +			fie->stream = 0;
>> +
>>   		memset(fie->reserved, 0, sizeof(fie->reserved));
>>   		return v4l2_subdev_call(sd, pad, enum_frame_interval, state,
>>   					fie);
>> @@ -724,6 +754,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   	case VIDIOC_SUBDEV_G_SELECTION: {
>>   		struct v4l2_subdev_selection *sel = arg;
>>   
>> +		if (!client_supports_streams)
>> +			sel->stream = 0;
>> +
>>   		memset(sel->reserved, 0, sizeof(sel->reserved));
>>   		return v4l2_subdev_call(
>>   			sd, pad, get_selection, state, sel);
>> @@ -735,6 +768,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   		if (sel->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
>>   			return -EPERM;
>>   
>> +		if (!client_supports_streams)
>> +			sel->stream = 0;
>> +
>>   		memset(sel->reserved, 0, sizeof(sel->reserved));
>>   		return v4l2_subdev_call(
>>   			sd, pad, set_selection, state, sel);
>> @@ -876,6 +912,33 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>>   					routing->which, &krouting);
>>   	}
>>   
>> +	case VIDIOC_SUBDEV_G_CLIENT_CAP: {
>> +		struct v4l2_subdev_client_capability *client_cap = arg;
>> +
>> +		client_cap->capabilities = subdev_fh->client_caps;
>> +
>> +		return 0;
>> +	}
>> +
>> +	case VIDIOC_SUBDEV_S_CLIENT_CAP: {
>> +		struct v4l2_subdev_client_capability *client_cap = arg;
>> +
>> +		/*
>> +		 * Clear V4L2_SUBDEV_CLIENT_CAP_STREAMS if streams API is not
>> +		 * enabled. Remove this when streams API is no longer
>> +		 * experimental.
>> +		 */
>> +		if (!v4l2_subdev_enable_streams_api)
>> +			client_cap->capabilities &= ~V4L2_SUBDEV_CLIENT_CAP_STREAMS;
>> +
>> +		/* Filter out unsupported capabilities */
>> +		client_cap->capabilities &= V4L2_SUBDEV_CLIENT_CAP_STREAMS;
>> +
>> +		subdev_fh->client_caps = client_cap->capabilities;
>> +
>> +		return 0;
>> +	}
>> +
>>   	default:
>>   		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
>>   	}
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index 17773be4a4ee..b5bb5b802929 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -1121,6 +1121,7 @@ struct v4l2_subdev_fh {
>>   	struct module *owner;
>>   #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>>   	struct v4l2_subdev_state *state;
>> +	u64 client_caps;
>>   #endif
>>   };
>>   
>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
>> index 654d659de835..4a195b68f28f 100644
>> --- a/include/uapi/linux/v4l2-subdev.h
>> +++ b/include/uapi/linux/v4l2-subdev.h
>> @@ -233,6 +233,24 @@ struct v4l2_subdev_routing {
>>   	__u32 reserved[6];
>>   };
>>   
>> +/*
>> + * The client is aware of streams. Setting this flag enables the use of 'stream'
>> + * fields (referring to the stream number) with various ioctls. If this is not
>> + * set (which is the default), the 'stream' fields will be forced to 0 by the
>> + * kernel.
>> + */
>> + #define V4L2_SUBDEV_CLIENT_CAP_STREAMS		(1U << 0)
>> +
>> +/**
>> + * struct v4l2_subdev_client_capability - Capabilities of the client accessing
>> + *					  the subdev
>> + *
>> + * @capabilities: A bitmask of V4L2_SUBDEV_CLIENT_CAP_* flags.
>> + */
>> +struct v4l2_subdev_client_capability {
>> +	__u64 capabilities;
>> +};
>> +
>>   /* Backwards compatibility define --- to be removed */
>>   #define v4l2_subdev_edid v4l2_edid
>>   
>> @@ -250,6 +268,9 @@ struct v4l2_subdev_routing {
>>   #define VIDIOC_SUBDEV_S_SELECTION		_IOWR('V', 62, struct v4l2_subdev_selection)
>>   #define VIDIOC_SUBDEV_G_ROUTING			_IOWR('V', 38, struct v4l2_subdev_routing)
>>   #define VIDIOC_SUBDEV_S_ROUTING			_IOWR('V', 39, struct v4l2_subdev_routing)
>> +#define VIDIOC_SUBDEV_G_CLIENT_CAP		_IOR('V',  101, struct v4l2_subdev_client_capability)
>> +#define VIDIOC_SUBDEV_S_CLIENT_CAP		_IOWR('V',  102, struct v4l2_subdev_client_capability)
>> +
>>   /* The following ioctls are identical to the ioctls in videodev2.h */
>>   #define VIDIOC_SUBDEV_G_STD			_IOR('V', 23, v4l2_std_id)
>>   #define VIDIOC_SUBDEV_S_STD			_IOW('V', 24, v4l2_std_id)
>
Sakari Ailus April 12, 2023, 1:22 p.m. UTC | #3
Moi,

On Wed, Apr 12, 2023 at 04:04:32PM +0300, Tomi Valkeinen wrote:
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks!
> 
> Hans, Mauro, Sakari, it's a bit late, but any chance to get this to the next
> merge window?

I think it may be a bit late now, given the number of patches in the media
stage tree, but if Hans (or Mauro) is willing to pick an additional PR now,
I'll send one.
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/v4l/user-func.rst b/Documentation/userspace-api/media/v4l/user-func.rst
index 228c1521f190..15ff0bf7bbe6 100644
--- a/Documentation/userspace-api/media/v4l/user-func.rst
+++ b/Documentation/userspace-api/media/v4l/user-func.rst
@@ -72,6 +72,7 @@  Function Reference
     vidioc-subdev-g-frame-interval
     vidioc-subdev-g-routing
     vidioc-subdev-g-selection
+    vidioc-subdev-g-client-cap
     vidioc-subdev-querycap
     vidioc-subscribe-event
     func-mmap
diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
new file mode 100644
index 000000000000..20f12a1cc0f7
--- /dev/null
+++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-client-cap.rst
@@ -0,0 +1,83 @@ 
+.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
+.. c:namespace:: V4L
+
+.. _VIDIOC_SUBDEV_G_CLIENT_CAP:
+
+************************************************************
+ioctl VIDIOC_SUBDEV_G_CLIENT_CAP, VIDIOC_SUBDEV_S_CLIENT_CAP
+************************************************************
+
+Name
+====
+
+VIDIOC_SUBDEV_G_CLIENT_CAP - VIDIOC_SUBDEV_S_CLIENT_CAP - Get or set client
+capabilities.
+
+Synopsis
+========
+
+.. c:macro:: VIDIOC_SUBDEV_G_CLIENT_CAP
+
+``int ioctl(int fd, VIDIOC_SUBDEV_G_CLIENT_CAP, struct v4l2_subdev_client_capability *argp)``
+
+.. c:macro:: VIDIOC_SUBDEV_S_CLIENT_CAP
+
+``int ioctl(int fd, VIDIOC_SUBDEV_S_CLIENT_CAP, struct v4l2_subdev_client_capability *argp)``
+
+Arguments
+=========
+
+``fd``
+    File descriptor returned by :ref:`open() <func-open>`.
+
+``argp``
+    Pointer to struct :c:type:`v4l2_subdev_client_capability`.
+
+Description
+===========
+
+These ioctls are used to get and set the client (the application using the
+subdevice ioctls) capabilities. The client capabilities are stored in the file
+handle of the opened subdev device node, and the client must set the
+capabilities for each opened subdev separately.
+
+By default no client capabilities are set when a subdev device node is opened.
+
+The purpose of the client capabilities are to inform the kernel of the behavior
+of the client, mainly related to maintaining compatibility with different
+kernel and userspace versions.
+
+The ``VIDIOC_SUBDEV_G_CLIENT_CAP`` ioctl returns the current client capabilities
+associated with the file handle ``fd``.
+
+The ``VIDIOC_SUBDEV_S_CLIENT_CAP`` ioctl sets client capabilities for the file
+handle ``fd``. The new capabilities fully replace the current capabilities, the
+ioctl can therefore also be used to remove capabilities that have previously
+been set.
+
+``VIDIOC_SUBDEV_S_CLIENT_CAP`` modifies the struct
+:c:type:`v4l2_subdev_client_capability` to reflect the capabilities that have
+been accepted. A common case for the kernel not accepting a capability is that
+the kernel is older than the headers the userspace uses, and thus the capability
+is unknown to the kernel.
+
+.. flat-table:: Client Capabilities
+    :header-rows:  1
+
+    * - Capability
+      - Description
+    * - ``V4L2_SUBDEV_CLIENT_CAP_STREAMS``
+      - The client is aware of streams. Setting this flag enables the use
+        of 'stream' fields (referring to the stream number) with various
+        ioctls. If this is not set (which is the default), the 'stream' fields
+        will be forced to 0 by the kernel.
+
+Return Value
+============
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes <gen-errors>` chapter.
+
+ENOIOCTLCMD
+   The kernel does not support this ioctl.
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index dff1d9be7841..9d47f4800ea8 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -498,8 +498,11 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
 	struct v4l2_fh *vfh = file->private_data;
+	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
 	bool ro_subdev = test_bit(V4L2_FL_SUBDEV_RO_DEVNODE, &vdev->flags);
 	bool streams_subdev = sd->flags & V4L2_SUBDEV_FL_STREAMS;
+	bool client_supports_streams = subdev_fh->client_caps &
+				       V4L2_SUBDEV_CLIENT_CAP_STREAMS;
 	int rval;
 
 	switch (cmd) {
@@ -624,6 +627,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 	case VIDIOC_SUBDEV_G_FMT: {
 		struct v4l2_subdev_format *format = arg;
 
+		if (!client_supports_streams)
+			format->stream = 0;
+
 		memset(format->reserved, 0, sizeof(format->reserved));
 		memset(format->format.reserved, 0, sizeof(format->format.reserved));
 		return v4l2_subdev_call(sd, pad, get_fmt, state, format);
@@ -635,6 +641,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 		if (format->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
 			return -EPERM;
 
+		if (!client_supports_streams)
+			format->stream = 0;
+
 		memset(format->reserved, 0, sizeof(format->reserved));
 		memset(format->format.reserved, 0, sizeof(format->format.reserved));
 		return v4l2_subdev_call(sd, pad, set_fmt, state, format);
@@ -644,6 +653,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 		struct v4l2_subdev_crop *crop = arg;
 		struct v4l2_subdev_selection sel;
 
+		if (!client_supports_streams)
+			crop->stream = 0;
+
 		memset(crop->reserved, 0, sizeof(crop->reserved));
 		memset(&sel, 0, sizeof(sel));
 		sel.which = crop->which;
@@ -665,6 +677,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 		if (crop->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
 			return -EPERM;
 
+		if (!client_supports_streams)
+			crop->stream = 0;
+
 		memset(crop->reserved, 0, sizeof(crop->reserved));
 		memset(&sel, 0, sizeof(sel));
 		sel.which = crop->which;
@@ -683,6 +698,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 	case VIDIOC_SUBDEV_ENUM_MBUS_CODE: {
 		struct v4l2_subdev_mbus_code_enum *code = arg;
 
+		if (!client_supports_streams)
+			code->stream = 0;
+
 		memset(code->reserved, 0, sizeof(code->reserved));
 		return v4l2_subdev_call(sd, pad, enum_mbus_code, state,
 					code);
@@ -691,6 +709,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 	case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: {
 		struct v4l2_subdev_frame_size_enum *fse = arg;
 
+		if (!client_supports_streams)
+			fse->stream = 0;
+
 		memset(fse->reserved, 0, sizeof(fse->reserved));
 		return v4l2_subdev_call(sd, pad, enum_frame_size, state,
 					fse);
@@ -699,6 +720,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 	case VIDIOC_SUBDEV_G_FRAME_INTERVAL: {
 		struct v4l2_subdev_frame_interval *fi = arg;
 
+		if (!client_supports_streams)
+			fi->stream = 0;
+
 		memset(fi->reserved, 0, sizeof(fi->reserved));
 		return v4l2_subdev_call(sd, video, g_frame_interval, arg);
 	}
@@ -709,6 +733,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 		if (ro_subdev)
 			return -EPERM;
 
+		if (!client_supports_streams)
+			fi->stream = 0;
+
 		memset(fi->reserved, 0, sizeof(fi->reserved));
 		return v4l2_subdev_call(sd, video, s_frame_interval, arg);
 	}
@@ -716,6 +743,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 	case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL: {
 		struct v4l2_subdev_frame_interval_enum *fie = arg;
 
+		if (!client_supports_streams)
+			fie->stream = 0;
+
 		memset(fie->reserved, 0, sizeof(fie->reserved));
 		return v4l2_subdev_call(sd, pad, enum_frame_interval, state,
 					fie);
@@ -724,6 +754,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 	case VIDIOC_SUBDEV_G_SELECTION: {
 		struct v4l2_subdev_selection *sel = arg;
 
+		if (!client_supports_streams)
+			sel->stream = 0;
+
 		memset(sel->reserved, 0, sizeof(sel->reserved));
 		return v4l2_subdev_call(
 			sd, pad, get_selection, state, sel);
@@ -735,6 +768,9 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 		if (sel->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
 			return -EPERM;
 
+		if (!client_supports_streams)
+			sel->stream = 0;
+
 		memset(sel->reserved, 0, sizeof(sel->reserved));
 		return v4l2_subdev_call(
 			sd, pad, set_selection, state, sel);
@@ -876,6 +912,33 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
 					routing->which, &krouting);
 	}
 
+	case VIDIOC_SUBDEV_G_CLIENT_CAP: {
+		struct v4l2_subdev_client_capability *client_cap = arg;
+
+		client_cap->capabilities = subdev_fh->client_caps;
+
+		return 0;
+	}
+
+	case VIDIOC_SUBDEV_S_CLIENT_CAP: {
+		struct v4l2_subdev_client_capability *client_cap = arg;
+
+		/*
+		 * Clear V4L2_SUBDEV_CLIENT_CAP_STREAMS if streams API is not
+		 * enabled. Remove this when streams API is no longer
+		 * experimental.
+		 */
+		if (!v4l2_subdev_enable_streams_api)
+			client_cap->capabilities &= ~V4L2_SUBDEV_CLIENT_CAP_STREAMS;
+
+		/* Filter out unsupported capabilities */
+		client_cap->capabilities &= V4L2_SUBDEV_CLIENT_CAP_STREAMS;
+
+		subdev_fh->client_caps = client_cap->capabilities;
+
+		return 0;
+	}
+
 	default:
 		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
 	}
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 17773be4a4ee..b5bb5b802929 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1121,6 +1121,7 @@  struct v4l2_subdev_fh {
 	struct module *owner;
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
 	struct v4l2_subdev_state *state;
+	u64 client_caps;
 #endif
 };
 
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index 654d659de835..4a195b68f28f 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -233,6 +233,24 @@  struct v4l2_subdev_routing {
 	__u32 reserved[6];
 };
 
+/*
+ * The client is aware of streams. Setting this flag enables the use of 'stream'
+ * fields (referring to the stream number) with various ioctls. If this is not
+ * set (which is the default), the 'stream' fields will be forced to 0 by the
+ * kernel.
+ */
+ #define V4L2_SUBDEV_CLIENT_CAP_STREAMS		(1U << 0)
+
+/**
+ * struct v4l2_subdev_client_capability - Capabilities of the client accessing
+ *					  the subdev
+ *
+ * @capabilities: A bitmask of V4L2_SUBDEV_CLIENT_CAP_* flags.
+ */
+struct v4l2_subdev_client_capability {
+	__u64 capabilities;
+};
+
 /* Backwards compatibility define --- to be removed */
 #define v4l2_subdev_edid v4l2_edid
 
@@ -250,6 +268,9 @@  struct v4l2_subdev_routing {
 #define VIDIOC_SUBDEV_S_SELECTION		_IOWR('V', 62, struct v4l2_subdev_selection)
 #define VIDIOC_SUBDEV_G_ROUTING			_IOWR('V', 38, struct v4l2_subdev_routing)
 #define VIDIOC_SUBDEV_S_ROUTING			_IOWR('V', 39, struct v4l2_subdev_routing)
+#define VIDIOC_SUBDEV_G_CLIENT_CAP		_IOR('V',  101, struct v4l2_subdev_client_capability)
+#define VIDIOC_SUBDEV_S_CLIENT_CAP		_IOWR('V',  102, struct v4l2_subdev_client_capability)
+
 /* The following ioctls are identical to the ioctls in videodev2.h */
 #define VIDIOC_SUBDEV_G_STD			_IOR('V', 23, v4l2_std_id)
 #define VIDIOC_SUBDEV_S_STD			_IOW('V', 24, v4l2_std_id)