[libcamera-devel,v4,3/9] ipu3: Use imgu0 as default
diff mbox series

Message ID 20220802102943.3221109-4-chenghaoyang@google.com
State New
Headers show
Series
  • Use two imgus in ipu3 pipeline handler
Related show

Commit Message

Harvey Yang Aug. 2, 2022, 10:29 a.m. UTC
From: Harvey Yang <chenghaoyang@chromium.org>

With only one camera being started, we can always use imgu0 to process
frames (for video/preview). In the following patches, we'll use imgu1
for still capture if needed.

Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/libcamera/pipeline/ipu3/imgu.cpp | 13 ++++
 src/libcamera/pipeline/ipu3/imgu.h   |  2 +
 src/libcamera/pipeline/ipu3/ipu3.cpp | 96 ++++++++++++++--------------
 3 files changed, 63 insertions(+), 48 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index e5bbc382..aa72b1ec 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -765,4 +765,17 @@  int ImgUDevice::enableLinks(bool enable)
 	return linkSetup(name_, PAD_STAT, statName, 0, enable);
 }
 
+/**
+ * \brief Disconnect bufferReady signals from |input_|, |param_|, |output_|,
+ * |viewfinder_|, and |stat_|.
+ */
+void ImgUDevice::disconnectSignals()
+{
+	input_->bufferReady.disconnect();
+	param_->bufferReady.disconnect();
+	output_->bufferReady.disconnect();
+	viewfinder_->bufferReady.disconnect();
+	stat_->bufferReady.disconnect();
+}
+
 } /* namespace libcamera */
diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
index 0af4dd8a..aa88e36e 100644
--- a/src/libcamera/pipeline/ipu3/imgu.h
+++ b/src/libcamera/pipeline/ipu3/imgu.h
@@ -92,6 +92,8 @@  public:
 
 	int enableLinks(bool enable);
 
+	void disconnectSignals();
+
 	std::unique_ptr<V4L2Subdevice> imgu_;
 	std::unique_ptr<V4L2VideoDevice> input_;
 	std::unique_ptr<V4L2VideoDevice> param_;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 091a40e1..c3e90f22 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -64,7 +64,8 @@  public:
 	void frameStart(uint32_t sequence);
 
 	CIO2Device cio2_;
-	ImgUDevice *imgu_;
+	ImgUDevice *imgu0_;
+	ImgUDevice *imgu1_;
 
 	Stream outStream_;
 	Stream vfStream_;
