[v5,5/6] ipa: rkisp1: algorithms: dpf: Add detailed config logging
diff mbox series

Message ID 20251214181646.573675-6-rui.wang@ideasonboard.com
State New
Headers show
Series
  • refactor DPF parsing and initialization
Related show

Commit Message

Rui Wang Dec. 14, 2025, 6:16 p.m. UTC 
Add logConfig() helper function to log DPF configuration
updates when they occur. This provides visibility into the active
DPF parameters including:

- Control mode and denoise enable state
- Filter sizes (9x9 vs 13x9 for rb)
- NLL scale mode (linear vs logarithmic)
- Gain mode
- Strength values (r, g, b)
- Spatial filter coefficients (g and rb arrays)
- Noise level lookup table coefficients

The logging is triggered in prepareEnabledMode() whenever the
configuration is updated, helping with debugging and tuning.

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

changelog :

- Renamed logConfigIfChanged to logConfig and updated implementation: 
  Moved the update check (if (update)) from inner to outer

 src/ipa/rkisp1/algorithms/dpf.cpp | 47 +++++++++++++++++++++++++++++++
 src/ipa/rkisp1/algorithms/dpf.h   |  1 +
 2 files changed, 48 insertions(+)

Comments

Jacopo Mondi Dec. 16, 2025, 5:03 p.m. UTC | #1 
Hi Rui

