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

[Jacopo Mondi]

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 ?

Thanks
  j

> +			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
>