From patchwork Sun Jun 28 23:19:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 8484 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 E0AE1C2E66 for ; Sun, 28 Jun 2020 23:19:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1FBD160A16; Mon, 29 Jun 2020 01:19:43 +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="H8VD6hvY"; 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 531EB603B6 for ; Mon, 29 Jun 2020 01:19:41 +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 AFEF54FB; Mon, 29 Jun 2020 01:19:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1593386380; bh=XoQGfpi3wtVYo3WXvtn35D4UxK/1Hv3rWKRqaTqDgVk=; h=From:To:Cc:Subject:Date:From; b=H8VD6hvYOvx9iM0SXyku4UI3GZrm/RDEcINQ9EROgdX0m8zOfeY4HQwG8MP5oxAmy ExxI0cZCRLCrzHWX9Uufbrtozhrxs6d51Cjx3v79S8t7PPLrBG9n/JwHEzr8uw+H5J ZImZy1+mhWGseM0/7bMvOsirDztX4zzRazamPRP0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Jun 2020 02:19:25 +0300 Message-Id: <20200628231934.29025-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v1 0/9] 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" 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 1/9 and 2/9. Patch 3/9 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 4/9 is another small drive-by fix. Patch 5/9 follows by moving code around in the Raspberry Pi pipeline handler to prepare for the upcoming changes, and patches 6/9 and 7/9 replace custom events and frame actions with the new IPA-specific data passed to configure(). Patch 8/9 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 9/9 is an additional simplification made possible by the rest of the series. I'm not entirely happy with patch 7/9 as it handles the data received from the IPA manually. The Raspberry Pi IPA can send both staggered write configuration data and sensor gain and exposure controls, with both set of parameters being optional. 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, but this will hopefully not be a blocker for this patch series. Naush, I'm not entirely sure about 9/9, could you confirm that RPI_IPA_ACTION_V4L2_SET_ISP never occurs in the stop state ? 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. This would lower the number of cross-thread calls, improving performances. Laurent Pinchart (9): 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: Constify parameter to StaggeredCtrl::set() libcamera: pipeline: raspberrypi: Move configureIPA() to RPiCameraData 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: pipeline: raspberrypi: Don't handle any actions in stop state .../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/raspberrypi.cpp | 64 +++--- 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 | 205 +++++++++--------- .../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 +- 16 files changed, 207 insertions(+), 148 deletions(-)