[1/5] ipa: rkisp1: agc: Read histogram weights from tuning file
diff mbox series

Message ID 20240405144729.2992219-2-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: rkisp1: Improve AGC (plumbing)
Related show

Commit Message

Paul Elder April 5, 2024, 2:47 p.m. UTC
Add support to the rkisp1 AGC to read histogram weights from the tuning
file. As controls for selecting the metering mode are not yet supported,
for now hardcode the matrix metering mode, which is the same as what the
AGC previously hardcoded.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/agc.cpp | 80 +++++++++++++++++++++++++++++--
 src/ipa/rkisp1/algorithms/agc.h   |  6 +++
 2 files changed, 83 insertions(+), 3 deletions(-)

Comments

Stefan Klug April 15, 2024, 1:22 p.m. UTC | #1
Hi Paul,

thanks for the patch.

On Fri, Apr 05, 2024 at 11:47:25PM +0900, Paul Elder wrote:
> Add support to the rkisp1 AGC to read histogram weights from the tuning
> file. As controls for selecting the metering mode are not yet supported,
> for now hardcode the matrix metering mode, which is the same as what the
> AGC previously hardcoded.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 80 +++++++++++++++++++++++++++++--
>  src/ipa/rkisp1/algorithms/agc.h   |  6 +++
>  2 files changed, 83 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 1cd977a0..fd47ba4e 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -17,6 +17,8 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/ipa/core_ipa_interface.h>
>  
> +#include "libcamera/internal/yaml_parser.h"
> +
>  #include "libipa/histogram.h"
>  
>  /**
> @@ -42,6 +44,62 @@ static constexpr double kMinAnalogueGain = 1.0;
>  /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
>  static constexpr utils::Duration kMaxShutterSpeed = 60ms;
>  
> +int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> +			    const char *prop)
> +{
> +	const YamlObject &yamlMeteringModes = tuningData[prop];
> +	if (!yamlMeteringModes.isDictionary()) {
> +		LOG(RkISP1Agc, Error)
> +			<< "'" << prop << "' parameter not found in tuning file";
> +		return -EINVAL;
> +	}
> +
> +	for (const auto &[key, value] : yamlMeteringModes.asDict()) {
> +		if (controls::AeMeteringModeNameValueMap.find(key) ==
> +		    controls::AeMeteringModeNameValueMap.end()) {
> +			LOG(RkISP1Agc, Warning)
> +				<< "Skipping unknown metering mode '" << key << "'";
> +			continue;
> +		}
> +
> +		std::vector<uint8_t> weights =
> +			value.getList<uint8_t>().value_or(std::vector<uint8_t>{});
> +		if (weights.size() != context.hw->numHistogramWeights) {
> +			LOG(RkISP1Agc, Error)
> +				<< "Invalid '" << key << "' values: expected "
> +				<< context.hw->numHistogramWeights
> +				<< " elements, got " << weights.size();
> +			return -ENODATA;
> +		}
> +
> +		meteringModes_[controls::AeMeteringModeNameValueMap.at(key)] = weights;
> +	}

Kieran already mentioned that. What about adding the matrix metering
mode (+ warning) in case meteringModes_ is still empty.

> +
> +	return 0;
> +}
> +
> +uint8_t Agc::predivider(Size &size)
> +{
> +	static const std::vector<std::pair<unsigned int, unsigned int>>
> +		pixelCountThresholds = {
> +			{  640 *  480,  3 },
> +			{  800 *  600,  4 },
> +			{ 1024 *  768,  5 },
> +			{ 1280 *  960,  6 },
> +			{ 1600 * 1200,  8 },
> +			{ 2048 * 1536, 10 },
> +			{ 2600 * 1950, 12 },
> +			{ 4096 * 3072, 16 },
> +		};

Where do these values come from? What would happen if we keep the predivider
of 4?

> +
> +	unsigned long pixels = size.width * size.height;
> +	for (auto &pair : pixelCountThresholds)
> +		if (pixels < pair.first)
> +			return pair.second;
> +
> +	return 24;
> +}
> +
>  Agc::Agc()
>  {
>  	supportsRaw_ = true;
> @@ -72,6 +130,10 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)
>  	if (ret)
>  		return ret;
>  
> +	ret = parseMeteringModes(context, tuningData, "AeMeteringMode");
> +	if (ret)
> +		return ret;
> +
>  	context.ctrlMap.merge(controls());
>  
>  	return 0;
> @@ -177,6 +239,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
>  		frameContext.agc.gain = context.activeState.agc.automatic.gain;
>  	}
>  
> +	/* \todo Remove this when we can set the below with controls */
>  	if (frame > 0)
>  		return;
>  
> @@ -195,14 +258,25 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
>  	params->meas.hst_config.meas_window = context.configuration.agc.measureWindow;
>  	/* Produce the luminance histogram. */
>  	params->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;
> +
>  	/* Set an average weighted histogram. */
>  	Span<uint8_t> weights{
>  		params->meas.hst_config.hist_weight,
>  		context.hw->numHistogramWeights
>  	};
> -	std::fill(weights.begin(), weights.end(), 1);
> -	/* Step size can't be less than 3. */
> -	params->meas.hst_config.histogram_predivider = 4;
> +	/* \todo Get this from control */
> +	std::vector<uint8_t> &modeWeights = meteringModes_.at(controls::MeteringMatrix);
> +	std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());
> +
> +	std::stringstream str;
> +	str << "Histogram weights : ";
> +	for (size_t i = 0; i < context.hw->numHistogramWeights; i++)
> +		str << (int)params->meas.hst_config.hist_weight[i] << " ";
> +	LOG(RkISP1Agc, Debug) << str.str();
> +
> +	/* \todo Add a control for this? */
> +	params->meas.hst_config.histogram_predivider =
> +		predivider(context.configuration.sensor.size);
>  
>  	/* Update the configuration for histogram. */
>  	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index 311d4e94..43e3d5b2 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -44,6 +44,10 @@ public:
>  		     ControlList &metadata) override;
>  
>  private:
> +	int parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> +			       const char *prop);
> +	uint8_t predivider(Size &size);
> +
>  	void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
>  			  ControlList &metadata);
>  	void parseStatistics(const rkisp1_stat_buffer *stats,
> @@ -52,6 +56,8 @@ private:
>  
>  	Histogram hist_;
>  	Span<const uint8_t> expMeans_;
> +
> +	std::map<int32_t, std::vector<uint8_t>> meteringModes_;
>  };
>  
>  } /* namespace ipa::rkisp1::algorithms */
> -- 
> 2.39.2
>
Paul Elder May 9, 2024, 11:08 a.m. UTC | #2
On Mon, Apr 15, 2024 at 03:22:51PM +0200, Stefan Klug wrote:
> Hi Paul,
> 
> thanks for the patch.
> 
> On Fri, Apr 05, 2024 at 11:47:25PM +0900, Paul Elder wrote:
> > Add support to the rkisp1 AGC to read histogram weights from the tuning
> > file. As controls for selecting the metering mode are not yet supported,
> > for now hardcode the matrix metering mode, which is the same as what the
> > AGC previously hardcoded.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp | 80 +++++++++++++++++++++++++++++--
> >  src/ipa/rkisp1/algorithms/agc.h   |  6 +++
> >  2 files changed, 83 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > index 1cd977a0..fd47ba4e 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -17,6 +17,8 @@
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/ipa/core_ipa_interface.h>
> >  
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> >  #include "libipa/histogram.h"
> >  
> >  /**
> > @@ -42,6 +44,62 @@ static constexpr double kMinAnalogueGain = 1.0;
> >  /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
> >  static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> >  
> > +int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> > +			    const char *prop)
> > +{
> > +	const YamlObject &yamlMeteringModes = tuningData[prop];
> > +	if (!yamlMeteringModes.isDictionary()) {
> > +		LOG(RkISP1Agc, Error)
> > +			<< "'" << prop << "' parameter not found in tuning file";
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (const auto &[key, value] : yamlMeteringModes.asDict()) {
> > +		if (controls::AeMeteringModeNameValueMap.find(key) ==
> > +		    controls::AeMeteringModeNameValueMap.end()) {
> > +			LOG(RkISP1Agc, Warning)
> > +				<< "Skipping unknown metering mode '" << key << "'";
> > +			continue;
> > +		}
> > +
> > +		std::vector<uint8_t> weights =
> > +			value.getList<uint8_t>().value_or(std::vector<uint8_t>{});
> > +		if (weights.size() != context.hw->numHistogramWeights) {
> > +			LOG(RkISP1Agc, Error)
> > +				<< "Invalid '" << key << "' values: expected "
> > +				<< context.hw->numHistogramWeights
> > +				<< " elements, got " << weights.size();
> > +			return -ENODATA;
> > +		}
> > +
> > +		meteringModes_[controls::AeMeteringModeNameValueMap.at(key)] = weights;
> > +	}
> 
> Kieran already mentioned that. What about adding the matrix metering
> mode (+ warning) in case meteringModes_ is still empty.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +uint8_t Agc::predivider(Size &size)
> > +{
> > +	static const std::vector<std::pair<unsigned int, unsigned int>>
> > +		pixelCountThresholds = {
> > +			{  640 *  480,  3 },
> > +			{  800 *  600,  4 },
> > +			{ 1024 *  768,  5 },
> > +			{ 1280 *  960,  6 },
> > +			{ 1600 * 1200,  8 },
> > +			{ 2048 * 1536, 10 },
> > +			{ 2600 * 1950, 12 },
> > +			{ 4096 * 3072, 16 },
> > +		};
> 
> Where do these values come from? What would happen if we keep the predivider
> of 4?

It comes from the programming guide. If the predivider is small and the
histogram is spikey then the counter will wraparound for the spikey bin.


Paul

> 
> > +
> > +	unsigned long pixels = size.width * size.height;
> > +	for (auto &pair : pixelCountThresholds)
> > +		if (pixels < pair.first)
> > +			return pair.second;
> > +
> > +	return 24;
> > +}
> > +
> >  Agc::Agc()
> >  {
> >  	supportsRaw_ = true;
> > @@ -72,6 +130,10 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)
> >  	if (ret)
> >  		return ret;
> >  
> > +	ret = parseMeteringModes(context, tuningData, "AeMeteringMode");
> > +	if (ret)
> > +		return ret;
> > +
> >  	context.ctrlMap.merge(controls());
> >  
> >  	return 0;
> > @@ -177,6 +239,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
> >  		frameContext.agc.gain = context.activeState.agc.automatic.gain;
> >  	}
> >  
> > +	/* \todo Remove this when we can set the below with controls */
> >  	if (frame > 0)
> >  		return;
> >  
> > @@ -195,14 +258,25 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
> >  	params->meas.hst_config.meas_window = context.configuration.agc.measureWindow;
> >  	/* Produce the luminance histogram. */
> >  	params->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;
> > +
> >  	/* Set an average weighted histogram. */
> >  	Span<uint8_t> weights{
> >  		params->meas.hst_config.hist_weight,
> >  		context.hw->numHistogramWeights
> >  	};
> > -	std::fill(weights.begin(), weights.end(), 1);
> > -	/* Step size can't be less than 3. */
> > -	params->meas.hst_config.histogram_predivider = 4;
> > +	/* \todo Get this from control */
> > +	std::vector<uint8_t> &modeWeights = meteringModes_.at(controls::MeteringMatrix);
> > +	std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());
> > +
> > +	std::stringstream str;
> > +	str << "Histogram weights : ";
> > +	for (size_t i = 0; i < context.hw->numHistogramWeights; i++)
> > +		str << (int)params->meas.hst_config.hist_weight[i] << " ";
> > +	LOG(RkISP1Agc, Debug) << str.str();
> > +
> > +	/* \todo Add a control for this? */
> > +	params->meas.hst_config.histogram_predivider =
> > +		predivider(context.configuration.sensor.size);
> >  
> >  	/* Update the configuration for histogram. */
> >  	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;
> > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> > index 311d4e94..43e3d5b2 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.h
> > +++ b/src/ipa/rkisp1/algorithms/agc.h
> > @@ -44,6 +44,10 @@ public:
> >  		     ControlList &metadata) override;
> >  
> >  private:
> > +	int parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> > +			       const char *prop);
> > +	uint8_t predivider(Size &size);
> > +
> >  	void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> >  			  ControlList &metadata);
> >  	void parseStatistics(const rkisp1_stat_buffer *stats,
> > @@ -52,6 +56,8 @@ private:
> >  
> >  	Histogram hist_;
> >  	Span<const uint8_t> expMeans_;
> > +
> > +	std::map<int32_t, std::vector<uint8_t>> meteringModes_;
> >  };
> >  
> >  } /* namespace ipa::rkisp1::algorithms */
> > -- 
> > 2.39.2
> >
Laurent Pinchart May 9, 2024, 11:11 a.m. UTC | #3
On Mon, Apr 15, 2024 at 03:22:51PM +0200, Stefan Klug wrote:
> Hi Paul,
> 
> thanks for the patch.
> 
> On Fri, Apr 05, 2024 at 11:47:25PM +0900, Paul Elder wrote:
> > Add support to the rkisp1 AGC to read histogram weights from the tuning
> > file. As controls for selecting the metering mode are not yet supported,
> > for now hardcode the matrix metering mode, which is the same as what the
> > AGC previously hardcoded.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp | 80 +++++++++++++++++++++++++++++--
> >  src/ipa/rkisp1/algorithms/agc.h   |  6 +++
> >  2 files changed, 83 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > index 1cd977a0..fd47ba4e 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -17,6 +17,8 @@
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/ipa/core_ipa_interface.h>
> >  
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> >  #include "libipa/histogram.h"
> >  
> >  /**
> > @@ -42,6 +44,62 @@ static constexpr double kMinAnalogueGain = 1.0;
> >  /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
> >  static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> >  
> > +int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> > +			    const char *prop)
> > +{
> > +	const YamlObject &yamlMeteringModes = tuningData[prop];
> > +	if (!yamlMeteringModes.isDictionary()) {
> > +		LOG(RkISP1Agc, Error)
> > +			<< "'" << prop << "' parameter not found in tuning file";
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (const auto &[key, value] : yamlMeteringModes.asDict()) {
> > +		if (controls::AeMeteringModeNameValueMap.find(key) ==
> > +		    controls::AeMeteringModeNameValueMap.end()) {
> > +			LOG(RkISP1Agc, Warning)
> > +				<< "Skipping unknown metering mode '" << key << "'";
> > +			continue;
> > +		}
> > +
> > +		std::vector<uint8_t> weights =
> > +			value.getList<uint8_t>().value_or(std::vector<uint8_t>{});
> > +		if (weights.size() != context.hw->numHistogramWeights) {
> > +			LOG(RkISP1Agc, Error)
> > +				<< "Invalid '" << key << "' values: expected "
> > +				<< context.hw->numHistogramWeights
> > +				<< " elements, got " << weights.size();
> > +			return -ENODATA;
> > +		}
> > +
> > +		meteringModes_[controls::AeMeteringModeNameValueMap.at(key)] = weights;
> > +	}
> 
> Kieran already mentioned that. What about adding the matrix metering
> mode (+ warning) in case meteringModes_ is still empty.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +uint8_t Agc::predivider(Size &size)
> > +{
> > +	static const std::vector<std::pair<unsigned int, unsigned int>>
> > +		pixelCountThresholds = {
> > +			{  640 *  480,  3 },
> > +			{  800 *  600,  4 },
> > +			{ 1024 *  768,  5 },
> > +			{ 1280 *  960,  6 },
> > +			{ 1600 * 1200,  8 },
> > +			{ 2048 * 1536, 10 },
> > +			{ 2600 * 1950, 12 },
> > +			{ 4096 * 3072, 16 },
> > +		};
> 
> Where do these values come from? What would happen if we keep the predivider
> of 4?

The predivider is meant to avoid overflows in the worst case situation.
I'd rather compute them in code instead of using a table.

> > +
> > +	unsigned long pixels = size.width * size.height;
> > +	for (auto &pair : pixelCountThresholds)
> > +		if (pixels < pair.first)
> > +			return pair.second;
> > +
> > +	return 24;
> > +}
> > +
> >  Agc::Agc()
> >  {
> >  	supportsRaw_ = true;
> > @@ -72,6 +130,10 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)
> >  	if (ret)
> >  		return ret;
> >  
> > +	ret = parseMeteringModes(context, tuningData, "AeMeteringMode");
> > +	if (ret)
> > +		return ret;
> > +
> >  	context.ctrlMap.merge(controls());
> >  
> >  	return 0;
> > @@ -177,6 +239,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
> >  		frameContext.agc.gain = context.activeState.agc.automatic.gain;
> >  	}
> >  
> > +	/* \todo Remove this when we can set the below with controls */
> >  	if (frame > 0)
> >  		return;
> >  
> > @@ -195,14 +258,25 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,
> >  	params->meas.hst_config.meas_window = context.configuration.agc.measureWindow;
> >  	/* Produce the luminance histogram. */
> >  	params->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;
> > +
> >  	/* Set an average weighted histogram. */
> >  	Span<uint8_t> weights{
> >  		params->meas.hst_config.hist_weight,
> >  		context.hw->numHistogramWeights
> >  	};
> > -	std::fill(weights.begin(), weights.end(), 1);
> > -	/* Step size can't be less than 3. */
> > -	params->meas.hst_config.histogram_predivider = 4;
> > +	/* \todo Get this from control */
> > +	std::vector<uint8_t> &modeWeights = meteringModes_.at(controls::MeteringMatrix);
> > +	std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());
> > +
> > +	std::stringstream str;
> > +	str << "Histogram weights : ";
> > +	for (size_t i = 0; i < context.hw->numHistogramWeights; i++)
> > +		str << (int)params->meas.hst_config.hist_weight[i] << " ";
> > +	LOG(RkISP1Agc, Debug) << str.str();
> > +
> > +	/* \todo Add a control for this? */
> > +	params->meas.hst_config.histogram_predivider =
> > +		predivider(context.configuration.sensor.size);
> >  
> >  	/* Update the configuration for histogram. */
> >  	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;
> > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> > index 311d4e94..43e3d5b2 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.h
> > +++ b/src/ipa/rkisp1/algorithms/agc.h
> > @@ -44,6 +44,10 @@ public:
> >  		     ControlList &metadata) override;
> >  
> >  private:
> > +	int parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
> > +			       const char *prop);
> > +	uint8_t predivider(Size &size);
> > +
> >  	void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
> >  			  ControlList &metadata);
> >  	void parseStatistics(const rkisp1_stat_buffer *stats,
> > @@ -52,6 +56,8 @@ private:
> >  
> >  	Histogram hist_;
> >  	Span<const uint8_t> expMeans_;
> > +
> > +	std::map<int32_t, std::vector<uint8_t>> meteringModes_;
> >  };
> >  
> >  } /* namespace ipa::rkisp1::algorithms */

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
index 1cd977a0..fd47ba4e 100644
--- a/src/ipa/rkisp1/algorithms/agc.cpp
+++ b/src/ipa/rkisp1/algorithms/agc.cpp
@@ -17,6 +17,8 @@ 
 #include <libcamera/control_ids.h>
 #include <libcamera/ipa/core_ipa_interface.h>
 
