[libcamera-devel,v4,22/32] ipa: rkisp1: dpf: Store per-frame information in frame context
diff mbox series

Message ID 20220908014200.28728-23-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: Frame context queue, IPU3 & RkISP consolidation, and RkISP1 improvements
Related show

Commit Message

Laurent Pinchart Sept. 8, 2022, 1:41 a.m. UTC 
Rework the algorithm's usage of the active state, to store the value of
controls for the last queued request in the queueRequest() function, and
store a copy of the values in the corresponding frame context. The
latter is used in the prepare() function to populate the ISP parameters
with values corresponding to the right frame.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/dpf.cpp | 22 +++++++++++-----------
 src/ipa/rkisp1/ipa_context.cpp    | 15 ++++++++++++---
 src/ipa/rkisp1/ipa_context.h      |  6 +++++-
 3 files changed, 28 insertions(+), 15 deletions(-)

Comments

Kieran Bingham Sept. 20, 2022, 11:05 p.m. UTC | #1 
Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:50)
> Rework the algorithm's usage of the active state, to store the value of
> controls for the last queued request in the queueRequest() function, and
> store a copy of the values in the corresponding frame context. The
> latter is used in the prepare() function to populate the ISP parameters
> with values corresponding to the right frame.
> 


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

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/dpf.cpp | 22 +++++++++++-----------
>  src/ipa/rkisp1/ipa_context.cpp    | 15 ++++++++++++---
>  src/ipa/rkisp1/ipa_context.h      |  6 +++++-
>  3 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index 94c35c36570c..5d44480c5059 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -176,10 +176,11 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
>   */
>  void Dpf::queueRequest(IPAContext &context,
>                        [[maybe_unused]] const uint32_t frame,
> -                      [[maybe_unused]] RkISP1FrameContext &frameContext,
> +                      RkISP1FrameContext &frameContext,
>                        const ControlList &controls)
>  {
>         auto &dpf = context.activeState.dpf;
> +       bool update = false;
>  
>         const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
>         if (denoise) {
> @@ -188,34 +189,35 @@ void Dpf::queueRequest(IPAContext &context,
>                 switch (*denoise) {
>                 case controls::draft::NoiseReductionModeOff:
>                         dpf.denoise = false;
> -                       dpf.updateParams = true;
> +                       update = true;
>                         break;
>                 case controls::draft::NoiseReductionModeMinimal:
>                 case controls::draft::NoiseReductionModeHighQuality:
>                 case controls::draft::NoiseReductionModeFast:
>                         dpf.denoise = true;
> -                       dpf.updateParams = true;
> +                       update = true;
>                         break;
>                 default:
>                         LOG(RkISP1Dpf, Error)
>                                 << "Unsupported denoise value "
>                                 << *denoise;
> +                       break;
>                 }
>         }
> +
> +       frameContext.dpf.denoise = dpf.denoise;
> +       frameContext.dpf.update = update;
>  }
>  
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
>  void Dpf::prepare(IPAContext &context, const uint32_t frame,
> -                 [[maybe_unused]] RkISP1FrameContext &frameContext,
> -                 rkisp1_params_cfg *params)
> +                 RkISP1FrameContext &frameContext, rkisp1_params_cfg *params)
>  {
>         if (!initialized_)
>                 return;
>  
> -       auto &dpf = context.activeState.dpf;
> -
>         if (frame == 0) {
>                 params->others.dpf_config = config_;
>                 params->others.dpf_strength_config = strengthConfig_;
> @@ -245,12 +247,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>                                              RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
>         }
>  
> -       if (dpf.updateParams) {
> +       if (frameContext.dpf.update) {
>                 params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;
> -               if (dpf.denoise)
> +               if (frameContext.dpf.denoise)
>                         params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;
> -
> -               dpf.updateParams = false;
>         }
>  }
>  
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 936b77709417..b0210a978559 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -168,9 +168,6 @@ namespace libcamera::ipa::rkisp1 {
>   *
>   * \var IPAActiveState::dpf.denoise
>   * \brief Indicates if denoise is activated
> - *
> - * \var IPAActiveState::dpf.updateParams
> - * \brief Indicates if ISP parameters need to be updated
>   */
>  
>  /**
> @@ -251,6 +248,18 @@ namespace libcamera::ipa::rkisp1 {
>   * compared to the previous frame
>   */
>  
> +/**
> + * \var RkISP1FrameContext::dpf
> + * \brief Denoise Pre-Filter parameters for this frame
> + *
> + * \var RkISP1FrameContext::dpf.denoise
> + * \brief Indicates if denoise is activated
> + *
> + * \var RkISP1FrameContext::dpf.update
> + * \brief Indicates if the denoise pre-filter parameters have been updated
> + * compared to the previous frame
> + */
> +
>  /**
>   * \var RkISP1FrameContext::sensor
>   * \brief Sensor configuration that used been used for this frame
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 78edb607d039..c22e1f099c23 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -79,7 +79,6 @@ struct IPAActiveState {
>  
>         struct {
>                 bool denoise;
> -               bool updateParams;
>         } dpf;
>  
>         struct {
> @@ -113,6 +112,11 @@ struct RkISP1FrameContext : public FrameContext {
>                 bool update;
>         } cproc;
>  
> +       struct {
> +               bool denoise;
> +               bool update;
> +       } dpf;
> +
>         struct {
>                 uint32_t exposure;
>                 double gain;
> -- 
> Regards,
> 
> Laurent Pinchart
>
Jacopo Mondi Sept. 21, 2022, 8:40 p.m. UTC | #2 
Hi Laurent

On Thu, Sep 08, 2022 at 04:41:50AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Rework the algorithm's usage of the active state, to store the value of
> controls for the last queued request in the queueRequest() function, and
> store a copy of the values in the corresponding frame context. The
> latter is used in the prepare() function to populate the ISP parameters
> with values corresponding to the right frame.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/dpf.cpp | 22 +++++++++++-----------
>  src/ipa/rkisp1/ipa_context.cpp    | 15 ++++++++++++---
>  src/ipa/rkisp1/ipa_context.h      |  6 +++++-
>  3 files changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index 94c35c36570c..5d44480c5059 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -176,10 +176,11 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
>   */
>  void Dpf::queueRequest(IPAContext &context,
>  		       [[maybe_unused]] const uint32_t frame,
> -		       [[maybe_unused]] RkISP1FrameContext &frameContext,
> +		       RkISP1FrameContext &frameContext,
>  		       const ControlList &controls)
>  {
>  	auto &dpf = context.activeState.dpf;
> +	bool update = false;
>
>  	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
>  	if (denoise) {
> @@ -188,34 +189,35 @@ void Dpf::queueRequest(IPAContext &context,
>  		switch (*denoise) {
>  		case controls::draft::NoiseReductionModeOff:
>  			dpf.denoise = false;
> -			dpf.updateParams = true;
> +			update = true;

Do we update unconditionally or should we check if the state has
changed since last time and the application is not re-sending the same
control. As this is not compliant with the API assumptions that
controls should be sent only when they change, I think it's fair to
assume we need to update ?

If that's the case
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j
>  			break;
>  		case controls::draft::NoiseReductionModeMinimal:
>  		case controls::draft::NoiseReductionModeHighQuality:
>  		case controls::draft::NoiseReductionModeFast:
>  			dpf.denoise = true;
> -			dpf.updateParams = true;
> +			update = true;
>  			break;
>  		default:
>  			LOG(RkISP1Dpf, Error)
>  				<< "Unsupported denoise value "
>  				<< *denoise;
> +			break;
>  		}
>  	}
> +
> +	frameContext.dpf.denoise = dpf.denoise;
> +	frameContext.dpf.update = update;
>  }
>
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
>  void Dpf::prepare(IPAContext &context, const uint32_t frame,
> -		  [[maybe_unused]] RkISP1FrameContext &frameContext,
> -		  rkisp1_params_cfg *params)
> +		  RkISP1FrameContext &frameContext, rkisp1_params_cfg *params)
>  {
>  	if (!initialized_)
>  		return;
>
> -	auto &dpf = context.activeState.dpf;
> -
>  	if (frame == 0) {
>  		params->others.dpf_config = config_;
>  		params->others.dpf_strength_config = strengthConfig_;
> @@ -245,12 +247,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
>  					     RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
>  	}
>
> -	if (dpf.updateParams) {
> +	if (frameContext.dpf.update) {
>  		params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;
> -		if (dpf.denoise)
> +		if (frameContext.dpf.denoise)
>  			params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;
> -
> -		dpf.updateParams = false;
>  	}
>  }
>
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 936b77709417..b0210a978559 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -168,9 +168,6 @@ namespace libcamera::ipa::rkisp1 {
>   *
>   * \var IPAActiveState::dpf.denoise
>   * \brief Indicates if denoise is activated
> - *
> - * \var IPAActiveState::dpf.updateParams
> - * \brief Indicates if ISP parameters need to be updated
>   */
>
>  /**
> @@ -251,6 +248,18 @@ namespace libcamera::ipa::rkisp1 {
>   * compared to the previous frame
>   */
>
> +/**
> + * \var RkISP1FrameContext::dpf
> + * \brief Denoise Pre-Filter parameters for this frame
> + *
> + * \var RkISP1FrameContext::dpf.denoise
> + * \brief Indicates if denoise is activated
> + *
> + * \var RkISP1FrameContext::dpf.update
> + * \brief Indicates if the denoise pre-filter parameters have been updated
> + * compared to the previous frame
> + */
> +
>  /**
>   * \var RkISP1FrameContext::sensor
>   * \brief Sensor configuration that used been used for this frame
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 78edb607d039..c22e1f099c23 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -79,7 +79,6 @@ struct IPAActiveState {
>
>  	struct {
>  		bool denoise;
> -		bool updateParams;
>  	} dpf;
>
>  	struct {
> @@ -113,6 +112,11 @@ struct RkISP1FrameContext : public FrameContext {
>  		bool update;
>  	} cproc;
>
> +	struct {
> +		bool denoise;
> +		bool update;
> +	} dpf;
> +
>  	struct {
>  		uint32_t exposure;
>  		double gain;
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart Sept. 22, 2022, 8:16 p.m. UTC | #3 
Hi Jacopo,

On Wed, Sep 21, 2022 at 10:40:41PM +0200, Jacopo Mondi wrote:
> On Thu, Sep 08, 2022 at 04:41:50AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > Rework the algorithm's usage of the active state, to store the value of
> > controls for the last queued request in the queueRequest() function, and
> > store a copy of the values in the corresponding frame context. The
> > latter is used in the prepare() function to populate the ISP parameters
> > with values corresponding to the right frame.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/dpf.cpp | 22 +++++++++++-----------
> >  src/ipa/rkisp1/ipa_context.cpp    | 15 ++++++++++++---
> >  src/ipa/rkisp1/ipa_context.h      |  6 +++++-
> >  3 files changed, 28 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > index 94c35c36570c..5d44480c5059 100644
> > --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > @@ -176,10 +176,11 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
> >   */
> >  void Dpf::queueRequest(IPAContext &context,
> >  		       [[maybe_unused]] const uint32_t frame,
> > -		       [[maybe_unused]] RkISP1FrameContext &frameContext,
> > +		       RkISP1FrameContext &frameContext,
> >  		       const ControlList &controls)
> >  {
> >  	auto &dpf = context.activeState.dpf;
> > +	bool update = false;
> >
> >  	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> >  	if (denoise) {
> > @@ -188,34 +189,35 @@ void Dpf::queueRequest(IPAContext &context,
> >  		switch (*denoise) {
> >  		case controls::draft::NoiseReductionModeOff:
> >  			dpf.denoise = false;
> > -			dpf.updateParams = true;
> > +			update = true;
> 
> Do we update unconditionally or should we check if the state has
> changed since last time and the application is not re-sending the same
> control. As this is not compliant with the API assumptions that
> controls should be sent only when they change, I think it's fair to
> assume we need to update ?

No, you're right, I think it's useful to check if the value has changed
an only update when it does. It's just a few CPU cycles here, and will
save way more cycles in the kernel.

> If that's the case
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> >  			break;
> >  		case controls::draft::NoiseReductionModeMinimal:
> >  		case controls::draft::NoiseReductionModeHighQuality:
> >  		case controls::draft::NoiseReductionModeFast:
> >  			dpf.denoise = true;
> > -			dpf.updateParams = true;
> > +			update = true;
> >  			break;
> >  		default:
> >  			LOG(RkISP1Dpf, Error)
> >  				<< "Unsupported denoise value "
> >  				<< *denoise;
> > +			break;
> >  		}
> >  	}
> > +
> > +	frameContext.dpf.denoise = dpf.denoise;
> > +	frameContext.dpf.update = update;
> >  }
> >
> >  /**
> >   * \copydoc libcamera::ipa::Algorithm::prepare
> >   */
> >  void Dpf::prepare(IPAContext &context, const uint32_t frame,
> > -		  [[maybe_unused]] RkISP1FrameContext &frameContext,
> > -		  rkisp1_params_cfg *params)
> > +		  RkISP1FrameContext &frameContext, rkisp1_params_cfg *params)
> >  {
> >  	if (!initialized_)
> >  		return;
> >
> > -	auto &dpf = context.activeState.dpf;
> > -
> >  	if (frame == 0) {
> >  		params->others.dpf_config = config_;
> >  		params->others.dpf_strength_config = strengthConfig_;
> > @@ -245,12 +247,10 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,
> >  					     RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
> >  	}
> >
> > -	if (dpf.updateParams) {
> > +	if (frameContext.dpf.update) {
> >  		params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;
> > -		if (dpf.denoise)
> > +		if (frameContext.dpf.denoise)
> >  			params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;
> > -
> > -		dpf.updateParams = false;
> >  	}
> >  }
> >
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 936b77709417..b0210a978559 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -168,9 +168,6 @@ namespace libcamera::ipa::rkisp1 {
> >   *
> >   * \var IPAActiveState::dpf.denoise
> >   * \brief Indicates if denoise is activated
> > - *
> > - * \var IPAActiveState::dpf.updateParams
> > - * \brief Indicates if ISP parameters need to be updated
> >   */
> >
> >  /**
> > @@ -251,6 +248,18 @@ namespace libcamera::ipa::rkisp1 {
> >   * compared to the previous frame
> >   */
> >
> > +/**
> > + * \var RkISP1FrameContext::dpf
> > + * \brief Denoise Pre-Filter parameters for this frame
> > + *
> > + * \var RkISP1FrameContext::dpf.denoise
> > + * \brief Indicates if denoise is activated
> > + *
> > + * \var RkISP1FrameContext::dpf.update
> > + * \brief Indicates if the denoise pre-filter parameters have been updated
> > + * compared to the previous frame
> > + */
> > +
> >  /**
> >   * \var RkISP1FrameContext::sensor
> >   * \brief Sensor configuration that used been used for this frame
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 78edb607d039..c22e1f099c23 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -79,7 +79,6 @@ struct IPAActiveState {
> >
> >  	struct {
> >  		bool denoise;
> > -		bool updateParams;
> >  	} dpf;
> >
> >  	struct {
> > @@ -113,6 +112,11 @@ struct RkISP1FrameContext : public FrameContext {
> >  		bool update;
> >  	} cproc;
> >
> > +	struct {
> > +		bool denoise;
> > +		bool update;
> > +	} dpf;
> > +
> >  	struct {
> >  		uint32_t exposure;
> >  		double gain;

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
index 94c35c36570c..5d44480c5059 100644
--- a/src/ipa/rkisp1/algorithms/dpf.cpp
+++ b/src/ipa/rkisp1/algorithms/dpf.cpp
@@ -176,10 +176,11 @@  int Dpf::init([[maybe_unused]] IPAContext &context,
  */
 void Dpf::queueRequest(IPAContext &context,
 		       [[maybe_unused]] const uint32_t frame,
-		       [[maybe_unused]] RkISP1FrameContext &frameContext,
+		       RkISP1FrameContext &frameContext,
 		       const ControlList &controls)
 {
 	auto &dpf = context.activeState.dpf;
+	bool update = false;
 
 	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
 	if (denoise) {
@@ -188,34 +189,35 @@  void Dpf::queueRequest(IPAContext &context,
 		switch (*denoise) {
 		case controls::draft::NoiseReductionModeOff:
 			dpf.denoise = false;
-			dpf.updateParams = true;
+			update = true;
 			break;
 		case controls::draft::NoiseReductionModeMinimal:
 		case controls::draft::NoiseReductionModeHighQuality:
 		case controls::draft::NoiseReductionModeFast:
 			dpf.denoise = true;
-			dpf.updateParams = true;
+			update = true;
 			break;
 		default:
 			LOG(RkISP1Dpf, Error)
 				<< "Unsupported denoise value "
 				<< *denoise;
+			break;
 		}
 	}
+
+	frameContext.dpf.denoise = dpf.denoise;
+	frameContext.dpf.update = update;
 }
 
 /**
  * \copydoc libcamera::ipa::Algorithm::prepare
  */
 void Dpf::prepare(IPAContext &context, const uint32_t frame,
-		  [[maybe_unused]] RkISP1FrameContext &frameContext,
-		  rkisp1_params_cfg *params)
+		  RkISP1FrameContext &frameContext, rkisp1_params_cfg *params)
 {
 	if (!initialized_)
 		return;
 
-	auto &dpf = context.activeState.dpf;
-
 	if (frame == 0) {
 		params->others.dpf_config = config_;
 		params->others.dpf_strength_config = strengthConfig_;
@@ -245,12 +247,10 @@  void Dpf::prepare(IPAContext &context, const uint32_t frame,
 					     RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
 	}
 
-	if (dpf.updateParams) {
+	if (frameContext.dpf.update) {
 		params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;
-		if (dpf.denoise)
+		if (frameContext.dpf.denoise)
 			params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;
-
-		dpf.updateParams = false;
 	}
 }
 
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index 936b77709417..b0210a978559 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -168,9 +168,6 @@  namespace libcamera::ipa::rkisp1 {
  *
  * \var IPAActiveState::dpf.denoise
  * \brief Indicates if denoise is activated
- *
- * \var IPAActiveState::dpf.updateParams
- * \brief Indicates if ISP parameters need to be updated
  */
 
 /**
@@ -251,6 +248,18 @@  namespace libcamera::ipa::rkisp1 {
  * compared to the previous frame
  */
 
+/**
+ * \var RkISP1FrameContext::dpf
+ * \brief Denoise Pre-Filter parameters for this frame
+ *
+ * \var RkISP1FrameContext::dpf.denoise
+ * \brief Indicates if denoise is activated
+ *
+ * \var RkISP1FrameContext::dpf.update
+ * \brief Indicates if the denoise pre-filter parameters have been updated
+ * compared to the previous frame
+ */
+
 /**
  * \var RkISP1FrameContext::sensor
  * \brief Sensor configuration that used been used for this frame
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 78edb607d039..c22e1f099c23 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -79,7 +79,6 @@  struct IPAActiveState {
 
 	struct {
 		bool denoise;
-		bool updateParams;
 	} dpf;
 
 	struct {
@@ -113,6 +112,11 @@  struct RkISP1FrameContext : public FrameContext {
 		bool update;
 	} cproc;
 
+	struct {
+		bool denoise;
+		bool update;
+	} dpf;
+
 	struct {
 		uint32_t exposure;
 		double gain;