[libcamera-devel,09/12] ipa: ipu3: awb: Use the line stride for the stats
diff mbox series

Message ID 20210923081625.60276-10-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • Improve ImgU statistics usage
Related show

Commit Message

Jean-Michel Hautbois Sept. 23, 2021, 8:16 a.m. UTC 
The statistics buffer 'ipu3_uapi_awb_raw_buffer' stores the ImgU
calculation results in a buffer aligned to a multiple of 4. The AWB loop
should take care of it to add the proper offset between lines and avoid
any staircase effect.

It is now no more required to pass the grid configuration context to the
private functions called from process() which simplifies the code flow.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/awb.cpp | 16 ++++++++--------
 src/ipa/ipu3/algorithms/awb.h   |  7 +++----
 2 files changed, 11 insertions(+), 12 deletions(-)

Comments

Kieran Bingham Sept. 28, 2021, 4:02 p.m. UTC | #1 
On Thu, Sep 23, 2021 at 10:16:22AM +0200, Jean-Michel Hautbois wrote:
> The statistics buffer 'ipu3_uapi_awb_raw_buffer' stores the ImgU
> calculation results in a buffer aligned to a multiple of 4. The AWB loop
> should take care of it to add the proper offset between lines and avoid
> any staircase effect.
> 
> It is now no more required to pass the grid configuration context to the

"It is no longer required to ..."

That's a nice perk indeed.

