diff mbox series

[v6,18/24] v4l: subdev: Add [GS]_ROUTING subdev ioctls and operations

Message ID 20210427124523.990938-19-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series v4l: subdev internal routing | expand

Commit Message

Tomi Valkeinen April 27, 2021, 12:45 p.m. UTC
Add support for subdev internal routing. A route is defined as a single
stream from a sink pad to a source pad.

The userspace can configure the routing via two new ioctls,
VIDIOC_SUBDEV_G_ROUTING and VIDIOC_SUBDEV_S_ROUTING, and subdevs can
implement the functionality with v4l2_subdev_pad_ops.get_routing() and
v4l2_subdev_pad_ops.set_routing().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>

- Add sink and source streams for multiplexed links
- Copy the argument back in case of an error. This is needed to let the
  caller know the number of routes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

- Expand and refine documentation.
- Make the 'routes' pointer a __u64 __user pointer so that a compat32
  version of the ioctl is not required.
- Add struct v4l2_subdev_krouting to be used for subdevice operations.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

- Fix typecasing warnings
- Check sink & source pad types
- Add 'which' field

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c  | 25 +++++++++++++-
 drivers/media/v4l2-core/v4l2-subdev.c | 46 +++++++++++++++++++++++++
 include/media/v4l2-subdev.h           | 24 ++++++++++++++
 include/uapi/linux/v4l2-subdev.h      | 48 +++++++++++++++++++++++++++
 4 files changed, 142 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart April 29, 2021, 2:28 a.m. UTC | #1
Hi Tomi,

Thank you for the patch.

For some reason, it seems the original authorship got lost. It was fine
in v5. I haven't checked if other patches in the series are affected.

On Tue, Apr 27, 2021 at 03:45:17PM +0300, Tomi Valkeinen wrote:
> Add support for subdev internal routing. A route is defined as a single

> stream from a sink pad to a source pad.

> 

> The userspace can configure the routing via two new ioctls,

> VIDIOC_SUBDEV_G_ROUTING and VIDIOC_SUBDEV_S_ROUTING, and subdevs can

> implement the functionality with v4l2_subdev_pad_ops.get_routing() and

> v4l2_subdev_pad_ops.set_routing().

> 

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

> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

> 

> - Add sink and source streams for multiplexed links

> - Copy the argument back in case of an error. This is needed to let the

>   caller know the number of routes.

> 

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 

> - Expand and refine documentation.

> - Make the 'routes' pointer a __u64 __user pointer so that a compat32

>   version of the ioctl is not required.

> - Add struct v4l2_subdev_krouting to be used for subdevice operations.

> 

> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

> 

> - Fix typecasing warnings

> - Check sink & source pad types

> - Add 'which' field

> 

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

> ---

>  drivers/media/v4l2-core/v4l2-ioctl.c  | 25 +++++++++++++-

>  drivers/media/v4l2-core/v4l2-subdev.c | 46 +++++++++++++++++++++++++

>  include/media/v4l2-subdev.h           | 24 ++++++++++++++

>  include/uapi/linux/v4l2-subdev.h      | 48 +++++++++++++++++++++++++++

>  4 files changed, 142 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c

> index 6a5d1c6d11d6..7b1dabc23909 100644

> --- a/drivers/media/v4l2-core/v4l2-ioctl.c

> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c

> @@ -16,6 +16,7 @@

>  #include <linux/kernel.h>

>  #include <linux/version.h>

>  

> +#include <linux/v4l2-subdev.h>

>  #include <linux/videodev2.h>

>  

>  #include <media/v4l2-common.h>

> @@ -3108,6 +3109,21 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,

>  		ret = 1;

>  		break;

>  	}

> +

> +	case VIDIOC_SUBDEV_G_ROUTING:

> +	case VIDIOC_SUBDEV_S_ROUTING: {

> +		struct v4l2_subdev_routing *routing = parg;

> +

> +		if (routing->num_routes > 256)

> +			return -EINVAL;

> +

> +		*user_ptr = u64_to_user_ptr(routing->routes);

> +		*kernel_ptr = (void **)&routing->routes;

> +		*array_size = sizeof(struct v4l2_subdev_route)

> +			    * routing->num_routes;

> +		ret = 1;

> +		break;

> +	}

>  	}

>  

>  	return ret;

> @@ -3369,8 +3385,15 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,

