[PATCH v3 16/16] pipeline: rkisp1: Apply initial controls

[Stefan Klug]

Controls passed to Camera::start() are now correctly applied.  This is needed
to pass the per-frame-controls test.  The code uses the return value of
ipa->start() to return the initial controls and pass them to the sensor instead
of the setSensorControls signal because setSensorControls is asynchronous and
would reach the sensor too late.

Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
---
 include/libcamera/ipa/rkisp1.mojom       |  7 ++++++-
 src/ipa/rkisp1/rkisp1.cpp                | 21 ++++++++++++++++-----
 src/libcamera/pipeline/rkisp1/rkisp1.cpp |  9 +++++++--
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
index 1009e970..828308c6 100644
--- a/include/libcamera/ipa/rkisp1.mojom
+++ b/include/libcamera/ipa/rkisp1.mojom
@@ -13,13 +13,18 @@ struct IPAConfigInfo {
 	libcamera.ControlInfoMap sensorControls;
 };
 
+struct StartResult {
+	libcamera.ControlList controls;
+	int32 code;
+};
+
 interface IPARkISP1Interface {
 	init(libcamera.IPASettings settings,
 	     uint32 hwRevision,
 	     libcamera.IPACameraSensorInfo sensorInfo,
 	     libcamera.ControlInfoMap sensorControls)
 		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
-	start() => (int32 ret);
+	start(libcamera.ControlList controls) => (StartResult result);
 	stop();
 
 	configure(IPAConfigInfo configInfo,
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 240e11e6..6cde23cb 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -53,7 +53,7 @@ public:
 		 const IPACameraSensorInfo &sensorInfo,
 		 const ControlInfoMap &sensorControls,
 		 ControlInfoMap *ipaControls) override;
-	int start() override;
+	void start(const ControlList &controls, StartResult *result) override;
 	void stop() override;
 
 	int configure(const IPAConfigInfo &ipaConfig,
@@ -86,6 +86,7 @@ private:
 
 	/* Local parameter storage */
 	struct IPAContext context_;
+	bool initialControlsDone_;
 };
 
 namespace {
@@ -199,12 +200,19 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
 	return 0;
 }
 
-int IPARkISP1::start()
+void IPARkISP1::start(const ControlList &controls, StartResult *result)
 {
-	ControlList ctrls = getControls(0);
-	setSensorControls.emit(0, ctrls);
+	initialControlsDone_ = false;
+	/*
+	 * Todo: This is really shady. In order to apply the initial controls we already
+	 * queue a request for frame 0 and return the results.
+	 * We need to discuss the official recommendation to handle start controls
+	 */
+	queueRequest(0, controls);
 
-	return 0;
+	result->controls = getControls(0);
+	result->code = 0;
+	initialControlsDone_ = true;
 }
 
 void IPARkISP1::stop()
@@ -323,6 +331,9 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)
 		algo->queueRequest(context_, frame, frameContext, controls);
 	}
 
+	if (!initialControlsDone_)
+		return;
+
 	/* Fast path, if we are not regulating */
 	if (!frameContext.agc.autoEnabled) {
 		ControlList ctrls = getControls(frame);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index ab452d8f..fec3c58b 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -927,13 +927,18 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL
 	if (ret)
 		return ret;
 
-	ret = data->ipa_->start();
-	if (ret) {
+	ControlList ctrls;
+	if (!!controls)
+		ctrls = *controls;
+	ipa::rkisp1::StartResult res;
+	data->ipa_->start(ctrls, &res);
+	if (res.code) {
 		freeBuffers(camera);
 		LOG(RkISP1, Error)
 			<< "Failed to start IPA " << camera->id();
 		return ret;
 	}
+	data->delayedCtrls_->reset(&res.controls);
 
 	data->frame_ = 0;
 
-- 
2.40.1