Patch Detail
Show a patch.
GET /api/patches/26693/?format=api
{ "id": 26693, "url": "https://patchwork.libcamera.org/api/patches/26693/?format=api", "web_url": "https://patchwork.libcamera.org/patch/26693/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20260508130804.8238-4-david.plowman@raspberrypi.com>", "date": "2026-05-08T12:57:43", "name": "[v5,3/3] pipeline: rpi: Make control lists in requests properly atomic", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "67bb553ee0cf9cb580bc7db23ea72ea33a5b512b", "submitter": { "id": 42, "url": "https://patchwork.libcamera.org/api/people/42/?format=api", "name": "David Plowman", "email": "david.plowman@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/26693/mbox/", "series": [ { "id": 5924, "url": "https://patchwork.libcamera.org/api/series/5924/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5924", "date": "2026-05-08T12:57:40", "name": "Atomic control lists on Raspberry Pi", "version": 5, "mbox": "https://patchwork.libcamera.org/series/5924/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/26693/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/26693/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AAFCFC32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 8 May 2026 13:08:13 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F22363021;\n\tFri, 8 May 2026 15:08:12 +0200 (CEST)", "from mail-wm1-x336.google.com (mail-wm1-x336.google.com\n\t[IPv6:2a00:1450:4864:20::336])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD62E62FE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 8 May 2026 15:08:08 +0200 (CEST)", "by mail-wm1-x336.google.com with SMTP id\n\t5b1f17b1804b1-4893940bb5eso12252505e9.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 08 May 2026 06:08:08 -0700 (PDT)", "from localhost.localdomain ([2a06:61c0:f337:0:9c1f:b517:931a:3b19])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-48e68f5d9b5sm34044905e9.15.2026.05.08.06.08.07\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 08 May 2026 06:08:07 -0700 (PDT)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"QjgoVAIx\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1778245688; x=1778850488;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=p0mMKpb1i8B3EQhJz0nd+cLEbb2uFro+HYoXPQ9QXgw=;\n\tb=QjgoVAIxa9Pmgypr5d4vOeelXdQwvwntBu4NrBAZBKJ3HQrT/pWwb2CPf9f8Zx38Xn\n\tGHlZrtpIP4tXKZxb8li62uJsRHrtQ4P+p3hplAt4Uy3FYRNTEvNIFErTJgKOMtPbLbO+\n\tsIkxHeus0n8PySaYk9Tlzx0VnAH6YfPHxP7KQI+B/P43UFsDatYFoln68VcSQ7g2dRc4\n\twItpcSi+g36/zkkdBA6YVHQcfHUh8AXv215/XUgpOrwsiXQS1ABT9czRDGnB6mP37Mma\n\tT9wmiQUgUtR1eH0uyYsv1rrUQJaWHs1dBahd87erbJ6RCeWl4qvspI4ibNXtjU11fjCi\n\t7Atg==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1778245688; x=1778850488;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=p0mMKpb1i8B3EQhJz0nd+cLEbb2uFro+HYoXPQ9QXgw=;\n\tb=DJrx4jE6QPYo9+WTIjXYrSAyp0pgBJNAflIen72G/Ndf8ABFWL/hv13NouapwbvXYe\n\t1zW+EpQt0Ysb81gM6Sd/D3nqcapfsrRCNnnlB51SBlCoJBB+iHy9OK5f9fGTRraJWykB\n\tShtRJAdj1byARu2fQrwhOC7+GV8d2fmkp/4ITGTM/i9jyh4NIw7FOdGjFTCpmYNKVNJ+\n\tGbKvqWTBI4tcjaDrqnrQT3i/kYyPfavxWs4xZNaCFCplkNjtm0tvKqF57mVlepbRAZKB\n\tLqGg8fic6JMt+rvT4/7n7m3d6j3o0Qs5pdO1id3/L1FRUIYYFDpMlXTexctw9wtSLklR\n\t40Ig==", "X-Gm-Message-State": "AOJu0YwP+6V2vy5Irp8StQfwabvTmnqB+L+WsBKlHqzm+dr6nyCQGLeO\n\tKqtmkrYEXxg3I2gjY1MCujL49AWY9vm07PIbO45dogUpkUQfqBgaX8B4zTwAF52DrPtcaU/bjQ2\n\tvT8An", "X-Gm-Gg": "AeBDieuXG9KtKxzYExQ4PWQlIp9kmVvlkU+S4Wc/oLkbxgNAkqu1sD8sVkO1x7Yi6vw\n\tLIMK9ncyYmjwnCqHnvYmyOLLt6jIi13291UeddFLxMskwdNYJA7z3kVFaEXrHy+dTCEfBM/4CAs\n\tob5FIyB6lNhSiPoIRXDOrhPRnwP6JV6HTA7SR6+FlvMHCFPddIpEPz7RSCmMxdMHiEkYveopKkj\n\ta/KBNPKtsMWYeaKPNZt26O9Yl/ZnWoNEjme9UiICM9tertukG4/3IV7VdN0R86XIJJBM12PFTtQ\n\tss8ATFbWsLFiyP8DGCvDVn+XXC60atQBQlozbZAzvqx6ivbTR3R4Xq/ZM3L/U09D80HHjJuAC8H\n\tJscViiRTRg0QzIsd2zB2wQdGZpBL78aySFPJECEk0zq7gDvtr7eRI3YV0OrXBsaZOuzVlaXEXav\n\tR5CY7aXYbgXQ8VU/fdUdSYF129JXamXEquYoQzza1SeXvs66KJ0GY++OwmfzBQ9Lrjb5yEDt4IM\n\tGI1vR6hqu1YxRkeN50i+C/1S5dzPf7aoszACQ==", "X-Received": "by 2002:a05:600c:198b:b0:48a:592c:e63d with SMTP id\n\t5b1f17b1804b1-48e676a5245mr41755705e9.14.1778245687886; \n\tFri, 08 May 2026 06:08:07 -0700 (PDT)", "From": "David Plowman <david.plowman@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "David Plowman <david.plowman@raspberrypi.com>,\n\tNaushir Patuck <naush@raspberrypi.com>", "Subject": "[PATCH v5 3/3] pipeline: rpi: Make control lists in requests\n\tproperly atomic", "Date": "Fri, 8 May 2026 13:57:43 +0100", "Message-ID": "<20260508130804.8238-4-david.plowman@raspberrypi.com>", "X-Mailer": "git-send-email 2.47.3", "In-Reply-To": "<20260508130804.8238-1-david.plowman@raspberrypi.com>", "References": "<20260508130804.8238-1-david.plowman@raspberrypi.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "When a request is about to be processed, this commit separates\n\"delayed controls\" (camera-related ones that take \"a few frames\" to\napply) from \"immediate controls\" (ISP-related controls) that will\nhappen instantly.\n\nThe immediate controls are held back until the delayed controls that\nthey were submitted with, have happened. We merge back together the\nimmediate controls that must happen now, along with the delayed\ncontrols that must be forwarded early, the result being that control\nlists submitted with a request happen atomically. We avoid overwriting\nthe original control list submitted with the request.\n\nWe therefore already have the sequence number of the request whose\ncontrols have just been applied, so we additionally attach this to the\ncurrent request as \"ControlId\" metadata.\n\nSigned-off-by: David Plowman <david.plowman@raspberrypi.com>\nReviewed-by: Naushir Patuck <naush@raspberrypi.com>\n---\n src/ipa/rpi/common/ipa_base.cpp | 24 ++++++---\n .../pipeline/rpi/common/pipeline_base.cpp | 49 +++++++++++++++++++\n .../pipeline/rpi/common/pipeline_base.h | 9 ++++\n src/libcamera/pipeline/rpi/pisp/pisp.cpp | 11 +++--\n src/libcamera/pipeline/rpi/vc4/vc4.cpp | 11 +++--\n 5 files changed, 88 insertions(+), 16 deletions(-)", "diff": "diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\nindex faa77719..dacafa57 100644\n--- a/src/ipa/rpi/common/ipa_base.cpp\n+++ b/src/ipa/rpi/common/ipa_base.cpp\n@@ -431,6 +431,15 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms)\n \trpiMetadata.clear();\n \tfillDeviceStatus(params.sensorControls, ipaContext);\n \n+\t/*\n+\t * When there are controls, it's important that we don't skip running the\n+\t * IPAs, as that can mess with synchronisation. Crucially though, we need\n+\t * to know whether there were controls when this comes back as the\n+\t * _delayed_ metadata, hence why we flag this in the metadata itself.\n+\t */\n+\tif (!params.requestControls.empty())\n+\t\trpiMetadata.set(\"ipa.request_controls\", true);\n+\n \tif (params.buffers.embedded) {\n \t\t/*\n \t\t * Pipeline handler has supplied us with an embedded data buffer,\n@@ -451,7 +460,7 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms)\n \t */\n \tAgcStatus agcStatus;\n \tbool hdrChange = false;\n-\tRPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext];\n+\tRPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()];\n \tif (!delayedMetadata.get<AgcStatus>(\"agc.status\", agcStatus)) {\n \t\trpiMetadata.set(\"agc.delayed_status\", agcStatus);\n \t\thdrChange = agcStatus.hdr.mode != hdrStatus_.mode;\n@@ -464,9 +473,13 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms)\n \t */\n \thelper_->prepare(embeddedBuffer, rpiMetadata);\n \n+\tbool delayedRequestControls = false;\n+\tdelayedMetadata.get<bool>(\"ipa.request_controls\", delayedRequestControls);\n+\n \t/* Allow a 10% margin on the comparison below. */\n \tDuration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;\n-\tif (lastRunTimestamp_ && frameCount_ > invalidCount_ &&\n+\tif (!delayedRequestControls && params.requestControls.empty() &&\n+\t lastRunTimestamp_ && frameCount_ > invalidCount_ &&\n \t delta < controllerMinFrameDuration_ * 0.9 && !hdrChange) {\n \t\t/*\n \t\t * Ensure we merge the previous frame's metadata with the current\n@@ -535,7 +548,7 @@ void IpaBase::processStats(const ProcessParams ¶ms)\n \t\tControlList ctrls(sensorCtrls_);\n \t\tapplyAGC(&agcStatus, ctrls);\n \t\trpiMetadata.set(\"agc.status\", agcStatus);\n-\t\tsetDelayedControls.emit(ctrls, ipaContext);\n+\t\tsetDelayedControls.emit(ctrls, params.ipaContext);\n \t\tsetCameraTimeoutValue();\n \t}\n \n@@ -951,8 +964,6 @@ void IpaBase::applyControls(const ControlList &controls)\n \n \t\t\t/* The control provides units of microseconds. */\n \t\t\tagc->setFixedExposureTime(0, ctrl.second.get<int32_t>() * 1.0us);\n-\n-\t\t\tlibcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());\n \t\t\tbreak;\n \t\t}\n \n@@ -976,9 +987,6 @@ void IpaBase::applyControls(const ControlList &controls)\n \t\t\t\tbreak;\n \n \t\t\tagc->setFixedGain(0, ctrl.second.get<float>());\n-\n-\t\t\tlibcameraMetadata_.set(controls::AnalogueGain,\n-\t\t\t\t\t ctrl.second.get<float>());\n \t\t\tbreak;\n \t\t}\n \ndiff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\nindex f6ef8a3b..ace38997 100644\n--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n@@ -1528,4 +1528,53 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request\n \t}\n }\n \n+static bool isControlDelayed(unsigned int id)\n+{\n+\treturn id == controls::ExposureTime ||\n+\t id == controls::AnalogueGain ||\n+\t id == controls::FrameDurationLimits ||\n+\t id == controls::AeEnable ||\n+\t id == controls::ExposureTimeMode ||\n+\t id == controls::AnalogueGainMode;\n+}\n+\n+void CameraData::handleControlLists(uint32_t delayContext, ControlList ¶mControls)\n+{\n+\t/*\n+\t * The delayContext is the sequence number after it's gone through the various\n+\t * pipeline delays, so that's what gets reported as the \"ControlListSequence\"\n+\t * in the metadata, being the sequence number of the request whose ControlList\n+\t * has just been applied.\n+\t */\n+\tRequest *request = requestQueue_.front();\n+\trequest->_d()->metadata().set(controls::rpi::ControlListSequence, delayContext);\n+\n+\t/*\n+\t * Controls that take effect immediately (typically ISP controls) have to be\n+\t * delayed so as to synchronise with those controls that do get delayed. So we\n+\t * must remove them from the current request, and push them onto a queue so\n+\t * that they can be used later.\n+\t *\n+\t * Note that we are given a separate control list (paramControls) so that\n+\t * we can pass back the controls that really need to happen now, without\n+\t * disturbing the controls that were submitted with the request.\n+\t */\n+\tASSERT(paramControls.empty());\n+\timmediateControls_.push({ request->sequence(), {} });\n+\tfor (const auto &ctrl : request->controls()) {\n+\t\tif (isControlDelayed(ctrl.first))\n+\t\t\tparamControls.set(ctrl.first, ctrl.second);\n+\t\telse\n+\t\t\timmediateControls_.back().controls.set(ctrl.first, ctrl.second);\n+\t}\n+\n+\t/* \"Immediate\" controls that have become due are now merged back into this request. */\n+\twhile (!immediateControls_.empty() &&\n+\t immediateControls_.front().controlListId <= delayContext) {\n+\t\tparamControls.merge(immediateControls_.front().controls,\n+\t\t\t\t ControlList::MergePolicy::OverwriteExisting);\n+\t\timmediateControls_.pop();\n+\t}\n+}\n+\n } /* namespace libcamera */\ndiff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\nindex 7bfac33e..758155ee 100644\n--- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n@@ -180,10 +180,19 @@ public:\n \n \tClockRecovery wallClockRecovery_;\n \n+\tstruct ImmediateControlsEntry {\n+\t\tuint64_t controlListId;\n+\t\tControlList controls;\n+\t};\n+\tstd::queue<ImmediateControlsEntry> immediateControls_;\n+\n protected:\n \tvoid fillRequestMetadata(const ControlList &bufferControls,\n \t\t\t\t Request *request);\n \n+\tvoid handleControlLists(uint32_t delayContext,\n+\t\t\t\tControlList ¶mControls);\n+\n \tvirtual void tryRunPipeline() = 0;\n \n private:\ndiff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\nindex cc7e32c3..b744c901 100644\n--- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n+++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n@@ -2322,9 +2322,6 @@ void PiSPCameraData::tryRunPipeline()\n \n \tfillRequestMetadata(job.sensorControls, request);\n \n-\t/* Set our state to say the pipeline is active. */\n-\tstate_ = State::Busy;\n-\n \tunsigned int bayerId = cfe_[Cfe::Output0].getBufferId(job.buffers[&cfe_[Cfe::Output0]]);\n \tunsigned int statsId = cfe_[Cfe::Stats].getBufferId(job.buffers[&cfe_[Cfe::Stats]]);\n \tASSERT(bayerId && statsId);\n@@ -2341,7 +2338,13 @@ void PiSPCameraData::tryRunPipeline()\n \tparams.ipaContext = requestQueue_.front()->sequence();\n \tparams.delayContext = job.delayContext;\n \tparams.sensorControls = std::move(job.sensorControls);\n-\tparams.requestControls = request->controls();\n+\t/* params.requestControls is set by handleControlLists. */\n+\n+\t/* This sorts out synchronisation with ControlLists in earlier requests. */\n+\thandleControlLists(job.delayContext, params.requestControls);\n+\n+\t/* Set our state to say the pipeline is active. */\n+\tstate_ = State::Busy;\n \n \tif (sensorMetadata_) {\n \t\tunsigned int embeddedId =\ndiff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\nindex f99cfdbc..3e9a4905 100644\n--- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n+++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n@@ -939,9 +939,6 @@ void Vc4CameraData::tryRunPipeline()\n \n \tfillRequestMetadata(bayerFrame.controls, request);\n \n-\t/* Set our state to say the pipeline is active. */\n-\tstate_ = State::Busy;\n-\n \tunsigned int bayer = unicam_[Unicam::Image].getBufferId(bayerFrame.buffer);\n \n \tLOG(RPI, Debug) << \"Signalling prepareIsp:\"\n@@ -950,10 +947,16 @@ void Vc4CameraData::tryRunPipeline()\n \tipa::RPi::PrepareParams params;\n \tparams.buffers.bayer = RPi::MaskBayerData | bayer;\n \tparams.sensorControls = std::move(bayerFrame.controls);\n-\tparams.requestControls = request->controls();\n \tparams.ipaContext = request->sequence();\n \tparams.delayContext = bayerFrame.delayContext;\n \tparams.buffers.embedded = 0;\n+\t/* params.requestControls is set by handleControlLists. */\n+\n+\t/* This sorts out synchronisation with ControlLists in earlier requests. */\n+\thandleControlLists(bayerFrame.delayContext, params.requestControls);\n+\n+\t/* Set our state to say the pipeline is active. */\n+\tstate_ = State::Busy;\n \n \tif (embeddedBuffer) {\n \t\tunsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer);\n", "prefixes": [ "v5", "3/3" ] }