From patchwork Mon Apr 13 21:26:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3447 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 60D6F6279B for ; Mon, 13 Apr 2020 23:27:08 +0200 (CEST) X-Halon-ID: 82610bfe-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 82610bfe-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:00 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= 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 Subject: [libcamera-devel] [PATCH v4 1/8] ipa: Add start() and stop() operations 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:08 -0000 Add two new operations to the IPA interface to start and stop it. The intention is that these functions shall be used by the IPA to perform actions when the camera is started and stopped. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- * Changes since v2 - Update commit message - Add cerr to test case - Add documentation * Changes since v3 - Return int from start() in ipa_context_ops and propagate it - Remove stray return statement in IPAContextWrapper::stop() - Update documentation - Line wrapping in test case --- include/ipa/ipa_interface.h | 4 +++ include/ipa/ipa_vimc.h | 2 ++ src/ipa/libipa/ipa_interface_wrapper.cpp | 16 ++++++++++ src/ipa/libipa/ipa_interface_wrapper.h | 2 ++ src/ipa/rkisp1/rkisp1.cpp | 2 ++ src/ipa/vimc/vimc.cpp | 20 +++++++++++++ src/libcamera/include/ipa_context_wrapper.h | 2 ++ src/libcamera/ipa_context_wrapper.cpp | 22 ++++++++++++++ src/libcamera/ipa_interface.cpp | 32 ++++++++++++++++++++ src/libcamera/proxy/ipa_proxy_linux.cpp | 2 ++ test/ipa/ipa_interface_test.cpp | 22 ++++++++++++++ test/ipa/ipa_wrappers_test.cpp | 33 +++++++++++++++++++-- 12 files changed, 156 insertions(+), 3 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 229d1124b19ec1c9..e65844bc7b3497d8 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -64,6 +64,8 @@ struct ipa_context_ops { void (*destroy)(struct ipa_context *ctx); void *(*get_interface)(struct ipa_context *ctx); void (*init)(struct ipa_context *ctx); + int (*start)(struct ipa_context *ctx); + void (*stop)(struct ipa_context *ctx); void (*register_callbacks)(struct ipa_context *ctx, const struct ipa_callback_ops *callbacks, void *cb_ctx); @@ -120,6 +122,8 @@ public: virtual ~IPAInterface() {} virtual int init() = 0; + virtual int start() = 0; + virtual void stop() = 0; virtual void configure(const std::map &streamConfig, const std::map &entityControls) = 0; diff --git a/include/ipa/ipa_vimc.h b/include/ipa/ipa_vimc.h index 9add122cf598aa6f..8e82dd94bf479e35 100644 --- a/include/ipa/ipa_vimc.h +++ b/include/ipa/ipa_vimc.h @@ -15,6 +15,8 @@ namespace libcamera { enum IPAOperationCode { IPAOperationNone, IPAOperationInit, + IPAOperationStart, + IPAOperationStop, }; } /* namespace libcamera */ diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp index b93c1c1f1c1a2018..f50f93a0185ae3f7 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.cpp +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp @@ -86,6 +86,20 @@ void IPAInterfaceWrapper::init(struct ipa_context *_ctx) ctx->ipa_->init(); } +int IPAInterfaceWrapper::start(struct ipa_context *_ctx) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + + return ctx->ipa_->start(); +} + +void IPAInterfaceWrapper::stop(struct ipa_context *_ctx) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + + ctx->ipa_->stop(); +} + void IPAInterfaceWrapper::register_callbacks(struct ipa_context *_ctx, const struct ipa_callback_ops *callbacks, void *cb_ctx) @@ -234,6 +248,8 @@ const struct ipa_context_ops IPAInterfaceWrapper::operations_ = { .destroy = &IPAInterfaceWrapper::destroy, .get_interface = &IPAInterfaceWrapper::get_interface, .init = &IPAInterfaceWrapper::init, + .start = &IPAInterfaceWrapper::start, + .stop = &IPAInterfaceWrapper::stop, .register_callbacks = &IPAInterfaceWrapper::register_callbacks, .configure = &IPAInterfaceWrapper::configure, .map_buffers = &IPAInterfaceWrapper::map_buffers, diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h index 3fb7b447643953ff..e4bc6dd4535dff4e 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.h +++ b/src/ipa/libipa/ipa_interface_wrapper.h @@ -24,6 +24,8 @@ private: static void destroy(struct ipa_context *ctx); static void *get_interface(struct ipa_context *ctx); static void init(struct ipa_context *ctx); + static int start(struct ipa_context *ctx); + static void stop(struct ipa_context *ctx); static void register_callbacks(struct ipa_context *ctx, const struct ipa_callback_ops *callbacks, void *cb_ctx); diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 438b3c66f77a1e57..af38e3299f2ff045 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -33,6 +33,8 @@ class IPARkISP1 : public IPAInterface { public: int init() override { return 0; } + int start() override { return 0; } + void stop() override {} void configure(const std::map &streamConfig, const std::map &entityControls) override; diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp index 6e2095b56bbcdaa3..566a66e404affbd5 100644 --- a/src/ipa/vimc/vimc.cpp +++ b/src/ipa/vimc/vimc.cpp @@ -32,6 +32,10 @@ public: ~IPAVimc(); int init() override; + + int start() override; + void stop() override; + void configure(const std::map &streamConfig, const std::map &entityControls) override {} void mapBuffers(const std::vector &buffers) override {} @@ -66,6 +70,22 @@ int IPAVimc::init() return 0; } +int IPAVimc::start() +{ + trace(IPAOperationStart); + + LOG(IPAVimc, Debug) << "start vimc IPA!"; + + return 0; +} + +void IPAVimc::stop() +{ + trace(IPAOperationStop); + + LOG(IPAVimc, Debug) << "stop vimc IPA!"; +} + void IPAVimc::initTrace() { struct stat fifoStat; diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h index c9e194120de6b69c..0a48bfe732c831d7 100644 --- a/src/libcamera/include/ipa_context_wrapper.h +++ b/src/libcamera/include/ipa_context_wrapper.h @@ -20,6 +20,8 @@ public: ~IPAContextWrapper(); int init() override; + int start() override; + void stop() override; void configure(const std::map &streamConfig, const std::map &entityControls) override; diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp index 946a2fd8cab1782a..ab6ce396b88a03e0 100644 --- a/src/libcamera/ipa_context_wrapper.cpp +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -82,6 +82,28 @@ int IPAContextWrapper::init() return 0; } +int IPAContextWrapper::start() +{ + if (intf_) + return intf_->start(); + + if (!ctx_) + return 0; + + return ctx_->ops->start(ctx_); +} + +void IPAContextWrapper::stop() +{ + if (intf_) + return intf_->stop(); + + if (!ctx_) + return; + + ctx_->ops->stop(ctx_); +} + void IPAContextWrapper::configure(const std::map &streamConfig, const std::map &entityControls) { diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 5959e7deda6c5172..0b785bdfe6e6a705 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -235,6 +235,20 @@ * \sa libcamera::IPAInterface::init() */ +/** + * \var ipa_context_ops::start + * \brief Start the IPA context + * + * \sa libcamera::IPAInterface::start() + */ + +/** + * \var ipa_context_ops::stop + * \brief Stop the IPA context + * + * \sa libcamera::IPAInterface::stop() + */ + /** * \var ipa_context_ops::register_callbacks * \brief Register callback operation from the IPA to the pipeline handler @@ -412,6 +426,24 @@ namespace libcamera { * \brief Initialise the IPAInterface */ +/** + * \fn IPAInterface::start() + * \brief Start the IPA + * + * This method informs the IPA module that the camera is about to be started. + * The IPA module shall prepare any resources it needs to operate. + * + * \return 0 on success or a negative error code otherwise + */ + +/** + * \fn IPAInterface::stop() + * \brief Stop the IPA + * + * This method informs the IPA module that the camera is stopped. The IPA module + * shall release resources prepared in start(). + */ + /** * \fn IPAInterface::configure() * \brief Configure the IPA stream and sensor settings diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp index c7218fb472490cc8..2aa80b9467044fbf 100644 --- a/src/libcamera/proxy/ipa_proxy_linux.cpp +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp @@ -27,6 +27,8 @@ public: ~IPAProxyLinux(); int init() override { return 0; } + int start() override { return 0; } + void stop() override {} void configure(const std::map &streamConfig, const std::map &entityControls) override {} void mapBuffers(const std::vector &buffers) override {} diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index cafc249bbbc96cf8..22f9ca41ef37cec1 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -109,6 +109,28 @@ protected: return TestFail; } + /* Test start of IPA module. */ + ipa_->start(); + timer.start(1000); + while (timer.isRunning() && trace_ != IPAOperationStart) + dispatcher->processEvents(); + + if (trace_ != IPAOperationStart) { + cerr << "Failed to test IPA start sequence" << endl; + return TestFail; + } + + /* Test stop of IPA module. */ + ipa_->stop(); + timer.start(1000); + while (timer.isRunning() && trace_ != IPAOperationStop) + dispatcher->processEvents(); + + if (trace_ != IPAOperationStop) { + cerr << "Failed to test IPA stop sequence" << endl; + return TestFail; + } + return TestPass; } diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp index 1ae1781169c85e6c..fae1d56b67c7933c 100644 --- a/test/ipa/ipa_wrappers_test.cpp +++ b/test/ipa/ipa_wrappers_test.cpp @@ -27,6 +27,8 @@ using namespace std; enum Operation { Op_init, + Op_start, + Op_stop, Op_configure, Op_mapBuffers, Op_unmapBuffers, @@ -47,6 +49,17 @@ public: return 0; } + int start() override + { + report(Op_start, TestPass); + return 0; + } + + void stop() override + { + report(Op_stop, TestPass); + } + void configure(const std::map &streamConfig, const std::map &entityControls) override { @@ -323,12 +336,26 @@ protected: return TestFail; /* - * Test init() last to ensure nothing in the wrappers or - * serializer depends on init() being called first. + * Test init(), start() and stop() last to ensure nothing in the + * wrappers or serializer depends on them being called first. */ ret = INVOKE(init); - if (ret == TestFail) + if (ret == TestFail) { + cerr << "Failed to run init()"; return TestFail; + } + + ret = INVOKE(start); + if (ret == TestFail) { + cerr << "Failed to run start()"; + return TestFail; + } + + ret = INVOKE(stop); + if (ret == TestFail) { + cerr << "Failed to run stop()"; + return TestFail; + } return TestPass; } From patchwork Mon Apr 13 21:26:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3448 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E66836279B for ; Mon, 13 Apr 2020 23:27:08 +0200 (CEST) X-Halon-ID: 82f6d3c2-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 82f6d3c2-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:00 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 13 Apr 2020 23:26:54 +0200 Message-Id: <20200413212700.1373247-3-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 Subject: [libcamera-devel] [PATCH v4 2/8] libcamera: pipeline: rkisp1: Queue parameters even if they are not ready 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:09 -0000 If the IPA has not filled in the parameters buffer still queue it to hardware. Not queuing the buffer results in the pipeline and hardware going out of sync. This is not a permanent fix of the problem and a todo is added to fix it properly. This change does not make the situation worse as the state of the pipeline is just as unknown as if no param buffer is queued as if one with old content in it. Signed-off-by: Niklas Söderlund Acked-by: Laurent Pinchart --- * Changes since v2 - Update commit message - Update todo comment in rkisp1.c --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 2f909cef7c75ba0f..3d37677fb433833a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -351,13 +351,23 @@ protected: if (!info) LOG(RkISP1, Fatal) << "Frame not known"; - if (info->paramFilled) - pipe_->param_->queueBuffer(info->paramBuffer); - else + /* + * \todo: If parameters are not filled a better method to handle + * the situation than queuing a buffer with unknown content + * should be used. + * + * It seems excessive to keep an internal zeroed scratch + * parameters buffer around as this should not happen unless the + * devices is under too much load. Perhaps failing the request + * and returning it to the application with an error code is + * better than queue it to hardware? + */ + if (!info->paramFilled) LOG(RkISP1, Error) << "Parameters not ready on time for frame " - << frame() << ", ignore parameters."; + << frame(); + pipe_->param_->queueBuffer(info->paramBuffer); pipe_->stat_->queueBuffer(info->statBuffer); pipe_->video_->queueBuffer(info->videoBuffer); } From patchwork Mon Apr 13 21:26:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3449 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7548262DEE for ; Mon, 13 Apr 2020 23:27:09 +0200 (CEST) X-Halon-ID: 8349f3a9-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 8349f3a9-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:01 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 13 Apr 2020 23:26:55 +0200 Message-Id: <20200413212700.1373247-4-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 Subject: [libcamera-devel] [PATCH v4 3/8] libcamera: pipeline: rkisp1: Initialize IPA 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:10 -0000 The IPA init() call is a no-op in the current IPA, but it's part of the IPA API and it should be called. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 3d37677fb433833a..de90615edf217cca 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -386,6 +386,8 @@ int RkISP1CameraData::loadIPA() ipa_->queueFrameAction.connect(this, &RkISP1CameraData::queueFrameAction); + ipa_->init(); + return 0; } From patchwork Mon Apr 13 21:26:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3450 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E5DD062DD9 for ; Mon, 13 Apr 2020 23:27:09 +0200 (CEST) X-Halon-ID: 838d85e6-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 838d85e6-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:01 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 13 Apr 2020 23:26:56 +0200 Message-Id: <20200413212700.1373247-5-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 Subject: [libcamera-devel] [PATCH v4 4/8] libcamera: pipeline: rkisp1: Add clear() to RkISP1Frames 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:11 -0000 When the camera is stopping the helper that keeps track of which buffers are associated with a request is not cleared. Add a clear operation and call it when the camera is stopped. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index de90615edf217cca..c4e46bea1cc9a47f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -64,6 +64,7 @@ public: RkISP1FrameInfo *create(unsigned int frame, Request *request, Stream *stream); int destroy(unsigned int frame); + void clear(); RkISP1FrameInfo *find(unsigned int frame); RkISP1FrameInfo *find(FrameBuffer *buffer); @@ -279,6 +280,21 @@ int RkISP1Frames::destroy(unsigned int frame) return 0; } +void RkISP1Frames::clear() +{ + auto it = frameInfo_.cbegin(); + while (it != frameInfo_.cend()) { + RkISP1FrameInfo *info = it->second; + + pipe_->availableParamBuffers_.push(info->paramBuffer); + pipe_->availableStatBuffers_.push(info->statBuffer); + + it = frameInfo_.erase(it); + + delete info; + } +} + RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame) { auto itInfo = frameInfo_.find(frame); @@ -832,6 +848,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) data->timeline_.reset(); + data->frameInfo_.clear(); + freeBuffers(camera); activeCamera_ = nullptr; From patchwork Mon Apr 13 21:26:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3451 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 505EF6279B for ; Mon, 13 Apr 2020 23:27:10 +0200 (CEST) X-Halon-ID: 83ce134a-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 83ce134a-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:02 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 13 Apr 2020 23:26:57 +0200 Message-Id: <20200413212700.1373247-6-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 Subject: [libcamera-devel] [PATCH v4 5/8] libcamera: pipeline: rkisp1: Do not try to process cancelled frames 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:12 -0000 There is no need to try and process cancelled frames, try to finish as quickly as possible. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- * Changes since v2 - If the buffer intended for the application is canceled complete it and the request as to not lockup applications. --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c4e46bea1cc9a47f..c7068c0c285f3775 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1033,6 +1033,12 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) RkISP1CameraData *data = cameraData(activeCamera_); Request *request = buffer->request(); + if (buffer->metadata().status == FrameMetadata::FrameCancelled) { + completeBuffer(activeCamera_, request, buffer); + completeRequest(activeCamera_, request); + return; + } + data->timeline_.bufferReady(buffer); if (data->frame_ <= buffer->metadata().sequence) @@ -1044,6 +1050,9 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer) { + if (buffer->metadata().status == FrameMetadata::FrameCancelled) + return; + ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); @@ -1055,6 +1064,9 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer) void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) { + if (buffer->metadata().status == FrameMetadata::FrameCancelled) + return; + ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); From patchwork Mon Apr 13 21:26:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3452 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C732762826 for ; Mon, 13 Apr 2020 23:27:10 +0200 (CEST) X-Halon-ID: 841a66d0-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 841a66d0-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:02 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 13 Apr 2020 23:26:58 +0200 Message-Id: <20200413212700.1373247-7-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 Subject: [libcamera-devel] [PATCH v4 6/8] libcamera: pipeline: rkisp1: Call IPA start() and stop() 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:12 -0000 Call the IPA start()/stop() functions before/after the camera is started. This makes sure the IPA functions properly once thread management is moved into the start/stop interface. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c7068c0c285f3775..e3cdc77ba6be560c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -780,10 +780,19 @@ int PipelineHandlerRkISP1::start(Camera *camera) if (ret) return ret; + ret = data->ipa_->start(); + if (ret) { + freeBuffers(camera); + LOG(RkISP1, Error) + << "Failed to start IPA " << camera->name(); + return ret; + } + data->frame_ = 0; ret = param_->streamOn(); if (ret) { + data->ipa_->stop(); freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start parameters " << camera->name(); @@ -793,6 +802,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) ret = stat_->streamOn(); if (ret) { param_->streamOff(); + data->ipa_->stop(); freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start statistics " << camera->name(); @@ -803,6 +813,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) if (ret) { param_->streamOff(); stat_->streamOff(); + data->ipa_->stop(); freeBuffers(camera); LOG(RkISP1, Error) @@ -846,6 +857,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) LOG(RkISP1, Warning) << "Failed to stop parameters " << camera->name(); + data->ipa_->stop(); + data->timeline_.reset(); data->frameInfo_.clear(); From patchwork Mon Apr 13 21:26:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3453 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 482D462E16 for ; Mon, 13 Apr 2020 23:27:11 +0200 (CEST) X-Halon-ID: 8467126f-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 8467126f-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:03 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 13 Apr 2020 23:26:59 +0200 Message-Id: <20200413212700.1373247-8-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 Subject: [libcamera-devel] [PATCH v4 7/8] libcamera: pipeline: vimc: Call IPA start() and stop() 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:12 -0000 Call the IPA start()/stop() functions before/after the camera is started. This makes sure the IPA functions properly once thread management is moved into the start/stop interface. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/vimc/vimc.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index b04a9726efa5ade6..c5eea3a01b0e9446 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -274,8 +274,15 @@ int PipelineHandlerVimc::start(Camera *camera) if (ret < 0) return ret; + ret = data->ipa_->start(); + if (ret) { + data->video_->releaseBuffers(); + return ret; + } + ret = data->video_->streamOn(); if (ret < 0) { + data->ipa_->stop(); data->video_->releaseBuffers(); return ret; } @@ -287,6 +294,7 @@ void PipelineHandlerVimc::stop(Camera *camera) { VimcCameraData *data = cameraData(camera); data->video_->streamOff(); + data->ipa_->stop(); data->video_->releaseBuffers(); } From patchwork Mon Apr 13 21:27:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 3454 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 236E662826 for ; Mon, 13 Apr 2020 23:27:13 +0200 (CEST) X-Halon-ID: 84ae242f-7dcd-11ea-aeed-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de [79.202.35.146]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 84ae242f-7dcd-11ea-aeed-005056917f90; Mon, 13 Apr 2020 23:27:04 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 13 Apr 2020 23:27:00 +0200 Message-Id: <20200413212700.1373247-9-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 Subject: [libcamera-devel] [PATCH v4 8/8] libcamera: ipa_manager: Proxy open-source IPAs to a thread 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: , X-List-Received-Date: Mon, 13 Apr 2020 21:27:13 -0000 From: Laurent Pinchart While closed-source IPA modules will always be sandboxed, open-source IPA modules may be run in the main libcamera process or be sandboxed, depending on platform configuration. These two models exhibit very different timings, which require extensive testing with both configurations. When run into the main libcamera process, IPA modules are executed in the pipeline handler thread (which is currently a global CameraManager thread). Time-consuming operations in the IPA may thus slow down the pipeline handler and compromise real-time behaviour. At least some pipeline handlers will thus likely spawn a thread to isolate the IPA, leading to code duplication in pipeline handlers. Solve both issues by always proxying IPA modules. For open-source IPA modules that run in the libcamera process, a new IPAProxyThread class is added to run the IPA in a separate thread. Signed-off-by: Laurent Pinchart [Niklas: Move thread start/stop of thread into start()/stop()] Signed-off-by: Niklas Söderlund --- * Changes since v2 - Move the thread at init() instead of start() - Call start() in the target thread. - Block frame actions in IPAProxyThread instead of leaving it up to each pipeline handler. --- src/libcamera/ipa_interface.cpp | 3 + src/libcamera/ipa_manager.cpp | 48 +++---- src/libcamera/proxy/ipa_proxy_thread.cpp | 162 +++++++++++++++++++++++ src/libcamera/proxy/meson.build | 1 + 4 files changed, 187 insertions(+), 27 deletions(-) create mode 100644 src/libcamera/proxy/ipa_proxy_thread.cpp diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 0b785bdfe6e6a705..890d4340e4f373c3 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -528,6 +528,9 @@ namespace libcamera { * \a data.operation field, as defined by the IPA protocol, and the rest of the * \a data is interpreted accordingly. The pipeline handler shall queue the * action and execute it as appropriate. + * + * The signal is only emitted when the IPA is running, that is after start() and + * before stop() have been called. */ } /* namespace libcamera */ diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index bcaae3564ea14e07..6d23f470506561b8 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -12,7 +12,6 @@ #include #include -#include "ipa_context_wrapper.h" #include "ipa_module.h" #include "ipa_proxy.h" #include "log.h" @@ -271,40 +270,35 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe, if (!m) return nullptr; - if (!m->isOpenSource()) { - IPAProxyFactory *pf = nullptr; - std::vector &factories = IPAProxyFactory::factories(); + /* + * Load and run the IPA module in a thread if it is open-source, or + * isolate it in a separate process otherwise. + * + * \todo Implement a better proxy selection + */ + const char *proxyName = m->isOpenSource() + ? "IPAProxyThread" : "IPAProxyLinux"; + IPAProxyFactory *pf = nullptr; - for (IPAProxyFactory *factory : factories) { - /* TODO: Better matching */ - if (!strcmp(factory->name().c_str(), "IPAProxyLinux")) { - pf = factory; - break; - } + for (IPAProxyFactory *factory : IPAProxyFactory::factories()) { + if (!strcmp(factory->name().c_str(), proxyName)) { + pf = factory; + break; } - - if (!pf) { - LOG(IPAManager, Error) << "Failed to get proxy factory"; - return nullptr; - } - - std::unique_ptr proxy = pf->create(m); - if (!proxy->isValid()) { - LOG(IPAManager, Error) << "Failed to load proxy"; - return nullptr; - } - - return proxy; } - if (!m->load()) + if (!pf) { + LOG(IPAManager, Error) << "Failed to get proxy factory"; return nullptr; + } - struct ipa_context *ctx = m->createContext(); - if (!ctx) + std::unique_ptr proxy = pf->create(m); + if (!proxy->isValid()) { + LOG(IPAManager, Error) << "Failed to load proxy"; return nullptr; + } - return std::make_unique(ctx); + return proxy; } } /* namespace libcamera */ diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp new file mode 100644 index 0000000000000000..368ccca1cf60b8ec --- /dev/null +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp @@ -0,0 +1,162 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * ipa_proxy_thread.cpp - Proxy running an Image Processing Algorithm in a thread + */ + +#include + +#include +#include + +#include "ipa_context_wrapper.h" +#include "ipa_module.h" +#include "ipa_proxy.h" +#include "log.h" +#include "thread.h" + +namespace libcamera { + +LOG_DECLARE_CATEGORY(IPAProxy) + +class IPAProxyThread : public IPAProxy, public Object +{ +public: + IPAProxyThread(IPAModule *ipam); + + int init() override; + int start() override; + void stop() override; + + void configure(const std::map &streamConfig, + const std::map &entityControls) override; + void mapBuffers(const std::vector &buffers) override; + void unmapBuffers(const std::vector &ids) override; + void processEvent(const IPAOperationData &event) override; + +private: + void queueFrameAction(unsigned int frame, const IPAOperationData &data); + + /* Helper class to invoke processEvent() in another thread. */ + class ThreadProxy : public Object + { + public: + void setIPA(IPAInterface *ipa) + { + ipa_ = ipa; + } + + int start() + { + return ipa_->start(); + } + + void stop() + { + ipa_->stop(); + } + + void processEvent(const IPAOperationData &event) + { + ipa_->processEvent(event); + } + + private: + IPAInterface *ipa_; + }; + + bool running_; + Thread thread_; + ThreadProxy proxy_; + std::unique_ptr ipa_; +}; + +IPAProxyThread::IPAProxyThread(IPAModule *ipam) + : running_(false) +{ + if (!ipam->load()) + return; + + struct ipa_context *ctx = ipam->createContext(); + if (!ctx) { + LOG(IPAProxy, Error) + << "Failed to create IPA context for " << ipam->path(); + return; + } + + ipa_ = std::make_unique(ctx); + proxy_.setIPA(ipa_.get()); + + /* + * Proxy the queueFrameAction signal to dispatch it in the caller's + * thread. + */ + ipa_->queueFrameAction.connect(this, &IPAProxyThread::queueFrameAction); + + valid_ = true; +} + +int IPAProxyThread::init() +{ + int ret = ipa_->init(); + if (ret) + return ret; + + proxy_.moveToThread(&thread_); + + return 0; +} + +int IPAProxyThread::start() +{ + running_ = true; + thread_.start(); + + return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking); +} + +void IPAProxyThread::stop() +{ + running_ = false; + + proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking); + + thread_.exit(); + thread_.wait(); +} + +void IPAProxyThread::configure(const std::map &streamConfig, + const std::map &entityControls) +{ + ipa_->configure(streamConfig, entityControls); +} + +void IPAProxyThread::mapBuffers(const std::vector &buffers) +{ + ipa_->mapBuffers(buffers); +} + +void IPAProxyThread::unmapBuffers(const std::vector &ids) +{ + ipa_->unmapBuffers(ids); +} + +void IPAProxyThread::processEvent(const IPAOperationData &event) +{ + if (!running_) + return; + + /* Dispatch the processEvent() call to the thread. */ + proxy_.invokeMethod(&ThreadProxy::processEvent, ConnectionTypeQueued, + event); +} + +void IPAProxyThread::queueFrameAction(unsigned int frame, const IPAOperationData &data) +{ + IPAInterface::queueFrameAction.emit(frame, data); +} + +REGISTER_IPA_PROXY(IPAProxyThread) + +} /* namespace libcamera */ diff --git a/src/libcamera/proxy/meson.build b/src/libcamera/proxy/meson.build index efc1132302176f63..6c00d5f30ad2e5f0 100644 --- a/src/libcamera/proxy/meson.build +++ b/src/libcamera/proxy/meson.build @@ -1,3 +1,4 @@ libcamera_sources += files([ 'ipa_proxy_linux.cpp', + 'ipa_proxy_thread.cpp', ])