@@ -406,7 +407,7 @@  CameraConfiguration::Status IPU3CameraConfiguration::validate()
 
 	/* Only compute the ImgU configuration if a YUV stream has been requested. */
 	if (yuvCount) {
-		pipeConfig_ = data_->imgu_->calculatePipeConfig(&pipe);
+		pipeConfig_ = data_->imgu0_->calculatePipeConfig(&pipe);
 		if (pipeConfig_.isNull()) {
 			LOG(IPU3, Error) << "Failed to calculate pipe configuration: "
 					 << "unsupported resolutions.";
@@ -518,16 +519,12 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	Stream *outStream = &data->outStream_;
 	Stream *vfStream = &data->vfStream_;
 	CIO2Device *cio2 = &data->cio2_;
-	ImgUDevice *imgu = data->imgu_;
 	V4L2DeviceFormat outputFormat;
 	int ret;
 
 	/*
-	 * FIXME: enabled links in one ImgU pipe interfere with capture
-	 * operations on the other one. This can be easily triggered by
-	 * capturing from one camera and then trying to capture from the other
-	 * one right after, without disabling media links on the first used
-	 * pipe.
+	 * Enabled links in one ImgU pipe interfere with capture
+	 * operations on the other one.
 	 *
 	 * The tricky part here is where to disable links on the ImgU instance
 	 * which is currently not in use:
@@ -543,11 +540,11 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	 * configuring the device, to allow alternate the usage of the two
 	 * ImgU pipes.
 	 *
-	 * As a consequence, a Camera using an ImgU shall be configured before
-	 * any start()/stop() sequence. An application that wants to
-	 * pre-configure all the camera and then start/stop them alternatively
-	 * without going through any re-configuration (a sequence that is
-	 * allowed by the Camera state machine) would now fail on the IPU3.
+	 * Now that only one camera is allowed to be used at the same time, we
+	 * don't need to handle the complicated interference between the two
+	 * ImgU links. Therefore, an application still cannot pre-configure all
+	 * the cameras and then start/stop them alternatively without going
+	 * throught any re-configuration.
 	 */
 	ret = imguMediaDev_->disableLinks();
 	if (ret)
@@ -560,7 +557,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	 * stream which is for raw capture, in which case no buffers will
 	 * ever be queued to the ImgU.
 	 */
-	ret = data->imgu_->enableLinks(true);
+	ret = imgu0_.enableLinks(true);
 	if (ret)
 		return ret;
 
@@ -610,7 +607,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	if (imguConfig.isNull())
 		return 0;
 
-	ret = imgu->configure(imguConfig, &cio2Format);
+	ret = imgu0_.configure(imguConfig, &cio2Format);
 	if (ret)
 		return ret;
 
@@ -624,12 +621,12 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 
 		if (stream == outStream) {
 			mainCfg = &cfg;
-			ret = imgu->configureOutput(cfg, &outputFormat);
+			ret = imgu0_.configureOutput(cfg, &outputFormat);
 			if (ret)
 				return ret;
 		} else if (stream == vfStream) {
 			vfCfg = &cfg;
-			ret = imgu->configureViewfinder(cfg, &outputFormat);
+			ret = imgu0_.configureViewfinder(cfg, &outputFormat);
 			if (ret)
 				return ret;
 		}
@@ -641,13 +638,13 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	 * be at least one active stream in the configuration request).
 	 */
 	if (!vfCfg) {
-		ret = imgu->configureViewfinder(*mainCfg, &outputFormat);
+		ret = imgu0_.configureViewfinder(*mainCfg, &outputFormat);
 		if (ret)
 			return ret;
 	}
 
 	/* Apply the "pipe_mode" control to the ImgU subdevice. */
-	ControlList ctrls(imgu->imgu_->controls());
+	ControlList ctrls(imgu0_.imgu_->controls());
 	/*
 	 * Set the ImgU pipe mode to 'Video' unconditionally to have statistics
 	 * generated.
@@ -657,7 +654,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	 */
 	ctrls.set(V4L2_CID_IPU3_PIPE_MODE,
 		  static_cast<int32_t>(IPU3PipeModeVideo));
-	ret = imgu->imgu_->setControls(&ctrls);
+	ret = imgu0_.imgu_->setControls(&ctrls);
 	if (ret) {
 		LOG(IPU3, Error) << "Unable to set pipe_mode control";
 		return ret;
@@ -691,9 +688,9 @@  int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
 	unsigned int count = stream->configuration().bufferCount;
 
 	if (stream == &data->outStream_)
-		return data->imgu_->output_->exportBuffers(count, buffers);
+		return imgu0_.output_->exportBuffers(count, buffers);
 	else if (stream == &data->vfStream_)
-		return data->imgu_->viewfinder_->exportBuffers(count, buffers);
+		return imgu0_.viewfinder_->exportBuffers(count, buffers);
 	else if (stream == &data->rawStream_)
 		return data->cio2_.exportBuffers(count, buffers);
 
@@ -711,7 +708,6 @@  int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
 int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
 {
 	IPU3CameraData *data = cameraData(camera);
-	ImgUDevice *imgu = data->imgu_;
 	unsigned int bufferCount;
 	int ret;
 
@@ -721,26 +717,26 @@  int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
 		data->rawStream_.configuration().bufferCount,
 	});
 
-	ret = imgu->allocateBuffers(bufferCount);
+	ret = imgu0_.allocateBuffers(bufferCount);
 	if (ret < 0)
 		return ret;
 
 	/* Map buffers to the IPA. */
 	unsigned int ipaBufferId = 1;
 
-	for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {
+	for (const std::unique_ptr<FrameBuffer> &buffer : imgu0_.paramBuffers_) {
 		buffer->setCookie(ipaBufferId++);
 		ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
 	}
 
-	for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {
+	for (const std::unique_ptr<FrameBuffer> &buffer : imgu0_.statBuffers_) {
 		buffer->setCookie(ipaBufferId++);
 		ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
 	}
 
 	data->ipa_->mapBuffers(ipaBuffers_);
 
-	data->frameInfos_.init(imgu->paramBuffers_, imgu->statBuffers_);
+	data->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);
 	data->frameInfos_.bufferAvailable.connect(
 		data, &IPU3CameraData::queuePendingRequests);
 
@@ -760,7 +756,7 @@  int PipelineHandlerIPU3::freeBuffers(Camera *camera)
 	data->ipa_->unmapBuffers(ids);
 	ipaBuffers_.clear();
 
-	data->imgu_->freeBuffers();
+	imgu0_.freeBuffers();
 
 	return 0;
 }
@@ -785,9 +781,18 @@  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
 
 	IPU3CameraData *data = cameraData(camera);
 	CIO2Device *cio2 = &data->cio2_;
-	ImgUDevice *imgu = data->imgu_;
 	int ret;
 
+	imgu0_.input_->bufferReady.connect(&data->cio2_,
+					   &CIO2Device::tryReturnBuffer);
+	imgu0_.output_->bufferReady.connect(data,
+					    &IPU3CameraData::imguOutputBufferReady);
+	imgu0_.viewfinder_->bufferReady.connect(data,
+						&IPU3CameraData::imguOutputBufferReady);
+	imgu0_.param_->bufferReady.connect(data,
+					   &IPU3CameraData::paramBufferReady);
+	imgu0_.stat_->bufferReady.connect(data,
+					  &IPU3CameraData::statBufferReady);
 	/* Disable test pattern mode on the sensor, if any. */
 	ret = cio2->sensor()->setTestPatternMode(
 		controls::draft::TestPatternModeEnum::TestPatternModeOff);
@@ -813,19 +818,21 @@  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
 	if (ret)
 		goto error;
 
-	ret = imgu->start();
+	ret = imgu0_.start();
 	if (ret)
 		goto error;
 
 	return 0;
 
 error:
-	imgu->stop();
+	imgu0_.stop();
 	cio2->stop();
 	data->ipa_->stop();
 	freeBuffers(camera);
 	LOG(IPU3, Error) << "Failed to start camera " << camera->id();
 
+	imgu0_.disconnectSignals();
+
 	return ret;
 }
 
