[v1,3/3] pipeline: rpi: Make control lists in requests properly atomic
diff mbox series

Message ID 20260324151714.3345-4-david.plowman@raspberrypi.com
State New
Headers show
Series
  • Atomic control lists on Raspberry Pi
Related show

Commit Message

David Plowman March 24, 2026, 2:32 p.m. UTC
When a request is about to be processed, this commit separates
"delayed controls" (camera-related ones that take "a few frames" to
apply) from "immediate controls" (ISP-related controls) that will
happen instantly.

The immediate controls are held back until the delayed controls that
they were submitted with, have happened. This means all the controls
submitted in a request happen atomically.

We therefore already have the sequence number of the request whose
controls have just been applied, so we additionally attach this to the
current request as "ControlId" metadata.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/rpi/common/ipa_base.cpp               | 24 ++++++----
 .../pipeline/rpi/common/pipeline_base.cpp     | 46 +++++++++++++++++++
 .../pipeline/rpi/common/pipeline_base.h       |  8 ++++
 src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  3 ++
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  3 ++
 5 files changed, 76 insertions(+), 8 deletions(-)

Patch
diff mbox series

diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index faa77719..dacafa57 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -431,6 +431,15 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 	rpiMetadata.clear();
 	fillDeviceStatus(params.sensorControls, ipaContext);
 
+	/*
+	 * When there are controls, it's important that we don't skip running the
+	 * IPAs, as that can mess with synchronisation. Crucially though, we need
+	 * to know whether there were controls when this comes back as the
+	 * _delayed_ metadata, hence why we flag this in the metadata itself.
+	 */
+	if (!params.requestControls.empty())
+		rpiMetadata.set("ipa.request_controls", true);
+
 	if (params.buffers.embedded) {
 		/*
 		 * Pipeline handler has supplied us with an embedded data buffer,
@@ -451,7 +460,7 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 	 */
 	AgcStatus agcStatus;
 	bool hdrChange = false;
-	RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext];
+	RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()];
 	if (!delayedMetadata.get<AgcStatus>("agc.status", agcStatus)) {
 		rpiMetadata.set("agc.delayed_status", agcStatus);
 		hdrChange = agcStatus.hdr.mode != hdrStatus_.mode;
@@ -464,9 +473,13 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 	 */
 	helper_->prepare(embeddedBuffer, rpiMetadata);
 
+	bool delayedRequestControls = false;
+	delayedMetadata.get<bool>("ipa.request_controls", delayedRequestControls);
+
 	/* Allow a 10% margin on the comparison below. */
 	Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;
-	if (lastRunTimestamp_ && frameCount_ > invalidCount_ &&
+	if (!delayedRequestControls && params.requestControls.empty() &&
+	    lastRunTimestamp_ && frameCount_ > invalidCount_ &&
 	    delta < controllerMinFrameDuration_ * 0.9 && !hdrChange) {
 		/*
 		 * Ensure we merge the previous frame's metadata with the current
@@ -535,7 +548,7 @@  void IpaBase::processStats(const ProcessParams &params)
 		ControlList ctrls(sensorCtrls_);
 		applyAGC(&agcStatus, ctrls);
 		rpiMetadata.set("agc.status", agcStatus);
-		setDelayedControls.emit(ctrls, ipaContext);
+		setDelayedControls.emit(ctrls, params.ipaContext);
 		setCameraTimeoutValue();
 	}
 
@@ -951,8 +964,6 @@  void IpaBase::applyControls(const ControlList &controls)
 
 			/* The control provides units of microseconds. */
 			agc->setFixedExposureTime(0, ctrl.second.get<int32_t>() * 1.0us);
-
-			libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());
 			break;
 		}
 
@@ -976,9 +987,6 @@  void IpaBase::applyControls(const ControlList &controls)
 				break;
 
 			agc->setFixedGain(0, ctrl.second.get<float>());
-
-			libcameraMetadata_.set(controls::AnalogueGain,
-					       ctrl.second.get<float>());
 			break;
 		}
 
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index 867ecf1b..9e162870 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -1528,4 +1528,50 @@  void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request
 	}
 }
 
+static bool isControlDelayed(unsigned int id)
+{
+	return id == controls::ExposureTime ||
+	       id == controls::AnalogueGain ||
+	       id == controls::FrameDurationLimits ||
+	       id == controls::AeEnable ||
+	       id == controls::ExposureTimeMode ||
+	       id == controls::AnalogueGainMode;
+}
+
+void CameraData::handleControlLists(uint32_t delayContext)
+{
+	/*
+	 * THe delayContext is the sequence number after it's gone through the various
+	 * pipeline delays, so that's what gets reported as the "ControlId" in the
+	 * metadata, being the sequence number of the request whose ControlList has
+	 * just been applied.
+	 */
+	Request *request = requestQueue_.front();
+	request->_d()->metadata().set(controls::rpi::ControlId, delayContext);
+
+	/*
+	 * Controls that take effect immediately (typically ISP controls) have to be
+	 * delayed so as to synchronise with those controls that do get delayed. So we
+	 * must remove them from the current request, and push them onto a queue so
+	 * that they can be used later.
+	 */
+	ControlList controls = std::move(request->controls());
+	request->controls().clear();
+	immediateControls_.push({ request->sequence(), {} });
+	for (const auto &ctrl : controls) {
+		if (isControlDelayed(ctrl.first))
+			request->controls().set(ctrl.first, ctrl.second);
+		else
+			immediateControls_.back().controls.set(ctrl.first, ctrl.second);
+	}
+
+	/* "Immediate" controls that have become due are now merged back into this request. */
+	while (!immediateControls_.empty() &&
+	       immediateControls_.front().controlListId <= delayContext) {
+		request->controls().merge(immediateControls_.front().controls,
+					  ControlList::MergePolicy::OverwriteExisting);
+		immediateControls_.pop();
+	}
+}
+
 } /* namespace libcamera */
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
index 597eb587..65d8efdc 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
@@ -180,10 +180,18 @@  public:
 
 	ClockRecovery wallClockRecovery_;
 
+	struct ImmediateControlsEntry {
+		uint64_t controlListId;
+		ControlList controls;
+	};
+	std::queue<ImmediateControlsEntry> immediateControls_;
+
 protected:
 	void fillRequestMetadata(const ControlList &bufferControls,
 				 Request *request);
 
+	void handleControlLists(uint32_t delayContext);
+
 	virtual void tryRunPipeline() = 0;
 
 private:
diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
index dff73a79..cc8aa4d4 100644
--- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
+++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
@@ -2322,6 +2322,9 @@  void PiSPCameraData::tryRunPipeline()
 
 	fillRequestMetadata(job.sensorControls, request);
 
+	/* This sorts out synchronisation with ControlLists in earlier requests. */
+	handleControlLists(job.delayContext);
+
 	/* Set our state to say the pipeline is active. */
 	state_ = State::Busy;
 
diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
index b734889d..f743c8a7 100644
--- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
+++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
@@ -939,6 +939,9 @@  void Vc4CameraData::tryRunPipeline()
 
 	fillRequestMetadata(bayerFrame.controls, request);
 
+	/* This sorts out synchronisation with ControlLists in earlier requests. */
+	handleControlLists(bayerFrame.delayContext);
+
 	/* Set our state to say the pipeline is active. */
 	state_ = State::Busy;