+#include "libcamera/internal/yaml_parser.h"
+
 #include "libipa/histogram.h"
 
 /**
@@ -42,6 +44,62 @@  static constexpr double kMinAnalogueGain = 1.0;
 /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
 static constexpr utils::Duration kMaxShutterSpeed = 60ms;
 
+int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
+			    const char *prop)
+{
+	const YamlObject &yamlMeteringModes = tuningData[prop];
+	if (!yamlMeteringModes.isDictionary()) {
+		LOG(RkISP1Agc, Error)
+			<< "'" << prop << "' parameter not found in tuning file";
+		return -EINVAL;
+	}
+
+	for (const auto &[key, value] : yamlMeteringModes.asDict()) {
+		if (controls::AeMeteringModeNameValueMap.find(key) ==
+		    controls::AeMeteringModeNameValueMap.end()) {
+			LOG(RkISP1Agc, Warning)
+				<< "Skipping unknown metering mode '" << key << "'";
+			continue;
+		}
+
+		std::vector<uint8_t> weights =
+			value.getList<uint8_t>().value_or(std::vector<uint8_t>{});
+		if (weights.size() != context.hw->numHistogramWeights) {
+			LOG(RkISP1Agc, Error)
+				<< "Invalid '" << key << "' values: expected "
+				<< context.hw->numHistogramWeights
+				<< " elements, got " << weights.size();
+			return -ENODATA;
+		}
+
+		meteringModes_[controls::AeMeteringModeNameValueMap.at(key)] = weights;
+	}
+
+	return 0;
+}
+
+uint8_t Agc::predivider(Size &size)
+{
+	static const std::vector<std::pair<unsigned int, unsigned int>>
+		pixelCountThresholds = {
+			{  640 *  480,  3 },
+			{  800 *  600,  4 },
+			{ 1024 *  768,  5 },
+			{ 1280 *  960,  6 },
+			{ 1600 * 1200,  8 },
+			{ 2048 * 1536, 10 },
+			{ 2600 * 1950, 12 },
+			{ 4096 * 3072, 16 },
+		};
+
+	unsigned long pixels = size.width * size.height;
+	for (auto &pair : pixelCountThresholds)
+		if (pixels < pair.first)
+			return pair.second;
+
+	return 24;
+}
+
 Agc::Agc()
 {
 	supportsRaw_ = true;
@@ -72,6 +130,10 @@  int Agc::init(IPAContext &context, const YamlObject &tuningData)
 	if (ret)
 		return ret;
 
+	ret = parseMeteringModes(context, tuningData, "AeMeteringMode");
+	if (ret)
+		return ret;
+
 	context.ctrlMap.merge(controls());
 
 	return 0;
@@ -177,6 +239,7 @@  void Agc::prepare(IPAContext &context, const uint32_t frame,
 		frameContext.agc.gain = context.activeState.agc.automatic.gain;
 	}
 
+	/* \todo Remove this when we can set the below with controls */
 	if (frame > 0)
 		return;
 