@@ -838,12 +845,14 @@  void PipelineHandlerIPU3::stopDevice(Camera *camera)
 
 	data->ipa_->stop();
 
-	ret |= data->imgu_->stop();
+	ret |= imgu0_.stop();
 	ret |= data->cio2_.stop();
 	if (ret)
 		LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
 
 	freeBuffers(camera);
+
+	imgu0_.disconnectSignals();
 }
 
 void IPU3CameraData::cancelPendingRequests()
@@ -1186,7 +1195,8 @@  int PipelineHandlerIPU3::registerCameras()
 		 * only, and assign imgu0 to the first one and imgu1 to the
 		 * second.
 		 */
-		data->imgu_ = numCameras ? &imgu1_ : &imgu0_;
+		data->imgu0_ = &imgu0_;
+		data->imgu1_ = &imgu1_;
 
 		/*
 		 * Connect video devices' 'bufferReady' signals to their
@@ -1200,16 +1210,6 @@  int PipelineHandlerIPU3::registerCameras()
 					&IPU3CameraData::cio2BufferReady);
 		data->cio2_.bufferAvailable.connect(
 			data.get(), &IPU3CameraData::queuePendingRequests);
-		data->imgu_->input_->bufferReady.connect(&data->cio2_,
-					&CIO2Device::tryReturnBuffer);
-		data->imgu_->output_->bufferReady.connect(data.get(),
-					&IPU3CameraData::imguOutputBufferReady);
-		data->imgu_->viewfinder_->bufferReady.connect(data.get(),
-					&IPU3CameraData::imguOutputBufferReady);
-		data->imgu_->param_->bufferReady.connect(data.get(),
-					&IPU3CameraData::paramBufferReady);
-		data->imgu_->stat_->bufferReady.connect(data.get(),
-					&IPU3CameraData::statBufferReady);
 
 		/* Create and register the Camera instance. */
 		const std::string &cameraId = cio2->sensor()->id();
@@ -1302,14 +1302,14 @@  void IPU3CameraData::paramsBufferReady(unsigned int id)
 		FrameBuffer *outbuffer = it.second;
 
 		if (stream == &outStream_)
-			imgu_->output_->queueBuffer(outbuffer);
+			imgu0_->output_->queueBuffer(outbuffer);
 		else if (stream == &vfStream_)
-			imgu_->viewfinder_->queueBuffer(outbuffer);
+			imgu0_->viewfinder_->queueBuffer(outbuffer);
 	}
 
-	imgu_->param_->queueBuffer(info->paramBuffer);
-	imgu_->stat_->queueBuffer(info->statBuffer);
-	imgu_->input_->queueBuffer(info->rawBuffer);
+	imgu0_->param_->queueBuffer(info->paramBuffer);
+	imgu0_->stat_->queueBuffer(info->statBuffer);
+	imgu0_->input_->queueBuffer(info->rawBuffer);
 }
 
 void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)