[PATCH 07/12] ipa: rkisp1: agc: Don't update histogram parameters unnecessarily

[Laurent Pinchart]

On Mon, Jun 17, 2024 at 08:47:44PM +0900, Paul Elder wrote:
> On Sun, Jun 16, 2024 at 07:39:05PM +0300, Laurent Pinchart wrote:
> > The ISP histogram parameters depends on the AE metering mode, but not on
> > the other AE algorithm controls. The exposure mode, constraints mode and
> > frame duration limits influence the behaviour of the algorithm, but not
> > the histogram computation parameters. Update the histogram parameters
> > only when AE metering mode changes.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > index 9dac60bdb24e..9f3b59b45f95 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -261,26 +261,21 @@ void Agc::queueRequest(IPAContext &context,
> >  	frameContext.agc.meteringMode = agc.meteringMode;
> >  
> >  	const auto &exposureMode = controls.get(controls::AeExposureMode);
> > -	if (exposureMode) {
> > -		frameContext.agc.update = agc.exposureMode != *exposureMode;
> 
> I wonder if this should be renamed to updateMetering or something like
> that? It's not a big deal at this point though I suppose.

Good idea. I'll send a patch on top.

> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> > +	if (exposureMode)
> >  		agc.exposureMode =
> >  			static_cast<controls::AeExposureModeEnum>(*exposureMode);
> > -	}
> >  	frameContext.agc.exposureMode = agc.exposureMode;
> >  
> >  	const auto &constraintMode = controls.get(controls::AeConstraintMode);
> > -	if (constraintMode) {
> > -		frameContext.agc.update = agc.constraintMode != *constraintMode;
> > +	if (constraintMode)
> >  		agc.constraintMode =
> >  			static_cast<controls::AeConstraintModeEnum>(*constraintMode);
> > -	}
> >  	frameContext.agc.constraintMode = agc.constraintMode;
> >  
> >  	const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
> >  	if (frameDurationLimits) {
> >  		utils::Duration maxShutterSpeed =
> >  			std::chrono::milliseconds((*frameDurationLimits).back());
> > -		frameContext.agc.update = agc.maxShutterSpeed != maxShutterSpeed;
> >  		agc.maxShutterSpeed = maxShutterSpeed;
> >  	}
> >  	frameContext.agc.maxShutterSpeed = agc.maxShutterSpeed;

-- 
Regards,

Laurent Pinchart