Patch Detail
Show a patch.
GET /api/1.1/patches/10926/?format=api
{ "id": 10926, "url": "https://patchwork.libcamera.org/api/1.1/patches/10926/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10926/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20210121115849.682130-5-naush@raspberrypi.com>", "date": "2021-01-21T11:58:49", "name": "[libcamera-devel,4/4] pipeline: raspberrypi: Add notion of immediate write to StaggeredCtrl", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "e9cf333e8d3091cd14b872a258ee6ba2e704e287", "submitter": { "id": 34, "url": "https://patchwork.libcamera.org/api/1.1/people/34/?format=api", "name": "Naushir Patuck", "email": "naush@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/10926/mbox/", "series": [ { "id": 1591, "url": "https://patchwork.libcamera.org/api/1.1/series/1591/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1591", "date": "2021-01-21T11:58:45", "name": "Raspberrypi: FrameDurations control refinements", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1591/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10926/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10926/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 9F29BC0F2B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 21 Jan 2021 11:59:00 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 665AA681EE;\n\tThu, 21 Jan 2021 12:59:00 +0100 (CET)", "from mail-wr1-x435.google.com (mail-wr1-x435.google.com\n\t[IPv6:2a00:1450:4864:20::435])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 64B70681F3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Jan 2021 12:58:58 +0100 (CET)", "by mail-wr1-x435.google.com with SMTP id a9so1452059wrt.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Jan 2021 03:58:58 -0800 (PST)", "from naushir-VirtualBox.patuck.local ([88.97.76.4])\n\tby smtp.gmail.com with ESMTPSA id\n\th13sm8044930wrm.28.2021.01.21.03.58.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 21 Jan 2021 03:58:56 -0800 (PST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"bb8/j4pH\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=wbMF+KvzLd6qSkStJhzEh/KUO8Z01x1gWbfkhwWRLzo=;\n\tb=bb8/j4pHC+simkj0oyESZxsCbuwNixKXgnt9unOWxq9sj53DvV2DZ4zfM6MVuflXek\n\tNZaag/rZ9IAQfwRBBOT2UKJ3JJRIbooKCZiwsm322rbPoaUJWhy4o2Psp7k1YShOzoiF\n\tC4TqvQctkwayEme41NrMTdubhgjpqf2nomaWY/jQzCAMrnzuhrbQEU71DJOEHnOzEelP\n\tO+blKOIafqCxSGxTBvgGBsriWl1FjRJm0qR/l+Wp0AVwQsMSzDbfS0zxiEo3CERlksjD\n\tFCFex0qWbfdpTIgN9sGOmaIbfKIBSyB7kXWpDE1uIlB/GPjEiksIPPbWvry8ZWphSr4s\n\tUk3A==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=wbMF+KvzLd6qSkStJhzEh/KUO8Z01x1gWbfkhwWRLzo=;\n\tb=FHLicJzG9IJQEkdgoU7s2c5FXpDPp3TNknpNM+Io+lpbpZ/93VC8mEQn9hsSEqnz0M\n\t2/HVaIJ4eR52Kq1/dRF6M0kzVLz8P5Vnao2eP6VIrydxNnz8K3Woi8Bn7sUcZ9tSPEvD\n\tcv6nN0EaYEjlKlf991//Bfwo5eE0He+U2yaOZ7iVwzyxbZVFAKbnLo1xnD7G3XR/7tlp\n\tFXj8BkuY2L4k7O2xGFep6DbrTVrrIGHrOfjDNfo91oUXBtlBPjRq3p2acsjqLYrSQImq\n\t2adcgJddmbypCqfW5LvQI4RCr/bZzCoc09hUzxPccesSQNQmCcxM0RBdYx58WKGaBGO7\n\t8OXQ==", "X-Gm-Message-State": "AOAM531i2ioV/bZ5dZu7Z+L8DnQ3LDKeeFp7g3afhp9bLT7izRDbL4Xa\n\tRO5miBFkn4sxHrncTxlHThNpgzK4xgGong==", "X-Google-Smtp-Source": "ABdhPJxeFHal94kBYe1In4wHhNIxCJlINQhRM+l2fqKfY20hNDPuoxB4Qj8j9QzMmQ2zliixMplXFA==", "X-Received": "by 2002:a05:6000:1043:: with SMTP id\n\tc3mr13139844wrx.140.1611230337764; \n\tThu, 21 Jan 2021 03:58:57 -0800 (PST)", "From": "Naushir Patuck <naush@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 21 Jan 2021 11:58:49 +0000", "Message-Id": "<20210121115849.682130-5-naush@raspberrypi.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "<20210121115849.682130-1-naush@raspberrypi.com>", "References": "<20210121115849.682130-1-naush@raspberrypi.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH 4/4] pipeline: raspberrypi: Add notion of\n\timmediate write to StaggeredCtrl", "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>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "If an exposure time change adjusts the vblanking limits, and we write\nboth VBLANK and EXPOSURE controls as a set, the latter may fail if the\nvalue is outside of the limits calculated by the old VBLANK value. This\nis a limitation in V4L2 and cannot be fixed by writing VBLANK before\nEXPOSURE.\n\nThe workaround here is to have the StaggeredCtrl mark the\nVBLANK control as \"immediate write\", which then write VBLANK separately\nfrom (and ahead of) any other controls. This way, the sensor driver will\nupdate the EXPOSURE control with new limits before the new values is\npresented, and will thus be seen as valid.\n\nStaggeredCtrl is due to be deprecated and replaced by DelayedCtrl, so\nthis change serves more a working proof-of-concept on the workaround,\nand not much care has been taken to provide a nice new API for applying\nthis immediate write flag to the control. A similar workaround must be\navailable to DelayedCtrl eventually.\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\n---\n src/ipa/raspberrypi/raspberrypi.cpp | 5 ++-\n .../pipeline/raspberrypi/raspberrypi.cpp | 11 ++++--\n .../pipeline/raspberrypi/staggered_ctrl.cpp | 39 ++++++++++++++-----\n .../pipeline/raspberrypi/staggered_ctrl.h | 3 +-\n 4 files changed, 43 insertions(+), 15 deletions(-)", "diff": "diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 2b71efc63f10..c6a1e8027284 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -1040,8 +1040,9 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n \n \t/*\n \t * Due to the behavior of V4L2, the current value of VBLANK could clip the\n-\t * exposure time without us knowing. The next time though this function should\n-\t * clip exposure correctly.\n+\t * exposure time without us knowing. We get around this by ensuring the\n+\t * staggered write component submits VBLANK separately from, and before the\n+\t * EXPOSURE control.\n \t */\n \tctrls.set(V4L2_CID_VBLANK, vblanking);\n \tctrls.set(V4L2_CID_EXPOSURE, exposureLines);\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 524cc960dd37..1485999ad2a0 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -1229,12 +1229,17 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n \t\t/*\n \t\t * Setup our staggered control writer with the sensor default\n \t\t * gain and exposure delays.\n+\t\t *\n+\t\t * VBLANK must be flagged as \"immediate write\" to allow it to\n+\t\t * be set immediately instead of being batched with all other\n+\t\t * controls. This is needed so that any update to the EXPOSURE\n+\t\t * control will be validated based on the new VBLANK control value.\n \t\t */\n \t\tif (!staggeredCtrl_) {\n \t\t\tstaggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n-\t\t\t\t\t { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },\n-\t\t\t\t\t { V4L2_CID_EXPOSURE, result.data[resultIdx++] },\n-\t\t\t\t\t { V4L2_CID_VBLANK, result.data[resultIdx++] } });\n+\t\t\t\t\t { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++], false },\n+\t\t\t\t\t { V4L2_CID_EXPOSURE, result.data[resultIdx++], false },\n+\t\t\t\t\t { V4L2_CID_VBLANK, result.data[resultIdx++], true } });\n \t\t\tsensorMetadata_ = result.data[resultIdx++];\n \t\t}\n \t}\ndiff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp\nindex 62605c0fceee..07f8c95d4f2c 100644\n--- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp\n@@ -22,21 +22,29 @@ LOG_DEFINE_CATEGORY(RPI_S_W)\n namespace RPi {\n \n void StaggeredCtrl::init(V4L2VideoDevice *dev,\n-\t std::initializer_list<std::pair<const uint32_t, uint8_t>> delayList)\n+\t\t\t std::initializer_list<std::tuple<uint32_t, uint8_t, bool>> delayList)\n {\n \tstd::lock_guard<std::mutex> lock(lock_);\n \n \tdev_ = dev;\n-\tdelay_ = delayList;\n+\tdelay_.clear();\n \tctrl_.clear();\n \n-\t/* Find the largest delay across all controls. */\n \tmaxDelay_ = 0;\n-\tfor (auto const &p : delay_) {\n+\tfor (auto const &c : delayList) {\n+\t\tuint32_t id = std::get<0>(c);\n+\t\tuint8_t delay = std::get<1>(c);\n+\t\tbool immediateWrite = std::get<2>(c);\n+\n+\t\tdelay_[id] = delay;\n+\t\timmediateWrite_[id] = immediateWrite;\n+\n \t\tLOG(RPI_S_W, Info) << \"Init ctrl \"\n-\t\t\t\t << utils::hex(p.first) << \" with delay \"\n-\t\t\t\t << static_cast<int>(p.second);\n-\t\tmaxDelay_ = std::max(maxDelay_, p.second);\n+\t\t\t\t << utils::hex(id) << \" with delay \"\n+\t\t\t\t << static_cast<int>(delay);\n+\n+\t\t/* Find the largest delay across all controls. */\n+\t\tmaxDelay_ = std::max(maxDelay_, delay);\n \t}\n \n \tinit_ = true;\n@@ -121,8 +129,21 @@ int StaggeredCtrl::write()\n \t\tint index = std::max<int>(0, setCount_ - delayDiff);\n \n \t\tif (p.second[index].updated) {\n-\t\t\t/* We need to write this value out. */\n-\t\t\tcontrols.set(p.first, p.second[index].value);\n+\t\t\tif (immediateWrite_[p.first]) {\n+\t\t\t\t/*\n+\t\t\t\t * This control must be written now, it could\n+\t\t\t\t * affect validity of the other controls.\n+\t\t\t\t */\n+\t\t\t\tControlList immediate(dev_->controls());\n+\t\t\t\timmediate.set(p.first, p.second[index].value);\n+\t\t\t\tdev_->setControls(&immediate);\n+\t\t\t} else {\n+\t\t\t\t/*\n+\t\t\t\t * Batch up the list of controls and write them\n+\t\t\t\t * at the end of the function.\n+\t\t\t\t */\n+\t\t\t\tcontrols.set(p.first, p.second[index].value);\n+\t\t\t}\n \t\t\tp.second[index].updated = false;\n \t\t\tLOG(RPI_S_W, Debug) << \"Writing ctrl \"\n \t\t\t\t\t << utils::hex(p.first) << \" to \"\ndiff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h\nindex 382fa31a6853..7c920c3a13c7 100644\n--- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h\n+++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h\n@@ -34,7 +34,7 @@ public:\n \t}\n \n \tvoid init(V4L2VideoDevice *dev,\n-\t\t std::initializer_list<std::pair<const uint32_t, uint8_t>> delayList);\n+\t\t std::initializer_list<std::tuple<uint32_t, uint8_t, bool>> delayList);\n \tvoid reset();\n \n \tvoid get(std::unordered_map<uint32_t, int32_t> &ctrl, uint8_t offset = 0);\n@@ -85,6 +85,7 @@ private:\n \tuint8_t maxDelay_;\n \tV4L2VideoDevice *dev_;\n \tstd::unordered_map<uint32_t, uint8_t> delay_;\n+\tstd::unordered_map<uint32_t, bool> immediateWrite_;\n \tstd::unordered_map<uint32_t, CircularArray> ctrl_;\n \tstd::mutex lock_;\n };\n", "prefixes": [ "libcamera-devel", "4/4" ] }