From patchwork Fri Mar 27 14:42: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: 26382 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 4D0F9C32DE for ; Fri, 27 Mar 2026 14:47:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DA8C362CD3; Fri, 27 Mar 2026 15:47:35 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="GQQZlGo8"; dkim-atps=neutral Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7742C62CDD for ; Fri, 27 Mar 2026 15:47:31 +0100 (CET) Received: by mail-wm1-x335.google.com with SMTP id 5b1f17b1804b1-48557c8ad47so16364115e9.0 for ; Fri, 27 Mar 2026 07:47:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1774622851; x=1775227651; 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=OzFBxGG9gFDSlVha3fKLwZcDwW2k/RRGuvZ0LossX9k=; b=GQQZlGo8aYLDKIbxAliUkSG8hxNsCmvOqnevl3cdCCxEZfUcGcLH2FIL6XGKxgFAEP g5C7gXmOxLZvpI2dkkRCjf2XYAC8G1jOXHSaQn8dJ19Y6nvHLbIkKaI3G9k1hTnvkEE2 5FnwPHlAVAfDD3hZoIEbM5MFJ9smIBowO1Y6PdP0KhMHCV1abO49DKB7aUF7mulmN85q roMBEz9i8/Eajz5LyBEwd/IXFQXZLyh50VtYJP1nIWNnKILol3qCS0IWVEe0VD0ePG0N tzFAGUf56WOkkilinIWTIOEJXALv3ibpX9Po8b3/U4xudDTYUPc7QxCBqQW9SRXvLWwg jDzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774622851; x=1775227651; 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=OzFBxGG9gFDSlVha3fKLwZcDwW2k/RRGuvZ0LossX9k=; b=lsY6QcNkwriwTIwuwIFKHt0bF2Wqbl/D+90pHxvmRr0JoNBKFlrXh3DkUGlfyby1Rx szj34kUa+HZ8B0RoZGFZ3B5UJnqN41I7hw+RzZRoLNwyxWm3REEbRFQxGcFHp35KYilO olhsNZVrtodOwainBYQE3y5SiJXVsHdJzBjp66pZ6pCpvcdwsl+tFmjVFlbSdjJCKWkY Dv2euoy59EELUKBfY76N2MygWpLnQJM4Utz2NESvCjUOIPCRpVZGFrb2LBN1czOt9209 lJ5rqCtPLSHIj6KRwyjUkR/ghpf9mYpS+zyjVT97i4aSsNB1OrV8v1HuToYbYwfSSzpu 9R4g== X-Gm-Message-State: AOJu0Yz+OzjP3PtP4g2dHut6dmJj17okprs44lGJIrxQfQ8582xgeQPG MNEyBm8l6KnSvmN5Oi8N28hGWBUAzw7qYgCAS1YFGlGqJF+0qwVe2cD8SMSSaHVOILa+T00qvSJ dcsxj X-Gm-Gg: ATEYQzyc0Joa2Q+dzFkk3MYKIfp9I8u16N4yX5/MLDTppHEUbnTLzW3Mspl6i0AXANO 37qXUvd+HaM91l1KDP68xUA4cPSqqHYcsVOXnsYlG+x34OH9C3MQ1nsRJJH1aAI32xzh1ZtGgAZ qHSHBKhUjsHfAX/iyJK0AedreDU8blnTdApr53OscnzAkUP9eZqpno7Yb8rLlR3E4svJIFOrbSv X43npCHPY6OwXpuuRvqeFRX7rL31G2dXPzAFwbGZlIIqDNd4F6vKPT2ux9WT1snvDhqMv44m27m 6my3iImliFa9wpklIC2uG8OroNEHHfuRGc0qJ9kg8iHa/YY2jywarlzpGcoysE97SGlVzHDWfpP GV7YZZtCF4c7WuzGMG2pVta4I+Nc5MI0ClU+vevVkKHMuJz2APUHsIqeZn21TrHYgjlW0K9uLwM GFBnrYuxxL5TRjufFh+HRk6xL6JVPwjrjiP8P8AJRBOtx2jd5k8UnPfi6nHwtzEPS4j2yskeUAc mn2pKyray697PLfKIBOLaus6BaACm0xi4fDyQ== X-Received: by 2002:a05:600c:4e55:b0:485:4533:9c47 with SMTP id 5b1f17b1804b1-48727ec77a9mr50048745e9.22.1774622850459; Fri, 27 Mar 2026 07:47:30 -0700 (PDT) Received: from localhost.localdomain ([2a06:61c0:f337:0:9c1f:b517:931a:3b19]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48722be608bsm125858545e9.0.2026.03.27.07.47.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Mar 2026 07:47:30 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 3/3] pipeline: rpi: Make control lists in requests properly atomic Date: Fri, 27 Mar 2026 14:42:36 +0000 Message-ID: <20260327144726.7983-4-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260327144726.7983-1-david.plowman@raspberrypi.com> References: <20260327144726.7983-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. 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 "ControlListSequence" metadata. Signed-off-by: David Plowman Reviewed-by: Naushir Patuck --- 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(-) 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 867ecf1b..c6df0934 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 "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. + */ + 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 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;