From patchwork Thu Sep 9 13:54:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13789 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 922EEBDB1D for ; Thu, 9 Sep 2021 13:54:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5EA796916F; Thu, 9 Sep 2021 15:54:57 +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="Eutx8OwD"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 487276916C for ; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:ab35:be36:579e:39cf]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E2606D88; Thu, 9 Sep 2021 15:54:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631195694; bh=xja9h82q4iWa3Ti+Kgy1RkAwvN3ON2fmFF9+qFsXmVo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Eutx8OwDTSoP3fKwsNmbSZ/UN2vWjofrxWClPQ8ssTItRSOTVNi1c1dbLGJJyqgmg ymRyfMMjLeUyaXbfpkNnSDq0iEv6xPIGbX1A3PB0HIZZhaIpcpEwCYzO0NB4ZNtlxY wQtqDLBBnGSBRdIRWDHa6NyiTnJs43QakcKwxmWM= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Thu, 9 Sep 2021 15:54:45 +0200 Message-Id: <20210909135449.68017-2-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> References: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v6 1/5] ipa: ipu3: Move the AWB stats structures 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 structure Ipu3AwbCell describes the AWB stats layout on the kernel side. We will need it to be used by the AGC algorithm to be introduced later, so let's make it visible from ipa::ipu3::algorithms and not only for the AWB class. The IspStatsRegion will be needed by AGC too, so let's move it in the same namespace too. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/ipu3/algorithms/awb.h | 37 ++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index a16dd68d..cc848060 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -23,6 +23,24 @@ namespace ipa::ipu3::algorithms { static constexpr uint32_t kAwbStatsSizeX = 16; static constexpr uint32_t kAwbStatsSizeY = 12; +/* \todo Move the cell layout into intel-ipu3.h kernel header */ +struct Ipu3AwbCell { + unsigned char greenRedAvg; + unsigned char redAvg; + unsigned char blueAvg; + unsigned char greenBlueAvg; + unsigned char satRatio; + unsigned char padding[3]; +} __attribute__((packed)); + +struct IspStatsRegion { + unsigned int counted; + unsigned int uncounted; + unsigned long long rSum; + unsigned long long gSum; + unsigned long long bSum; +}; + class Awb : public Algorithm { public: @@ -32,16 +50,7 @@ public: void prepare(IPAContext &context, ipu3_uapi_params *params) override; void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; - struct Ipu3AwbCell { - unsigned char greenRedAvg; - unsigned char redAvg; - unsigned char blueAvg; - unsigned char greenBlueAvg; - unsigned char satRatio; - unsigned char padding[3]; - } __attribute__((packed)); - - /* \todo Make these three structs available to all the ISPs ? */ + /* \todo Make these structs available to all the ISPs ? */ struct RGB { RGB(double _R = 0, double _G = 0, double _B = 0) : R(_R), G(_G), B(_B) @@ -55,14 +64,6 @@ public: } }; - struct IspStatsRegion { - unsigned int counted; - unsigned int uncounted; - unsigned long long rSum; - unsigned long long gSum; - unsigned long long bSum; - }; - struct AwbStatus { double temperatureK; double redGain; From patchwork Thu Sep 9 13:54:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13790 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 7DF3FBDB1D for ; Thu, 9 Sep 2021 13:54:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EC04C69171; Thu, 9 Sep 2021 15:54:57 +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="G3xjskZl"; 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 7F5B06916D for ; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:ab35:be36:579e:39cf]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 22A2ADA1; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631195694; bh=VmlFURE+ZfL8K4MsTFmKB9zaLrQVAVYGhikYFIIOyBQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=G3xjskZlXJ+KylTba1vQJ99E7Gh2TkfZsXzs9PaeuCvMDKx5cKfoMU69QWhK80oSN xKii31GzJwsV7ySX0COmB7wfkvE7JjZSGPOY063aEp+Fc6lCsvO7WoHY0ailAYrYNN uWlP27X5ZUYUZpHw/oBqaEdVf7UxYGXBEiCt3A78= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Thu, 9 Sep 2021 15:54:46 +0200 Message-Id: <20210909135449.68017-3-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> References: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v6 2/5] ipa: ipu3: Rename IspStatsRegion to Accumulator 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 IspStatsRegion structure was introduced as an attempt to prepare for a generic AWB algorithm structure. The structure name by itself is not explicit and it is too optimistic to try and make a generic one for now. Its role is to accumulate the pixels in a given zone. Rename it to accumulator, and remove the uncounted field at the same time. It is always possible to know how many pixels are not relevant for the algorithm by calculating total-counted. The uncounted field was only declared and not used. Amend the documentation accordingly. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/ipu3/algorithms/awb.cpp | 79 +++++++++++++++++++++++++-------- src/ipa/ipu3/algorithms/awb.h | 5 +-- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index e05647c9..9b22c783 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -21,26 +21,70 @@ static constexpr uint32_t kMinZonesCounted = 16; static constexpr uint32_t kMinGreenLevelInZone = 32; /** - * \struct IspStatsRegion - * \brief RGB statistics for a given region + * \struct Accumulator + * \brief RGB statistics for a given zone * - * The IspStatsRegion structure is intended to abstract the ISP specific - * statistics and use an agnostic algorithm to compute AWB. + * - Cells are defined in Pixels + * - Zones are defined in Cells + * - The total zones may overlap the image width and height to meet hardware + * constraints * - * \var IspStatsRegion::counted - * \brief Number of pixels used to calculate the sums + * 81 cells + * /───────────── 1280 pixels ───────────\ + * 16 zones + * 16 + * ┌────┬────┬────┬────┬────┬─ ──────┬────┐ \ + * │Cell│ │ │ │ │ | │ │ │ + *16 │ px │ │ │ │ │ | │ │ │ + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ │ + * │ │ │ │ │ │ | │ │ + * │ │ │ │ │ │ | │ │ 7 + * │ ── │ ── │ ── │ ── │ ── │ ── ── ─┤ ── │ 1 2 4 + * │ │ │ │ │ │ | │ │ 2 0 5 * - * \var IspStatsRegion::uncounted - * \brief Remaining number of pixels in the region + * │ │ │ │ │ │ | │ │ z p c + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ o i e + * │ │ │ │ │ │ | │ │ n x l + * │ │ | │ │ e e l + * ├─── ───┼─ ──────┼────┤ s l s + * │ │ | │ │ s + * │ │ | │ │ + * ├─── Zone of Cells ───┼─ ──────┼────┤ │ + * │ (5 x 4) │ | │ │ │ + * │ │ | │ │ │ + * ├── ───┼─ ──────┼────┤ │ + * │ │ │ | │ │ │ + * │ │ │ │ │ │ | │ │ │ + * └────┴────┴────┴────┴────┴─ ──────┴────┘ / * - * \var IspStatsRegion::rSum - * \brief Sum of the red values in the region + * The algorithm works with a fixed number of zones \a kAwbStatsSizeX x + * \a kAwbStatsSizeY. For example, a frame of 1280x720 is divided into 81x45 + * cells of [16x16] pixels with a BDS output size of 1296x720 to account for the + * 16x16 pixel alignment restrictions. In the case of \a kAwbStatsSizeX=16 and + * \a kAwbStatsSizeY=12 the zones are made of [5x4] cells. The cells are + * left-aligned and calculated by IPAIPU3::calculateBdsGrid(). * - * \var IspStatsRegion::gSum - * \brief Sum of the green values in the region + * Each statistics cell represents the average value of the pixels in that cell + * split by colour components. * - * \var IspStatsRegion::bSum - * \brief Sum of the blue values in the region + * The Accumulator structure stores the sum of the average of each cell in a + * zone of the image, as well as the number of cells which were unsaturated and + * therefore included in the average. + * + * Cells which are saturated beyond the threshold defined in + * ipu3_uapi_awb_config_s are not included in the average. + * + * \var Accumulator::counted + * \brief Number of unsaturated cells used to calculate the sums + * + * \var Accumulator::rSum + * \brief Sum of the average red values of each cell in the zone + * + * \var Accumulator::gSum + * \brief Sum of the average green values of each cell in the zone + * + * \var Accumulator::bSum + * \brief Sum of the average blue values of each cell in the zone */ /** @@ -157,7 +201,7 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33; } -/* Generate an RGB vector with the average values for each region */ +/* Generate an RGB vector with the average values for each zone */ void Awb::generateZones(std::vector &zones) { for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { @@ -174,7 +218,7 @@ void Awb::generateZones(std::vector &zones) } } -/* Translate the IPU3 statistics into the default statistics region array */ +/* Translate the IPU3 statistics into the default statistics zone array */ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) { @@ -215,7 +259,6 @@ void Awb::clearAwbStats() awbStats_[i].rSum = 0; awbStats_[i].gSum = 0; awbStats_[i].counted = 0; - awbStats_[i].uncounted = 0; } } @@ -304,7 +347,7 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) /* * Optical center is column start (respectively row start) of the - * region of interest minus its X center (respectively Y center). + * cell of interest minus its X center (respectively Y center). * * For the moment use BDS as a first approximation, but it should * be calculated based on Shading (SHD) parameters. diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index cc848060..ac8ccc84 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -33,9 +33,8 @@ struct Ipu3AwbCell { unsigned char padding[3]; } __attribute__((packed)); -struct IspStatsRegion { +struct Accumulator { unsigned int counted; - unsigned int uncounted; unsigned long long rSum; unsigned long long gSum; unsigned long long bSum; @@ -82,7 +81,7 @@ private: uint32_t estimateCCT(double red, double green, double blue); std::vector zones_; - IspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; + Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; AwbStatus asyncResults_; }; From patchwork Thu Sep 9 13:54:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13791 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 25C9CBDB1D for ; Thu, 9 Sep 2021 13:54:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C305B69174; Thu, 9 Sep 2021 15:54:58 +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="TleEeI5j"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B49A6916B for ; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:ab35:be36:579e:39cf]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 57161883; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631195694; bh=QkK9niSqi5mwEAMsbZlZWTdkO7B3fRNFb9Ob0fhj9Q0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TleEeI5jNZ0s/wTM43fsvjN2prToK1whM1s0WRr4txYD6BAvlvIcyIA1Ybzsum4r/ oIGzfnIQR7c0iYxUJaw+NFq970OijR2/0HU4OvA4tq/gaAYvmbjPPgwz5MiCqB2vKI /qRR9aDWdEVl9na9VQQE/mEXSjHIM+kqO8SU0db4= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Thu, 9 Sep 2021 15:54:47 +0200 Message-Id: <20210909135449.68017-4-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> References: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v6 3/5] ipa: ipu3: Change Accumulator structure layout 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 pixel component sums for the Accumulator are inconsistent with other similar structures such as the IPAFrameContext::awb::gains. Group the red, green, and blue sums together in a struct and store them as uint64_t to reduce potential architectural differences. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/ipu3/algorithms/awb.cpp | 24 ++++++++++++------------ src/ipa/ipu3/algorithms/awb.h | 8 +++++--- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index 9b22c783..6bf114ac 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -77,13 +77,13 @@ static constexpr uint32_t kMinGreenLevelInZone = 32; * \var Accumulator::counted * \brief Number of unsaturated cells used to calculate the sums * - * \var Accumulator::rSum + * \var Accumulator::sum::red * \brief Sum of the average red values of each cell in the zone * - * \var Accumulator::gSum + * \var Accumulator::sum::green * \brief Sum of the average green values of each cell in the zone * - * \var Accumulator::bSum + * \var Accumulator::sum::blue * \brief Sum of the average blue values of each cell in the zone */ @@ -208,10 +208,10 @@ void Awb::generateZones(std::vector &zones) RGB zone; double counted = awbStats_[i].counted; if (counted >= kMinZonesCounted) { - zone.G = awbStats_[i].gSum / counted; + zone.G = awbStats_[i].sum.green / counted; if (zone.G >= kMinGreenLevelInZone) { - zone.R = awbStats_[i].rSum / counted; - zone.B = awbStats_[i].bSum / counted; + zone.R = awbStats_[i].sum.red / counted; + zone.B = awbStats_[i].sum.blue / counted; zones.push_back(zone); } } @@ -244,9 +244,9 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, /* The cell is not saturated, use the current cell */ awbStats_[awbRegionPosition].counted++; uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg; - awbStats_[awbRegionPosition].gSum += greenValue / 2; - awbStats_[awbRegionPosition].rSum += currentCell->redAvg; - awbStats_[awbRegionPosition].bSum += currentCell->blueAvg; + awbStats_[awbRegionPosition].sum.green += greenValue / 2; + awbStats_[awbRegionPosition].sum.red += currentCell->redAvg; + awbStats_[awbRegionPosition].sum.blue += currentCell->blueAvg; } } } @@ -255,9 +255,9 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, void Awb::clearAwbStats() { for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { - awbStats_[i].bSum = 0; - awbStats_[i].rSum = 0; - awbStats_[i].gSum = 0; + awbStats_[i].sum.blue = 0; + awbStats_[i].sum.red = 0; + awbStats_[i].sum.green = 0; awbStats_[i].counted = 0; } } diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index ac8ccc84..3385ebe7 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -35,9 +35,11 @@ struct Ipu3AwbCell { struct Accumulator { unsigned int counted; - unsigned long long rSum; - unsigned long long gSum; - unsigned long long bSum; + struct { + uint64_t red; + uint64_t green; + uint64_t blue; + } sum; }; class Awb : public Algorithm From patchwork Thu Sep 9 13:54:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13792 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 B7790BDB1D for ; Thu, 9 Sep 2021 13:54:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 645B069170; Thu, 9 Sep 2021 15:54:59 +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="Q/2X2E+W"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D3D726916E for ; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:ab35:be36:579e:39cf]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8AD32D88; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631195694; bh=q4EF09sQM7DQ23QWMWNX+GP1Wvvd9uNuDXCflsUhDXQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Q/2X2E+WCmHWXDYdpk8oSgt1K/0/z9d4vrX4ksgXTccpfqa/hK2omxJN1nWmD5Aeq 2j/qoStVNnwgC72d4ea4I3R+wDx1i4cE0yGSj06BPmohqzRpUThXE4JN6NGB7JVPgR 0AAaRxdKN5nRRIEActwlTYpWkg8QasQl6W8mbKSo= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Thu, 9 Sep 2021 15:54:48 +0200 Message-Id: <20210909135449.68017-5-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> References: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v6 4/5] ipa: ipu3: Replace ipa::ipu3::algorithms::Ipu3AwbCell 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 intel-ipu3.h public interface from the kernel does not define how to parse the statistics for a cell. This had to be identified by a process of reverse engineering, and later identifying the structures from [0] leading to our custom definition of struct Ipu3AwbCell. [0] https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h To improve the kernel interface, a proposal has been made to the linux-kernel [1] to incorporate the memory layout for each cell into the intel-ipu3 header directly. [1] https://lore.kernel.org/linux-media/20210831185140.77400-1-jeanmichel.hautbois@ideasonboard.com/ Update our local copy of the intel-ipu3.h to match the proposal and change the AGC and AWB algorithms to reference that structure directly, allowing us to remove the deprecated custom Ipu3AwbCell definition. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/linux/intel-ipu3.h | 28 ++++++++++++++++++++++++++-- src/ipa/ipu3/algorithms/agc.cpp | 7 ++++--- src/ipa/ipu3/algorithms/awb.cpp | 11 +++++------ src/ipa/ipu3/algorithms/awb.h | 10 ---------- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/include/linux/intel-ipu3.h b/include/linux/intel-ipu3.h index ee0e6d0e..a9de8c11 100644 --- a/include/linux/intel-ipu3.h +++ b/include/linux/intel-ipu3.h @@ -59,6 +59,29 @@ struct ipu3_uapi_grid_config { __u16 y_end; } __attribute__((packed)); +/** + * struct ipu3_uapi_awb_set_item - Memory layout for each cell in AWB + * + * @Gr_avg: Green average for red lines in the cell. + * @R_avg: Red average in the cell. + * @B_avg: Blue average in the cell. + * @Gb_avg: Green average for blue lines in the cell. + * @sat_ratio: Saturation ratio in the cell. + * @padding0: Unused byte for padding. + * @padding1: Unused byte for padding. + * @padding2: Unused byte for padding. + */ +struct ipu3_uapi_awb_set_item { + unsigned char Gr_avg; + unsigned char R_avg; + unsigned char B_avg; + unsigned char Gb_avg; + unsigned char sat_ratio; + unsigned char padding0; + unsigned char padding1; + unsigned char padding2; +} __attribute__((packed)); + /* * The grid based data is divided into "slices" called set, each slice of setX * refers to ipu3_uapi_grid_config width * height_per_slice. @@ -72,7 +95,8 @@ struct ipu3_uapi_grid_config { IPU3_UAPI_AWB_MD_ITEM_SIZE) #define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \ (IPU3_UAPI_AWB_MAX_SETS * \ - (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) + (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) / \ + sizeof(ipu3_uapi_awb_set_item) /** @@ -82,7 +106,7 @@ struct ipu3_uapi_grid_config { * the average values for each color channel. */ struct ipu3_uapi_awb_raw_buffer { - __u8 meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE] + struct ipu3_uapi_awb_set_item meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE] __attribute__((aligned(32))); } __attribute__((packed)); diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 5ff50f4a..8740dcdf 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -96,9 +96,10 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, * We observed a bit shift which makes the value 160 to be 32 in the stats grid. * Use the one passed at init time. */ - if (stats->awb_raw_buffer.meta_data[i + 4 + j * grid.width] == 0) { - uint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width]; - uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width]; + const ipu3_uapi_awb_set_item *currentCell = &stats->awb_raw_buffer.meta_data[i + j * grid.width]; + if (currentCell->sat_ratio == 0) { + uint8_t Gr = currentCell->Gr_avg; + uint8_t Gb = currentCell->Gb_avg; hist[(Gr + Gb) / 2]++; count++; } diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index 6bf114ac..cf97208f 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -236,17 +236,16 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, uint32_t cellY = ((cellPosition / grid.width) / regionHeight) % kAwbStatsSizeY; uint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX; - cellPosition *= 8; /* Cast the initial IPU3 structure to simplify the reading */ - Ipu3AwbCell *currentCell = reinterpret_cast(const_cast(&stats->awb_raw_buffer.meta_data[cellPosition])); - if (currentCell->satRatio == 0) { + const ipu3_uapi_awb_set_item *currentCell = &stats->awb_raw_buffer.meta_data[cellPosition]; + if (currentCell->sat_ratio == 0) { /* The cell is not saturated, use the current cell */ awbStats_[awbRegionPosition].counted++; - uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg; + uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg; awbStats_[awbRegionPosition].sum.green += greenValue / 2; - awbStats_[awbRegionPosition].sum.red += currentCell->redAvg; - awbStats_[awbRegionPosition].sum.blue += currentCell->blueAvg; + awbStats_[awbRegionPosition].sum.red += currentCell->R_avg; + awbStats_[awbRegionPosition].sum.blue += currentCell->B_avg; } } } diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index 3385ebe7..17905ae8 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -23,16 +23,6 @@ namespace ipa::ipu3::algorithms { static constexpr uint32_t kAwbStatsSizeX = 16; static constexpr uint32_t kAwbStatsSizeY = 12; -/* \todo Move the cell layout into intel-ipu3.h kernel header */ -struct Ipu3AwbCell { - unsigned char greenRedAvg; - unsigned char redAvg; - unsigned char blueAvg; - unsigned char greenBlueAvg; - unsigned char satRatio; - unsigned char padding[3]; -} __attribute__((packed)); - struct Accumulator { unsigned int counted; struct { From patchwork Thu Sep 9 13:54:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13793 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 4488ABDB1D for ; Thu, 9 Sep 2021 13:55:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E1A2269179; Thu, 9 Sep 2021 15:54:59 +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="nZW8qNQu"; 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 148F169174 for ; Thu, 9 Sep 2021 15:54:55 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:ab35:be36:579e:39cf]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BD98E883; Thu, 9 Sep 2021 15:54:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631195694; bh=6J+lMhFZi8TE/AV+xB1YgdHE096t06sor6UQb+0IQNI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nZW8qNQuvpoc1pVtn6H3yf/z1Me2qrqOXtfhxHpRfGku+Be2Da6nH2zASlam8PSD7 ba71rSrWdUZKolsWxwYRdR12OycRkanxRKcK/yInAgEdouEw/LEBhoPQ2uqrrSxtVE Ti5T2KS23B93sW74FCkRvgtq8XUnY5yNHz8KAVbU= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Thu, 9 Sep 2021 15:54:49 +0200 Message-Id: <20210909135449.68017-6-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> References: <20210909135449.68017-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v6 5/5] ipa: ipu3: awb: Make the naming consistent 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 variables mix the terms cell, region and zone. It can confuse the reader, and make the algorithm more difficult to follow. Rename the local variables consistent with their definitions: - Cells are defined in Pixels - Zones are defined in Cells Their is no "region" as such, so replace it with the correct term. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham --- src/ipa/ipu3/algorithms/awb.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index cf97208f..fd68b359 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -222,30 +222,30 @@ void Awb::generateZones(std::vector &zones) void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) { - uint32_t regionWidth = round(grid.width / static_cast(kAwbStatsSizeX)); - uint32_t regionHeight = round(grid.height / static_cast(kAwbStatsSizeY)); + uint32_t cellWidth = round(grid.width / static_cast(kAwbStatsSizeX)); + uint32_t cellHeight = round(grid.height / static_cast(kAwbStatsSizeY)); /* * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is * (grid.width x grid.height). */ - for (unsigned int j = 0; j < kAwbStatsSizeY * regionHeight; j++) { - for (unsigned int i = 0; i < kAwbStatsSizeX * regionWidth; i++) { + for (unsigned int j = 0; j < kAwbStatsSizeY * cellHeight; j++) { + for (unsigned int i = 0; i < kAwbStatsSizeX * cellWidth; i++) { uint32_t cellPosition = j * grid.width + i; - uint32_t cellX = (cellPosition / regionWidth) % kAwbStatsSizeX; - uint32_t cellY = ((cellPosition / grid.width) / regionHeight) % kAwbStatsSizeY; + uint32_t cellX = (cellPosition / cellWidth) % kAwbStatsSizeX; + uint32_t cellY = ((cellPosition / grid.width) / cellHeight) % kAwbStatsSizeY; - uint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX; + uint32_t awbZonePosition = cellY * kAwbStatsSizeX + cellX; /* Cast the initial IPU3 structure to simplify the reading */ const ipu3_uapi_awb_set_item *currentCell = &stats->awb_raw_buffer.meta_data[cellPosition]; if (currentCell->sat_ratio == 0) { /* The cell is not saturated, use the current cell */ - awbStats_[awbRegionPosition].counted++; + awbStats_[awbZonePosition].counted++; uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg; - awbStats_[awbRegionPosition].sum.green += greenValue / 2; - awbStats_[awbRegionPosition].sum.red += currentCell->R_avg; - awbStats_[awbRegionPosition].sum.blue += currentCell->B_avg; + awbStats_[awbZonePosition].sum.green += greenValue / 2; + awbStats_[awbZonePosition].sum.red += currentCell->R_avg; + awbStats_[awbZonePosition].sum.blue += currentCell->B_avg; } } }