On Sun, Dec 14, 2025 at 01:16:45PM -0500, Rui Wang wrote:
> Add logConfig() helper function to log DPF configuration
> updates when they occur. This provides visibility into the active
> DPF parameters including:
>
> - Control mode and denoise enable state
> - Filter sizes (9x9 vs 13x9 for rb)
> - NLL scale mode (linear vs logarithmic)
> - Gain mode
> - Strength values (r, g, b)
> - Spatial filter coefficients (g and rb arrays)
> - Noise level lookup table coefficients
>
> The logging is triggered in prepareEnabledMode() whenever the
> configuration is updated, helping with debugging and tuning.
>
> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> ---
>
> changelog :
>
> - Renamed logConfigIfChanged to logConfig and updated implementation:
>   Moved the update check (if (update)) from inner to outer
>
>  src/ipa/rkisp1/algorithms/dpf.cpp | 47 +++++++++++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/dpf.h   |  1 +
>  2 files changed, 48 insertions(+)
>
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index 4818ef28..4342e580 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -271,6 +271,50 @@ bool Dpf::loadReductionConfig(int32_t mode)
>  	return true;
>  }
>
> +void Dpf::logConfig(const IPAFrameContext &frameContext)
> +{
> +	std::ostringstream ss;
> +
> +	ss << "DPF config update: ";
> +	ss << " control mode=" << kModesMap.at(runningMode_);
> +	ss << ", denoise=" << (frameContext.dpf.denoise ? "enabled" : "disabled");
> +
> +	ss << ", rb_fltsize="
> +	   << (config_.rb_flt.fltsize == RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 ? "13x9" : "9x9");
> +	ss << ", nll_scale="
> +	   << (config_.nll.scale_mode == RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC ? "log" : "linear");
> +	ss << ", gain_mode=" << config_.gain.mode;
> +	ss << ", strength=" << int(strengthConfig_.r) << ',' << int(strengthConfig_.g) << ',' << int(strengthConfig_.b);
> +
> +	ss << ", g=[";
> +	for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {
> +		if (i) {
> +			ss << ',';
> +		}

no braces for single line statements, here and in all other places.

Doesn't checkstyle.py complains ?

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

Thanks
  j

> +		ss << int(config_.g_flt.spatial_coeff[i]);
> +	}
> +	ss << "]";
> +
> +	ss << ", rb=[";
> +	for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {
> +		if (i) {
> +			ss << ',';
> +		}
> +		ss << int(config_.rb_flt.spatial_coeff[i]);
> +	}
> +	ss << "]";
> +
> +	ss << ", nll=[";
> +	for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS; ++i) {
> +		if (i) {
> +			ss << ',';
> +		}
> +		ss << int(config_.nll.coeff[i]);
> +	}
> +	ss << "]";
> +	LOG(RkISP1Dpf, Debug) << ss.str();
> +}
> +
>  /**
>   * \copydoc libcamera::ipa::Algorithm::queueRequest
>   */
> @@ -379,6 +423,9 @@ void Dpf::prepareEnabledMode(IPAContext &context, [[maybe_unused]] const uint32_
>  	auto strengthConfig = params->block<BlockType::DpfStrength>();
>  	strengthConfig.setEnabled(true);
>  	*strengthConfig = strengthConfig_;
> +
> +	if (frameContext.dpf.update)
> +		logConfig(frameContext);
>  }
>
>  REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> index fcf121cb..1821da39 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.h
> +++ b/src/ipa/rkisp1/algorithms/dpf.h
> @@ -43,6 +43,7 @@ private:
>  			      rkisp1_cif_isp_dpf_strength_config &strengthConfig);
>
>  	bool loadReductionConfig(int32_t mode);
> +	void logConfig(const IPAFrameContext &frameContext);
>
>  	void prepareDisabledMode(IPAContext &context, const uint32_t frame,
>  				 IPAFrameContext &frameContext,
> --
> 2.43.0
>
Rui Wang Dec. 18, 2025, 4:34 p.m. UTC | #2 
Quoting Jacopo Mondi (2025-12-16 12:03:56)
> Hi Rui
> 
> On Sun, Dec 14, 2025 at 01:16:45PM -0500, Rui Wang wrote:
> > Add logConfig() helper function to log DPF configuration
> > updates when they occur. This provides visibility into the active
> > DPF parameters including:
> >
> > - Control mode and denoise enable state
> > - Filter sizes (9x9 vs 13x9 for rb)
> > - NLL scale mode (linear vs logarithmic)
> > - Gain mode
> > - Strength values (r, g, b)
> > - Spatial filter coefficients (g and rb arrays)
> > - Noise level lookup table coefficients
> >
> > The logging is triggered in prepareEnabledMode() whenever the
> > configuration is updated, helping with debugging and tuning.
> >
> > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> > ---
> >
> > changelog :
> >
> > - Renamed logConfigIfChanged to logConfig and updated implementation:
> >   Moved the update check (if (update)) from inner to outer
> >
> >  src/ipa/rkisp1/algorithms/dpf.cpp | 47 +++++++++++++++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/dpf.h   |  1 +
> >  2 files changed, 48 insertions(+)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > index 4818ef28..4342e580 100644
> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > @@ -271,6 +271,50 @@ bool Dpf::loadReductionConfig(int32_t mode)
> >       return true;
> >  }
> >
> > +void Dpf::logConfig(const IPAFrameContext &frameContext)
> > +{
> > +     std::ostringstream ss;
> > +
> > +     ss << "DPF config update: ";
> > +     ss << " control mode=" << kModesMap.at(runningMode_);
> > +     ss << ", denoise=" << (frameContext.dpf.denoise ? "enabled" : "disabled");
> > +
> > +     ss << ", rb_fltsize="
> > +        << (config_.rb_flt.fltsize == RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 ? "13x9" : "9x9");
> > +     ss << ", nll_scale="
> > +        << (config_.nll.scale_mode == RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC ? "log" : "linear");
> > +     ss << ", gain_mode=" << config_.gain.mode;
> > +     ss << ", strength=" << int(strengthConfig_.r) << ',' << int(strengthConfig_.g) << ',' << int(strengthConfig_.b);
> > +
> > +     ss << ", g=[";
> > +     for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {
> > +             if (i) {
> > +                     ss << ',';
> > +             }
> 
> no braces for single line statements, here and in all other places.
> 
> Doesn't checkstyle.py complains ?
> 
Will updated in next series.
I run the utils/checkstyle.py when commit , it doesnot post any issue

> With that fixed
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> Thanks
>   j
> 
> > +             ss << int(config_.g_flt.spatial_coeff[i]);
> > +     }
> > +     ss << "]";
> > +
> > +     ss << ", rb=[";
> > +     for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {
> > +             if (i) {
> > +                     ss << ',';
> > +             }
> > +             ss << int(config_.rb_flt.spatial_coeff[i]);
> > +     }
> > +     ss << "]";
> > +
> > +     ss << ", nll=[";
> > +     for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS; ++i) {
> > +             if (i) {
> > +                     ss << ',';
> > +             }
> > +             ss << int(config_.nll.coeff[i]);
> > +     }
> > +     ss << "]";
> > +     LOG(RkISP1Dpf, Debug) << ss.str();
> > +}
> > +
> >  /**
> >   * \copydoc libcamera::ipa::Algorithm::queueRequest
> >   */
> > @@ -379,6 +423,9 @@ void Dpf::prepareEnabledMode(IPAContext &context, [[maybe_unused]] const uint32_
> >       auto strengthConfig = params->block<BlockType::DpfStrength>();
> >       strengthConfig.setEnabled(true);
> >       *strengthConfig = strengthConfig_;
> > +
> > +     if (frameContext.dpf.update)
> > +             logConfig(frameContext);
> >  }
> >
> >  REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> > index fcf121cb..1821da39 100644
> > --- a/src/ipa/rkisp1/algorithms/dpf.h
> > +++ b/src/ipa/rkisp1/algorithms/dpf.h
> > @@ -43,6 +43,7 @@ private:
> >                             rkisp1_cif_isp_dpf_strength_config &strengthConfig);
> >
> >       bool loadReductionConfig(int32_t mode);
> > +     void logConfig(const IPAFrameContext &frameContext);
> >
> >       void prepareDisabledMode(IPAContext &context, const uint32_t frame,
> >                                IPAFrameContext &frameContext,
> > --
> > 2.43.0
> >

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
index 4818ef28..4342e580 100644
--- a/src/ipa/rkisp1/algorithms/dpf.cpp
+++ b/src/ipa/rkisp1/algorithms/dpf.cpp
@@ -271,6 +271,50 @@  bool Dpf::loadReductionConfig(int32_t mode)
 	return true;
 }
 
