[PATCH 0/3] Reduce rkisp1 flicker on first start

[Kieran Bingham]

Quoting Mikhail Rudenko (2024-10-21 16:08:56)
> 
> On 2024-10-20 at 18:33 +01, Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:
> 
> > Quoting Mikhail Rudenko (2024-10-20 14:39:45)
> >> Hi Kieran,
> >>
> >> On 2024-10-18 at 23:26 +01, Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:
> >>
> >> > Hi Mikhail,
> >> >
> >> > Quoting Kieran Bingham (2024-10-17 14:35:40)
> >> >> Hi Mikhail,
> >> >>
> >> >> Quoting Mikhail Rudenko (2024-10-17 13:47:04)
> >> >> > The chart from camshark (see attachment).
> >> >> >
> >> >>
> >> >> That looks like a good result indeed!
> >> >>
> >> >> I'll test out the patches.
> >> >
> >> > I've just tried this out - and it seems to work well on the IMX283 I
> >> > have - but caused quite noticible AnalogueGain oscillations on the
> >> > IMX335 ... I would 'bet' that's not the fault of this series - but the
> >> > fact that we don't have per-sensor delayed-controls set up correctly
> >> > which I already know about - but I haven't seen that get fixed up yet.
> >>
> >> I did a few more tests myself (I use OV4689 btw), and also observed
> >> analogue gain oscillations on darker scenes when AGC tries to steer the
> >> gain above 1.0. It looks like the hardcoded gain delay value of 1 is
> >> wrong at least for my sensor. When I increased it to 2, gain
> >> oscillations were gone. I have also tweaked a few things here and there
> >> and will send v2 soon.
> >>
> >> > I think your series probably highlights the fault that already exists...
> >>
> >> I feel like we have reached the moment when we should switch from the
> >> delays hardcoded in the pipeline handler to sensor specific values like
> >> rpi does. I could try doing that, but in a separate series. What do you
> >> think?
> >
> > Oh - absolutely. If it's something you're willing to work on - I think
> > that would be a massive help. Indeed, I think that's a separate series.
> 
> I'd like to give it a try. Am I right that adding
> 
>     virtual void getDelays(int &exposureDelay, int &gainDelay, int &vblankDelay, int &hblankDelay)

Yes, quite likely. I don't know what the plumbing will look like yet,
and maybe it would help to use something more structured than a series
of int references, but that's what's needed somewhere...

> to CameraSensorHelper returning generic delays, then overriding it for
> sensors where we know the exact delays is a valid approach? Also, why do
> we have both CamHelper and CameraSensorHelper, with former being
> significantly more featureful, but all the IPAs except rpi using the
> latter?

Because Raspberry Pi create their own IPA - which doesn't use the
CameraSensorHelpers - because they are not as featureful ... and all the
other IPAs can not use the Raspberry Pi helpers - because they are in
the Raspberry Pi IPA instead of common code ... and no one has yet tried
to align the libipa code until now.

Ultimately - Making sure that the helpers in libipa can be grown in a
way that Raspberry Pi can also use them will then mean that we only need
to have *one* location for all the common camera sensor specific
helpers.

--
Kieran


> 
> --
> Best regards,
> Mikhail