[libcamera-devel,3/3] ipa: rkisp1: Add support of De-noise Pre-Filter control
diff mbox series

Message ID 20220804141228.417211-4-fsylvestre@baylibre.com
State Superseded
Headers show
Series
  • Add DPF tuning support for RkISP1
Related show

Commit Message

Florian Sylvestre Aug. 4, 2022, 2:12 p.m. UTC 
The De-noise pre-filter algorithm is a bilateral filter which combines a range
filter and a domain filter. The De-noise pre-filter is applied before
demosaicing.

Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
---
 src/ipa/rkisp1/algorithms/dpf.cpp     | 279 ++++++++++++++++++++++++++
 src/ipa/rkisp1/algorithms/dpf.h       |  36 ++++
 src/ipa/rkisp1/algorithms/meson.build |   1 +
 src/ipa/rkisp1/data/ov5640.yaml       |  11 +
 src/ipa/rkisp1/ipa_context.cpp        |  11 +
 src/ipa/rkisp1/ipa_context.h          |   5 +
 6 files changed, 343 insertions(+)
 create mode 100644 src/ipa/rkisp1/algorithms/dpf.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/dpf.h

Comments

Laurent Pinchart Aug. 5, 2022, 1:11 a.m. UTC | #1 
Hi Florian,

Thank you for the patch.

On Thu, Aug 04, 2022 at 04:12:28PM +0200, Florian Sylvestre via libcamera-devel wrote:
> The De-noise pre-filter algorithm is a bilateral filter which combines a range

I would write "de-noise" without a capital D, and probably "denoise"
actually. Same below where applicable.

> filter and a domain filter. The De-noise pre-filter is applied before
> demosaicing.
> 
> Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com>
> ---
>  src/ipa/rkisp1/algorithms/dpf.cpp     | 279 ++++++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/dpf.h       |  36 ++++
>  src/ipa/rkisp1/algorithms/meson.build |   1 +
>  src/ipa/rkisp1/data/ov5640.yaml       |  11 +
>  src/ipa/rkisp1/ipa_context.cpp        |  11 +
>  src/ipa/rkisp1/ipa_context.h          |   5 +
>  6 files changed, 343 insertions(+)
>  create mode 100644 src/ipa/rkisp1/algorithms/dpf.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/dpf.h
> 
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> new file mode 100644
> index 00000000..d99bb728
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -0,0 +1,279 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * dpf.cpp - RkISP1 De-noise Pre-Filter control
> + */
> +
> +#include "dpf.h"
> +
> +#include <cmath>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
> +#include "linux/rkisp1-config.h"
> +
> +/**
> + * \file dpf.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class Dpf
> + * \brief RkISP1 De-noise Pre-Filter control
> + *
> + * The De-noise pre-filter algorithm is a bilateral filter which combines a
> + * range filter and a domain filter. The De-noise pre-filter is applied before
> + * demosaicing.
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Dpf)
> +
> +Dpf::Dpf()
> +	: initialized_(false), config_({}), strength_config_({})
> +{
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Dpf::init([[maybe_unused]] IPAContext &context,
> +	      const YamlObject &tuningData)
> +{
> +	std::vector<double> values;
> +
> +	/*
> +	 * 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"];
> +	if (!dFObject.isDictionary()) {
> +		LOG(RkISP1Dpf, Error)
> +			<< "Issue while parsing 'DomainFilter' in tuning "
> +			<< "file: entry must be a dictionary";
> +		return -EINVAL;
> +	}

I think you could drop the error handling here. If dFObject isn't
correct, then the check on dFObject["g"] below will fail, and that
should be good enough.

> +
> +	/*
> +	 * For the green component, we have the 9x9 kernel specified
> +	 * as 6 coefficients:
> +	 *    Y
> +	 *    ^
> +	 *  4 | 6   5   4   5   6
> +	 *  3 |   5   3   3   5
> +	 *  2 | 5   3   2   3   5
> +	 *  1 |   3   1   1   3
> +	 *  0 - 4   2   0   2   4
> +	 * -1 |   3   1   1   3
> +	 * -2 | 5   3   2   3   5
> +	 * -3 |   5   3   3   5
> +	 * -4 | 6   5   4   5   6
> +	 *    +---------|--------> X
> +	 *     -4....-1 0 1 2 3 4
> +	 */
> +	values = dFObject["g"].getList<double>().value_or(utils::defopt);
> +	if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS) {
> +		LOG(RkISP1Dpf, Error)
> +			<< "Invalid 'DomainFilter:g': expected "
> +			<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
> +			<< " elements, got " << values.size();
> +		return -EINVAL;
> +	}
> +
> +	for (unsigned int i = 0; i < values.size(); ++i) {
> +		config_.g_flt.spatial_coeff[i] = std::round(values[i] * 16);
> +	}

