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

[Kieran Bingham]

Quoting Stefan Klug (2025-02-28 12:55:55)
> 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

s/save/safe/

> 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);
> -

Aha, I thought I had recollection of this being added here more recently
but this dates all the way back to 2021
(1456efe7d51a3a0c6b57db4310f75b2a08ab1756) ... so I don't think it
tramples on any active development of AGC.


The above makes enough sense to me so:

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


Though - now I check through patchwork - I see:

https://patchwork.libcamera.org/patch/21708/

from https://patchwork.libcamera.org/project/libcamera/list/?series=4728
also touches code here. Have you reviewed that series? Does anything
there need to be also considered or reviewed ?

--
Kieran


>         return 0;
>  }
>  
> -- 
> 2.43.0
>