[libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum analogue gain

[Jacopo Mondi]

Hello again

On Thu, Jan 19, 2023 at 06:14:31PM +0100, Jacopo Mondi via libcamera-devel wrote:
> Hi Mikhail
>
> On Thu, Jan 19, 2023 at 06:59:05PM +0300, Mikhail Rudenko via libcamera-devel wrote:
> > Omnivision OV4689 sensor driver exposes maximum analogue gain of
> > 16x. Raise kMaxAnalogueGain to 16.0, so that the full gain range can
> > be used.
> >
> > Signed-off-by: Mikhail Rudenko <mike.rudenko at gmail.com>
> > ---
> >  src/ipa/rkisp1/algorithms/agc.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> > index e3470e25..e4cb2fc7 100644
> > --- a/src/ipa/rkisp1/algorithms/agc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> > @@ -38,7 +38,7 @@ LOG_DEFINE_CATEGORY(RkISP1Agc)
> >
> >  /* Limits for analogue gain values */
> >  static constexpr double kMinAnalogueGain = 1.0;
> > -static constexpr double kMaxAnalogueGain = 8.0;
> > +static constexpr double kMaxAnalogueGain = 16.0;
>
> I'm very surprised we have such an hard limit..
>
> Should it be made configurable ? Should we allow the sensor tuning
> file to provide this value ? Not something required for you to do to
> fix this ofc
>

In facts, this is already overriden using the sensor's provided max
gain as returned from the CameraHelper

src/ipa/rkisp1/rkisp1.cpp:      context_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain);

But we limit it to 8.0

src/ipa/rkisp1/algorithms/agc.cpp:      double maxAnalogueGain = std::min(configuration.sensor.maxAnalogueGain,
src/ipa/rkisp1/algorithms/agc.cpp-                                        kMaxAnalogueGain);

Should the camera sensor/helper be let express their max gain as they
like ?


> >
> >  /* \todo Honour the FrameDurationLimits control instead of hardcoding a limit */
> >  static constexpr utils::Duration kMaxShutterSpeed = 60ms;
> > --
> > 2.39.0
> >