From patchwork Thu May 7 12:20:36 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 26674 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1CD34C32F8 for ; Thu, 7 May 2026 12:55:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D26656301E; Thu, 7 May 2026 14:55:08 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="QAQeXgLB"; dkim-atps=neutral Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EEC8662010 for ; Thu, 7 May 2026 14:55:03 +0200 (CEST) Received: by mail-wm1-x32e.google.com with SMTP id 5b1f17b1804b1-48a7fe4f40bso9326305e9.0 for ; Thu, 07 May 2026 05:55:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1778158503; x=1778763303; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=7qXhgolOuJk656ES/U26gIkNoakVkmPr3O4bl7zAoi4=; b=QAQeXgLBt1B4CGCOVEmQsoj67ezSqHZfhMC1kLQkLMDhbCUjDFLcyUo96QkdDBKFov TrXJ0spUDaG+zupzMpmFrR6Zyn5e5ikUKe7qFHo913C3jOEB8yQTIvenMuZaik7+vcg4 i2JHbf8gIx2iGZA4vEfA1LqtbNirPCd0GDaGTdfzVPsJLdH5jDmxK1tpfP80SxHfyHKZ zZMan8Qo+Fz2ShPGJxqWvmbhIAisA2X5Cwu526+4YiV8lSb6fgyUpXcC3EWrXxtQsfdf nWfkXRvvFO/lv/9QazjJuH6Cs8jnS1YjnN7ejumF1q3gNHXkws39wzpSDH40XS+ZBDmx bNDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778158503; x=1778763303; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=7qXhgolOuJk656ES/U26gIkNoakVkmPr3O4bl7zAoi4=; b=SHWtDXEcO9cfJyncXCk83B+ah0KKquU9S6M3uDyS9Vql8RiJVy+oC2JIVJycTDFEic HeVuLeC902AlQL69f+lkFveXBwsQAODzCvIUpdWOlNb8jMkjjZfAZN/KqSc3TIZcE/Fp VA0osta1yAwTD008knE2KInDm1zx56ClLP1ksTGmU1sAkHNMSrH2cD1aXDK+RrbbZp4w Pnj5xEcIpi1Bo6pnr1gbNoSsSvwTmb2DrGmYQXHrsEBEGRIDigmL0QNr5PHsN14l/kVH rnRWiA+aA+mKHOvwM5PsbcSxSS7licLmwb57Hj6I66qHKFsI300VtO8g3kEePvzVsUru bXwQ== X-Gm-Message-State: AOJu0Yygt9ygEGTYWlHPDyhBmGf4hnth1I0rJSBdEy2CEPp4QuJfKfoJ qQB4EMSgmlaeriTIDt56bNxXzMtrHcStLuP2VR5YZ1NqnHMhwdsDKt9am+/KfhCnrixkTxa/V1R Qjbls X-Gm-Gg: AeBDiesHv2gOMEzYb+nTI34Fb0Auk2a0F64hVvNR2S7d+GWNrqymUYCptQCrql75nkc Ez3w9YgpcJfkez0Jgl64VOVfI2CAyy+TRHBR/q7tkFEnTupFxp41ZBG6tXZt5ck6yvJ69ApHZXf F3Wm2Q+VT70pGillyoDxS/ZCjlpho4Q2gNbjOzANICczq0B2V5x6lkxmY6y192QhO5HmvRcMcyF Ji5+X8EqpeZtPAEHt/KEzYVHvz3Y9/9PsByKEmS/nWoSLIKEC8oRNZFgJ9b/3Xcslp9o7KHf5f/ qmZPciyFU1wkU6ZV7lS1fXOK7mDNS+FLgMtrfdwdS35zrQdVPBtuw1bnmKIUX7K0XX4rcHtC7vr lHXTgGrsFTDaF1jQHr1FKVJfPM4DOivPDp7CBSnwFS0q1/L88vDRSOKMUTH8kd5Guu1v5Fxx/EG 353SGvYylgx93yyOuARQXe+QrARHmjUWevjrNk6p/I7/YfznnpSFc6WFQRnYCHHYIYxj4eRCcGS T1GRorKqatgUWNhtz+B39SxxAH0HOiEkt/1zwrPXtBB+RapJENZxENGu2TGRq0= X-Received: by 2002:a05:600c:a305:b0:48a:5970:2005 with SMTP id 5b1f17b1804b1-48e51e08362mr87094505e9.2.1778158503007; Thu, 07 May 2026 05:55:03 -0700 (PDT) Received: from davidp-pi5.pitowers.org ([2a00:1098:3142:1f:88ea:c658:5b20:5e46]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-450524833e1sm20396324f8f.2.2026.05.07.05.55.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 May 2026 05:55:02 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v4 3/3] pipeline: rpi: Make control lists in requests properly atomic Date: Thu, 7 May 2026 13:20:36 +0100 Message-ID: <20260507125458.12140-4-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260507125458.12140-1-david.plowman@raspberrypi.com> References: <20260507125458.12140-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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. It does so in a copy of the request's control list, so as to avoid disturbing the request's original version. 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 Reviewed-by: Naushir Patuck --- src/ipa/rpi/common/ipa_base.cpp | 24 ++++++--- .../pipeline/rpi/common/pipeline_base.cpp | 49 +++++++++++++++++++ .../pipeline/rpi/common/pipeline_base.h | 9 ++++ src/libcamera/pipeline/rpi/pisp/pisp.cpp | 9 ++-- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 9 ++-- 5 files changed, 86 insertions(+), 14 deletions(-) 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 ¶ms) 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 ¶ms) */ AgcStatus agcStatus; bool hdrChange = false; - RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext]; + RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()]; if (!delayedMetadata.get("agc.status", agcStatus)) { rpiMetadata.set("agc.delayed_status", agcStatus); hdrChange = agcStatus.hdr.mode != hdrStatus_.mode; @@ -464,9 +473,13 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) */ helper_->prepare(embeddedBuffer, rpiMetadata); + bool delayedRequestControls = false; + delayedMetadata.get("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 ¶ms) 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() * 1.0us); - - libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get()); break; } @@ -976,9 +987,6 @@ void IpaBase::applyControls(const ControlList &controls) break; agc->setFixedGain(0, ctrl.second.get()); - - libcameraMetadata_.set(controls::AnalogueGain, - ctrl.second.get()); break; } diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index f6ef8a3b..0c82f549 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -1528,4 +1528,53 @@ 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, ControlList &requestControls) +{ + /* + * The delayContext is the sequence number after it's gone through the various + * pipeline delays, so that's what gets reported as the "ControlListSequence" + * 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::ControlListSequence, 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. + * + * Note that we are passed a copy of the request's controls, so that we can + * avoid disturbing what was in the original request. + */ + ControlList controls = std::move(requestControls); + requestControls.clear(); + immediateControls_.push({ request->sequence(), {} }); + for (const auto &ctrl : controls) { + if (isControlDelayed(ctrl.first)) + requestControls.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) { + requestControls.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 7bfac33e..134262b6 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h @@ -180,10 +180,19 @@ public: ClockRecovery wallClockRecovery_; + struct ImmediateControlsEntry { + uint64_t controlListId; + ControlList controls; + }; + std::queue immediateControls_; + protected: void fillRequestMetadata(const ControlList &bufferControls, Request *request); + void handleControlLists(uint32_t delayContext, + ControlList &requestControls); + virtual void tryRunPipeline() = 0; private: diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp index cc7e32c3..690d2511 100644 --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp @@ -2322,9 +2322,6 @@ void PiSPCameraData::tryRunPipeline() fillRequestMetadata(job.sensorControls, request); - /* Set our state to say the pipeline is active. */ - state_ = State::Busy; - unsigned int bayerId = cfe_[Cfe::Output0].getBufferId(job.buffers[&cfe_[Cfe::Output0]]); unsigned int statsId = cfe_[Cfe::Stats].getBufferId(job.buffers[&cfe_[Cfe::Stats]]); ASSERT(bayerId && statsId); @@ -2343,6 +2340,12 @@ void PiSPCameraData::tryRunPipeline() params.sensorControls = std::move(job.sensorControls); params.requestControls = request->controls(); + /* This sorts out synchronisation with ControlLists in earlier requests. */ + handleControlLists(job.delayContext, params.requestControls); + + /* Set our state to say the pipeline is active. */ + state_ = State::Busy; + if (sensorMetadata_) { unsigned int embeddedId = cfe_[Cfe::Embedded].getBufferId(job.buffers[&cfe_[Cfe::Embedded]]); diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index f99cfdbc..28e7ad46 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -939,9 +939,6 @@ void Vc4CameraData::tryRunPipeline() fillRequestMetadata(bayerFrame.controls, request); - /* Set our state to say the pipeline is active. */ - state_ = State::Busy; - unsigned int bayer = unicam_[Unicam::Image].getBufferId(bayerFrame.buffer); LOG(RPI, Debug) << "Signalling prepareIsp:" @@ -955,6 +952,12 @@ void Vc4CameraData::tryRunPipeline() params.delayContext = bayerFrame.delayContext; params.buffers.embedded = 0; + /* This sorts out synchronisation with ControlLists in earlier requests. */ + handleControlLists(bayerFrame.delayContext, params.requestControls); + + /* Set our state to say the pipeline is active. */ + state_ = State::Busy; + if (embeddedBuffer) { unsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer);