[{"id":25777,"web_url":"https://patchwork.libcamera.org/comment/25777/","msgid":"<CAPY8ntARxFwo-pYjBUm+jkFHCKEncAt0=cxH4rj-zYkFnHjztw@mail.gmail.com>","date":"2022-11-11T17:00:25","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"Hi Kieran\n\nOn Fri, 11 Nov 2022 at 16:37, Kieran Bingham via libcamera-devel\n<libcamera-devel@lists.libcamera.org> wrote:\n>\n> Add an entry to the sensor properties for the OmniVision OV9281. Test\n> patterns are not enabled yet as the driver is not in an upstream kernel.\n\nIt's not going to be merged to an upstream kernel.\nOV9282 (which is upstream) is the same as OV9281 except for the CRA in\nthe optical path.\nI've sent patches to make ov9282 work with libcamera ([1] and [2]),\nand the Pi kernel is hopefully going to drop our downstream ov9281\ndriver [3] once the patches are accepted by Sakari.\n\nIt does leave the odd subject that Alexander started with regard\nadopting the compatible name rather than module name for the sensor\n[4], and that seems to have gone a bit quiet.\n\nCan I ask what the fascination is with test patterns? Are they really\nthat useful for drivers that have been merged to mainline and\ntherefore really should work?\n\nCheers\n  Dave\n\n[1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/\n[2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/\n[3] https://github.com/raspberrypi/linux/pull/5187\n[4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/\n\n> Unit cell size obtained from [0].\n>\n> [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/libcamera/camera_sensor_properties.cpp | 6 ++++++\n>  1 file changed, 6 insertions(+)\n>\n> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> index e5f27f06eb1d..3a6682f37707 100644\n> --- a/src/libcamera/camera_sensor_properties.cpp\n> +++ b/src/libcamera/camera_sensor_properties.cpp\n> @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  */\n>                         },\n>                 } },\n> +               { \"ov9281\", {\n> +                       .unitCellSize = { 3000, 3000 },\n> +                       .testPatternModes = {\n> +                               { controls::draft::TestPatternModeOff, 0 },\n> +                       },\n> +               } },\n>                 { \"ov13858\", {\n>                         .unitCellSize = { 1120, 1120 },\n>                         .testPatternModes =  {\n> --\n> 2.34.1\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 42B8FBD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 11 Nov 2022 17:00:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 97EC263088;\n\tFri, 11 Nov 2022 18:00:43 +0100 (CET)","from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com\n\t[IPv6:2a00:1450:4864:20::62c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 79BB761F3D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Nov 2022 18:00:42 +0100 (CET)","by mail-ej1-x62c.google.com with SMTP id i10so5214452ejg.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 11 Nov 2022 09:00:42 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668186043;\n\tbh=3AxtS7S2nOtU+yJ3dN8P34HqdmGgEm4YJkZn/G/oMws=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=clw4lmAExIQ/0rIgImNsE4YJN3yX0qSFCYVLVRAcjJZT4bisPwvFwIKNbDodS9y92\n\tAm6ZXrbi+HqiyO0cgw0lz0zAsFqHKynPYhnDminw84u7U9FZj3vmL585mSKshY+JJi\n\tWCSRl7zAl00qgzO60BCk+3Bkny0XJP+iiFGpgvKfKusWuC2lEak6rvOXW23CjxMyh4\n\t2edo9t2ss3oXsJ3Kg4+SrK82YZSuu/4nQk5d27ozBrp+hTD96Jekf47INyXN+4rO55\n\tzjwW5hwLAI9rMhXQC7Mk0ANHSRU7Zdh3bvGNqk749xw4ik1sQ/0J2lbNDF3HmmIK+F\n\t4uINn7NEi91AQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\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=+L8PHW6WyomoQ88aSEUGe1YUCpHAOtWTuVXgLweFGDs=;\n\tb=Dpr4ZCxXZxIKTFNWf4mACIziXW+mlkNwd96piuDqryYTq3pX8bX6QmXwmWrShn8sAm\n\tnkXFbATdBh2d5z5b3EPOrUFbtywDcs7HhVdIOe5p77JA/PqXJ3WcoW8pqlX/ibszFuhn\n\tjENP0lMVhlFwYd7OK1/5Pztmr9+oFFF/LeDdvVFOEvI1ONOy8UJTmDj3OGVs4j/Q3FmA\n\tLUYeLCSzg7vxtVvAxj+I75AjOFVZY+2dOHuCezD6TzVpTs23BOWK8/kKKf0e5sixb0/g\n\tmfIDBD72RZvwIW/Y7/vh/MTkq+w2/lHhMSl7lCjJbWfViOdEb32YL2x3Z5R7yf9cwqtH\n\tQ+sQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"Dpr4ZCxX\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=+L8PHW6WyomoQ88aSEUGe1YUCpHAOtWTuVXgLweFGDs=;\n\tb=ewsuqPEJhbyu16rDgI2hzY83e9fQreJEOe2gE6aEdHnBuj6obrkiM6lQNm2LzLT1h+\n\trETRX025kFMyMn1y090QPGjwKepBCjqtejKdYx/nukQO2zIRaQkSZl0ET4z32Sb5WYHU\n\tGLT7OrdoRjlkWAgQzWJm1MDtF0c9DmMWuo1saxW4NBmhp+QyJ/DPIiAB95GVyDc0qDX2\n\tG8AuZOt4/BFk/pmELCusDe0FJnYHjAu1kEx0OgizeCn0cvXC1ptJzWmVcW4rsO3WPv2t\n\taIqzUl98PpBaU6ENdO8+Ijkp2UokuIVRCtpwhdPoZjxEkGO4XakruxkBFNTRucpglPW4\n\tq3Lg==","X-Gm-Message-State":"ANoB5pnFZxPh4qeiPihh720gG+tt8RFGM0gN/e8ZWCSr5h2Aj2/MRU3C\n\t5w/Q0pHzZGycXMouAAv4OLG0lldSMQ5wHg0707ToUBhgqp0big==","X-Google-Smtp-Source":"AA0mqf7Dg6HqIG9+XHfkUY3TGLaHji3Z47t4I+JGh7vRi6UW80ak+BH2WIiAUl0i0vJEFjXESc/Fc224O87wxmhqYLc=","X-Received":"by 2002:a17:907:8b92:b0:7ae:b43f:cea0 with SMTP id\n\ttb18-20020a1709078b9200b007aeb43fcea0mr2650435ejc.154.1668186042028;\n\tFri, 11 Nov 2022 09:00:42 -0800 (PST)","MIME-Version":"1.0","References":"<20221111163655.280751-1-kieran.bingham@ideasonboard.com>","In-Reply-To":"<20221111163655.280751-1-kieran.bingham@ideasonboard.com>","Date":"Fri, 11 Nov 2022 17:00:25 +0000","Message-ID":"<CAPY8ntARxFwo-pYjBUm+jkFHCKEncAt0=cxH4rj-zYkFnHjztw@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","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>","From":"Dave Stevenson via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Dave Stevenson <dave.stevenson@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":25778,"web_url":"https://patchwork.libcamera.org/comment/25778/","msgid":"<166821085629.50677.16591099318330739884@Monstersaurus>","date":"2022-11-11T23:54:16","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Dave Stevenson (2022-11-11 17:00:25)\n> Hi Kieran\n> \n> On Fri, 11 Nov 2022 at 16:37, Kieran Bingham via libcamera-devel\n> <libcamera-devel@lists.libcamera.org> wrote:\n> >\n> > Add an entry to the sensor properties for the OmniVision OV9281. Test\n> > patterns are not enabled yet as the driver is not in an upstream kernel.\n> \n> It's not going to be merged to an upstream kernel.\n> OV9282 (which is upstream) is the same as OV9281 except for the CRA in\n> the optical path.\n\nWill the sensor differences be exposed to userspace? (i.e. distinct\ncompatibles / identifiers to userspace?) (Ok, I see that's a yes -\nlater).\n\nI believe I had this patch locally, because I saw someone complain about\none of the libcamera warnings so I must have looked up the data sheet to\nget the basics started.\n\n\n> I've sent patches to make ov9282 work with libcamera ([1] and [2]),\n> and the Pi kernel is hopefully going to drop our downstream ov9281\n> driver [3] once the patches are accepted by Sakari.\n> \n\nOk - interesting, everything related to this so far in libcamera is\nov9281.\nlibcamera$ find | grep ov928\n\n./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json\n./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n./src/ipa/raspberrypi/data/ov9281_mono.json\n./src/ipa/raspberrypi/cam_helper_ov9281.cpp\n\n> It does leave the odd subject that Alexander started with regard\n> adopting the compatible name rather than module name for the sensor\n> [4], and that seems to have gone a bit quiet.\n\nThat sounds correct to me, the compatible string could / should\nhighlight the distinction between the two sensors IMO. I'll go throw my\nhat into that thread to state that I see compatibles as the correct way\nto support both of these, even if they are 'almost the same'.\n\nOf course these 'two' sensors are *very* alike, so it does beg, what do\nwe really want to duplicate in libcamera. Or should we have the ability\nto match a single helper to multiple camera identifiers?\n\nI also now wonder if we need to track those differences in CRA in any\nhelper - or if any applications (or IPA) care about that specific\nproperty ?\n\n \n> Can I ask what the fascination is with test patterns? Are they really\n> that useful for drivers that have been merged to mainline and\n> therefore really should work?\n\nIt's purely to be able to map test patterns to the corresponding V4L2\ncontrol id. Because there's no standardised way to express it ... it's a\nmapping of an expected pattern in libcamera, to what that should be\nrepresented as in the V4L2 specific control on the sensor. Each sensor\nseems to do things differently ... ?\n\nI don't think it's anything special except for a workaround for a poorly\ndefined implementation of V4L2_CID_TEST_PATTERN and the like...\n\n--\nKieran\n\n\n> Cheers\n>   Dave\n> \n> [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/\n> [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/\n> [3] https://github.com/raspberrypi/linux/pull/5187\n> [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/\n> \n> > Unit cell size obtained from [0].\n> >\n> > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf\n> >\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++\n> >  1 file changed, 6 insertions(+)\n> >\n> > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > index e5f27f06eb1d..3a6682f37707 100644\n> > --- a/src/libcamera/camera_sensor_properties.cpp\n> > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> >                                  */\n> >                         },\n> >                 } },\n> > +               { \"ov9281\", {\n> > +                       .unitCellSize = { 3000, 3000 },\n> > +                       .testPatternModes = {\n> > +                               { controls::draft::TestPatternModeOff, 0 },\n> > +                       },\n> > +               } },\n> >                 { \"ov13858\", {\n> >                         .unitCellSize = { 1120, 1120 },\n> >                         .testPatternModes =  {\n> > --\n> > 2.34.1\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 A7CF2BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 11 Nov 2022 23:54:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DD45863088;\n\tSat, 12 Nov 2022 00:54:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 82E0763075\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 12 Nov 2022 00:54:19 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03EB289F;\n\tSat, 12 Nov 2022 00:54:18 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668210861;\n\tbh=Y/VWRWKolmYAbsPB2qoW3e1bd2Wadtfa5yJmR+avsA0=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=isGxLAerObqWA1AhwF2t28T1FLCEtJ8qANZKHqMSjtLjTowaQVGfrPBhTIPObDRZX\n\tRRDPCI9RcwWveGlna0JwAp9Hfxql/57junlp9k+KKNmIYGSj9uv7vjTEeiHidlKouC\n\tu0MX1B0KK+hMdNfzP01z4YUp6mnoEfY3RES+UjWULZDA1aREyKX2jdK49z+7ikczcq\n\tJxUXc/H9PHJUwPDb1fjoK+bAdykBQmIpTorOfHIPhrpOyxLVVXF86gEcG9ISLzDxsD\n\t4M4oLI6kIDPQ0q9r3ykkyaGVdPlp3IuZcGJUbi2rwiJADHkjOtNJ3RcnIZMuK+nLEE\n\tMWNjn6mcJmeKQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668210859;\n\tbh=Y/VWRWKolmYAbsPB2qoW3e1bd2Wadtfa5yJmR+avsA0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=R7+11ITjrJxXGby03x6FC9c5MpAItSX6Swy/96mGabpL1MiGClbpeIhwakMltIOEK\n\tJsCgXVJyhzxLzvZHf5na/4OxcnvM7Bq+LJUNQTEOfHDVG2e8uTh4MBm4v96tjI87mS\n\tmMm0zVIboYfJKhS0luUlNDgW/BWya4QKKm0OvKb8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"R7+11ITj\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAPY8ntARxFwo-pYjBUm+jkFHCKEncAt0=cxH4rj-zYkFnHjztw@mail.gmail.com>","References":"<20221111163655.280751-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntARxFwo-pYjBUm+jkFHCKEncAt0=cxH4rj-zYkFnHjztw@mail.gmail.com>","To":"Dave Stevenson <dave.stevenson@raspberrypi.com>","Date":"Fri, 11 Nov 2022 23:54:16 +0000","Message-ID":"<166821085629.50677.16591099318330739884@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":25779,"web_url":"https://patchwork.libcamera.org/comment/25779/","msgid":"<Y3AIRobS2xMl9tsz@pendragon.ideasonboard.com>","date":"2022-11-12T20:55:34","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran\n\nOn Fri, Nov 11, 2022 at 11:54:16PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Dave Stevenson (2022-11-11 17:00:25)\n> > On Fri, 11 Nov 2022 at 16:37, Kieran Bingham wrote:\n> > >\n> > > Add an entry to the sensor properties for the OmniVision OV9281. Test\n> > > patterns are not enabled yet as the driver is not in an upstream kernel.\n> > \n> > It's not going to be merged to an upstream kernel.\n> > OV9282 (which is upstream) is the same as OV9281 except for the CRA in\n> > the optical path.\n> \n> Will the sensor differences be exposed to userspace? (i.e. distinct\n> compatibles / identifiers to userspace?) (Ok, I see that's a yes -\n> later).\n> \n> I believe I had this patch locally, because I saw someone complain about\n> one of the libcamera warnings so I must have looked up the data sheet to\n> get the basics started.\n> \n> > I've sent patches to make ov9282 work with libcamera ([1] and [2]),\n> > and the Pi kernel is hopefully going to drop our downstream ov9281\n> > driver [3] once the patches are accepted by Sakari.\n> \n> Ok - interesting, everything related to this so far in libcamera is\n> ov9281.\n> libcamera$ find | grep ov928\n> \n> ./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json\n> ./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> ./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> ./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> ./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> ./src/ipa/raspberrypi/data/ov9281_mono.json\n> ./src/ipa/raspberrypi/cam_helper_ov9281.cpp\n> \n> > It does leave the odd subject that Alexander started with regard\n> > adopting the compatible name rather than module name for the sensor\n> > [4], and that seems to have gone a bit quiet.\n> \n> That sounds correct to me, the compatible string could / should\n> highlight the distinction between the two sensors IMO. I'll go throw my\n> hat into that thread to state that I see compatibles as the correct way\n> to support both of these, even if they are 'almost the same'.\n> \n> Of course these 'two' sensors are *very* alike, so it does beg, what do\n> we really want to duplicate in libcamera. Or should we have the ability\n> to match a single helper to multiple camera identifiers?\n\nThere's not much data to duplicate at the moment, so I don't mind\nseparate entries, but matching on a set of identifiers is fine too.\n\n> I also now wonder if we need to track those differences in CRA in any\n> helper - or if any applications (or IPA) care about that specific\n> property ?\n\nI'd expect that to affect the tuning file mostly, not the helpers.\n\n> > Can I ask what the fascination is with test patterns? Are they really\n> > that useful for drivers that have been merged to mainline and\n> > therefore really should work?\n> \n> It's purely to be able to map test patterns to the corresponding V4L2\n> control id. Because there's no standardised way to express it ... it's a\n> mapping of an expected pattern in libcamera, to what that should be\n> represented as in the V4L2 specific control on the sensor. Each sensor\n> seems to do things differently ... ?\n> \n> I don't think it's anything special except for a workaround for a poorly\n> defined implementation of V4L2_CID_TEST_PATTERN and the like...\n> \n> > [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/\n> > [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/\n> > [3] https://github.com/raspberrypi/linux/pull/5187\n> > [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/\n> > \n> > > Unit cell size obtained from [0].\n> > >\n> > > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf\n> > >\n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > ---\n> > >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++\n> > >  1 file changed, 6 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > > index e5f27f06eb1d..3a6682f37707 100644\n> > > --- a/src/libcamera/camera_sensor_properties.cpp\n> > > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > >                                  */\n> > >                         },\n> > >                 } },\n> > > +               { \"ov9281\", {\n> > > +                       .unitCellSize = { 3000, 3000 },\n> > > +                       .testPatternModes = {\n> > > +                               { controls::draft::TestPatternModeOff, 0 },\n> > > +                       },\n> > > +               } },\n> > >                 { \"ov13858\", {\n> > >                         .unitCellSize = { 1120, 1120 },\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 5B26BBE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 12 Nov 2022 20:55:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A7E8763086;\n\tSat, 12 Nov 2022 21:55:54 +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 44EA061F37\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 12 Nov 2022 21:55:53 +0100 (CET)","from pendragon.ideasonboard.com (d5152d7bc.static.telenet.be\n\t[81.82.215.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 96A838B;\n\tSat, 12 Nov 2022 21:55:52 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668286554;\n\tbh=TbR7JlnQb8cZ+1HZSXjutJ70cJdVtH9PAuuZPyux1V4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=18tKawN5kOtDsNCI7JqlecoQQQP+o9V5SL7A7bd658wr51//o0cxnncOfeW/2BIfh\n\txocGTx+leqoQTTYLOewtC3Lxao2s8h1h36QNImasFAol4JPsFaKGlKPe9mKyadFXO1\n\t+jCB/EieOi7PPeHXkInscfAh+DKYlpP0OqwwrtRvCi0usj+oGq7C6mFEZMOGwbnWL/\n\t866c1TW2JlgSqCHkqRhW+k+gH9OM+QlJ1DnLoJLhV6MlmpUG6bPXOfarn4RcNAq/sM\n\tofp5/5YwZJWZ2k45mHw9A7yt0x4Ut+H4oD4ZtrgwopekLucrRUi94A8ZqqRmICXxQw\n\tDzeY9CpjWZBuA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668286552;\n\tbh=TbR7JlnQb8cZ+1HZSXjutJ70cJdVtH9PAuuZPyux1V4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YVjHR/gnPyceH+En9ecZJXvkKBskh5UBzesKiOh7sQbZXt2HZAgxO9csgYlCEOmMJ\n\tZ9tWBCXuwDKV+4fM4IQ0KHR6bN9Q6X7F3pOuUsCd3UNOQ/icwks2sggnKgwNxThNCO\n\t9BpEGVT1sWD2ILns3XZjfbr7ThQD7Dr81ltbr8TA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YVjHR/gn\"; dkim-atps=neutral","Date":"Sat, 12 Nov 2022 22:55:34 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<Y3AIRobS2xMl9tsz@pendragon.ideasonboard.com>","References":"<20221111163655.280751-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntARxFwo-pYjBUm+jkFHCKEncAt0=cxH4rj-zYkFnHjztw@mail.gmail.com>\n\t<166821085629.50677.16591099318330739884@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166821085629.50677.16591099318330739884@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":25782,"web_url":"https://patchwork.libcamera.org/comment/25782/","msgid":"<CAPY8ntDm16EXayTHY8sG9PX8gKn8yA5smfL9e_NAwQipp-vp5Q@mail.gmail.com>","date":"2022-11-14T11:47:17","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"HI Kieran & Laurent\n\nOn Sat, 12 Nov 2022 at 20:55, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Kieran\n>\n> On Fri, Nov 11, 2022 at 11:54:16PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > Quoting Dave Stevenson (2022-11-11 17:00:25)\n> > > On Fri, 11 Nov 2022 at 16:37, Kieran Bingham wrote:\n> > > >\n> > > > Add an entry to the sensor properties for the OmniVision OV9281. Test\n> > > > patterns are not enabled yet as the driver is not in an upstream kernel.\n> > >\n> > > It's not going to be merged to an upstream kernel.\n> > > OV9282 (which is upstream) is the same as OV9281 except for the CRA in\n> > > the optical path.\n> >\n> > Will the sensor differences be exposed to userspace? (i.e. distinct\n> > compatibles / identifiers to userspace?) (Ok, I see that's a yes -\n> > later).\n> >\n> > I believe I had this patch locally, because I saw someone complain about\n> > one of the libcamera warnings so I must have looked up the data sheet to\n> > get the basics started.\n> >\n> > > I've sent patches to make ov9282 work with libcamera ([1] and [2]),\n> > > and the Pi kernel is hopefully going to drop our downstream ov9281\n> > > driver [3] once the patches are accepted by Sakari.\n\nSakari's latest pull for 6.2 has just picked the main series, but not\nthe regulator support (needs a review on the driver changes, but DT\nhas been Acked-by RobH). The regulator series has been tagged as\n\"Under Review\" in Patchwork for longer than the main series, but\nperhaps that means nothing.\n\nAt least I can go and fix up my Pi kernel PR with the accepted\npatches, and the Pi kernel will be switching over.\n\n> > Ok - interesting, everything related to this so far in libcamera is\n> > ov9281.\n> > libcamera$ find | grep ov928\n> >\n> > ./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json\n> > ./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > ./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > ./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > ./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > ./src/ipa/raspberrypi/data/ov9281_mono.json\n> > ./src/ipa/raspberrypi/cam_helper_ov9281.cpp\n\n> > > It does leave the odd subject that Alexander started with regard\n> > > adopting the compatible name rather than module name for the sensor\n> > > [4], and that seems to have gone a bit quiet.\n> >\n> > That sounds correct to me, the compatible string could / should\n> > highlight the distinction between the two sensors IMO. I'll go throw my\n> > hat into that thread to state that I see compatibles as the correct way\n> > to support both of these, even if they are 'almost the same'.\n\nMy pending PR picks up patches so that the ov9281 compatible string is\nadded to the driver and it follows through to the sensor name, so it\nmakes no difference to us in userspace.\nFor those using ov9281 on other platforms, they need to make their own\nchoice on extra patches or userspace name.\n\n> > Of course these 'two' sensors are *very* alike, so it does beg, what do\n> > we really want to duplicate in libcamera. Or should we have the ability\n> > to match a single helper to multiple camera identifiers?\n>\n> There's not much data to duplicate at the moment, so I don't mind\n> separate entries, but matching on a set of identifiers is fine too.\n>\n> > I also now wonder if we need to track those differences in CRA in any\n> > helper - or if any applications (or IPA) care about that specific\n> > property ?\n>\n> I'd expect that to affect the tuning file mostly, not the helpers.\n\nMost likely lens shading, but that will change based on the lens\nattached anyway and we have no way of representing that in the\nsystem.....\n\n> > > Can I ask what the fascination is with test patterns? Are they really\n> > > that useful for drivers that have been merged to mainline and\n> > > therefore really should work?\n> >\n> > It's purely to be able to map test patterns to the corresponding V4L2\n> > control id. Because there's no standardised way to express it ... it's a\n> > mapping of an expected pattern in libcamera, to what that should be\n> > represented as in the V4L2 specific control on the sensor. Each sensor\n> > seems to do things differently ... ?\n> >\n> > I don't think it's anything special except for a workaround for a poorly\n> > defined implementation of V4L2_CID_TEST_PATTERN and the like...\n\nBut where is the desperate desire to be able to control test patterns\nwithin libcamera? It seems a huge amount of effort to support for very\nlittle real world use. Just my opinion.\n\n  Dave\n\n> > > [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/\n> > > [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/\n> > > [3] https://github.com/raspberrypi/linux/pull/5187\n> > > [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/\n> > >\n> > > > Unit cell size obtained from [0].\n> > > >\n> > > > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf\n> > > >\n> > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > > ---\n> > > >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++\n> > > >  1 file changed, 6 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > > > index e5f27f06eb1d..3a6682f37707 100644\n> > > > --- a/src/libcamera/camera_sensor_properties.cpp\n> > > > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > > > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > >                                  */\n> > > >                         },\n> > > >                 } },\n> > > > +               { \"ov9281\", {\n> > > > +                       .unitCellSize = { 3000, 3000 },\n> > > > +                       .testPatternModes = {\n> > > > +                               { controls::draft::TestPatternModeOff, 0 },\n> > > > +                       },\n> > > > +               } },\n> > > >                 { \"ov13858\", {\n> > > >                         .unitCellSize = { 1120, 1120 },\n> > > >                         .testPatternModes =  {\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 CC36EBD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Nov 2022 11:47:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 380E76307F;\n\tMon, 14 Nov 2022 12:47:35 +0100 (CET)","from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com\n\t[IPv6:2a00:1450:4864:20::62a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A86F461F3B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Nov 2022 12:47:33 +0100 (CET)","by mail-ej1-x62a.google.com with SMTP id f5so27645494ejc.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Nov 2022 03:47:33 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668426455;\n\tbh=6Sl4FkEDZkUYbXNgP+/qw4aHraiqR5Xi3RIxyVfcF5M=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=gn47xTuwcRveofHHYsKw80dOKMMYxPiGpvd+jFCAp7MY6lWtmEvX6QrXk72HtK9sb\n\tOMIT5mYGIfNi3WkBqcWORHUHYMZw+TkSmn0yJsI9tV1Na4ZJnoqLms/I9wphPJbN2F\n\tFaAtnZ7LjyStIxzfX6VvMUsZrBsNhFC2RRLKLvGP7Vz4Fc3/C962cePeKkYwNWO7rf\n\trZBjKDQ3YNKHYXP3s1aAQmqn8Ox6ujkbqryrTD0Y51qr1VgKiJxpPkL4uhKfKsj0a+\n\tFpz4PziKUUcRi3sB558tLVkesTjSZr/O4pBaiZlkOUpNn/jsYNcN6UVqjqwXz/fM35\n\tG3AfBFlxlsF1Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\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=frvRePKTsWGA6tI+0xiKn8IjZ/RBroqeB2174G45ROI=;\n\tb=WC4XyIRpDekXklhgus6rMtLmfBVI0EZxD5sDcHoZlh3HLmqjlDy4zpyUMmlWYGyvtF\n\tmIL5WWGinF+AjVoGRtdBlSg6A6C4/bfiZBrg/Gv/eJd0FYgeHYeNRkpC8cPTbOoKB70s\n\tLAdhHDLt8jHpO+nDrxtzdEwFXJZZ1dX4lEHXhWD882GbKl/lS0Xzy8kgZQL++BWdoc1f\n\tHF5oywu1LkLU2yC/ok7WKBgugCySO178kp+YadY0RTVrvHTpZ8PsPU86E5BSWUfZXs40\n\tYkT4UHgn1Lbe1ucJr+zR9N/14y6IMfsn6jojrD9iLWMSx4PQK4tHUtBGk2CVHWZgyQb9\n\tW68Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"WC4XyIRp\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=frvRePKTsWGA6tI+0xiKn8IjZ/RBroqeB2174G45ROI=;\n\tb=qtnKQvAti+uYlcdcb8bdvITSLP18aC2Xb2ayQIr+ez2ZWJdXZeT5QdS3A+KD6OSu0S\n\tVJitMtg/qhO02hVQNVCZSGFDwAhFZXJfjX41yCrDYvP71HdTTOPh44QUX7isab7hnIV2\n\tvoKBB17Dtm3ExmxBku1iRZJRdmM+w3vDq0SQ8+6C08K7fkEcvOFTdeSfhb5zerxNCnU0\n\tHJMWeQJga+PjuKxdj1I3zlxjY/7kBs059CbqCIYic04WqUTpNmJvKBwbGdy1YDiBTKBL\n\tKQQ9Ga6nbMNuOVFltHkmCmBLfj/SdmM5xZrnX+vcUtR8vHfQLtgqvj6d7bH7SdcbxlLE\n\tqanA==","X-Gm-Message-State":"ANoB5pl/2EzW8cy/9MvJAWuWx9lgzcZuWiwphOQ9PwSEaixXTpCI4Bls\n\tGApZjfAeM9EIVw34xSal7mCHVd5QMCtMvCJoD6cIvFmFYzw=","X-Google-Smtp-Source":"AA0mqf5WLfhJFpC9fQ4WbfV7bOsl66S/B3FUDN3GsC3URHTaq+UV5qlokkTNX+90Lqww9DePHgAW9Kku/fRPK7QmGv4=","X-Received":"by 2002:a17:906:850a:b0:7ad:bbcc:814 with SMTP id\n\ti10-20020a170906850a00b007adbbcc0814mr10010932ejx.425.1668426453160;\n\tMon, 14 Nov 2022 03:47:33 -0800 (PST)","MIME-Version":"1.0","References":"<20221111163655.280751-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntARxFwo-pYjBUm+jkFHCKEncAt0=cxH4rj-zYkFnHjztw@mail.gmail.com>\n\t<166821085629.50677.16591099318330739884@Monstersaurus>\n\t<Y3AIRobS2xMl9tsz@pendragon.ideasonboard.com>","In-Reply-To":"<Y3AIRobS2xMl9tsz@pendragon.ideasonboard.com>","Date":"Mon, 14 Nov 2022 11:47:17 +0000","Message-ID":"<CAPY8ntDm16EXayTHY8sG9PX8gKn8yA5smfL9e_NAwQipp-vp5Q@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","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>","From":"Dave Stevenson via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Dave Stevenson <dave.stevenson@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":25783,"web_url":"https://patchwork.libcamera.org/comment/25783/","msgid":"<166843142345.50677.17080442461962638626@Monstersaurus>","date":"2022-11-14T13:10:23","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Dave,\n\nQuoting Dave Stevenson (2022-11-14 11:47:17)\n> HI Kieran & Laurent\n> \n> On Sat, 12 Nov 2022 at 20:55, Laurent Pinchart\n> <laurent.pinchart@ideasonboard.com> wrote:\n> >\n> > Hi Kieran\n> >\n> > On Fri, Nov 11, 2022 at 11:54:16PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > > Quoting Dave Stevenson (2022-11-11 17:00:25)\n> > > > On Fri, 11 Nov 2022 at 16:37, Kieran Bingham wrote:\n> > > > >\n> > > > > Add an entry to the sensor properties for the OmniVision OV9281. Test\n> > > > > patterns are not enabled yet as the driver is not in an upstream kernel.\n> > > >\n> > > > It's not going to be merged to an upstream kernel.\n> > > > OV9282 (which is upstream) is the same as OV9281 except for the CRA in\n> > > > the optical path.\n> > >\n> > > Will the sensor differences be exposed to userspace? (i.e. distinct\n> > > compatibles / identifiers to userspace?) (Ok, I see that's a yes -\n> > > later).\n> > >\n> > > I believe I had this patch locally, because I saw someone complain about\n> > > one of the libcamera warnings so I must have looked up the data sheet to\n> > > get the basics started.\n> > >\n> > > > I've sent patches to make ov9282 work with libcamera ([1] and [2]),\n> > > > and the Pi kernel is hopefully going to drop our downstream ov9281\n> > > > driver [3] once the patches are accepted by Sakari.\n> \n> Sakari's latest pull for 6.2 has just picked the main series, but not\n> the regulator support (needs a review on the driver changes, but DT\n> has been Acked-by RobH). The regulator series has been tagged as\n> \"Under Review\" in Patchwork for longer than the main series, but\n> perhaps that means nothing.\n> \n> At least I can go and fix up my Pi kernel PR with the accepted\n> patches, and the Pi kernel will be switching over.\n> \n> > > Ok - interesting, everything related to this so far in libcamera is\n> > > ov9281.\n> > > libcamera$ find | grep ov928\n> > >\n> > > ./install/libcamera-gcc/usr/share/libcamera/ipa/raspberrypi/ov9281.json\n> > > ./build/package/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > > ./build/test/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > > ./build/rpi/bullseye/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > > ./build/gcc/src/ipa/raspberrypi/ipa_rpi.so.p/cam_helper_ov9281.cpp.o\n> > > ./src/ipa/raspberrypi/data/ov9281_mono.json\n> > > ./src/ipa/raspberrypi/cam_helper_ov9281.cpp\n> \n> > > > It does leave the odd subject that Alexander started with regard\n> > > > adopting the compatible name rather than module name for the sensor\n> > > > [4], and that seems to have gone a bit quiet.\n> > >\n> > > That sounds correct to me, the compatible string could / should\n> > > highlight the distinction between the two sensors IMO. I'll go throw my\n> > > hat into that thread to state that I see compatibles as the correct way\n> > > to support both of these, even if they are 'almost the same'.\n> \n> My pending PR picks up patches so that the ov9281 compatible string is\n> added to the driver and it follows through to the sensor name, so it\n> makes no difference to us in userspace.\n> For those using ov9281 on other platforms, they need to make their own\n> choice on extra patches or userspace name.\n> \n> > > Of course these 'two' sensors are *very* alike, so it does beg, what do\n> > > we really want to duplicate in libcamera. Or should we have the ability\n> > > to match a single helper to multiple camera identifiers?\n> >\n> > There's not much data to duplicate at the moment, so I don't mind\n> > separate entries, but matching on a set of identifiers is fine too.\n> >\n> > > I also now wonder if we need to track those differences in CRA in any\n> > > helper - or if any applications (or IPA) care about that specific\n> > > property ?\n> >\n> > I'd expect that to affect the tuning file mostly, not the helpers.\n> \n> Most likely lens shading, but that will change based on the lens\n> attached anyway and we have no way of representing that in the\n> system.....\n> \n> > > > Can I ask what the fascination is with test patterns? Are they really\n> > > > that useful for drivers that have been merged to mainline and\n> > > > therefore really should work?\n> > >\n> > > It's purely to be able to map test patterns to the corresponding V4L2\n> > > control id. Because there's no standardised way to express it ... it's a\n> > > mapping of an expected pattern in libcamera, to what that should be\n> > > represented as in the V4L2 specific control on the sensor. Each sensor\n> > > seems to do things differently ... ?\n> > >\n> > > I don't think it's anything special except for a workaround for a poorly\n> > > defined implementation of V4L2_CID_TEST_PATTERN and the like...\n> \n> But where is the desperate desire to be able to control test patterns\n> within libcamera? It seems a huge amount of effort to support for very\n> little real world use. Just my opinion.\n\nI believe test tools from ChromeOS require being able to set the test\npattern on a sensor. They expect to know what pattern will be generated,\nand validate that image is received.\n\nIt's nothing more than to change \"Give me 'any' test image\" into \"Give\nme an expected / defined test image\"...\n\nThis is also an optional feature - I don't believe there is a\nrequirement to add the I would imagine us doing similar test pattern\nsupport. That's why this patch ... hasn't!\n\n--\nKieran\n\n\n\n>   Dave\n> \n> > > > [1] https://patchwork.linuxtv.org/project/linux-media/cover/20221005152018.3783890-1-dave.stevenson@raspberrypi.com/\n> > > > [2] https://patchwork.linuxtv.org/project/linux-media/cover/20221028160902.2696973-1-dave.stevenson@raspberrypi.com/\n> > > > [3] https://github.com/raspberrypi/linux/pull/5187\n> > > > [4] https://patchwork.linuxtv.org/project/linux-media/patch/20220728130237.3396663-7-alexander.stein@ew.tq-group.com/\n> > > >\n> > > > > Unit cell size obtained from [0].\n> > > > >\n> > > > > [0] https://www.ovt.com/wp-content/uploads/2022/01/OV9281-OV9282-PB-v1.3-WEB.pdf\n> > > > >\n> > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > > > ---\n> > > > >  src/libcamera/camera_sensor_properties.cpp | 6 ++++++\n> > > > >  1 file changed, 6 insertions(+)\n> > > > >\n> > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > > > > index e5f27f06eb1d..3a6682f37707 100644\n> > > > > --- a/src/libcamera/camera_sensor_properties.cpp\n> > > > > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > > > > @@ -160,6 +160,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > > >                                  */\n> > > > >                         },\n> > > > >                 } },\n> > > > > +               { \"ov9281\", {\n> > > > > +                       .unitCellSize = { 3000, 3000 },\n> > > > > +                       .testPatternModes = {\n> > > > > +                               { controls::draft::TestPatternModeOff, 0 },\n> > > > > +                       },\n> > > > > +               } },\n> > > > >                 { \"ov13858\", {\n> > > > >                         .unitCellSize = { 1120, 1120 },\n> > > > >                         .testPatternModes =  {\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","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 8BFD6BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Nov 2022 13:10:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B4FCC63088;\n\tMon, 14 Nov 2022 14:10:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4F5B461F3B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Nov 2022 14:10:26 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BD2EC891;\n\tMon, 14 Nov 2022 14:10:25 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668431427;\n\tbh=8bItR+3AG1ck29GhfoHo3lhfkR3owdjJmCtK1dMrFgQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=MjPgE1Gs0Ax6r4Tc/mtgxpVyi9eANAFuv5TOpIIxhiAt6nnkOo25bOlRFJMmbB1Kg\n\tSLE3ejDh5QWd+jxN0SH1/v043VCHr+8kNte5OVIWgFHzHsNGVrh1vkMlULiZrIaxFO\n\tA3bwUZILUwVaEH0XV/mbTnPSnRElmYwNrbBfA8hR0megcjVfSMAypAgXIZdX3JpJ5h\n\t5d0Rn19bzSTFEboniDCn5Hmau1K7GahkWbei6sTjkZVbGKLcMz55x3c7EGF1umHglJ\n\tZe2v7L6OrqHnoD3BFlQToix6ODIhqRVbCQF21LgG8yts3oiBMcYiwoIxcwIdU3A9MO\n\txQkCAcaVMgWBw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668431425;\n\tbh=8bItR+3AG1ck29GhfoHo3lhfkR3owdjJmCtK1dMrFgQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=VQ8Q/idQfhi/lvCHZXonuwAgpQOlS048YrYp+en/WD1q3cFi8UDygp6xde+zFzCEZ\n\tRWp618XBLv4trFikkFYOTXgCEiQLm5z5Qrhv4Ryy96gmDKsDreFk/WI7RNVbI2QJCK\n\tF58q7DWX4tRo0IkmE6cOk5ozB/Mjfd795vMvVIXI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VQ8Q/idQ\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAPY8ntDm16EXayTHY8sG9PX8gKn8yA5smfL9e_NAwQipp-vp5Q@mail.gmail.com>","References":"<20221111163655.280751-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntARxFwo-pYjBUm+jkFHCKEncAt0=cxH4rj-zYkFnHjztw@mail.gmail.com>\n\t<166821085629.50677.16591099318330739884@Monstersaurus>\n\t<Y3AIRobS2xMl9tsz@pendragon.ideasonboard.com>\n\t<CAPY8ntDm16EXayTHY8sG9PX8gKn8yA5smfL9e_NAwQipp-vp5Q@mail.gmail.com>","To":"Dave Stevenson <dave.stevenson@raspberrypi.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Mon, 14 Nov 2022 13:10:23 +0000","Message-ID":"<166843142345.50677.17080442461962638626@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Add OV9281\n\tsensor properties","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]