[v2] ipa: rkisp1: dpf: Enable strength after enable Dpf

24996 diff mbox series

The filter strength configuration block was previously only enabled on the
first frame (frame == 0). Apply the strength values when denoise is active.
This prevents the strength config from being disabled on subsequent frames.

Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/dpf.cpp | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Hi Rui,

Thank you for the patch.

Quoting Rui Wang (2025-11-11 15:53:26)
> The filter strength configuration block was previously only enabled on the
> first frame (frame == 0). Apply the strength values when denoise is active.
> This prevents the strength config from being disabled on subsequent frames.
> 
> Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

You also got a reviewed-by tag from Kieran on v1. The usual process is
to collect that (either using b4 or manually) and add it before sending
out a v2.

Additionally a short changelog is usually added below the commit
message separated by '---'. How this changelog is added depends on
personal preferences. I usually add it when reworking a commit which
also helps myself in not forgetting what I changed.

Could you send a v3 just with the collected tags added?

I'll then merge it to mainline.

Best regards,
Stefan


> ---
>  src/ipa/rkisp1/algorithms/dpf.cpp | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index cb6095da..39f3e461 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -225,8 +225,12 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>         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;
> @@ -250,12 +254,6 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>                 else
>                         mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
>         }
> -
> -       if (frame == 0) {
> -               auto strengthConfig = params->block<BlockType::DpfStrength>();
> -               strengthConfig.setEnabled(true);
> -               *strengthConfig = strengthConfig_;
> -       }
>  }
>  
>  REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
> -- 
> 2.43.0
>

Quoting Stefan Klug (2025-11-14 16:52:52)
> Hi Rui,
> 
> Thank you for the patch.
> 
> Quoting Rui Wang (2025-11-11 15:53:26)
> > The filter strength configuration block was previously only enabled on the
> > first frame (frame == 0). Apply the strength values when denoise is active.
> > This prevents the strength config from being disabled on subsequent frames.
> > 
> > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
> 
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 
> 
> You also got a reviewed-by tag from Kieran on v1. The usual process is
> to collect that (either using b4 or manually) and add it before sending
> out a v2.
> 
> Additionally a short changelog is usually added below the commit
> message separated by '---'. How this changelog is added depends on
> personal preferences. I usually add it when reworking a commit which
> also helps myself in not forgetting what I changed.
> 
> Could you send a v3 just with the collected tags added?

If this is just missing my tag I'll add it for patchworks sake:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> I'll then merge it to mainline.
> 
> Best regards,
> Stefan
> 
> 
> > ---
> >  src/ipa/rkisp1/algorithms/dpf.cpp | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > index cb6095da..39f3e461 100644
> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > @@ -225,8 +225,12 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
> >         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;
> > @@ -250,12 +254,6 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
> >                 else
> >                         mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
> >         }
> > -
> > -       if (frame == 0) {
> > -               auto strengthConfig = params->block<BlockType::DpfStrength>();
> > -               strengthConfig.setEnabled(true);
> > -               *strengthConfig = strengthConfig_;
> > -       }
> >  }
> >  
> >  REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
> > -- 
> > 2.43.0
> >