[PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219 and imx258

[Jacopo Mondi]

Sorry I have not woken up yet

On Wed, Jul 03, 2024 at 09:48:51AM GMT, Jacopo Mondi wrote:
> Hi Stefan
>
> On Tue, Jul 02, 2024 at 04:03:50PM GMT, Stefan Klug wrote:
> > Hi Laurent,
> >
> > On Tue, Jul 02, 2024 at 04:19:52PM +0300, Laurent Pinchart wrote:
> > > On Tue, Jul 02, 2024 at 10:53:55AM +0200, Jacopo Mondi wrote:
> > > > On Mon, Jul 01, 2024 at 04:38:28PM GMT, Stefan Klug wrote:
> > > > > For imx219 the black level was incorrectly set to 256. According to the
> > > > > datasheet it is 64d for raw 10. Mapped to 16bit, this becomes 4096.
> > > >
> > > > Might it have been expressed in 12 bits maybe ?
> > > > (however the mainline imx219 driver only supports 10 and 8 bpp codes)
> > >
> > > I suspect that's because the sensor has a 10-bit ADC :-) I'm not sure
> > > why we would have expressed the value on 12 bits. I'm fine treating it
> > > as a mystery and moving on.
> > >
> >
> > For the record: The value was correct. It was expressed in 12 bits
> > because the rkisp1 hardware expects that value to be with regards to
> > 12bits. The new value is also correct, as we treat all values to be
>
> As the tuning file are platform-specific, what's the purpose of
> expressing the value as 16 bits if it has to be unconditionally
> converted to 12 ?
>

As you're moving it to the platform-independent CameraSensorHelper, it
is indeed fine to have it in 16 bits :/

Sorry for the noise

> > based on 16bits as described in the controls section. Conversion to the
> > required bitdepth happens when passing the data to the hardware (this
> > needs to be fixed in this series)
> >
> > >
> > > > > For the imx258, BLC was not included at all. As only LSC data with
> > > > > rather low maximum values, the image quality is expected to only get
> > > > > better by adding black level correction.
> > > >
> > > > Not sure if this last paragraph is necessary.
> > >
> > > I don't get the part of the second sentence before the comma actually.
> >
> > Well something is missing in that sentence. I wanted to say that the LSC
> > tables in that tuning file do so little (small factors) that adding BLC
> > without recalculating the LSC tables should be ok. I'll drop it.
> >
> > Cheers,
> > Stefan
> >
> > >
> > > > > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > >
> > > > Anyway, we now have them in camera sensor helpers
> > > > Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > >
> > > > > ---
> > > > >  src/ipa/rkisp1/data/imx219.yaml | 4 ----
> > > > >  src/ipa/rkisp1/data/imx258.yaml | 1 +
> > > > >  2 files changed, 1 insertion(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/src/ipa/rkisp1/data/imx219.yaml b/src/ipa/rkisp1/data/imx219.yaml
> > > > > index cbcc43b84ac7..0d99cb529392 100644
> > > > > --- a/src/ipa/rkisp1/data/imx219.yaml
> > > > > +++ b/src/ipa/rkisp1/data/imx219.yaml
> > > > > @@ -6,10 +6,6 @@ algorithms:
> > > > >    - Agc:
> > > > >    - Awb:
> > > > >    - BlackLevelCorrection:
> > > > > -      R:  256
> > > > > -      Gr: 256
> > > > > -      Gb: 256
> > > > > -      B:  256
> > > > >    - LensShadingCorrection:
> > > > >        x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]
> > > > >        y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]
> > > > > diff --git a/src/ipa/rkisp1/data/imx258.yaml b/src/ipa/rkisp1/data/imx258.yaml
> > > > > index 43dddf20dcd2..202af36afbee 100644
> > > > > --- a/src/ipa/rkisp1/data/imx258.yaml
> > > > > +++ b/src/ipa/rkisp1/data/imx258.yaml
> > > > > @@ -5,6 +5,7 @@ version: 1
> > > > >  algorithms:
> > > > >    - Agc:
> > > > >    - Awb:
> > > > > +  - BlackLevelCorrection:
> > > > >    - LensShadingCorrection:
> > > > >        x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]
> > > > >        y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart