[v2,1/2] ipa/rkisp1: refactory DPF parsing and initialization
diff mbox series

Message ID 20251204202500.1075575-2-rui.wang@ideasonboard.com
State Superseded
Headers show
Series
  • rebase_dpf_refactory_patch_v2
Related show

Commit Message

Rui Wang Dec. 4, 2025, 8:24 p.m. UTC
Split DPF configuration parsing and initialization into clearer,
self-contained helpers and modernized initialization patterns.

Add parseConfig, parseModes and loadReductionConfig to factor
parsing logic and mode lookups.
Introduce ModeConfig and store mode entries in
noiseReductionModes_ to decouple parsing from runtime selection.
Move sentinel member initializers into the constructor initializer
list (runningMode_) and declare vectors
without in-class initializers for default construction.
Replace ad-hoc string handling with value_or and const auto
where appropriate for clearer and safer parsing.
Replace domain/range/strength YAML keys mapping and error returns
(use filter, nll, strength keys and return false from parse
helpers instead of -EINVAL).
Add loadReductionConfig to centralize loading of DPF configs for
reduction modes and preserve logging and failure behavior.
Adjust queueRequest, prepare, and helpers to use the new
parsing/initialization flow while preserving existing behavior.
This refactor improves separation of concerns, makes parsing easier
to maintain, and reduces duplicated logic. No functional behaviour is
intended to be changed.

Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/dpf.cpp | 290 ++++++++++++++++++++++--------
 src/ipa/rkisp1/algorithms/dpf.h   |  19 ++
 2 files changed, 233 insertions(+), 76 deletions(-)

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
index 39f3e461..cf1f9199 100644
--- a/src/ipa/rkisp1/algorithms/dpf.cpp
+++ b/src/ipa/rkisp1/algorithms/dpf.cpp
@@ -37,7 +37,8 @@  namespace ipa::rkisp1::algorithms {
 LOG_DEFINE_CATEGORY(RkISP1Dpf)
 
 Dpf::Dpf()
-	: config_({}), strengthConfig_({})
+	: config_({}), strengthConfig_({}),
+	  runningMode_(controls::draft::NoiseReductionModeOff)
 {
 }
 
@@ -46,6 +47,69 @@  Dpf::Dpf()
  */
 int Dpf::init([[maybe_unused]] IPAContext &context,
 	      const YamlObject &tuningData)
+{
+	/* Parse tuning block. */
+	if (!parseConfig(tuningData)) {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+bool Dpf::parseConfig(const YamlObject &tuningData)
+{
+	/* Parse base config. */
+	if (!parseSingleConfig(tuningData, config_, strengthConfig_)) {
+		return false;
+	}
+	if (!parseModes(tuningData)) {
+		return false;
+	}
+	return true;
+}
+
+bool Dpf::parseModes(const YamlObject &tuningData)
+{
+	/* Parse noise reduction modes. */
+	if (!tuningData.contains("modes")) {
+		return true;
+	}
+
+	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 false;
+		}
+
+		int32_t modeValue = controls::draft::NoiseReductionModeOff;
+		if (*typeOpt == "minimal") {
+			modeValue = controls::draft::NoiseReductionModeMinimal;
+		} else if (*typeOpt == "fast") {
+			modeValue = controls::draft::NoiseReductionModeFast;
+		} else if (*typeOpt == "highquality") {
+			modeValue = controls::draft::NoiseReductionModeHighQuality;
+		} else if (*typeOpt == "zsl") {
+			modeValue = controls::draft::NoiseReductionModeZSL;
+		} else {
+			LOG(RkISP1Dpf, Error) << "Unknown mode type: " << *typeOpt;
+			return false;
+		}
+
+		ModeConfig mode{};
+		mode.modeValue = modeValue;
+		if (!parseSingleConfig(entry, mode.dpf, mode.strength)) {
+			return false;
+		}
+		noiseReductionModes_.push_back(mode);
+	}
+
+	return true;
+}
+bool Dpf::parseSingleConfig(const YamlObject &tuningData,
+			    rkisp1_cif_isp_dpf_config &config,
+			    rkisp1_cif_isp_dpf_strength_config &strengthConfig)
 {
 	std::vector<uint8_t> values;
 
@@ -53,7 +117,11 @@  int Dpf::init([[maybe_unused]] IPAContext &context,
 	 * The domain kernel is configured with a 9x9 kernel for the green
 	 * pixels, and a 13x9 or 9x9 kernel for red and blue pixels.
 	 */
-	const YamlObject &dFObject = tuningData["DomainFilter"];
+	const YamlObject &dFObject = tuningData["filter"];
+	if (!dFObject) {
+		LOG(RkISP1Dpf, Error) << "filter section missing";
+		return false;
+	}
 
 	/*
 	 * For the green component, we have the 9x9 kernel specified
@@ -75,10 +143,10 @@  int Dpf::init([[maybe_unused]] IPAContext &context,
 	values = dFObject["g"].getList<uint8_t>().value_or(std::vector<uint8_t>{});
 	if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS) {
 		LOG(RkISP1Dpf, Error)
-			<< "Invalid 'DomainFilter:g': expected "
+			<< "Invalid 'filter:g': expected "
 			<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
 			<< " elements, got " << values.size();
-		return -EINVAL;
+		return false;
 	}
 
 	std::copy_n(values.begin(), values.size(),
@@ -112,16 +180,16 @@  int Dpf::init([[maybe_unused]] IPAContext &context,
 	if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS &&
 	    values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1) {
 		LOG(RkISP1Dpf, Error)
-			<< "Invalid 'DomainFilter:rb': expected "
+			<< "Invalid 'filter:rb': expected "
 			<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1
 			<< " or " << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
 			<< " elements, got " << values.size();
-		return -EINVAL;
+		return false;
 	}
 
 	config_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
-			       ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9
-			       : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;
+					 ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9
+					 : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;
 
 	std::copy_n(values.begin(), values.size(),
 		    std::begin(config_.rb_flt.spatial_coeff));
@@ -134,43 +202,86 @@  int Dpf::init([[maybe_unused]] IPAContext &context,
 	 * which stores a piecewise linear function that characterizes the
 	 * sensor noise profile as a noise level function curve (NLF).
 	 */
-	const YamlObject &rFObject = tuningData["NoiseLevelFunction"];
+	const YamlObject &rFObject = tuningData["nll"];
+	if (!rFObject) {
+		LOG(RkISP1Dpf, Error) << "nll section missing";
+		return false;
+	}
 
-	std::vector<uint16_t> nllValues;
-	nllValues = rFObject["coeff"].getList<uint16_t>().value_or(std::vector<uint16_t>{});
+	const auto nllValues =
+		rFObject["coeff"].getList<uint16_t>().value_or(std::vector<uint16_t>{});
 	if (nllValues.size() != RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS) {
 		LOG(RkISP1Dpf, Error)
-			<< "Invalid 'RangeFilter:coeff': expected "
+			<< "Invalid 'nll:coeff': expected "
 			<< RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS
 			<< " elements, got " << nllValues.size();
-		return -EINVAL;
+		return false;
 	}
 
 	std::copy_n(nllValues.begin(), nllValues.size(),
 		    std::begin(config_.nll.coeff));
 
-	std::string scaleMode = rFObject["scale-mode"].get<std::string>("");
+	const auto scaleMode = rFObject["scale-mode"].get<std::string>("");
 	if (scaleMode == "linear") {
 		config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR;
 	} else if (scaleMode == "logarithmic") {
 		config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC;
 	} else {
 		LOG(RkISP1Dpf, Error)
-			<< "Invalid 'RangeFilter:scale-mode': expected "
+			<< "Invalid 'nll:scale-mode': expected "
 			<< "'linear' or 'logarithmic' value, got "
 			<< scaleMode;
-		return -EINVAL;
+		return false;
 	}
 
-	const YamlObject &fSObject = tuningData["FilterStrength"];
+	const YamlObject &gObject = tuningData["gain"];
+	if (!gObject) {
+		LOG(RkISP1Dpf, Error) << "gain section missing";
+		return false;
+	}
 
-	strengthConfig_.r = fSObject["r"].get<uint16_t>(64);
-	strengthConfig_.g = fSObject["g"].get<uint16_t>(64);
-	strengthConfig_.b = fSObject["b"].get<uint16_t>(64);
+	config.gain.mode =
+		gObject["gain_mode"].get<uint32_t>().value_or(
+			RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS);
+	config.gain.nf_r_gain = gObject["r"].get<uint16_t>().value_or(256);
+	config.gain.nf_b_gain = gObject["b"].get<uint16_t>().value_or(256);
+	config.gain.nf_gr_gain = gObject["gr"].get<uint16_t>().value_or(256);
+	config.gain.nf_gb_gain = gObject["gb"].get<uint16_t>().value_or(256);
+
+	const YamlObject &fSObject = tuningData["strength"];
+	if (!fSObject) {
+		LOG(RkISP1Dpf, Error) << "strength section missing";
+		return false;
+	}
 
-	return 0;
+	strengthConfig.r = fSObject["r"].get<uint8_t>().value_or(64);
+	strengthConfig.g = fSObject["g"].get<uint8_t>().value_or(64);
+	strengthConfig.b = fSObject["b"].get<uint8_t>().value_or(64);
+	return true;
 }
 
+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);
+		return false;
+	}
+
+	config_ = it->dpf;
+	strengthConfig_ = it->strength;
+
+	LOG(RkISP1Dpf, Info)
+		<< "DPF mode=Reduction (config loaded)"
+		<< " mode=" << static_cast<int>(mode);
+
+	return true;
+}
 /**
  * \copydoc libcamera::ipa::Algorithm::queueRequest
  */
