[v2,27/32] pipeline: rkisp1: Correctly handle params buffer for frame 0
diff mbox series

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

Commit Message

Stefan Klug March 25, 2026, 3:13 p.m. UTC 
The parameters for frame 0 are only active on frame 0 if the
corresponding parameter buffer is queued before STREAMON is called on
the isp. Therefore the normal mechanics of calling ipa->computeParams()
do not work, as it gets called after the first request is queued and
therefore the isp is already started. To fix that, handle the parameter
buffer the same way the initial sensor controls are handled by passing
it to the ipa->start() function and then queuing the params buffer
before starting the isp.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---

Changes in v2:
- Small cleanup
- Fixed crash in raw case where a null buffer is passed on start
- Collected tag
---
 include/libcamera/ipa/rkisp1.mojom       |  5 ++-
 src/ipa/rkisp1/rkisp1.cpp                | 39 +++++++++++++-----------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++++++++--
 3 files changed, 42 insertions(+), 21 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
index 4c29b53cd7f9..56c9fe8ab92a 100644
--- a/include/libcamera/ipa/rkisp1.mojom
+++ b/include/libcamera/ipa/rkisp1.mojom
@@ -17,6 +17,7 @@  struct IPAConfigInfo {
 struct StartResult {
 	libcamera.ControlList controls;
 	int32 code;
+	uint32 paramBufferBytesUsed;
 };
 
 interface IPARkISP1Interface {
@@ -25,7 +26,9 @@  interface IPARkISP1Interface {
 	     libcamera.IPACameraSensorInfo sensorInfo,
 	     libcamera.ControlInfoMap sensorControls)
 		=> (int32 ret, libcamera.ControlInfoMap ipaControls);
-	start(libcamera.ControlList controls) => (StartResult result);
+	start(libcamera.ControlList controls,
+	      uint32 paramBufferId)
+		=> (StartResult result);
 	stop();
 
 	configure(IPAConfigInfo configInfo,
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index d0c753976dd4..8a657a170b0d 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -57,7 +57,8 @@  public:
 		 const IPACameraSensorInfo &sensorInfo,
 		 const ControlInfoMap &sensorControls,
 		 ControlInfoMap *ipaControls) override;
-	void start(const ControlList &controls, StartResult *result) override;
+	void start(const ControlList &controls, const uint32_t paramBufferId,
+		   StartResult *result) override;
 	void stop() override;
 
 	int configure(const IPAConfigInfo &ipaConfig,
@@ -77,6 +78,8 @@  protected:
 	std::string logPrefix() const override;
 
 private:
+	uint32_t computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId);
+
 	void updateControls(const IPACameraSensorInfo &sensorInfo,
 			    const ControlInfoMap &sensorControls,
 			    ControlInfoMap *ipaControls);
@@ -219,9 +222,11 @@  int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
 	return 0;
 }
 
-void IPARkISP1::start(const ControlList &controls, StartResult *result)
+void IPARkISP1::start(const ControlList &controls, const uint32_t paramBufferId,
+		      StartResult *result)
 {
 	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(0, controls);
+	result->paramBufferBytesUsed = computeParamsInternal(frameContext, paramBufferId);
 	result->controls = getSensorControls(frameContext);
 	result->code = 0;
 }
@@ -353,32 +358,30 @@  void IPARkISP1::initializeFrameContext(IPAFrameContext &fc, const ControlList &c
 	}
 }
 
-void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)
+uint32_t IPARkISP1::computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
-
-	/*
-	 * \todo: This needs discussion. In raw mode, computeParams is
-	 * called without a params buffer, to trigger the setSensorControls
-	 * signal. Currently our algorithms don't support prepare calls with
-	 * a nullptr. Do we need that or can we safely skip it?
-	 */
-	if (bufferId == 0) {
-		ControlList ctrls = getSensorControls(frameContext);
-		setSensorControls.emit(frame, ctrls);
-		return;
-	}
+	if (bufferId == 0)
+		return 0;
 
 	RkISP1Params params(context_.configuration.paramFormat,
 			    mappedBuffers_.at(bufferId).planes()[0]);
 
 	for (const auto &algo : algorithms())
-		algo->prepare(context_, frame, frameContext, &params);
+		algo->prepare(context_, frameContext.frame(), frameContext, &params);
 
+	return params.bytesused();
+}
+
+void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)
+{
+	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+
+	uint32_t size = computeParamsInternal(frameContext, bufferId);
 	ControlList ctrls = getSensorControls(frameContext);
 	setSensorControls.emit(frame, ctrls);
 
-	paramsComputed.emit(frame, params.bytesused());
+	if (bufferId != 0)
+		paramsComputed.emit(frame, size);
 }
 
 void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 46c157526ea1..c2af5ef8897f 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -105,8 +105,8 @@  public:
 	bool canUseDewarper_;
 	bool usesDewarper_;
 
-private:
 	void paramsComputed(unsigned int frame, unsigned int bytesused);
+private:
 	void setSensorControls(unsigned int frame,
 			       const ControlList &sensorControls);
 
@@ -1202,13 +1202,28 @@  int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL
 	nextStatsToProcess_ = 0;
 	data->frame_ = 0;
 
+	uint32_t paramBufferId = 0;
+	FrameBuffer *paramBuffer = nullptr;
+	if (!isRaw_) {
+		paramBuffer = availableParamBuffers_.front();
+		paramBufferId = paramBuffer->cookie();
+	}
+
 	ipa::rkisp1::StartResult res;
-	data->ipa_->start(ctrls, &res);
+	data->ipa_->start(ctrls, paramBufferId, &res);
 	if (res.code) {
 		LOG(RkISP1, Error)
 			<< "Failed to start IPA " << camera->id();
 		return ret;
 	}
+
+	if (paramBuffer) {
+		availableParamBuffers_.pop();
+		computingParamBuffers_.push({ paramBuffer, nextParamsSequence_++ });
+		paramsSyncHelper_.pushCorrection(0);
+		data->paramsComputed(0, res.paramBufferBytesUsed);
+	}
+
 	actions += [&]() { data->ipa_->stop(); };
 	data->sensor_->setControls(&res.controls);
 	data->delayedCtrls_->reset();