[v2,12/12] pipeline: rkisp1: Apply initial controls
diff mbox series

Message ID 20240313121223.138150-13-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Preparation for per-frame-controls and initial tests
Related show

Commit Message

Stefan Klug March 13, 2024, 12:12 p.m. UTC
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@ideasonboard.com>
---
 include/libcamera/ipa/rkisp1.mojom       |  7 ++++++-
 src/ipa/rkisp1/rkisp1.cpp                | 20 ++++++++++++++++----
 src/libcamera/pipeline/rkisp1/rkisp1.cpp |  9 +++++++--
 3 files changed, 29 insertions(+), 7 deletions(-)

Patch
diff mbox series

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 8780abc9..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,11 +200,19 @@  int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
 	return 0;
 }
 
-int IPARkISP1::start()
+void IPARkISP1::start(const ControlList &controls, StartResult *result)
 {
-	setControls(0);
+	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()
@@ -322,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 3e061b52..7c68073d 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;