@@ -183,30 +294,34 @@  void Dpf::queueRequest(IPAContext &context,
 	bool update = false;
 
 	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) {
-				dpf.denoise = false;
-				update = true;
-			}
-			break;
-		case controls::draft::NoiseReductionModeMinimal:
-		case controls::draft::NoiseReductionModeHighQuality:
-		case controls::draft::NoiseReductionModeFast:
-			if (!dpf.denoise) {
-				dpf.denoise = true;
-				update = true;
-			}
-			break;
-		default:
-			LOG(RkISP1Dpf, Error)
-				<< "Unsupported denoise value "
-				<< *denoise;
-			break;
+	if (!denoise) {
+		return;
+	}
+	runningMode_ = *denoise;
+	switch (runningMode_) {
+	case controls::draft::NoiseReductionModeOff:
+		if (dpf.denoise) {
+			dpf.denoise = false;
+			update = true;
+		}
+		break;
+	case controls::draft::NoiseReductionModeMinimal:
+	case controls::draft::NoiseReductionModeHighQuality:
+	case controls::draft::NoiseReductionModeFast:
+	case controls::draft::NoiseReductionModeZSL:
+		if (loadReductionConfig(runningMode_)) {
+			update = true;
+			dpf.denoise = true;
+		} else {
+			dpf.denoise = false;
+			update = true;
 		}
+		break;
+	default:
+		LOG(RkISP1Dpf, Error)
+			<< "Unsupported denoise value "
+			<< *denoise;
+		break;
 	}
 
 	frameContext.dpf.denoise = dpf.denoise;
