{"id":3447,"url":"https://patchwork.libcamera.org/api/patches/3447/?format=json","web_url":"https://patchwork.libcamera.org/patch/3447/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200413212700.1373247-2-niklas.soderlund@ragnatech.se>","date":"2020-04-13T21:26:53","name":"[libcamera-devel,v4,1/8] ipa: Add start() and stop() operations","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"1e751ec3f390ee46b5e281d96f599fb9dc1325d9","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/3447/mbox/","series":[{"id":805,"url":"https://patchwork.libcamera.org/api/series/805/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=805","date":"2020-04-13T21:26:52","name":"libcamera: ipa_manager: Proxy open-source IPAs to a thread","version":4,"mbox":"https://patchwork.libcamera.org/series/805/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/3447/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/3447/checks/","tags":{},"headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60D6F6279B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 13 Apr 2020 23:27:08 +0200 (CEST)","from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de\n\t[79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid 82610bfe-7dcd-11ea-aeed-005056917f90;\n\tMon, 13 Apr 2020 23:27:00 +0200 (CEST)"],"X-Halon-ID":"82610bfe-7dcd-11ea-aeed-005056917f90","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 13 Apr 2020 23:26:53 +0200","Message-Id":"<20200413212700.1373247-2-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.26.0","In-Reply-To":"<20200413212700.1373247-1-niklas.soderlund@ragnatech.se>","References":"<20200413212700.1373247-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v4 1/8] ipa: Add start() and stop()\n\toperations","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>","X-List-Received-Date":"Mon, 13 Apr 2020 21:27:08 -0000"},"content":"Add two new operations to the IPA interface to start and stop it. The\nintention is that these functions shall be used by the IPA to perform\nactions when the camera is started and stopped.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n* Changes since v2\n- Update commit message\n- Add cerr to test case\n- Add documentation\n\n* Changes since v3\n- Return int from start() in ipa_context_ops and propagate it\n- Remove stray return statement in IPAContextWrapper::stop()\n- Update documentation\n- Line wrapping in test case\n---\n include/ipa/ipa_interface.h                 |  4 +++\n include/ipa/ipa_vimc.h                      |  2 ++\n src/ipa/libipa/ipa_interface_wrapper.cpp    | 16 ++++++++++\n src/ipa/libipa/ipa_interface_wrapper.h      |  2 ++\n src/ipa/rkisp1/rkisp1.cpp                   |  2 ++\n src/ipa/vimc/vimc.cpp                       | 20 +++++++++++++\n src/libcamera/include/ipa_context_wrapper.h |  2 ++\n src/libcamera/ipa_context_wrapper.cpp       | 22 ++++++++++++++\n src/libcamera/ipa_interface.cpp             | 32 ++++++++++++++++++++\n src/libcamera/proxy/ipa_proxy_linux.cpp     |  2 ++\n test/ipa/ipa_interface_test.cpp             | 22 ++++++++++++++\n test/ipa/ipa_wrappers_test.cpp              | 33 +++++++++++++++++++--\n 12 files changed, 156 insertions(+), 3 deletions(-)","diff":"diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\nindex 229d1124b19ec1c9..e65844bc7b3497d8 100644\n--- a/include/ipa/ipa_interface.h\n+++ b/include/ipa/ipa_interface.h\n@@ -64,6 +64,8 @@ struct ipa_context_ops {\n \tvoid (*destroy)(struct ipa_context *ctx);\n \tvoid *(*get_interface)(struct ipa_context *ctx);\n \tvoid (*init)(struct ipa_context *ctx);\n+\tint (*start)(struct ipa_context *ctx);\n+\tvoid (*stop)(struct ipa_context *ctx);\n \tvoid (*register_callbacks)(struct ipa_context *ctx,\n \t\t\t\t   const struct ipa_callback_ops *callbacks,\n \t\t\t\t   void *cb_ctx);\n@@ -120,6 +122,8 @@ public:\n \tvirtual ~IPAInterface() {}\n \n \tvirtual int init() = 0;\n+\tvirtual int start() = 0;\n+\tvirtual void stop() = 0;\n \n \tvirtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls) = 0;\ndiff --git a/include/ipa/ipa_vimc.h b/include/ipa/ipa_vimc.h\nindex 9add122cf598aa6f..8e82dd94bf479e35 100644\n--- a/include/ipa/ipa_vimc.h\n+++ b/include/ipa/ipa_vimc.h\n@@ -15,6 +15,8 @@ namespace libcamera {\n enum IPAOperationCode {\n \tIPAOperationNone,\n \tIPAOperationInit,\n+\tIPAOperationStart,\n+\tIPAOperationStop,\n };\n \n } /* namespace libcamera */\ndiff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\nindex b93c1c1f1c1a2018..f50f93a0185ae3f7 100644\n--- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n@@ -86,6 +86,20 @@ void IPAInterfaceWrapper::init(struct ipa_context *_ctx)\n \tctx->ipa_->init();\n }\n \n+int IPAInterfaceWrapper::start(struct ipa_context *_ctx)\n+{\n+\tIPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);\n+\n+\treturn ctx->ipa_->start();\n+}\n+\n+void IPAInterfaceWrapper::stop(struct ipa_context *_ctx)\n+{\n+\tIPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);\n+\n+\tctx->ipa_->stop();\n+}\n+\n void IPAInterfaceWrapper::register_callbacks(struct ipa_context *_ctx,\n \t\t\t\t\t     const struct ipa_callback_ops *callbacks,\n \t\t\t\t\t     void *cb_ctx)\n@@ -234,6 +248,8 @@ const struct ipa_context_ops IPAInterfaceWrapper::operations_ = {\n \t.destroy = &IPAInterfaceWrapper::destroy,\n \t.get_interface = &IPAInterfaceWrapper::get_interface,\n \t.init = &IPAInterfaceWrapper::init,\n+\t.start = &IPAInterfaceWrapper::start,\n+\t.stop = &IPAInterfaceWrapper::stop,\n \t.register_callbacks = &IPAInterfaceWrapper::register_callbacks,\n \t.configure = &IPAInterfaceWrapper::configure,\n \t.map_buffers = &IPAInterfaceWrapper::map_buffers,\ndiff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h\nindex 3fb7b447643953ff..e4bc6dd4535dff4e 100644\n--- a/src/ipa/libipa/ipa_interface_wrapper.h\n+++ b/src/ipa/libipa/ipa_interface_wrapper.h\n@@ -24,6 +24,8 @@ private:\n \tstatic void destroy(struct ipa_context *ctx);\n \tstatic void *get_interface(struct ipa_context *ctx);\n \tstatic void init(struct ipa_context *ctx);\n+\tstatic int start(struct ipa_context *ctx);\n+\tstatic void stop(struct ipa_context *ctx);\n \tstatic void register_callbacks(struct ipa_context *ctx,\n \t\t\t\t       const struct ipa_callback_ops *callbacks,\n \t\t\t\t       void *cb_ctx);\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 438b3c66f77a1e57..af38e3299f2ff045 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -33,6 +33,8 @@ class IPARkISP1 : public IPAInterface\n {\n public:\n \tint init() override { return 0; }\n+\tint start() override { return 0; }\n+\tvoid stop() override {}\n \n \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;\ndiff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\nindex 6e2095b56bbcdaa3..566a66e404affbd5 100644\n--- a/src/ipa/vimc/vimc.cpp\n+++ b/src/ipa/vimc/vimc.cpp\n@@ -32,6 +32,10 @@ public:\n \t~IPAVimc();\n \n \tint init() override;\n+\n+\tint start() override;\n+\tvoid stop() override;\n+\n \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}\n \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\n@@ -66,6 +70,22 @@ int IPAVimc::init()\n \treturn 0;\n }\n \n+int IPAVimc::start()\n+{\n+\ttrace(IPAOperationStart);\n+\n+\tLOG(IPAVimc, Debug) << \"start vimc IPA!\";\n+\n+\treturn 0;\n+}\n+\n+void IPAVimc::stop()\n+{\n+\ttrace(IPAOperationStop);\n+\n+\tLOG(IPAVimc, Debug) << \"stop vimc IPA!\";\n+}\n+\n void IPAVimc::initTrace()\n {\n \tstruct stat fifoStat;\ndiff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h\nindex c9e194120de6b69c..0a48bfe732c831d7 100644\n--- a/src/libcamera/include/ipa_context_wrapper.h\n+++ b/src/libcamera/include/ipa_context_wrapper.h\n@@ -20,6 +20,8 @@ public:\n \t~IPAContextWrapper();\n \n \tint init() override;\n+\tint start() override;\n+\tvoid stop() override;\n \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;\n \ndiff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\nindex 946a2fd8cab1782a..ab6ce396b88a03e0 100644\n--- a/src/libcamera/ipa_context_wrapper.cpp\n+++ b/src/libcamera/ipa_context_wrapper.cpp\n@@ -82,6 +82,28 @@ int IPAContextWrapper::init()\n \treturn 0;\n }\n \n+int IPAContextWrapper::start()\n+{\n+\tif (intf_)\n+\t\treturn intf_->start();\n+\n+\tif (!ctx_)\n+\t\treturn 0;\n+\n+\treturn ctx_->ops->start(ctx_);\n+}\n+\n+void IPAContextWrapper::stop()\n+{\n+\tif (intf_)\n+\t\treturn intf_->stop();\n+\n+\tif (!ctx_)\n+\t\treturn;\n+\n+\tctx_->ops->stop(ctx_);\n+}\n+\n void IPAContextWrapper::configure(const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t\t\t  const std::map<unsigned int, const ControlInfoMap &> &entityControls)\n {\ndiff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\nindex 5959e7deda6c5172..0b785bdfe6e6a705 100644\n--- a/src/libcamera/ipa_interface.cpp\n+++ b/src/libcamera/ipa_interface.cpp\n@@ -235,6 +235,20 @@\n  * \\sa libcamera::IPAInterface::init()\n  */\n \n+/**\n+ * \\var ipa_context_ops::start\n+ * \\brief Start the IPA context\n+ *\n+ * \\sa libcamera::IPAInterface::start()\n+ */\n+\n+/**\n+ * \\var ipa_context_ops::stop\n+ * \\brief Stop the IPA context\n+ *\n+ * \\sa libcamera::IPAInterface::stop()\n+ */\n+\n /**\n  * \\var ipa_context_ops::register_callbacks\n  * \\brief Register callback operation from the IPA to the pipeline handler\n@@ -412,6 +426,24 @@ namespace libcamera {\n  * \\brief Initialise the IPAInterface\n  */\n \n+/**\n+ * \\fn IPAInterface::start()\n+ * \\brief Start the IPA\n+ *\n+ * This method informs the IPA module that the camera is about to be started.\n+ * The IPA module shall prepare any resources it needs to operate.\n+ *\n+ * \\return 0 on success or a negative error code otherwise\n+ */\n+\n+/**\n+ * \\fn IPAInterface::stop()\n+ * \\brief Stop the IPA\n+ *\n+ * This method informs the IPA module that the camera is stopped. The IPA module\n+ * shall release resources prepared in start().\n+ */\n+\n /**\n  * \\fn IPAInterface::configure()\n  * \\brief Configure the IPA stream and sensor settings\ndiff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\nindex c7218fb472490cc8..2aa80b9467044fbf 100644\n--- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n@@ -27,6 +27,8 @@ public:\n \t~IPAProxyLinux();\n \n \tint init() override { return 0; }\n+\tint start() override { return 0; }\n+\tvoid stop() override {}\n \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}\n \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override {}\ndiff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\nindex cafc249bbbc96cf8..22f9ca41ef37cec1 100644\n--- a/test/ipa/ipa_interface_test.cpp\n+++ b/test/ipa/ipa_interface_test.cpp\n@@ -109,6 +109,28 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n+\t\t/* Test start of IPA module. */\n+\t\tipa_->start();\n+\t\ttimer.start(1000);\n+\t\twhile (timer.isRunning() && trace_ != IPAOperationStart)\n+\t\t\tdispatcher->processEvents();\n+\n+\t\tif (trace_ != IPAOperationStart) {\n+\t\t\tcerr << \"Failed to test IPA start sequence\" << endl;\n+\t\t\treturn TestFail;\n+\t\t}\n+\n+\t\t/* Test stop of IPA module. */\n+\t\tipa_->stop();\n+\t\ttimer.start(1000);\n+\t\twhile (timer.isRunning() && trace_ != IPAOperationStop)\n+\t\t\tdispatcher->processEvents();\n+\n+\t\tif (trace_ != IPAOperationStop) {\n+\t\t\tcerr << \"Failed to test IPA stop sequence\" << endl;\n+\t\t\treturn TestFail;\n+\t\t}\n+\n \t\treturn TestPass;\n \t}\n \ndiff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\nindex 1ae1781169c85e6c..fae1d56b67c7933c 100644\n--- a/test/ipa/ipa_wrappers_test.cpp\n+++ b/test/ipa/ipa_wrappers_test.cpp\n@@ -27,6 +27,8 @@ using namespace std;\n \n enum Operation {\n \tOp_init,\n+\tOp_start,\n+\tOp_stop,\n \tOp_configure,\n \tOp_mapBuffers,\n \tOp_unmapBuffers,\n@@ -47,6 +49,17 @@ public:\n \t\treturn 0;\n \t}\n \n+\tint start() override\n+\t{\n+\t\treport(Op_start, TestPass);\n+\t\treturn 0;\n+\t}\n+\n+\tvoid stop() override\n+\t{\n+\t\treport(Op_stop, TestPass);\n+\t}\n+\n \tvoid configure(const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override\n \t{\n@@ -323,12 +336,26 @@ protected:\n \t\t\treturn TestFail;\n \n \t\t/*\n-\t\t * Test init() last to ensure nothing in the wrappers or\n-\t\t * serializer depends on init() being called first.\n+\t\t * Test init(), start() and stop() last to ensure nothing in the\n+\t\t * wrappers or serializer depends on them being called first.\n \t\t */\n \t\tret = INVOKE(init);\n-\t\tif (ret == TestFail)\n+\t\tif (ret == TestFail) {\n+\t\t\tcerr << \"Failed to run init()\";\n \t\t\treturn TestFail;\n+\t\t}\n+\n+\t\tret = INVOKE(start);\n+\t\tif (ret == TestFail) {\n+\t\t\tcerr << \"Failed to run start()\";\n+\t\t\treturn TestFail;\n+\t\t}\n+\n+\t\tret = INVOKE(stop);\n+\t\tif (ret == TestFail) {\n+\t\t\tcerr << \"Failed to run stop()\";\n+\t\t\treturn TestFail;\n+\t\t}\n \n \t\treturn TestPass;\n \t}\n","prefixes":["libcamera-devel","v4","1/8"]}