[libcamera-devel] [PATCH 01/10] libcamera: ipu3: Group CIO2 devices

Jacopo Mondi jacopo at jmondi.org
Thu Feb 28 21:04:01 CET 2019


Group CIO2 devices (cio2, csi2 and image sensor) in a structure
associated with the CameraData, to ease management of the CIO2 devices.

Update the IPU3 pipeline handler implementation to avoid name clashes
and break-out IPU3CameraData from the pipeline handler class for
clarity.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 251 ++++++++++++++-------------
 1 file changed, 133 insertions(+), 118 deletions(-)

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 9694d0ce51ab..d3f1d9a95f81 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -24,6 +24,30 @@ namespace libcamera {
 
 LOG_DEFINE_CATEGORY(IPU3)
 
+struct Cio2Device {
+	Cio2Device()
+		: output(nullptr), csi2(nullptr), sensor(nullptr) {}
+
+	~Cio2Device()
+	{
+		delete output;
+		delete csi2;
+		delete sensor;
+	}
+
+	V4L2Device *output;
+	V4L2Subdevice *csi2;
+	V4L2Subdevice *sensor;
+};
+
+class IPU3CameraData : public CameraData
+{
+public:
+	Cio2Device cio2;
+
+	Stream stream_;
+};
+
 class PipelineHandlerIPU3 : public PipelineHandler
 {
 public:
@@ -47,65 +71,47 @@ public:
 	bool match(DeviceEnumerator *enumerator);
 
 private:
-	class IPU3CameraData : public CameraData
-	{
-	public:
-		IPU3CameraData()
-			: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}
-
-		~IPU3CameraData()
-		{
-			delete cio2_;
-			delete csi2_;
-			delete sensor_;
-		}
-
-		V4L2Device *cio2_;
-		V4L2Subdevice *csi2_;
-		V4L2Subdevice *sensor_;
-
-		Stream stream_;
-	};
-
 	IPU3CameraData *cameraData(const Camera *camera)
 	{
 		return static_cast<IPU3CameraData *>(
 			PipelineHandler::cameraData(camera));
 	}
 
+	int initCio2(unsigned int index, Cio2Device *cio2);
 	void registerCameras();
 
-	std::shared_ptr<MediaDevice> cio2_;
-	std::shared_ptr<MediaDevice> imgu_;
+	std::shared_ptr<MediaDevice> cio2MediaDev_;
+	std::shared_ptr<MediaDevice> imguMediaDev_;
 };
 
 PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)
