[{"id":30198,"web_url":"https://patchwork.libcamera.org/comment/30198/","msgid":"","date":"2024-07-02T08:53:55","subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Mon, Jul 01, 2024 at 04:38:28PM GMT, Stefan Klug wrote:\n> For imx219 the black level was incorrectly set to 256. According to the\n> datasheet it is 64d for raw 10. Mapped to 16bit, this becomes 4096.\n\nMight it have been expressed in 12 bits maybe ?\n(however the mainline imx219 driver only supports 10 and 8 bpp codes)\n\n>\n> For the imx258, BLC was not included at all. As only LSC data with\n> rather low maximum values, the image quality is expected to only get\n> better by adding black level correction.\n\nNot sure if this last paragraph is necessary.\n\n>\n> Signed-off-by: Stefan Klug \n\nAnyway, we now have them in camera sensor helpers\nReviewed-by: Jacopo Mondi \n\n> ---\n> src/ipa/rkisp1/data/imx219.yaml | 4 ----\n> src/ipa/rkisp1/data/imx258.yaml | 1 +\n> 2 files changed, 1 insertion(+), 4 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/data/imx219.yaml b/src/ipa/rkisp1/data/imx219.yaml\n> index cbcc43b84ac7..0d99cb529392 100644\n> --- a/src/ipa/rkisp1/data/imx219.yaml\n> +++ b/src/ipa/rkisp1/data/imx219.yaml\n> @@ -6,10 +6,6 @@ algorithms:\n> - Agc:\n> - Awb:\n> - BlackLevelCorrection:\n> - R: 256\n> - Gr: 256\n> - Gb: 256\n> - B: 256\n> - LensShadingCorrection:\n> x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> diff --git a/src/ipa/rkisp1/data/imx258.yaml b/src/ipa/rkisp1/data/imx258.yaml\n> index 43dddf20dcd2..202af36afbee 100644\n> --- a/src/ipa/rkisp1/data/imx258.yaml\n> +++ b/src/ipa/rkisp1/data/imx258.yaml\n> @@ -5,6 +5,7 @@ version: 1\n> algorithms:\n> - Agc:\n> - Awb:\n> + - BlackLevelCorrection:\n> - LensShadingCorrection:\n> x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> --\n> 2.43.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E7B67BEFBE\n\tfor ;\n\tTue, 2 Jul 2024 08:54:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B898B62E24;\n\tTue, 2 Jul 2024 10:54:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 674C662E01\n\tfor ;\n\tTue, 2 Jul 2024 10:53:58 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F06AC5A4;\n\tTue, 2 Jul 2024 10:53:30 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UE7YGW3w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719910411;\n\tbh=/j7fyWLByc7aOKwOHktsQF+VpApIcFJt68fy6HS7Rvs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UE7YGW3wmtp+d5aCPA6gnvh2XqA+fLnHcVxnTDND08z5BptXe+zUL6wICTaJ64KvR\n\tsMXTS39mnFGxi7DXL+a3Di3lWsSPZx8oDiLawK7Z3pMrPWC8C92PdHXEWPBF8+9//V\n\tu/1YagJ5lReRxrt0THuQlrUZ8OG+4GBGQYiPllqc=","Date":"Tue, 2 Jul 2024 10:53:55 +0200","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-6-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240701144122.3418955-6-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30220,"web_url":"https://patchwork.libcamera.org/comment/30220/","msgid":"<20240702131952.GH8704@pendragon.ideasonboard.com>","date":"2024-07-02T13:19:52","subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jul 02, 2024 at 10:53:55AM +0200, Jacopo Mondi wrote:\n> On Mon, Jul 01, 2024 at 04:38:28PM GMT, Stefan Klug wrote:\n> > For imx219 the black level was incorrectly set to 256. According to the\n> > datasheet it is 64d for raw 10. Mapped to 16bit, this becomes 4096.\n> \n> Might it have been expressed in 12 bits maybe ?\n> (however the mainline imx219 driver only supports 10 and 8 bpp codes)\n\nI suspect that's because the sensor has a 10-bit ADC :-) I'm not sure\nwhy we would have expressed the value on 12 bits. I'm fine treating it\nas a mystery and moving on.\n\n> > For the imx258, BLC was not included at all. As only LSC data with\n> > rather low maximum values, the image quality is expected to only get\n> > better by adding black level correction.\n> \n> Not sure if this last paragraph is necessary.\n\nI don't get the part of the second sentence before the comma actually.\n\n> > Signed-off-by: Stefan Klug \n> \n> Anyway, we now have them in camera sensor helpers\n> Reviewed-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\n> > ---\n> > src/ipa/rkisp1/data/imx219.yaml | 4 ----\n> > src/ipa/rkisp1/data/imx258.yaml | 1 +\n> > 2 files changed, 1 insertion(+), 4 deletions(-)\n> >\n> > diff --git a/src/ipa/rkisp1/data/imx219.yaml b/src/ipa/rkisp1/data/imx219.yaml\n> > index cbcc43b84ac7..0d99cb529392 100644\n> > --- a/src/ipa/rkisp1/data/imx219.yaml\n> > +++ b/src/ipa/rkisp1/data/imx219.yaml\n> > @@ -6,10 +6,6 @@ algorithms:\n> > - Agc:\n> > - Awb:\n> > - BlackLevelCorrection:\n> > - R: 256\n> > - Gr: 256\n> > - Gb: 256\n> > - B: 256\n> > - LensShadingCorrection:\n> > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > diff --git a/src/ipa/rkisp1/data/imx258.yaml b/src/ipa/rkisp1/data/imx258.yaml\n> > index 43dddf20dcd2..202af36afbee 100644\n> > --- a/src/ipa/rkisp1/data/imx258.yaml\n> > +++ b/src/ipa/rkisp1/data/imx258.yaml\n> > @@ -5,6 +5,7 @@ version: 1\n> > algorithms:\n> > - Agc:\n> > - Awb:\n> > + - BlackLevelCorrection:\n> > - LensShadingCorrection:\n> > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 04D15BD87C\n\tfor ;\n\tTue, 2 Jul 2024 13:20:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 24D4E62E22;\n\tTue, 2 Jul 2024 15:20:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6518862E01\n\tfor ;\n\tTue, 2 Jul 2024 15:20:13 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CDAD2664;\n\tTue, 2 Jul 2024 15:19:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"E8vwVHH8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719926385;\n\tbh=NzhUxH++6FM/QFdQV/eIl44Fk91RbdllBXcU4I0RY00=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=E8vwVHH80Ox8SXUKwG2qK9nn4kiesJeTcN1XvT8+rBEKboRF0klIFB1Vr2m2HSoJM\n\t/xnqP9g75DOPFAJdWDbDlJXDDYRvdfe9oU7U4ihb1pGBAHaUwGUd2VLeTAfY0s5XYh\n\trV9BzhineNvlWEXl6Yss8RC9m9XLsKrDH6m1We5g=","Date":"Tue, 2 Jul 2024 16:19:52 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","Message-ID":"<20240702131952.GH8704@pendragon.ideasonboard.com>","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-6-stefan.klug@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30226,"web_url":"https://patchwork.libcamera.org/comment/30226/","msgid":"<2scelcum2whwtbazkrxwvxssbgbjqdul7bcydzstd4lts6dtd4@fhgyivvzhmti>","date":"2024-07-02T14:03:50","subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Tue, Jul 02, 2024 at 04:19:52PM +0300, Laurent Pinchart wrote:\n> On Tue, Jul 02, 2024 at 10:53:55AM +0200, Jacopo Mondi wrote:\n> > On Mon, Jul 01, 2024 at 04:38:28PM GMT, Stefan Klug wrote:\n> > > For imx219 the black level was incorrectly set to 256. According to the\n> > > datasheet it is 64d for raw 10. Mapped to 16bit, this becomes 4096.\n> > \n> > Might it have been expressed in 12 bits maybe ?\n> > (however the mainline imx219 driver only supports 10 and 8 bpp codes)\n> \n> I suspect that's because the sensor has a 10-bit ADC :-) I'm not sure\n> why we would have expressed the value on 12 bits. I'm fine treating it\n> as a mystery and moving on.\n>\n\nFor the record: The value was correct. It was expressed in 12 bits\nbecause the rkisp1 hardware expects that value to be with regards to\n12bits. The new value is also correct, as we treat all values to be\nbased on 16bits as described in the controls section. Conversion to the\nrequired bitdepth happens when passing the data to the hardware (this\nneeds to be fixed in this series)\n\n> \n> > > For the imx258, BLC was not included at all. As only LSC data with\n> > > rather low maximum values, the image quality is expected to only get\n> > > better by adding black level correction.\n> > \n> > Not sure if this last paragraph is necessary.\n> \n> I don't get the part of the second sentence before the comma actually.\n\nWell something is missing in that sentence. I wanted to say that the LSC\ntables in that tuning file do so little (small factors) that adding BLC\nwithout recalculating the LSC tables should be ok. I'll drop it.\n\nCheers,\nStefan\n\n> \n> > > Signed-off-by: Stefan Klug \n> > \n> > Anyway, we now have them in camera sensor helpers\n> > Reviewed-by: Jacopo Mondi \n> \n> Reviewed-by: Laurent Pinchart \n> \n> > > ---\n> > > src/ipa/rkisp1/data/imx219.yaml | 4 ----\n> > > src/ipa/rkisp1/data/imx258.yaml | 1 +\n> > > 2 files changed, 1 insertion(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/data/imx219.yaml b/src/ipa/rkisp1/data/imx219.yaml\n> > > index cbcc43b84ac7..0d99cb529392 100644\n> > > --- a/src/ipa/rkisp1/data/imx219.yaml\n> > > +++ b/src/ipa/rkisp1/data/imx219.yaml\n> > > @@ -6,10 +6,6 @@ algorithms:\n> > > - Agc:\n> > > - Awb:\n> > > - BlackLevelCorrection:\n> > > - R: 256\n> > > - Gr: 256\n> > > - Gb: 256\n> > > - B: 256\n> > > - LensShadingCorrection:\n> > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > diff --git a/src/ipa/rkisp1/data/imx258.yaml b/src/ipa/rkisp1/data/imx258.yaml\n> > > index 43dddf20dcd2..202af36afbee 100644\n> > > --- a/src/ipa/rkisp1/data/imx258.yaml\n> > > +++ b/src/ipa/rkisp1/data/imx258.yaml\n> > > @@ -5,6 +5,7 @@ version: 1\n> > > algorithms:\n> > > - Agc:\n> > > - Awb:\n> > > + - BlackLevelCorrection:\n> > > - LensShadingCorrection:\n> > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DB3A9BEFBE\n\tfor ;\n\tTue, 2 Jul 2024 14:03:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED00D62C96;\n\tTue, 2 Jul 2024 16:03:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B55C619CC\n\tfor ;\n\tTue, 2 Jul 2024 16:03:53 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:15de:d83a:d962:e44])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E54C7836;\n\tTue, 2 Jul 2024 16:03:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ji438k08\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719929006;\n\tbh=ItmiELfqQwU8biFaQM59DE5ArsOZPpAzjxt9w0fwqQk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ji438k08nI3I1kwDZSGBMxS9V4iuxRR5Dzpn48tFZ71bhYK4jNJeYk7GSIYIWVxGS\n\tqUF/qUejfpXvM9qR4tLkKse+RT+Js2pSF+bKMMT8EeErWod10CUPHJoyVZU8WIJjH5\n\twjucJnOQpiCgbkcTsLPgraUeaJfspICeVwuy0o88=","Date":"Tue, 2 Jul 2024 16:03:50 +0200","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","Message-ID":"<2scelcum2whwtbazkrxwvxssbgbjqdul7bcydzstd4lts6dtd4@fhgyivvzhmti>","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-6-stefan.klug@ideasonboard.com>\n\t\n\t<20240702131952.GH8704@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240702131952.GH8704@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30240,"web_url":"https://patchwork.libcamera.org/comment/30240/","msgid":"","date":"2024-07-03T07:48:51","subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Tue, Jul 02, 2024 at 04:03:50PM GMT, Stefan Klug wrote:\n> Hi Laurent,\n>\n> On Tue, Jul 02, 2024 at 04:19:52PM +0300, Laurent Pinchart wrote:\n> > On Tue, Jul 02, 2024 at 10:53:55AM +0200, Jacopo Mondi wrote:\n> > > On Mon, Jul 01, 2024 at 04:38:28PM GMT, Stefan Klug wrote:\n> > > > For imx219 the black level was incorrectly set to 256. According to the\n> > > > datasheet it is 64d for raw 10. Mapped to 16bit, this becomes 4096.\n> > >\n> > > Might it have been expressed in 12 bits maybe ?\n> > > (however the mainline imx219 driver only supports 10 and 8 bpp codes)\n> >\n> > I suspect that's because the sensor has a 10-bit ADC :-) I'm not sure\n> > why we would have expressed the value on 12 bits. I'm fine treating it\n> > as a mystery and moving on.\n> >\n>\n> For the record: The value was correct. It was expressed in 12 bits\n> because the rkisp1 hardware expects that value to be with regards to\n> 12bits. The new value is also correct, as we treat all values to be\n\nAs the tuning file are platform-specific, what's the purpose of\nexpressing the value as 16 bits if it has to be unconditionally\nconverted to 12 ?\n\n> based on 16bits as described in the controls section. Conversion to the\n> required bitdepth happens when passing the data to the hardware (this\n> needs to be fixed in this series)\n>\n> >\n> > > > For the imx258, BLC was not included at all. As only LSC data with\n> > > > rather low maximum values, the image quality is expected to only get\n> > > > better by adding black level correction.\n> > >\n> > > Not sure if this last paragraph is necessary.\n> >\n> > I don't get the part of the second sentence before the comma actually.\n>\n> Well something is missing in that sentence. I wanted to say that the LSC\n> tables in that tuning file do so little (small factors) that adding BLC\n> without recalculating the LSC tables should be ok. I'll drop it.\n>\n> Cheers,\n> Stefan\n>\n> >\n> > > > Signed-off-by: Stefan Klug \n> > >\n> > > Anyway, we now have them in camera sensor helpers\n> > > Reviewed-by: Jacopo Mondi \n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > > ---\n> > > > src/ipa/rkisp1/data/imx219.yaml | 4 ----\n> > > > src/ipa/rkisp1/data/imx258.yaml | 1 +\n> > > > 2 files changed, 1 insertion(+), 4 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/rkisp1/data/imx219.yaml b/src/ipa/rkisp1/data/imx219.yaml\n> > > > index cbcc43b84ac7..0d99cb529392 100644\n> > > > --- a/src/ipa/rkisp1/data/imx219.yaml\n> > > > +++ b/src/ipa/rkisp1/data/imx219.yaml\n> > > > @@ -6,10 +6,6 @@ algorithms:\n> > > > - Agc:\n> > > > - Awb:\n> > > > - BlackLevelCorrection:\n> > > > - R: 256\n> > > > - Gr: 256\n> > > > - Gb: 256\n> > > > - B: 256\n> > > > - LensShadingCorrection:\n> > > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > > diff --git a/src/ipa/rkisp1/data/imx258.yaml b/src/ipa/rkisp1/data/imx258.yaml\n> > > > index 43dddf20dcd2..202af36afbee 100644\n> > > > --- a/src/ipa/rkisp1/data/imx258.yaml\n> > > > +++ b/src/ipa/rkisp1/data/imx258.yaml\n> > > > @@ -5,6 +5,7 @@ version: 1\n> > > > algorithms:\n> > > > - Agc:\n> > > > - Awb:\n> > > > + - BlackLevelCorrection:\n> > > > - LensShadingCorrection:\n> > > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2EBDFBEFBE\n\tfor ;\n\tWed, 3 Jul 2024 07:48:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1408762E22;\n\tWed, 3 Jul 2024 09:48:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6221F619C7\n\tfor ;\n\tWed, 3 Jul 2024 09:48:55 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D62D3E6;\n\tWed, 3 Jul 2024 09:48:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"eR3bo6P4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719992907;\n\tbh=Bau2hYuu3jfNW/bzs3Nx0ItVkn5iKwrB7hY8FfgtOzA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eR3bo6P4zFRf/MqApr6GCihkgEztCWJDzzvzQSapW+zbahAgCeUA3NHQxXSo0Vcuh\n\tmiQSjZpNSNiz266YV6cUE8FztZ8/7uvAe4itd/E5Nwt23+18f510aUUi5KYudfuTEY\n\t3NousTLDAxh0Eg8uh7qH9s1guVbsZVMnpoeVx728=","Date":"Wed, 3 Jul 2024 09:48:51 +0200","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"Laurent Pinchart , \n\tJacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-6-stefan.klug@ideasonboard.com>\n\t\n\t<20240702131952.GH8704@pendragon.ideasonboard.com>\n\t<2scelcum2whwtbazkrxwvxssbgbjqdul7bcydzstd4lts6dtd4@fhgyivvzhmti>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<2scelcum2whwtbazkrxwvxssbgbjqdul7bcydzstd4lts6dtd4@fhgyivvzhmti>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30244,"web_url":"https://patchwork.libcamera.org/comment/30244/","msgid":"","date":"2024-07-03T08:17:31","subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Sorry I have not woken up yet\n\nOn Wed, Jul 03, 2024 at 09:48:51AM GMT, Jacopo Mondi wrote:\n> Hi Stefan\n>\n> On Tue, Jul 02, 2024 at 04:03:50PM GMT, Stefan Klug wrote:\n> > Hi Laurent,\n> >\n> > On Tue, Jul 02, 2024 at 04:19:52PM +0300, Laurent Pinchart wrote:\n> > > On Tue, Jul 02, 2024 at 10:53:55AM +0200, Jacopo Mondi wrote:\n> > > > On Mon, Jul 01, 2024 at 04:38:28PM GMT, Stefan Klug wrote:\n> > > > > For imx219 the black level was incorrectly set to 256. According to the\n> > > > > datasheet it is 64d for raw 10. Mapped to 16bit, this becomes 4096.\n> > > >\n> > > > Might it have been expressed in 12 bits maybe ?\n> > > > (however the mainline imx219 driver only supports 10 and 8 bpp codes)\n> > >\n> > > I suspect that's because the sensor has a 10-bit ADC :-) I'm not sure\n> > > why we would have expressed the value on 12 bits. I'm fine treating it\n> > > as a mystery and moving on.\n> > >\n> >\n> > For the record: The value was correct. It was expressed in 12 bits\n> > because the rkisp1 hardware expects that value to be with regards to\n> > 12bits. The new value is also correct, as we treat all values to be\n>\n> As the tuning file are platform-specific, what's the purpose of\n> expressing the value as 16 bits if it has to be unconditionally\n> converted to 12 ?\n>\n\nAs you're moving it to the platform-independent CameraSensorHelper, it\nis indeed fine to have it in 16 bits :/\n\nSorry for the noise\n\n> > based on 16bits as described in the controls section. Conversion to the\n> > required bitdepth happens when passing the data to the hardware (this\n> > needs to be fixed in this series)\n> >\n> > >\n> > > > > For the imx258, BLC was not included at all. As only LSC data with\n> > > > > rather low maximum values, the image quality is expected to only get\n> > > > > better by adding black level correction.\n> > > >\n> > > > Not sure if this last paragraph is necessary.\n> > >\n> > > I don't get the part of the second sentence before the comma actually.\n> >\n> > Well something is missing in that sentence. I wanted to say that the LSC\n> > tables in that tuning file do so little (small factors) that adding BLC\n> > without recalculating the LSC tables should be ok. I'll drop it.\n> >\n> > Cheers,\n> > Stefan\n> >\n> > >\n> > > > > Signed-off-by: Stefan Klug \n> > > >\n> > > > Anyway, we now have them in camera sensor helpers\n> > > > Reviewed-by: Jacopo Mondi \n> > >\n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > > > ---\n> > > > > src/ipa/rkisp1/data/imx219.yaml | 4 ----\n> > > > > src/ipa/rkisp1/data/imx258.yaml | 1 +\n> > > > > 2 files changed, 1 insertion(+), 4 deletions(-)\n> > > > >\n> > > > > diff --git a/src/ipa/rkisp1/data/imx219.yaml b/src/ipa/rkisp1/data/imx219.yaml\n> > > > > index cbcc43b84ac7..0d99cb529392 100644\n> > > > > --- a/src/ipa/rkisp1/data/imx219.yaml\n> > > > > +++ b/src/ipa/rkisp1/data/imx219.yaml\n> > > > > @@ -6,10 +6,6 @@ algorithms:\n> > > > > - Agc:\n> > > > > - Awb:\n> > > > > - BlackLevelCorrection:\n> > > > > - R: 256\n> > > > > - Gr: 256\n> > > > > - Gb: 256\n> > > > > - B: 256\n> > > > > - LensShadingCorrection:\n> > > > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > > > diff --git a/src/ipa/rkisp1/data/imx258.yaml b/src/ipa/rkisp1/data/imx258.yaml\n> > > > > index 43dddf20dcd2..202af36afbee 100644\n> > > > > --- a/src/ipa/rkisp1/data/imx258.yaml\n> > > > > +++ b/src/ipa/rkisp1/data/imx258.yaml\n> > > > > @@ -5,6 +5,7 @@ version: 1\n> > > > > algorithms:\n> > > > > - Agc:\n> > > > > - Awb:\n> > > > > + - BlackLevelCorrection:\n> > > > > - LensShadingCorrection:\n> > > > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > >\n> > > --\n> > > Regards,\n> > >\n> > > Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A8486BEFBE\n\tfor ;\n\tWed, 3 Jul 2024 08:17:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C99C62E22;\n\tWed, 3 Jul 2024 10:17:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35BFD619C7\n\tfor ;\n\tWed, 3 Jul 2024 10:17:34 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 32D3C6D6;\n\tWed, 3 Jul 2024 10:17:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EQ6Qaxzh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719994626;\n\tbh=ElGZXG/Az6f+JTLqU7FdV4dxsUfhAvO1n1w/8RR6Uos=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EQ6QaxzhVNr4UXZLLeIaXBJp1ans2pcjZjHmDl7csmzN5qKwx5tO5RKUpIVxaQ4NU\n\tgR4Ct/EUwVbrOaLao1zyiUEpdTsHqL9QrzCFQUfdNUOj8oqw28zBsHzVTema9/dxw5\n\ttREbdXE7bFWhEe7mQyJn/6mzVa67jB2SCulnf8gg=","Date":"Wed, 3 Jul 2024 10:17:31 +0200","From":"Jacopo Mondi ","To":"Jacopo Mondi ","Cc":"Stefan Klug , \n\tLaurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-6-stefan.klug@ideasonboard.com>\n\t\n\t<20240702131952.GH8704@pendragon.ideasonboard.com>\n\t<2scelcum2whwtbazkrxwvxssbgbjqdul7bcydzstd4lts6dtd4@fhgyivvzhmti>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30250,"web_url":"https://patchwork.libcamera.org/comment/30250/","msgid":"","date":"2024-07-03T11:21:57","subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Jul 02, 2024 at 04:19:52PM +0300, Laurent Pinchart wrote:\n> On Tue, Jul 02, 2024 at 10:53:55AM +0200, Jacopo Mondi wrote:\n> > On Mon, Jul 01, 2024 at 04:38:28PM GMT, Stefan Klug wrote:\n> > > For imx219 the black level was incorrectly set to 256. According to the\n> > > datasheet it is 64d for raw 10. Mapped to 16bit, this becomes 4096.\n> > \n> > Might it have been expressed in 12 bits maybe ?\n> > (however the mainline imx219 driver only supports 10 and 8 bpp codes)\n> \n> I suspect that's because the sensor has a 10-bit ADC :-) I'm not sure\n> why we would have expressed the value on 12 bits. I'm fine treating it\n> as a mystery and moving on.\n> \n> > > For the imx258, BLC was not included at all. As only LSC data with\n> > > rather low maximum values, the image quality is expected to only get\n> > > better by adding black level correction.\n> > \n> > Not sure if this last paragraph is necessary.\n> \n> I don't get the part of the second sentence before the comma actually.\n\nSame.\n\n> \n> > > Signed-off-by: Stefan Klug \n> > \n> > Anyway, we now have them in camera sensor helpers\n> > Reviewed-by: Jacopo Mondi \n> \n> Reviewed-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> \n> > > ---\n> > > src/ipa/rkisp1/data/imx219.yaml | 4 ----\n> > > src/ipa/rkisp1/data/imx258.yaml | 1 +\n> > > 2 files changed, 1 insertion(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/data/imx219.yaml b/src/ipa/rkisp1/data/imx219.yaml\n> > > index cbcc43b84ac7..0d99cb529392 100644\n> > > --- a/src/ipa/rkisp1/data/imx219.yaml\n> > > +++ b/src/ipa/rkisp1/data/imx219.yaml\n> > > @@ -6,10 +6,6 @@ algorithms:\n> > > - Agc:\n> > > - Awb:\n> > > - BlackLevelCorrection:\n> > > - R: 256\n> > > - Gr: 256\n> > > - Gb: 256\n> > > - B: 256\n> > > - LensShadingCorrection:\n> > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > diff --git a/src/ipa/rkisp1/data/imx258.yaml b/src/ipa/rkisp1/data/imx258.yaml\n> > > index 43dddf20dcd2..202af36afbee 100644\n> > > --- a/src/ipa/rkisp1/data/imx258.yaml\n> > > +++ b/src/ipa/rkisp1/data/imx258.yaml\n> > > @@ -5,6 +5,7 @@ version: 1\n> > > algorithms:\n> > > - Agc:\n> > > - Awb:\n> > > + - BlackLevelCorrection:\n> > > - LensShadingCorrection:\n> > > x-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]\n> > > y-size: [ 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625, 0.0625 ]","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 175A6BD87C\n\tfor ;\n\tWed, 3 Jul 2024 11:22:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0F69A62E22;\n\tWed, 3 Jul 2024 13:22:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 876B162C95\n\tfor ;\n\tWed, 3 Jul 2024 13:22:04 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EE8644CA;\n\tWed, 3 Jul 2024 13:21:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FqKL9t9j\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720005696;\n\tbh=JGb3ePMSq2rfqZoqcgmmL6kbxqitsahYozCaZ2S6rF8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FqKL9t9jviimrCP8zCjxT757FVB61paitp5e/LKXjiS7FYmcVG5ZSkxN2UMrxZUZF\n\tckMpFCNgECPF+piCtMjdoRw/XQrq1g5wT/yaWmlQJJ1i7g+IqoTEwNYrWEp/vCwA0N\n\tMc2YpmErhiJiaAdqNpSNb5ZvGhvxZqVp1bjYFcT4=","Date":"Wed, 3 Jul 2024 20:21:57 +0900","From":"Paul Elder ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi ,\n\tStefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 5/5] ipa: rkisp1: data: Update tuning files for imx219\n\tand imx258","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-6-stefan.klug@ideasonboard.com>\n\t\n\t<20240702131952.GH8704@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240702131952.GH8704@pendragon.ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]