[libcamera-devel,v2,5/5] libcamera: pipeline: Add Intel IPU3 pipeline

Message ID 20190116135949.2097-6-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: pipeline: Add Intel IPU3 pipeline handler
Related show

Commit Message

Jacopo Mondi Jan. 16, 2019, 1:59 p.m. UTC
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@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

Comments

Niklas Söderlund Jan. 16, 2019, 3:47 p.m. UTC | #1
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@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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Jan. 16, 2019, 4:39 p.m. UTC | #2
Hi Niklas,

On Wed, Jan 16, 2019 at 04:47:29PM +0100, Niklas Söderlund wrote:
> 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@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?

I could, I don't see any benefit if not optimizing away a integer
variable.

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

As the comments suggest, if this is a vector, the registration order
depends on the entity enumeration order, which might change between
driver versions, the probing order, and I suspect even through reboots
if there are timeouts involved in probing sensors.

Using a map makes sure the element are sorted according to their keys
value, and in this case I'm keying them on the CSI-2 receiver index
they are connected to.

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

I think the same problems of ordering cameras we have here will be
present again even if the issue is moved to 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).
>

Not sure. As the comment explains, the n-th camera in the system is
not always in position map[n].

That's why I have to iterate on map entries unregardles of theit key
value.

Eg.

CSI-2 index       Camera#
        0               0
        1               -
        2               1
        3               -

When asked for camera(1) the pipeline manager will give the camera
connected to CSI-2 receiver 2.

Again, that's meaningful order, consistent through driver versions
(and reboots, if that's an issue) that only depends on the device.

Question: hot-plug/hot-remove
Suppose we can hot-plug a camera to CSI-2 receiver #1. What now? What
was camera(1) is now camera(2)... So this approach as surely issues
with that.

I also think the big question here is: how would cameras be identified
by applications? By name? By index?

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

Back to the big question then: how will cameras be identified by
applications?

> > + *
> > + * 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@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund

Patch

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_;
+	std::map<unsigned int, Camera *> camerasMap_;
+
+	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;
+	}
+
+	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.
+ *
+ * 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')