>  	/*

>  	 * Some ioctls can return an error, but still have valid

>  	 * results that must be returned.

> +	 *

> +	 * FIXME: subdev IOCTLS are partially handled here and partially in

> +	 * v4l2-subdev.c and the 'always_copy' flag can only be set for IOCTLS

> +	 * defined here as part of the 'v4l2_ioctls' array. As

> +	 * VIDIOC_SUBDEV_G_ROUTING needs to return results to applications even

> +	 * in case of failure, but it is not defined here as part of the

> +	 * 'v4l2_ioctls' array, insert an ad-hoc check to address that.

>  	 */

> -	if (err < 0 && !always_copy)

> +	if (err < 0 && !always_copy && cmd != VIDIOC_SUBDEV_G_ROUTING)

>  		goto out;

>  

>  out_array_args:

> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c

> index 956dafab43d4..ad79ce121cee 100644

> --- a/drivers/media/v4l2-core/v4l2-subdev.c

> +++ b/drivers/media/v4l2-core/v4l2-subdev.c

> @@ -681,6 +681,52 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)

>  	case VIDIOC_SUBDEV_QUERYSTD:

>  		return v4l2_subdev_call(sd, video, querystd, arg);

>  

> +	case VIDIOC_SUBDEV_G_ROUTING: {

> +		struct v4l2_subdev_routing *routing = arg;

> +		struct v4l2_subdev_krouting krouting = {

> +			.which = routing->which,

> +			.num_routes = routing->num_routes,

> +			.routes = (struct v4l2_subdev_route *)(uintptr_t)

> +					  routing->routes,

> +		};

> +		int ret;

> +

> +		ret = v4l2_subdev_call(sd, pad, get_routing, &krouting);

> +

> +		routing->num_routes = krouting.num_routes;

> +

> +		return ret;

> +	}

> +

> +	case VIDIOC_SUBDEV_S_ROUTING: {

> +		struct v4l2_subdev_routing *routing = arg;

> +		struct v4l2_subdev_route *routes =

> +			(struct v4l2_subdev_route *)(uintptr_t)routing->routes;

> +		struct v4l2_subdev_krouting krouting = {};

> +		unsigned int i;

> +

> +		if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)

> +			return -EPERM;

> +

> +		for (i = 0; i < routing->num_routes; ++i) {

> +			if (routes[i].sink_pad >= sd->entity.num_pads ||

> +			    routes[i].source_pad >= sd->entity.num_pads)

> +				return -EINVAL;

> +

> +			if (!(sd->entity.pads[routes[i].sink_pad].flags &

> +			      MEDIA_PAD_FL_SINK) ||

> +			    !(sd->entity.pads[routes[i].source_pad].flags &

> +			      MEDIA_PAD_FL_SOURCE))

> +				return -EINVAL;

> +		}

> +

> +		krouting.which = routing->which;

> +		krouting.num_routes = routing->num_routes;

> +		krouting.routes = routes;

> +

> +		return v4l2_subdev_call(sd, pad, set_routing, &krouting);

> +	}

> +

>  	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 70a21298020b..c45f5f0156c9 100644

> --- a/include/media/v4l2-subdev.h

> +++ b/include/media/v4l2-subdev.h

> @@ -660,6 +660,22 @@ struct v4l2_subdev_pad_config {

>  	struct v4l2_rect try_compose;

>  };

>  

> +/**

> + * struct v4l2_subdev_krouting - subdev routing table

> + *

> + * @which: format type (from enum v4l2_subdev_format_whence)

> + * @routes: &struct v4l2_subdev_route

> + * @num_routes: number of routes

> + *

> + * This structure is used to translate arguments received from

> + * VIDIOC_SUBDEV_G/S_ROUTING() ioctl to subdev device drivers operations.

> + */

> +struct v4l2_subdev_krouting {

> +	u32 which;

> +	struct v4l2_subdev_route *routes;

> +	unsigned int num_routes;

> +};

> +

>  /**

>   * struct v4l2_subdev_pad_ops - v4l2-subdev pad level operations

>   *

> @@ -721,6 +737,10 @@ struct v4l2_subdev_pad_config {

>   *		     applied to the hardware. The operation shall fail if the

>   *		     pad index it has been called on is not valid or in case of

>   *		     unrecoverable failures.

> + *

> + * @get_routing: get the subdevice routing table.

> + * @set_routing: enable or disable data connection routes described in the

> + *		 subdevice routing table.

>   */