No need for curly braces.

I wonder if we should store the values as integers in the config file ?
That would simplify the code here, you could then even write it as

	std::copy_n(values.begin(), values.size(),
		    std::begin(config_.g_flt.spatial_coeff));

> +
> +	config_.g_flt.gr_enable = true;
> +	config_.g_flt.gb_enable = true;
> +
> +	/*
> +	 * For the red and blue components, we have the 13x9 kernel specified
> +	 * as 6 coefficients:
> +	 *
> +	 *    Y
> +	 *    ^
> +	 *  4 | 6   5   4   3   4   5   6
> +	 *    |
> +	 *  2 | 5   4   2   1   2   4   5
> +	 *    |
> +	 *  0 - 5   3   1   0   1   3   5
> +	 *    |
> +	 * -2 | 5   4   2   1   2   4   5
> +	 *    |
> +	 * -4 | 6   5   4   3   4   5   6
> +	 *    +-------------|------------> X
> +	 *     -6  -4  -2   0   2   4   6
> +	 *
> +	 * For a 9x9 kernel, columns -6 and -6 are dropped, so coefficient

"-6 and 6"

> +	 * number 6 is not used.
> +	 */
> +	values = dFObject["rb"].getList<double>().value_or(utils::defopt);
> +	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 "
> +			<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1
> +			<< " or " << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
> +			<< " elements, got " << values.size();
> +		return -EINVAL;
> +	}
> +
> +	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;

I wonder if there would ever be a use case for switching between two
different filter sizes at runtime. Not something we need to bother with
for now, this looks nice.

> +
> +	for (unsigned int i = 0; i < values.size(); ++i) {
> +		config_.rb_flt.spatial_coeff[i] = std::round(values[i] * 16);
> +	}

Same as above.

> +
> +	config_.rb_flt.r_enable = true;
> +	config_.rb_flt.b_enable = true;
> +
> +	/*
> +	 * The range kernel is configured with a noise level lookup table (NLL)
> +	 * which stores a piecewise linear function that characterizes the
> +	 * sensor noise profile as a noise level function curve (NLF).
> +	 */
> +	const YamlObject &rFObject = tuningData["RangeFilter"];

The data in the tuning file describe the noise level function, so I'd
name the element "NoiseLevelFunction" or something similar.

> +	if (!rFObject.isDictionary()) {
> +		LOG(RkISP1Dpf, Error)
> +			<< "Issue while parsing 'RangeFilter' in tuning "
> +			<< "file: entry must be a dictionary";
> +		return -EINVAL;
> +	}

Same as above for the error check.

> +
> +	values = rFObject["coeff"].getList<double>().value_or(utils::defopt);
> +	if (values.size() != RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS) {
> +		LOG(RkISP1Dpf, Error)
> +			<< "Invalid 'RangeFilter:coeff': expected "
> +			<< RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS
> +			<< " elements, got " << values.size();
> +		return -EINVAL;
> +	}
> +
> +	for (unsigned int i = 0; i < values.size(); ++i) {
> +		config_.nll.coeff[i] = std::round(values[i] * 1024);
> +	}
> +
> +	std::string scaleMode = rFObject["scale-mode"].get<std::string>("");
> +	if (scaleMode != "linear" && scaleMode != "logarithmic") {
> +		LOG(RkISP1Dpf, Error)
> +			<< "Invalid 'RangeFilter:scale-mode': expected "
> +			<< "'linear' or 'logarithmic' value, got "
> +			<< scaleMode;
> +		return -EINVAL;
> +	}
> +
> +	config_.nll.scale_mode = scaleMode == "linear"
> +			       ? RKISP1_CIF_ISP_NLL_SCALE_LINEAR
> +			       : RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC;

