[v10,4/7] ipa: rkisp1: algorithms: dpf: Refactor prepare() into helpers

25894 diff mbox series

Split the prepare() function into prepareDisabledMode() and
prepareEnabledMode() to improve readability and maintainability.

This separates the logic for disabling and enabling the DPF, making
the main prepare() function cleaner and easier to follow.

Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

---
changelog since v5/v6:  Nothing changed

changelog since v8 :
 - remove [[maybe_unused] for prepareEnable and prepareDisable

 Reviewed-by tags from v5 are carried over (no code changes).
---
 src/ipa/rkisp1/algorithms/dpf.cpp | 80 +++++++++++++++++++------------
 src/ipa/rkisp1/algorithms/dpf.h   |  3 ++
 2 files changed, 52 insertions(+), 31 deletions(-)

Hi Rui,

Quoting Rui Wang (2026-01-20 15:30:54)
> Split the prepare() function into prepareDisabledMode() and
> prepareEnabledMode() to improve readability and maintainability.
> 
> This separates the logic for disabling and enabling the DPF, making
> the main prepare() function cleaner and easier to follow.
> 
> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> ---
> changelog since v5/v6:  Nothing changed
> 
> changelog since v8 :
>  - remove [[maybe_unused] for prepareEnable and prepareDisable
> 
>  Reviewed-by tags from v5 are carried over (no code changes).
> ---
>  src/ipa/rkisp1/algorithms/dpf.cpp | 80 +++++++++++++++++++------------
>  src/ipa/rkisp1/algorithms/dpf.h   |  3 ++
>  2 files changed, 52 insertions(+), 31 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index 21ce8ace..ff2cbc28 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -358,40 +358,58 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>         if (!frameContext.dpf.update && frame > 0)
>                 return;
>  
> +       if (!frameContext.dpf.denoise) {
> +               prepareDisabledMode(params);
> +               return;
> +       }
> +
> +       prepareEnabledMode(context, params);
> +}
> +
> +void Dpf::prepareDisabledMode(RkISP1Params *params)
> +{
> +       auto dpfConfig = params->block<BlockType::Dpf>();
> +       dpfConfig.setEnabled(false);
> +       auto dpfStrength = params->block<BlockType::DpfStrength>();
> +       dpfStrength.setEnabled(false);
> +}
> +
> +void Dpf::prepareEnabledMode(IPAContext &context, RkISP1Params *params)
> +{
> +       if (activeMode_ == noiseReductionModes_.end())
> +               return;
> +
> +       const ModeConfig &modeConfig = *activeMode_;
> +
>         auto config = params->block<BlockType::Dpf>();
> -       config.setEnabled(frameContext.dpf.denoise);
> +       config.setEnabled(true);
> +       *config = modeConfig.dpf;
> +
> +       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;

Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>

>  
>         auto strengthConfig = params->block<BlockType::DpfStrength>();
> -       strengthConfig.setEnabled(frameContext.dpf.denoise);
> -
> -       if (frameContext.dpf.denoise) {
> -               const ModeConfig &modeConfig = *activeMode_;
> -
> -               *config = modeConfig.dpf;
> -               *strengthConfig = modeConfig.strength;
> -
> -               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;
> -       }
> +       strengthConfig.setEnabled(true);
> +       *strengthConfig = modeConfig.strength;
>  }
>  
>  REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> index 43effcbe..208d8fe9 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.h
> +++ b/src/ipa/rkisp1/algorithms/dpf.h
> @@ -44,6 +44,9 @@ private:
>  
>         bool loadConfig(int32_t mode);
>  
> +       void prepareDisabledMode(RkISP1Params *params);
> +       void prepareEnabledMode(IPAContext &context, RkISP1Params *params);
> +
>         std::vector<ModeConfig> noiseReductionModes_;
>         std::vector<ModeConfig>::const_iterator activeMode_;
>  };
> -- 
> 2.43.0
>