[libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create ImgUDevice class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 2 11:26:13 CEST 2019
Hi Jacopo,
Thank you for the patch.
On Tue, Mar 26, 2019 at 09:38:54AM +0100, Jacopo Mondi wrote:
> Group ImgU components in a class associated with a camera at camera
> registration time and provide an intialization method to create and open
> all video devices and subdevices of the ImgU.
>
> Statically assign imgu0 to the first camera and imgu1 to the second one
> and limit support to two cameras. This will have to be revised in the future.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 154 +++++++++++++++++++++++++--
> 1 file changed, 145 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 21205b39afee..0a059b75cadf 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -44,6 +44,43 @@ static int mediaBusToCIO2Format(unsigned int code)
> }
> }
>
> +class ImgUDevice
> +{
> +public:
> + static constexpr unsigned int PAD_INPUT = 0;
> + static constexpr unsigned int PAD_OUTPUT = 2;
> + static constexpr unsigned int PAD_VF = 3;
> + static constexpr unsigned int PAD_STAT = 4;
> +
> + ImgUDevice()
> + : imgu_(nullptr), input_(nullptr), output_(nullptr),
> + viewfinder_(nullptr), stat_(nullptr)
> + {
> + }
> +
> + ~ImgUDevice()
> + {
> + delete imgu_;
> + delete input_;
> + delete output_;
> + delete viewfinder_;
> + delete stat_;
> + }
> +
> + int init(MediaDevice *media, unsigned int index);
> +
> + unsigned int index_;
> + std::string name_;
> + MediaDevice *media_;
> +
> + V4L2Subdevice *imgu_;
> + V4L2Device *input_;
> + V4L2Device *output_;
> + V4L2Device *viewfinder_;
> + V4L2Device *stat_;
> + /* \todo Add param video device for 3A tuning */
> +};
> +
> class CIO2Device
> {
> public:
> @@ -106,6 +143,7 @@ private:
> void bufferReady(Buffer *buffer);
>
> CIO2Device cio2_;
> + ImgUDevice *imgu_;
>
> Stream stream_;
> };
> @@ -116,8 +154,10 @@ private:
> PipelineHandler::cameraData(camera));
> }
>
> - void registerCameras();
> + int registerCameras();
>
> + ImgUDevice imgu0_;
> + ImgUDevice imgu1_;
> std::shared_ptr<MediaDevice> cio2MediaDev_;
> std::shared_ptr<MediaDevice> imguMediaDev_;
> };
> @@ -303,6 +343,8 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
>
> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> {
> + int ret;
> +
> DeviceMatch cio2_dm("ipu3-cio2");
> cio2_dm.add("ipu3-csi2 0");
> cio2_dm.add("ipu3-cio2 0");
> @@ -358,36 +400,75 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> return false;
> }
>
> - registerCameras();
> + if (imguMediaDev_->open()) {
> + cio2MediaDev_->close();
> + return false;
> + }
> +
> + if (imguMediaDev_->disableLinks())
> + goto error;
> +
> + ret = registerCameras();
> + if (ret)
> + goto error;
>
> cio2MediaDev_->close();
> + imguMediaDev_->close();
>
> return true;
> +
> +error:
> + cio2MediaDev_->close();
> + imguMediaDev_->close();
> +
> + return false;
> }
>
> -/*
> - * Cameras are created associating an image sensor (represented by a
> - * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four
> - * CIO2 CSI-2 receivers.
> +/**
> + * \brief Initialize ImgU devices and CIO2 ones associated with cameras
s/ImgU devices and CIO2 ones/the ImgU and CIO2 devices/
I'm sure Kieran would also tell you to s/Initialize/Initialise/ :-)
> + *
> + * Initialize the two ImgU instances and create cameras with an associated
> + * CIO2 device instance.
> + *
> + * \return 0 on success or a negative error code for error or if no camera
> + * has been created
> + * \retval -ENODEV no camera has been created
> */
> -void PipelineHandlerIPU3::registerCameras()
> +int PipelineHandlerIPU3::registerCameras()
> {
> + int ret;
> +
> + ret = imgu0_.init(imguMediaDev_.get(), 0);
> + if (ret)
> + return ret;
> +
> + ret = imgu1_.init(imguMediaDev_.get(), 1);
> + if (ret)
> + return ret;
> +
> /*
> * For each CSI-2 receiver on the IPU3, create a Camera if an
> * image sensor is connected to it and it can produce images in
> * a compatible format.
> */
> unsigned int numCameras = 0;
> - for (unsigned int id = 0; id < 4; ++id) {
> + for (unsigned int id = 0; id < 4 && numCameras < 2; ++id) {
> std::unique_ptr<IPU3CameraData> data =
> utils::make_unique<IPU3CameraData>(this);
> std::set<Stream *> streams{ &data->stream_ };
> CIO2Device *cio2 = &data->cio2_;
>
> - int ret = cio2->init(cio2MediaDev_.get(), id);
> + ret = cio2->init(cio2MediaDev_.get(), id);
> if (ret)
> continue;
>
> + /**
> + * \todo Dynamically assign ImgU devices; as of now, limit
> + * support to two cameras only, and assign imgu0 to the first
> + * one and imgu1 to the second.
> + */
> + data->imgu_ = numCameras ? &imgu1_ : &imgu0_;
> +
> std::string cameraName = cio2->name() + " "
> + std::to_string(id);
> std::shared_ptr<Camera> camera = Camera::create(this,
> @@ -406,6 +487,8 @@ void PipelineHandlerIPU3::registerCameras()
>
> numCameras++;
> }
> +
> + return numCameras ? 0 : -ENODEV;
> }
>
> void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)
> @@ -416,6 +499,59 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)
> pipe_->completeRequest(camera_, request);
> }
>
> +/* -----------------------------------------------------------------------------
> + * ImgU Device
> + */
> +
> +/**
> + * \brief Initialize components of the ImgU instance
> + * \param[in] mediaDevice The ImgU instance media device
> + * \param[in] index The ImgU instance index
> + *
> + * Create and open the V4L2 devices and subdevices of the ImgU instance
> + * with \a index.
> + *
> + * In case of errors the created V4L2Device and V4L2Subdevice instances
> + * are destroyed at pipeline handler delete time.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int ImgUDevice::init(MediaDevice *media, unsigned int index)
It could make sense to pass the media device pointer and index to the
constructor, in order to avoid leaving the media_ and index_ members
unitialized. The initialization of course belongs here. This would imply
creating the ImgUDevice instances dynamically. What do you think ? This
comment also applies to the previous patch.
> +{
> + int ret;
> +
> + index_ = index;
> + name_ = "ipu3-imgu " + std::to_string(index_);
> + media_ = media;
> +
> + imgu_ = V4L2Subdevice::fromEntityName(media, name_);
Shouldn't test if this (and the other V4L2Device instances below) is
null before trying to open it on the next line ?
With these small issues fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + ret = imgu_->open();
> + if (ret)
> + return ret;
> +
> + input_ = V4L2Device::fromEntityName(media, name_ + " input");
> + ret = input_->open();
> + if (ret)
> + return ret;
> +
> + output_ = V4L2Device::fromEntityName(media, name_ + " output");
> + ret = output_->open();
> + if (ret)
> + return ret;
> +
> + viewfinder_ = V4L2Device::fromEntityName(media, name_ + " viewfinder");
> + ret = viewfinder_->open();
> + if (ret)
> + return ret;
> +
> + stat_ = V4L2Device::fromEntityName(media, name_ + " 3a stat");
> + ret = stat_->open();
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> /*------------------------------------------------------------------------------
> * CIO2 Device
> */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list