[{"id":26897,"web_url":"https://patchwork.libcamera.org/comment/26897/","msgid":"<20230419051408.GN30837@pendragon.ideasonboard.com>","date":"2023-04-19T05:14:08","subject":"Re: [libcamera-devel] [PATCH 04/11] pipeline: ipu3: Identify\n\tsensors that do not need the Imgu","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dan,\n\nThank you for the patch.\n\nI think it's \"ImgU\", not \"Imgu\". That applies to the subject line,\ncommit message and patch, and possibly to other patches in the series.\n\nOn Sat, Mar 18, 2023 at 11:40:07PM +0000, Daniel Scally via libcamera-devel wrote:\n> Some sensors connected to the CIO2 device do not need to be connected\n> to the Imgu - for example the OmniVision 7251 outputs Y10 data which\n> needn't be debayered and which is unsupported by the kernel's ipu3-imgu\n> driver.\n> \n> To be able to handle those sensors we need to be able to skip the Imgu\n> related parts of the processing in the IPU3 pipeline. As a preliminary\n> step, check whether the sensor needs the Imgu during CIO2Device::init()\n> and provide a method to check that later.\n> \n> Signed-off-by: Daniel Scally \n> ---\n> src/libcamera/pipeline/ipu3/cio2.cpp | 20 ++++++++++++++++++++\n> src/libcamera/pipeline/ipu3/cio2.h | 4 ++++\n> 2 files changed, 24 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 7400cb0b..d100df57 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -17,6 +17,7 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/bayer_format.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> #include \"libcamera/internal/media_device.h\"\n> @@ -160,6 +161,25 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\t/*\n> +\t * A camera sensor connected to the CIO2 device need not necessarily be\n> +\t * connected to the Imgu. This is the case for some IR cameras for\n> +\t * example, which output IPU3-packed Y10 data rather than packed data in\n> +\t * bayer patterns. In the case of those sensors we need to be able to\n> +\t * skip dealing with the Imgu during the pipeline, so we need to be able\n> +\t * to identify them.\n> +\t *\n> +\t * Check whether this sensor needs to be connected to the Imgu.\n\nYUV sensors would also fall in that category, so we could prepare for\nthat by explaining that the ImgU can only deal with bayer formats at its\ninput, but I don't think we'll ever see an IPU3-based system with a YUV\nsensor.\n\nWe could also talk about whether the sensor can be handled by the ImgU\ninstead of whether it needs the ImgU, as capturing raw frames only\nwithout running any IPA algorithm can also be done with bayer sensors.\nAs with my other point, this is likely a niche use cases.\n\nI'm fine with the text above.\n\n> +\t */\n> +\tneedsImgu_ = false;\n> +\tfor (unsigned int mbusCode : sensorCodes) {\n> +\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(mbusCode);\n> +\t\tif (bayerFormat.isValid() && bayerFormat.order < BayerFormat::MONO) {\n\nCould we white-list the formats supported by the ImgU instead ? The ImgU\nsupports 10-bit bayer formats only, while the BayerFormat class supports\nmore.\n\n> +\t\t\tneedsImgu_ = true;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> \t/*\n> \t * \\todo Define when to open and close video device nodes, as they\n> \t * might impact on power consumption.\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> index bbd87eb8..064673d9 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.h\n> +++ b/src/libcamera/pipeline/ipu3/cio2.h\n> @@ -63,6 +63,8 @@ public:\n> \n> \tSignal<> bufferAvailable;\n> \n> +\tbool needsImgu() { return needsImgu_; }\n> +\n> private:\n> \tvoid freeBuffers();\n> \n> @@ -74,6 +76,8 @@ private:\n> \n> \tstd::vector> buffers_;\n> \tstd::queue availableBuffers_;\n> +\n> +\tbool needsImgu_;\n> };\n> \n> } /* namespace libcamera */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9BFCDBD1F1\n\tfor ;\n\tWed, 19 Apr 2023 05:13:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F2ADE627B8;\n\tWed, 19 Apr 2023 07:13:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0D68B603A0\n\tfor ;\n\tWed, 19 Apr 2023 07:13:56 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(133-32-181-51.west.xps.vectant.ne.jp [133.32.181.51])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A75773F1;\n\tWed, 19 Apr 2023 07:13:48 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1681881238;\n\tbh=cCjav9fjURNklIruZeVCPnocENxF7zI9rvlXy1VZqq0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=KzkacPiA5WCxP2sqg5SZEJ9iWO6xU0dDJLjmpJPTU4NO6fDDiCA6aPBaat188sSwT\n\tD7DZYDcw1LGu9LUiziVQPQkEOLcMvuujc5cI6mxZu/vAFePWAhpqAZ3Io9jHD07FfE\n\tjNr5rxc4aLhwfqzvdYRGO7lJT4K4h+ifOycL+Eco9ox7JntIQS4YoTnImZK/fdVTEr\n\t9sywVzxL1XTZ12WGIVjgCks2ub5YaBLucqhP4Zb8fawin7e7GZoZVy3fDmVjCUGj19\n\to218r43Jx2NzrZ8drRpkTVCtNxuc0apumhXdTXwQYO1urjpYfP3R6OMe58BI/KGUaA\n\tfTpD0oP7PyxXQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1681881229;\n\tbh=cCjav9fjURNklIruZeVCPnocENxF7zI9rvlXy1VZqq0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nyaUGJoVPUj2PaA4fLGJxPC0v6W9K6rYeM92RG9uTQGudElf1dhJyA9AIAjbCCN/X\n\tjUHfcjaJXGZgAELgKhaLS/bGwBmEju7TDO2xo+0l0k86x4aXi2f23H/uWwnsOBgNjS\n\tVjuMmUP9xYQt8GiYIH5mH6XzAKa1se2EGaYsFMsU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nyaUGJoV\"; dkim-atps=neutral","Date":"Wed, 19 Apr 2023 08:14:08 +0300","To":"Daniel Scally ","Message-ID":"<20230419051408.GN30837@pendragon.ideasonboard.com>","References":"<20230318234014.29506-1-dan.scally@ideasonboard.com>\n\t<20230318234014.29506-5-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230318234014.29506-5-dan.scally@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 04/11] pipeline: ipu3: Identify\n\tsensors that do not need the Imgu","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]