[libcamera-devel] [PATCH v2 12/12] ipa: ipu3: Replace ipa::ipu3::algorithms::Ipu3AwbCell

Jean-Michel Hautbois jeanmichel.hautbois at ideasonboard.com
Thu Sep 30 11:37:15 CEST 2021


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/20210930092021.65741-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 <jeanmichel.hautbois at ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 include/linux/intel-ipu3.h      | 31 +++++++++++++++++++++----
 src/ipa/ipu3/algorithms/agc.cpp | 14 +++++------
 src/ipa/ipu3/algorithms/awb.cpp | 41 ++++++---------------------------
 src/ipa/ipu3/algorithms/awb.h   | 10 --------
 4 files changed, 40 insertions(+), 56 deletions(-)

diff --git a/include/linux/intel-ipu3.h b/include/linux/intel-ipu3.h
index ee0e6d0e..a5a71ca8 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.
@@ -71,9 +94,9 @@ struct ipu3_uapi_grid_config {
 	(IPU3_UAPI_MAX_BUBBLE_SIZE * IPU3_UAPI_MAX_STRIPES * \
 	 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_MAX_SETS * \
+	 (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) / \
+	 sizeof(struct ipu3_uapi_awb_set_item))
 
 /**
  * struct ipu3_uapi_awb_raw_buffer - AWB raw buffer
@@ -82,7 +105,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 46e01fc4..44008632 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -6,7 +6,6 @@
  */
 
 #include "agc.h"
-#include "awb.h"
 
 #include <algorithm>
 #include <chrono>
@@ -73,18 +72,17 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats,
 
 	for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
 		for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
-			uint32_t cellPosition = cellY * stride_ + cellX
-					      * sizeof(Ipu3AwbCell);
+			uint32_t cellPosition = cellY * stride_ + cellX;
 
 			/* Cast the initial IPU3 structure to simplify the reading */
-			const Ipu3AwbCell *cell =
-				reinterpret_cast<const Ipu3AwbCell *>(
+			const ipu3_uapi_awb_set_item *cell =
+				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
 					&stats->awb_raw_buffer.meta_data[cellPosition]
 				);
 
-			if (cell->satRatio == 0) {
-				uint8_t gr = cell->greenRedAvg;
-				uint8_t gb = cell->greenBlueAvg;
+			if (cell->sat_ratio == 0) {
+				uint8_t gr = cell->Gr_avg;
+				uint8_t gb = cell->Gb_avg;
 				hist[(gr + gb) / 2]++;
 			}
 		}
diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
index fcd1469f..4558f85a 100644
--- a/src/ipa/ipu3/algorithms/awb.cpp
+++ b/src/ipa/ipu3/algorithms/awb.cpp
@@ -104,32 +104,6 @@ static constexpr uint32_t kMinGreenLevelInZone = 32;
  * \brief Gain calculated for the blue channel
  */
 
-/**
- * \struct Ipu3AwbCell
- * \brief Memory layout for each cell in AWB metadata
- *
- * The Ipu3AwbCell structure is used to get individual values
- * such as red average or saturation ratio in a particular cell.
- *
- * \var Ipu3AwbCell::greenRedAvg
- * \brief Green average for red lines in the cell
- *
- * \var Ipu3AwbCell::redAvg
- * \brief Red average in the cell
- *
- * \var Ipu3AwbCell::blueAvg
- * \brief blue average in the cell
- *
- * \var Ipu3AwbCell::greenBlueAvg
- * \brief Green average for blue lines
- *
- * \var Ipu3AwbCell::satRatio
- * \brief Saturation ratio in the cell
- *
- * \var Ipu3AwbCell::padding
- * \brief array of unused bytes for padding
- */
-
 /* Default settings for Bayer noise reduction replicated from the Kernel */
 static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {
 	.wb_gains = { 16, 16, 16, 16 },
@@ -243,25 +217,24 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
 	 */
 	for (unsigned int cellY = 0; cellY < kAwbStatsSizeY * cellsPerZoneY_; cellY++) {
 		for (unsigned int cellX = 0; cellX < kAwbStatsSizeX * cellsPerZoneX_; cellX++) {
-			uint32_t cellPosition = (cellY * stride_ + cellX)
-					      * sizeof(Ipu3AwbCell);
+			uint32_t cellPosition = cellY * stride_ + cellX;
 			uint32_t zoneX = cellX / cellsPerZoneX_;
 			uint32_t zoneY = cellY / cellsPerZoneY_;
 
 			uint32_t awbZonePosition = zoneY * kAwbStatsSizeX + zoneX;
 
 			/* Cast the initial IPU3 structure to simplify the reading */
-			const Ipu3AwbCell *currentCell =
-				reinterpret_cast<const Ipu3AwbCell *>(
+			const ipu3_uapi_awb_set_item *currentCell =
+				reinterpret_cast<const ipu3_uapi_awb_set_item *>(
 					&stats->awb_raw_buffer.meta_data[cellPosition]
 				);
-			if (currentCell->satRatio == 0) {
+			if (currentCell->sat_ratio == 0) {
 				/* The cell is not saturated, use the current cell */
 				awbStats_[awbZonePosition].counted++;
-				uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg;
+				uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg;
 				awbStats_[awbZonePosition].sum.green += greenValue / 2;
-				awbStats_[awbZonePosition].sum.red += currentCell->redAvg;
-				awbStats_[awbZonePosition].sum.blue += currentCell->blueAvg;
+				awbStats_[awbZonePosition].sum.red += currentCell->R_avg;
+				awbStats_[awbZonePosition].sum.blue += currentCell->B_avg;
 			}
 		}
 	}
diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
index b3e0ad82..677384ed 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 {
-- 
2.30.2



More information about the libcamera-devel mailing list