From patchwork Mon Apr 27 03:17:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3552 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 31622601B8 for ; Mon, 27 Apr 2020 05:17:33 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="dYgnXWjE"; dkim-atps=neutral Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8BD7D72C for ; Mon, 27 Apr 2020 05:17:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1587957452; bh=hVKYzvsOPbggvXoZKrnEuvrTPCJKJmh12rMoSexcwIc=; h=From:To:Subject:Date:From; b=dYgnXWjEvfhW7MCq5JSauaimtLPEmZJ+YUtHlUcziqCM+uvfQRsp8PZd3rdt24wIK sxFhUakwMA/Q2Gxzq94JV3LnO9NmLnVC0NkP1iKgCVng0bw4nC0yK5k/ymnREZUpSh eDvud7h/0ao0G/Cfrj15ZVOZTzu+0+OSxwJYvHZg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 27 Apr 2020 06:17:02 +0300 Message-Id: <20200427031713.14013-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.25.3 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 00/11] libcamera: Add support for IPA configuration 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, 27 Apr 2020 03:17:33 -0000 Hello, It is expected that IPA will need configuration files. To standardize the location of those files and avoid putting too much burden on pipeline handlers and IPA modules, support for this features is needed in the libcamera core. This patch series is an attempt to do so. The series starts in patch 01/11 by changing usage of the IPA module name, as stored in the IPAModuleInfo structure. The name was a free-formed string, and is now required to match the directory name of the IPA module in the source tree for modules part of libcamera. For external modules, the name is required to be usable as a directory component. Patch 02/11 then adds a new function to retrieve the libcamera source tree directory. This is similar to the build tree directory retrieval that is already used to locate IPA modules or proxy workers in the build tree, and will be used to locate configuration files in the source tree. There's a small issue with that patch in that it hardcodes the path to the source tree in the libcamera binary. I would like to strip that at install time, but haven't found a good way to do so yet. Patch 03/11 and 04/11 are small cleanups. Patch 05/11 starts the bulk of the work by adding a function to the IPAProxy class to retrieve the absolute path to a configuration file for an IPA module based on the name of the file. Please see the patch for more details. Patch 06/11 changes the IPAManager::createIPA() function to return an IPAProxy instead of an IPAInterface, giving the pipeline handlers a way to access the IPAProxy helper functions. This is fairly non-intrusive as the IPAInterface returned by that function is actually an IPAProxy instance already. Patch 07/11 extends the IPAInterface::init() function with an IPASettings argument, that only contains the configuration file path for now. This is expected to be extended later with more initialization data. Patches 08/11 to 11/11 then update the vimc IPA, and vimc pipeline handler and the tests to add a (currently dummy) configuration file to the source tree (08/11), pass it from the pipeline handler and tests to the IPA (09/11 and 10/11), and verifying it in the IPA (11/11). Except for the source tree path issue, I'm fairly happy with the result, but I'm open to reconsidering the approach if needed. Any magic solution for the source tree path issue will be highly appreciated :-) On a side note, I've discovered a problem with IPA module signatures when installing the modules. Both the module and the signature files are installed in the right location, but the module is modified to strip the DT_RPATH and DT_RUNPATH tags, which renders the signature invalid. I can see two options to fix this, regenerating the signature at install time, and ignoring the dynamic section when generating and verifying the signature. I'm not sure yet whether the former is possible with meson. The latter should ideally not ignore the whole dynamic section, as it may contain data that should be signed, but strip the DT_RPATH and DT_RUNPATH tags exactly as done by meson when generating the signature and verifying it. This may however be fragile, so ignoring the whole section may be the best option for now. I'm also open to ideas. Laurent Pinchart (11): ipa: Name IPA modules after their source directory libcamera: utils: Add a function to retrieve the libcamera source tree libcamera: ipa_proxy: Add missing space in info message libcamera: ipa_proxy: Don't include libcamera: ipa_proxy: Provide suport for IPA configuration files libcamera: ipa_manager: Return an IPAProxy from createIPA() ipa: Pass IPA initialization settings to IPAInterface::init() ipa: vimc: Add a dummy configuration file to the vimc IPA libcamera: pipeline: vimc: Pass configuration file to IPA init() test: ipa: ipa_interface: Pass configuration file to IPA init() ipa: vimc: Validate configuration file in init() include/ipa/ipa_interface.h | 13 ++- meson.build | 2 + src/ipa/libipa/ipa_interface_wrapper.cpp | 8 +- src/ipa/libipa/ipa_interface_wrapper.h | 3 +- src/ipa/meson.build | 6 ++ src/ipa/rkisp1/rkisp1.cpp | 4 +- src/ipa/vimc/data/meson.build | 6 ++ src/ipa/vimc/data/vimc.conf | 1 + src/ipa/vimc/meson.build | 2 + src/ipa/vimc/vimc.cpp | 17 +++- src/libcamera/include/ipa_context_wrapper.h | 2 +- src/libcamera/include/ipa_manager.h | 6 +- src/libcamera/include/ipa_proxy.h | 11 ++- src/libcamera/include/pipeline_handler.h | 5 +- src/libcamera/include/utils.h | 1 + src/libcamera/ipa_context_wrapper.cpp | 9 +- src/libcamera/ipa_interface.cpp | 32 +++++++ src/libcamera/ipa_manager.cpp | 14 +-- src/libcamera/ipa_module.cpp | 20 +++++ src/libcamera/ipa_proxy.cpp | 95 +++++++++++++++++++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/vimc/vimc.cpp | 8 +- src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +- src/libcamera/proxy/ipa_proxy_thread.cpp | 8 +- src/libcamera/utils.cpp | 37 +++++++- test/ipa/ipa_interface_test.cpp | 10 ++- test/ipa/ipa_module_test.cpp | 2 +- test/ipa/ipa_wrappers_test.cpp | 13 ++- 28 files changed, 288 insertions(+), 53 deletions(-) create mode 100644 src/ipa/vimc/data/meson.build create mode 100644 src/ipa/vimc/data/vimc.conf