[libcamera-devel,v1,02/10] ipa: raspberrypi: Add hardware configuration to the controller
diff mbox series

Message ID 20230322130612.5208-3-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi: Generalised algorithms
Related show

Commit Message

Naushir Patuck March 22, 2023, 1:06 p.m. UTC 
Add a new Controller::HardwareConfig structure that captures the
hardware statistics grid/histogram sizes and pipeline widths. This
ensures there is a single centralised places for these parameters.

Add a getHardwareConfig() helper function to retrieve these values for a
given hardware target.

Update the statistics populating routine in the IPA to use the values
from this structure instead of the hardcoded numbers.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/algorithm.h    |  4 ++
 src/ipa/raspberrypi/controller/controller.cpp | 39 +++++++++++++++++++
 src/ipa/raspberrypi/controller/controller.h   | 11 ++++++
 src/ipa/raspberrypi/raspberrypi.cpp           | 21 ++++------
 4 files changed, 62 insertions(+), 13 deletions(-)

Comments

Jacopo Mondi March 23, 2023, 4:11 p.m. UTC | #1 
Hi Naush

On Wed, Mar 22, 2023 at 01:06:04PM +0000, Naushir Patuck via libcamera-devel wrote:
> Add a new Controller::HardwareConfig structure that captures the
> hardware statistics grid/histogram sizes and pipeline widths. This
> ensures there is a single centralised places for these parameters.
>
> Add a getHardwareConfig() helper function to retrieve these values for a
> given hardware target.
>
> Update the statistics populating routine in the IPA to use the values
> from this structure instead of the hardcoded numbers.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/raspberrypi/controller/algorithm.h    |  4 ++
>  src/ipa/raspberrypi/controller/controller.cpp | 39 +++++++++++++++++++
>  src/ipa/raspberrypi/controller/controller.h   | 11 ++++++
>  src/ipa/raspberrypi/raspberrypi.cpp           | 21 ++++------
>  4 files changed, 62 insertions(+), 13 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h
> index 7c22fbe4945c..4aa814ebbebd 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.h
> +++ b/src/ipa/raspberrypi/controller/algorithm.h
> @@ -45,6 +45,10 @@ public:
>  	{
>  		return controller_->getTarget();
>  	}
> +	const Controller::HardwareConfig &getHardwareConfig() const
> +	{
> +		return controller_->getHardwareConfig();
> +	}
>

Will this be used ?