> private functions called from process() which simplifies the code flow.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/awb.cpp | 16 ++++++++--------
>  src/ipa/ipu3/algorithms/awb.h   |  7 +++----
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> index 8a926691..a5391653 100644
> --- a/src/ipa/ipu3/algorithms/awb.cpp
> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> @@ -172,8 +172,10 @@ int Awb::configure(IPAContext &context,
>  		   [[maybe_unused]] const IPAConfigInfo &configInfo)
>  {
>  	const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
> +	/* The grid is aligned to the next multiple of 4 */
> +	stride_ = (grid.width + 3) / 4 * 4;

Feels like something that could have been handled by an alignedUp macro.
We have that in geometry.h but for Size ...

So perhaps we can just open code it (unless anyone knows of a
std:: macro that does this?)


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  
> -	cellsPerZoneX_ = round(grid.width / static_cast<double>(kAwbStatsSizeX));
> +	cellsPerZoneX_ = round(stride_ / static_cast<double>(kAwbStatsSizeX));
>  	cellsPerZoneY_ = round(grid.height / static_cast<double>(kAwbStatsSizeY));
>  
>  	/*
> @@ -234,8 +236,7 @@ void Awb::generateZones(std::vector<RGB> &zones)
>  }
>  
>  /* 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)
> +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
>  {
>  	/*
>  	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
> @@ -243,7 +244,7 @@ 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 * grid.width + cellX)
> +			uint32_t cellPosition = (cellY * stride_ + cellX)
>  					      * sizeof(Ipu3AwbCell);
>  			uint32_t zoneX = cellX / cellsPerZoneX_;
>  			uint32_t zoneY = cellY / cellsPerZoneY_;
> @@ -318,13 +319,12 @@ void Awb::awbGreyWorld()
>  	asyncResults_.blueGain = blueGain;
>  }
>  
> -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
> -			   const ipu3_uapi_grid_config &grid)
> +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
>  {
>  	ASSERT(stats->stats_3a_status.awb_en);
>  	zones_.clear();
>  	clearAwbStats();
> -	generateAwbStats(stats, grid);
> +	generateAwbStats(stats);
>  	generateZones(zones_);
>  	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
>  	if (zones_.size() > 10) {
> @@ -336,7 +336,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
>  
>  void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>  {
> -	calculateWBGains(stats, context.configuration.grid.bdsGrid);
> +	calculateWBGains(stats);
>  
>  	/*
>  	 * Gains are only recalculated if enough zones were detected.
> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> index 681d8c2b..b3e0ad82 100644
> --- a/src/ipa/ipu3/algorithms/awb.h
> +++ b/src/ipa/ipu3/algorithms/awb.h
> @@ -74,11 +74,9 @@ public:
>  	};
>  
>  private:
> -	void calculateWBGains(const ipu3_uapi_stats_3a *stats,
> -			      const ipu3_uapi_grid_config &grid);
> +	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
>  	void generateZones(std::vector<RGB> &zones);
> -	void generateAwbStats(const ipu3_uapi_stats_3a *stats,
> -			      const ipu3_uapi_grid_config &grid);
> +	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
>  	void clearAwbStats();
>  	void awbGreyWorld();
>  	uint32_t estimateCCT(double red, double green, double blue);
> @@ -87,6 +85,7 @@ private:
>  	Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
>  	AwbStatus asyncResults_;
>  
> +	uint32_t stride_;
>  	uint32_t cellsPerZoneX_;
>  	uint32_t cellsPerZoneY_;
>  	uint32_t cellsPerZoneThreshold_;
> -- 
> 2.30.2
> 

--
Kieran
Laurent Pinchart Sept. 29, 2021, 12:44 p.m. UTC | #2 
Hello,

On Tue, Sep 28, 2021 at 05:02:42PM +0100, Kieran Bingham wrote:
> On Thu, Sep 23, 2021 at 10:16:22AM +0200, Jean-Michel Hautbois wrote:
> > The statistics buffer 'ipu3_uapi_awb_raw_buffer' stores the ImgU
> > calculation results in a buffer aligned to a multiple of 4. The AWB loop

s/aligned to a multiple of 4/aligned horizontally to a multiple of 4 cells/

> > should take care of it to add the proper offset between lines and avoid
> > any staircase effect.
> > 
> > It is now no more required to pass the grid configuration context to the
> 
> "It is no longer required to ..."
> 
> That's a nice perk indeed.
> 
> > private functions called from process() which simplifies the code flow.
> > 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > ---
> >  src/ipa/ipu3/algorithms/awb.cpp | 16 ++++++++--------
> >  src/ipa/ipu3/algorithms/awb.h   |  7 +++----
> >  2 files changed, 11 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> > index 8a926691..a5391653 100644
> > --- a/src/ipa/ipu3/algorithms/awb.cpp
> > +++ b/src/ipa/ipu3/algorithms/awb.cpp
> > @@ -172,8 +172,10 @@ int Awb::configure(IPAContext &context,
> >  		   [[maybe_unused]] const IPAConfigInfo &configInfo)
> >  {
> >  	const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
> > +	/* The grid is aligned to the next multiple of 4 */

s/grid/grid width/

> > +	stride_ = (grid.width + 3) / 4 * 4;
> 
> Feels like something that could have been handled by an alignedUp macro.
> We have that in geometry.h but for Size ...
> 
> So perhaps we can just open code it (unless anyone knows of a
> std:: macro that does this?)

utils::alignUp()

> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > -	cellsPerZoneX_ = round(grid.width / static_cast<double>(kAwbStatsSizeX));
> > +	cellsPerZoneX_ = round(stride_ / static_cast<double>(kAwbStatsSizeX));

This isn't right, you will end up using the padding cells in your
calculations.

> >  	cellsPerZoneY_ = round(grid.height / static_cast<double>(kAwbStatsSizeY));
> >  
> >  	/*
> > @@ -234,8 +236,7 @@ void Awb::generateZones(std::vector<RGB> &zones)
> >  }
> >  
> >  /* 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)
> > +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
> >  {
> >  	/*
> >  	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
> > @@ -243,7 +244,7 @@ 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 * grid.width + cellX)
> > +			uint32_t cellPosition = (cellY * stride_ + cellX)
> >  					      * sizeof(Ipu3AwbCell);
> >  			uint32_t zoneX = cellX / cellsPerZoneX_;
> >  			uint32_t zoneY = cellY / cellsPerZoneY_;
> > @@ -318,13 +319,12 @@ void Awb::awbGreyWorld()
> >  	asyncResults_.blueGain = blueGain;
> >  }
> >  
> > -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
> > -			   const ipu3_uapi_grid_config &grid)
> > +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> >  {
> >  	ASSERT(stats->stats_3a_status.awb_en);
> >  	zones_.clear();
> >  	clearAwbStats();
> > -	generateAwbStats(stats, grid);
> > +	generateAwbStats(stats);
> >  	generateZones(zones_);
> >  	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
> >  	if (zones_.size() > 10) {
> > @@ -336,7 +336,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
> >  
> >  void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >  {
> > -	calculateWBGains(stats, context.configuration.grid.bdsGrid);
> > +	calculateWBGains(stats);
> >  
> >  	/*
> >  	 * Gains are only recalculated if enough zones were detected.
> > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> > index 681d8c2b..b3e0ad82 100644
> > --- a/src/ipa/ipu3/algorithms/awb.h
> > +++ b/src/ipa/ipu3/algorithms/awb.h
> > @@ -74,11 +74,9 @@ public:
> >  	};
> >  
> >  private:
> > -	void calculateWBGains(const ipu3_uapi_stats_3a *stats,
> > -			      const ipu3_uapi_grid_config &grid);
> > +	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> >  	void generateZones(std::vector<RGB> &zones);
> > -	void generateAwbStats(const ipu3_uapi_stats_3a *stats,
> > -			      const ipu3_uapi_grid_config &grid);
> > +	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
> >  	void clearAwbStats();
> >  	void awbGreyWorld();
> >  	uint32_t estimateCCT(double red, double green, double blue);
> > @@ -87,6 +85,7 @@ private:
> >  	Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
> >  	AwbStatus asyncResults_;
> >  
> > +	uint32_t stride_;
> >  	uint32_t cellsPerZoneX_;
> >  	uint32_t cellsPerZoneY_;
> >  	uint32_t cellsPerZoneThreshold_;
Jean-Michel Hautbois Sept. 29, 2021, 12:47 p.m. UTC | #3 
Hi Laurent,

On 29/09/2021 14:44, Laurent Pinchart wrote:
> Hello,
> 
> On Tue, Sep 28, 2021 at 05:02:42PM +0100, Kieran Bingham wrote:
>> On Thu, Sep 23, 2021 at 10:16:22AM +0200, Jean-Michel Hautbois wrote:
>>> The statistics buffer 'ipu3_uapi_awb_raw_buffer' stores the ImgU
>>> calculation results in a buffer aligned to a multiple of 4. The AWB loop
> 
> s/aligned to a multiple of 4/aligned horizontally to a multiple of 4 cells/
> 
>>> should take care of it to add the proper offset between lines and avoid
>>> any staircase effect.
>>>
>>> It is now no more required to pass the grid configuration context to the
>>
>> "It is no longer required to ..."
>>
>> That's a nice perk indeed.
>>
>>> private functions called from process() which simplifies the code flow.
>>>
>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
>>> ---
>>>  src/ipa/ipu3/algorithms/awb.cpp | 16 ++++++++--------
>>>  src/ipa/ipu3/algorithms/awb.h   |  7 +++----
>>>  2 files changed, 11 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
>>> index 8a926691..a5391653 100644
>>> --- a/src/ipa/ipu3/algorithms/awb.cpp
>>> +++ b/src/ipa/ipu3/algorithms/awb.cpp
>>> @@ -172,8 +172,10 @@ int Awb::configure(IPAContext &context,
>>>  		   [[maybe_unused]] const IPAConfigInfo &configInfo)
>>>  {
>>>  	const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
>>> +	/* The grid is aligned to the next multiple of 4 */
> 
> s/grid/grid width/
> 
>>> +	stride_ = (grid.width + 3) / 4 * 4;
>>
>> Feels like something that could have been handled by an alignedUp macro.
>> We have that in geometry.h but for Size ...
>>
>> So perhaps we can just open code it (unless anyone knows of a
>> std:: macro that does this?)
> 
> utils::alignUp()
> 
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>>> -	cellsPerZoneX_ = round(grid.width / static_cast<double>(kAwbStatsSizeX));
>>> +	cellsPerZoneX_ = round(stride_ / static_cast<double>(kAwbStatsSizeX));
> 
> This isn't right, you will end up using the padding cells in your
> calculations.
> 

The padding cell is all black, so it will not be taken into account in
Awb::generateZones() as the green value will be lower than
kMinGreenLevelInZone.

>>>  	cellsPerZoneY_ = round(grid.height / static_cast<double>(kAwbStatsSizeY));
>>>  
>>>  	/*
>>> @@ -234,8 +236,7 @@ void Awb::generateZones(std::vector<RGB> &zones)
>>>  }
>>>  
>>>  /* 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)
>>> +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
>>>  {
>>>  	/*
>>>  	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
>>> @@ -243,7 +244,7 @@ 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 * grid.width + cellX)
>>> +			uint32_t cellPosition = (cellY * stride_ + cellX)
>>>  					      * sizeof(Ipu3AwbCell);
>>>  			uint32_t zoneX = cellX / cellsPerZoneX_;
>>>  			uint32_t zoneY = cellY / cellsPerZoneY_;
>>> @@ -318,13 +319,12 @@ void Awb::awbGreyWorld()
>>>  	asyncResults_.blueGain = blueGain;
>>>  }
>>>  
>>> -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
>>> -			   const ipu3_uapi_grid_config &grid)
>>> +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
>>>  {
>>>  	ASSERT(stats->stats_3a_status.awb_en);
>>>  	zones_.clear();
>>>  	clearAwbStats();
>>> -	generateAwbStats(stats, grid);
>>> +	generateAwbStats(stats);
>>>  	generateZones(zones_);
>>>  	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
>>>  	if (zones_.size() > 10) {
>>> @@ -336,7 +336,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
>>>  
>>>  void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>>>  {
>>> -	calculateWBGains(stats, context.configuration.grid.bdsGrid);
>>> +	calculateWBGains(stats);
>>>  
>>>  	/*
>>>  	 * Gains are only recalculated if enough zones were detected.
>>> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
>>> index 681d8c2b..b3e0ad82 100644
>>> --- a/src/ipa/ipu3/algorithms/awb.h
>>> +++ b/src/ipa/ipu3/algorithms/awb.h
>>> @@ -74,11 +74,9 @@ public:
>>>  	};
>>>  
>>>  private:
>>> -	void calculateWBGains(const ipu3_uapi_stats_3a *stats,
>>> -			      const ipu3_uapi_grid_config &grid);
>>> +	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
>>>  	void generateZones(std::vector<RGB> &zones);
>>> -	void generateAwbStats(const ipu3_uapi_stats_3a *stats,
>>> -			      const ipu3_uapi_grid_config &grid);
>>> +	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
>>>  	void clearAwbStats();
>>>  	void awbGreyWorld();
>>>  	uint32_t estimateCCT(double red, double green, double blue);
>>> @@ -87,6 +85,7 @@ private:
>>>  	Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
>>>  	AwbStatus asyncResults_;
>>>  
>>> +	uint32_t stride_;
>>>  	uint32_t cellsPerZoneX_;
>>>  	uint32_t cellsPerZoneY_;
>>>  	uint32_t cellsPerZoneThreshold_;
>
Laurent Pinchart Sept. 29, 2021, 12:52 p.m. UTC | #4 
On Wed, Sep 29, 2021 at 02:47:15PM +0200, Jean-Michel Hautbois wrote:
> On 29/09/2021 14:44, Laurent Pinchart wrote:
> > On Tue, Sep 28, 2021 at 05:02:42PM +0100, Kieran Bingham wrote:
> >> On Thu, Sep 23, 2021 at 10:16:22AM +0200, Jean-Michel Hautbois wrote:
> >>> The statistics buffer 'ipu3_uapi_awb_raw_buffer' stores the ImgU
> >>> calculation results in a buffer aligned to a multiple of 4. The AWB loop
> > 
> > s/aligned to a multiple of 4/aligned horizontally to a multiple of 4 cells/
> > 
> >>> should take care of it to add the proper offset between lines and avoid
> >>> any staircase effect.
> >>>
> >>> It is now no more required to pass the grid configuration context to the
> >>
> >> "It is no longer required to ..."
> >>
> >> That's a nice perk indeed.
> >>
> >>> private functions called from process() which simplifies the code flow.
> >>>
> >>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> >>> ---
> >>>  src/ipa/ipu3/algorithms/awb.cpp | 16 ++++++++--------
> >>>  src/ipa/ipu3/algorithms/awb.h   |  7 +++----
> >>>  2 files changed, 11 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
> >>> index 8a926691..a5391653 100644
> >>> --- a/src/ipa/ipu3/algorithms/awb.cpp
> >>> +++ b/src/ipa/ipu3/algorithms/awb.cpp
> >>> @@ -172,8 +172,10 @@ int Awb::configure(IPAContext &context,
> >>>  		   [[maybe_unused]] const IPAConfigInfo &configInfo)
> >>>  {
> >>>  	const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
> >>> +	/* The grid is aligned to the next multiple of 4 */
> > 
> > s/grid/grid width/
> > 
> >>> +	stride_ = (grid.width + 3) / 4 * 4;
> >>
> >> Feels like something that could have been handled by an alignedUp macro.
> >> We have that in geometry.h but for Size ...
> >>
> >> So perhaps we can just open code it (unless anyone knows of a
> >> std:: macro that does this?)
> > 
> > utils::alignUp()
> > 
> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >>
> >>> -	cellsPerZoneX_ = round(grid.width / static_cast<double>(kAwbStatsSizeX));
> >>> +	cellsPerZoneX_ = round(stride_ / static_cast<double>(kAwbStatsSizeX));
> > 
> > This isn't right, you will end up using the padding cells in your
> > calculations.
> 
> The padding cell is all black, so it will not be taken into account in
> Awb::generateZones() as the green value will be lower than
> kMinGreenLevelInZone.

That's not a documented guarantee, I don't think we should rely on this.

> >>>  	cellsPerZoneY_ = round(grid.height / static_cast<double>(kAwbStatsSizeY));
> >>>  
> >>>  	/*
> >>> @@ -234,8 +236,7 @@ void Awb::generateZones(std::vector<RGB> &zones)
> >>>  }
> >>>  
> >>>  /* 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)
> >>> +void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
> >>>  {
> >>>  	/*
> >>>  	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
> >>> @@ -243,7 +244,7 @@ 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 * grid.width + cellX)
> >>> +			uint32_t cellPosition = (cellY * stride_ + cellX)
> >>>  					      * sizeof(Ipu3AwbCell);
> >>>  			uint32_t zoneX = cellX / cellsPerZoneX_;
> >>>  			uint32_t zoneY = cellY / cellsPerZoneY_;
> >>> @@ -318,13 +319,12 @@ void Awb::awbGreyWorld()
> >>>  	asyncResults_.blueGain = blueGain;
> >>>  }
> >>>  
> >>> -void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
> >>> -			   const ipu3_uapi_grid_config &grid)
> >>> +void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
> >>>  {
> >>>  	ASSERT(stats->stats_3a_status.awb_en);
> >>>  	zones_.clear();
> >>>  	clearAwbStats();
> >>> -	generateAwbStats(stats, grid);
> >>> +	generateAwbStats(stats);
> >>>  	generateZones(zones_);
> >>>  	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
> >>>  	if (zones_.size() > 10) {
> >>> @@ -336,7 +336,7 @@ void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
> >>>  
> >>>  void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >>>  {
> >>> -	calculateWBGains(stats, context.configuration.grid.bdsGrid);
> >>> +	calculateWBGains(stats);
> >>>  
> >>>  	/*
> >>>  	 * Gains are only recalculated if enough zones were detected.
> >>> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
> >>> index 681d8c2b..b3e0ad82 100644
> >>> --- a/src/ipa/ipu3/algorithms/awb.h
> >>> +++ b/src/ipa/ipu3/algorithms/awb.h
> >>> @@ -74,11 +74,9 @@ public:
> >>>  	};
> >>>  
> >>>  private:
> >>> -	void calculateWBGains(const ipu3_uapi_stats_3a *stats,
> >>> -			      const ipu3_uapi_grid_config &grid);
> >>> +	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> >>>  	void generateZones(std::vector<RGB> &zones);
> >>> -	void generateAwbStats(const ipu3_uapi_stats_3a *stats,
> >>> -			      const ipu3_uapi_grid_config &grid);
> >>> +	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
> >>>  	void clearAwbStats();
> >>>  	void awbGreyWorld();
> >>>  	uint32_t estimateCCT(double red, double green, double blue);
> >>> @@ -87,6 +85,7 @@ private:
> >>>  	Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
> >>>  	AwbStatus asyncResults_;
> >>>  
> >>> +	uint32_t stride_;
> >>>  	uint32_t cellsPerZoneX_;
> >>>  	uint32_t cellsPerZoneY_;
> >>>  	uint32_t cellsPerZoneThreshold_;

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp
index 8a926691..a5391653 100644
--- a/src/ipa/ipu3/algorithms/awb.cpp
+++ b/src/ipa/ipu3/algorithms/awb.cpp
@@ -172,8 +172,10 @@  int Awb::configure(IPAContext &context,
 		   [[maybe_unused]] const IPAConfigInfo &configInfo)
 {
 	const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid;
+	/* The grid is aligned to the next multiple of 4 */
+	stride_ = (grid.width + 3) / 4 * 4;
 
-	cellsPerZoneX_ = round(grid.width / static_cast<double>(kAwbStatsSizeX));
+	cellsPerZoneX_ = round(stride_ / static_cast<double>(kAwbStatsSizeX));
 	cellsPerZoneY_ = round(grid.height / static_cast<double>(kAwbStatsSizeY));
 
 	/*
@@ -234,8 +236,7 @@  void Awb::generateZones(std::vector<RGB> &zones)
 }
 
 /* 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)
+void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats)
 {
 	/*
 	 * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is
@@ -243,7 +244,7 @@  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 * grid.width + cellX)
+			uint32_t cellPosition = (cellY * stride_ + cellX)
 					      * sizeof(Ipu3AwbCell);
 			uint32_t zoneX = cellX / cellsPerZoneX_;
 			uint32_t zoneY = cellY / cellsPerZoneY_;
@@ -318,13 +319,12 @@  void Awb::awbGreyWorld()
 	asyncResults_.blueGain = blueGain;
 }
 
-void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
-			   const ipu3_uapi_grid_config &grid)
+void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
 {
 	ASSERT(stats->stats_3a_status.awb_en);
 	zones_.clear();
 	clearAwbStats();
-	generateAwbStats(stats, grid);
+	generateAwbStats(stats);
 	generateZones(zones_);
 	LOG(IPU3Awb, Debug) << "Valid zones: " << zones_.size();
 	if (zones_.size() > 10) {
@@ -336,7 +336,7 @@  void Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats,
 
 void Awb::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
 {
-	calculateWBGains(stats, context.configuration.grid.bdsGrid);
+	calculateWBGains(stats);
 
 	/*
 	 * Gains are only recalculated if enough zones were detected.
diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h
index 681d8c2b..b3e0ad82 100644
--- a/src/ipa/ipu3/algorithms/awb.h
+++ b/src/ipa/ipu3/algorithms/awb.h
@@ -74,11 +74,9 @@  public:
 	};
 
 private:
-	void calculateWBGains(const ipu3_uapi_stats_3a *stats,
-			      const ipu3_uapi_grid_config &grid);
+	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
 	void generateZones(std::vector<RGB> &zones);
-	void generateAwbStats(const ipu3_uapi_stats_3a *stats,
-			      const ipu3_uapi_grid_config &grid);
+	void generateAwbStats(const ipu3_uapi_stats_3a *stats);
 	void clearAwbStats();
 	void awbGreyWorld();
 	uint32_t estimateCCT(double red, double green, double blue);
@@ -87,6 +85,7 @@  private:
 	Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY];
 	AwbStatus asyncResults_;
 
+	uint32_t stride_;
 	uint32_t cellsPerZoneX_;
 	uint32_t cellsPerZoneY_;
 	uint32_t cellsPerZoneThreshold_;