[v3,14/41] ipa: rkisp1: Move setSensorControls signal to computeParams
diff mbox series

Message ID 20260914140309.3354666-15-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • rkisp1: pipeline rework for PFC
Related show

Commit Message

Stefan Klug Sept. 14, 2026, 2:02 p.m. UTC
The setSensorControls event is emitted in the processStats() function.
On first sight this looks reasonable as in processStats() we got the
latest statistics and can therefore calculate the most up to date sensor
controls.

In the light of per-frame-controls however it produces difficult to
solve timing issues:

- The frame context in processStats() is the frame context of the frame
  that produced the stats, not for the frame that should be prepared and
sent to the sensor.

- To synchronize digital gain applied in the ISP with the analog gain
  applied in the sensor the set of parameters prepared for sensor and
ISP must also be synchronized, which is currently not the case.

To fix that, move the calculation and setting of sensor controls into
the computeParams(). This way the model is far more easy to understand.

We lose a tiny option for optimizations in that (in theory) we could
delay the calculation of ISP parameters by another frame (assuming the
sensor has a typical 2-frame delay). But all discussions and tests
showed that keeping all parameters in sync is more important than that
possible optimization for one frame.

To ensure setSensorControls() still gets emitted in raw mode, allow
computeParams() to be called with a zero bufferId. This strategy is also
used for processStats() to ensure that metadata gets filled in raw mode.
Then call computeParams() for raw mode also.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

Changes in v3:
- Squashed the fix for raw mode into this patch as it is a logical unit

Changes in v2:
- Collected tag
---
 src/ipa/rkisp1/rkisp1.cpp                | 22 ++++++++++------------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp |  6 ++++++
 2 files changed, 16 insertions(+), 12 deletions(-)

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index eff1f40d605b..98fc3244d9a6 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -302,13 +302,18 @@  void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)
 {
 	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
 
-	RkISP1Params params(context_.configuration.paramFormat,
-			    mappedBuffers_.at(bufferId).planes()[0]);
+	if (bufferId != 0) {
+		RkISP1Params params(context_.configuration.paramFormat,
+				    mappedBuffers_.at(bufferId).planes()[0]);
 
-	for (const auto &algo : algorithms())
-		algo->prepare(context_, frame, frameContext, &params);
+		for (const auto &algo : algorithms())
+			algo->prepare(context_, frame, frameContext, &params);
 
-	paramsComputed.emit(frame, params.bytesused());
+		paramsComputed.emit(frame, params.bytesused());
+	}
+
+	ControlList ctrls = getSensorControls(frameContext);
+	setSensorControls.emit(frame, ctrls);
 }
 
 void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
@@ -337,13 +342,6 @@  void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
 		algo->process(context_, frame, frameContext, stats, metadata);
 	}
 
-	/*
-	 * \todo: Here we should do a lookahead that takes the sensor delays
-	 * into account.
-	 */
-	ControlList ctrls = getSensorControls(frameContext);
-	setSensorControls.emit(frame, ctrls);
-
 	context_.debugMetadata.moveEntries(metadata);
 	metadataReady.emit(frame, metadata);
 }
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index ffd49157ec67..d3940692c11b 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1352,6 +1352,12 @@  int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
 
 		if (data->selfPath_ && info->selfPathBuffer)
 			data->selfPath_->queueBuffer(info->selfPathBuffer);
+
+		/*
+		 * Call computeParams with an empty param buffer to trigger the
+		 * setSensorControls signal.
+		 */
+		data->ipa_->computeParams(data->frame_, 0);
 	} else {
 		data->ipa_->computeParams(data->frame_,
 					  info->paramBuffer->cookie());