[libcamera-devel] [PATCH v2 5/5] libcamera: pipeline: Add Intel IPU3 pipeline
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Jan 16 16:47:29 CET 2019
Hi Jacopo,
Thanks for the updated version.
On 2019-01-16 14:59:49 +0100, Jacopo Mondi wrote:
> Add a pipeline handler for the Intel IPU3 device.
>
> The pipeline handler creates a Camera for each image sensor it finds to be
> connected to an IPU3 CSI-2 receiver, and enables the link between the two.
>
> Tested on Soraka, listing detected cameras on the system, verifying the
> pipeline handler gets matched and links properly enabled.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 263 ++++++++++++++++++++++++
> src/libcamera/pipeline/ipu3/meson.build | 3 +
> src/libcamera/pipeline/meson.build | 2 +
> 3 files changed, 268 insertions(+)
> create mode 100644 src/libcamera/pipeline/ipu3/ipu3.cpp
> create mode 100644 src/libcamera/pipeline/ipu3/meson.build
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> new file mode 100644
> index 0000000..066e9da
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -0,0 +1,263 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipu3.cpp - Pipeline handler for Intel IPU3
> + */
> +
> +#include <map>
> +#include <vector>
> +
> +#include <libcamera/camera.h>
> +
> +#include "device_enumerator.h"
> +#include "log.h"
> +#include "media_device.h"
> +#include "pipeline_handler.h"
> +
> +namespace libcamera {
> +
> +class PipelineHandlerIPU3 : public PipelineHandler
> +{
> +public:
> + PipelineHandlerIPU3();
> + ~PipelineHandlerIPU3();
> +
> + bool match(DeviceEnumerator *enumerator);
> +
> + unsigned int count();
> + Camera *camera(unsigned int id) final;
> +
> +private:
> + MediaDevice *cio2_;
> + MediaDevice *imgu_;
> +
> + unsigned int numCameras_;
Is this member needed? Can't you retrieve the number of items in
camerasMap_ container?
> + std::map<unsigned int, Camera *> camerasMap_;
Does this need to be a map? Can't it be a vector instead? Looking at the
usage you loop over the elements anyhow.
This is not a big deal as the camera registration is to be reworked and
the count() and camera() functions will go away and the camera will
register it self with the Camera manager.
> +
> + unsigned int registerCameras();
> +};
> +
> +PipelineHandlerIPU3::PipelineHandlerIPU3()
> + : cio2_(nullptr), imgu_(nullptr), numCameras_(0)
> +{
> +}
> +
> +PipelineHandlerIPU3::~PipelineHandlerIPU3()
> +{
> + if (cio2_)
> + cio2_->release();
> +
> + if (imgu_)
> + imgu_->release();
> +
> + for (auto map : camerasMap_) {
> + Camera *camera = map.second;
> + if (camera)
> + camera->put();
> + }
> +
> + cio2_ = nullptr;
> + imgu_ = nullptr;
> + camerasMap_.clear();
> +}
> +
> +unsigned int PipelineHandlerIPU3::count()
> +{
> + return numCameras_;
> +}
> +
> +Camera *PipelineHandlerIPU3::camera(unsigned int id)
> +{
> + if (id >= numCameras_)
> + return nullptr;
> +
> + /*
> + * The requested camera id does not match the key index used to store
> + * Camera instances in the 'camerasMap_' map.
> + *
> + * The 'id' argument represent the n-th valid cameras registered
> + * in the system, while the indexing key is the CSI-2 receiver index
> + * the camera sensor is associated to, and some receiver might have no
> + * camera sensor connected.
> + */
> + for (auto it = camerasMap_.begin(); it != camerasMap_.end(); ++it, --id) {
> + if (id == 0)
> + return (*it).second;
> + }
If you keep camerasMap_ as a map I think you should use any of the
std::map helpers to resolve the id (which is the key in the map) to the
camera (the value).
> +
> + return nullptr;
> +}
> +
> +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> +{
> + DeviceMatch cio2_dm("ipu3-cio2");
> + cio2_dm.add("ipu3-csi2 0");
> + cio2_dm.add("ipu3-cio2 0");
> + cio2_dm.add("ipu3-csi2 1");
> + cio2_dm.add("ipu3-cio2 1");
> + cio2_dm.add("ipu3-csi2 2");
> + cio2_dm.add("ipu3-cio2 2");
> + cio2_dm.add("ipu3-csi2 3");
> + cio2_dm.add("ipu3-cio2 3");
> +
> + DeviceMatch imgu_dm("ipu3-imgu");
> + imgu_dm.add("ipu3-imgu 0");
> + imgu_dm.add("ipu3-imgu 0 input");
> + imgu_dm.add("ipu3-imgu 0 parameters");
> + imgu_dm.add("ipu3-imgu 0 output");
> + imgu_dm.add("ipu3-imgu 0 viewfinder");
> + imgu_dm.add("ipu3-imgu 0 3a stat");
> + imgu_dm.add("ipu3-imgu 1");
> + imgu_dm.add("ipu3-imgu 1 input");
> + imgu_dm.add("ipu3-imgu 1 parameters");
> + imgu_dm.add("ipu3-imgu 1 output");
> + imgu_dm.add("ipu3-imgu 1 viewfinder");
> + imgu_dm.add("ipu3-imgu 1 3a stat");
> +
> + cio2_ = enumerator->search(cio2_dm);
> + if (!cio2_)
> + return false;
> +
> + imgu_ = enumerator->search(imgu_dm);
> + if (!imgu_)
> + return false;
> +
> + /*
> + * It is safe to acquire both media devices at this point as
> + * DeviceEnumerator::search() skips the busy ones for us.
> + */
> + cio2_->acquire();
> + imgu_->acquire();
> +
> + /*
> + * Disable all links that are enabled by default on CIO2, as camera
> + * creation enables all valid links it finds.
> + *
> + * Close the CIO2 media device after, as links are enabled and should
> + * not need to be changed after.
> + */
> + if (cio2_->open())
> + goto error_release_mdev;
> +
> + if (cio2_->disableLinks())
> + goto error_close_cio2;
> +
> + numCameras_ = registerCameras();
> + LOG(Debug) << "\"Intel IPU3\" pipeline handler initialized with "
> + << numCameras_ << " cameras registered";
> +
> + cio2_->close();
> +
> + return true;
> +
> +error_close_cio2:
> + cio2_->close();
> +
> +error_release_mdev:
> + cio2_->release();
> + imgu_->release();
> +
> + 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.
> + *
> + * Cameras are here created and stored in the member field 'camerasMap_' map,
> + * indexed by the id of the CSI-2 receiver they are connected, to maintain
> + * an ordering that does not depend on the device enumeration order.
Is keeping order between the cameras a issue? As we will support
hot-plug/unplug the over all order of cameras if one looks from the
applications point of view can be different anyhow between two listings.
Maybe we should sort all cameras alphabetically before handing over a
list to the application to provide some sort of consistency in list
order?
> + *
> + * The function returns the number of cameras found in the system.
> + */
> +unsigned int PipelineHandlerIPU3::registerCameras()
> +{
> + const std::vector<MediaEntity *> entities = cio2_->entities();
> + unsigned int numCameras = 0;
> + struct {
> + unsigned int id;
> + MediaEntity *csi2;
> + } csi2Receivers[] = {
> + { 0, cio2_->getEntityByName("ipu3-csi2 0") },
> + { 1, cio2_->getEntityByName("ipu3-csi2 1") },
> + { 2, cio2_->getEntityByName("ipu3-csi2 2") },
> + { 3, cio2_->getEntityByName("ipu3-csi2 3") },
> + };
> +
> + /*
> + * For each CSI-2 receiver on the IPU3, create a Camera if an
> + * image sensor is connected to it.
> + */
> + for (auto csi2Receiver : csi2Receivers) {
> + MediaEntity *csi2 = csi2Receiver.csi2;
> + unsigned int id = csi2Receiver.id;
> +
> + /*
> + * This shall not happen, as the device enumerator matched
> + * all entities described in the cio2_dm DeviceMatch.
> + *
> + * As this check is basically free, better stay safe than sorry.
> + */
> + if (!csi2)
> + continue;
> +
> + std::vector<MediaPad *> pads = csi2->pads();
> + MediaPad *sink;
> + for (MediaPad *pad : pads) {
> + if (!(pad->flags() & MEDIA_PAD_FL_SINK))
> + continue;
> +
> + /* IPU3 CSI-2 receivers have a single sink pad. */
> + sink = pad;
> + break;
> + }
> +
> + std::vector<MediaLink *> links = sink->links();
> + if (links.empty())
> + continue;
> +
> + /*
> + * Verify that the receiver is connected to a sensor, enable
> + * the media link between the two, and create a Camera with
> + * a unique name.
> + *
> + * FIXME: This supports creating a single camera per CSI-2 receiver.
> + */
> + for (MediaLink *link : links) {
> + /* Again, this shall not happen, but better stay safe. */
> + if (!link->source())
> + continue;
> +
> + MediaEntity *sensor = link->source()->entity();
> + if (!sensor)
> + continue;
> +
> + if (sensor->function() != MEDIA_ENT_F_CAM_SENSOR)
> + continue;
> +
> + if (link->setEnabled(true))
> + continue;
> +
> + std::size_t pos = sensor->name().find(" ");
> + std::string cameraName = sensor->name().substr(0, pos);
> + cameraName += " " + std::to_string(id);
> +
> + camerasMap_[id] = new Camera(cameraName);
> +
> + LOG(Info) << "Registered Camera[" << numCameras
> + << "] \"" << cameraName << "\""
> + << " connected to CSI-2 receiver " << id;
> +
> + numCameras++;
> + break;
> + }
> + }
> +
> + return numCameras;
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, "Intel IPU3 pipeline handler");
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build
> new file mode 100644
> index 0000000..0ab766a
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/meson.build
> @@ -0,0 +1,3 @@
> +libcamera_sources += files([
> + 'ipu3.cpp',
> +])
> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> index 615ecd2..811c075 100644
> --- a/src/libcamera/pipeline/meson.build
> +++ b/src/libcamera/pipeline/meson.build
> @@ -1,3 +1,5 @@
> libcamera_sources += files([
> 'vimc.cpp',
> ])
> +
> +subdir('ipu3')
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list