[PATCH 3/3] ipa: rksip1: Remove setControls(0) to reduce startup oscillations

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Fri, Feb 28, 2025 at 01:55:55PM +0100, Stefan Klug wrote:
> The call to setControls(0) is counter productive. At start() time, no
> requests were queued and no stats were received. So setControls(0)
> accesses a zeroed frame context and in turn sends 0 as gain, exposure
> and vblank to the pipeline handler and DelayedControls. This leads to
> strong oscillations on every start of the camera.
> 
> A proper fix for handling the startup controls still needs to be done
> and was already started in [1] and [2].
> 
> From a DelayedControls point of view the call to setControls(0) is also
> unnecessary as DelayedControls treat frame 0 as already being queued in
> after initialization.
> 
> So it is save to just remove it and the removal fixes the zero
> effectiveExposureValue discussed in the previous patch for rkisp1.
> 
> [1]: https://patchwork.libcamera.org/patch/21708/
> [2]: https://patchwork.libcamera.org/patch/22445/
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/rkisp1/rkisp1.cpp | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 5f1583e8219b..b09dd6efdf08 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -211,8 +211,6 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
>  
>  int IPARkISP1::start()
>  {
> -	setControls(0);
> -

Could you add a \todo comment ? With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart