From patchwork Sun Nov 2 19:19:52 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 24947 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 044D7BDE4C for ; Sun, 2 Nov 2025 19:20:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E4E8F606E6; Sun, 2 Nov 2025 20:20:03 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="rUEt2Gcj"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A318606E6 for ; Sun, 2 Nov 2025 20:20:02 +0100 (CET) Received: from [100.93.44.16] (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9FE928D4; Sun, 2 Nov 2025 20:18:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1762111089; bh=M21vn7FHVpALK/Varxs4WBCQvWvgLRxqZat6h2i57YQ=; h=From:Date:Subject:To:Cc:From; b=rUEt2GcjG4DmHMC1apCaKANGCAZqpX40QMViPHsxSufGGuyB6dZSgDVIfaRnR18N3 d8fgQRoprplvAR1AE1njtGL5MuOu0FM2TGDii5FjnHa2t2khUT633I9v5Ssvtw2xp9 C+CRrtGzmX96s9tUM4BttVZThllPEtKve08pMDf4= From: Jacopo Mondi Date: Sun, 02 Nov 2025 20:19:52 +0100 Subject: [PATCH] libcamera: delayed_controls: Make VBLANK priority by default MIME-Version: 1.0 Message-Id: <20251102-rkisp1-vblank-v1-1-d2d6458696aa@ideasonboard.com> X-B4-Tracking: v=1; b=H4sIANeuB2kC/x3MQQqAIBBA0avIrBNUzKKrRAutqQbDQkEC8e5Jy 7f4v0DCSJhgYgUiZkp0hwbZMVhPGw7ktDWDEqqXUigePaVH8uwuGzx3VolRG22cHaA1T8Sd3v8 3L7V+WKuUoV8AAAA= X-Change-ID: 20251102-rkisp1-vblank-ba2084646ba7 To: libcamera-devel@lists.libcamera.org Cc: Jacopo Mondi X-Mailer: b4 0.14.2 X-Developer-Signature: v=1; a=openpgp-sha256; l=12042; i=jacopo.mondi@ideasonboard.com; h=from:subject:message-id; bh=M21vn7FHVpALK/Varxs4WBCQvWvgLRxqZat6h2i57YQ=; b=owEBbQKS/ZANAwAKAXI0Bo8WoVY8AcsmYgBpB67hXV9cNYGIOuYUaLAE6L27P62psKDKJ3CUF JUKaO4udAeJAjMEAAEKAB0WIQS1xD1IgJogio9YOMByNAaPFqFWPAUCaQeu4QAKCRByNAaPFqFW PNqSEACN/6qA6h0tmJcLd81EdT3yxB3xxALLTF6BZh0nDzypPMJqwIgSacgM1MwZGawPaXPvbp1 hkliBqTKgVKg5MD8KoBEKrBuKYYYM3vo7FuAnqtIHLmzfq3NNDSqYqVu6wwiOhdAzwACEULa8yP snjmlwZDSmezHyjFYPsjQLEvmHM6UlIaEspOwOGvfPPF+Vq+1YMIl/Suvt/zRdbn15VxZ4ClecM cqobFuH963I28+LcBRG9FBVgDU5kG8CiUIIvJHKN2hYCh8NJMvjTSHk8CBl5Xa3icKWUkO3WLwb HecF5EXNBaCGcoUUdQj6sG3JcQtz+09bYnnd40XEsGNSz4mxXWEGLwjaMdzOrmjM5lmDL3YTaSY XBsZmgDU6ECrSNRVIIyTa1DfeLxQwkj/TLIjmGePlUW27EjDYZR4UPH1tD5clpmzJ4H40Rjtg4S VsUBjxGQV2vRBOJWsGJx8oLuZlc2E8nP0kLIPM4Qs1Hxzi+1q9WmeHATwYBpfIUJF+nDm+SgkSy 0y5XB82rV3DWmVIvgLQqAZDZsrAPYlOMeeVQTNhPpj3Jt8mpvqMhgBSrS84PKo91W//Fe+wlb8K jTKNtFN38yjclWVMfvzdhExqemUKM+kNr7RgBH4yGjPXgd0tgzSLV8qY9JZiB6Es445zpNVfCDd Fb/M7OQAujQVYCw== X-Developer-Key: i=jacopo.mondi@ideasonboard.com; a=openpgp; fpr=72392EDC88144A65C701EA9BA5826A2587AD026B 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" The DelayedControls class works around a limitation of the V4L2 controls API by assigning to controls that modify the limits of other controls a 'priority' flag. 'Priority' controls are written to the device before others to make sure the limits of dependent controls are correctly updated. A typical example of a priority control is VBLANK, whose value changes the limits of the EXPOSURE control. This doesn't apply to a specific hardware platform but to all V4L2 sensors. It's currently up to users of delayed controls to apply to each control the correct flag by populating a ControlParams instance. However, this is error prone, and the RkISP1 pipeline handler wrongly sets the VBLANK priority to false. Instead of simply fixing the RkISP1 implementation, change how the priorty flag is set, by making DelayedControls automatically flag VBLANK as priority. This limits what controls the pipelines can set as priority, but so far only VBLANK needs that flag to be set. Split the ControlParams type into an external one that only associates delays to control ids and an internal one that is populated at class creation time. Signed-off-by: Jacopo Mondi --- Tested on rkisp1: delayed_controls.cpp:100 Set a delay of 2 and priority write flag 1 for Vertical Blanking --- include/libcamera/internal/delayed_controls.h | 14 ++++++--- src/libcamera/delayed_controls.cpp | 41 +++++++++++++-------------- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++-- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 6 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 +++---- src/libcamera/pipeline/simple/simple.cpp | 6 ++-- test/delayed_controls.cpp | 20 ++++++------- 7 files changed, 53 insertions(+), 50 deletions(-) --- base-commit: b1f09c013a01a82c739f0e30b71fd8d000ef5655 change-id: 20251102-rkisp1-vblank-ba2084646ba7 Best regards, diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index b64d8bba7cf73e10491d13d64bb4b3f7db80681b..c966fbedb1b832bcb50d0931204a011a334e6514 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -21,13 +21,14 @@ class V4L2Device; class DelayedControls : public Object { public: - struct ControlParams { + struct Controls { + uint32_t id; unsigned int delay; - bool priorityWrite; }; - DelayedControls(V4L2Device *device, - const std::unordered_map &controlParams); + using Params = std::vector; + + DelayedControls(V4L2Device *device, const Params &controlParams); void reset(); @@ -37,6 +38,11 @@ public: void applyControls(uint32_t sequence); private: + struct ControlParams { + unsigned int delay; + bool priorityWrite; + }; + class Info : public ControlValue { public: diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 94d0a575b01b72416bdf05ea595f1c8e0ab6082c..f3092481ed804077dedd2cce0cda72f22d3563f8 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -39,40 +39,35 @@ LOG_DEFINE_CATEGORY(DelayedControls) */ /** - * \struct DelayedControls::ControlParams - * \brief Parameters associated with controls handled by the \a DelayedControls + * \struct DelayedControls::Controls + * \brief Delays associated with controls handled by the \a DelayedControls * helper class * - * \var ControlParams::delay + * \var Controls::id + * \brief The control id + * + * \var Controls::delay * \brief Frame delay from setting the control on a sensor device to when it is * consumed during framing. - * - * \var ControlParams::priorityWrite - * \brief Flag to indicate that this control must be applied ahead of, and - * separately from the other controls. - * - * Typically set for the \a V4L2_CID_VBLANK control so that the device driver - * does not reject \a V4L2_CID_EXPOSURE control values that may be outside of - * the existing vertical blanking specified bounds, but are within the new - * blanking bounds. + */ + +/** + * \typedef DelayedControls::Params + * \brief Vector of DelayedControls::Controls */ /** * \brief Construct a DelayedControls instance * \param[in] device The V4L2 device the controls have to be applied to - * \param[in] controlParams Map of the numerical V4L2 control ids to their - * associated control parameters. + * \param[in] controlParams Control ids and delays * - * The control parameters comprise of delays (in frames) and a priority write - * flag. If this flag is set, the relevant control is written separately from, - * and ahead of the rest of the batched controls. + * The control parameters associate a delay (in frames) to control ids. * * Only controls specified in \a controlParams are handled. If it's desired to * mix delayed controls and controls that take effect immediately the immediate * controls must be listed in the \a controlParams map with a delay value of 0. */ -DelayedControls::DelayedControls(V4L2Device *device, - const std::unordered_map &controlParams) +DelayedControls::DelayedControls(V4L2Device *device, const Params &controlParams) : device_(device), maxDelay_(0) { const ControlInfoMap &controls = device_->controls(); @@ -82,11 +77,11 @@ DelayedControls::DelayedControls(V4L2Device *device, * device. */ for (auto const ¶m : controlParams) { - auto it = controls.find(param.first); + auto it = controls.find(param.id); if (it == controls.end()) { LOG(DelayedControls, Error) << "Delay request for control id " - << utils::hex(param.first) + << utils::hex(param.id) << " but control is not exposed by device " << device_->deviceNode(); continue; @@ -94,7 +89,9 @@ DelayedControls::DelayedControls(V4L2Device *device, const ControlId *id = it->first; - controlParams_[id] = param.second; + controlParams_[id].delay = param.delay; + if (param.id == V4L2_CID_VBLANK) + controlParams_[id].priorityWrite = true; LOG(DelayedControls, Debug) << "Set a delay of " << controlParams_[id].delay diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index d6b7edcb5a7f9fcb4083d4105a193978e265ef67..0d6d2e30065e03defbc47ae45fd67a22f6bff25a 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1083,9 +1083,9 @@ int PipelineHandlerIPU3::registerCameras() continue; const CameraSensorProperties::SensorDelays &delays = cio2->sensor()->sensorDelays(); - std::unordered_map params = { - { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } }, - { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } }, + DelayedControls::Params params = { + { V4L2_CID_ANALOGUE_GAIN, delays.gainDelay }, + { V4L2_CID_EXPOSURE, delays.exposureDelay }, }; data->delayedCtrls_ = diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 38bdc6138ed167a2c05f43ca30e60ed549fd953c..85f87ab979b23117c47bda4ca9418e4cb50c878d 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1607,9 +1607,9 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink) data->properties_ = data->sensor_->properties(); const CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays(); - std::unordered_map params = { - { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } }, - { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } }, + DelayedControls::Params params = { + { V4L2_CID_ANALOGUE_GAIN, delays.gainDelay }, + { V4L2_CID_EXPOSURE, delays.exposureDelay }, }; data->delayedCtrls_ = diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index ecd13831539fdf5cb79da2ea4b33a353514328ae..525c8be6f19282e96df47ead602b67f6a74a8f96 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1345,12 +1345,12 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) scalerMaxCrop_ = Rectangle(data->sensor_->resolution()); const CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays(); - std::unordered_map params = { - { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } }, - { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } }, - { V4L2_CID_VBLANK, { delays.vblankDelay, false } }, - }; + DelayedControls::Params params = { + { V4L2_CID_ANALOGUE_GAIN, delays.gainDelay }, + { V4L2_CID_EXPOSURE, delays.exposureDelay }, + { V4L2_CID_VBLANK, delays.vblankDelay }, + }; data->delayedCtrls_ = std::make_unique(data->sensor_->device(), params); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 91715b7f8afd2058c42856ef38144847f81bd7b0..617ffa8eef490a1301920b3f30ab1f13b3ff62ff 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -553,9 +553,9 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe, return; const CameraSensorProperties::SensorDelays &delays = sensor_->sensorDelays(); - std::unordered_map params = { - { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } }, - { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } }, + DelayedControls::Params params = { + { V4L2_CID_ANALOGUE_GAIN, delays.gainDelay }, + { V4L2_CID_EXPOSURE, delays.exposureDelay }, }; delayedCtrls_ = std::make_unique(sensor_->device(), params); diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp index 7bd30e7aead87f9350f1e0b48203f32804cfeef0..88425abc829a83ab7212775a8cab2165692a0a52 100644 --- a/test/delayed_controls.cpp +++ b/test/delayed_controls.cpp @@ -72,8 +72,8 @@ protected: int singleControlNoDelay() { - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, { 0, false } }, + DelayedControls::Params delays = { + { V4L2_CID_BRIGHTNESS, 0 }, }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays); @@ -113,8 +113,8 @@ protected: int singleControlWithDelay() { - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, { 1, false } }, + DelayedControls::Params delays = { + { V4L2_CID_BRIGHTNESS, 1 }, }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays); @@ -159,9 +159,9 @@ protected: { static const unsigned int maxDelay = 2; - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, { 1, false } }, - { V4L2_CID_CONTRAST, { maxDelay, false } }, + DelayedControls::Params delays = { + { V4L2_CID_BRIGHTNESS, 1 }, + { V4L2_CID_CONTRAST, maxDelay }, }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays); @@ -210,9 +210,9 @@ protected: { static const unsigned int maxDelay = 2; - std::unordered_map delays = { - { V4L2_CID_BRIGHTNESS, { 1, false } }, - { V4L2_CID_CONTRAST, { maxDelay, false } } + DelayedControls::Params delays = { + { V4L2_CID_BRIGHTNESS, 1 }, + { V4L2_CID_CONTRAST, maxDelay } }; std::unique_ptr delayed = std::make_unique(dev_.get(), delays);