libcamera: ipa: Pass a set of controls and return results from ipa::start()

This change allows controls passed into PipelineHandler::start to be
forwarded onto IPAInterface::start(). We also add a return channel if the
pipeline handler must action any of these controls, e.g. setting the
analogue gain or shutter speed in the sensor device.

The IPA interface wrapper isn't addressed as it will soon be replaced by
a new mechanism to handle IPC.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

15 files changed, 51 insertions, 23 deletions

diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h
index 8f767e84..ec27e48e 100644
--- a/include/libcamera/internal/ipa_context_wrapper.h
+++ b/include/libcamera/internal/ipa_context_wrapper.h
@@ -20,7 +20,8 @@ public:
 	~IPAContextWrapper();
 
 	int init(const IPASettings &settings) override;
-	int start() override;
+	int start(const IPAOperationData &data,
+		  IPAOperationData *result) override;
 	void stop() override;
 	void configure(const CameraSensorInfo &sensorInfo,
 		       const std::map<unsigned int, IPAStream> &streamConfig,
diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
index 322b7079..df12f9ce 100644
--- a/include/libcamera/ipa/ipa_interface.h
+++ b/include/libcamera/ipa/ipa_interface.h
@@ -153,7 +153,8 @@ public:
 	virtual ~IPAInterface() = default;
 
 	virtual int init(const IPASettings &settings) = 0;
-	virtual int start() = 0;
+	virtual int start(const IPAOperationData &data,
+			  IPAOperationData *result) = 0;
 	virtual void stop() = 0;
 
 	virtual void configure(const CameraSensorInfo &sensorInfo,
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index cee532e3..40628489 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -95,7 +95,9 @@ int IPAInterfaceWrapper::start(struct ipa_context *_ctx)
 {
 	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
 
-	return ctx->ipa_->start();
+	/* \todo Translate the data and result. */
+	IPAOperationData data = {};
+	return ctx->ipa_->start(data, nullptr);
 }
 
 void IPAInterfaceWrapper::stop(struct ipa_context *_ctx)
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 5824d3b5..d40e9f09 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -78,13 +78,14 @@ public:
 	}
 
 	int init(const IPASettings &settings) override;
-	int start() override { return 0; }
+	int start([[maybe_unused]] const IPAOperationData &data,
+		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
 	void stop() override {}
 
 	void configure(const CameraSensorInfo &sensorInfo,
 		       const std::map<unsigned int, IPAStream> &streamConfig,
 		       const std::map<unsigned int, const ControlInfoMap &> &entityControls,
-		       const IPAOperationData &data,
+		       const IPAOperationData &ipaConfig,
 		       IPAOperationData *response) override;
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 07d7f1b2..39783abd 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -37,7 +37,8 @@ public:
 	{
 		return 0;
 	}
-	int start() override { return 0; }
+	int start([[maybe_unused]] const IPAOperationData &data,
+		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
 	void stop() override {}
 
 	void configure(const CameraSensorInfo &info,
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index cf841135..074902ee 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -34,7 +34,8 @@ public:
 
 	int init(const IPASettings &settings) override;
 
-	int start() override;
+	int start(const IPAOperationData &data,
+		  IPAOperationData *result) override;
 	void stop() override;
 
 	void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
@@ -82,7 +83,8 @@ int IPAVimc::init(const IPASettings &settings)
 	return 0;
 }
 
-int IPAVimc::start()
+int IPAVimc::start([[maybe_unused]] const IPAOperationData &data,
+		   [[maybe_unused]] IPAOperationData *result)
 {
 	trace(IPAOperationStart);
 
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index 231300ce..19c44ad8 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -86,10 +86,11 @@ int IPAContextWrapper::init(const IPASettings &settings)
 	return 0;
 }
 
-int IPAContextWrapper::start()
+int IPAContextWrapper::start(const IPAOperationData &data,
+			     IPAOperationData *result)
 {
 	if (intf_)
-		return intf_->start();
+		return intf_->start(data, result);
 
 	if (!ctx_)
 		return 0;
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index 23fc56d7..5be6f787 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -536,10 +536,17 @@ namespace libcamera {
 /**
  * \fn IPAInterface::start()
  * \brief Start the IPA
+ * \param[in] data Protocol-specific data for the start operation
+ * \param[out] result Result of the start operation
  *
  * This method informs the IPA module that the camera is about to be started.
  * The IPA module shall prepare any resources it needs to operate.
  *
+ * The \a data and \a result parameters carry custom data passed by the
+ * pipeline handler to the IPA and back. The pipeline handler may set the \a
+ * result parameter to null if the IPA protocol doesn't need to pass a result
+ * back through the start() function.
+ *
  * \return 0 on success or a negative error code otherwise
  */
 
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 8f67873a..791ff199 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -752,7 +752,11 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
 	}
 
 	/* Start the IPA. */
-	ret = data->ipa_->start();
+	IPAOperationData ipaData = {};
+	IPAOperationData result = {};
+	if (controls)
+		ipaData.controls.emplace_back(*controls);
+	ret = data->ipa_->start(ipaData, &result);
 	if (ret) {
 		LOG(RPI, Error)
 			<< "Failed to start IPA for " << camera->id();
@@ -1189,7 +1193,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
 	}
 
 	/* Ready the IPA - it must know about the sensor resolution. */
-	IPAOperationData result;
+	IPAOperationData result = {};
 
 	ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
 			&result);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4e959fde..eaa10f9f 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -842,7 +842,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
 	if (ret)
 		return ret;
 
-	ret = data->ipa_->start();
+	IPAOperationData ipaData = {};
+	ret = data->ipa_->start(ipaData, nullptr);
 	if (ret) {
 		freeBuffers(camera);
 		LOG(RkISP1, Error)
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index d81b8598..2a5054a8 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -322,7 +322,8 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con
 	if (ret < 0)
 		return ret;
 
-	ret = data->ipa_->start();
+	IPAOperationData ipaData = {};
+	ret = data->ipa_->start(ipaData, nullptr);
 	if (ret) {
 		data->video_->releaseBuffers();
 		return ret;
diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
index b78a0e45..ea6f3e5e 100644
--- a/src/libcamera/proxy/ipa_proxy_linux.cpp
+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
@@ -30,7 +30,8 @@ public:
 	{
 		return 0;
 	}
-	int start() override { return 0; }
+	int start([[maybe_unused]] const IPAOperationData &data,
+		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
 	void stop() override {}
 	void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
 		       [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
index eead2883..a5fda2c8 100644
--- a/src/libcamera/proxy/ipa_proxy_thread.cpp
+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
@@ -26,7 +26,8 @@ public:
 	IPAProxyThread(IPAModule *ipam);
 
 	int init(const IPASettings &settings) override;
-	int start() override;
+	int start(const IPAOperationData &data,
+		  IPAOperationData *result) override;
 	void stop() override;
 
 	void configure(const CameraSensorInfo &sensorInfo,
@@ -50,9 +51,9 @@ private:
 			ipa_ = ipa;
 		}
 
-		int start()
+		int start(const IPAOperationData &data, IPAOperationData *result)
 		{
-			return ipa_->start();
+			return ipa_->start(data, result);
 		}
 
 		void stop()
@@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings)
 	return 0;
 }
 
-int IPAProxyThread::start()
+int IPAProxyThread::start(const IPAOperationData &data,
+			  IPAOperationData *result)
 {
 	running_ = true;
 	thread_.start();
 
-	return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking);
+	return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking,
+				   data, result);
 }
 
 void IPAProxyThread::stop()
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index 9f575f93..dbb6727f 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -120,7 +120,8 @@ protected:
 		}
 
 		/* Test start of IPA module. */
-		ipa_->start();
+		IPAOperationData data = {};
+		ipa_->start(data, nullptr);
 		timer.start(1000);
 		while (timer.isRunning() && trace_ != IPAOperationStart)
 			dispatcher->processEvents();
diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
index 59d991cb..47533d10 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -56,7 +56,8 @@ public:
 		return 0;
 	}
 
-	int start() override
+	int start([[maybe_unused]] const IPAOperationData &data,
+		  [[maybe_unused]] IPAOperationData *result) override
 	{
 		report(Op_start, TestPass);
 		return 0;
@@ -376,7 +377,7 @@ protected:
 			return TestFail;
 		}
 
-		ret = INVOKE(start);
+		ret = INVOKE(start, ipaConfig, nullptr);
 		if (ret == TestFail) {
 			cerr << "Failed to run start()";
 			return TestFail;