>  private:
>  	Controller *controller_;
> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
> index a6250ee140b0..2c7517aec6b4 100644
> --- a/src/ipa/raspberrypi/controller/controller.cpp
> +++ b/src/ipa/raspberrypi/controller/controller.cpp
> @@ -20,6 +20,31 @@ using namespace libcamera;
>
>  LOG_DEFINE_CATEGORY(RPiController)
>
> +static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap = {
> +	{
> +		"bcm2835",
> +		{
> +			/*
> +			 * There are only ever 15 AGC regions computed by the firmware
> +			 * due to zoning, but the HW defines AGC_REGIONS == 16!
> +			 */
> +			.agcRegions = { 15 , 1 },
> +			.agcZoneWeights = { 15 , 1 },
> +			.awbRegions = { 16, 12 },
> +			.focusRegions = { 4, 3 },
> +			.numHistogramBins = 128,
> +			.numGammaPoints = 33,
> +			.pipelineWidth = 13
> +		}
> +	},
> +	/* For error handling. */
> +	{
> +		"error",
> +		{
> +		}
> +	}

Do you need this ?

> +};
> +
>  Controller::Controller()
>  	: switchModeCalled_(false)
>  {
> @@ -148,3 +173,17 @@ const std::string &Controller::getTarget() const
>  {
>  	return target_;
>  }
> +
> +const Controller::HardwareConfig &Controller::getHardwareConfig() const
> +{
> +	auto cfg = HardwareConfigMap.find(getTarget());
> +
> +	/*
> +	 * This really should not happen, the IPA ought to validate the target
> +	 * on initialisation.
> +	 */
> +	if (cfg == HardwareConfigMap.end())
> +		return HardwareConfigMap.at("error");

Could you just return {} here and remove "error" from the map ?

> +
> +	return cfg->second;
> +}
> diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h
> index 24e02903d438..c6af5cd6c7d4 100644
> --- a/src/ipa/raspberrypi/controller/controller.h
> +++ b/src/ipa/raspberrypi/controller/controller.h
> @@ -37,6 +37,16 @@ typedef std::unique_ptr<Algorithm> AlgorithmPtr;
>  class Controller
>  {
>  public:
> +	struct HardwareConfig {
> +		libcamera::Size agcRegions;
> +		libcamera::Size agcZoneWeights;
> +		libcamera::Size awbRegions;
> +		libcamera::Size focusRegions;
> +		unsigned int numHistogramBins;
> +		unsigned int numGammaPoints;
> +		unsigned int pipelineWidth;
> +	};
> +
>  	Controller();
>  	~Controller();
>  	int read(char const *filename);
> @@ -47,6 +57,7 @@ public:
>  	Metadata &getGlobalMetadata();
>  	Algorithm *getAlgorithm(std::string const &name) const;
>  	const std::string &getTarget() const;
> +	const HardwareConfig &getHardwareConfig() const;
>
>  protected:
>  	int createAlgorithm(const std::string &name, const libcamera::YamlObject &params);
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 86359538cf67..b64cb96e2dde 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -1392,20 +1392,19 @@ RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
>  {
>  	using namespace RPiController;
>
> +	const Controller::HardwareConfig &hw = controller_.getHardwareConfig();
>  	unsigned int i;
>  	StatisticsPtr statistics =
>  		std::make_unique<Statistics>(Statistics::AgcStatsPos::PreWb, Statistics::ColourStatsPos::PostLsc);
>
>  	/* RGB histograms are not used, so do not populate them. */
> -	statistics->yHist = RPiController::Histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS);
> +	statistics->yHist = RPiController::Histogram(stats->hist[0].g_hist,
> +						     hw.numHistogramBins);
>
> -	/*
> -	 * All region sums are based on a 13-bit pipeline bit-depth. Normalise
> -	 * this to 16-bits for the AGC/AWB/ALSC algorithms.
> -	 */
> -	constexpr unsigned int scale = Statistics::NormalisationFactorPow2 - 13;
> +	/* All region sums are based on a 16-bit normalised pipeline bit-depth. */
> +	unsigned int scale = Statistics::NormalisationFactorPow2 - hw.pipelineWidth;
>
> -	statistics->awbRegions.init({ DEFAULT_AWB_REGIONS_X, DEFAULT_AWB_REGIONS_Y });
> +	statistics->awbRegions.init(hw.awbRegions);
>  	for (i = 0; i < statistics->awbRegions.numRegions(); i++)
>  		statistics->awbRegions.set(i, { { stats->awb_stats[i].r_sum << scale,
>  						  stats->awb_stats[i].g_sum << scale,
> @@ -1413,11 +1412,7 @@ RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
>  						stats->awb_stats[i].counted,
>  						stats->awb_stats[i].notcounted });
>
> -	/*
> -	 * There are only ever 15 regions computed by the firmware due to zoning,
> -	 * but the HW defines AGC_REGIONS == 16!
> -	 */
> -	statistics->agcRegions.init(15);
> +	statistics->agcRegions.init(hw.agcRegions);
>  	for (i = 0; i < statistics->agcRegions.numRegions(); i++)
>  		statistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum << scale,
>  						  stats->agc_stats[i].g_sum << scale,
> @@ -1425,7 +1420,7 @@ RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
>  						stats->agc_stats[i].counted,
>  						stats->awb_stats[i].notcounted });
>
> -	statistics->focusRegions.init({ 4, 3 });
> +	statistics->focusRegions.init(hw.focusRegions);
>  	for (i = 0; i < statistics->focusRegions.numRegions(); i++)
>  		statistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,
>  						  stats->focus_stats[i].contrast_val_num[1][1],
> --
> 2.34.1
>
Naushir Patuck March 24, 2023, 7:52 a.m. UTC | #2 
Hi Jacopo,

Thank you for your review!

On Thu, 23 Mar 2023 at 16:11, Jacopo Mondi <jacopo.mondi@ideasonboard.com>
wrote:

> Hi Naush
>
> On Wed, Mar 22, 2023 at 01:06:04PM +0000, Naushir Patuck via
> libcamera-devel wrote:
> > Add a new Controller::HardwareConfig structure that captures the
> > hardware statistics grid/histogram sizes and pipeline widths. This
> > ensures there is a single centralised places for these parameters.
> >
> > Add a getHardwareConfig() helper function to retrieve these values for a
> > given hardware target.
> >
> > Update the statistics populating routine in the IPA to use the values
> > from this structure instead of the hardcoded numbers.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/controller/algorithm.h    |  4 ++
> >  src/ipa/raspberrypi/controller/controller.cpp | 39 +++++++++++++++++++
> >  src/ipa/raspberrypi/controller/controller.h   | 11 ++++++
> >  src/ipa/raspberrypi/raspberrypi.cpp           | 21 ++++------
> >  4 files changed, 62 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/algorithm.h
> b/src/ipa/raspberrypi/controller/algorithm.h
> > index 7c22fbe4945c..4aa814ebbebd 100644
> > --- a/src/ipa/raspberrypi/controller/algorithm.h
> > +++ b/src/ipa/raspberrypi/controller/algorithm.h
> > @@ -45,6 +45,10 @@ public:
> >       {
> >               return controller_->getTarget();
> >       }
> > +     const Controller::HardwareConfig &getHardwareConfig() const
> > +     {
> > +             return controller_->getHardwareConfig();
> > +     }
> >
>
> Will this be used ?
>

Yes, in subsequent patches :)