-	: PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr)
+	: PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr)
 {
 }
 
 PipelineHandlerIPU3::~PipelineHandlerIPU3()
 {
-	if (cio2_)
-		cio2_->release();
+	if (cio2MediaDev_)
+		cio2MediaDev_->release();
 
-	if (imgu_)
-		imgu_->release();
+	if (imguMediaDev_)
+		imguMediaDev_->release();
 }
 
 std::map<Stream *, StreamConfiguration>
 PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 					 std::vector<Stream *> &streams)
 {
-	IPU3CameraData *data = cameraData(camera);
 	std::map<Stream *, StreamConfiguration> configs;
+	IPU3CameraData *data = cameraData(camera);
+	V4L2Subdevice *sensor = data->cio2.sensor;
 	V4L2SubdeviceFormat format = {};
 
 	/*
 	 * FIXME: As of now, return the image format reported by the sensor.
 	 * In future good defaults should be provided for each stream.
 	 */
-	if (data->sensor_->getFormat(0, &format)) {
+	if (sensor->getFormat(0, &format)) {
 		LOG(IPU3, Error) << "Failed to create stream configurations";
 		return configs;
 	}
@@ -126,9 +132,9 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
 {
 	IPU3CameraData *data = cameraData(camera);
 	StreamConfiguration *cfg = &config[&data->stream_];
-	V4L2Subdevice *sensor = data->sensor_;
-	V4L2Subdevice *csi2 = data->csi2_;
-	V4L2Device *cio2 = data->cio2_;
+	V4L2Subdevice *sensor = data->cio2.sensor;
+	V4L2Subdevice *csi2 = data->cio2.csi2;
+	V4L2Device *cio2 = data->cio2.output;
 	V4L2SubdeviceFormat subdevFormat = {};
 	V4L2DeviceFormat devFormat = {};
 	int ret;
@@ -185,13 +191,14 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
 
 int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
 {
-	IPU3CameraData *data = cameraData(camera);
 	const StreamConfiguration &cfg = stream->configuration();
+	IPU3CameraData *data = cameraData(camera);
+	V4L2Device *cio2 = data->cio2.output;
 
 	if (!cfg.bufferCount)
 		return -EINVAL;
 
-	int ret = data->cio2_->exportBuffers(&stream->bufferPool());
+	int ret = cio2->exportBuffers(&stream->bufferPool());
 	if (ret) {
 		LOG(IPU3, Error) << "Failed to request memory";
 		return ret;
@@ -203,8 +210,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
 int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
 {
 	IPU3CameraData *data = cameraData(camera);
+	V4L2Device *cio2 = data->cio2.output;
 
-	int ret = data->cio2_->releaseBuffers();
+	int ret = cio2->releaseBuffers();
 	if (ret) {
 		LOG(IPU3, Error) << "Failed to release memory";
 		return ret;
@@ -216,9 +224,10 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
 int PipelineHandlerIPU3::start(const Camera *camera)
 {
 	IPU3CameraData *data = cameraData(camera);
+	V4L2Device *cio2 = data->cio2.output;
 	int ret;
 
-	ret = data->cio2_->streamOn();
+	ret = cio2->streamOn();
 	if (ret) {
 		LOG(IPU3, Info) << "Failed to start camera " << camera->name();
 		return ret;
@@ -230,14 +239,16 @@ int PipelineHandlerIPU3::start(const Camera *camera)
 void PipelineHandlerIPU3::stop(const Camera *camera)
 {
 	IPU3CameraData *data = cameraData(camera);
+	V4L2Device *cio2 = data->cio2.output;
 
-	if (data->cio2_->streamOff())
+	if (cio2->streamOff())
 		LOG(IPU3, Info) << "Failed to stop camera " << camera->name();
 }
 
 int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)
 {
 	IPU3CameraData *data = cameraData(camera);
+	V4L2Device *cio2 = data->cio2.output;
 	Stream *stream = &data->stream_;
 
 	Buffer *buffer = request->findBuffer(stream);
@@ -247,7 +258,7 @@ int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)
 		return -ENOENT;
 	}
 
-	data->cio2_->queueBuffer(buffer);
+	cio2->queueBuffer(buffer);
 
 	return 0;
 }
@@ -278,20 +289,20 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
 	imgu_dm.add("ipu3-imgu 1 viewfinder");
 	imgu_dm.add("ipu3-imgu 1 3a stat");
 
-	cio2_ = enumerator->search(cio2_dm);
-	if (!cio2_)
+	cio2MediaDev_ = enumerator->search(cio2_dm);
+	if (!cio2MediaDev_)
 		return false;
 
-	imgu_ = enumerator->search(imgu_dm);
-	if (!imgu_)
+	imguMediaDev_ = enumerator->search(imgu_dm);
+	if (!imguMediaDev_)
 		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();
+	cio2MediaDev_->acquire();
+	imguMediaDev_->acquire();
 
 	/*
 	 * Disable all links that are enabled by default on CIO2, as camera
@@ -300,28 +311,90 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
 	 * Close the CIO2 media device after, as links are enabled and should
 	 * not need to be changed after.
 	 */
-	if (cio2_->open())
+	if (cio2MediaDev_->open())
 		goto error_release_mdev;
 
-	if (cio2_->disableLinks())
+	if (cio2MediaDev_->disableLinks())
 		goto error_close_cio2;
 
 	registerCameras();
 
-	cio2_->close();
+	cio2MediaDev_->close();
 
 	return true;
 
 error_close_cio2:
-	cio2_->close();
+	cio2MediaDev_->close();
 
 error_release_mdev:
-	cio2_->release();
-	imgu_->release();
+	cio2MediaDev_->release();
+	imguMediaDev_->release();
 
 	return false;
 }
 
+int PipelineHandlerIPU3::initCio2(unsigned int index, Cio2Device *cio2)
+{
+	int ret;
+
+	/* Verify a sensor subdevice is connected to this CIO2 instance. */
+	std::string csi2Name = "ipu3-csi2 " + std::to_string(index);
+	MediaEntity *entity = cio2MediaDev_->getEntityByName(csi2Name);
+	if (!entity) {
+		LOG(IPU3, Error)
+			<< "Failed to get entity '" << csi2Name << "'";
+		return -ENODEV;
+	}
+
+	const std::vector<MediaPad *> &pads = entity->pads();
+	if (pads.empty())
+		return -EINVAL;
+
+	/* IPU3 CSI-2 receivers have a single sink pad at index 0. */
+	MediaPad *sink = pads[0];
+	const std::vector<MediaLink *> &links = sink->links();
+	if (links.empty())
+		return -EINVAL;
+
+	MediaLink *link = links[0];
+	MediaEntity *sensorEntity = link->source()->entity();
+	if (sensorEntity->function() != MEDIA_ENT_F_CAM_SENSOR)
+		return -ENODEV;
+
+	ret = link->setEnabled(true);
+	if (ret)
+		return ret;
+
+	/*
+	 * Now that we're sure a sensor subdevice is connected, create and open
+	 * video devices and subdevices associated with this CIO2 unit.
+	 */
+	cio2->sensor = new V4L2Subdevice(sensorEntity);
+	ret = cio2->sensor->open();
+	if (ret)
+		return ret;
+
+	cio2->csi2 = new V4L2Subdevice(entity);
+	ret = cio2->csi2->open();
+	if (ret)
+		return ret;
+
+	std::string cio2Name = "ipu3-cio2 " + std::to_string(index);
+	entity = cio2MediaDev_->getEntityByName(cio2Name);
+	if (!entity) {
+		LOG(IPU3, Error)
+			<< "Failed to get entity '" << cio2Name << "'";
+		return -EINVAL;
+	}
+
+	cio2->output = new V4L2Device(entity);
+	ret = cio2->output->open();
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 /*
  * Cameras are created associating an image sensor (represented by a
  * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four
@@ -329,85 +402,27 @@ error_release_mdev:
  */
 void PipelineHandlerIPU3::registerCameras()
 {
+	int ret;
+
 	/*
 	 * For each CSI-2 receiver on the IPU3, create a Camera if an
 	 * image sensor is connected to it.
 	 */
 	unsigned int numCameras = 0;
 	for (unsigned int id = 0; id < 4; ++id) {
-		std::string csi2Name = "ipu3-csi2 " + std::to_string(id);
-		MediaEntity *csi2 = cio2_->getEntityByName(csi2Name);
-		int ret;
-
-		/*
-		 * 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;
-
-		const std::vector<MediaPad *> &pads = csi2->pads();
-		if (pads.empty())
-			continue;
-
-		/* IPU3 CSI-2 receivers have a single sink pad at index 0. */
-		MediaPad *sink = pads[0];
-		const 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.
-		 */
-		MediaLink *link = links[0];
-		MediaEntity *sensor = link->source()->entity();
-		if (sensor->function() != MEDIA_ENT_F_CAM_SENSOR)
-			continue;
-
-		if (link->setEnabled(true))
-			continue;
-
-		std::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>();
-
-		std::string cameraName = sensor->name() + " " + std::to_string(id);
+		std::unique_ptr<IPU3CameraData> data =
+			utils::make_unique<IPU3CameraData>();
 		std::vector<Stream *> streams{ &data->stream_ };
-		std::shared_ptr<Camera> camera = Camera::create(this, cameraName, streams);
-
-		/*
-		 * Create and open video devices and subdevices associated with
-		 * the camera.
-		 *
-		 * If any of these operations fails, the Camera instance won't
-		 * be registered. The 'camera' shared pointer and the 'data'
-		 * unique pointers go out of scope and delete the objects they
-		 * manage.
-		 */
-		std::string cio2Name = "ipu3-cio2 " + std::to_string(id);
-		MediaEntity *cio2 = cio2_->getEntityByName(cio2Name);
-		if (!cio2) {
-			LOG(IPU3, Error)
-				<< "Failed to get entity '" << cio2Name << "'";
-			continue;
-		}
-
-		data->cio2_ = new V4L2Device(cio2);
-		ret = data->cio2_->open();
-		if (ret)
-			continue;
+		Cio2Device *cio2 = &data->cio2;
 
-		data->sensor_ = new V4L2Subdevice(sensor);
-		ret = data->sensor_->open();
+		ret = initCio2(id, cio2);
 		if (ret)
 			continue;
 
-		data->csi2_ = new V4L2Subdevice(csi2);
-		ret = data->csi2_->open();
-		if (ret)
-			continue;
+		std::string cameraName = cio2->sensor->deviceName() + " "
+				       + std::to_string(id);
+		std::shared_ptr<Camera> camera =
+			Camera::create(this, cameraName, streams);
 
 		setCameraData(camera.get(), std::move(data));
 		registerCamera(std::move(camera));
-- 
2.20.1



More information about the libcamera-devel mailing list