+void Dpf::logConfig(const IPAFrameContext &frameContext)
+{
+	std::ostringstream ss;
+
+	ss << "DPF config update: ";
+	ss << " control mode=" << kModesMap.at(runningMode_);
+	ss << ", denoise=" << (frameContext.dpf.denoise ? "enabled" : "disabled");
+
+	ss << ", rb_fltsize="
+	   << (config_.rb_flt.fltsize == RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 ? "13x9" : "9x9");
+	ss << ", nll_scale="
+	   << (config_.nll.scale_mode == RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC ? "log" : "linear");
+	ss << ", gain_mode=" << config_.gain.mode;
+	ss << ", strength=" << int(strengthConfig_.r) << ',' << int(strengthConfig_.g) << ',' << int(strengthConfig_.b);
+
+	ss << ", g=[";
+	for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {
+		if (i) {
+			ss << ',';
+		}
+		ss << int(config_.g_flt.spatial_coeff[i]);
+	}
+	ss << "]";
+
+	ss << ", rb=[";
+	for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {
+		if (i) {
+			ss << ',';
+		}
+		ss << int(config_.rb_flt.spatial_coeff[i]);
+	}
+	ss << "]";
+
+	ss << ", nll=[";
+	for (size_t i = 0; i < RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS; ++i) {
+		if (i) {
+			ss << ',';
+		}
+		ss << int(config_.nll.coeff[i]);
+	}
+	ss << "]";
+	LOG(RkISP1Dpf, Debug) << ss.str();
+}
+
 /**
  * \copydoc libcamera::ipa::Algorithm::queueRequest
  */
@@ -379,6 +423,9 @@  void Dpf::prepareEnabledMode(IPAContext &context, [[maybe_unused]] const uint32_
 	auto strengthConfig = params->block<BlockType::DpfStrength>();
 	strengthConfig.setEnabled(true);
 	*strengthConfig = strengthConfig_;
+
+	if (frameContext.dpf.update)
+		logConfig(frameContext);
 }
 
 REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
index fcf121cb..1821da39 100644
--- a/src/ipa/rkisp1/algorithms/dpf.h
+++ b/src/ipa/rkisp1/algorithms/dpf.h
@@ -43,6 +43,7 @@  private:
 			      rkisp1_cif_isp_dpf_strength_config &strengthConfig);
 
 	bool loadReductionConfig(int32_t mode);
+	void logConfig(const IPAFrameContext &frameContext);
 
 	void prepareDisabledMode(IPAContext &context, const uint32_t frame,
 				 IPAFrameContext &frameContext,