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

[Jacopo Mondi]

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 ?

> 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