[{"id":14360,"web_url":"https://patchwork.libcamera.org/comment/14360/","msgid":"","date":"2020-12-27T14:55:40","subject":"Re: [libcamera-devel] [PATCH v6 10/11] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your patch.\n\nOn 2020-12-24 17:16:12 +0900, Paul Elder wrote:\n> Add support to vimc pipeline handler and IPA for the new IPC mechanism.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> \n> ---\n> Changes in v6:\n> - move the enum and const from the header to the mojom file\n> - remove the per-pipeline ControlInfoMap\n> - since vimc.h has nothing in it anymore, remove it\n> - use the namespace in the pipeline handler and IPA\n> \n> Changes in v5:\n> - use the new generated header naming scheme\n> - add dummy event, since event interfaces are now required to have at\n> least one event\n> \n> Changes in v4:\n> - rename Controls to controls\n> - rename IPAVimcCallbackInterface to IPAVimcEventInterface\n> - rename libcamera_generated_headers to libcamera_generated_ipa_headers\n> in meson\n> - use the new header name, vimc_ipa_interface.h\n> \n> Changes in v3:\n> - change namespacing of base ControlInfoMap structure\n> \n> New in v2\n> ---\n> include/libcamera/ipa/meson.build | 1 +\n> include/libcamera/ipa/vimc.h | 28 ---------------------\n> include/libcamera/ipa/vimc.mojom | 24 ++++++++++++++++++\n> src/ipa/vimc/meson.build | 2 +-\n> src/ipa/vimc/vimc.cpp | 37 ++++++++++------------------\n> src/libcamera/pipeline/vimc/vimc.cpp | 10 +++++---\n> 6 files changed, 46 insertions(+), 56 deletions(-)\n> delete mode 100644 include/libcamera/ipa/vimc.h\n> create mode 100644 include/libcamera/ipa/vimc.mojom\n> \n> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> index 785c1241..67e3f049 100644\n> --- a/include/libcamera/ipa/meson.build\n> +++ b/include/libcamera/ipa/meson.build\n> @@ -56,6 +56,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',\n> \n> ipa_mojom_files = [\n> 'raspberrypi.mojom',\n> + 'vimc.mojom',\n> ]\n> \n> ipa_mojoms = []\n> diff --git a/include/libcamera/ipa/vimc.h b/include/libcamera/ipa/vimc.h\n> deleted file mode 100644\n> index 27a4a61d..00000000\n> --- a/include/libcamera/ipa/vimc.h\n> +++ /dev/null\n> @@ -1,28 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * vimc.h - Vimc Image Processing Algorithm module\n> - */\n> -\n> -#ifndef __LIBCAMERA_IPA_VIMC_H__\n> -#define __LIBCAMERA_IPA_VIMC_H__\n> -\n> -#ifndef __DOXYGEN__\n> -\n> -namespace libcamera {\n> -\n> -#define VIMC_IPA_FIFO_PATH \"/tmp/libcamera_ipa_vimc_fifo\"\n> -\n> -enum IPAOperationCode {\n> -\tIPAOperationNone,\n> -\tIPAOperationInit,\n> -\tIPAOperationStart,\n> -\tIPAOperationStop,\n> -};\n> -\n> -} /* namespace libcamera */\n> -\n> -#endif /* __DOXYGEN__ */\n> -\n> -#endif /* __LIBCAMERA_IPA_VIMC_H__ */\n> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> new file mode 100644\n> index 00000000..165d9401\n> --- /dev/null\n> +++ b/include/libcamera/ipa/vimc.mojom\n> @@ -0,0 +1,24 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +\n> +module ipa.vimc;\n> +\n> +import \"include/libcamera/ipa/core.mojom\";\n> +\n> +const string VimcIPAFIFOPath = \"/tmp/libcamera_ipa_vimc_fifo\";\n> +\n> +enum IPAOperationCode {\n> +\tIPAOperationNone,\n> +\tIPAOperationInit,\n> +\tIPAOperationStart,\n> +\tIPAOperationStop,\n> +};\n> +\n> +interface IPAVimcInterface {\n> +\tinit(IPASettings settings) => (int32 ret);\n> +\tstart() => (int32 ret);\n> +\tstop();\n> +};\n> +\n> +interface IPAVimcEventInterface {\n> +\tdummyEvent(uint32 val);\n> +};\n> diff --git a/src/ipa/vimc/meson.build b/src/ipa/vimc/meson.build\n> index 8c9df854..e982ebba 100644\n> --- a/src/ipa/vimc/meson.build\n> +++ b/src/ipa/vimc/meson.build\n> @@ -3,7 +3,7 @@\n> ipa_name = 'ipa_vimc'\n> \n> mod = shared_module(ipa_name,\n> - 'vimc.cpp',\n> + ['vimc.cpp', libcamera_generated_ipa_headers],\n> name_prefix : '',\n> include_directories : [ipa_includes, libipa_includes],\n> dependencies : libcamera_dep,\n> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> index 436a5559..13681d88 100644\n> --- a/src/ipa/vimc/vimc.cpp\n> +++ b/src/ipa/vimc/vimc.cpp\n> @@ -5,7 +5,7 @@\n> * ipa_vimc.cpp - Vimc Image Processing Algorithm module\n> */\n> \n> -#include \n> +#include \n> \n> #include \n> #include \n> @@ -24,7 +24,7 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPAVimc)\n> \n> -class IPAVimc : public IPAInterface\n> +class IPAVimc : public ipa::vimc::IPAVimcInterface\n> {\n> public:\n> \tIPAVimc();\n> @@ -32,22 +32,12 @@ public:\n> \n> \tint init(const IPASettings &settings) override;\n> \n> -\tint start(const IPAOperationData &data,\n> -\t\t IPAOperationData *result) override;\n> +\tint start() override;\n> \tvoid stop() override;\n> \n> -\tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n> -\t\t [[maybe_unused]] const std::map &streamConfig,\n> -\t\t [[maybe_unused]] const std::map &entityControls,\n> -\t\t [[maybe_unused]] const IPAOperationData &ipaConfig,\n> -\t\t [[maybe_unused]] IPAOperationData *result) override {}\n> -\tvoid mapBuffers([[maybe_unused]] const std::vector &buffers) override {}\n> -\tvoid unmapBuffers([[maybe_unused]] const std::vector &ids) override {}\n> -\tvoid processEvent([[maybe_unused]] const IPAOperationData &event) override {}\n> -\n> private:\n> \tvoid initTrace();\n> -\tvoid trace(enum IPAOperationCode operation);\n> +\tvoid trace(enum ipa::vimc::IPAOperationCode operation);\n> \n> \tint fd_;\n> };\n> @@ -66,7 +56,7 @@ IPAVimc::~IPAVimc()\n> \n> int IPAVimc::init(const IPASettings &settings)\n> {\n> -\ttrace(IPAOperationInit);\n> +\ttrace(ipa::vimc::IPAOperationInit);\n> \n> \tLOG(IPAVimc, Debug)\n> \t\t<< \"initializing vimc IPA with configuration file \"\n> @@ -81,10 +71,9 @@ int IPAVimc::init(const IPASettings &settings)\n> \treturn 0;\n> }\n> \n> -int IPAVimc::start([[maybe_unused]] const IPAOperationData &data,\n> -\t\t [[maybe_unused]] IPAOperationData *result)\n> +int IPAVimc::start()\n> {\n> -\ttrace(IPAOperationStart);\n> +\ttrace(ipa::vimc::IPAOperationStart);\n> \n> \tLOG(IPAVimc, Debug) << \"start vimc IPA!\";\n> \n> @@ -93,7 +82,7 @@ int IPAVimc::start([[maybe_unused]] const IPAOperationData &data,\n> \n> void IPAVimc::stop()\n> {\n> -\ttrace(IPAOperationStop);\n> +\ttrace(ipa::vimc::IPAOperationStop);\n> \n> \tLOG(IPAVimc, Debug) << \"stop vimc IPA!\";\n> }\n> @@ -101,11 +90,11 @@ void IPAVimc::stop()\n> void IPAVimc::initTrace()\n> {\n> \tstruct stat fifoStat;\n> -\tint ret = stat(VIMC_IPA_FIFO_PATH, &fifoStat);\n> +\tint ret = stat(ipa::vimc::VimcIPAFIFOPath.c_str(), &fifoStat);\n> \tif (ret)\n> \t\treturn;\n> \n> -\tret = ::open(VIMC_IPA_FIFO_PATH, O_WRONLY);\n> +\tret = ::open(ipa::vimc::VimcIPAFIFOPath.c_str(), O_WRONLY);\n> \tif (ret < 0) {\n> \t\tret = errno;\n> \t\tLOG(IPAVimc, Error) << \"Failed to open vimc IPA test FIFO: \"\n> @@ -116,7 +105,7 @@ void IPAVimc::initTrace()\n> \tfd_ = ret;\n> }\n> \n> -void IPAVimc::trace(enum IPAOperationCode operation)\n> +void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)\n> {\n> \tif (fd_ < 0)\n> \t\treturn;\n> @@ -141,9 +130,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \t\"vimc\",\n> };\n> \n> -struct ipa_context *ipaCreate()\n> +IPAInterface *ipaCreate()\n> {\n> -\treturn new IPAInterfaceWrapper(std::make_unique());\n> +\treturn new IPAVimc();\n> }\n> }\n> \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 8bda746f..d258090e 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -34,6 +34,9 @@\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> +#include \n> +#include \n> +\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(VIMC)\n> @@ -56,6 +59,8 @@ public:\n> \tstd::unique_ptr video_;\n> \tstd::unique_ptr raw_;\n> \tStream stream_;\n> +\n> +\tstd::unique_ptr ipa_;\n> };\n> \n> class VimcCameraConfiguration : public CameraConfiguration\n> @@ -311,8 +316,7 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tIPAOperationData ipaData = {};\n> -\tret = data->ipa_->start(ipaData, nullptr);\n> +\tret = data->ipa_->start();\n> \tif (ret) {\n> \t\tdata->video_->releaseBuffers();\n> \t\treturn ret;\n> @@ -418,7 +422,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \n> \tstd::unique_ptr data = std::make_unique(this, media);\n> \n> -\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> +\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> \tif (data->ipa_ != nullptr) {\n> \t\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> \t\tdata->ipa_->init(IPASettings{ conf });\n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","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 D1CA2C0F1A\n\tfor ;\n\tSun, 27 Dec 2020 14:55:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 27474615D5;\n\tSun, 27 Dec 2020 15:55:43 +0100 (CET)","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n\t[IPv6:2a00:1450:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 680EA60526\n\tfor ;\n\tSun, 27 Dec 2020 15:55:42 +0100 (CET)","by mail-lf1-x12a.google.com with SMTP id y19so18656488lfa.13\n\tfor ;\n\tSun, 27 Dec 2020 06:55:42 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\td24sm5096497lfl.30.2020.12.27.06.55.40\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 27 Dec 2020 06:55:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"sDYCqfoQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=jBXLyHjGFdWs/AbmZgla+sRVwmHI7fFnEBu6jps9kO8=;\n\tb=sDYCqfoQaY5wgbG6hT81qF0/p3fikLQvC+6GaoNPGx+gI9HKORRf/aGHbXu2x2Xm3v\n\tqVvCYJyqNJH60sTi3F+BByYi5emixpMrqNqYhGHypVNN2VaWSg/kvW5HsLzMig1sSzgH\n\t0tALHioa8Xo5sgdQUxoLZz+/HkCJpuOzWRzmrcGOokrySq5PeRKkfwLV19GIZEQfamrQ\n\tWGvIRJ8Vl+YEmB4thPBt/nBTtLr+pbPIuID/Q/x3sFmBGrsNDcC0I1KuNf9nMVcW9nsJ\n\t90CPQcPj/fxslrFXo/Dg+0yfFP1WqOoGCgcz987XNXbO11qKM3FQMuw8pxF+XYChfnZ8\n\tsX/w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=jBXLyHjGFdWs/AbmZgla+sRVwmHI7fFnEBu6jps9kO8=;\n\tb=A8Q5GLzVXjr3UWCkESidBRY0iAp+yXDABigtH+gDLZUGhk7u3zkdQ74e22EuRhhLkl\n\t9mmZJRvAOc3JZ6oHEzpcQriBl+7o23roSSlH3Ja2VVVlAeWeJrC+xORby2fBDQzNsX24\n\tGGJhO+IXaDBNRREvPHsKQSCrQJ/W2kso8rNVVJsP2dkc+hvuZkAYeKL3GSEMOy/ALWaW\n\t7qGdwjG/OWwEZaoIrTuKli91Lm7dJQUQk9KTgA73Oq2jtxvIHcnlS/9PNZXQ5H28luPH\n\tg4+/XV0NflJMeO5sCQXl3shJLvSAcZzAbxepN4+b4vn9bQu/Ne/CS67rulI7eckVB3xG\n\t8xUQ==","X-Gm-Message-State":"AOAM5300Y0n/9p3hWIZi03xq91UP6TmaFv0OlHpwSN1QquQ+2MVM3NuE\n\tWrHGOOgZinAH50rzIMPnJFdFDyqhagUZOw==","X-Google-Smtp-Source":"ABdhPJwI4jOQwJq0z5CfMMUCCK6gK+GBU7BBKR8dND/+6Y7o3RTNwI1Nwqn1cK21Q0XzUHZi+jkifg==","X-Received":"by 2002:a05:6512:320d:: with SMTP id\n\td13mr16210538lfe.376.1609080941600; \n\tSun, 27 Dec 2020 06:55:41 -0800 (PST)","Date":"Sun, 27 Dec 2020 15:55:40 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"","References":"<20201224081613.41662-1-paul.elder@ideasonboard.com>\n\t<20201224081613.41662-11-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201224081613.41662-11-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 10/11] libcamera: pipeline,\n\tipa: vimc: Support the new IPC mechanism","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14361,"web_url":"https://patchwork.libcamera.org/comment/14361/","msgid":"","date":"2020-12-27T14:59:09","subject":"Re: [libcamera-devel] [PATCH v6 11/11] libcamera: pipeline,\n\tipa: rkisp1: Support the new IPC mechanism","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2020-12-24 17:16:13 +0900, Paul Elder wrote:\n> Add support to the rkisp1 pipeline handler and IPA for the new IPC\n> mechanism.\n> \n> Signed-off-by: Paul Elder \n\nReviewed-by: Niklas Söderlund \n\n> \n> ---\n> Changes in v6:\n> - move the enum and const from the header to the mojom file\n> - remove the per-pipeline ControlInfoMap\n> - since rkisp1.h has nothing in it anymore, remove it\n> - use the namespace in the pipeline handler and IPA\n> \n> Changes in v5:\n> - import the generated header with the new file name\n> \n> Changes in v4:\n> - rename Controls to controls\n> - rename IPARkISP1CallbackInterface to IPARkISP1EventInterface\n> - rename libcamera_generated_headers to libcamera_generated_ipa_headers\n> in meson\n> - use the new header name, rkisp1_ipa_interface.h\n> \n> Changes in v3:\n> - change namespacing of base ControlInfoMap structure\n> \n> New in v2\n> ---\n> include/libcamera/ipa/meson.build | 1 +\n> include/libcamera/ipa/rkisp1.h | 22 -------\n> include/libcamera/ipa/rkisp1.mojom | 44 +++++++++++++\n> src/ipa/rkisp1/meson.build | 2 +-\n> src/ipa/rkisp1/rkisp1.cpp | 61 +++++++++---------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 79 ++++++++++++------------\n> 6 files changed, 115 insertions(+), 94 deletions(-)\n> delete mode 100644 include/libcamera/ipa/rkisp1.h\n> create mode 100644 include/libcamera/ipa/rkisp1.mojom\n> \n> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> index 67e3f049..d701bccc 100644\n> --- a/include/libcamera/ipa/meson.build\n> +++ b/include/libcamera/ipa/meson.build\n> @@ -56,6 +56,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',\n> \n> ipa_mojom_files = [\n> 'raspberrypi.mojom',\n> + 'rkisp1.mojom',\n> 'vimc.mojom',\n> ]\n> \n> diff --git a/include/libcamera/ipa/rkisp1.h b/include/libcamera/ipa/rkisp1.h\n> deleted file mode 100644\n> index bb824f29..00000000\n> --- a/include/libcamera/ipa/rkisp1.h\n> +++ /dev/null\n> @@ -1,22 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * rkisp1.h - Image Processing Algorithm interface for RkISP1\n> - */\n> -#ifndef __LIBCAMERA_IPA_INTERFACE_RKISP1_H__\n> -#define __LIBCAMERA_IPA_INTERFACE_RKISP1_H__\n> -\n> -#ifndef __DOXYGEN__\n> -\n> -enum RkISP1Operations {\n> -\tRKISP1_IPA_ACTION_V4L2_SET = 1,\n> -\tRKISP1_IPA_ACTION_PARAM_FILLED = 2,\n> -\tRKISP1_IPA_ACTION_METADATA = 3,\n> -\tRKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER = 4,\n> -\tRKISP1_IPA_EVENT_QUEUE_REQUEST = 5,\n> -};\n> -\n> -#endif /* __DOXYGEN__ */\n> -\n> -#endif /* __LIBCAMERA_IPA_INTERFACE_RKISP1_H__ */\n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> new file mode 100644\n> index 00000000..9270f9c7\n> --- /dev/null\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -0,0 +1,44 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +\n> +module ipa.rkisp1;\n> +\n> +import \"include/libcamera/ipa/core.mojom\";\n> +\n> +enum RkISP1Operations {\n> +\tActionV4L2Set = 1,\n> +\tActionParamFilled = 2,\n> +\tActionMetadata = 3,\n> +\tEventSignalStatBuffer = 4,\n> +\tEventQueueRequest = 5,\n> +};\n> +\n> +struct RkISP1Event {\n> +\tRkISP1Operations op;\n> +\tuint32 frame;\n> +\tuint32 bufferId;\n> +\tControlList controls;\n> +};\n> +\n> +struct RkISP1Action {\n> +\tRkISP1Operations op;\n> +\tControlList controls;\n> +};\n> +\n> +interface IPARkISP1Interface {\n> +\tinit(IPASettings settings) => (int32 ret);\n> +\tstart() => (int32 ret);\n> +\tstop();\n> +\n> +\tconfigure(CameraSensorInfo sensorInfo,\n> +\t\t map streamConfig,\n> +\t\t map entityControls) => ();\n> +\n> +\tmapBuffers(array buffers);\n> +\tunmapBuffers(array ids);\n> +\n> +\t[async] processEvent(RkISP1Event ev);\n> +};\n> +\n> +interface IPARkISP1EventInterface {\n> +\tqueueFrameAction(uint32 frame, RkISP1Action action);\n> +};\n> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n> index ed9a6b6b..3061d53c 100644\n> --- a/src/ipa/rkisp1/meson.build\n> +++ b/src/ipa/rkisp1/meson.build\n> @@ -3,7 +3,7 @@\n> ipa_name = 'ipa_rkisp1'\n> \n> mod = shared_module(ipa_name,\n> - 'rkisp1.cpp',\n> + ['rkisp1.cpp', libcamera_generated_ipa_headers],\n> name_prefix : '',\n> include_directories : [ipa_includes, libipa_includes],\n> dependencies : libcamera_dep,\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index f4812d8d..67bac986 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -19,7 +19,7 @@\n> #include \n> #include \n> #include \n> -#include \n> +#include \n> #include \n> \n> #include \"libcamera/internal/log.h\"\n> @@ -28,25 +28,22 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPARkISP1)\n> \n> -class IPARkISP1 : public IPAInterface\n> +class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n> {\n> public:\n> \tint init([[maybe_unused]] const IPASettings &settings) override\n> \t{\n> \t\treturn 0;\n> \t}\n> -\tint start([[maybe_unused]] const IPAOperationData &data,\n> -\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n> +\tint start() override { return 0; }\n> \tvoid stop() override {}\n> \n> \tvoid configure(const CameraSensorInfo &info,\n> -\t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls,\n> -\t\t const IPAOperationData &ipaConfig,\n> -\t\t IPAOperationData *response) override;\n> +\t\t const std::map &streamConfig,\n> +\t\t const std::map &entityControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> -\tvoid processEvent(const IPAOperationData &event) override;\n> +\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n> \n> private:\n> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> @@ -79,10 +76,8 @@ private:\n> * before accessing them.\n> */\n> void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,\n> -\t\t\t [[maybe_unused]] const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls,\n> -\t\t\t [[maybe_unused]] const IPAOperationData &ipaConfig,\n> -\t\t\t [[maybe_unused]] IPAOperationData *result)\n> +\t\t\t [[maybe_unused]] const std::map &streamConfig,\n> +\t\t\t const std::map &entityControls)\n> {\n> \tif (entityControls.empty())\n> \t\treturn;\n> @@ -158,12 +153,12 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPARkISP1::processEvent(const IPAOperationData &event)\n> +void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> {\n> -\tswitch (event.operation) {\n> -\tcase RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER: {\n> -\t\tunsigned int frame = event.data[0];\n> -\t\tunsigned int bufferId = event.data[1];\n> +\tswitch (event.op) {\n> +\tcase ipa::rkisp1::EventSignalStatBuffer: {\n> +\t\tunsigned int frame = event.frame;\n> +\t\tunsigned int bufferId = event.bufferId;\n> \n> \t\tconst rkisp1_stat_buffer *stats =\n> \t\t\tstatic_cast(buffersMemory_[bufferId]);\n> @@ -171,18 +166,18 @@ void IPARkISP1::processEvent(const IPAOperationData &event)\n> \t\tupdateStatistics(frame, stats);\n> \t\tbreak;\n> \t}\n> -\tcase RKISP1_IPA_EVENT_QUEUE_REQUEST: {\n> -\t\tunsigned int frame = event.data[0];\n> -\t\tunsigned int bufferId = event.data[1];\n> +\tcase ipa::rkisp1::EventQueueRequest: {\n> +\t\tunsigned int frame = event.frame;\n> +\t\tunsigned int bufferId = event.bufferId;\n> \n> \t\trkisp1_params_cfg *params =\n> \t\t\tstatic_cast(buffersMemory_[bufferId]);\n> \n> -\t\tqueueRequest(frame, params, event.controls[0]);\n> +\t\tqueueRequest(frame, params, event.controls);\n> \t\tbreak;\n> \t}\n> \tdefault:\n> -\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.operation;\n> +\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.op;\n> \t\tbreak;\n> \t}\n> }\n> @@ -202,8 +197,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n> \t}\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_ACTION_PARAM_FILLED;\n> +\tipa::rkisp1::RkISP1Action op;\n> +\top.op = ipa::rkisp1::ActionParamFilled;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> @@ -255,13 +250,13 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> \n> void IPARkISP1::setControls(unsigned int frame)\n> {\n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_ACTION_V4L2_SET;\n> +\tipa::rkisp1::RkISP1Action op;\n> +\top.op = ipa::rkisp1::ActionV4L2Set;\n> \n> \tControlList ctrls(ctrls_);\n> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain_));\n> -\top.controls.push_back(ctrls);\n> +\top.controls = ctrls;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> @@ -273,9 +268,9 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> \tif (aeState)\n> \t\tctrls.set(controls::AeLocked, aeState == 2);\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_ACTION_METADATA;\n> -\top.controls.push_back(ctrls);\n> +\tipa::rkisp1::RkISP1Action op;\n> +\top.op = ipa::rkisp1::ActionMetadata;\n> +\top.controls = ctrls;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> @@ -292,9 +287,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \t\"rkisp1\",\n> };\n> \n> -struct ipa_context *ipaCreate()\n> +IPAInterface *ipaCreate()\n> {\n> -\treturn new IPAInterfaceWrapper(std::make_unique());\n> +\treturn new IPARkISP1();\n> }\n> }\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 021d0ffe..15b5e16e 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -18,7 +18,9 @@\n> #include \n> #include \n> #include \n> -#include \n> +#include \n> +#include \n> +#include \n> #include \n> #include \n> \n> @@ -139,9 +141,11 @@ public:\n> \tRkISP1MainPath *mainPath_;\n> \tRkISP1SelfPath *selfPath_;\n> \n> +\tstd::unique_ptr ipa_;\n> +\n> private:\n> \tvoid queueFrameAction(unsigned int frame,\n> -\t\t\t const IPAOperationData &action);\n> +\t\t\t const ipa::rkisp1::RkISP1Action &action);\n> \n> \tvoid metadataReady(unsigned int frame, const ControlList &metadata);\n> };\n> @@ -406,7 +410,7 @@ private:\n> \n> int RkISP1CameraData::loadIPA()\n> {\n> -\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> \n> @@ -419,27 +423,27 @@ int RkISP1CameraData::loadIPA()\n> }\n> \n> void RkISP1CameraData::queueFrameAction(unsigned int frame,\n> -\t\t\t\t\tconst IPAOperationData &action)\n> +\t\t\t\t\tconst ipa::rkisp1::RkISP1Action &action)\n> {\n> -\tswitch (action.operation) {\n> -\tcase RKISP1_IPA_ACTION_V4L2_SET: {\n> -\t\tconst ControlList &controls = action.controls[0];\n> +\tswitch (action.op) {\n> +\tcase ipa::rkisp1::ActionV4L2Set: {\n> +\t\tconst ControlList &controls = action.controls;\n> \t\ttimeline_.scheduleAction(std::make_unique(frame,\n> \t\t\t\t\t\t\t\t\t\t sensor_.get(),\n> \t\t\t\t\t\t\t\t\t\t controls));\n> \t\tbreak;\n> \t}\n> -\tcase RKISP1_IPA_ACTION_PARAM_FILLED: {\n> +\tcase ipa::rkisp1::ActionParamFilled: {\n> \t\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n> \t\tif (info)\n> \t\t\tinfo->paramFilled = true;\n> \t\tbreak;\n> \t}\n> -\tcase RKISP1_IPA_ACTION_METADATA:\n> -\t\tmetadataReady(frame, action.controls[0]);\n> +\tcase ipa::rkisp1::ActionMetadata:\n> +\t\tmetadataReady(frame, action.controls);\n> \t\tbreak;\n> \tdefault:\n> -\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.operation;\n> +\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.op;\n> \t\tbreak;\n> \t}\n> }\n> @@ -766,15 +770,15 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> \n> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> -\t\t\t\t\t .planes = buffer->planes() });\n> +\t\tdata->ipaBuffers_.push_back(IPABuffer(buffer->cookie(),\n> +\t\t\t\t\t\t buffer->planes()));\n> \t\tavailableParamBuffers_.push(buffer.get());\n> \t}\n> \n> \tfor (std::unique_ptr &buffer : statBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> -\t\t\t\t\t .planes = buffer->planes() });\n> +\t\tdata->ipaBuffers_.push_back(IPABuffer(buffer->cookie(),\n> +\t\t\t\t\t\t buffer->planes()));\n> \t\tavailableStatBuffers_.push(buffer.get());\n> \t}\n> \n> @@ -828,8 +832,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tIPAOperationData ipaData = {};\n> -\tret = data->ipa_->start(ipaData, nullptr);\n> +\tret = data->ipa_->start();\n> \tif (ret) {\n> \t\tfreeBuffers(camera);\n> \t\tLOG(RkISP1, Error)\n> @@ -870,10 +873,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\t\treturn ret;\n> \t\t}\n> \n> -\t\tstreamConfig[0] = {\n> -\t\t\t.pixelFormat = data->mainPathStream_.configuration().pixelFormat,\n> -\t\t\t.size = data->mainPathStream_.configuration().size,\n> -\t\t};\n> +\t\tstreamConfig[0] = IPAStream(\n> +\t\t\tdata->mainPathStream_.configuration().pixelFormat,\n> +\t\t\tdata->mainPathStream_.configuration().size\n> +\t\t);\n> \t}\n> \n> \tif (data->selfPath_->isEnabled()) {\n> @@ -887,10 +890,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\t\treturn ret;\n> \t\t}\n> \n> -\t\tstreamConfig[1] = {\n> -\t\t\t.pixelFormat = data->selfPathStream_.configuration().pixelFormat,\n> -\t\t\t.size = data->selfPathStream_.configuration().size,\n> -\t\t};\n> +\t\tstreamConfig[1] = IPAStream(\n> +\t\t\tdata->selfPathStream_.configuration().pixelFormat,\n> +\t\t\tdata->selfPathStream_.configuration().size\n> +\t\t);\n> \t}\n> \n> \tactiveCamera_ = camera;\n> @@ -905,12 +908,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\tret = 0;\n> \t}\n> \n> -\tstd::map entityControls;\n> +\tstd::map entityControls;\n> \tentityControls.emplace(0, data->sensor_->controls());\n> \n> -\tIPAOperationData ipaConfig;\n> -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> -\t\t\t ipaConfig, nullptr);\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> \n> \treturn ret;\n> }\n> @@ -952,11 +953,12 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> \tif (!info)\n> \t\treturn -ENOENT;\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST;\n> -\top.data = { data->frame_, info->paramBuffer->cookie() };\n> -\top.controls = { request->controls() };\n> -\tdata->ipa_->processEvent(op);\n> +\tipa::rkisp1::RkISP1Event ev;\n> +\tev.op = ipa::rkisp1::EventQueueRequest;\n> +\tev.frame = data->frame_;\n> +\tev.bufferId = info->paramBuffer->cookie();\n> +\tev.controls = request->controls();\n> +\tdata->ipa_->processEvent(ev);\n> \n> \tdata->timeline_.scheduleAction(std::make_unique(data->frame_,\n> \t\t\t\t\t\t\t\t\t\t data,\n> @@ -1178,10 +1180,11 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> \tif (data->frame_ <= buffer->metadata().sequence)\n> \t\tdata->frame_ = buffer->metadata().sequence + 1;\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> -\top.data = { info->frame, info->statBuffer->cookie() };\n> -\tdata->ipa_->processEvent(op);\n> +\tipa::rkisp1::RkISP1Event ev;\n> +\tev.op = ipa::rkisp1::EventSignalStatBuffer;\n> +\tev.frame = info->frame;\n> +\tev.bufferId = info->statBuffer->cookie();\n> +\tdata->ipa_->processEvent(ev);\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)\n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","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 097D9C0F1B\n\tfor ;\n\tSun, 27 Dec 2020 14:59:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F17361D25;\n\tSun, 27 Dec 2020 15:59:13 +0100 (CET)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F94160526\n\tfor ;\n\tSun, 27 Dec 2020 15:59:11 +0100 (CET)","by mail-lf1-x12b.google.com with SMTP id x20so18719672lfe.12\n\tfor ;\n\tSun, 27 Dec 2020 06:59:11 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tg17sm5020011lfh.167.2020.12.27.06.59.09\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 27 Dec 2020 06:59:10 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"uKtYWjqA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=g5ZJyxPh8Q/7Sh+fFaDxTDBcTo7nLVkIn0eOToBz/SE=;\n\tb=uKtYWjqAT49yhQGNip6kJ2GGbmirLEabP0UVQEPq/xNnAkwQi534oLs3bNEEe7Wph9\n\tE+Lw2MpOQZ8aKXCD/QG4Qjfs/O72H7HXzziEOaOeMEKocB1HHFngUkYW0EtGsQ0Y71s1\n\tAY0WrNwtUYuwyWqputVQjF4QT26ZHTa/MZ6/vJbxwleoxFMCwPbymBVp4Nd2BVP09w68\n\tYPgZmA9B5H11/CNyns7YcGecjhTxMH4TdrL0t4j+vlQfEpZXwKxqZ2ubaCZujc8lYjcl\n\t8dWURtr/sLifGapSVapNISJ1mFHmgg1xszJiNOcvQjLepS8/V5per77HzLOBR8LojI2/\n\t0jCA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=g5ZJyxPh8Q/7Sh+fFaDxTDBcTo7nLVkIn0eOToBz/SE=;\n\tb=uNoEeRe/aeVQeCNUGng3Ee2cgDCEKQR+h132dRhSnGUgaHN9JIgn5QoAucTHEWTQD0\n\tpTpVe9Nu+zmFxooE9RFuuM6JChyLlvtqS9Mi4fXFK7GPX8FgZLqVWNJnjPnE+ua8fhHj\n\tNqDED1krRQcBI58et/A4qhir2Pzm1kPrK2cGfnUf4wDQls8AdoqUsgN6ZtAlmESwRfP3\n\tiP8AZDd59G4vHSdXWw+5vtryacmeqem8mMgEVSjI1offMqpxdN64m6jTsNhfa85k2EmK\n\tloDOtcerI0zWYcWzToITMTEADV7X6teQKHHr/8CD/FpwubX6i+oTt2+DzNJut+5pQTQZ\n\tPWfw==","X-Gm-Message-State":"AOAM531+af2cOj5BSz9BfA+WmBZcjGPeR9Z6Vzz2S8S413tdmUjkQ2oJ\n\tTJqa33aryI9qlu3K0q9U5O+5Kw==","X-Google-Smtp-Source":"ABdhPJxf6l8s7V50MgNPSDTzwHYynp9lWIp6SccujpMgbz412Y/5kqsm0r0F1Xrrvog8asuXVc4YMA==","X-Received":"by 2002:a2e:8e2a:: with SMTP id\n\tr10mr19514360ljk.237.1609081150781; \n\tSun, 27 Dec 2020 06:59:10 -0800 (PST)","Date":"Sun, 27 Dec 2020 15:59:09 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"","References":"<20201224081613.41662-1-paul.elder@ideasonboard.com>\n\t<20201224081613.41662-12-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201224081613.41662-12-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 11/11] libcamera: pipeline,\n\tipa: rkisp1: Support the new IPC mechanism","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14941,"web_url":"https://patchwork.libcamera.org/comment/14941/","msgid":"","date":"2021-02-04T01:44:19","subject":"Re: [libcamera-devel] [PATCH v6 08/11] ipa: raspberrypi: Add mojom\n\tdata definition file","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Thu, Dec 24, 2020 at 05:16:10PM +0900, Paul Elder wrote:\n> Add a mojom data definition for raspberrypi pipeline handler's IPAs.\n> This is a direct translation of what the raspberrypi pipeline handler\n> and IPA was using before with IPAOperationData.\n> \n> Also move the enums from raspberrypi.h to raspberrypi.mojom\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v6:\n> - rebase on \"pipeline: ipa: raspberrypi: Handle failures during IPA\n> configuration\"\n> - move enums and consts to the mojom file\n> - use the custom start()\n> - remove unnecessary ConfigParameters, and remove lsTableHandleStatic\n> from ConfigInput\n> \n> Changes in v5:\n> - rename some structs\n> \n> Changes in v4:\n> - rename IPARPiCallbackInterface to IPARPiEventInterface\n> \n> Changes in v3:\n> - remove stray comment about how our compiler will generate code\n> - add ipa.rpi namespace\n> - not ipa.RPi, because namespace conflict\n> - remove RPi prefix from struct and enum names\n> - add transform parameter to struct ConfigInput\n> \n> Changes in v2:\n> - rebased on \"libcamera: pipeline: ipa: raspberrypi: Rework drop frame\n> signalling\"\n> - add license\n> - move generic documentation to core.mojom\n> - move documentation from IPA interface\n> - customize the RPi IPA interface to make the pipeline and IPA code\n> cleaner\n> ---\n> include/libcamera/ipa/meson.build | 4 +-\n> include/libcamera/ipa/raspberrypi.h | 20 ----\n> include/libcamera/ipa/raspberrypi.mojom | 134 ++++++++++++++++++++++++\n> 3 files changed, 137 insertions(+), 21 deletions(-)\n> create mode 100644 include/libcamera/ipa/raspberrypi.mojom\n> \n> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> index 499d1bc0..785c1241 100644\n> --- a/include/libcamera/ipa/meson.build\n> +++ b/include/libcamera/ipa/meson.build\n> @@ -54,7 +54,9 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',\n> './' +'@INPUT@'\n> ])\n> \n> -ipa_mojom_files = []\n> +ipa_mojom_files = [\n> + 'raspberrypi.mojom',\n> +]\n> \n> ipa_mojoms = []\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index 01fe5abc..df30b4a2 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -16,26 +16,6 @@ namespace libcamera {\n> \n> namespace RPi {\n> \n> -enum ConfigParameters {\n> -\tIPA_CONFIG_LS_TABLE = (1 << 0),\n> -\tIPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n> -\tIPA_CONFIG_SENSOR = (1 << 2),\n> -\tIPA_CONFIG_DROP_FRAMES = (1 << 3),\n> -\tIPA_CONFIG_FAILED = (1 << 4),\n> -\tIPA_CONFIG_STARTUP = (1 << 5),\n> -};\n> -\n> -enum Operations {\n> -\tIPA_ACTION_V4L2_SET_STAGGERED = 1,\n> -\tIPA_ACTION_V4L2_SET_ISP,\n> -\tIPA_ACTION_STATS_METADATA_COMPLETE,\n> -\tIPA_ACTION_RUN_ISP,\n> -\tIPA_ACTION_EMBEDDED_COMPLETE,\n> -\tIPA_EVENT_SIGNAL_STAT_READY,\n> -\tIPA_EVENT_SIGNAL_ISP_PREPARE,\n> -\tIPA_EVENT_QUEUE_REQUEST,\n> -};\n> -\n> enum BufferMask {\n> \tID\t\t= 0x00ffff,\n> \tSTATS\t\t= 0x010000,\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> new file mode 100644\n> index 00000000..53521ce9\n> --- /dev/null\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -0,0 +1,134 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +\n> +module ipa.rpi;\n> +\n> +import \"include/libcamera/ipa/core.mojom\";\n> +\n> +enum BufferMask {\n> +\tMaskID\t\t\t= 0x00ffff,\n> +\tMaskStats\t\t= 0x010000,\n> +\tMaskEmbeddedData\t= 0x020000,\n> +\tMaskBayerData\t\t= 0x040000,\n> +\tMaskExternalBuffer\t= 0x100000,\n> +};\n> +\n> +/* Size of the LS grid allocation. */\n> +const uint32 MaxLsGridSize = 0x8000;\n\nIt's annoying that the mojom parse doesn't allow us to write 32 * 1024\n:-( Should we file a bug to Google to get this fixed eventually ?\n\n> +\n> +enum ConfigParameters {\n> +\tConfigLsTable = 0x01,\n> +\tConfigStaggeredWrite = 0x02,\n> +\tConfigSensor = 0x04,\n> +\tConfigFailed = 0x08,\n> +};\n\nNow that we have a more dynamic interface, we could rework this. The\nConfigLsTable flag isn't needed, as we can instead check if the\nlsTableHandle is valid or not.\n\nConfigFailed would be best returned through a dedicated status field in\nConfigOutput that would return 0 on success or a negative error code on\nerror, through a boolean field named \"success\" or \"error\", or through a\nret return value for the configure() function, like done with start().\n\nWe can keep ConfigStaggeredWrite and ConfigSensor for now (until we get\nsupport for nullable values), but I would already rename\nConfigParameters to ConfigOutputParameters to emphasize the fact it's\nused for ConfigOutput only. The ConfigOutput::op field should be renamed\nto params, and the ConfigInput::op field should be dropped.\n\nYou can do this as part as this patch already (but it then wouldn't be a\ndirect translation anymore, as stated in the commit message) or as a\npatch on top (which can be moved to a Part 4 to avoid blocking the merge\nof parts 1 to 3), up to you.\n\n> +\n> +struct SensorConfig {\n> +\tuint32 gainDelay;\n> +\tuint32 exposureDelay;\n> +\tuint32 sensorMetadata;\n> +};\n> +\n> +struct ISPConfig {\n> +\tuint32 embeddedbufferId;\n> +\tuint32 bayerbufferId;\n> +};\n> +\n> +struct ConfigInput {\n> +\tuint32 op;\n> +\tuint32 transform;\n> +\tFileDescriptor lsTableHandle;\n> +};\n> +\n> +struct ConfigOutput {\n> +\tuint32 op;\n> +\tSensorConfig sensorConfig;\n> +\tControlList controls;\n> +};\n> +\n> +struct StartControls {\n> +\tbool hasControls;\n> +\tControlList controls;\n> +\tint32 dropFrameCount;\n> +};\n> +\n> +interface IPARPiInterface {\n> +\tinit(IPASettings settings) => (int32 ret);\n> +\tstart(StartControls controls) => (StartControls result, int32 ret);\n> +\tstop();\n> +\n> +\t/**\n> +\t * \\fn configure()\n> +\t * \\brief Configure the IPA stream and sensor settings\n> +\t * \\param[in] sensorInfo Camera sensor information\n> +\t * \\param[in] streamConfig Configuration of all active streams\n> +\t * \\param[in] entityControls Controls provided by the pipeline entities\n> +\t * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n> +\t * \\param[out] result Pipeline-handler-specific configuration result\n\nThe return value is named \"results\".\n\n> +\t *\n> +\t * This method shall be called when the camera is started to inform the IPA of\n\n\"started\" or \"configured\" ?\n\n> +\t * the camera's streams and the sensor settings.\n> +\t *\n> +\t * The \\a sensorInfo conveys information about the camera sensor settings that\n> +\t * the pipeline handler has selected for the configuration.\n> +\t *\n> +\t * The \\a ipaConfig and \\a result parameters carry data passed by the\n\ns/result/results/\n\nReviewed-by: Laurent Pinchart \n\n> +\t * pipeline handler to the IPA and back.\n> +\t */\n> +\tconfigure(CameraSensorInfo sensorInfo,\n> +\t\t map streamConfig,\n> +\t\t map entityControls,\n> +\t\t ConfigInput ipaConfig)\n> +\t\t=> (ConfigOutput results);\n> +\n> +\t/**\n> +\t * \\fn mapBuffers()\n> +\t * \\brief Map buffers shared between the pipeline handler and the IPA\n> +\t * \\param[in] buffers List of buffers to map\n> +\t *\n> +\t * This method informs the IPA module of memory buffers set up by the pipeline\n> +\t * handler that the IPA needs to access. It provides dmabuf file handles for\n> +\t * each buffer, and associates the buffers with unique numerical IDs.\n> +\t *\n> +\t * IPAs shall map the dmabuf file handles to their address space and keep a\n> +\t * cache of the mappings, indexed by the buffer numerical IDs. The IDs are used\n> +\t * in all other IPA interface methods to refer to buffers, including the\n> +\t * unmapBuffers() method.\n> +\t *\n> +\t * All buffers that the pipeline handler wishes to share with an IPA shall be\n> +\t * mapped with this method. Buffers may be mapped all at once with a single\n> +\t * call, or mapped and unmapped dynamically at runtime, depending on the IPA\n> +\t * protocol. Regardless of the protocol, all buffers mapped at a given time\n> +\t * shall have unique numerical IDs.\n> +\t *\n> +\t * The numerical IDs have no meaning defined by the IPA interface, and \n> +\t * should be treated as opaque handles by IPAs, with the only exception\n> +\t * that ID zero is invalid.\n> +\t *\n> +\t * \\sa unmapBuffers()\n> +\t */\n> +\tmapBuffers(array buffers);\n> +\n> +\t/**\n> +\t * \\fn unmapBuffers()\n> +\t * \\brief Unmap buffers shared by the pipeline to the IPA\n> +\t * \\param[in] ids List of buffer IDs to unmap\n> +\t *\n> +\t * This method removes mappings set up with mapBuffers(). Numerical IDs\n> +\t * of unmapped buffers may be reused when mapping new buffers.\n> +\t *\n> +\t * \\sa mapBuffers()\n> +\t */\n> +\tunmapBuffers(array ids);\n> +\n> +\t[async] signalStatReady(uint32 bufferId);\n> +\t[async] signalQueueRequest(ControlList controls);\n> +\t[async] signalIspPrepare(ISPConfig data);\n> +};\n> +\n> +interface IPARPiEventInterface {\n> +\tstatsMetadataComplete(uint32 bufferId, ControlList controls);\n> +\trunIsp(uint32 bufferId);\n> +\tembeddedComplete(uint32 bufferId);\n> +\tsetIsp(ControlList controls);\n> +\tsetStaggered(ControlList controls);\n> +};","headers":{"Return-Path":"","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 7B39ABD160\n\tfor ;\n\tThu, 4 Feb 2021 01:44:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E5017613E9;\n\tThu, 4 Feb 2021 02:44:43 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0DBA2613DA\n\tfor ;\n\tThu, 4 Feb 2021 02:44:42 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 679455A5;\n\tThu, 4 Feb 2021 02:44:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YJTEQJCx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612403081;\n\tbh=bRBIQKS58TuJtEwASpYiRZkFAxl22Q4c+eki0zDG56A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YJTEQJCxKzIhnX5l1BwN8+FpXf+tpZt7Wu3CHkiCbRMsjMb7IcEZFVq0UiD9Urc7w\n\tfFGmEkgA92hBaZkiWKTLGPCluAxPjPzdlqRKLkwKzaGPPN85UwkEar07HvFHHlXNnJ\n\tJI72H5Wb8Sa9/yKeIEGxa8vG+sKs6Pz7P0L1MkRs=","Date":"Thu, 4 Feb 2021 03:44:19 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20201224081613.41662-1-paul.elder@ideasonboard.com>\n\t<20201224081613.41662-9-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201224081613.41662-9-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 08/11] ipa: raspberrypi: Add mojom\n\tdata definition file","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14943,"web_url":"https://patchwork.libcamera.org/comment/14943/","msgid":"","date":"2021-02-04T02:53:49","subject":"Re: [libcamera-devel] [PATCH v6 11/11] libcamera: pipeline,\n\tipa: rkisp1: Support the new IPC mechanism","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Thu, Dec 24, 2020 at 05:16:13PM +0900, Paul Elder wrote:\n> Add support to the rkisp1 pipeline handler and IPA for the new IPC\n> mechanism.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v6:\n> - move the enum and const from the header to the mojom file\n> - remove the per-pipeline ControlInfoMap\n> - since rkisp1.h has nothing in it anymore, remove it\n> - use the namespace in the pipeline handler and IPA\n> \n> Changes in v5:\n> - import the generated header with the new file name\n> \n> Changes in v4:\n> - rename Controls to controls\n> - rename IPARkISP1CallbackInterface to IPARkISP1EventInterface\n> - rename libcamera_generated_headers to libcamera_generated_ipa_headers\n> in meson\n> - use the new header name, rkisp1_ipa_interface.h\n> \n> Changes in v3:\n> - change namespacing of base ControlInfoMap structure\n> \n> New in v2\n> ---\n> include/libcamera/ipa/meson.build | 1 +\n> include/libcamera/ipa/rkisp1.h | 22 -------\n> include/libcamera/ipa/rkisp1.mojom | 44 +++++++++++++\n> src/ipa/rkisp1/meson.build | 2 +-\n> src/ipa/rkisp1/rkisp1.cpp | 61 +++++++++---------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 79 ++++++++++++------------\n> 6 files changed, 115 insertions(+), 94 deletions(-)\n> delete mode 100644 include/libcamera/ipa/rkisp1.h\n> create mode 100644 include/libcamera/ipa/rkisp1.mojom\n> \n> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> index 67e3f049..d701bccc 100644\n> --- a/include/libcamera/ipa/meson.build\n> +++ b/include/libcamera/ipa/meson.build\n> @@ -56,6 +56,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',\n> \n> ipa_mojom_files = [\n> 'raspberrypi.mojom',\n> + 'rkisp1.mojom',\n> 'vimc.mojom',\n> ]\n> \n> diff --git a/include/libcamera/ipa/rkisp1.h b/include/libcamera/ipa/rkisp1.h\n> deleted file mode 100644\n> index bb824f29..00000000\n> --- a/include/libcamera/ipa/rkisp1.h\n> +++ /dev/null\n> @@ -1,22 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * rkisp1.h - Image Processing Algorithm interface for RkISP1\n> - */\n> -#ifndef __LIBCAMERA_IPA_INTERFACE_RKISP1_H__\n> -#define __LIBCAMERA_IPA_INTERFACE_RKISP1_H__\n> -\n> -#ifndef __DOXYGEN__\n> -\n> -enum RkISP1Operations {\n> -\tRKISP1_IPA_ACTION_V4L2_SET = 1,\n> -\tRKISP1_IPA_ACTION_PARAM_FILLED = 2,\n> -\tRKISP1_IPA_ACTION_METADATA = 3,\n> -\tRKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER = 4,\n> -\tRKISP1_IPA_EVENT_QUEUE_REQUEST = 5,\n> -};\n> -\n> -#endif /* __DOXYGEN__ */\n> -\n> -#endif /* __LIBCAMERA_IPA_INTERFACE_RKISP1_H__ */\n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> new file mode 100644\n> index 00000000..9270f9c7\n> --- /dev/null\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -0,0 +1,44 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +\n> +module ipa.rkisp1;\n> +\n> +import \"include/libcamera/ipa/core.mojom\";\n> +\n> +enum RkISP1Operations {\n> +\tActionV4L2Set = 1,\n> +\tActionParamFilled = 2,\n> +\tActionMetadata = 3,\n> +\tEventSignalStatBuffer = 4,\n> +\tEventQueueRequest = 5,\n> +};\n> +\n> +struct RkISP1Event {\n> +\tRkISP1Operations op;\n> +\tuint32 frame;\n> +\tuint32 bufferId;\n> +\tControlList controls;\n> +};\n> +\n> +struct RkISP1Action {\n> +\tRkISP1Operations op;\n> +\tControlList controls;\n> +};\n\nIt would be nice to replace the op-based mechanism with dedicated API\nfunctions, but given that this will be extensively reworked anyway, it's\nfine for now.\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +interface IPARkISP1Interface {\n> +\tinit(IPASettings settings) => (int32 ret);\n> +\tstart() => (int32 ret);\n> +\tstop();\n> +\n> +\tconfigure(CameraSensorInfo sensorInfo,\n> +\t\t map streamConfig,\n> +\t\t map entityControls) => ();\n> +\n> +\tmapBuffers(array buffers);\n> +\tunmapBuffers(array ids);\n> +\n> +\t[async] processEvent(RkISP1Event ev);\n> +};\n> +\n> +interface IPARkISP1EventInterface {\n> +\tqueueFrameAction(uint32 frame, RkISP1Action action);\n> +};\n> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n> index ed9a6b6b..3061d53c 100644\n> --- a/src/ipa/rkisp1/meson.build\n> +++ b/src/ipa/rkisp1/meson.build\n> @@ -3,7 +3,7 @@\n> ipa_name = 'ipa_rkisp1'\n> \n> mod = shared_module(ipa_name,\n> - 'rkisp1.cpp',\n> + ['rkisp1.cpp', libcamera_generated_ipa_headers],\n> name_prefix : '',\n> include_directories : [ipa_includes, libipa_includes],\n> dependencies : libcamera_dep,\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index f4812d8d..67bac986 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -19,7 +19,7 @@\n> #include \n> #include \n> #include \n> -#include \n> +#include \n> #include \n> \n> #include \"libcamera/internal/log.h\"\n> @@ -28,25 +28,22 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPARkISP1)\n> \n> -class IPARkISP1 : public IPAInterface\n> +class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n> {\n> public:\n> \tint init([[maybe_unused]] const IPASettings &settings) override\n> \t{\n> \t\treturn 0;\n> \t}\n> -\tint start([[maybe_unused]] const IPAOperationData &data,\n> -\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n> +\tint start() override { return 0; }\n> \tvoid stop() override {}\n> \n> \tvoid configure(const CameraSensorInfo &info,\n> -\t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls,\n> -\t\t const IPAOperationData &ipaConfig,\n> -\t\t IPAOperationData *response) override;\n> +\t\t const std::map &streamConfig,\n> +\t\t const std::map &entityControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> -\tvoid processEvent(const IPAOperationData &event) override;\n> +\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n> \n> private:\n> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> @@ -79,10 +76,8 @@ private:\n> * before accessing them.\n> */\n> void IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,\n> -\t\t\t [[maybe_unused]] const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls,\n> -\t\t\t [[maybe_unused]] const IPAOperationData &ipaConfig,\n> -\t\t\t [[maybe_unused]] IPAOperationData *result)\n> +\t\t\t [[maybe_unused]] const std::map &streamConfig,\n> +\t\t\t const std::map &entityControls)\n> {\n> \tif (entityControls.empty())\n> \t\treturn;\n> @@ -158,12 +153,12 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPARkISP1::processEvent(const IPAOperationData &event)\n> +void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> {\n> -\tswitch (event.operation) {\n> -\tcase RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER: {\n> -\t\tunsigned int frame = event.data[0];\n> -\t\tunsigned int bufferId = event.data[1];\n> +\tswitch (event.op) {\n> +\tcase ipa::rkisp1::EventSignalStatBuffer: {\n> +\t\tunsigned int frame = event.frame;\n> +\t\tunsigned int bufferId = event.bufferId;\n> \n> \t\tconst rkisp1_stat_buffer *stats =\n> \t\t\tstatic_cast(buffersMemory_[bufferId]);\n> @@ -171,18 +166,18 @@ void IPARkISP1::processEvent(const IPAOperationData &event)\n> \t\tupdateStatistics(frame, stats);\n> \t\tbreak;\n> \t}\n> -\tcase RKISP1_IPA_EVENT_QUEUE_REQUEST: {\n> -\t\tunsigned int frame = event.data[0];\n> -\t\tunsigned int bufferId = event.data[1];\n> +\tcase ipa::rkisp1::EventQueueRequest: {\n> +\t\tunsigned int frame = event.frame;\n> +\t\tunsigned int bufferId = event.bufferId;\n> \n> \t\trkisp1_params_cfg *params =\n> \t\t\tstatic_cast(buffersMemory_[bufferId]);\n> \n> -\t\tqueueRequest(frame, params, event.controls[0]);\n> +\t\tqueueRequest(frame, params, event.controls);\n> \t\tbreak;\n> \t}\n> \tdefault:\n> -\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.operation;\n> +\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.op;\n> \t\tbreak;\n> \t}\n> }\n> @@ -202,8 +197,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n> \t}\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_ACTION_PARAM_FILLED;\n> +\tipa::rkisp1::RkISP1Action op;\n> +\top.op = ipa::rkisp1::ActionParamFilled;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> @@ -255,13 +250,13 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> \n> void IPARkISP1::setControls(unsigned int frame)\n> {\n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_ACTION_V4L2_SET;\n> +\tipa::rkisp1::RkISP1Action op;\n> +\top.op = ipa::rkisp1::ActionV4L2Set;\n> \n> \tControlList ctrls(ctrls_);\n> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain_));\n> -\top.controls.push_back(ctrls);\n> +\top.controls = ctrls;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> @@ -273,9 +268,9 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> \tif (aeState)\n> \t\tctrls.set(controls::AeLocked, aeState == 2);\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_ACTION_METADATA;\n> -\top.controls.push_back(ctrls);\n> +\tipa::rkisp1::RkISP1Action op;\n> +\top.op = ipa::rkisp1::ActionMetadata;\n> +\top.controls = ctrls;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> @@ -292,9 +287,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \t\"rkisp1\",\n> };\n> \n> -struct ipa_context *ipaCreate()\n> +IPAInterface *ipaCreate()\n> {\n> -\treturn new IPAInterfaceWrapper(std::make_unique());\n> +\treturn new IPARkISP1();\n> }\n> }\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 021d0ffe..15b5e16e 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -18,7 +18,9 @@\n> #include \n> #include \n> #include \n> -#include \n> +#include \n> +#include \n> +#include \n> #include \n> #include \n> \n> @@ -139,9 +141,11 @@ public:\n> \tRkISP1MainPath *mainPath_;\n> \tRkISP1SelfPath *selfPath_;\n> \n> +\tstd::unique_ptr ipa_;\n> +\n> private:\n> \tvoid queueFrameAction(unsigned int frame,\n> -\t\t\t const IPAOperationData &action);\n> +\t\t\t const ipa::rkisp1::RkISP1Action &action);\n> \n> \tvoid metadataReady(unsigned int frame, const ControlList &metadata);\n> };\n> @@ -406,7 +410,7 @@ private:\n> \n> int RkISP1CameraData::loadIPA()\n> {\n> -\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> \n> @@ -419,27 +423,27 @@ int RkISP1CameraData::loadIPA()\n> }\n> \n> void RkISP1CameraData::queueFrameAction(unsigned int frame,\n> -\t\t\t\t\tconst IPAOperationData &action)\n> +\t\t\t\t\tconst ipa::rkisp1::RkISP1Action &action)\n> {\n> -\tswitch (action.operation) {\n> -\tcase RKISP1_IPA_ACTION_V4L2_SET: {\n> -\t\tconst ControlList &controls = action.controls[0];\n> +\tswitch (action.op) {\n> +\tcase ipa::rkisp1::ActionV4L2Set: {\n> +\t\tconst ControlList &controls = action.controls;\n> \t\ttimeline_.scheduleAction(std::make_unique(frame,\n> \t\t\t\t\t\t\t\t\t\t sensor_.get(),\n> \t\t\t\t\t\t\t\t\t\t controls));\n> \t\tbreak;\n> \t}\n> -\tcase RKISP1_IPA_ACTION_PARAM_FILLED: {\n> +\tcase ipa::rkisp1::ActionParamFilled: {\n> \t\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n> \t\tif (info)\n> \t\t\tinfo->paramFilled = true;\n> \t\tbreak;\n> \t}\n> -\tcase RKISP1_IPA_ACTION_METADATA:\n> -\t\tmetadataReady(frame, action.controls[0]);\n> +\tcase ipa::rkisp1::ActionMetadata:\n> +\t\tmetadataReady(frame, action.controls);\n> \t\tbreak;\n> \tdefault:\n> -\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.operation;\n> +\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.op;\n> \t\tbreak;\n> \t}\n> }\n> @@ -766,15 +770,15 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> \n> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> -\t\t\t\t\t .planes = buffer->planes() });\n> +\t\tdata->ipaBuffers_.push_back(IPABuffer(buffer->cookie(),\n> +\t\t\t\t\t\t buffer->planes()));\n> \t\tavailableParamBuffers_.push(buffer.get());\n> \t}\n> \n> \tfor (std::unique_ptr &buffer : statBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> -\t\t\t\t\t .planes = buffer->planes() });\n> +\t\tdata->ipaBuffers_.push_back(IPABuffer(buffer->cookie(),\n> +\t\t\t\t\t\t buffer->planes()));\n> \t\tavailableStatBuffers_.push(buffer.get());\n> \t}\n> \n> @@ -828,8 +832,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tIPAOperationData ipaData = {};\n> -\tret = data->ipa_->start(ipaData, nullptr);\n> +\tret = data->ipa_->start();\n> \tif (ret) {\n> \t\tfreeBuffers(camera);\n> \t\tLOG(RkISP1, Error)\n> @@ -870,10 +873,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\t\treturn ret;\n> \t\t}\n> \n> -\t\tstreamConfig[0] = {\n> -\t\t\t.pixelFormat = data->mainPathStream_.configuration().pixelFormat,\n> -\t\t\t.size = data->mainPathStream_.configuration().size,\n> -\t\t};\n> +\t\tstreamConfig[0] = IPAStream(\n> +\t\t\tdata->mainPathStream_.configuration().pixelFormat,\n> +\t\t\tdata->mainPathStream_.configuration().size\n> +\t\t);\n> \t}\n> \n> \tif (data->selfPath_->isEnabled()) {\n> @@ -887,10 +890,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\t\treturn ret;\n> \t\t}\n> \n> -\t\tstreamConfig[1] = {\n> -\t\t\t.pixelFormat = data->selfPathStream_.configuration().pixelFormat,\n> -\t\t\t.size = data->selfPathStream_.configuration().size,\n> -\t\t};\n> +\t\tstreamConfig[1] = IPAStream(\n> +\t\t\tdata->selfPathStream_.configuration().pixelFormat,\n> +\t\t\tdata->selfPathStream_.configuration().size\n> +\t\t);\n> \t}\n> \n> \tactiveCamera_ = camera;\n> @@ -905,12 +908,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\tret = 0;\n> \t}\n> \n> -\tstd::map entityControls;\n> +\tstd::map entityControls;\n> \tentityControls.emplace(0, data->sensor_->controls());\n> \n> -\tIPAOperationData ipaConfig;\n> -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> -\t\t\t ipaConfig, nullptr);\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> \n> \treturn ret;\n> }\n> @@ -952,11 +953,12 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> \tif (!info)\n> \t\treturn -ENOENT;\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST;\n> -\top.data = { data->frame_, info->paramBuffer->cookie() };\n> -\top.controls = { request->controls() };\n> -\tdata->ipa_->processEvent(op);\n> +\tipa::rkisp1::RkISP1Event ev;\n> +\tev.op = ipa::rkisp1::EventQueueRequest;\n> +\tev.frame = data->frame_;\n> +\tev.bufferId = info->paramBuffer->cookie();\n> +\tev.controls = request->controls();\n> +\tdata->ipa_->processEvent(ev);\n> \n> \tdata->timeline_.scheduleAction(std::make_unique(data->frame_,\n> \t\t\t\t\t\t\t\t\t\t data,\n> @@ -1178,10 +1180,11 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> \tif (data->frame_ <= buffer->metadata().sequence)\n> \t\tdata->frame_ = buffer->metadata().sequence + 1;\n> \n> -\tIPAOperationData op;\n> -\top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> -\top.data = { info->frame, info->statBuffer->cookie() };\n> -\tdata->ipa_->processEvent(op);\n> +\tipa::rkisp1::RkISP1Event ev;\n> +\tev.op = ipa::rkisp1::EventSignalStatBuffer;\n> +\tev.frame = info->frame;\n> +\tev.bufferId = info->statBuffer->cookie();\n> +\tdata->ipa_->processEvent(ev);\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)","headers":{"Return-Path":"","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 97FF9BD160\n\tfor ;\n\tThu, 4 Feb 2021 02:54:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 29778613ED;\n\tThu, 4 Feb 2021 03:54:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B480613DA\n\tfor ;\n\tThu, 4 Feb 2021 03:54:12 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DEB05A5;\n\tThu, 4 Feb 2021 03:54:11 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W6+JUnH6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612407251;\n\tbh=gYDOnKPjxu/Khk2o0hR8CKVjkNIK3ndvlkajLrSswLw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=W6+JUnH6kbyA6GjrRRurV4TOOxucXo2BYcOJXpQEyd36BcaNS1g/gAjVh5cy3gtxv\n\twCBPQnP0Ia1YlyNZv2Zg+uzz4zIKeeGKP8x/8siDHJYgiJPrhW8LtCLByab6LfO5V/\n\tde1KAzNPret8Eh57sFub4DLqvvfjvAf+y7oPnH8E=","Date":"Thu, 4 Feb 2021 04:53:49 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20201224081613.41662-1-paul.elder@ideasonboard.com>\n\t<20201224081613.41662-12-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201224081613.41662-12-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 11/11] libcamera: pipeline,\n\tipa: rkisp1: Support the new IPC mechanism","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]