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

[Laurent Pinchart]

On Mon, Oct 21, 2024 at 04:40:30PM +0100, Kieran Bingham wrote:
> Quoting Mikhail Rudenko (2024-10-21 16:08:56)
> > On 2024-10-20 at 18:33 +01, Kieran Bingham wrote:
> > > Quoting Mikhail Rudenko (2024-10-20 14:39:45)
> > >> On 2024-10-18 at 23:26 +01, Kieran Bingham wrote:
> > >> > Quoting Kieran Bingham (2024-10-17 14:35:40)
> > >> >> 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...

A map of controls to delay values maybe.

> > 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.

It's also because the RPi IPA predates CameraSensorHelper if I recall
correctly.

> 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.

-- 
Regards,

Laurent Pinchart