From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: linux-media@vger.kernel.org, sakari.ailus@linux.intel.com,
Jacopo Mondi <jacopo.mondi@ideasonboard.com>,
niklas.soderlund+renesas@ragnatech.se,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
satish.nagireddy@getcruise.com
Subject: Re: [PATCH v4 2/8] media-ctl: Add support for routes and streams
Date: Mon, 24 Apr 2023 10:29:32 +0300 [thread overview]
Message-ID: <20230424072932.GD4926@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20230421124428.393261-3-tomi.valkeinen@ideasonboard.com>
Hi Tomi,
Thank you for the patch.
On Fri, Apr 21, 2023 at 03:44:22PM +0300, Tomi Valkeinen wrote:
> Add support to get and set subdev routes and to get and set
> configurations per stream.
>
> Based on work from Sakari Ailus <sakari.ailus@linux.intel.com>.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> utils/media-ctl/libmediactl.c | 41 +++++
> utils/media-ctl/libv4l2subdev.c | 281 ++++++++++++++++++++++++++++----
> utils/media-ctl/media-ctl.c | 121 ++++++++++++--
> utils/media-ctl/mediactl.h | 16 ++
> utils/media-ctl/options.c | 15 +-
> utils/media-ctl/options.h | 1 +
> utils/media-ctl/v4l2subdev.h | 58 ++++++-
> 7 files changed, 476 insertions(+), 57 deletions(-)
>
> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> index a18b063e..c32fe56a 100644
> --- a/utils/media-ctl/libmediactl.c
> +++ b/utils/media-ctl/libmediactl.c
> @@ -874,6 +874,47 @@ struct media_pad *media_parse_pad(struct media_device *media,
> return &entity->pads[pad];
> }
>
> +struct media_pad *media_parse_pad_stream(struct media_device *media,
> + const char *p, unsigned int *stream,
> + char **endp)
> +{
> + struct media_pad *pad;
> + const char *orig_p = p;
> + char *ep;
> +
> + pad = media_parse_pad(media, p, &ep);
> + if (pad == NULL)
> + return NULL;
> +
> + p = ep;
> +
> + if (*p == '/') {
> + unsigned int s;
> +
> + p++;
> +
> + s = strtoul(p, &ep, 10);
> +
> + if (ep == p) {
> + media_dbg(media, "Unable to parse stream: '%s'\n", orig_p);
> + if (endp)
> + *endp = (char*)p;
> + return NULL;
> + }
> +
> + *stream = s;
> +
> + p++;
Skip spaces at the end.
> + } else {
> + *stream = 0;
> + }
> +
> + if (endp)
> + *endp = (char*)p;
> +
> + return pad;
> +}
> +
> struct media_link *media_parse_link(struct media_device *media,
> const char *p, char **endp)
> {
> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index 63bb3d75..9205cfa4 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -64,7 +64,7 @@ void v4l2_subdev_close(struct media_entity *entity)
> }
>
> int v4l2_subdev_get_format(struct media_entity *entity,
> - struct v4l2_mbus_framefmt *format, unsigned int pad,
> + struct v4l2_mbus_framefmt *format, unsigned int pad, unsigned int stream,
> enum v4l2_subdev_format_whence which)
> {
> struct v4l2_subdev_format fmt;
> @@ -76,6 +76,7 @@ int v4l2_subdev_get_format(struct media_entity *entity,
>
> memset(&fmt, 0, sizeof(fmt));
> fmt.pad = pad;
> + fmt.stream = stream;
> fmt.which = which;
>
> ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
> @@ -88,6 +89,7 @@ int v4l2_subdev_get_format(struct media_entity *entity,
>
> int v4l2_subdev_set_format(struct media_entity *entity,
> struct v4l2_mbus_framefmt *format, unsigned int pad,
> + unsigned int stream,
> enum v4l2_subdev_format_whence which)
> {
> struct v4l2_subdev_format fmt;
> @@ -99,6 +101,7 @@ int v4l2_subdev_set_format(struct media_entity *entity,
>
> memset(&fmt, 0, sizeof(fmt));
> fmt.pad = pad;
> + fmt.stream = stream;
> fmt.which = which;
> fmt.format = *format;
>
> @@ -111,8 +114,8 @@ int v4l2_subdev_set_format(struct media_entity *entity,
> }
>
> int v4l2_subdev_get_selection(struct media_entity *entity,
> - struct v4l2_rect *rect, unsigned int pad, unsigned int target,
> - enum v4l2_subdev_format_whence which)
> + struct v4l2_rect *rect, unsigned int pad, unsigned int stream,
> + unsigned int target, enum v4l2_subdev_format_whence which)
> {
> union {
> struct v4l2_subdev_selection sel;
> @@ -150,8 +153,8 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
> }
>
> int v4l2_subdev_set_selection(struct media_entity *entity,
> - struct v4l2_rect *rect, unsigned int pad, unsigned int target,
> - enum v4l2_subdev_format_whence which)
> + struct v4l2_rect *rect, unsigned int pad, unsigned int stream,
> + unsigned int target, enum v4l2_subdev_format_whence which)
> {
> union {
> struct v4l2_subdev_selection sel;
> @@ -165,6 +168,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
>
> memset(&u.sel, 0, sizeof(u.sel));
> u.sel.pad = pad;
> + u.sel.stream = stream;
> u.sel.target = target;
> u.sel.which = which;
> u.sel.r = *rect;
> @@ -179,6 +183,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
>
> memset(&u.crop, 0, sizeof(u.crop));
> u.crop.pad = pad;
> + u.crop.stream = stream;
> u.crop.which = which;
> u.crop.rect = *rect;
>
> @@ -190,6 +195,69 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
> return 0;
> }
>
> +int v4l2_subdev_set_routing(struct media_entity *entity,
> + struct v4l2_subdev_route *routes,
> + unsigned int num_routes)
> +{
> + struct v4l2_subdev_routing routing = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .routes = (uintptr_t)routes,
> + .num_routes = num_routes,
> + };
> + int ret;
> +
> + ret = v4l2_subdev_open(entity);
> + if (ret < 0)
> + return ret;
> +
> + ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_ROUTING, &routing);
> + if (ret == -1)
> + return -errno;
> +
> + return 0;
> +}
> +
> +int v4l2_subdev_get_routing(struct media_entity *entity,
> + struct v4l2_subdev_route **routes,
> + unsigned int *num_routes)
Move get before set.
> +{
> + struct v4l2_subdev_routing routing = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> + struct v4l2_subdev_route *r;
> + int ret;
> +
> + ret = v4l2_subdev_open(entity);
> + if (ret < 0)
> + return ret;
> +
> + ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_ROUTING, &routing);
> + if (ret == -1 && errno != ENOSPC)
> + return -errno;
> +
> + if (!routing.num_routes) {
> + *routes = NULL;
> + *num_routes = 0;
> + return 0;
> + }
> +
> + r = calloc(routing.num_routes, sizeof(*r));
> + if (!r)
> + return -ENOMEM;
> +
> + routing.routes = (uintptr_t)r;
> + ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_ROUTING, &routing);
> + if (ret) {
ret = -errno;
> + free(r);
> + return ret;
> + }
> +
> + *num_routes = routing.num_routes;
> + *routes = r;
> +
> + return 0;
> +}
> +
> int v4l2_subdev_get_dv_timings_caps(struct media_entity *entity,
> struct v4l2_dv_timings_cap *caps)
> {
> @@ -264,7 +332,7 @@ int v4l2_subdev_set_dv_timings(struct media_entity *entity,
>
> int v4l2_subdev_get_frame_interval(struct media_entity *entity,
> struct v4l2_fract *interval,
> - unsigned int pad)
> + unsigned int pad, unsigned int stream)
> {
> struct v4l2_subdev_frame_interval ival;
> int ret;
> @@ -275,6 +343,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity *entity,
>
> memset(&ival, 0, sizeof(ival));
> ival.pad = pad;
> + ival.stream = stream;
>
> ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
> if (ret < 0)
> @@ -286,7 +355,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity *entity,
>
> int v4l2_subdev_set_frame_interval(struct media_entity *entity,
> struct v4l2_fract *interval,
> - unsigned int pad)
> + unsigned int pad, unsigned int stream)
> {
> struct v4l2_subdev_frame_interval ival;
> int ret;
> @@ -297,6 +366,7 @@ int v4l2_subdev_set_frame_interval(struct media_entity *entity,
>
> memset(&ival, 0, sizeof(ival));
> ival.pad = pad;
> + ival.stream = stream;
> ival.interval = *interval;
>
> ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
> @@ -307,6 +377,153 @@ int v4l2_subdev_set_frame_interval(struct media_entity *entity,
> return 0;
> }
>
> +static int v4l2_subdev_parse_setup_route(struct media_device *media,
> + struct v4l2_subdev_route *r,
> + const char *p, char **endp)
At some point we should probably use a real parser...
> +{
> + char *end;
> +
> + /* sink pad/stream */
> +
> + r->sink_pad = strtoul(p, &end, 10);
> + if (*end != '/') {
> + media_dbg(media, "Expected '/'\n");
> + return -EINVAL;
> + }
> +
> + p = end + 1;
> +
> + r->sink_stream = strtoul(p, &end, 10);
> +
> + for (; isspace(*end); ++end);
> +
> + if (end[0] != '-' || end[1] != '>') {
> + media_dbg(media, "Expected '->'\n");
> + return -EINVAL;
> + }
> + p = end + 2;
> +
> + /* source pad/stream */
> +
> + r->source_pad = strtoul(p, &end, 10);
> + if (*end != '/') {
> + media_dbg(media, "Expected '/'\n");
> + return -EINVAL;
> + }
> +
> + p = end + 1;
> +
> + r->source_stream = strtoul(p, &end, 10);
> +
> + /* flags */
> +
> + for (; isspace(*end); ++end);
> +
> + if (*end != '[') {
> + media_dbg(media, "Expected '['\n");
> + return -EINVAL;
> + }
> +
> + for (end++; isspace(*end); ++end);
> +
> + p = end;
> +
> + r->flags = strtoul(p, &end, 0);
> + if (r->flags & ~(V4L2_SUBDEV_ROUTE_FL_ACTIVE)) {
> + media_dbg(media, "Bad route flags %#x\n", r->flags);
> + return -EINVAL;
> + }
> +
> + for (; isspace(*end); ++end);
> +
> + if (*end != ']') {
> + media_dbg(media, "Expected ']'\n");
> + return -EINVAL;
> + }
> + end++;
Skip spaces at the end.
> +
> + *endp = end;
> +
> + return 0;
> +}
> +
> +int v4l2_subdev_parse_setup_routes(struct media_device *media, const char *p)
> +{
> + struct media_entity *entity;
> + struct v4l2_subdev_route *routes;
> + unsigned int num_routes;
> + char *end;
> + int ret;
> + int i;
unsigned int
> +
> + entity = media_parse_entity(media, p, &end);
> + if (!entity)
> + return -EINVAL;
> +
> + p = end;
> +
> + if (*p != '[') {
> + media_dbg(media, "Expected '['\n");
> + return -EINVAL;
> + }
> +
> + p++;
> +
> + routes = calloc(256, sizeof(routes[0]));
> + if (!routes)
> + return -ENOMEM;
> +
> + num_routes = 0;
> +
> + while (*p != 0) {
> + struct v4l2_subdev_route *r = &routes[num_routes];
> +
> + ret = v4l2_subdev_parse_setup_route(media, r, p, &end);
> + if (ret)
> + goto out;
> +
> + p = end;
> +
> + num_routes++;
> +
> + if (*p == ',') {
> + p++;
> + continue;
> + }
> +
> + break;
> + }
> +
> + if (*p != ']') {
> + media_dbg(media, "Expected ']'\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + for (i = 0; i < num_routes; ++i) {
> + struct v4l2_subdev_route *r = &routes[i];
> +
> + media_dbg(entity->media,
> + "Setting up route %s : %u/%u -> %u/%u, flags 0x%8.8x\n",
Flags are usually printed in "[0x%08x]" format.
> + entity->info.name,
> + r->sink_pad, r->sink_stream,
> + r->source_pad, r->source_stream,
> + r->flags);
> + }
> +
> + ret = v4l2_subdev_set_routing(entity, routes, num_routes);
> + if (ret) {
> + media_dbg(entity->media, "VIDIOC_SUBDEV_S_ROUTING failed: %d\n",
> + ret);
> + goto out;
> + }
> +
> +out:
> + free(routes);
> +
> + return ret;
> +}
> +
> static int v4l2_subdev_parse_format(struct media_device *media,
> struct v4l2_mbus_framefmt *format,
> const char *p, char **endp)
> @@ -442,7 +659,8 @@ static bool strhazit(const char *str, const char **p)
> }
>
> static struct media_pad *v4l2_subdev_parse_pad_format(
> - struct media_device *media, struct v4l2_mbus_framefmt *format,
> + struct media_device *media, unsigned int *stream,
> + struct v4l2_mbus_framefmt *format,
> struct v4l2_rect *crop, struct v4l2_rect *compose,
> struct v4l2_fract *interval, const char *p, char **endp)
> {
> @@ -453,7 +671,7 @@ static struct media_pad *v4l2_subdev_parse_pad_format(
>
> for (; isspace(*p); ++p);
>
> - pad = media_parse_pad(media, p, &end);
> + pad = media_parse_pad_stream(media, p, stream, &end);
> if (pad == NULL) {
> *endp = end;
> return NULL;
> @@ -675,6 +893,7 @@ static struct media_pad *v4l2_subdev_parse_pad_format(
> }
>
> static int set_format(struct media_pad *pad,
> + unsigned int stream,
> struct v4l2_mbus_framefmt *format)
> {
> int ret;
> @@ -683,12 +902,12 @@ static int set_format(struct media_pad *pad,
> return 0;
>
> media_dbg(pad->entity->media,
> - "Setting up format %s %ux%u on pad %s/%u\n",
> + "Setting up format %s %ux%u on pad %s/%u/%u\n",
> v4l2_subdev_pixelcode_to_string(format->code),
> format->width, format->height,
> - pad->entity->info.name, pad->index);
> + pad->entity->info.name, pad->index, stream);
>
> - ret = v4l2_subdev_set_format(pad->entity, format, pad->index,
> + ret = v4l2_subdev_set_format(pad->entity, format, pad->index, stream,
> V4L2_SUBDEV_FORMAT_ACTIVE);
> if (ret < 0) {
> media_dbg(pad->entity->media,
> @@ -705,8 +924,8 @@ static int set_format(struct media_pad *pad,
> return 0;
> }
>
> -static int set_selection(struct media_pad *pad, unsigned int target,
> - struct v4l2_rect *rect)
> +static int set_selection(struct media_pad *pad, unsigned int stream,
> + unsigned int target, struct v4l2_rect *rect)
> {
> int ret;
>
> @@ -714,11 +933,11 @@ static int set_selection(struct media_pad *pad, unsigned int target,
> return 0;
>
> media_dbg(pad->entity->media,
> - "Setting up selection target %u rectangle (%u,%u)/%ux%u on pad %s/%u\n",
> + "Setting up selection target %u rectangle (%u,%u)/%ux%u on pad %s/%u/%u\n",
> target, rect->left, rect->top, rect->width, rect->height,
> - pad->entity->info.name, pad->index);
> + pad->entity->info.name, pad->index, stream);
>
> - ret = v4l2_subdev_set_selection(pad->entity, rect, pad->index,
> + ret = v4l2_subdev_set_selection(pad->entity, rect, pad->index, stream,
> target, V4L2_SUBDEV_FORMAT_ACTIVE);
> if (ret < 0) {
> media_dbg(pad->entity->media,
> @@ -734,7 +953,7 @@ static int set_selection(struct media_pad *pad, unsigned int target,
> return 0;
> }
>
> -static int set_frame_interval(struct media_pad *pad,
> +static int set_frame_interval(struct media_pad *pad, unsigned int stream,
> struct v4l2_fract *interval)
> {
> int ret;
> @@ -743,11 +962,12 @@ static int set_frame_interval(struct media_pad *pad,
> return 0;
>
> media_dbg(pad->entity->media,
> - "Setting up frame interval %u/%u on pad %s/%u\n",
> + "Setting up frame interval %u/%u on pad %s/%u/%u\n",
> interval->numerator, interval->denominator,
> - pad->entity->info.name, pad->index);
> + pad->entity->info.name, pad->index, stream);
>
> - ret = v4l2_subdev_set_frame_interval(pad->entity, interval, pad->index);
> + ret = v4l2_subdev_set_frame_interval(pad->entity, interval, pad->index,
> + stream);
> if (ret < 0) {
> media_dbg(pad->entity->media,
> "Unable to set frame interval: %s (%d)",
> @@ -770,11 +990,13 @@ static int v4l2_subdev_parse_setup_format(struct media_device *media,
> struct v4l2_rect crop = { -1, -1, -1, -1 };
> struct v4l2_rect compose = crop;
> struct v4l2_fract interval = { 0, 0 };
> + unsigned int stream;
> unsigned int i;
> char *end;
> int ret;
>
> - pad = v4l2_subdev_parse_pad_format(media, &format, &crop, &compose,
> + pad = v4l2_subdev_parse_pad_format(media, &stream,
> + &format, &crop, &compose,
> &interval, p, &end);
> if (pad == NULL) {
> media_print_streampos(media, p, end);
> @@ -783,30 +1005,29 @@ static int v4l2_subdev_parse_setup_format(struct media_device *media,
> }
>
> if (pad->flags & MEDIA_PAD_FL_SINK) {
> - ret = set_format(pad, &format);
> + ret = set_format(pad, stream, &format);
> if (ret < 0)
> return ret;
> }
>
> - ret = set_selection(pad, V4L2_SEL_TGT_CROP, &crop);
> + ret = set_selection(pad, stream, V4L2_SEL_TGT_CROP, &crop);
> if (ret < 0)
> return ret;
>
> - ret = set_selection(pad, V4L2_SEL_TGT_COMPOSE, &compose);
> + ret = set_selection(pad, stream, V4L2_SEL_TGT_COMPOSE, &compose);
> if (ret < 0)
> return ret;
>
> if (pad->flags & MEDIA_PAD_FL_SOURCE) {
> - ret = set_format(pad, &format);
> + ret = set_format(pad, stream, &format);
> if (ret < 0)
> return ret;
> }
>
> - ret = set_frame_interval(pad, &interval);
> + ret = set_frame_interval(pad, stream, &interval);
> if (ret < 0)
> return ret;
>
> -
> /* If the pad is an output pad, automatically set the same format and
> * frame interval on the remote subdev input pads, if any.
> */
> @@ -821,9 +1042,9 @@ static int v4l2_subdev_parse_setup_format(struct media_device *media,
> if (link->source == pad &&
> link->sink->entity->info.type == MEDIA_ENT_T_V4L2_SUBDEV) {
> remote_format = format;
> - set_format(link->sink, &remote_format);
> + set_format(link->sink, stream, &remote_format);
>
> - ret = set_frame_interval(link->sink, &interval);
> + ret = set_frame_interval(link->sink, stream, &interval);
> if (ret < 0 && ret != -EINVAL && ret != -ENOTTY)
> return ret;
> }
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index 84ee7a83..831136a0 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -28,6 +28,7 @@
> #include <errno.h>
> #include <fcntl.h>
> #include <stdbool.h>
> +#include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -75,23 +76,43 @@ static void print_flags(const struct flag_name *flag_names, unsigned int num_ent
> }
> }
>
> +static void v4l2_subdev_print_routes(struct media_entity *entity,
> + struct v4l2_subdev_route *routes,
> + unsigned int num_routes)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < num_routes; i++) {
> + const struct v4l2_subdev_route *r = &routes[i];
Naming the variable 'route' would be more explicit.
> +
> + if (i == 0)
> + printf("\troutes:\n");
You could move this before the loop with
if (num_routes)
printf("\troutes:\n");
> +
> + printf("\t\t%u/%u -> %u/%u [%s]\n",
> + r->sink_pad, r->sink_stream,
> + r->source_pad, r->source_stream,
> + r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE ? "ACTIVE" : "INACTIVE");
> + }
> +}
> +
> static void v4l2_subdev_print_format(struct media_entity *entity,
> - unsigned int pad, enum v4l2_subdev_format_whence which)
> + unsigned int pad, unsigned int stream,
> + enum v4l2_subdev_format_whence which)
> {
> struct v4l2_mbus_framefmt format;
> struct v4l2_fract interval = { 0, 0 };
> struct v4l2_rect rect;
> int ret;
>
> - ret = v4l2_subdev_get_format(entity, &format, pad, which);
> + ret = v4l2_subdev_get_format(entity, &format, pad, stream, which);
> if (ret != 0)
> return;
>
> - ret = v4l2_subdev_get_frame_interval(entity, &interval, pad);
> + ret = v4l2_subdev_get_frame_interval(entity, &interval, pad, stream);
> if (ret != 0 && ret != -ENOTTY && ret != -EINVAL)
> return;
>
> - printf("\t\t[fmt:%s/%ux%u",
> + printf("\t\t[stream:%u fmt:%s/%ux%u", stream,
> v4l2_subdev_pixelcode_to_string(format.code),
> format.width, format.height);
>
> @@ -118,28 +139,28 @@ static void v4l2_subdev_print_format(struct media_entity *entity,
> v4l2_subdev_quantization_to_string(format.quantization));
> }
>
> - ret = v4l2_subdev_get_selection(entity, &rect, pad,
> + ret = v4l2_subdev_get_selection(entity, &rect, pad, stream,
> V4L2_SEL_TGT_CROP_BOUNDS,
> which);
> if (ret == 0)
> printf("\n\t\t crop.bounds:(%u,%u)/%ux%u", rect.left, rect.top,
> rect.width, rect.height);
>
> - ret = v4l2_subdev_get_selection(entity, &rect, pad,
> + ret = v4l2_subdev_get_selection(entity, &rect, pad, stream,
> V4L2_SEL_TGT_CROP,
> which);
> if (ret == 0)
> printf("\n\t\t crop:(%u,%u)/%ux%u", rect.left, rect.top,
> rect.width, rect.height);
>
> - ret = v4l2_subdev_get_selection(entity, &rect, pad,
> + ret = v4l2_subdev_get_selection(entity, &rect, pad, stream,
> V4L2_SEL_TGT_COMPOSE_BOUNDS,
> which);
> if (ret == 0)
> printf("\n\t\t compose.bounds:(%u,%u)/%ux%u",
> rect.left, rect.top, rect.width, rect.height);
>
> - ret = v4l2_subdev_get_selection(entity, &rect, pad,
> + ret = v4l2_subdev_get_selection(entity, &rect, pad, stream,
> V4L2_SEL_TGT_COMPOSE,
> which);
> if (ret == 0)
> @@ -455,16 +476,58 @@ static void media_print_topology_dot(struct media_device *media)
> }
>
> static void media_print_pad_text(struct media_entity *entity,
> - const struct media_pad *pad)
> + const struct media_pad *pad,
> + struct v4l2_subdev_route *routes,
> + unsigned int num_routes)
> {
> + unsigned int i;
> + uint64_t printed_streams_mask;
I'd swap those two variables. printed_streams_mask could be initialized
to 0 here.
> +
> if (media_entity_type(entity) != MEDIA_ENT_T_V4L2_SUBDEV)
> return;
>
> - v4l2_subdev_print_format(entity, pad->index, V4L2_SUBDEV_FORMAT_ACTIVE);
> - v4l2_subdev_print_pad_dv(entity, pad->index, V4L2_SUBDEV_FORMAT_ACTIVE);
> + if (!routes) {
> + v4l2_subdev_print_format(entity, pad->index, 0, V4L2_SUBDEV_FORMAT_ACTIVE);
> + v4l2_subdev_print_pad_dv(entity, pad->index, V4L2_SUBDEV_FORMAT_ACTIVE);
> +
> + if (pad->flags & MEDIA_PAD_FL_SOURCE)
> + v4l2_subdev_print_subdev_dv(entity);
> +
> + return;
> + }
> +
> + printed_streams_mask = 0;
> +
> + for (i = 0; i < num_routes; ++i) {
> + const struct v4l2_subdev_route *r = &routes[i];
Naming the variable 'route' would be more explicit.
> + unsigned int stream;
>
> - if (pad->flags & MEDIA_PAD_FL_SOURCE)
> - v4l2_subdev_print_subdev_dv(entity);
> + if (!(r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
> + continue;
> +
> + if (pad->flags & MEDIA_PAD_FL_SINK) {
> + if (r->sink_pad != pad->index)
> + continue;
> +
> + stream = r->sink_stream;
> + } else {
> + if (r->source_pad != pad->index)
> + continue;
> +
> + stream = r->source_stream;
> + }
> +
> + if (printed_streams_mask & (1 << stream))
> + continue;
> +
> + v4l2_subdev_print_format(entity, pad->index, stream, V4L2_SUBDEV_FORMAT_ACTIVE);
> + v4l2_subdev_print_pad_dv(entity, pad->index, V4L2_SUBDEV_FORMAT_ACTIVE);
> +
> + if (pad->flags & MEDIA_PAD_FL_SOURCE)
> + v4l2_subdev_print_subdev_dv(entity);
v4l2_subdev_print_pad_dv() and v4l2_subdev_print_subdev_dv() don't
depend on routes or streams, should they be printed outside of the loop
?
> +
> + printed_streams_mask |= (1 << stream);
> + }
> }
>
> static void media_print_topology_text_entity(struct media_device *media,
> @@ -480,11 +543,17 @@ static void media_print_topology_text_entity(struct media_device *media,
> unsigned int num_links = media_entity_get_links_count(entity);
> unsigned int j, k;
> unsigned int padding;
> + struct v4l2_subdev_route *routes = NULL;
> + unsigned int num_routes = 0;
Move these two before 'j, k'.
> +
> + if (media_entity_type(entity) == MEDIA_ENT_T_V4L2_SUBDEV)
> + v4l2_subdev_get_routing(entity, &routes, &num_routes);
>
> padding = printf("- entity %u: ", info->id);
> - printf("%s (%u pad%s, %u link%s)\n", info->name,
> + printf("%s (%u pad%s, %u link%s, %u route%s)\n", info->name,
Should we skip printing the number of routes when the entity isn't a
subdev ?
> info->pads, info->pads > 1 ? "s" : "",
> - num_links, num_links > 1 ? "s" : "");
> + num_links, num_links > 1 ? "s" : "",
> + num_routes, num_routes > 1 ? "s" : "");
> printf("%*ctype %s subtype %s flags %x\n", padding, ' ',
> media_entity_type_to_string(info->type),
> media_entity_subtype_to_string(info->type),
> @@ -492,12 +561,15 @@ static void media_print_topology_text_entity(struct media_device *media,
> if (devname)
> printf("%*cdevice node name %s\n", padding, ' ', devname);
>
> + if (media_entity_type(entity) == MEDIA_ENT_T_V4L2_SUBDEV)
> + v4l2_subdev_print_routes(entity, routes, num_routes);
> +
> for (j = 0; j < info->pads; j++) {
> const struct media_pad *pad = media_entity_get_pad(entity, j);
>
> printf("\tpad%u: %s\n", j, media_pad_type_to_string(pad->flags));
>
> - media_print_pad_text(entity, pad);
> + media_print_pad_text(entity, pad, routes, num_routes);
>
> for (k = 0; k < num_links; k++) {
> const struct media_link *link = media_entity_get_link(entity, k);
> @@ -521,6 +593,8 @@ static void media_print_topology_text_entity(struct media_device *media,
> }
> }
> printf("\n");
> +
> + free(routes);
> }
>
> static void media_print_topology_text(struct media_device *media)
> @@ -594,14 +668,16 @@ int main(int argc, char **argv)
>
> if (media_opts.fmt_pad) {
> struct media_pad *pad;
> + unsigned int stream;
> + char *p;
>
> - pad = media_parse_pad(media, media_opts.fmt_pad, NULL);
> + pad = media_parse_pad_stream(media, media_opts.fmt_pad, &stream, &p);
> if (pad == NULL) {
> printf("Pad '%s' not found\n", media_opts.fmt_pad);
> goto out;
> }
>
> - v4l2_subdev_print_format(pad->entity, pad->index,
> + v4l2_subdev_print_format(pad->entity, pad->index, stream,
> V4L2_SUBDEV_FORMAT_ACTIVE);
> }
>
> @@ -685,6 +761,15 @@ int main(int argc, char **argv)
> }
> }
>
> + if (media_opts.routes) {
> + ret = v4l2_subdev_parse_setup_routes(media, media_opts.routes);
> + if (ret) {
> + printf("Unable to setup routes: %s (%d)\n",
> + strerror(-ret), -ret);
> + goto out;
> + }
> + }
> +
> if (media_opts.interactive) {
> while (1) {
> char buffer[32];
> diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
> index af360518..c0fc2962 100644
> --- a/utils/media-ctl/mediactl.h
> +++ b/utils/media-ctl/mediactl.h
> @@ -394,6 +394,22 @@ struct media_entity *media_parse_entity(struct media_device *media,
> struct media_pad *media_parse_pad(struct media_device *media,
> const char *p, char **endp);
>
> +/**
> + * @brief Parse string to a pad and stream on the media device.
> + * @param media - media device.
> + * @param p - input string
> + * @param stream - pointer to uint where the stream number is stored
> + * @param endp - pointer to string where parsing ended
> + *
> + * Parse NULL terminated string describing a pad and stream and return its struct
> + * media_pad instance and the stream number.
> + *
> + * @return Pointer to struct media_pad on success, NULL on failure.
> + */
> +struct media_pad *media_parse_pad_stream(struct media_device *media,
> + const char *p, unsigned int *stream,
> + char **endp);
> +
> /**
> * @brief Parse string to a link on the media device.
> * @param media - media device.
> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
> index 6d30d3dc..3c408a1b 100644
> --- a/utils/media-ctl/options.c
> +++ b/utils/media-ctl/options.c
> @@ -63,6 +63,7 @@ static void usage(const char *argv0)
> printf(" --get-v4l2 pad Print the active format on a given pad\n");
> printf(" --get-dv pad Print detected and current DV timings on a given pad\n");
> printf(" --set-dv pad Configure DV timings on a given pad\n");
> + printf("-R, --set-routes routes Configure routes on a given subdev entity\n");
> printf("-h, --help Show verbose help and exit\n");
> printf("-i, --interactive Modify links interactively\n");
> printf("-l, --links links Comma-separated list of link descriptors to setup\n");
> @@ -78,7 +79,7 @@ static void usage(const char *argv0)
> printf("Links and formats are defined as\n");
> printf("\tlinks = link { ',' link } ;\n");
> printf("\tlink = pad '->' pad '[' flags ']' ;\n");
> - printf("\tpad = entity ':' pad-number ;\n");
> + printf("\tpad = entity ':' pad-number { '/' stream-number } ;\n");
> printf("\tentity = entity-number | ( '\"' entity-name '\"' ) ;\n");
> printf("\n");
> printf("\tv4l2 = pad '[' v4l2-properties ']' ;\n");
> @@ -95,11 +96,16 @@ static void usage(const char *argv0)
> printf("\trectangle = '(' left ',' top, ')' '/' size ;\n");
> printf("\tsize = width 'x' height ;\n");
> printf("\n");
> + printf("\troutes = entity '[' route { ',' route } ']' ;\n");
> + printf("\troute = pad-number '/' stream-number '->' pad-number '/' stream-number '[' route-flags ']' ;\n");
> + printf("\n");
> printf("where the fields are\n");
> printf("\tentity-number Entity numeric identifier\n");
> printf("\tentity-name Entity name (string) \n");
> printf("\tpad-number Pad numeric identifier\n");
> + printf("\tstream-number Stream numeric identifier\n");
> printf("\tflags Link flags (0: inactive, 1: active)\n");
> + printf("\troute-flags Route flags (bitmask of route flags: active - 0x1)\n");
> printf("\tfcc Format FourCC\n");
> printf("\twidth Image width in pixels\n");
> printf("\theight Image height in pixels\n");
> @@ -152,6 +158,7 @@ static struct option opts[] = {
> {"get-v4l2", 1, 0, OPT_GET_FORMAT},
> {"get-dv", 1, 0, OPT_GET_DV},
> {"set-dv", 1, 0, OPT_SET_DV},
> + {"set-routes", 1, 0, 'R'},
> {"help", 0, 0, 'h'},
> {"interactive", 0, 0, 'i'},
> {"links", 1, 0, 'l'},
> @@ -237,7 +244,7 @@ int parse_cmdline(int argc, char **argv)
> }
>
> /* parse options */
> - while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:",
> + while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:R:",
> opts, NULL)) != -1) {
> switch (opt) {
> case 'd':
> @@ -283,6 +290,10 @@ int parse_cmdline(int argc, char **argv)
> media_opts.verbose = 1;
> break;
>
> + case 'R':
> + media_opts.routes = optarg;
> + break;
> +
> case OPT_PRINT_DOT:
> media_opts.print_dot = 1;
> break;
> diff --git a/utils/media-ctl/options.h b/utils/media-ctl/options.h
> index 7e0556fc..753d0934 100644
> --- a/utils/media-ctl/options.h
> +++ b/utils/media-ctl/options.h
> @@ -36,6 +36,7 @@ struct media_options
> const char *fmt_pad;
> const char *get_dv_pad;
> const char *dv_pad;
> + const char *routes;
> };
>
> extern struct media_options media_opts;
> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
> index a1813911..a8a6e7ad 100644
> --- a/utils/media-ctl/v4l2subdev.h
> +++ b/utils/media-ctl/v4l2subdev.h
> @@ -64,7 +64,7 @@ void v4l2_subdev_close(struct media_entity *entity);
> * @return 0 on success, or a negative error code on failure.
> */
> int v4l2_subdev_get_format(struct media_entity *entity,
> - struct v4l2_mbus_framefmt *format, unsigned int pad,
> + struct v4l2_mbus_framefmt *format, unsigned int pad, unsigned int stream,
Missing documentation for the new parameter. Same below.
> enum v4l2_subdev_format_whence which);
>
> /**
> @@ -86,6 +86,7 @@ int v4l2_subdev_get_format(struct media_entity *entity,
> */
> int v4l2_subdev_set_format(struct media_entity *entity,
> struct v4l2_mbus_framefmt *format, unsigned int pad,
> + unsigned int stream,
> enum v4l2_subdev_format_whence which);
>
> /**
> @@ -107,8 +108,8 @@ int v4l2_subdev_set_format(struct media_entity *entity,
> * @return 0 on success, or a negative error code on failure.
> */
> int v4l2_subdev_get_selection(struct media_entity *entity,
> - struct v4l2_rect *rect, unsigned int pad, unsigned int target,
> - enum v4l2_subdev_format_whence which);
> + struct v4l2_rect *rect, unsigned int pad, unsigned int stream,
> + unsigned int target, enum v4l2_subdev_format_whence which);
>
> /**
> * @brief Set a selection rectangle on a pad.
> @@ -129,8 +130,40 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
> * @return 0 on success, or a negative error code on failure.
> */
> int v4l2_subdev_set_selection(struct media_entity *entity,
> - struct v4l2_rect *rect, unsigned int pad, unsigned int target,
> - enum v4l2_subdev_format_whence which);
> + struct v4l2_rect *rect, unsigned int pad, unsigned int stream,
> + unsigned int target, enum v4l2_subdev_format_whence which);
> +
> +/**
> + * @brief Get the routing table of a subdev media entity.
> + * @param entity - subdev-device media entity.
> + * @param routes - routes of the subdev.
> + * @param num_routes - number of routes.
> + *
> + * Get the routes of @a entity and return them in an allocated array in @a routes
> + * and the number of routes in @a num_routes.
> + *
> + * The caller is responsible for freeing the routes array after use.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int v4l2_subdev_get_routing(struct media_entity *entity,
> + struct v4l2_subdev_route **routes,
> + unsigned int *num_routes);
> +
> +/**
> + * @brief Set the routing table of a subdev media entity.
> + * @param entity - subdev-device media entity.
> + * @param routes - routes of the subdev.
> + * @param num_routes - number of routes.
> + *
> + * Set the routes of @a entity. The routes are given in @a routes with the
> + * length of @a num_routes.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int v4l2_subdev_set_routing(struct media_entity *entity,
> + struct v4l2_subdev_route *route,
> + unsigned int num_routes);
>
> /**
> * @brief Query the digital video capabilities of a pad.
> @@ -200,7 +233,7 @@ int v4l2_subdev_set_dv_timings(struct media_entity *entity,
> */
>
> int v4l2_subdev_get_frame_interval(struct media_entity *entity,
> - struct v4l2_fract *interval, unsigned int pad);
> + struct v4l2_fract *interval, unsigned int pad, unsigned int stream);
>
> /**
> * @brief Set the frame interval on a sub-device.
> @@ -217,7 +250,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity *entity,
> * @return 0 on success, or a negative error code on failure.
> */
> int v4l2_subdev_set_frame_interval(struct media_entity *entity,
> - struct v4l2_fract *interval, unsigned int pad);
> + struct v4l2_fract *interval, unsigned int pad, unsigned int stream);
>
> /**
> * @brief Parse a string and apply format, crop and frame interval settings.
> @@ -235,6 +268,17 @@ int v4l2_subdev_set_frame_interval(struct media_entity *entity,
> */
> int v4l2_subdev_parse_setup_formats(struct media_device *media, const char *p);
>
> +/**
> + * @brief Parse a string and apply route settings.
> + * @param media - media device.
> + * @param p - input string
> + *
> + * Parse string @a p and apply route settings to a subdev.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int v4l2_subdev_parse_setup_routes(struct media_device *media, const char *p);
> +
> /**
> * @brief Convert media bus pixel code to string.
> * @param code - input string
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2023-04-24 7:29 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-21 12:44 [PATCH v4 0/8] v4l-utils: Support multiplexed streams Tomi Valkeinen
2023-04-21 12:44 ` [PATCH v4 1/8] v4l2-ctl: Add routing and streams support Tomi Valkeinen
2023-04-24 7:04 ` Laurent Pinchart
2023-05-29 7:02 ` Tomi Valkeinen
2023-05-29 7:47 ` Laurent Pinchart
2023-04-21 12:44 ` [PATCH v4 2/8] media-ctl: Add support for routes and streams Tomi Valkeinen
2023-04-24 7:29 ` Laurent Pinchart [this message]
2023-05-29 7:34 ` Tomi Valkeinen
2023-05-29 7:49 ` Laurent Pinchart
2023-05-29 8:14 ` Tomi Valkeinen
2023-04-21 12:44 ` [PATCH v4 3/8] v4l2-ctl/compliance: Add routing and streams multiplexed streams Tomi Valkeinen
2023-04-21 12:44 ` [PATCH v4 4/8] v4l2-ctl/compliance: Add simple routing test Tomi Valkeinen
2023-04-24 8:04 ` Laurent Pinchart
2023-04-21 12:44 ` [PATCH v4 5/8] HACK: include/linux: Add client capabilities Tomi Valkeinen
2023-04-24 7:32 ` Laurent Pinchart
2023-05-25 14:05 ` Hans Verkuil
2023-05-26 8:19 ` Tomi Valkeinen
2023-04-21 12:44 ` [PATCH v4 6/8] media-ctl: Check for Streams API support Tomi Valkeinen
2023-04-24 7:54 ` Laurent Pinchart
2023-04-21 12:44 ` [PATCH v4 7/8] utils/common: Set V4L2_SUBDEV_CLIENT_CAP_STREAMS for subdevs Tomi Valkeinen
2023-04-24 8:01 ` Laurent Pinchart
2023-04-21 12:44 ` [PATCH v4 8/8] v4l2-ctl: Check for Streams API support Tomi Valkeinen
2023-04-24 8:03 ` Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230424072932.GD4926@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=hverkuil-cisco@xs4all.nl \
--cc=jacopo.mondi@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=sakari.ailus@linux.intel.com \
--cc=satish.nagireddy@getcruise.com \
--cc=tomi.valkeinen@ideasonboard.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox