[{"id":32281,"web_url":"https://patchwork.libcamera.org/comment/32281/","msgid":"<6js3bcqpud6dmnelb3xav3ox7phxv5opcj3mn2iqbug5gmmbp6@zlyg4t5rziif>","date":"2024-11-19T12:56:14","subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Geoffrey\n\nOn Mon, Nov 18, 2024 at 11:53:07PM +0100, Geoffrey Van Landeghem wrote:\n> The sensor is largely compatible with the already supported Sony IMX290 so we can reuse the same helpers for the analogue gain conversion functions.\n\nI presume my suggestion to read https://cbea.ms/git-commit/ first\nwasn't helpful.\n\nSubject line should be shortened (I would not force it to 50 cols\nas the website suggests, but strive for staying in at least 72), and\nno full stop at the end.\n\nSame thing for the commit message.\n\nWe can adjust it when applying the patch if you don't have to resend\nfor other reasons.\n\n>\n> Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>\n> ---\n>  src/ipa/libipa/camera_sensor_helper.cpp           | 5 +++++\n>  src/ipa/rpi/cam_helper/cam_helper_imx290.cpp      | 1 +\n>  src/libcamera/sensor/camera_sensor_properties.cpp | 4 ++++\n>  3 files changed, 10 insertions(+)\n>\n> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> index c6169bdc..f870dc28 100644\n> --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> @@ -622,6 +622,11 @@ public:\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n>\n> +class CameraSensorHelperImx462 : public CameraSensorHelperImx290\n> +{\n> +};\n> +REGISTER_CAMERA_SENSOR_HELPER(\"imx462\", CameraSensorHelperImx462)\n\nOne thing we're missing for imx290 is the black level pedestal. The\nimx290 datasheet reports a (programmable) black level of\n10 bits: 0x3c\n12 bits: 0xf0\n\nCould you confirm the imx462 reports the same default values ?\n\n> +\n>  class CameraSensorHelperImx477 : public CameraSensorHelper\n>  {\n>  public:\n> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> index e57ab538..0cc24a6d 100644\n> --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> @@ -73,3 +73,4 @@ static CamHelper *create()\n>  }\n>\n>  static RegisterCamHelper reg(\"imx290\", &create);\n> +static RegisterCamHelper reg462(\"imx462\", &create);\n> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> index 6d4136d0..e2305166 100644\n> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> @@ -142,6 +142,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>  \t\t\t.unitCellSize = { 1450, 1450 },\n>  \t\t\t.testPatternModes = {},\n>  \t\t} },\n> +\t\t{ \"imx462\", {\n> +\t\t\t.unitCellSize = { 2900, 2900 },\n> +\t\t\t.testPatternModes = {},\n> +\t\t} },\n\nI don't have a datasheet but a few online sources confirm a 2.9u pixel\nsize.\n\nThe patch looks good\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n>  \t\t{ \"imx477\", {\n>  \t\t\t.unitCellSize = { 1550, 1550 },\n>  \t\t\t.testPatternModes = {},\n> --\n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A51B1C32F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Nov 2024 12:56:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94DE265F0F;\n\tTue, 19 Nov 2024 13:56:22 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3320C658A2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Nov 2024 13:56:21 +0100 (CET)","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 B3C3F3A2;\n\tTue, 19 Nov 2024 13:56:01 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"eHRkyod1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732020961;\n\tbh=5N0wEDLQ+u/HgCOe56vxZIyVHn9XQh/AJSE457A6kU4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eHRkyod1MGwnO5CFigZUeqGxvGFXypvmabbEogzjssR6fU89ZpcKjRg7ZXwkYCio9\n\tzf5FoVTiDAapNX0ecqDTG9X62TaxIsowTGNzdlTf1QaXjMynRvE6LnOULZozd/6gUY\n\tKna0TTgPYpClBlu4oPMJIJazklrFdobJL/nOt+F0=","Date":"Tue, 19 Nov 2024 13:56:14 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Geoffrey Van Landeghem <geoffrey.vl@gmail.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","Message-ID":"<6js3bcqpud6dmnelb3xav3ox7phxv5opcj3mn2iqbug5gmmbp6@zlyg4t5rziif>","References":"<20241118225310.446706-1-geoffrey.vl@gmail.com>\n\t<20241118225310.446706-2-geoffrey.vl@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241118225310.446706-2-geoffrey.vl@gmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32293,"web_url":"https://patchwork.libcamera.org/comment/32293/","msgid":"<CABX3Wh67aod2jHd=R1mrb8kYmRvUkbveg-xsq+jHDiv39mb9aQ@mail.gmail.com>","date":"2024-11-19T21:50:53","subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","submitter":{"id":214,"url":"https://patchwork.libcamera.org/api/people/214/","name":"Geoffrey Van Landeghem","email":"geoffrey.vl@gmail.com"},"content":"Hi Jacopo,\n\nsorry for screwing up the git commit message. I took over the\nsuggestion entirely from the V1 patch, which also got me a bit\nconfused. The entire git format-patch and git email is still a bit\nawkward and needs some more getting used to it from my side. At work\nwe rely on other systems for centralizing source control, but I learn\nsomething new and I also appreciate your feedback (and patience!).\n\nRegarding the pixel size, it's indeed 2.9u pixel. I guess the most\ntrustworthy resource is this one from Sony:\nhttps://www.sony-semicon.com/files/62/flyer_security/IMX462LQR_LQR1_Flyer.pdf\n(look for \"unit cell size\")\n\nAnd regarding the black level offset setting (register 300Ah). I\nchecked the IMX290 and IMX327 datasheets and they match in that they\nboth have F0h as default value.\nFor IMX462 I couldn't find any datasheet, but since I own the sensor I\nqueried it using i2c-tools, and it seems it's also defaulting to F0h:\n\n$ i2ctransfer -f -y 10 w2@0x1a 0x30 0x0A r1\n0xf0\n\nKinds regards,\nGeoffrey\n\nOp di 19 nov 2024 om 13:56 schreef Jacopo Mondi <jacopo.mondi@ideasonboard.com>:\n\n\n>\n> Hi Geoffrey\n>\n> On Mon, Nov 18, 2024 at 11:53:07PM +0100, Geoffrey Van Landeghem wrote:\n> > The sensor is largely compatible with the already supported Sony IMX290 so we can reuse the same helpers for the analogue gain conversion functions.\n>\n> I presume my suggestion to read https://cbea.ms/git-commit/ first\n> wasn't helpful.\n>\n> Subject line should be shortened (I would not force it to 50 cols\n> as the website suggests, but strive for staying in at least 72), and\n> no full stop at the end.\n>\n> Same thing for the commit message.\n>\n> We can adjust it when applying the patch if you don't have to resend\n> for other reasons.\n>\n> >\n> > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>\n> > ---\n> >  src/ipa/libipa/camera_sensor_helper.cpp           | 5 +++++\n> >  src/ipa/rpi/cam_helper/cam_helper_imx290.cpp      | 1 +\n> >  src/libcamera/sensor/camera_sensor_properties.cpp | 4 ++++\n> >  3 files changed, 10 insertions(+)\n> >\n> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > index c6169bdc..f870dc28 100644\n> > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > @@ -622,6 +622,11 @@ public:\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n> >\n> > +class CameraSensorHelperImx462 : public CameraSensorHelperImx290\n> > +{\n> > +};\n> > +REGISTER_CAMERA_SENSOR_HELPER(\"imx462\", CameraSensorHelperImx462)\n>\n> One thing we're missing for imx290 is the black level pedestal. The\n> imx290 datasheet reports a (programmable) black level of\n> 10 bits: 0x3c\n> 12 bits: 0xf0\n>\n> Could you confirm the imx462 reports the same default values ?\n>\n> > +\n> >  class CameraSensorHelperImx477 : public CameraSensorHelper\n> >  {\n> >  public:\n> > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > index e57ab538..0cc24a6d 100644\n> > --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > @@ -73,3 +73,4 @@ static CamHelper *create()\n> >  }\n> >\n> >  static RegisterCamHelper reg(\"imx290\", &create);\n> > +static RegisterCamHelper reg462(\"imx462\", &create);\n> > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > index 6d4136d0..e2305166 100644\n> > --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > @@ -142,6 +142,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> >                       .unitCellSize = { 1450, 1450 },\n> >                       .testPatternModes = {},\n> >               } },\n> > +             { \"imx462\", {\n> > +                     .unitCellSize = { 2900, 2900 },\n> > +                     .testPatternModes = {},\n> > +             } },\n>\n> I don't have a datasheet but a few online sources confirm a 2.9u pixel\n> size.\n>\n> The patch looks good\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>\n> Thanks\n>   j\n>\n> >               { \"imx477\", {\n> >                       .unitCellSize = { 1550, 1550 },\n> >                       .testPatternModes = {},\n> > --\n> > 2.43.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B7A72C326B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Nov 2024 21:51:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 543E865F2A;\n\tTue, 19 Nov 2024 22:51:10 +0100 (CET)","from mail-ej1-x635.google.com (mail-ej1-x635.google.com\n\t[IPv6:2a00:1450:4864:20::635])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42BA2658A2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Nov 2024 22:51:06 +0100 (CET)","by mail-ej1-x635.google.com with SMTP id\n\ta640c23a62f3a-a9ee097a478so253318866b.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Nov 2024 13:51:06 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Awgm8fyB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1732053065; x=1732657865;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=XEuUEAjFIvjbVbBe3mXlmtXky69Dy+p2kF4eY0mD+vA=;\n\tb=Awgm8fyBowYBr2MWyYAGZa/qsG9Cmubm7VsF5hLBvixT/ryVjhhbZTqcJ+4fL8/nTC\n\t9/6hq5J5IKVjWJiW/VSMa/eCwZ+lkVbmbQJgn1x+9deREVfiI/0ycYsmqEnTdCIIHdKi\n\tn8kmAMxX3sLs8LIJx8t+Ly+vAjab/e7V88E+fElfX6h95QFAmXVQ/Sk+OFDn3OJKEOjy\n\tPvlR6NHTj4fZbRNrUfafQy3Pf9sEDYsZcVSQLzjTPA1pxpLTyg81EFhb018FigJjeYDE\n\tu71Vc2w71nmxRVsst8vOGIYLNNPe0KhNiB76MNcJaRPSOxfWrTEXlDYuIPMYBJtNLQG+\n\t8vLA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1732053065; x=1732657865;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=XEuUEAjFIvjbVbBe3mXlmtXky69Dy+p2kF4eY0mD+vA=;\n\tb=wS2Cb4z+UoagqVA3p04SyY1t2mVzr6Uw4DkUryhX0msJ/BKzPPMnYR4hAg4IrSHFcu\n\tydy6AVJVCkCS3XGjQm6feegYKBI5YDZwL8yKL1GgkyQ3WvywyzV6Zc7RvNLQCvoqvMbT\n\te8fmTjPIVF8VizhloUQ5y8sgGC9BxhZPdzT7b0FRfye2tvLBqxuGZA3LfZNDW4HR3sg5\n\teRNmW/Xa/oy3DVgZakEFJcoowDy7q2XZS2WSujS7FL/OdWXbKFdbDJt8hqQYjpGUyfID\n\tOyip4PTXiscPOSmqoGjqWImDHCOsnsjL89ENZcfXk4XffOwKLh1L6aCo8MQJbfKol6ij\n\tjTtw==","X-Gm-Message-State":"AOJu0Yz9+gk9P3IqhZbezIvhA8jmStgDNIpuk1MlbrZstX/bLGFzD7f4\n\t+y3wOz31A6L1ijjRBO2K6tZNT/m7AbQUj0nDKHDyTEqsQOJouI8Ad5kgQmBa6+pGKrok2y2psgK\n\tpUsTeB1QIuFwPVyCWcFnhEJ5UslY=","X-Google-Smtp-Source":"AGHT+IFTWOyi+8M9gbHmLpqideUGhSo6rWEV3y25nAqzFhzatV/5XCz53FsioAtd5curhJ1BvVKQ2Sdp2kzHBptSdOg=","X-Received":"by 2002:a05:6402:42c7:b0:5cb:699c:30dd with SMTP id\n\t4fb4d7f45d1cf-5cff4cdbc21mr166385a12.32.1732053065245;\n\tTue, 19 Nov 2024 13:51:05 -0800 (PST)","MIME-Version":"1.0","References":"<20241118225310.446706-1-geoffrey.vl@gmail.com>\n\t<20241118225310.446706-2-geoffrey.vl@gmail.com>\n\t<6js3bcqpud6dmnelb3xav3ox7phxv5opcj3mn2iqbug5gmmbp6@zlyg4t5rziif>","In-Reply-To":"<6js3bcqpud6dmnelb3xav3ox7phxv5opcj3mn2iqbug5gmmbp6@zlyg4t5rziif>","From":"Geoffrey Van Landeghem <geoffrey.vl@gmail.com>","Date":"Tue, 19 Nov 2024 22:50:53 +0100","Message-ID":"<CABX3Wh67aod2jHd=R1mrb8kYmRvUkbveg-xsq+jHDiv39mb9aQ@mail.gmail.com>","Subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32298,"web_url":"https://patchwork.libcamera.org/comment/32298/","msgid":"<ck5bbkj4eo3zfjk7bjcumnskolnkbw6uyursv6mxivjmzggjfi@rfle57isyikz>","date":"2024-11-20T09:00:52","subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Geoffrey\n\nOn Tue, Nov 19, 2024 at 10:50:53PM +0100, Geoffrey Van Landeghem wrote:\n> Hi Jacopo,\n>\n> sorry for screwing up the git commit message. I took over the\n> suggestion entirely from the V1 patch, which also got me a bit\n> confused. The entire git format-patch and git email is still a bit\n> awkward and needs some more getting used to it from my side. At work\n> we rely on other systems for centralizing source control, but I learn\n> something new and I also appreciate your feedback (and patience!).\n\nNo worries at all, it's more than fine to have a few hiccups when\nfirst approaching mail-based patch review.\n\n>\n> Regarding the pixel size, it's indeed 2.9u pixel. I guess the most\n> trustworthy resource is this one from Sony:\n> https://www.sony-semicon.com/files/62/flyer_security/IMX462LQR_LQR1_Flyer.pdf\n> (look for \"unit cell size\")\n\nThanks for confirming.\n\n>\n> And regarding the black level offset setting (register 300Ah). I\n> checked the IMX290 and IMX327 datasheets and they match in that they\n> both have F0h as default value.\n> For IMX462 I couldn't find any datasheet, but since I own the sensor I\n> queried it using i2c-tools, and it seems it's also defaulting to F0h:\n>\n> $ i2ctransfer -f -y 10 w2@0x1a 0x30 0x0A r1\n> 0xf0\n\nNice, thanks for checking. Feel free to add a patch to add the black\nlevels to camera sensor helpers if you like to :)\n\nThanks\n   j\n\n>\n> Kinds regards,\n> Geoffrey\n>\n> Op di 19 nov 2024 om 13:56 schreef Jacopo Mondi <jacopo.mondi@ideasonboard.com>:\n>\n>\n> >\n> > Hi Geoffrey\n> >\n> > On Mon, Nov 18, 2024 at 11:53:07PM +0100, Geoffrey Van Landeghem wrote:\n> > > The sensor is largely compatible with the already supported Sony IMX290 so we can reuse the same helpers for the analogue gain conversion functions.\n> >\n> > I presume my suggestion to read https://cbea.ms/git-commit/ first\n> > wasn't helpful.\n> >\n> > Subject line should be shortened (I would not force it to 50 cols\n> > as the website suggests, but strive for staying in at least 72), and\n> > no full stop at the end.\n> >\n> > Same thing for the commit message.\n> >\n> > We can adjust it when applying the patch if you don't have to resend\n> > for other reasons.\n> >\n> > >\n> > > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>\n> > > ---\n> > >  src/ipa/libipa/camera_sensor_helper.cpp           | 5 +++++\n> > >  src/ipa/rpi/cam_helper/cam_helper_imx290.cpp      | 1 +\n> > >  src/libcamera/sensor/camera_sensor_properties.cpp | 4 ++++\n> > >  3 files changed, 10 insertions(+)\n> > >\n> > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > index c6169bdc..f870dc28 100644\n> > > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > @@ -622,6 +622,11 @@ public:\n> > >  };\n> > >  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n> > >\n> > > +class CameraSensorHelperImx462 : public CameraSensorHelperImx290\n> > > +{\n> > > +};\n> > > +REGISTER_CAMERA_SENSOR_HELPER(\"imx462\", CameraSensorHelperImx462)\n> >\n> > One thing we're missing for imx290 is the black level pedestal. The\n> > imx290 datasheet reports a (programmable) black level of\n> > 10 bits: 0x3c\n> > 12 bits: 0xf0\n> >\n> > Could you confirm the imx462 reports the same default values ?\n> >\n> > > +\n> > >  class CameraSensorHelperImx477 : public CameraSensorHelper\n> > >  {\n> > >  public:\n> > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > index e57ab538..0cc24a6d 100644\n> > > --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > @@ -73,3 +73,4 @@ static CamHelper *create()\n> > >  }\n> > >\n> > >  static RegisterCamHelper reg(\"imx290\", &create);\n> > > +static RegisterCamHelper reg462(\"imx462\", &create);\n> > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > index 6d4136d0..e2305166 100644\n> > > --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > @@ -142,6 +142,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > >                       .unitCellSize = { 1450, 1450 },\n> > >                       .testPatternModes = {},\n> > >               } },\n> > > +             { \"imx462\", {\n> > > +                     .unitCellSize = { 2900, 2900 },\n> > > +                     .testPatternModes = {},\n> > > +             } },\n> >\n> > I don't have a datasheet but a few online sources confirm a 2.9u pixel\n> > size.\n> >\n> > The patch looks good\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> >\n> > Thanks\n> >   j\n> >\n> > >               { \"imx477\", {\n> > >                       .unitCellSize = { 1550, 1550 },\n> > >                       .testPatternModes = {},\n> > > --\n> > > 2.43.0\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 88E15C32F5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 20 Nov 2024 09:00:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CCAF65F3E;\n\tWed, 20 Nov 2024 10:00:58 +0100 (CET)","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 DC911658B6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Nov 2024 10:00:56 +0100 (CET)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 874B455A;\n\tWed, 20 Nov 2024 10:00:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ExQdHgj2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732093238;\n\tbh=3m/Iozi2Bj8uXcuzXOXvaC9zkyuHIcD00sHOkuu54jo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ExQdHgj2KCcyEVuAiRNcQpZ883SccRIoxBn6ugE/t3KTzTQI5g6GnZZ+Ncg067+m1\n\tTGm6GXe8b6GsDio3+yJwNL7ess3t0CNCo4Zj5egs0UDMYt28xIuDinM+cSHETIGcDV\n\tFnB0HKvDioQrryMTbQNmSX7VORo2lxWfyjkMLcAE=","Date":"Wed, 20 Nov 2024 10:00:52 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Geoffrey Van Landeghem <geoffrey.vl@gmail.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","Message-ID":"<ck5bbkj4eo3zfjk7bjcumnskolnkbw6uyursv6mxivjmzggjfi@rfle57isyikz>","References":"<20241118225310.446706-1-geoffrey.vl@gmail.com>\n\t<20241118225310.446706-2-geoffrey.vl@gmail.com>\n\t<6js3bcqpud6dmnelb3xav3ox7phxv5opcj3mn2iqbug5gmmbp6@zlyg4t5rziif>\n\t<CABX3Wh67aod2jHd=R1mrb8kYmRvUkbveg-xsq+jHDiv39mb9aQ@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CABX3Wh67aod2jHd=R1mrb8kYmRvUkbveg-xsq+jHDiv39mb9aQ@mail.gmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32316,"web_url":"https://patchwork.libcamera.org/comment/32316/","msgid":"<CABX3Wh4KQ1LWutGpqpzj+Sg+aUoM+he8fLxNApwd1vqC7qW9QA@mail.gmail.com>","date":"2024-11-20T22:06:58","subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","submitter":{"id":214,"url":"https://patchwork.libcamera.org/api/people/214/","name":"Geoffrey Van Landeghem","email":"geoffrey.vl@gmail.com"},"content":"Hi Jacopo,\n\nIf I understand correctly, even though the black level register is\nonly 9 bits wide, you take the bit-depth of the sensor in the mode it\nis configured to work (in my case 12), and then shift it to 16-bits as\nexplained in the libcamera sources. If so the black level for all 3\nsensors is:\n\n/* From datasheet: 0xF0 at 12bits. */\nblackLevel_ = 3840;\n\nCorrect?\n\nKind regards,\nGeoffrey\n\n\nOp wo 20 nov 2024 om 10:00 schreef Jacopo Mondi <jacopo.mondi@ideasonboard.com>:\n>\n> Hi Geoffrey\n>\n> On Tue, Nov 19, 2024 at 10:50:53PM +0100, Geoffrey Van Landeghem wrote:\n> > Hi Jacopo,\n> >\n> > sorry for screwing up the git commit message. I took over the\n> > suggestion entirely from the V1 patch, which also got me a bit\n> > confused. The entire git format-patch and git email is still a bit\n> > awkward and needs some more getting used to it from my side. At work\n> > we rely on other systems for centralizing source control, but I learn\n> > something new and I also appreciate your feedback (and patience!).\n>\n> No worries at all, it's more than fine to have a few hiccups when\n> first approaching mail-based patch review.\n>\n> >\n> > Regarding the pixel size, it's indeed 2.9u pixel. I guess the most\n> > trustworthy resource is this one from Sony:\n> > https://www.sony-semicon.com/files/62/flyer_security/IMX462LQR_LQR1_Flyer.pdf\n> > (look for \"unit cell size\")\n>\n> Thanks for confirming.\n>\n> >\n> > And regarding the black level offset setting (register 300Ah). I\n> > checked the IMX290 and IMX327 datasheets and they match in that they\n> > both have F0h as default value.\n> > For IMX462 I couldn't find any datasheet, but since I own the sensor I\n> > queried it using i2c-tools, and it seems it's also defaulting to F0h:\n> >\n> > $ i2ctransfer -f -y 10 w2@0x1a 0x30 0x0A r1\n> > 0xf0\n>\n> Nice, thanks for checking. Feel free to add a patch to add the black\n> levels to camera sensor helpers if you like to :)\n>\n> Thanks\n>    j\n>\n> >\n> > Kinds regards,\n> > Geoffrey\n> >\n> > Op di 19 nov 2024 om 13:56 schreef Jacopo Mondi <jacopo.mondi@ideasonboard.com>:\n> >\n> >\n> > >\n> > > Hi Geoffrey\n> > >\n> > > On Mon, Nov 18, 2024 at 11:53:07PM +0100, Geoffrey Van Landeghem wrote:\n> > > > The sensor is largely compatible with the already supported Sony IMX290 so we can reuse the same helpers for the analogue gain conversion functions.\n> > >\n> > > I presume my suggestion to read https://cbea.ms/git-commit/ first\n> > > wasn't helpful.\n> > >\n> > > Subject line should be shortened (I would not force it to 50 cols\n> > > as the website suggests, but strive for staying in at least 72), and\n> > > no full stop at the end.\n> > >\n> > > Same thing for the commit message.\n> > >\n> > > We can adjust it when applying the patch if you don't have to resend\n> > > for other reasons.\n> > >\n> > > >\n> > > > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>\n> > > > ---\n> > > >  src/ipa/libipa/camera_sensor_helper.cpp           | 5 +++++\n> > > >  src/ipa/rpi/cam_helper/cam_helper_imx290.cpp      | 1 +\n> > > >  src/libcamera/sensor/camera_sensor_properties.cpp | 4 ++++\n> > > >  3 files changed, 10 insertions(+)\n> > > >\n> > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > index c6169bdc..f870dc28 100644\n> > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > @@ -622,6 +622,11 @@ public:\n> > > >  };\n> > > >  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n> > > >\n> > > > +class CameraSensorHelperImx462 : public CameraSensorHelperImx290\n> > > > +{\n> > > > +};\n> > > > +REGISTER_CAMERA_SENSOR_HELPER(\"imx462\", CameraSensorHelperImx462)\n> > >\n> > > One thing we're missing for imx290 is the black level pedestal. The\n> > > imx290 datasheet reports a (programmable) black level of\n> > > 10 bits: 0x3c\n> > > 12 bits: 0xf0\n> > >\n> > > Could you confirm the imx462 reports the same default values ?\n> > >\n> > > > +\n> > > >  class CameraSensorHelperImx477 : public CameraSensorHelper\n> > > >  {\n> > > >  public:\n> > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > > index e57ab538..0cc24a6d 100644\n> > > > --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > > @@ -73,3 +73,4 @@ static CamHelper *create()\n> > > >  }\n> > > >\n> > > >  static RegisterCamHelper reg(\"imx290\", &create);\n> > > > +static RegisterCamHelper reg462(\"imx462\", &create);\n> > > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > > index 6d4136d0..e2305166 100644\n> > > > --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > > @@ -142,6 +142,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > >                       .unitCellSize = { 1450, 1450 },\n> > > >                       .testPatternModes = {},\n> > > >               } },\n> > > > +             { \"imx462\", {\n> > > > +                     .unitCellSize = { 2900, 2900 },\n> > > > +                     .testPatternModes = {},\n> > > > +             } },\n> > >\n> > > I don't have a datasheet but a few online sources confirm a 2.9u pixel\n> > > size.\n> > >\n> > > The patch looks good\n> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > >\n> > > Thanks\n> > >   j\n> > >\n> > > >               { \"imx477\", {\n> > > >                       .unitCellSize = { 1550, 1550 },\n> > > >                       .testPatternModes = {},\n> > > > --\n> > > > 2.43.0\n> > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B2CDCC32F5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 20 Nov 2024 22:07:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D692B65F84;\n\tWed, 20 Nov 2024 23:07:12 +0100 (CET)","from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com\n\t[IPv6:2a00:1450:4864:20::52d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB47E65F69\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Nov 2024 23:07:10 +0100 (CET)","by mail-ed1-x52d.google.com with SMTP id\n\t4fb4d7f45d1cf-5cfe5da1251so240536a12.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Nov 2024 14:07:10 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"G8Hoz1p8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1732140430; x=1732745230;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=Ese43jplvPucz8sDCuHf+Dbp0iUxJX1C9+zAGGtqIoE=;\n\tb=G8Hoz1p8aRXzVBNzTB/u5qYVvZW43JENT1EADr371G/2qx2JXDdwMe2O0QEsWStlIz\n\txthYq0OEi9Ppilv4G9b3nr8tbhHjd41RbgVxxe+5cvDQZXqNLXLZ5h+BvC/gFikN4yY/\n\tAjCH5kmov92JcL94LWXEDCgMFPTyWrH95P17ANrf/8zXIRVV4puZEbbuALc0zOC2yIb5\n\tilzViFkssifpL/PhB2yYeOU0LVpbBSUi7/we339fO61zm2U5cMi+wW/nybOZnxw3Qr2d\n\tZA/dq5VR5c+RA+SZWYU3uA+EWZn17dmQFBNGtk38t3Wxt2SnrvFqtEcdCXVKoUE8cZhH\n\tQp0A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1732140430; x=1732745230;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=Ese43jplvPucz8sDCuHf+Dbp0iUxJX1C9+zAGGtqIoE=;\n\tb=ciCeGQiA2CqptOrumU3kvL0gMyL5+agg1veWPhrBZHUdCd13tKHbumZQ3JkLaHA4Vc\n\tMft7jMILAgc6EumbZ2SWxE08BADWDD/zcpqUqGOHRi5tEUSMH8SFvlZ5cMWNskaJhfT9\n\tI9HX+PCJ/SdVWvo7cFabPtfENFrLjl5kno3CwKDK+uAkdrWnnK3HBmcXqOYhBKORWJwB\n\tLy5Y4HsQvO6duN6d5v1MwbUZXEdSpIpPNhzU+XjtDoMN0l+T1dUkx7JkN36aI5JEiJsH\n\tb9nlTK5CiJtd1p0bYKaAJXOQAmFrUKqFQv5W3XtJd14cpOgunFdzmnsVsBDGG3iOrwXX\n\tONwg==","X-Gm-Message-State":"AOJu0Yzgd13/AgoXa/cijEaMn+d4nPNLijvV7uMPQKefUW7dqLsRLnn1\n\tN408TdhPkBrhqvFVLxrmag18J5SRWzF1jArhdn+Lgwiz+8wqMOBAgWIfF5LRaVO53wg8i/ITbGZ\n\tp1uWdWHBS3J/JZ4ibNl8hYdwDc+4UFhr0","X-Google-Smtp-Source":"AGHT+IFYjR7nNlQKVAYl/0bwrrBmtQiWMtJZ/Q2ZmJsfQOyqpxQPSHgWZYRs9G8jM9shKMnr+bWN8F1/uWUJ9UX1GE8=","X-Received":"by 2002:a05:6402:2712:b0:5cf:c188:81bd with SMTP id\n\t4fb4d7f45d1cf-5cff4c5436dmr2929525a12.9.1732140429909;\n\tWed, 20 Nov 2024 14:07:09 -0800 (PST)","MIME-Version":"1.0","References":"<20241118225310.446706-1-geoffrey.vl@gmail.com>\n\t<20241118225310.446706-2-geoffrey.vl@gmail.com>\n\t<6js3bcqpud6dmnelb3xav3ox7phxv5opcj3mn2iqbug5gmmbp6@zlyg4t5rziif>\n\t<CABX3Wh67aod2jHd=R1mrb8kYmRvUkbveg-xsq+jHDiv39mb9aQ@mail.gmail.com>\n\t<ck5bbkj4eo3zfjk7bjcumnskolnkbw6uyursv6mxivjmzggjfi@rfle57isyikz>","In-Reply-To":"<ck5bbkj4eo3zfjk7bjcumnskolnkbw6uyursv6mxivjmzggjfi@rfle57isyikz>","From":"Geoffrey Van Landeghem <geoffrey.vl@gmail.com>","Date":"Wed, 20 Nov 2024 23:06:58 +0100","Message-ID":"<CABX3Wh4KQ1LWutGpqpzj+Sg+aUoM+he8fLxNApwd1vqC7qW9QA@mail.gmail.com>","Subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, \n\tGeoffrey Van Landeghem <geoffrey.vl@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32322,"web_url":"https://patchwork.libcamera.org/comment/32322/","msgid":"<20241121033623.GE12409@pendragon.ideasonboard.com>","date":"2024-11-21T03:36:23","subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Nov 20, 2024 at 11:06:58PM +0100, Geoffrey Van Landeghem wrote:\n> Hi Jacopo,\n> \n> If I understand correctly, even though the black level register is\n> only 9 bits wide, you take the bit-depth of the sensor in the mode it\n> is configured to work (in my case 12), and then shift it to 16-bits as\n> explained in the libcamera sources. If so the black level for all 3\n> sensors is:\n> \n> /* From datasheet: 0xF0 at 12bits. */\n> blackLevel_ = 3840;\n> \n> Correct?\n\nThat's right, we express all black level values normalized to a 16 bits\ndepth.\n\n> Op wo 20 nov 2024 om 10:00 schreef Jacopo Mondi:\n> > On Tue, Nov 19, 2024 at 10:50:53PM +0100, Geoffrey Van Landeghem wrote:\n> > > Hi Jacopo,\n> > >\n> > > sorry for screwing up the git commit message. I took over the\n> > > suggestion entirely from the V1 patch, which also got me a bit\n> > > confused. The entire git format-patch and git email is still a bit\n> > > awkward and needs some more getting used to it from my side. At work\n> > > we rely on other systems for centralizing source control, but I learn\n> > > something new and I also appreciate your feedback (and patience!).\n> >\n> > No worries at all, it's more than fine to have a few hiccups when\n> > first approaching mail-based patch review.\n> >\n> > >\n> > > Regarding the pixel size, it's indeed 2.9u pixel. I guess the most\n> > > trustworthy resource is this one from Sony:\n> > > https://www.sony-semicon.com/files/62/flyer_security/IMX462LQR_LQR1_Flyer.pdf\n> > > (look for \"unit cell size\")\n> >\n> > Thanks for confirming.\n> >\n> > > And regarding the black level offset setting (register 300Ah). I\n> > > checked the IMX290 and IMX327 datasheets and they match in that they\n> > > both have F0h as default value.\n> > > For IMX462 I couldn't find any datasheet, but since I own the sensor I\n> > > queried it using i2c-tools, and it seems it's also defaulting to F0h:\n> > >\n> > > $ i2ctransfer -f -y 10 w2@0x1a 0x30 0x0A r1\n> > > 0xf0\n> >\n> > Nice, thanks for checking. Feel free to add a patch to add the black\n> > levels to camera sensor helpers if you like to :)\n> >\n> > > Op di 19 nov 2024 om 13:56 schreef Jacopo Mondi <jacopo.mondi@ideasonboard.com>:\n> > > > On Mon, Nov 18, 2024 at 11:53:07PM +0100, Geoffrey Van Landeghem wrote:\n> > > > > The sensor is largely compatible with the already supported Sony IMX290 so we can reuse the same helpers for the analogue gain conversion functions.\n> > > >\n> > > > I presume my suggestion to read https://cbea.ms/git-commit/ first\n> > > > wasn't helpful.\n> > > >\n> > > > Subject line should be shortened (I would not force it to 50 cols\n> > > > as the website suggests, but strive for staying in at least 72), and\n> > > > no full stop at the end.\n> > > >\n> > > > Same thing for the commit message.\n> > > >\n> > > > We can adjust it when applying the patch if you don't have to resend\n> > > > for other reasons.\n> > > >\n> > > > >\n> > > > > Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>\n> > > > > ---\n> > > > >  src/ipa/libipa/camera_sensor_helper.cpp           | 5 +++++\n> > > > >  src/ipa/rpi/cam_helper/cam_helper_imx290.cpp      | 1 +\n> > > > >  src/libcamera/sensor/camera_sensor_properties.cpp | 4 ++++\n> > > > >  3 files changed, 10 insertions(+)\n> > > > >\n> > > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > > index c6169bdc..f870dc28 100644\n> > > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > > @@ -622,6 +622,11 @@ public:\n> > > > >  };\n> > > > >  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n> > > > >\n> > > > > +class CameraSensorHelperImx462 : public CameraSensorHelperImx290\n> > > > > +{\n> > > > > +};\n> > > > > +REGISTER_CAMERA_SENSOR_HELPER(\"imx462\", CameraSensorHelperImx462)\n> > > >\n> > > > One thing we're missing for imx290 is the black level pedestal. The\n> > > > imx290 datasheet reports a (programmable) black level of\n> > > > 10 bits: 0x3c\n> > > > 12 bits: 0xf0\n> > > >\n> > > > Could you confirm the imx462 reports the same default values ?\n> > > >\n> > > > > +\n> > > > >  class CameraSensorHelperImx477 : public CameraSensorHelper\n> > > > >  {\n> > > > >  public:\n> > > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > > > index e57ab538..0cc24a6d 100644\n> > > > > --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > > > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp\n> > > > > @@ -73,3 +73,4 @@ static CamHelper *create()\n> > > > >  }\n> > > > >\n> > > > >  static RegisterCamHelper reg(\"imx290\", &create);\n> > > > > +static RegisterCamHelper reg462(\"imx462\", &create);\n> > > > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > > > index 6d4136d0..e2305166 100644\n> > > > > --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > > > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > > > @@ -142,6 +142,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > > >                       .unitCellSize = { 1450, 1450 },\n> > > > >                       .testPatternModes = {},\n> > > > >               } },\n> > > > > +             { \"imx462\", {\n> > > > > +                     .unitCellSize = { 2900, 2900 },\n> > > > > +                     .testPatternModes = {},\n> > > > > +             } },\n> > > >\n> > > > I don't have a datasheet but a few online sources confirm a 2.9u pixel\n> > > > size.\n> > > >\n> > > > The patch looks good\n> > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > >\n> > > > >               { \"imx477\", {\n> > > > >                       .unitCellSize = { 1550, 1550 },\n> > > > >                       .testPatternModes = {},","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7A610C32F9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 21 Nov 2024 03:36:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 93E7965FA1;\n\tThu, 21 Nov 2024 04:36:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E692F65F54\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 21 Nov 2024 04:36:34 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A3F8E219;\n\tThu, 21 Nov 2024 04:36:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Iq4deQQL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732160176;\n\tbh=FEGCjEQtYL0M8u0Pme4z2zJCHznjIsJXxlRM30F8onE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Iq4deQQLkbqxTFnOVJldQC8w43pfn6hL8DDkUnfd7Ngmga01J/uWwnpps9xGPnHF9\n\teDds9pSH4TBbTnvNXdPIdpS8SSPLHDNJIqrv+1louzDcm0NapNCMZe/7vLIlHEUDi8\n\tMkRjl7/jYABTxfKwyKNHuL+3a3PL/gK0te6EkDOU=","Date":"Thu, 21 Nov 2024 05:36:23 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Geoffrey Van Landeghem <geoffrey.vl@gmail.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 1/4] libcamera: libipa: camera_sensor: Provide\n\tCameraSensorHelper and CameraSensorProperties for the Sony IMX462\n\timage sensor.","Message-ID":"<20241121033623.GE12409@pendragon.ideasonboard.com>","References":"<20241118225310.446706-1-geoffrey.vl@gmail.com>\n\t<20241118225310.446706-2-geoffrey.vl@gmail.com>\n\t<6js3bcqpud6dmnelb3xav3ox7phxv5opcj3mn2iqbug5gmmbp6@zlyg4t5rziif>\n\t<CABX3Wh67aod2jHd=R1mrb8kYmRvUkbveg-xsq+jHDiv39mb9aQ@mail.gmail.com>\n\t<ck5bbkj4eo3zfjk7bjcumnskolnkbw6uyursv6mxivjmzggjfi@rfle57isyikz>\n\t<CABX3Wh4KQ1LWutGpqpzj+Sg+aUoM+he8fLxNApwd1vqC7qW9QA@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CABX3Wh4KQ1LWutGpqpzj+Sg+aUoM+he8fLxNApwd1vqC7qW9QA@mail.gmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]