>
> >  private:
> >       Controller *controller_;
> > diff --git a/src/ipa/raspberrypi/controller/controller.cpp
> b/src/ipa/raspberrypi/controller/controller.cpp
> > index a6250ee140b0..2c7517aec6b4 100644
> > --- a/src/ipa/raspberrypi/controller/controller.cpp
> > +++ b/src/ipa/raspberrypi/controller/controller.cpp
> > @@ -20,6 +20,31 @@ using namespace libcamera;
> >
> >  LOG_DEFINE_CATEGORY(RPiController)
> >
> > +static const std::map<std::string, Controller::HardwareConfig>
> HardwareConfigMap = {
> > +     {
> > +             "bcm2835",
> > +             {
> > +                     /*
> > +                      * There are only ever 15 AGC regions computed by
> the firmware
> > +                      * due to zoning, but the HW defines AGC_REGIONS
> == 16!
> > +                      */
> > +                     .agcRegions = { 15 , 1 },
> > +                     .agcZoneWeights = { 15 , 1 },
> > +                     .awbRegions = { 16, 12 },
> > +                     .focusRegions = { 4, 3 },
> > +                     .numHistogramBins = 128,
> > +                     .numGammaPoints = 33,
> > +                     .pipelineWidth = 13
> > +             }
> > +     },
> > +     /* For error handling. */
> > +     {
> > +             "error",
> > +             {
> > +             }
> > +     }
>
> Do you need this ?
>

Good question.  I think the answer is no.
We validate the platform on ipa init, so really we cannot get into this
error state.

I'll remove it in the next version.

Regards,
Naush



>
> > +};
> > +
> >  Controller::Controller()
> >       : switchModeCalled_(false)
> >  {
> > @@ -148,3 +173,17 @@ const std::string &Controller::getTarget() const
> >  {
> >       return target_;
> >  }
> > +
> > +const Controller::HardwareConfig &Controller::getHardwareConfig() const
> > +{
> > +     auto cfg = HardwareConfigMap.find(getTarget());
> > +
> > +     /*
> > +      * This really should not happen, the IPA ought to validate the
> target
> > +      * on initialisation.
> > +      */
> > +     if (cfg == HardwareConfigMap.end())
> > +             return HardwareConfigMap.at("error");
>
> Could you just return {} here and remove "error" from the map ?
>
> > +
> > +     return cfg->second;
> > +}
> > diff --git a/src/ipa/raspberrypi/controller/controller.h
> b/src/ipa/raspberrypi/controller/controller.h
> > index 24e02903d438..c6af5cd6c7d4 100644
> > --- a/src/ipa/raspberrypi/controller/controller.h
> > +++ b/src/ipa/raspberrypi/controller/controller.h
> > @@ -37,6 +37,16 @@ typedef std::unique_ptr<Algorithm> AlgorithmPtr;
> >  class Controller
> >  {
> >  public:
> > +     struct HardwareConfig {
> > +             libcamera::Size agcRegions;
> > +             libcamera::Size agcZoneWeights;
> > +             libcamera::Size awbRegions;
> > +             libcamera::Size focusRegions;
> > +             unsigned int numHistogramBins;
> > +             unsigned int numGammaPoints;
> > +             unsigned int pipelineWidth;
> > +     };
> > +
> >       Controller();
> >       ~Controller();
> >       int read(char const *filename);
> > @@ -47,6 +57,7 @@ public:
> >       Metadata &getGlobalMetadata();
> >       Algorithm *getAlgorithm(std::string const &name) const;
> >       const std::string &getTarget() const;
> > +     const HardwareConfig &getHardwareConfig() const;
> >
> >  protected:
> >       int createAlgorithm(const std::string &name, const
> libcamera::YamlObject &params);
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 86359538cf67..b64cb96e2dde 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -1392,20 +1392,19 @@ RPiController::StatisticsPtr
> IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
> >  {
> >       using namespace RPiController;
> >
> > +     const Controller::HardwareConfig &hw =
> controller_.getHardwareConfig();
> >       unsigned int i;
> >       StatisticsPtr statistics =
> >
>  std::make_unique<Statistics>(Statistics::AgcStatsPos::PreWb,
> Statistics::ColourStatsPos::PostLsc);
> >
> >       /* RGB histograms are not used, so do not populate them. */
> > -     statistics->yHist =
> RPiController::Histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS);
> > +     statistics->yHist = RPiController::Histogram(stats->hist[0].g_hist,
> > +                                                  hw.numHistogramBins);
> >
> > -     /*
> > -      * All region sums are based on a 13-bit pipeline bit-depth.
> Normalise
> > -      * this to 16-bits for the AGC/AWB/ALSC algorithms.
> > -      */
> > -     constexpr unsigned int scale = Statistics::NormalisationFactorPow2
> - 13;
> > +     /* All region sums are based on a 16-bit normalised pipeline
> bit-depth. */
> > +     unsigned int scale = Statistics::NormalisationFactorPow2 -
> hw.pipelineWidth;
> >
> > -     statistics->awbRegions.init({ DEFAULT_AWB_REGIONS_X,
> DEFAULT_AWB_REGIONS_Y });
> > +     statistics->awbRegions.init(hw.awbRegions);
> >       for (i = 0; i < statistics->awbRegions.numRegions(); i++)
> >               statistics->awbRegions.set(i, { {
> stats->awb_stats[i].r_sum << scale,
> >
>  stats->awb_stats[i].g_sum << scale,
> > @@ -1413,11 +1412,7 @@ RPiController::StatisticsPtr
> IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
> >
>  stats->awb_stats[i].counted,
> >
>  stats->awb_stats[i].notcounted });
> >
> > -     /*
> > -      * There are only ever 15 regions computed by the firmware due to
> zoning,
> > -      * but the HW defines AGC_REGIONS == 16!
> > -      */
> > -     statistics->agcRegions.init(15);
> > +     statistics->agcRegions.init(hw.agcRegions);
> >       for (i = 0; i < statistics->agcRegions.numRegions(); i++)
> >               statistics->agcRegions.set(i, { {
> stats->agc_stats[i].r_sum << scale,
> >
>  stats->agc_stats[i].g_sum << scale,
> > @@ -1425,7 +1420,7 @@ RPiController::StatisticsPtr
> IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
> >
>  stats->agc_stats[i].counted,
> >
>  stats->awb_stats[i].notcounted });
> >
> > -     statistics->focusRegions.init({ 4, 3 });
> > +     statistics->focusRegions.init(hw.focusRegions);
> >       for (i = 0; i < statistics->focusRegions.numRegions(); i++)
> >               statistics->focusRegions.set(i, {
> stats->focus_stats[i].contrast_val[1][1] / 1000,
> >
>  stats->focus_stats[i].contrast_val_num[1][1],
> > --
> > 2.34.1
> >
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h
index 7c22fbe4945c..4aa814ebbebd 100644
--- a/src/ipa/raspberrypi/controller/algorithm.h
+++ b/src/ipa/raspberrypi/controller/algorithm.h
@@ -45,6 +45,10 @@  public:
 	{
 		return controller_->getTarget();
 	}
+	const Controller::HardwareConfig &getHardwareConfig() const
+	{
+		return controller_->getHardwareConfig();
+	}
 
 private:
 	Controller *controller_;
diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
index a6250ee140b0..2c7517aec6b4 100644
--- a/src/ipa/raspberrypi/controller/controller.cpp
+++ b/src/ipa/raspberrypi/controller/controller.cpp
@@ -20,6 +20,31 @@  using namespace libcamera;
 
 LOG_DEFINE_CATEGORY(RPiController)
 
+static const std::map<std::string, Controller::HardwareConfig> HardwareConfigMap = {
+	{
+		"bcm2835",
+		{
+			/*
+			 * There are only ever 15 AGC regions computed by the firmware
+			 * due to zoning, but the HW defines AGC_REGIONS == 16!
+			 */
+			.agcRegions = { 15 , 1 },
+			.agcZoneWeights = { 15 , 1 },
+			.awbRegions = { 16, 12 },
+			.focusRegions = { 4, 3 },
+			.numHistogramBins = 128,
+			.numGammaPoints = 33,
+			.pipelineWidth = 13
+		}
+	},
+	/* For error handling. */
+	{
+		"error",
+		{
+		}
+	}
+};
+
 Controller::Controller()
 	: switchModeCalled_(false)
 {
@@ -148,3 +173,17 @@  const std::string &Controller::getTarget() const
 {
 	return target_;
 }
+
+const Controller::HardwareConfig &Controller::getHardwareConfig() const
+{
+	auto cfg = HardwareConfigMap.find(getTarget());
+
+	/*
+	 * This really should not happen, the IPA ought to validate the target
+	 * on initialisation.
+	 */
+	if (cfg == HardwareConfigMap.end())
+		return HardwareConfigMap.at("error");
+
+	return cfg->second;
+}
diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h
index 24e02903d438..c6af5cd6c7d4 100644
--- a/src/ipa/raspberrypi/controller/controller.h
+++ b/src/ipa/raspberrypi/controller/controller.h
@@ -37,6 +37,16 @@  typedef std::unique_ptr<Algorithm> AlgorithmPtr;
 class Controller
 {
 public:
+	struct HardwareConfig {
+		libcamera::Size agcRegions;
+		libcamera::Size agcZoneWeights;
+		libcamera::Size awbRegions;
+		libcamera::Size focusRegions;
+		unsigned int numHistogramBins;
+		unsigned int numGammaPoints;
+		unsigned int pipelineWidth;
+	};
+
 	Controller();
 	~Controller();
 	int read(char const *filename);
@@ -47,6 +57,7 @@  public:
 	Metadata &getGlobalMetadata();
 	Algorithm *getAlgorithm(std::string const &name) const;
 	const std::string &getTarget() const;
+	const HardwareConfig &getHardwareConfig() const;
 
 protected:
 	int createAlgorithm(const std::string &name, const libcamera::YamlObject &params);
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 86359538cf67..b64cb96e2dde 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -1392,20 +1392,19 @@  RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
 {
 	using namespace RPiController;
 
+	const Controller::HardwareConfig &hw = controller_.getHardwareConfig();
 	unsigned int i;
 	StatisticsPtr statistics =
 		std::make_unique<Statistics>(Statistics::AgcStatsPos::PreWb, Statistics::ColourStatsPos::PostLsc);
 
 	/* RGB histograms are not used, so do not populate them. */
-	statistics->yHist = RPiController::Histogram(stats->hist[0].g_hist, NUM_HISTOGRAM_BINS);
+	statistics->yHist = RPiController::Histogram(stats->hist[0].g_hist,
+						     hw.numHistogramBins);
 
-	/*
-	 * All region sums are based on a 13-bit pipeline bit-depth. Normalise
-	 * this to 16-bits for the AGC/AWB/ALSC algorithms.
-	 */
-	constexpr unsigned int scale = Statistics::NormalisationFactorPow2 - 13;
+	/* All region sums are based on a 16-bit normalised pipeline bit-depth. */
+	unsigned int scale = Statistics::NormalisationFactorPow2 - hw.pipelineWidth;
 
-	statistics->awbRegions.init({ DEFAULT_AWB_REGIONS_X, DEFAULT_AWB_REGIONS_Y });
+	statistics->awbRegions.init(hw.awbRegions);
 	for (i = 0; i < statistics->awbRegions.numRegions(); i++)
 		statistics->awbRegions.set(i, { { stats->awb_stats[i].r_sum << scale,
 						  stats->awb_stats[i].g_sum << scale,
@@ -1413,11 +1412,7 @@  RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
 						stats->awb_stats[i].counted,
 						stats->awb_stats[i].notcounted });
 
-	/*
-	 * There are only ever 15 regions computed by the firmware due to zoning,
-	 * but the HW defines AGC_REGIONS == 16!
-	 */
-	statistics->agcRegions.init(15);
+	statistics->agcRegions.init(hw.agcRegions);
 	for (i = 0; i < statistics->agcRegions.numRegions(); i++)
 		statistics->agcRegions.set(i, { { stats->agc_stats[i].r_sum << scale,
 						  stats->agc_stats[i].g_sum << scale,
@@ -1425,7 +1420,7 @@  RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co
 						stats->agc_stats[i].counted,
 						stats->awb_stats[i].notcounted });
 
-	statistics->focusRegions.init({ 4, 3 });
+	statistics->focusRegions.init(hw.focusRegions);
 	for (i = 0; i < statistics->focusRegions.numRegions(); i++)
 		statistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,
 						  stats->focus_stats[i].contrast_val_num[1][1],