[PATCH/RFC 11/32] libcamera: v4l2_subdevice: Update to the new kernel routing API

[Laurent Pinchart]

Hi Jacopo,

On Tue, Mar 05, 2024 at 09:57:51AM +0100, Jacopo Mondi wrote:
> Hi Laurent,
>   one question
> 
> On Fri, Mar 01, 2024 at 11:21:00PM +0200, Laurent Pinchart wrote:
> > The subdev embedded data support series includes a change to the
> > VIDIOC_SUBDEV_G_ROUTING and VIDIOC_SUBDEV_S_ROUTING ioctls that impacts
> > the userspace API.
> >
> > Update to the new API. Backward compatibility can't be easily preserved
> > as the ioctl structure size has changed. This is not a major issue, as
> > the routing API isn't enabled in any upstream kernel.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/v4l2_subdevice.cpp | 43 ++++++++++++++++++++++++++------
> >  1 file changed, 35 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 3653f57a7d10..d5d400cdfd58 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -1332,19 +1332,23 @@ int V4L2Subdevice::getRouting(Routing *routing, Whence whence)
> >  	rt.which = whence;
> >
> >  	int ret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt);
> > -	if (ret == 0 || ret == -ENOTTY)
> > -		return ret;
> > -
> > -	if (ret != -ENOSPC) {
> > -		LOG(V4L2, Error)
> > -			<< "Failed to retrieve number of routes: "
> > -			<< strerror(-ret);
> > +	if (ret) {
> > +		if (ret != -ENOTTY)
> 
> If we get here, it means
> 
> 	caps_.hasStreams()
> 
> If a subdevice claims stream support, shouldn't the routing kAPI be
> enabled, or is it still legit to get a ENOTTY back ?

Good question. For setRouting() I think it may be considered legit, but
for getRouting(), likely not. I'll drop the check and log an error
unconditionally.

> > +			LOG(V4L2, Error)
> > +				<< "Failed to retrieve number of routes: "
> > +				<< strerror(-ret);
> >  		return ret;
> >  	}
> >
> > +	if (!rt.num_routes)
> > +		return 0;
> > +
> >  	std::vector<struct v4l2_subdev_route> routes{ rt.num_routes };
> >  	rt.routes = reinterpret_cast<uintptr_t>(routes.data());
> >
> > +	rt.len_routes = rt.num_routes;
> > +	rt.num_routes = 0;
> > +
> >  	ret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt);
> >  	if (ret) {
> >  		LOG(V4L2, Error)
> > @@ -1391,6 +1395,7 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)
> >
> >  	struct v4l2_subdev_routing rt = {};
> >  	rt.which = whence;
> > +	rt.len_routes = routes.size();
> >  	rt.num_routes = routes.size();
> >  	rt.routes = reinterpret_cast<uintptr_t>(routes.data());
> >
> > @@ -1400,7 +1405,29 @@ int V4L2Subdevice::setRouting(Routing *routing, Whence whence)
> >  		return ret;
> >  	}
> >
> > -	routes.resize(rt.num_routes);
> > +	/*
> > +	 * The kernel wants to return more routes than we have space for. We
> > +	 * need to issue a VIDIOC_SUBDEV_G_ROUTING call.
> > +	 */
> > +	if (rt.num_routes > routes.size()) {
> > +		routes.resize(rt.num_routes);
> > +
> > +		rt.len_routes = rt.num_routes;
> > +		rt.num_routes = 0;
> > +
> > +		ret = ioctl(VIDIOC_SUBDEV_G_ROUTING, &rt);
> > +		if (ret) {
> > +			LOG(V4L2, Error)
> > +				<< "Failed to retrieve routes: " << strerror(-ret);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	if (rt.num_routes != routes.size()) {
> > +		LOG(V4L2, Error) << "Invalid number of routes";
> > +		return -EINVAL;
> > +	}
> > +
> >  	routing->resize(rt.num_routes);
> >
> >  	for (const auto &[i, route] : utils::enumerate(routes))

-- 
Regards,

Laurent Pinchart