| Message ID | 20210830110116.488338-29-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | v4l: subdev internal routing and streams | expand |
Hi Tomi, On Mon, Aug 30, 2021 at 02:01:08PM +0300, Tomi Valkeinen wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > 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.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> > > - 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 > - Add V4L2_SUBDEV_ROUTE_FL_SOURCE > - Routing to subdev state > - Dropped get_routing subdev op > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/v4l2-core/v4l2-ioctl.c | 25 ++++++++- > drivers/media/v4l2-core/v4l2-subdev.c | 75 +++++++++++++++++++++++++++ > include/media/v4l2-subdev.h | 24 +++++++++ > include/uapi/linux/v4l2-subdev.h | 57 ++++++++++++++++++++ > 4 files changed, 180 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index 05d5db3d85e5..8e9315ffcb99 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> > @@ -3065,6 +3066,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; > @@ -3328,8 +3344,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 b1e65488210d..0e1f325b3159 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -395,6 +395,12 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh, > which = ((struct v4l2_subdev_selection *)arg)->which; > break; > } > + > + case VIDIOC_SUBDEV_G_ROUTING: > + case VIDIOC_SUBDEV_S_ROUTING: { > + which = ((struct v4l2_subdev_routing *)arg)->which; > + break; > + } > } > > return which == V4L2_SUBDEV_FORMAT_TRY ? > @@ -711,6 +717,74 @@ 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; > + > + if (!(sd->flags & V4L2_SUBDEV_FL_MULTIPLEXED)) > + return -ENOIOCTLCMD; > + > + memset(routing->reserved, 0, sizeof(routing->reserved)); > + > + krouting = &state->routing; > + > + if (routing->num_routes < krouting->num_routes) { > + routing->num_routes = krouting->num_routes; > + return -ENOSPC; > + } > + > + memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes, > + krouting->routes, > + krouting->num_routes * sizeof(*krouting->routes)); > + routing->num_routes = krouting->num_routes; > + > + return 0; > + } > + > + 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 (!(sd->flags & V4L2_SUBDEV_FL_MULTIPLEXED)) > + return -ENOIOCTLCMD; > + > + if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev) > + return -EPERM; > + > + memset(routing->reserved, 0, sizeof(routing->reserved)); > + > + for (i = 0; i < routing->num_routes; ++i) { > + const struct v4l2_subdev_route *route = &routes[i]; > + const struct media_pad *pads = sd->entity.pads; > + > + /* Do not check sink pad for source routes */ > + if (!(route->flags & V4L2_SUBDEV_ROUTE_FL_SOURCE)) { > + if (route->sink_pad >= sd->entity.num_pads) > + return -EINVAL; > + > + if (!(pads[route->sink_pad].flags & > + MEDIA_PAD_FL_SINK)) > + return -EINVAL; > + } > + > + if (route->source_pad >= sd->entity.num_pads) > + return -EINVAL; > + > + if (!(pads[route->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, state, &krouting); > + } > + > default: > return v4l2_subdev_call(sd, core, ioctl, cmd, arg); > } > @@ -975,6 +1049,7 @@ void v4l2_free_subdev_state(struct v4l2_subdev_state *state) > > mutex_destroy(&state->lock); > > + kvfree(state->routing.routes); > kvfree(state->pads); > kfree(state); > } > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 356901d8a948..cd6aad21ae0c 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -694,12 +694,29 @@ 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_state - Used for storing subdev state information. > * > * @lock: mutex for the state > * @which: state type (from enum v4l2_subdev_format_whence) > * @pads: &struct v4l2_subdev_pad_config array > + * @routing: routing table for the subdev > * > * This structure only needs to be passed to the pad op if the 'which' field > * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For > @@ -709,6 +726,7 @@ struct v4l2_subdev_state { > struct mutex lock; > u32 which; > struct v4l2_subdev_pad_config *pads; > + struct v4l2_subdev_krouting routing; > }; > > /** > @@ -772,6 +790,9 @@ struct v4l2_subdev_state { > * 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. > + * > + * @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, > @@ -816,6 +837,9 @@ 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 (*set_routing)(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, > + struct v4l2_subdev_krouting *route); > }; > > /** > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h > index 658106f5b5dc..3aa623e0e5f9 100644 > --- a/include/uapi/linux/v4l2-subdev.h > +++ b/include/uapi/linux/v4l2-subdev.h > @@ -188,6 +188,61 @@ 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) Am I wrong or BIT() is not available to userspace ? ../../include/linux/v4l2-subdev.h:209:38: error: ‘BIT’ was not declared in this scope 209 | #define V4L2_SUBDEV_ROUTE_FL_ACTIVE BIT(0) Header was exported with headers_install. > + > +/* > + * Is the route immutable, i.e. can it be activated and inactivated? > + * Set by the driver. > + */ > +#define V4L2_SUBDEV_ROUTE_FL_IMMUTABLE BIT(1) > + > +/* > + * Is the route a source endpoint? A source endpoint route refers to a stream > + * generated internally by the subdevice (usually a sensor), and thus there > + * is no sink-side endpoint for the route. The sink_pad and sink_stream > + * fields are unused. > + * Set by the driver. > + */ > +#define V4L2_SUBDEV_ROUTE_FL_SOURCE BIT(2) > + > +/** > + * 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 +258,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) > -- > 2.25.1 >
Hi, On 15/09/2021 19:10, Jacopo Mondi wrote: >> +#define V4L2_SUBDEV_ROUTE_FL_ACTIVE BIT(0) > > Am I wrong or BIT() is not available to userspace ? > > ../../include/linux/v4l2-subdev.h:209:38: error: ‘BIT’ was not declared in this scope > 209 | #define V4L2_SUBDEV_ROUTE_FL_ACTIVE BIT(0) > > Header was exported with headers_install. Yes, I think you are correct. I see only two other BIT() uses in uapi headers, and those might be errors too. Tomi
Hi, On 30.08.21 13:01, Tomi Valkeinen wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > 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.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> > > - 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 > - Add V4L2_SUBDEV_ROUTE_FL_SOURCE > - Routing to subdev state > - Dropped get_routing subdev op > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > drivers/media/v4l2-core/v4l2-ioctl.c | 25 ++++++++- > drivers/media/v4l2-core/v4l2-subdev.c | 75 +++++++++++++++++++++++++++ > include/media/v4l2-subdev.h | 24 +++++++++ > include/uapi/linux/v4l2-subdev.h | 57 ++++++++++++++++++++ > 4 files changed, 180 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index 05d5db3d85e5..8e9315ffcb99 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> > @@ -3065,6 +3066,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; > @@ -3328,8 +3344,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 b1e65488210d..0e1f325b3159 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -395,6 +395,12 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh, > which = ((struct v4l2_subdev_selection *)arg)->which; > break; > } > + > + case VIDIOC_SUBDEV_G_ROUTING: > + case VIDIOC_SUBDEV_S_ROUTING: { > + which = ((struct v4l2_subdev_routing *)arg)->which; > + break; > + } > } > > return which == V4L2_SUBDEV_FORMAT_TRY ? > @@ -711,6 +717,74 @@ 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; > + > + if (!(sd->flags & V4L2_SUBDEV_FL_MULTIPLEXED)) > + return -ENOIOCTLCMD; > + > + memset(routing->reserved, 0, sizeof(routing->reserved)); > + > + krouting = &state->routing; > + > + if (routing->num_routes < krouting->num_routes) { > + routing->num_routes = krouting->num_routes; > + return -ENOSPC; > + } > + > + memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes, > + krouting->routes, > + krouting->num_routes * sizeof(*krouting->routes)); > + routing->num_routes = krouting->num_routes; > + > + return 0; > + } > + > + 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 (!(sd->flags & V4L2_SUBDEV_FL_MULTIPLEXED)) > + return -ENOIOCTLCMD; > + > + if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev) > + return -EPERM; > + > + memset(routing->reserved, 0, sizeof(routing->reserved)); > + > + for (i = 0; i < routing->num_routes; ++i) { > + const struct v4l2_subdev_route *route = &routes[i]; > + const struct media_pad *pads = sd->entity.pads; > + > + /* Do not check sink pad for source routes */ > + if (!(route->flags & V4L2_SUBDEV_ROUTE_FL_SOURCE)) { > + if (route->sink_pad >= sd->entity.num_pads) > + return -EINVAL; > + > + if (!(pads[route->sink_pad].flags & > + MEDIA_PAD_FL_SINK)) > + return -EINVAL; > + } > + > + if (route->source_pad >= sd->entity.num_pads) > + return -EINVAL; > + > + if (!(pads[route->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, state, &krouting); > + } > + > default: > return v4l2_subdev_call(sd, core, ioctl, cmd, arg); > } > @@ -975,6 +1049,7 @@ void v4l2_free_subdev_state(struct v4l2_subdev_state *state) > > mutex_destroy(&state->lock); > > + kvfree(state->routing.routes); > kvfree(state->pads); > kfree(state); > } > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 356901d8a948..cd6aad21ae0c 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -694,12 +694,29 @@ 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_state - Used for storing subdev state information. > * > * @lock: mutex for the state > * @which: state type (from enum v4l2_subdev_format_whence) > * @pads: &struct v4l2_subdev_pad_config array > + * @routing: routing table for the subdev > * > * This structure only needs to be passed to the pad op if the 'which' field > * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For > @@ -709,6 +726,7 @@ struct v4l2_subdev_state { > struct mutex lock; > u32 which; > struct v4l2_subdev_pad_config *pads; > + struct v4l2_subdev_krouting routing; > }; > > /** > @@ -772,6 +790,9 @@ struct v4l2_subdev_state { > * 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. > + * > + * @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, > @@ -816,6 +837,9 @@ 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 (*set_routing)(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, > + struct v4l2_subdev_krouting *route); > }; > > /** > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h > index 658106f5b5dc..3aa623e0e5f9 100644 > --- a/include/uapi/linux/v4l2-subdev.h > +++ b/include/uapi/linux/v4l2-subdev.h > @@ -188,6 +188,61 @@ 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) > + > +/* > + * Is the route a source endpoint? A source endpoint route refers to a stream > + * generated internally by the subdevice (usually a sensor), and thus there > + * is no sink-side endpoint for the route. The sink_pad and sink_stream > + * fields are unused. > + * Set by the driver. > + */ > +#define V4L2_SUBDEV_ROUTE_FL_SOURCE BIT(2) > + > +/** > + * 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]; > +}; > + I don't understand that struct, what is the meaning of the two sink_stream, source_stream fields? What is the relation between sink_pad and source_pad? A 'route' between two pads means that 'streams' can flow through them? If I have for example: sink_pad = sink_stream = 0 source_pad = source_stream = 1 what does that mean? Thanks, Dafna > +/** > + * 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 +258,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) >
Hi, On 03/10/2021 22:52, Dafna Hirschfeld wrote: >> +/** >> + * 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]; >> +}; >> + > > I don't understand that struct, what is the meaning of the two > sink_stream, source_stream fields? > What is the relation between sink_pad and source_pad? A 'route' between > two pads means that 'streams' can flow through them? > > If I have for example: > > sink_pad = sink_stream = 0 > source_pad = source_stream = 1 > > what does that mean? It means that a stream with stream ID 0 that comes to the subdev's sink pad 0 will be routed to the subdev's source pad 1 with stream ID 1. So the sink_pad and source_pad tell via which pads the streams flows. Stream IDs are numbers used to uniquely identify the stream for the specific pad. In other words, for the subdev's pad 0, each stream ID must be unique. That pad 0 is linked (media link) to another subdev's pad, and on that pad there must be streams with the matching IDs. Tomi
On 04.10.21 07:15, Tomi Valkeinen wrote: > Hi, > > On 03/10/2021 22:52, Dafna Hirschfeld wrote: > >>> +/** >>> + * 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]; >>> +}; >>> + >> >> I don't understand that struct, what is the meaning of the two sink_stream, source_stream fields? >> What is the relation between sink_pad and source_pad? A 'route' between two pads means that 'streams' can flow through them? >> >> If I have for example: >> >> sink_pad = sink_stream = 0 >> source_pad = source_stream = 1 >> >> what does that mean? > > It means that a stream with stream ID 0 that comes to the subdev's sink pad 0 will be routed to the subdev's source pad 1 with stream ID 1. > > So the sink_pad and source_pad tell via which pads the streams flows. > > Stream IDs are numbers used to uniquely identify the stream for the specific pad. In other words, for the subdev's pad 0, each stream ID must be unique. That pad 0 is linked (media link) to another subdev's pad, and on that pad there must be streams with the matching IDs. Ok, so the stream ID for the same stream changes from link to link. So different streams can have the same ID on different pads? Won't it be more simple if the stream ID is forced to stay the same along all the links and pads? > > Tomi
Hi, On 05/10/2021 13:19, Dafna Hirschfeld wrote: > > > On 04.10.21 07:15, Tomi Valkeinen wrote: >> Hi, >> >> On 03/10/2021 22:52, Dafna Hirschfeld wrote: >> >>>> +/** >>>> + * 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]; >>>> +}; >>>> + >>> >>> I don't understand that struct, what is the meaning of the two >>> sink_stream, source_stream fields? >>> What is the relation between sink_pad and source_pad? A 'route' >>> between two pads means that 'streams' can flow through them? >>> >>> If I have for example: >>> >>> sink_pad = sink_stream = 0 >>> source_pad = source_stream = 1 >>> >>> what does that mean? >> >> It means that a stream with stream ID 0 that comes to the subdev's >> sink pad 0 will be routed to the subdev's source pad 1 with stream ID 1. >> >> So the sink_pad and source_pad tell via which pads the streams flows. >> >> Stream IDs are numbers used to uniquely identify the stream for the >> specific pad. In other words, for the subdev's pad 0, each stream ID >> must be unique. That pad 0 is linked (media link) to another subdev's >> pad, and on that pad there must be streams with the matching IDs. Btw, I've just sent v9 of the series, which includes documentation that covers this. > Ok, so the stream ID for the same stream changes from link to link. Not necessarily, but yes, it may. > So different streams can have the same ID on different pads? True. > Won't it be more simple if the stream ID is forced to stay the same > along all the links and pads? I don't think so. For one, it would be difficult, or at least laborious, to verify that the above holds true. Also, streams can appear and disappear, e.g. a single stream in a subdev's input side might be split or duplicated into multiple streams on the output side. And I think that it makes sense that e.g. a sensor that outputs two streams (pixel & metadata) can have a hardcoded routing, with pad 0 and streams 0 and 1. Stream 0 is always the pixel data, and stream 1 is always the metadata, and the user knows this so he can connect those streams appropriately. If you have multiple cameras, such a scheme would not be possible. What would be the benefit of such rule? Afaik, it would be just for the benefit of the human who writes the configuration for the pipeline. And with the current series, nothing prevents the user from assigning "unique" stream IDs for the pipeline being configured. You can pick stream ID 6001 for pixel stream from camera 1, and keep that stream ID for the whole pipeline for that stream (assuming the stream won't be split). That said, I agree that "stream ID" kind of sounds like something used to identify the stream, regardless where in the pipeline we look. But that is not the case. As I write in the docs: "A stream ID (often just "stream") is a media link-local identifier for a stream". Tomi
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 05d5db3d85e5..8e9315ffcb99 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> @@ -3065,6 +3066,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; @@ -3328,8 +3344,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 b1e65488210d..0e1f325b3159 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -395,6 +395,12 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh, which = ((struct v4l2_subdev_selection *)arg)->which; break; } + + case VIDIOC_SUBDEV_G_ROUTING: + case VIDIOC_SUBDEV_S_ROUTING: { + which = ((struct v4l2_subdev_routing *)arg)->which; + break; + } } return which == V4L2_SUBDEV_FORMAT_TRY ? @@ -711,6 +717,74 @@ 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; + + if (!(sd->flags & V4L2_SUBDEV_FL_MULTIPLEXED)) + return -ENOIOCTLCMD; + + memset(routing->reserved, 0, sizeof(routing->reserved)); + + krouting = &state->routing; + + if (routing->num_routes < krouting->num_routes) { + routing->num_routes = krouting->num_routes; + return -ENOSPC; + } + + memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes, + krouting->routes, + krouting->num_routes * sizeof(*krouting->routes)); + routing->num_routes = krouting->num_routes; + + return 0; + } + + 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 (!(sd->flags & V4L2_SUBDEV_FL_MULTIPLEXED)) + return -ENOIOCTLCMD; + + if (routing->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev) + return -EPERM; + + memset(routing->reserved, 0, sizeof(routing->reserved)); + + for (i = 0; i < routing->num_routes; ++i) { + const struct v4l2_subdev_route *route = &routes[i]; + const struct media_pad *pads = sd->entity.pads; + + /* Do not check sink pad for source routes */ + if (!(route->flags & V4L2_SUBDEV_ROUTE_FL_SOURCE)) { + if (route->sink_pad >= sd->entity.num_pads) + return -EINVAL; + + if (!(pads[route->sink_pad].flags & + MEDIA_PAD_FL_SINK)) + return -EINVAL; + } + + if (route->source_pad >= sd->entity.num_pads) + return -EINVAL; + + if (!(pads[route->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, state, &krouting); + } + default: return v4l2_subdev_call(sd, core, ioctl, cmd, arg); } @@ -975,6 +1049,7 @@ void v4l2_free_subdev_state(struct v4l2_subdev_state *state) mutex_destroy(&state->lock); + kvfree(state->routing.routes); kvfree(state->pads); kfree(state); } diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 356901d8a948..cd6aad21ae0c 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -694,12 +694,29 @@ 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_state - Used for storing subdev state information. * * @lock: mutex for the state * @which: state type (from enum v4l2_subdev_format_whence) * @pads: &struct v4l2_subdev_pad_config array + * @routing: routing table for the subdev * * This structure only needs to be passed to the pad op if the 'which' field * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For @@ -709,6 +726,7 @@ struct v4l2_subdev_state { struct mutex lock; u32 which; struct v4l2_subdev_pad_config *pads; + struct v4l2_subdev_krouting routing; }; /** @@ -772,6 +790,9 @@ struct v4l2_subdev_state { * 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. + * + * @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, @@ -816,6 +837,9 @@ 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 (*set_routing)(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state, + struct v4l2_subdev_krouting *route); }; /** diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h index 658106f5b5dc..3aa623e0e5f9 100644 --- a/include/uapi/linux/v4l2-subdev.h +++ b/include/uapi/linux/v4l2-subdev.h @@ -188,6 +188,61 @@ 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) + +/* + * Is the route a source endpoint? A source endpoint route refers to a stream + * generated internally by the subdevice (usually a sensor), and thus there + * is no sink-side endpoint for the route. The sink_pad and sink_stream + * fields are unused. + * Set by the driver. + */ +#define V4L2_SUBDEV_ROUTE_FL_SOURCE BIT(2) + +/** + * 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 +258,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)