[libcamera-devel] [PATCH 07/13] libcamera: v4l2_subdevice: Collect subdev capabilities

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-08-01 01:05:37)
> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> Collect subdev capabilities at open() time.
> 
> Model the V4L2SubdevCapabilties as V4L2Capability from the video
> device class.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes on top of Jacopo's initial work:
> 
> - Rename V4L2SubdevCapabilities to V4L2SubdeviceCapability and make it
>   final, to match the V4L2VideoDevice class
> - Add V4L2Subdevice::caps()
> ---
>  include/libcamera/internal/v4l2_subdevice.h | 13 ++++++
>  src/libcamera/v4l2_subdevice.cpp            | 50 ++++++++++++++++++++-
>  2 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> index 2db392d5e37a..a1d3144c6a7f 100644
> --- a/include/libcamera/internal/v4l2_subdevice.h
> +++ b/include/libcamera/internal/v4l2_subdevice.h
> @@ -29,6 +29,17 @@ namespace libcamera {
>  
>  class MediaDevice;
>  
> +struct V4L2SubdeviceCapability final : v4l2_subdev_capability {
> +       bool isReadOnly() const
> +       {
> +               return capabilities & V4L2_SUBDEV_CAP_RO_SUBDEV;
> +       }
> +       bool hasStreams() const
> +       {
> +               return capabilities & V4L2_SUBDEV_CAP_MPLEXED;
> +       }
> +};
> +
>  struct V4L2SubdeviceFormat {
>         uint32_t mbus_code;
>         Size size;
> @@ -70,6 +81,7 @@ public:
>                       Whence whence = ActiveFormat);
>  
>         const std::string &model();
> +       const V4L2SubdeviceCapability &caps() const { return caps_; }
>  
>         static std::unique_ptr<V4L2Subdevice>
>         fromEntityName(const MediaDevice *media, const std::string &entity);
> @@ -87,6 +99,7 @@ private:
>         const MediaEntity *entity_;
>  
>         std::string model_;
> +       struct V4L2SubdeviceCapability caps_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 37960b76a044..a1672b2365f2 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -133,6 +133,30 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = {
>  
>  } /* namespace */
>  
> +/**
> + * \struct V4L2SubdeviceCapability
> + * \brief struct v4l2_subdev_capability object wrapper and helpers
> + *
> + * The V4L2SubdeviceCapability structure manages the information returned by the
> + * VIDIOC_SUBDEV_QUERYCAP ioctl.
> + */
> +
> +/**
> + * \fn V4L2SubdeviceCapability::isReadOnly()
> + * \brief Retrieve if a subdevice is registered as read-only
> + *
> + * A V4L2 subdevice is registered as read-only if V4L2_SUBDEV_CAP_RO_SUBDEV
> + * is listed as part of its capabilities.
> + *
> + * \return True if the subdevice is registered as read-only, false otherwise
> + */
> +
> +/**
> + * \fn V4L2SubdeviceCapability::hasStreams()
> + * \brief Retrieve if a subdevice supports the V4L2 streams API
> + * \return True if the subdevice supports the streams API, false otherwise
> + */
> +
>  /**
>   * \struct V4L2SubdeviceFormat
>   * \brief The V4L2 sub-device image format and sizes
> @@ -284,7 +308,25 @@ V4L2Subdevice::~V4L2Subdevice()
>   */
>  int V4L2Subdevice::open()
>  {
> -       return V4L2Device::open(O_RDWR);
> +       int ret = V4L2Device::open(O_RDWR);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Try to query the subdev capabilities. The VIDIOC_SUBDEV_QUERYCAP API
> +        * was introduced in kernel v5.8, ENOTTY errors must be ignored to
> +        * support older kernels.
> +        */
> +       caps_ = {};
> +       ret = ioctl(VIDIOC_SUBDEV_QUERYCAP, &caps_);
> +       if (ret < 0 && errno != ENOTTY) {
> +               ret = -errno;
> +               LOG(V4L2, Error)
> +                       << "Unable to query capabilities: " << strerror(-ret);
> +               return ret;
> +       }

I wonder if we should report ENOTTY errors through a Debug level print,
but perhaps we could do that generically in our ioctl call (in a
separate patch) to report that an attempt was made on an unsupported
call?

Otherwise, I like this.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> +
> +       return 0;
>  }
>  
>  /**
> @@ -527,6 +569,12 @@ const std::string &V4L2Subdevice::model()
>         return model_;
>  }
>  
> +/**
> + * \fn V4L2Subdevice::caps()
> + * \brief Retrieve the subdevice V4L2 capabilities
> + * \return The subdevice V4L2 capabilities
> + */
> +
>  /**
>   * \brief Create a new video subdevice instance from \a entity in media device
>   * \a media
> -- 
> Regards,
> 
> Laurent Pinchart
>