[v5,2/6] ipa: rkisp1: algorithms: dpf: Implement mode switching

25550 diff mbox series

Add support for switching between different noise reduction modes.

Introduce `noiseReductionModes_` to store mode-specific configs.

LoadReductionConfig() select specific config from configs.

Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
---

changelog: 
 - Format commit message
 - Refactored parseModes to utilize kModesMap for string-to-mode lookups, 
   replacing the previous if-else chain.
 - Fixed logic in queueRequest to load configuration using the requested 
   *denoise mode instead of the stale runningMode_.

 src/ipa/rkisp1/algorithms/dpf.cpp | 91 +++++++++++++++++++++++++++++--
 src/ipa/rkisp1/algorithms/dpf.h   | 11 ++++
 2 files changed, 97 insertions(+), 5 deletions(-)

On Sun, Dec 14, 2025 at 01:16:42PM -0500, Rui Wang wrote:
> Add support for switching between different noise reduction modes.
>
> Introduce `noiseReductionModes_` to store mode-specific configs.
>
> LoadReductionConfig() select specific config from configs.
>
> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> ---
>
> changelog:
>  - Format commit message
>  - Refactored parseModes to utilize kModesMap for string-to-mode lookups,
>    replacing the previous if-else chain.
>  - Fixed logic in queueRequest to load configuration using the requested
>    *denoise mode instead of the stale runningMode_.
>
>  src/ipa/rkisp1/algorithms/dpf.cpp | 91 +++++++++++++++++++++++++++++--
>  src/ipa/rkisp1/algorithms/dpf.h   | 11 ++++
>  2 files changed, 97 insertions(+), 5 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index dd3effa1..392edda1 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -8,6 +8,7 @@
>  #include "dpf.h"
>
>  #include <algorithm>
> +#include <map>
>  #include <string>
>  #include <vector>
>
> @@ -36,8 +37,20 @@ namespace ipa::rkisp1::algorithms {
>
>  LOG_DEFINE_CATEGORY(RkISP1Dpf)
>
> +namespace {
> +
> +const std::map<int32_t, std::string> kModesMap = {
> +	{ controls::draft::NoiseReductionModeMinimal, "minimal" },
> +	{ controls::draft::NoiseReductionModeFast, "fast" },
> +	{ controls::draft::NoiseReductionModeHighQuality, "highquality" },
> +	{ controls::draft::NoiseReductionModeZSL, "zsl" },
> +};
> +
> +} /* namespace */
> +
>  Dpf::Dpf()
> -	: config_({}), strengthConfig_({})
> +	: config_({}), strengthConfig_({}), noiseReductionModes_({}),
> +	  runningMode_(controls::draft::NoiseReductionModeOff)
>  {
>  }
>
> @@ -62,6 +75,48 @@ int Dpf::parseConfig(const YamlObject &tuningData)
>  	if (ret)
>  		return ret;
>
> +	/* Parse modes. */
> +	ret = parseModes(tuningData);

        return parseModes(tuningData);

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +int Dpf::parseModes(const YamlObject &tuningData)
> +{
> +	/* Parse noise reduction modes. */
> +	if (!tuningData.contains("modes"))
> +		return -EINVAL;
> +
> +	noiseReductionModes_.clear();
> +	for (const auto &entry : tuningData["modes"].asList()) {
> +		std::optional<std::string> typeOpt =
> +			entry["type"].get<std::string>();
> +		if (!typeOpt) {
> +			LOG(RkISP1Dpf, Error) << "Modes entry missing type";
> +			return -EINVAL;
> +		}
> +
> +		ModeConfig mode;
> +		auto it = std::find_if(kModesMap.begin(), kModesMap.end(),
> +				       [&typeOpt](const auto &pair) {
> +					       return pair.second == *typeOpt;
> +				       });
> +
> +		if (it == kModesMap.end()) {
> +			LOG(RkISP1Dpf, Error) << "Unknown mode type: " << *typeOpt;
> +			return -EINVAL;
> +		}
> +
> +		mode.modeValue = it->first;
> +		auto ret = parseSingleConfig(entry, mode.dpf, mode.strength);
> +		if (ret != 0)
                if (ret)

> +			return ret;
> +
> +		noiseReductionModes_.push_back(mode);
> +	}
> +
>  	return 0;
>  }
>
> @@ -193,6 +248,29 @@ int Dpf::parseSingleConfig(const YamlObject &tuningData,
>  	return 0;
>  }
>
> +bool Dpf::loadReductionConfig(int32_t mode)
> +{
> +	auto it = std::find_if(noiseReductionModes_.begin(), noiseReductionModes_.end(),
> +			       [mode](const ModeConfig &m) {
> +				       return m.modeValue == mode;
> +			       });
> +	if (it == noiseReductionModes_.end()) {
> +		LOG(RkISP1Dpf, Warning)
> +			<< "No DPF config for reduction mode "
> +			<< static_cast<int>(mode);

You don't need a cast I guess

> +		return false;
> +	}
> +
> +	config_ = it->dpf;
> +	strengthConfig_ = it->strength;
> +
> +	LOG(RkISP1Dpf, Info)

I still think this is too verbose, this is about loading a
configuration from tuning file for an algorithm, this is a debug
message, nothing here's worth an Info imho

> +		<< "DPF mode=Reduction (config loaded)"
> +		<< " mode=" << kModesMap.at(mode);
> +
> +	return true;
> +}
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::queueRequest
>   */
> @@ -206,8 +284,6 @@ void Dpf::queueRequest(IPAContext &context,
>
>  	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
>  	if (denoise) {
> -		LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
> -
>  		switch (*denoise) {
>  		case controls::draft::NoiseReductionModeOff:
>  			if (dpf.denoise) {
> @@ -218,9 +294,10 @@ void Dpf::queueRequest(IPAContext &context,
>  		case controls::draft::NoiseReductionModeMinimal:
>  		case controls::draft::NoiseReductionModeHighQuality:
>  		case controls::draft::NoiseReductionModeFast:
> -			if (!dpf.denoise) {
> -				dpf.denoise = true;
> +		case controls::draft::NoiseReductionModeZSL:
> +			if (loadReductionConfig(*denoise)) {
>  				update = true;
> +				dpf.denoise = true;
>  			}
>  			break;
>  		default:
> @@ -229,6 +306,10 @@ void Dpf::queueRequest(IPAContext &context,
>  				<< *denoise;
>  			break;
>  		}
> +		if (update) {
> +			runningMode_ = *denoise;
> +			LOG(RkISP1Dpf, Debug) << "Set denoise to " << kModesMap.at(runningMode_);
> +		}
>  	}
>
>  	frameContext.dpf.denoise = dpf.denoise;
> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> index 39186c55..2a2c7052 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.h
> +++ b/src/ipa/rkisp1/algorithms/dpf.h
> @@ -30,13 +30,24 @@ public:
>  		     RkISP1Params *params) override;
>
>  private:
> +	struct ModeConfig {
> +		int32_t modeValue;
> +		rkisp1_cif_isp_dpf_config dpf;
> +		rkisp1_cif_isp_dpf_strength_config strength;
> +	};
> +
>  	int parseConfig(const YamlObject &tuningData);
> +	int parseModes(const YamlObject &tuningData);
>  	int parseSingleConfig(const YamlObject &tuningData,
>  			      rkisp1_cif_isp_dpf_config &config,
>  			      rkisp1_cif_isp_dpf_strength_config &strengthConfig);
>
> +	bool loadReductionConfig(int32_t mode);
> +
>  	struct rkisp1_cif_isp_dpf_config config_;
>  	struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
> +	std::vector<ModeConfig> noiseReductionModes_;
> +	int32_t runningMode_;

With the above addressed

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>  };
>
>  } /* namespace ipa::rkisp1::algorithms */
> --
> 2.43.0
>