>  struct v4l2_subdev_pad_ops {

>  	int (*init_cfg)(struct v4l2_subdev *sd,

> @@ -765,6 +785,10 @@ struct v4l2_subdev_pad_ops {

>  			       struct v4l2_mbus_config *config);

>  	int (*set_mbus_config)(struct v4l2_subdev *sd, unsigned int pad,

>  			       struct v4l2_mbus_config *config);

> +	int (*get_routing)(struct v4l2_subdev *sd,

> +			   struct v4l2_subdev_krouting *route);

> +	int (*set_routing)(struct v4l2_subdev *sd,

> +			   struct v4l2_subdev_krouting *route);

>  };

>  

>  /**

> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h

> index 658106f5b5dc..3a360cdcacf4 100644

> --- a/include/uapi/linux/v4l2-subdev.h

> +++ b/include/uapi/linux/v4l2-subdev.h

> @@ -188,6 +188,52 @@ struct v4l2_subdev_capability {

>  /* The v4l2 sub-device video device node is registered in read-only mode. */

>  #define V4L2_SUBDEV_CAP_RO_SUBDEV		0x00000001

>  

> +/*


Shouldn't this be /** ?

> + * Is the route active? An active route will start when streaming is enabled

> + * on a video node.


Even with an a priori knowledge of the topic, I'm not sure to really
understand this, and I don't think it reflects the purpose of the flag.

> + */

> +#define V4L2_SUBDEV_ROUTE_FL_ACTIVE		BIT(0)

> +

> +/*

> + * Is the route immutable, i.e. can it be activated and inactivated?


Maybe adding "An immutable route is always active".

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


> + * Set by the driver.

> + */

> +#define V4L2_SUBDEV_ROUTE_FL_IMMUTABLE		BIT(1)

> +

> +/**

> + * struct v4l2_subdev_route - A route inside a subdev

> + *

> + * @sink_pad: the sink pad index

> + * @sink_stream: the sink stream identifier

> + * @source_pad: the source pad index

> + * @source_stream: the source stream identifier

> + * @flags: route flags V4L2_SUBDEV_ROUTE_FL_*

> + * @reserved: drivers and applications must zero this array

> + */

> +struct v4l2_subdev_route {

> +	__u32 sink_pad;

> +	__u32 sink_stream;

> +	__u32 source_pad;

> +	__u32 source_stream;

> +	__u32 flags;

> +	__u32 reserved[5];

> +};

> +

> +/**

> + * struct v4l2_subdev_routing - Subdev routing information

> + *

> + * @which: configuration type (from enum v4l2_subdev_format_whence)

> + * @routes: pointer to the routes array

> + * @num_routes: the total number of routes in the routes array

> + * @reserved: drivers and applications must zero this array

> + */

> +struct v4l2_subdev_routing {

> +	__u32 which;

> +	__u64 routes;

> +	__u32 num_routes;

> +	__u32 reserved[5];

> +};

> +

>  /* Backwards compatibility define --- to be removed */

>  #define v4l2_subdev_edid v4l2_edid

>  

> @@ -203,6 +249,8 @@ struct v4l2_subdev_capability {

>  #define VIDIOC_SUBDEV_S_CROP			_IOWR('V', 60, struct v4l2_subdev_crop)

>  #define VIDIOC_SUBDEV_G_SELECTION		_IOWR('V', 61, struct v4l2_subdev_selection)

>  #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)

>  /* 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)


-- 
Regards,

Laurent Pinchart
Tomi Valkeinen April 29, 2021, 6:15 a.m. UTC | #2
On 29/04/2021 05:28, Laurent Pinchart wrote:
> Hi Tomi,

> 

> Thank you for the patch.

> 

> For some reason, it seems the original authorship got lost. It was fine

> in v5. I haven't checked if other patches in the series are affected.

> 


Thanks for pointing this out. I found the issue in my tooling. I'll fix 
the authorship information.

  Tomi
Tomi Valkeinen May 4, 2021, 7:04 a.m. UTC | #3
On 29/04/2021 05:28, Laurent Pinchart wrote:
> Hi Tomi,

> 

> Thank you for the patch.

> 

> For some reason, it seems the original authorship got lost. It was fine

> in v5. I haven't checked if other patches in the series are affected.

> 

> On Tue, Apr 27, 2021 at 03:45:17PM +0300, Tomi Valkeinen wrote:

>> Add support for subdev internal routing. A route is defined as a single

>> stream from a sink pad to a source pad.

>>

>> The userspace can configure the routing via two new ioctls,

>> VIDIOC_SUBDEV_G_ROUTING and VIDIOC_SUBDEV_S_ROUTING, and subdevs can

>> implement the functionality with v4l2_subdev_pad_ops.get_routing() and

>> v4l2_subdev_pad_ops.set_routing().

>>

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

>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

>>

>> - Add sink and source streams for multiplexed links

>> - Copy the argument back in case of an error. This is needed to let the

>>    caller know the number of routes.

>>

>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

>> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

>>

>> - Expand and refine documentation.

>> - Make the 'routes' pointer a __u64 __user pointer so that a compat32

>>    version of the ioctl is not required.

>> - Add struct v4l2_subdev_krouting to be used for subdevice operations.

>>

>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

>>

>> - Fix typecasing warnings

>> - Check sink & source pad types

>> - Add 'which' field

>>

>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

>> ---

>>   drivers/media/v4l2-core/v4l2-ioctl.c  | 25 +++++++++++++-

>>   drivers/media/v4l2-core/v4l2-subdev.c | 46 +++++++++++++++++++++++++

>>   include/media/v4l2-subdev.h           | 24 ++++++++++++++

>>   include/uapi/linux/v4l2-subdev.h      | 48 +++++++++++++++++++++++++++

>>   4 files changed, 142 insertions(+), 1 deletion(-)

>>

>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c

>> index 6a5d1c6d11d6..7b1dabc23909 100644

>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c

>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c

>> @@ -16,6 +16,7 @@

>>   #include <linux/kernel.h>

>>   #include <linux/version.h>

>>   

>> +#include <linux/v4l2-subdev.h>

>>   #include <linux/videodev2.h>

>>   

>>   #include <media/v4l2-common.h>

>> @@ -3108,6 +3109,21 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,

>>   		ret = 1;

>>   		break;

>>   	}

>> +

>> +	case VIDIOC_SUBDEV_G_ROUTING:

>> +	case VIDIOC_SUBDEV_S_ROUTING: {

>> +		struct v4l2_subdev_routing *routing = parg;

>> +

>> +		if (routing->num_routes > 256)

>> +			return -EINVAL;

>> +

>> +		*user_ptr = u64_to_user_ptr(routing->routes);

>> +		*kernel_ptr = (void **)&routing->routes;

>> +		*array_size = sizeof(struct v4l2_subdev_route)

>> +			    * routing->num_routes;

>> +		ret = 1;

>> +		break;

>> +	}

>>   	}

>>   

>>   	return ret;

>> @@ -3369,8 +3385,15 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,

>>   	/*

>>   	 * Some ioctls can return an error, but still have valid

>>   	 * results that must be returned.

>> +	 *

>> +	 * FIXME: subdev IOCTLS are partially handled here and partially in

>> +	 * v4l2-subdev.c and the 'always_copy' flag can only be set for IOCTLS

>> +	 * defined here as part of the 'v4l2_ioctls' array. As

>> +	 * VIDIOC_SUBDEV_G_ROUTING needs to return results to applications even

>> +	 * in case of failure, but it is not defined here as part of the

>> +	 * 'v4l2_ioctls' array, insert an ad-hoc check to address that.

>>   	 */

>> -	if (err < 0 && !always_copy)

>> +	if (err < 0 && !always_copy && cmd != VIDIOC_SUBDEV_G_ROUTING)

>>   		goto out;

>>   

>>   out_array_args:

>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c

>> index 956dafab43d4..ad79ce121cee 100644

>> --- a/drivers/media/v4l2-core/v4l2-subdev.c

>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c

>> @@ -681,6 +681,52 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)

>>   	case VIDIOC_SUBDEV_QUERYSTD:

>>   		return v4l2_subdev_call(sd, video, querystd, arg);

>>   

>> +	case VIDIOC_SUBDEV_G_ROUTING: {

>> +		struct v4l2_subdev_routing *routing = arg;

>> +		struct v4l2_subdev_krouting krouting = {

>> +			.which = routing->which,

>> +			.num_routes = routing->num_routes,

>> +			.routes = (struct v4l2_subdev_route *)(uintptr_t)

>> +					  routing->routes,

>> +		};

>> +		int ret;

>> +

>> +		ret = v4l2_subdev_call(sd, pad, get_routing, &krouting);

>> +

>> +		routing->num_routes = krouting.num_routes;

>> +

>> +		return ret;

>> +	}

>> +

>> +	case VIDIOC_SUBDEV_S_ROUTING: {

>> +		struct v4l2_subdev_routing *routing = arg;

>> +		struct v4l2_subdev_route *routes =

>> +			(struct v4l2_subdev_route *)(uintptr_t)routing->routes;

>> +		struct v4l2_subdev_krouting krouting = {};

>> +		unsigned int i;

>> +

>> +		if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)

>> +			return -EPERM;

>> +

>> +		for (i = 0; i < routing->num_routes; ++i) {

>> +			if (routes[i].sink_pad >= sd->entity.num_pads ||

>> +			    routes[i].source_pad >= sd->entity.num_pads)

>> +				return -EINVAL;

>> +

>> +			if (!(sd->entity.pads[routes[i].sink_pad].flags &

>> +			      MEDIA_PAD_FL_SINK) ||

>> +			    !(sd->entity.pads[routes[i].source_pad].flags &

>> +			      MEDIA_PAD_FL_SOURCE))

>> +				return -EINVAL;

>> +		}

>> +

>> +		krouting.which = routing->which;

>> +		krouting.num_routes = routing->num_routes;

>> +		krouting.routes = routes;

>> +

>> +		return v4l2_subdev_call(sd, pad, set_routing, &krouting);

>> +	}

