From patchwork Sat Jul 4 00:52:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 8604 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A8F95BE905 for ; Sat, 4 Jul 2020 00:52:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 02C0860D4D; Sat, 4 Jul 2020 02:52:38 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="pM0DUqU/"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CC4A603BB for ; Sat, 4 Jul 2020 02:52:36 +0200 (CEST) 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 0C95029E; Sat, 4 Jul 2020 02:52:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1593823956; bh=H7Ayfsjp4jQHUzg9vj2u284ogXq7ovsvz8V8RePZ4YY=; h=From:To:Cc:Subject:Date:From; b=pM0DUqU/KoU7lLHI33r29ARbxVNnvah9ic5gcTk2Uj5LOFq5bbbwUVfAX/jfeKuDz 7xRX9GYOxpXxR5trYZZceQwCoTpkkHKVSPh2plLQKVCiBGVYuqaFoDkOd50lUOcNwR ZTLu9ZxiWgjn67pJX9Fpc5igbpxR6wTfUl8jo+E0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 4 Jul 2020 03:52:15 +0300 Message-Id: <20200704005227.21782-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 00/12] Support passing custom data to IPA configure() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Hello, This patch series is the second version of an improvement to the IPAInterface::configure() function to support passing IPA-specific data. The IPAInterface::configure() function passes miscellaneous configuration data to the IPA, but offers no way for pipeline handlers to pass custom data, or to receive custom data back. This has led to the Raspberry Pi IPA protocol using custom frame actions and events to pass initialization data to the IPA and to receive initial sensor configuration back. To support this, the Raspberry Pi IPA needs to start the IPA thread before configure() time, while it was meant to be started when starting the camera. This patch series addresses this issue by adding a way to pass custom configuration data to the IPA and receive data back. It starts with two small drive-by cleanups in patches 01/12 and 02/12. Patch 03/12 performs the bulk of the work by modifying IPAInterface::configure(). The C API isn't addressed yet, as larger refactoring is considered to handle the C <-> C++ translation in a better way. Patch 04/12 and 05/12 are other small drive-by fixes. Patch 06/12 follows by moving code around in the Raspberry Pi pipeline handler to prepare for the upcoming changes, and patch 07/12 slightly simplifies frame action handling. Patches 08/12 and 09/12 are the main changes in v2, and move sensor orientation handling from the Raspberry Pi IPA to the pipeline handler, based on the rotation reported through the device tree. The corresponding kernel patches have been posted as "[PATCH 00/12] raspberrypi: Report sensor orientation through DT". This change simplifies the Raspberry Pi IPA API, addressing a concern on the remaining patches from v1. Patches 10/12 and 11/12 replace custom events and frame actions with the new IPA-specific data passed to configure(). Patch 12/12 finally moves the IPA start and stop to the camera start and stop, aligning the implementation with how the IPAInterface API is meant to be used. Patch 11/12 is now cleaner, although the concern I had in v1 are still theoretically valid. Better code is needed to construct a protocol on top of the IPAOperationData passed to IPAInterface::configure(). This also applies to processEvent and queueFrameAction, and I think we should investigate usage of something similar to protobuf to handle both the IPA C <-> C++ translation and the IPA protocol built on top of IPAOperationData. Clever ideas are welcome. An additional improvement that I believe could be done on top of this would be to merge the RPI_IPA_ACTION_RUN_ISP, RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME and RPI_IPA_ACTION_V4L2_SET_ISP actions. The first two are essentially the same with an option, which could be passed to IPAOperationData::data, and the last one carries a set of controls that could be passed in one go. RPI_IPA_ACTION_EMBEDDED_COMPLETE could also be merged, as it's always sent with RPI_IPA_ACTION_RUN_ISP(_AND_DROP_FRAME), but there may be a future use case to keep it separate that I can't think about now. Similarly, RPI_IPA_ACTION_V4L2_SET_STAGGERED and RPI_IPA_ACTION_STATS_METADATA_COMPLETE could also be merged into a single action. Those two changes would lower the number of cross-thread calls, improving performances. The code has been tested with the imx219 camera module. Laurent Pinchart (12): libcamera: ipa: Document the parameters of the IPA C configure function libcamera: ipa_context_wrapper: Fix bad copy&paste in comment libcamera: ipa_interface: Add support for custom IPA data to configure() libcamera: pipeline: raspberrypi: Drop unused local variable libcamera: pipeline: raspberrypi: Constify parameter to StaggeredCtrl::set() libcamera: pipeline: raspberrypi: Move configureIPA() to RPiCameraData libcamera: pipeline: raspberrypi: Don't call handleState() in stop state libcamera: pipeline: raspberrypi: Set sensor flip based on rotation ipa: raspberrypi: Drop CameraHelper::GetOrientation() ipa: raspberrypi: Pass lens shading table through configure() function ipa: raspberrypi: Pass sensor config back from configure() libcamera: pipeline: raspberrypi: Start IPA when starting camera .../libcamera/internal/ipa_context_wrapper.h | 4 +- include/libcamera/ipa/ipa_interface.h | 4 +- include/libcamera/ipa/raspberrypi.h | 8 +- src/ipa/libipa/ipa_interface_wrapper.cpp | 5 +- src/ipa/raspberrypi/cam_helper.cpp | 6 - src/ipa/raspberrypi/cam_helper.hpp | 7 - src/ipa/raspberrypi/cam_helper_imx219.cpp | 7 - src/ipa/raspberrypi/cam_helper_imx477.cpp | 7 - src/ipa/raspberrypi/raspberrypi.cpp | 68 +++--- src/ipa/rkisp1/rkisp1.cpp | 8 +- src/ipa/vimc/vimc.cpp | 4 +- src/libcamera/ipa_context_wrapper.cpp | 10 +- src/libcamera/ipa_interface.cpp | 12 + .../pipeline/raspberrypi/raspberrypi.cpp | 213 +++++++++--------- .../pipeline/raspberrypi/staggered_ctrl.cpp | 2 +- .../pipeline/raspberrypi/staggered_ctrl.h | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +- src/libcamera/proxy/ipa_proxy_thread.cpp | 11 +- test/ipa/ipa_wrappers_test.cpp | 8 +- 20 files changed, 214 insertions(+), 180 deletions(-)