Re: [PATCH 1/2] v4l2-ctl: Add --try-routing option

[Dan Scally <dan.scally@xxxxxxxxxxxxxxxx>]

Hi Laurent

On 17/01/2024 13:21, Laurent Pinchart wrote:
> Hi Dan,
>
> Thank you for the patch.
>
> On Wed, Jan 17, 2024 at 01:08:04PM +0000, Daniel Scally wrote:
> > v4l2-ctl's --list-subdev-mbus-codes option sets the which flag to
> > V4L2_SUBDEV_FORMAT_TRY, which is an entirely reasonable choice, but
> > means it's currently impossible to list mbus codes for pads that are
> > only part of inactive routes as the --set-routing option sets ACTIVE
> > routing rather than TRY.
> >
> > Add a --try-routing option that has identical functionality to the
> > existing --set-routing but which uses the TRY format instead.
> I don't think this will help fixing your problem. They TRY context is
> local to the file handle, v4l2-ctl will never seen the TRY routes you
> set here.


It sees them provided you use both functions in the same run of the program. So for example this 
won't work because the file is closed in between the two:

v4l2-ctl -d /dev/v4l-subdev2 --try-routing "0/0->1/0[0],2/0->1/0[1]"

v4l2-ctl -d /dev/v4l-subdev2 --list-subdev-mbus-codes pad=2


But this works just fine, because it's kept open throughout:


v4l2-ctl -d /dev/v4l-subdev2 --try-routing "0/0->1/0[0],2/0->1/0[1]" --list-subdev-mbus-codes pad=2


> > Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> > ---
> >   utils/v4l2-ctl/v4l2-ctl-subdev.cpp | 11 +++++++----
> >   utils/v4l2-ctl/v4l2-ctl.cpp        |  1 +
> >   utils/v4l2-ctl/v4l2-ctl.h          |  1 +
> >   3 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> > index 86e6c689..48b79288 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> > +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp
> > @@ -96,7 +96,8 @@ void subdev_usage()
> >   	       "  --set-subdev-fps pad=<pad>,stream=<stream>,fps=<fps> (for testing only, otherwise use media-ctl)\n"
> >   	       "                     set the frame rate [VIDIOC_SUBDEV_S_FRAME_INTERVAL]\n"
> >   	       "  --get-routing      Print the route topology\n"
> > -	       "  --set-routing <routes>\n"
> > +	       "  --set-routing      (for testing only, otherwise use media-ctl)\n"
> > +	       "  --try-routing <routes>\n"
> >   	       "                     Comma-separated list of route descriptors to setup\n"
> >   	       "\n"
> >   	       "Routes are defined as\n"
> > @@ -458,14 +459,16 @@ void subdev_cmd(int ch, char *optarg)
> >   			}
> >   		}
> >   		break;
> > -	case OptSetRouting: {
> > +	case OptSetRouting:
> > +	case OptTryRouting: {
> >   		struct v4l2_subdev_route *r;
> >   		char *end, *ref, *tok;
> >   		unsigned int flags;
> >   
> >   		memset(&routing, 0, sizeof(routing));
> >   		memset(routes, 0, sizeof(routes[0]) * NUM_ROUTES_MAX);
> > -		routing.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > +		routing.which = ch == OptSetRouting ? V4L2_SUBDEV_FORMAT_ACTIVE :
> > +				      V4L2_SUBDEV_FORMAT_TRY;
> >   		routing.num_routes = 0;
> >   		routing.routes = (__u64)routes;
> >   
> > @@ -683,7 +686,7 @@ void subdev_set(cv4l_fd &_fd)
> >   					fival.interval.denominator, fival.interval.numerator);
> >   		}
> >   	}
> > -	if (options[OptSetRouting]) {
> > +	if (options[OptSetRouting] || options[OptTryRouting]) {
> >   		if (!_fd.has_streams()) {
> >   			printf("Streams API not supported.\n");
> >   			return;
> > diff --git a/utils/v4l2-ctl/v4l2-ctl.cpp b/utils/v4l2-ctl/v4l2-ctl.cpp
> > index e195ad8e..f9121284 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl.cpp
> > +++ b/utils/v4l2-ctl/v4l2-ctl.cpp
> > @@ -65,6 +65,7 @@ static struct option long_options[] = {
> >   	{"set-fmt-video-out", required_argument, nullptr, OptSetVideoOutFormat},
> >   	{"try-fmt-video-out", required_argument, nullptr, OptTryVideoOutFormat},
> >   	{"get-routing", no_argument, 0, OptGetRouting},
> > +	{"try-routing", required_argument, 0, OptTryRouting},
> >   	{"set-routing", required_argument, 0, OptSetRouting},
> >   	{"help", no_argument, nullptr, OptHelp},
> >   	{"help-tuner", no_argument, nullptr, OptHelpTuner},
> > diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
> > index cc7f1184..6382619c 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl.h
> > +++ b/utils/v4l2-ctl/v4l2-ctl.h
> > @@ -193,6 +193,7 @@ enum Option {
> >   	OptShowEdid,
> >   	OptFixEdidChecksums,
> >   	OptGetRouting,
> > +	OptTryRouting,
> >   	OptSetRouting,
> >   	OptFreqSeek,
> >   	OptEncoderCmd,