>> +

>>   	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 70a21298020b..c45f5f0156c9 100644

>> --- a/include/media/v4l2-subdev.h

>> +++ b/include/media/v4l2-subdev.h

>> @@ -660,6 +660,22 @@ struct v4l2_subdev_pad_config {

>>   	struct v4l2_rect try_compose;

>>   };

>>   

>> +/**

>> + * struct v4l2_subdev_krouting - subdev routing table

>> + *

>> + * @which: format type (from enum v4l2_subdev_format_whence)

>> + * @routes: &struct v4l2_subdev_route

>> + * @num_routes: number of routes

>> + *

>> + * This structure is used to translate arguments received from

>> + * VIDIOC_SUBDEV_G/S_ROUTING() ioctl to subdev device drivers operations.

>> + */

>> +struct v4l2_subdev_krouting {

>> +	u32 which;

>> +	struct v4l2_subdev_route *routes;

>> +	unsigned int num_routes;

>> +};

>> +

>>   /**

>>    * struct v4l2_subdev_pad_ops - v4l2-subdev pad level operations

>>    *

>> @@ -721,6 +737,10 @@ struct v4l2_subdev_pad_config {

>>    *		     applied to the hardware. The operation shall fail if the

>>    *		     pad index it has been called on is not valid or in case of

>>    *		     unrecoverable failures.

>> + *

>> + * @get_routing: get the subdevice routing table.

>> + * @set_routing: enable or disable data connection routes described in the

>> + *		 subdevice routing table.

>>    */