@@ -219,41 +334,64 @@  void Dpf::queueRequest(IPAContext &context,
 void Dpf::prepare(IPAContext &context, const uint32_t frame,
 		  IPAFrameContext &frameContext, RkISP1Params *params)
 {
-	if (!frameContext.dpf.update && frame > 0)
+	if (!frameContext.dpf.update && frame > 0) {
+		return;
+	}
+
+	if (!frameContext.dpf.denoise) {
+		prepareDisabledMode(context, frame, frameContext, params);
 		return;
+	}
+
+	prepareEnabledMode(context, frame, frameContext, params);
+}
+
+void Dpf::prepareDisabledMode([[maybe_unused]] IPAContext &context,
+			      [[maybe_unused]] const uint32_t frame,
+			      [[maybe_unused]] IPAFrameContext &frameContext,
+			      RkISP1Params *params)
+{
+	frameContext.dpf.denoise = false;
+	auto dpfConfig = params->block<BlockType::Dpf>();
+	dpfConfig.setEnabled(false);
+	auto dpfStrength = params->block<BlockType::DpfStrength>();
+	dpfStrength.setEnabled(false);
+}
 
-	auto config = params->block<BlockType::Dpf>();
-	config.setEnabled(frameContext.dpf.denoise);
-
-	auto strengthConfig = params->block<BlockType::DpfStrength>();
-	strengthConfig.setEnabled(frameContext.dpf.denoise);
-
-	if (frameContext.dpf.denoise) {
-		*config = config_;
-		*strengthConfig = strengthConfig_;
-
-		const auto &awb = context.configuration.awb;
-		const auto &lsc = context.configuration.lsc;
-
-		auto &mode = config->gain.mode;
-
-		/*
-		 * The DPF needs to take into account the total amount of
-		 * digital gain, which comes from the AWB and LSC modules. The
-		 * DPF hardware can be programmed with a digital gain value
-		 * manually, but can also use the gains supplied by the AWB and
-		 * LSC modules automatically when they are enabled. Use that
-		 * mode of operation as it simplifies control of the DPF.
-		 */
-		if (awb.enabled && lsc.enabled)
-			mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;
-		else if (awb.enabled)
-			mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;
-		else if (lsc.enabled)
-			mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;
-		else
-			mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
+void Dpf::prepareEnabledMode(IPAContext &context, [[maybe_unused]] const uint32_t frame,
+			     [[maybe_unused]] IPAFrameContext &frameContext, RkISP1Params *params)
+{
+	auto dpfConfig = params->block<BlockType::Dpf>();
+	dpfConfig.setEnabled(true);
+	*dpfConfig = config_;
+
+	/*
+	 * The DPF needs to take into account the total amount of
+	 * digital gain, which comes from the AWB and LSC modules. The
+	 * DPF hardware can be programmed with a digital gain value
+	 * manually, but can also use the gains supplied by the AWB and
+	 * LSC modules automatically when they are enabled. Use that
+	 * mode of operation as it simplifies control of the DPF.
+	 */
+	const auto &awb = context.configuration.awb;
+	const auto &lsc = context.configuration.lsc;
+	auto &mode = dpfConfig->gain.mode;
+
+	if (awb.enabled && lsc.enabled) {
+		mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;
+	} else if (awb.enabled) {
+		mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;
+	} else if (lsc.enabled) {
+		mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;
+	} else {
+		mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
 	}
+	config_.gain.mode = mode;
+
+	auto dpfStrength = params->block<BlockType::DpfStrength>();
+	dpfStrength.setEnabled(true);
+
+	*dpfStrength = strengthConfig_;
 }
 
 REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
index 2dd8cd36..163a5b1f 100644
--- a/src/ipa/rkisp1/algorithms/dpf.h
+++ b/src/ipa/rkisp1/algorithms/dpf.h
@@ -30,8 +30,27 @@  public:
 		     RkISP1Params *params) override;
 
 private:
+	struct ModeConfig {
+		int32_t modeValue;
+		rkisp1_cif_isp_dpf_config dpf;
+		rkisp1_cif_isp_dpf_strength_config strength;
+	};
+	bool parseConfig(const YamlObject &tuningData);
+	bool parseModes(const YamlObject &tuningData);
+	bool parseSingleConfig(const YamlObject &tuningData,
+			       rkisp1_cif_isp_dpf_config &config,
+			       rkisp1_cif_isp_dpf_strength_config &strengthConfig);
+	bool loadReductionConfig(int32_t mode);
+	void logConfigIfChanged(const IPAFrameContext &frameContext);
+	void prepareDisabledMode(IPAContext &context, const uint32_t frame,
+				 IPAFrameContext &frameContext,
+				 RkISP1Params *params);
+	void prepareEnabledMode(IPAContext &context, const uint32_t frame,
+				IPAFrameContext &frameContext, RkISP1Params *params);
 	struct rkisp1_cif_isp_dpf_config config_;
 	struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
+	std::vector<ModeConfig> noiseReductionModes_;
+	int32_t runningMode_;
 };
 
 } /* namespace ipa::rkisp1::algorithms */