You could avoid comparing to "linear" twice:

	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 "
			<< "'linear' or 'logarithmic' value, got "
			<< scaleMode;
		return -EINVAL;
	}

(I wish a switch/case on a string would be possible)

> +
> +	const YamlObject &fSObject = tuningData["FilterStrength"];
> +	if (!fSObject.isDictionary()) {
> +		LOG(RkISP1Dpf, Error)
> +			<< "Issue while parsing 'FilterStrength' in tuning "
> +			<< "file: entry must be a dictionary";
> +		return -EINVAL;
> +	}
> +
> +	strength_config_.r = fSObject["r"].get<uint16_t>(64);
> +	strength_config_.g = fSObject["g"].get<uint16_t>(64);
> +	strength_config_.b = fSObject["b"].get<uint16_t>(64);
> +
> +	initialized_ = true;
> +
> +	return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Dpf::queueRequest(IPAContext &context,
> +		       [[maybe_unused]] const uint32_t frame,
> +		       const ControlList &controls)
> +{
> +	auto &dpf = context.frameContext.dpf;
> +
> +	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> +	if (denoise) {
> +		LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
> +
> +		switch (*denoise) {
> +		case controls::draft::NoiseReductionModeOff:
> +			dpf.denoise = false;
> +			dpf.updateParams = true;
> +			break;
> +		case controls::draft::NoiseReductionModeMinimal:
> +		case controls::draft::NoiseReductionModeHighQuality:
> +		case controls::draft::NoiseReductionModeFast:
> +			dpf.denoise = true;
> +			dpf.updateParams = true;
> +			break;
> +		default:
> +			LOG(RkISP1Dpf, Error)
> +				<< "Unsupported denoise value "
> +				<< *denoise;
> +		}
> +	}
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void Dpf::prepare(IPAContext &context, rkisp1_params_cfg *params)
> +{
> +	auto &dpf = context.frameContext.dpf;
> +	auto &awb = context.configuration.awb;
> +	auto &lsc = context.configuration.lsc;
> +	auto &dpf_config = params->others.dpf_config;
> +	auto &dpf_strength_config = params->others.dpf_strength_config;
> +
> +	if ((context.frameContext.frameCount > 0) && (!dpf.updateParams))

No need for the inner parentheses.

> +		return;
> +
> +	if (!initialized_)
> +		return;

Move this to the very beginning, before the variables.

> +
> +	/*
> +	 * We must copy the static configuration each time we modify a
> +	 * parameter in the config because the strucure is zeroed between each
> +	 * call.
> +	 */
> +	dpf_config = config_;
> +	dpf_strength_config = strength_config_;
> +
> +	if (dpf.updateParams) {
> +		if (!dpf.denoise || (!awb.enabled && !lsc.enabled)) {
> +			dpf_config.gain.mode =
> +				RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
> +		} else if (awb.enabled && lsc.enabled) {
> +			dpf_config.gain.mode =
> +				RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;
> +		} else if (awb.enabled) {
> +			dpf_config.gain.mode =
> +				RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;
> +		} else { /* lsc.enabled only */
> +			dpf_config.gain.mode =
> +				RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;
> +		}
> +	}
> +
> +	dpf.updateParams = false;
> +
> +	uint32_t flags = RKISP1_CIF_ISP_MODULE_DPF |
> +			 RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
> +	params->module_en_update |= flags;
> +	params->module_ens |= flags;
> +	params->module_cfg_update |= flags;

I don't think this is right. When denoising is disabled, you should
clear RKISP1_CIF_ISP_MODULE_DPF in module_ens to disable it.

The module_cfg_update field indicates which ISP configuration parameters
need to be changed, so it should only be set for the very first frame. I
think the following should do.

	if (context.frameContext.frameCount == 0) {
		dpf_config = config_;
		dpf_strength_config = strength_config_;

		if (awb.enabled && lsc.enabled) {
			dpf_config.gain.mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;
		else if (awb.enabled)
			dpf_config.gain.mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;
		else if (lsc.enabled)
			dpf_config.gain.mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;
		else
			dpf_config.gain.mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;

		params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF
					  |  RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
	}

	if (dpf.updateParams) {
		params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;
		if (dpf.denoise)
			params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;

		dpf.updateParams = false;
	}

> +}
> +
> +REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> new file mode 100644
> index 00000000..c8b8419a
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/dpf.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * dpf.h - RkISP1 De-noise Pre-Filter control
> + */
> +
> +#pragma once
> +
> +#include <sys/types.h>
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class Dpf : public Algorithm
> +{
> +public:
> +	Dpf();
> +	~Dpf() = default;
> +
> +	int init(IPAContext &context, const YamlObject &tuningData) override;
> +	void queueRequest(IPAContext &context, const uint32_t frame,
> +			  const ControlList &controls) override;
> +	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> +
> +private:
> +	bool initialized_;
> +	struct rkisp1_cif_isp_dpf_config config_;
> +	struct rkisp1_cif_isp_dpf_strength_config strength_config_;

strengthConfig_

> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index e48974b4..e1492dd5 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([
>      'awb.cpp',
>      'blc.cpp',
>      'cproc.cpp',
> +    'dpf.cpp',
>      'dpcc.cpp',

You nearly got the alphabetical order right :-)

>      'filter.cpp',
>      'gsl.cpp',
> diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> index 93d7d1e7..69c6ad97 100644
> --- a/src/ipa/rkisp1/data/ov5640.yaml
> +++ b/src/ipa/rkisp1/data/ov5640.yaml
> @@ -11,6 +11,17 @@ algorithms:
>        Gb: 256
>        B:  256
>    - ColorProcessing:
> +  - Dpf:
> +      DomainFilter:
> +        g: [ 1.0, 1.0, 1.0, 1.0, 1.0, 1.0 ]
> +        rb: [ 1.0, 1.0, 1.0, 1.0, 1.0, 1.0 ]
> +      RangeFilter:
> +        coeff: [ 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999 ]
> +        scale-mode: "linear"
> +      FilterStrength:
> +        r: 64
> +        g: 64
> +        b: 64

It won't make a difference for now, but I'd move this after LSC as the
DPF depends on the LSC gains. Actually, I think it would make sense to
sort the algorithms according to the hardware order, with exceptions
where needed.

>    - GammaSensorLinearization:
>        x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ]
>        y:
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index c90bf992..3b92e95e 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -165,6 +165,17 @@ namespace libcamera::ipa::rkisp1 {
>   * \brief Indicates if ISP parameters need to be updated
>   */
>  
> +/**
> + * \var IPAFrameContext::dpf
> + * \brief Context for the De-noise Pre-Filter algorithm
> + *
> + * \var IPAFrameContext::dpf.denoise
> + * \brief Indicates if de-noise is activated
> + *
> + * \var IPAFrameContext::dpf.updateParams
> + * \brief Indicates if ISP parameters need to be updated
> + */
> +
>  /**
>   * \var IPAFrameContext::filter
>   * \brief Context for the Filter algorithm
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 0cd6aadb..3a743ac3 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -69,6 +69,11 @@ struct IPAFrameContext {
>  		bool updateParams;
>  	} cproc;
>  
> +	struct {
> +		bool denoise;
> +		bool updateParams;
> +	} dpf;
> +
>  	struct {
>  		uint8_t denoise;
>  		uint8_t sharpness;

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
new file mode 100644
index 00000000..d99bb728
--- /dev/null
+++ b/src/ipa/rkisp1/algorithms/dpf.cpp
@@ -0,0 +1,279 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021-2022, Ideas On Board
+ *
+ * dpf.cpp - RkISP1 De-noise Pre-Filter control
+ */
+
+#include "dpf.h"
+
+#include <cmath>
+
+#include <libcamera/base/log.h>
+
+#include <libcamera/control_ids.h>
+#include "linux/rkisp1-config.h"
+
+/**
+ * \file dpf.h
+ */
+
+namespace libcamera {
+
+namespace ipa::rkisp1::algorithms {
+
+/**
+ * \class Dpf
+ * \brief RkISP1 De-noise Pre-Filter control
+ *
+ * The De-noise pre-filter algorithm is a bilateral filter which combines a
+ * range filter and a domain filter. The De-noise pre-filter is applied before
+ * demosaicing.
+ */
+
+LOG_DEFINE_CATEGORY(RkISP1Dpf)
+
+Dpf::Dpf()
+	: initialized_(false), config_({}), strength_config_({})
+{
+}
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::init
+ */
+int Dpf::init([[maybe_unused]] IPAContext &context,
+	      const YamlObject &tuningData)
+{
+	std::vector<double> values;
+
+	/*
+	 * 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"];
+	if (!dFObject.isDictionary()) {
+		LOG(RkISP1Dpf, Error)
+			<< "Issue while parsing 'DomainFilter' in tuning "
+			<< "file: entry must be a dictionary";
+		return -EINVAL;
+	}
+
+	/*
+	 * For the green component, we have the 9x9 kernel specified
+	 * as 6 coefficients:
+	 *    Y
+	 *    ^
+	 *  4 | 6   5   4   5   6
+	 *  3 |   5   3   3   5
+	 *  2 | 5   3   2   3   5
+	 *  1 |   3   1   1   3
+	 *  0 - 4   2   0   2   4
+	 * -1 |   3   1   1   3
+	 * -2 | 5   3   2   3   5
+	 * -3 |   5   3   3   5
+	 * -4 | 6   5   4   5   6
+	 *    +---------|--------> X
+	 *     -4....-1 0 1 2 3 4
+	 */
+	values = dFObject["g"].getList<double>().value_or(utils::defopt);
+	if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS) {
+		LOG(RkISP1Dpf, Error)
+			<< "Invalid 'DomainFilter:g': expected "
+			<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
+			<< " elements, got " << values.size();
+		return -EINVAL;
+	}
+
+	for (unsigned int i = 0; i < values.size(); ++i) {
+		config_.g_flt.spatial_coeff[i] = std::round(values[i] * 16);
+	}
+
+	config_.g_flt.gr_enable = true;
+	config_.g_flt.gb_enable = true;
+
+	/*
+	 * For the red and blue components, we have the 13x9 kernel specified
+	 * as 6 coefficients:
+	 *
+	 *    Y
+	 *    ^
+	 *  4 | 6   5   4   3   4   5   6
+	 *    |
+	 *  2 | 5   4   2   1   2   4   5
+	 *    |
+	 *  0 - 5   3   1   0   1   3   5
+	 *    |
+	 * -2 | 5   4   2   1   2   4   5
+	 *    |
+	 * -4 | 6   5   4   3   4   5   6
+	 *    +-------------|------------> X
+	 *     -6  -4  -2   0   2   4   6
+	 *
+	 * For a 9x9 kernel, columns -6 and -6 are dropped, so coefficient
+	 * number 6 is not used.
+	 */
+	values = dFObject["rb"].getList<double>().value_or(utils::defopt);
+	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 "
+			<< RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1
+			<< " or " << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
+			<< " elements, got " << values.size();
+		return -EINVAL;
+	}
+
+	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;
+
+	for (unsigned int i = 0; i < values.size(); ++i) {
+		config_.rb_flt.spatial_coeff[i] = std::round(values[i] * 16);
+	}
+
+	config_.rb_flt.r_enable = true;
+	config_.rb_flt.b_enable = true;
+
+	/*
+	 * The range kernel is configured with a noise level lookup table (NLL)
+	 * which stores a piecewise linear function that characterizes the
+	 * sensor noise profile as a noise level function curve (NLF).
+	 */
+	const YamlObject &rFObject = tuningData["RangeFilter"];
+	if (!rFObject.isDictionary()) {
+		LOG(RkISP1Dpf, Error)
+			<< "Issue while parsing 'RangeFilter' in tuning "
+			<< "file: entry must be a dictionary";
+		return -EINVAL;
+	}
+
+	values = rFObject["coeff"].getList<double>().value_or(utils::defopt);
+	if (values.size() != RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS) {
+		LOG(RkISP1Dpf, Error)
+			<< "Invalid 'RangeFilter:coeff': expected "
+			<< RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS
+			<< " elements, got " << values.size();
+		return -EINVAL;
+	}
+
+	for (unsigned int i = 0; i < values.size(); ++i) {
+		config_.nll.coeff[i] = std::round(values[i] * 1024);
+	}
+
+	std::string scaleMode = rFObject["scale-mode"].get<std::string>("");
+	if (scaleMode != "linear" && scaleMode != "logarithmic") {
+		LOG(RkISP1Dpf, Error)
+			<< "Invalid 'RangeFilter:scale-mode': expected "
+			<< "'linear' or 'logarithmic' value, got "
+			<< scaleMode;
+		return -EINVAL;
+	}
+
+	config_.nll.scale_mode = scaleMode == "linear"
+			       ? RKISP1_CIF_ISP_NLL_SCALE_LINEAR
+			       : RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC;
+
+	const YamlObject &fSObject = tuningData["FilterStrength"];
+	if (!fSObject.isDictionary()) {
+		LOG(RkISP1Dpf, Error)
+			<< "Issue while parsing 'FilterStrength' in tuning "
+			<< "file: entry must be a dictionary";
+		return -EINVAL;
+	}
+
+	strength_config_.r = fSObject["r"].get<uint16_t>(64);
+	strength_config_.g = fSObject["g"].get<uint16_t>(64);
+	strength_config_.b = fSObject["b"].get<uint16_t>(64);
+
+	initialized_ = true;
+
+	return 0;
+}
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::queueRequest
+ */
+void Dpf::queueRequest(IPAContext &context,
+		       [[maybe_unused]] const uint32_t frame,
+		       const ControlList &controls)
+{
+	auto &dpf = context.frameContext.dpf;
+
+	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
+	if (denoise) {
+		LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
+
+		switch (*denoise) {
+		case controls::draft::NoiseReductionModeOff:
+			dpf.denoise = false;
+			dpf.updateParams = true;
+			break;
+		case controls::draft::NoiseReductionModeMinimal:
+		case controls::draft::NoiseReductionModeHighQuality:
+		case controls::draft::NoiseReductionModeFast:
+			dpf.denoise = true;
+			dpf.updateParams = true;
+			break;
+		default:
+			LOG(RkISP1Dpf, Error)
+				<< "Unsupported denoise value "
+				<< *denoise;
+		}
+	}
+}
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::prepare
+ */
+void Dpf::prepare(IPAContext &context, rkisp1_params_cfg *params)
+{
+	auto &dpf = context.frameContext.dpf;
+	auto &awb = context.configuration.awb;
+	auto &lsc = context.configuration.lsc;
+	auto &dpf_config = params->others.dpf_config;
+	auto &dpf_strength_config = params->others.dpf_strength_config;
+
+	if ((context.frameContext.frameCount > 0) && (!dpf.updateParams))
+		return;
+
+	if (!initialized_)
+		return;
+
+	/*
+	 * We must copy the static configuration each time we modify a
+	 * parameter in the config because the strucure is zeroed between each
+	 * call.
+	 */
+	dpf_config = config_;
+	dpf_strength_config = strength_config_;
+
+	if (dpf.updateParams) {
+		if (!dpf.denoise || (!awb.enabled && !lsc.enabled)) {
+			dpf_config.gain.mode =
+				RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
+		} else if (awb.enabled && lsc.enabled) {
+			dpf_config.gain.mode =
+				RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;
+		} else if (awb.enabled) {
+			dpf_config.gain.mode =
+				RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;
+		} else { /* lsc.enabled only */
+			dpf_config.gain.mode =
+				RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;
+		}
+	}
+
+	dpf.updateParams = false;
+
+	uint32_t flags = RKISP1_CIF_ISP_MODULE_DPF |
+			 RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
+	params->module_en_update |= flags;
+	params->module_ens |= flags;
+	params->module_cfg_update |= flags;
+}
+
+REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
+
+} /* namespace ipa::rkisp1::algorithms */
+
+} /* namespace libcamera */
diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
new file mode 100644
index 00000000..c8b8419a
--- /dev/null
+++ b/src/ipa/rkisp1/algorithms/dpf.h
@@ -0,0 +1,36 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021-2022, Ideas On Board
+ *
+ * dpf.h - RkISP1 De-noise Pre-Filter control
+ */
+
+#pragma once
+
+#include <sys/types.h>
+
+#include "algorithm.h"
+
+namespace libcamera {
+
+namespace ipa::rkisp1::algorithms {
+
+class Dpf : public Algorithm
+{
+public:
+	Dpf();
+	~Dpf() = default;
+
+	int init(IPAContext &context, const YamlObject &tuningData) override;
+	void queueRequest(IPAContext &context, const uint32_t frame,
+			  const ControlList &controls) override;
+	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
+
+private:
+	bool initialized_;
+	struct rkisp1_cif_isp_dpf_config config_;
+	struct rkisp1_cif_isp_dpf_strength_config strength_config_;
+};
+
+} /* namespace ipa::rkisp1::algorithms */
+} /* namespace libcamera */
diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
index e48974b4..e1492dd5 100644
--- a/src/ipa/rkisp1/algorithms/meson.build
+++ b/src/ipa/rkisp1/algorithms/meson.build
@@ -5,6 +5,7 @@  rkisp1_ipa_algorithms = files([
     'awb.cpp',
     'blc.cpp',
     'cproc.cpp',
+    'dpf.cpp',
     'dpcc.cpp',
     'filter.cpp',
     'gsl.cpp',
diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
index 93d7d1e7..69c6ad97 100644
--- a/src/ipa/rkisp1/data/ov5640.yaml
+++ b/src/ipa/rkisp1/data/ov5640.yaml
@@ -11,6 +11,17 @@  algorithms:
       Gb: 256
       B:  256
   - ColorProcessing:
+  - Dpf:
+      DomainFilter:
+        g: [ 1.0, 1.0, 1.0, 1.0, 1.0, 1.0 ]
+        rb: [ 1.0, 1.0, 1.0, 1.0, 1.0, 1.0 ]
+      RangeFilter:
+        coeff: [ 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999, 0.999 ]
+        scale-mode: "linear"
+      FilterStrength:
+        r: 64
+        g: 64
+        b: 64
   - GammaSensorLinearization:
       x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ]
       y:
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index c90bf992..3b92e95e 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -165,6 +165,17 @@  namespace libcamera::ipa::rkisp1 {
  * \brief Indicates if ISP parameters need to be updated
  */
 
+/**
+ * \var IPAFrameContext::dpf
+ * \brief Context for the De-noise Pre-Filter algorithm
+ *
+ * \var IPAFrameContext::dpf.denoise
+ * \brief Indicates if de-noise is activated
+ *
+ * \var IPAFrameContext::dpf.updateParams
+ * \brief Indicates if ISP parameters need to be updated
+ */
+
 /**
  * \var IPAFrameContext::filter
  * \brief Context for the Filter algorithm
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 0cd6aadb..3a743ac3 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -69,6 +69,11 @@  struct IPAFrameContext {
 		bool updateParams;
 	} cproc;
 
+	struct {
+		bool denoise;
+		bool updateParams;
+	} dpf;
+
 	struct {
 		uint8_t denoise;
 		uint8_t sharpness;