@@ -195,14 +258,25 @@  void Agc::prepare(IPAContext &context, const uint32_t frame,
 	params->meas.hst_config.meas_window = context.configuration.agc.measureWindow;
 	/* Produce the luminance histogram. */
 	params->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;
+
 	/* Set an average weighted histogram. */
 	Span<uint8_t> weights{
 		params->meas.hst_config.hist_weight,
 		context.hw->numHistogramWeights
 	};
-	std::fill(weights.begin(), weights.end(), 1);
-	/* Step size can't be less than 3. */
-	params->meas.hst_config.histogram_predivider = 4;
+	/* \todo Get this from control */
+	std::vector<uint8_t> &modeWeights = meteringModes_.at(controls::MeteringMatrix);
+	std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());
+
+	std::stringstream str;
+	str << "Histogram weights : ";
+	for (size_t i = 0; i < context.hw->numHistogramWeights; i++)
+		str << (int)params->meas.hst_config.hist_weight[i] << " ";
+	LOG(RkISP1Agc, Debug) << str.str();
+
+	/* \todo Add a control for this? */
+	params->meas.hst_config.histogram_predivider =
+		predivider(context.configuration.sensor.size);
 
 	/* Update the configuration for histogram. */
 	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;
diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
index 311d4e94..43e3d5b2 100644
--- a/src/ipa/rkisp1/algorithms/agc.h
+++ b/src/ipa/rkisp1/algorithms/agc.h
@@ -44,6 +44,10 @@  public:
 		     ControlList &metadata) override;
 
 private:
+	int parseMeteringModes(IPAContext &context, const YamlObject &tuningData,
+			       const char *prop);
+	uint8_t predivider(Size &size);
+
 	void fillMetadata(IPAContext &context, IPAFrameContext &frameContext,
 			  ControlList &metadata);
 	void parseStatistics(const rkisp1_stat_buffer *stats,
@@ -52,6 +56,8 @@  private:
 
 	Histogram hist_;
 	Span<const uint8_t> expMeans_;
+
+	std::map<int32_t, std::vector<uint8_t>> meteringModes_;
 };
 
 } /* namespace ipa::rkisp1::algorithms */