>>   struct v4l2_subdev_pad_ops {

>>   	int (*init_cfg)(struct v4l2_subdev *sd,

>> @@ -765,6 +785,10 @@ struct v4l2_subdev_pad_ops {

>>   			       struct v4l2_mbus_config *config);

>>   	int (*set_mbus_config)(struct v4l2_subdev *sd, unsigned int pad,

>>   			       struct v4l2_mbus_config *config);

>> +	int (*get_routing)(struct v4l2_subdev *sd,

>> +			   struct v4l2_subdev_krouting *route);

>> +	int (*set_routing)(struct v4l2_subdev *sd,

>> +			   struct v4l2_subdev_krouting *route);

>>   };

>>   

>>   /**

>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h

>> index 658106f5b5dc..3a360cdcacf4 100644

>> --- a/include/uapi/linux/v4l2-subdev.h

>> +++ b/include/uapi/linux/v4l2-subdev.h

>> @@ -188,6 +188,52 @@ struct v4l2_subdev_capability {

>>   /* The v4l2 sub-device video device node is registered in read-only mode. */

>>   #define V4L2_SUBDEV_CAP_RO_SUBDEV		0x00000001

>>   

>> +/*

> 

> Shouldn't this be /** ?

> 

>> + * Is the route active? An active route will start when streaming is enabled

>> + * on a video node.

> 

> Even with an a priori knowledge of the topic, I'm not sure to really

> understand this, and I don't think it reflects the purpose of the flag.

> 

>> + */

>> +#define V4L2_SUBDEV_ROUTE_FL_ACTIVE		BIT(0)

>> +

>> +/*

>> + * Is the route immutable, i.e. can it be activated and inactivated?

> 

> Maybe adding "An immutable route is always active".


Is it? I was thinking that immutable could be used as a way to have a 
static routing table, but the routes could still be enabled and 
disables. Say, a sensor could have pixel and metadata streams. They 
would be immutable, you can't remove them, change them or add any other 
streams. But you could still enable/disable them.

But the doc doesn't convey this. And I think we would still need a way 
to describe what you mentioned, a route that's always there and enabled.

  Tomi
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 6a5d1c6d11d6..7b1dabc23909 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -16,6 +16,7 @@ 
 #include <linux/kernel.h>
 #include <linux/version.h>
 
+#include <linux/v4l2-subdev.h>
 #include <linux/videodev2.h>
 
 #include <media/v4l2-common.h>
@@ -3108,6 +3109,21 @@  static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
 		ret = 1;
 		break;
 	}
+
+	case VIDIOC_SUBDEV_G_ROUTING:
+	case VIDIOC_SUBDEV_S_ROUTING: {
+		struct v4l2_subdev_routing *routing = parg;
+
+		if (routing->num_routes > 256)
+			return -EINVAL;
+
+		*user_ptr = u64_to_user_ptr(routing->routes);
+		*kernel_ptr = (void **)&routing->routes;
+		*array_size = sizeof(struct v4l2_subdev_route)
+			    * routing->num_routes;
+		ret = 1;
+		break;
+	}
 	}
 
 	return ret;
@@ -3369,8 +3385,15 @@  video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
 	/*
 	 * Some ioctls can return an error, but still have valid
 	 * results that must be returned.
+	 *
+	 * FIXME: subdev IOCTLS are partially handled here and partially in
+	 * v4l2-subdev.c and the 'always_copy' flag can only be set for IOCTLS
+	 * defined here as part of the 'v4l2_ioctls' array. As
+	 * VIDIOC_SUBDEV_G_ROUTING needs to return results to applications even
+	 * in case of failure, but it is not defined here as part of the
+	 * 'v4l2_ioctls' array, insert an ad-hoc check to address that.
 	 */
-	if (err < 0 && !always_copy)
+	if (err < 0 && !always_copy && cmd != VIDIOC_SUBDEV_G_ROUTING)
 		goto out;
 
 out_array_args:
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 956dafab43d4..ad79ce121cee 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -681,6 +681,52 @@  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_QUERYSTD:
 		return v4l2_subdev_call(sd, video, querystd, arg);
 
+	case VIDIOC_SUBDEV_G_ROUTING: {
+		struct v4l2_subdev_routing *routing = arg;
+		struct v4l2_subdev_krouting krouting = {
+			.which = routing->which,
+			.num_routes = routing->num_routes,
+			.routes = (struct v4l2_subdev_route *)(uintptr_t)
+					  routing->routes,
+		};
+		int ret;
+
+		ret = v4l2_subdev_call(sd, pad, get_routing, &krouting);
+
+		routing->num_routes = krouting.num_routes;
+
+		return ret;
+	}
+
+	case VIDIOC_SUBDEV_S_ROUTING: {
+		struct v4l2_subdev_routing *routing = arg;
+		struct v4l2_subdev_route *routes =
+			(struct v4l2_subdev_route *)(uintptr_t)routing->routes;
+		struct v4l2_subdev_krouting krouting = {};
+		unsigned int i;
+
+		if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
+			return -EPERM;
+
+		for (i = 0; i < routing->num_routes; ++i) {
+			if (routes[i].sink_pad >= sd->entity.num_pads ||
+			    routes[i].source_pad >= sd->entity.num_pads)
+				return -EINVAL;
+
+			if (!(sd->entity.pads[routes[i].sink_pad].flags &
+			      MEDIA_PAD_FL_SINK) ||
+			    !(sd->entity.pads[routes[i].source_pad].flags &
+			      MEDIA_PAD_FL_SOURCE))
+				return -EINVAL;
+		}
+
+		krouting.which = routing->which;
+		krouting.num_routes = routing->num_routes;
+		krouting.routes = routes;
+
+		return v4l2_subdev_call(sd, pad, set_routing, &krouting);
+	}
+
 	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 70a21298020b..c45f5f0156c9 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -660,6 +660,22 @@  struct v4l2_subdev_pad_config {
 	struct v4l2_rect try_compose;
 };
 
+/**
+ * struct v4l2_subdev_krouting - subdev routing table
+ *
+ * @which: format type (from enum v4l2_subdev_format_whence)
+ * @routes: &struct v4l2_subdev_route
+ * @num_routes: number of routes
+ *
+ * This structure is used to translate arguments received from
+ * VIDIOC_SUBDEV_G/S_ROUTING() ioctl to subdev device drivers operations.
+ */
+struct v4l2_subdev_krouting {
+	u32 which;
+	struct v4l2_subdev_route *routes;
+	unsigned int num_routes;
+};
+
 /**
  * struct v4l2_subdev_pad_ops - v4l2-subdev pad level operations
  *
@@ -721,6 +737,10 @@  struct v4l2_subdev_pad_config {
  *		     applied to the hardware. The operation shall fail if the
  *		     pad index it has been called on is not valid or in case of
  *		     unrecoverable failures.
+ *
+ * @get_routing: get the subdevice routing table.
+ * @set_routing: enable or disable data connection routes described in the
+ *		 subdevice routing table.
  */
 struct v4l2_subdev_pad_ops {
 	int (*init_cfg)(struct v4l2_subdev *sd,
@@ -765,6 +785,10 @@  struct v4l2_subdev_pad_ops {
 			       struct v4l2_mbus_config *config);
 	int (*set_mbus_config)(struct v4l2_subdev *sd, unsigned int pad,
 			       struct v4l2_mbus_config *config);
+	int (*get_routing)(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_krouting *route);
+	int (*set_routing)(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_krouting *route);
 };
 
 /**
diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index 658106f5b5dc..3a360cdcacf4 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -188,6 +188,52 @@  struct v4l2_subdev_capability {
 /* The v4l2 sub-device video device node is registered in read-only mode. */
 #define V4L2_SUBDEV_CAP_RO_SUBDEV		0x00000001
 
+/*
+ * Is the route active? An active route will start when streaming is enabled
+ * on a video node.
+ */
+#define V4L2_SUBDEV_ROUTE_FL_ACTIVE		BIT(0)
+
+/*
+ * Is the route immutable, i.e. can it be activated and inactivated?
+ * Set by the driver.
+ */
+#define V4L2_SUBDEV_ROUTE_FL_IMMUTABLE		BIT(1)
+
+/**
+ * struct v4l2_subdev_route - A route inside a subdev
+ *
+ * @sink_pad: the sink pad index
+ * @sink_stream: the sink stream identifier
+ * @source_pad: the source pad index
+ * @source_stream: the source stream identifier
+ * @flags: route flags V4L2_SUBDEV_ROUTE_FL_*
+ * @reserved: drivers and applications must zero this array
+ */
+struct v4l2_subdev_route {
+	__u32 sink_pad;
+	__u32 sink_stream;
+	__u32 source_pad;
+	__u32 source_stream;
+	__u32 flags;
+	__u32 reserved[5];
+};
+
+/**
+ * struct v4l2_subdev_routing - Subdev routing information
+ *
+ * @which: configuration type (from enum v4l2_subdev_format_whence)
+ * @routes: pointer to the routes array
+ * @num_routes: the total number of routes in the routes array
+ * @reserved: drivers and applications must zero this array
+ */
+struct v4l2_subdev_routing {
+	__u32 which;
+	__u64 routes;
+	__u32 num_routes;
+	__u32 reserved[5];
+};
+
 /* Backwards compatibility define --- to be removed */
 #define v4l2_subdev_edid v4l2_edid
 
@@ -203,6 +249,8 @@  struct v4l2_subdev_capability {
 #define VIDIOC_SUBDEV_S_CROP			_IOWR('V', 60, struct v4l2_subdev_crop)
 #define VIDIOC_SUBDEV_G_SELECTION		_IOWR('V', 61, struct v4l2_subdev_selection)
 #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)
 /* 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)