[{"id":28053,"web_url":"https://patchwork.libcamera.org/comment/28053/","msgid":"<20231102184133.GB7129@pendragon.ideasonboard.com>","date":"2023-11-02T18:41:33","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Thu, Nov 02, 2023 at 06:09:16PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Provide support for the Sony IMX335 in both libipa and RaspberryPi IPA\n> modules.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/libipa/camera_sensor_helper.cpp | 23 ++++++\n> src/ipa/rpi/cam_helper/cam_helper_imx335.cpp | 74 ++++++++++++++++++++\n> src/ipa/rpi/cam_helper/meson.build | 1 +\n> src/libcamera/camera_sensor_properties.cpp | 4 ++\n> 4 files changed, 102 insertions(+)\n> create mode 100644 src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> \n> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> index f0ecc3830115..ddab5af6eac2 100644\n> --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> @@ -444,6 +444,29 @@ class CameraSensorHelperImx327 : public CameraSensorHelperImx290\n> };\n> REGISTER_CAMERA_SENSOR_HELPER(\"imx327\", CameraSensorHelperImx327)\n> \n> +class CameraSensorHelperImx335 : public CameraSensorHelper\n> +{\n> +public:\n> +\tuint32_t gainCode(double gain) const override;\n> +\tdouble gain(uint32_t gainCode) const override;\n> +private:\n> +\tstatic constexpr uint32_t maxGainCode_ = 240;\n> +};\n> +\n> +uint32_t CameraSensorHelperImx335::gainCode(double gain) const\n> +{\n> +\tuint32_t code = 10 * std::log10(gain) * 10 / 3;\n\nThat looks like an exponential model, there's a helper for that.\n\n> +\n> +\treturn std::min(code, maxGainCode_);\n> +}\n> +\n> +double CameraSensorHelperImx335::gain(uint32_t gainCode) const\n> +{\n> +\treturn std::pow(10.0, gainCode / (10 * 10 / 3));\n> +}\n> +\n> +REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> +\n> class CameraSensorHelperImx477 : public CameraSensorHelper\n> {\n> public:\n> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> new file mode 100644\n> index 000000000000..659c69d6b6c7\n> --- /dev/null\n> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> @@ -0,0 +1,74 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board Oy.\n> + *\n> + * cam_helper_imx335.cpp - camera information for the Sony IMX335 sensor\n> + */\n> +\n> +#include \n> +\n> +#include \"cam_helper.h\"\n> +#include \"math.h\"\n> +\n> +using namespace RPiController;\n> +\n> +class CamHelperImx335 : public CamHelper\n> +{\n> +public:\n> +\tCamHelperImx335();\n> +\tuint32_t gainCode(double gain) const override;\n> +\tdouble gain(uint32_t gainCode) const override;\n> +\tvoid getDelays(int &exposureDelay, int &gainDelay,\n> +\t\t int &vblankDelay, int &hblankDelay) const override;\n> +\tunsigned int hideFramesModeSwitch() const override;\n> +\n> +private:\n> +\t/*\n> +\t * Smallest difference between the frame length and integration time,\n> +\t * in units of lines.\n> +\t */\n> +\tstatic constexpr int frameIntegrationDiff = 4;\n> +\tstatic constexpr uint32_t maxGainCode = 240;\n> +};\n> +\n> +/*\n> + * IMX335 Metadata isn't yet supported.\n> + */\n> +\n> +CamHelperImx335::CamHelperImx335()\n> +\t: CamHelper({}, frameIntegrationDiff)\n> +{\n> +}\n> +\n> +uint32_t CamHelperImx335::gainCode(double gain) const\n> +{\n> +\tuint32_t code = 10 * std::log10(gain) * 10 / 3;\n> +\treturn std::min(code, maxGainCode);\n> +}\n> +\n> +double CamHelperImx335::gain(uint32_t gainCode) const\n> +{\n> +\treturn std::pow(10.0, gainCode / (10 * 10 / 3));\n> +}\n> +\n> +void CamHelperImx335::getDelays(int &exposureDelay, int &gainDelay,\n> +\t\t\t\tint &vblankDelay, int &hblankDelay) const\n> +{\n> +\texposureDelay = 2;\n> +\tgainDelay = 2;\n> +\tvblankDelay = 2;\n> +\thblankDelay = 2;\n\nHave you validated all these delays ?\n\n> +}\n> +\n> +unsigned int CamHelperImx335::hideFramesModeSwitch() const\n> +{\n> +\t/* One bad frame can be expected after a mode switch. */\n> +\treturn 1;\n\nSame here.\n\n> +}\n> +\n> +static CamHelper *create()\n> +{\n> +\treturn new CamHelperImx335();\n> +}\n> +\n> +static RegisterCamHelper reg(\"imx335\", &create);\n> diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build\n> index bdf2db8eb742..17c25cb0e4a6 100644\n> --- a/src/ipa/rpi/cam_helper/meson.build\n> +++ b/src/ipa/rpi/cam_helper/meson.build\n> @@ -6,6 +6,7 @@ rpi_ipa_cam_helper_sources = files([\n> 'cam_helper_imx219.cpp',\n> 'cam_helper_imx290.cpp',\n> 'cam_helper_imx296.cpp',\n> + 'cam_helper_imx335.cpp',\n> 'cam_helper_imx477.cpp',\n> 'cam_helper_imx519.cpp',\n> 'cam_helper_imx708.cpp',\n> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> index 27d6799a2686..dc76051fa349 100644\n> --- a/src/libcamera/camera_sensor_properties.cpp\n> +++ b/src/libcamera/camera_sensor_properties.cpp\n> @@ -111,6 +111,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t.unitCellSize = { 2900, 2900 },\n> \t\t\t.testPatternModes = {},\n> \t\t} },\n> +\t\t{ \"imx335\", {\n> +\t\t\t.unitCellSize = { 2000, 2000 },\n> +\t\t\t.testPatternModes = {},\n> +\t\t} },\n> \t\t{ \"imx477\", {\n> \t\t\t.unitCellSize = { 1550, 1550 },\n> \t\t\t.testPatternModes = {},","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7C4E2BD16B\n\tfor ;\n\tThu, 2 Nov 2023 18:41:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB4AA6298F;\n\tThu, 2 Nov 2023 19:41:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 479EA61DC6\n\tfor ;\n\tThu, 2 Nov 2023 19:41:27 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 58F338C2;\n\tThu, 2 Nov 2023 19:41:09 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1698950488;\n\tbh=PpxpjsUHHTKqSjP9Tw8kh8nBSjgj2+6gfoF/FHJ1TXE=;\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=1ysAvMXYQwJeEZOve2vgZEZOB9usTqIUQqDJ2HAlKA+T4eng/ieOYpv7L/bdGd75y\n\tOHWUA9RRter/sa82NsefGK8KMzsa9+b/u4vDFIa5DiGIliXVfspv1c8LSWpN7OVmH1\n\t+xmq35blnWXKA7qdQTqegnng/KKZgGQV2RwLUylfH6bBV/ZDTbawdcIL0NYvK20hRS\n\t0jY2E363GhgHCs1qPvwZ8x3B9Zjnc2FNUtFHNAA0ncqs/MF3XtXXIi2i22baSGn7iN\n\tmQ9c86fYHE3ZIIdJ1h+/9ac9qaN66nBLf5os2LJEl7ULLwAO8vHnidQUknDSN+Xnas\n\tTSmMgklQK3SZw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1698950469;\n\tbh=PpxpjsUHHTKqSjP9Tw8kh8nBSjgj2+6gfoF/FHJ1TXE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dv7tjpTKw5/EZQ3ouDFVsMyKiGeMaA1K5IaNVqCyqppJyGuo72vmC3WME1B7Pba34\n\tU/ORAjVRDfBtBEt2Kx17/Z3RO0Mi68FPfjmOpfW7wVQAJ+iGARssQ8IpZA49610pht\n\tmRzWX5AdT4oJBDmCNF+BkqaQifzhgKROvyrW6gfk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dv7tjpTK\"; dkim-atps=neutral","Date":"Thu, 2 Nov 2023 20:41:33 +0200","To":"Kieran Bingham ","Message-ID":"<20231102184133.GB7129@pendragon.ideasonboard.com>","References":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28054,"web_url":"https://patchwork.libcamera.org/comment/28054/","msgid":"","date":"2023-11-03T09:30:29","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Thu, 2 Nov 2023 at 18:09, Kieran Bingham via libcamera-devel\n wrote:\n>\n> Provide support for the Sony IMX335 in both libipa and RaspberryPi IPA\n> modules.\n\nWithout an imx335.json camera tuning file present, this will not work\non the RPi platform. Was that file meant to be added in this patch?\n\nRegards,\nNaush\n\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/libipa/camera_sensor_helper.cpp | 23 ++++++\n> src/ipa/rpi/cam_helper/cam_helper_imx335.cpp | 74 ++++++++++++++++++++\n> src/ipa/rpi/cam_helper/meson.build | 1 +\n> src/libcamera/camera_sensor_properties.cpp | 4 ++\n> 4 files changed, 102 insertions(+)\n> create mode 100644 src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n>\n> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> index f0ecc3830115..ddab5af6eac2 100644\n> --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> @@ -444,6 +444,29 @@ class CameraSensorHelperImx327 : public CameraSensorHelperImx290\n> };\n> REGISTER_CAMERA_SENSOR_HELPER(\"imx327\", CameraSensorHelperImx327)\n>\n> +class CameraSensorHelperImx335 : public CameraSensorHelper\n> +{\n> +public:\n> + uint32_t gainCode(double gain) const override;\n> + double gain(uint32_t gainCode) const override;\n> +private:\n> + static constexpr uint32_t maxGainCode_ = 240;\n> +};\n> +\n> +uint32_t CameraSensorHelperImx335::gainCode(double gain) const\n> +{\n> + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> +\n> + return std::min(code, maxGainCode_);\n> +}\n> +\n> +double CameraSensorHelperImx335::gain(uint32_t gainCode) const\n> +{\n> + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> +}\n> +\n> +REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> +\n> class CameraSensorHelperImx477 : public CameraSensorHelper\n> {\n> public:\n> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> new file mode 100644\n> index 000000000000..659c69d6b6c7\n> --- /dev/null\n> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> @@ -0,0 +1,74 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board Oy.\n> + *\n> + * cam_helper_imx335.cpp - camera information for the Sony IMX335 sensor\n> + */\n> +\n> +#include \n> +\n> +#include \"cam_helper.h\"\n> +#include \"math.h\"\n> +\n> +using namespace RPiController;\n> +\n> +class CamHelperImx335 : public CamHelper\n> +{\n> +public:\n> + CamHelperImx335();\n> + uint32_t gainCode(double gain) const override;\n> + double gain(uint32_t gainCode) const override;\n> + void getDelays(int &exposureDelay, int &gainDelay,\n> + int &vblankDelay, int &hblankDelay) const override;\n> + unsigned int hideFramesModeSwitch() const override;\n> +\n> +private:\n> + /*\n> + * Smallest difference between the frame length and integration time,\n> + * in units of lines.\n> + */\n> + static constexpr int frameIntegrationDiff = 4;\n> + static constexpr uint32_t maxGainCode = 240;\n> +};\n> +\n> +/*\n> + * IMX335 Metadata isn't yet supported.\n> + */\n> +\n> +CamHelperImx335::CamHelperImx335()\n> + : CamHelper({}, frameIntegrationDiff)\n> +{\n> +}\n> +\n> +uint32_t CamHelperImx335::gainCode(double gain) const\n> +{\n> + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> + return std::min(code, maxGainCode);\n> +}\n> +\n> +double CamHelperImx335::gain(uint32_t gainCode) const\n> +{\n> + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> +}\n> +\n> +void CamHelperImx335::getDelays(int &exposureDelay, int &gainDelay,\n> + int &vblankDelay, int &hblankDelay) const\n> +{\n> + exposureDelay = 2;\n> + gainDelay = 2;\n> + vblankDelay = 2;\n> + hblankDelay = 2;\n> +}\n> +\n> +unsigned int CamHelperImx335::hideFramesModeSwitch() const\n> +{\n> + /* One bad frame can be expected after a mode switch. */\n> + return 1;\n> +}\n> +\n> +static CamHelper *create()\n> +{\n> + return new CamHelperImx335();\n> +}\n> +\n> +static RegisterCamHelper reg(\"imx335\", &create);\n> diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build\n> index bdf2db8eb742..17c25cb0e4a6 100644\n> --- a/src/ipa/rpi/cam_helper/meson.build\n> +++ b/src/ipa/rpi/cam_helper/meson.build\n> @@ -6,6 +6,7 @@ rpi_ipa_cam_helper_sources = files([\n> 'cam_helper_imx219.cpp',\n> 'cam_helper_imx290.cpp',\n> 'cam_helper_imx296.cpp',\n> + 'cam_helper_imx335.cpp',\n> 'cam_helper_imx477.cpp',\n> 'cam_helper_imx519.cpp',\n> 'cam_helper_imx708.cpp',\n> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> index 27d6799a2686..dc76051fa349 100644\n> --- a/src/libcamera/camera_sensor_properties.cpp\n> +++ b/src/libcamera/camera_sensor_properties.cpp\n> @@ -111,6 +111,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> .unitCellSize = { 2900, 2900 },\n> .testPatternModes = {},\n> } },\n> + { \"imx335\", {\n> + .unitCellSize = { 2000, 2000 },\n> + .testPatternModes = {},\n> + } },\n> { \"imx477\", {\n> .unitCellSize = { 1550, 1550 },\n> .testPatternModes = {},\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B7041BDE6B\n\tfor ;\n\tFri, 3 Nov 2023 09:31:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9FA556298F;\n\tFri, 3 Nov 2023 10:31:01 +0100 (CET)","from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com\n\t[IPv6:2607:f8b0:4864:20::b29])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3B1F61DC3\n\tfor ;\n\tFri, 3 Nov 2023 10:30:58 +0100 (CET)","by mail-yb1-xb29.google.com with SMTP id\n\t3f1490d57ef6-da0359751dbso1639524276.1\n\tfor ;\n\tFri, 03 Nov 2023 02:30:58 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699003861;\n\tbh=UbJf0iUxWq9w2tpQQCXV7Od2SS1AybTwqk5O9myYi0Y=;\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=PoMr8LETPoEspMJstgkqgSX8pfwDzCqiTl3bbDUFcnIO4M0fJeyf7sOWiUdD7yaEV\n\tnjxh1L7k8hNllGDUCQJj4Kz7r59NMOkiEfvWAN6219sm1OBFB9neAsIUfQqVHD+oqd\n\tSXsFNuTAbEl+eu8Vh0c4FkeLn3UlARIxai9W79aqspzKEsG0Iej6YQO/h+p/zqzl+w\n\tj3jeHAW6ecJk+7iNXGkg7130Sl2+vlmElu+qtbw7qR4m8qSPwfzklc/myRqyFF0oLm\n\teRV5xyjqmuivQ49Tou2qMOLWYPssLEFU7CzPlcHyn2FV771f1CPjErUaoCc79kdGMo\n\t67Il+CS715gcQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1699003857; x=1699608657;\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=+/5MRQ517EWxWVvvmSGBf3yIck4UmSx1ejI0j8fuFUY=;\n\tb=REmcYOdIE+GOvaZhBJUV6rHFsUH0Y2wHVGyKQcf0FhclNi5IREq6+78VpTc6beVBbk\n\te0wpNr6F5coKR5VBvimH3tMgFkyM+YjhM5VQuguK9zCkjJwJ7A0oJuG03ea1mdR/3ihB\n\tKKH7kmn6Zs7+uMgfHgPv/0OuqWzjqThIXz+FI6eOqrKQxjDbzxUzxcvvAhQvjORLtWY6\n\tbPZ/sgSD17a73kon0CE3+R6RC17ZjqGIwgWuhFU/MQ4Z+7NFgSpbf1Rarn0G/j/la3nk\n\tkBh2lVMQkkFV6TCMqPBU2HNNoncySYnzkUDBPdl438XR0pxkvBgXSmoxSJdsGzH5v1zs\n\thlLw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"REmcYOdI\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1699003857; x=1699608657;\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=+/5MRQ517EWxWVvvmSGBf3yIck4UmSx1ejI0j8fuFUY=;\n\tb=Xpni4tgyRKOhWM0PLejl4ussVy6B46oq2UI/mtNUSHIKZjgAikUch3VPntVeXxctzS\n\titJWh52e3cGb2kLRRHYh//WwDSy19zAjM87M7jkSRT9ZzIX/mZhFBHIn0gnGbAxtaM7y\n\tqcjSkERxjKFUYGaWuLKoaPxwjdOxuQI3YmaFSY9AVl8tQEzZTrwNSoKhGHWaObRbtxrT\n\tOsc/+rbiqr1efo3kNFsm3ma2kyiP6HwXhiN1ncTOHuQwMUXL6MIuVZhVT1hJiO/qCuGL\n\tVSYsQLiXdRuTVZhr3+NKsb4VME1k+qMSKoFnP3Lj1ma9+ov2eatPX8h8IWomDMy5PJDj\n\tr/nw==","X-Gm-Message-State":"AOJu0YxfAZp6gDVKjd2loYBu4S9GpF3OIhuU/JtbWIno42NGsmUZthkb\n\tsve8WpKMQgQD/tUio2kjcRVGJx/2vfTKZqMU8F+HvA==","X-Google-Smtp-Source":"AGHT+IEVWmD8ye6oUObyUihZrnqHAKYl0l1Q6yi3+5Zl1zMLswRFWu7oNdWsVu/cH4oEDHLGn1aZ5hBOTFOwun7SWLc=","X-Received":"by 2002:a25:73cb:0:b0:d86:4342:290 with SMTP id\n\to194-20020a2573cb000000b00d8643420290mr1985309ybc.21.1699003857423;\n\tFri, 03 Nov 2023 02:30:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>","In-Reply-To":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>","Date":"Fri, 3 Nov 2023 09:30:29 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28059,"web_url":"https://patchwork.libcamera.org/comment/28059/","msgid":"<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>","date":"2023-11-06T22:17:58","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nQuoting Naushir Patuck (2023-11-03 09:30:29)\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, 2 Nov 2023 at 18:09, Kieran Bingham via libcamera-devel\n> wrote:\n> >\n> > Provide support for the Sony IMX335 in both libipa and RaspberryPi IPA\n> > modules.\n> \n> Without an imx335.json camera tuning file present, this will not work\n> on the RPi platform. Was that file meant to be added in this patch?\n\nSo far I've used only an unmodified 'uncalibrated.json' for both RPi4\nand Pi5.\n\nI've now successfully tested the module and updated kernel driver with\nboth 2 and 4 lane operation (2 lane on RPi4, 4 lane on Pi5).\n\nI haven't yet gone through a full 'tuning' process on Raspberry Pi\nthough as it's not my target platform for this camera module. But it's\n/very/ helpful that I can test on Raspberry Pi.\n\nIt gives good results even using uncalibrated.json on both platforms\ntoo! (Which I believe is a testament to Raspberry Pi's IPA development!)\n\n\nI would always like to think that 'every camera' which can be connected\nshould be supported on all platforms. But I don't know if that means to\nget a camera into libcamera it should be tuned for each platform\nexplicitly?\n\n\nI'm weary of providing a Raspberry Pi 'tuning' file that implies I have\nperformed any full tuning on the module. What are your thoughts about\ndefaulting to use the 'uncalibrated.json' when no tuning file is\nidentified (and printing a warning to report this?)\n\nOtherwise I would be submitting essentially\n 'cp uncalibrated.json imx335.json'\n\nAnd that would perhaps give potential future users undue belief in the\ntuning file. I'm going to go through the tuning process on a Pi now -\nbut even with that - this would only be a very 'home lab' basic effort.\n\n--\nRegards\n\nKieran\n\n\n> \n> Regards,\n> Naush\n> \n> >\n> > Signed-off-by: Kieran Bingham \n> > ---\n> > src/ipa/libipa/camera_sensor_helper.cpp | 23 ++++++\n> > src/ipa/rpi/cam_helper/cam_helper_imx335.cpp | 74 ++++++++++++++++++++\n> > src/ipa/rpi/cam_helper/meson.build | 1 +\n> > src/libcamera/camera_sensor_properties.cpp | 4 ++\n> > 4 files changed, 102 insertions(+)\n> > create mode 100644 src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> >\n> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > index f0ecc3830115..ddab5af6eac2 100644\n> > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > @@ -444,6 +444,29 @@ class CameraSensorHelperImx327 : public CameraSensorHelperImx290\n> > };\n> > REGISTER_CAMERA_SENSOR_HELPER(\"imx327\", CameraSensorHelperImx327)\n> >\n> > +class CameraSensorHelperImx335 : public CameraSensorHelper\n> > +{\n> > +public:\n> > + uint32_t gainCode(double gain) const override;\n> > + double gain(uint32_t gainCode) const override;\n> > +private:\n> > + static constexpr uint32_t maxGainCode_ = 240;\n> > +};\n> > +\n> > +uint32_t CameraSensorHelperImx335::gainCode(double gain) const\n> > +{\n> > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > +\n> > + return std::min(code, maxGainCode_);\n> > +}\n> > +\n> > +double CameraSensorHelperImx335::gain(uint32_t gainCode) const\n> > +{\n> > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > +}\n> > +\n> > +REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> > +\n> > class CameraSensorHelperImx477 : public CameraSensorHelper\n> > {\n> > public:\n> > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > new file mode 100644\n> > index 000000000000..659c69d6b6c7\n> > --- /dev/null\n> > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > @@ -0,0 +1,74 @@\n> > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > +/*\n> > + * Copyright (C) 2023, Ideas on Board Oy.\n> > + *\n> > + * cam_helper_imx335.cpp - camera information for the Sony IMX335 sensor\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \"cam_helper.h\"\n> > +#include \"math.h\"\n> > +\n> > +using namespace RPiController;\n> > +\n> > +class CamHelperImx335 : public CamHelper\n> > +{\n> > +public:\n> > + CamHelperImx335();\n> > + uint32_t gainCode(double gain) const override;\n> > + double gain(uint32_t gainCode) const override;\n> > + void getDelays(int &exposureDelay, int &gainDelay,\n> > + int &vblankDelay, int &hblankDelay) const override;\n> > + unsigned int hideFramesModeSwitch() const override;\n> > +\n> > +private:\n> > + /*\n> > + * Smallest difference between the frame length and integration time,\n> > + * in units of lines.\n> > + */\n> > + static constexpr int frameIntegrationDiff = 4;\n> > + static constexpr uint32_t maxGainCode = 240;\n> > +};\n> > +\n> > +/*\n> > + * IMX335 Metadata isn't yet supported.\n> > + */\n> > +\n> > +CamHelperImx335::CamHelperImx335()\n> > + : CamHelper({}, frameIntegrationDiff)\n> > +{\n> > +}\n> > +\n> > +uint32_t CamHelperImx335::gainCode(double gain) const\n> > +{\n> > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > + return std::min(code, maxGainCode);\n> > +}\n> > +\n> > +double CamHelperImx335::gain(uint32_t gainCode) const\n> > +{\n> > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > +}\n> > +\n> > +void CamHelperImx335::getDelays(int &exposureDelay, int &gainDelay,\n> > + int &vblankDelay, int &hblankDelay) const\n> > +{\n> > + exposureDelay = 2;\n> > + gainDelay = 2;\n> > + vblankDelay = 2;\n> > + hblankDelay = 2;\n> > +}\n> > +\n> > +unsigned int CamHelperImx335::hideFramesModeSwitch() const\n> > +{\n> > + /* One bad frame can be expected after a mode switch. */\n> > + return 1;\n> > +}\n> > +\n> > +static CamHelper *create()\n> > +{\n> > + return new CamHelperImx335();\n> > +}\n> > +\n> > +static RegisterCamHelper reg(\"imx335\", &create);\n> > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build\n> > index bdf2db8eb742..17c25cb0e4a6 100644\n> > --- a/src/ipa/rpi/cam_helper/meson.build\n> > +++ b/src/ipa/rpi/cam_helper/meson.build\n> > @@ -6,6 +6,7 @@ rpi_ipa_cam_helper_sources = files([\n> > 'cam_helper_imx219.cpp',\n> > 'cam_helper_imx290.cpp',\n> > 'cam_helper_imx296.cpp',\n> > + 'cam_helper_imx335.cpp',\n> > 'cam_helper_imx477.cpp',\n> > 'cam_helper_imx519.cpp',\n> > 'cam_helper_imx708.cpp',\n> > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > index 27d6799a2686..dc76051fa349 100644\n> > --- a/src/libcamera/camera_sensor_properties.cpp\n> > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > @@ -111,6 +111,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > .unitCellSize = { 2900, 2900 },\n> > .testPatternModes = {},\n> > } },\n> > + { \"imx335\", {\n> > + .unitCellSize = { 2000, 2000 },\n> > + .testPatternModes = {},\n> > + } },\n> > { \"imx477\", {\n> > .unitCellSize = { 1550, 1550 },\n> > .testPatternModes = {},\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9C4A6BD16B\n\tfor ;\n\tMon, 6 Nov 2023 22:18:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F1CCD629AA;\n\tMon, 6 Nov 2023 23:18:01 +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 E6CC061DBA\n\tfor ;\n\tMon, 6 Nov 2023 23:18:00 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 593BF4A9;\n\tMon, 6 Nov 2023 23:17:40 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699309082;\n\tbh=+a2BN9tLk8tbpWwBz7+JTXStPPSqWj0TPclGX97TRCo=;\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=ZmUygyhGqz2kQ3lRkASZ0xTSnlmFLMrMCjTZLeD1WrFb7AC0UL0/H2g3DJ3z/Dn41\n\tE/4t2LTOhjd0ayr0I1x2BeZsq+EZdXggEUiBs2rd78F5XiupVWCSccYM6uOvHKPhmF\n\t90jFJ5te0FTzOHWeW1iLw1ihkqbxbuU5tl8bCxPise8N43kBDO9wCxL2to05/qH48v\n\tmWzMkZxu7+K/2rkQv/Sw+Oc87rGu4rvHiYeu8OhSEtSa2DJ3qFSylaJhJzSieNLXT2\n\t5t8PvNZAyByzJmgNpQ02JtK6+LRxiQhocdnIhJx0NADPl9xnCXGnwjXjol8airIPpc\n\taOt99vkmhXgBw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1699309060;\n\tbh=+a2BN9tLk8tbpWwBz7+JTXStPPSqWj0TPclGX97TRCo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=U+xCtCB9U0meHZ+m258L7ntPU0PS7YiZAnsESgXvAwN8r6rygV01kgBFQ2CYifJs1\n\t05g7J85MnoKxvDKmHEPwyjKW4Q53pr5mUKjLzedcnCdHWmXP4ODb6l+px8O4S7j5td\n\tN8DhtVZ3lc/LYCbVwnYn0YvGcTmOcNTKS2Qws1ss="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"U+xCtCB9\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>\n\t","To":"Naushir Patuck ","Date":"Mon, 06 Nov 2023 22:17:58 +0000","Message-ID":"<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28060,"web_url":"https://patchwork.libcamera.org/comment/28060/","msgid":"<20231107085024.GA995@pendragon.ideasonboard.com>","date":"2023-11-07T08:50:24","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Nov 06, 2023 at 10:17:58PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Naushir Patuck (2023-11-03 09:30:29)\n> > On Thu, 2 Nov 2023 at 18:09, Kieran Bingham via libcamera-devel wrote:\n> > >\n> > > Provide support for the Sony IMX335 in both libipa and RaspberryPi IPA\n> > > modules.\n> > \n> > Without an imx335.json camera tuning file present, this will not work\n> > on the RPi platform. Was that file meant to be added in this patch?\n> \n> So far I've used only an unmodified 'uncalibrated.json' for both RPi4\n> and Pi5.\n> \n> I've now successfully tested the module and updated kernel driver with\n> both 2 and 4 lane operation (2 lane on RPi4, 4 lane on Pi5).\n> \n> I haven't yet gone through a full 'tuning' process on Raspberry Pi\n> though as it's not my target platform for this camera module. But it's\n> /very/ helpful that I can test on Raspberry Pi.\n> \n> It gives good results even using uncalibrated.json on both platforms\n> too! (Which I believe is a testament to Raspberry Pi's IPA development!)\n> \n> \n> I would always like to think that 'every camera' which can be connected\n> should be supported on all platforms. But I don't know if that means to\n> get a camera into libcamera it should be tuned for each platform\n> explicitly?\n\nI don't think that would be a reasonable expectation, it just wouldn't\nscale.\n\nThere's a difference between teaching libcamera about intrinsic\nproperties of a camera sensor, which by definition are not dependent on\nthe platform it is connected to, and providing tuning data that covers\nthe combination of a camera module and an ISP. The former should simply\nbe a matter of updating a shared implementation, without a need to test\non all platforms, while the latter requires per-platform work.\n\nOne issue we have today is that there are, for historical reasons, two\nsets of helpers for camera sensors, one in libipa, and one in the\nRaspberry Pi IPA. It seems to be time to add missing information to the\nformer and drop the latter. I'm tempted to give a strong incentive in\nthat direction by refusing new additions to the Raspberry Pi-specific\ncamera helpers.\n\n> I'm weary of providing a Raspberry Pi 'tuning' file that implies I have\n> performed any full tuning on the module. What are your thoughts about\n> defaulting to use the 'uncalibrated.json' when no tuning file is\n> identified (and printing a warning to report this?)\n>\n> Otherwise I would be submitting essentially\n> 'cp uncalibrated.json imx335.json'\n> \n> And that would perhaps give potential future users undue belief in the\n> tuning file.\n\nDefaulting to uncalibrated.json is what we do on some other platforms,\nand I think it's better than pretending we have tuned a particular\ncamera module for a Pi 4 or Pi 5 board.\n\n> I'm going to go through the tuning process on a Pi now -\n> but even with that - this would only be a very 'home lab' basic effort.\n\nThat's of course useful too :-) It just shouldn't be a hard requirement.\n\n> > > Signed-off-by: Kieran Bingham \n> > > ---\n> > > src/ipa/libipa/camera_sensor_helper.cpp | 23 ++++++\n> > > src/ipa/rpi/cam_helper/cam_helper_imx335.cpp | 74 ++++++++++++++++++++\n> > > src/ipa/rpi/cam_helper/meson.build | 1 +\n> > > src/libcamera/camera_sensor_properties.cpp | 4 ++\n> > > 4 files changed, 102 insertions(+)\n> > > create mode 100644 src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > >\n> > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > index f0ecc3830115..ddab5af6eac2 100644\n> > > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > @@ -444,6 +444,29 @@ class CameraSensorHelperImx327 : public CameraSensorHelperImx290\n> > > };\n> > > REGISTER_CAMERA_SENSOR_HELPER(\"imx327\", CameraSensorHelperImx327)\n> > >\n> > > +class CameraSensorHelperImx335 : public CameraSensorHelper\n> > > +{\n> > > +public:\n> > > + uint32_t gainCode(double gain) const override;\n> > > + double gain(uint32_t gainCode) const override;\n> > > +private:\n> > > + static constexpr uint32_t maxGainCode_ = 240;\n> > > +};\n> > > +\n> > > +uint32_t CameraSensorHelperImx335::gainCode(double gain) const\n> > > +{\n> > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > +\n> > > + return std::min(code, maxGainCode_);\n> > > +}\n> > > +\n> > > +double CameraSensorHelperImx335::gain(uint32_t gainCode) const\n> > > +{\n> > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > +}\n> > > +\n> > > +REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> > > +\n> > > class CameraSensorHelperImx477 : public CameraSensorHelper\n> > > {\n> > > public:\n> > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > new file mode 100644\n> > > index 000000000000..659c69d6b6c7\n> > > --- /dev/null\n> > > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > @@ -0,0 +1,74 @@\n> > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > +/*\n> > > + * Copyright (C) 2023, Ideas on Board Oy.\n> > > + *\n> > > + * cam_helper_imx335.cpp - camera information for the Sony IMX335 sensor\n> > > + */\n> > > +\n> > > +#include \n> > > +\n> > > +#include \"cam_helper.h\"\n> > > +#include \"math.h\"\n> > > +\n> > > +using namespace RPiController;\n> > > +\n> > > +class CamHelperImx335 : public CamHelper\n> > > +{\n> > > +public:\n> > > + CamHelperImx335();\n> > > + uint32_t gainCode(double gain) const override;\n> > > + double gain(uint32_t gainCode) const override;\n> > > + void getDelays(int &exposureDelay, int &gainDelay,\n> > > + int &vblankDelay, int &hblankDelay) const override;\n> > > + unsigned int hideFramesModeSwitch() const override;\n> > > +\n> > > +private:\n> > > + /*\n> > > + * Smallest difference between the frame length and integration time,\n> > > + * in units of lines.\n> > > + */\n> > > + static constexpr int frameIntegrationDiff = 4;\n> > > + static constexpr uint32_t maxGainCode = 240;\n> > > +};\n> > > +\n> > > +/*\n> > > + * IMX335 Metadata isn't yet supported.\n> > > + */\n> > > +\n> > > +CamHelperImx335::CamHelperImx335()\n> > > + : CamHelper({}, frameIntegrationDiff)\n> > > +{\n> > > +}\n> > > +\n> > > +uint32_t CamHelperImx335::gainCode(double gain) const\n> > > +{\n> > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > + return std::min(code, maxGainCode);\n> > > +}\n> > > +\n> > > +double CamHelperImx335::gain(uint32_t gainCode) const\n> > > +{\n> > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > +}\n> > > +\n> > > +void CamHelperImx335::getDelays(int &exposureDelay, int &gainDelay,\n> > > + int &vblankDelay, int &hblankDelay) const\n> > > +{\n> > > + exposureDelay = 2;\n> > > + gainDelay = 2;\n> > > + vblankDelay = 2;\n> > > + hblankDelay = 2;\n> > > +}\n> > > +\n> > > +unsigned int CamHelperImx335::hideFramesModeSwitch() const\n> > > +{\n> > > + /* One bad frame can be expected after a mode switch. */\n> > > + return 1;\n> > > +}\n> > > +\n> > > +static CamHelper *create()\n> > > +{\n> > > + return new CamHelperImx335();\n> > > +}\n> > > +\n> > > +static RegisterCamHelper reg(\"imx335\", &create);\n> > > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build\n> > > index bdf2db8eb742..17c25cb0e4a6 100644\n> > > --- a/src/ipa/rpi/cam_helper/meson.build\n> > > +++ b/src/ipa/rpi/cam_helper/meson.build\n> > > @@ -6,6 +6,7 @@ rpi_ipa_cam_helper_sources = files([\n> > > 'cam_helper_imx219.cpp',\n> > > 'cam_helper_imx290.cpp',\n> > > 'cam_helper_imx296.cpp',\n> > > + 'cam_helper_imx335.cpp',\n> > > 'cam_helper_imx477.cpp',\n> > > 'cam_helper_imx519.cpp',\n> > > 'cam_helper_imx708.cpp',\n> > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > > index 27d6799a2686..dc76051fa349 100644\n> > > --- a/src/libcamera/camera_sensor_properties.cpp\n> > > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > > @@ -111,6 +111,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > .unitCellSize = { 2900, 2900 },\n> > > .testPatternModes = {},\n> > > } },\n> > > + { \"imx335\", {\n> > > + .unitCellSize = { 2000, 2000 },\n> > > + .testPatternModes = {},\n> > > + } },\n> > > { \"imx477\", {\n> > > .unitCellSize = { 1550, 1550 },\n> > > .testPatternModes = {},","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1BEDDBD16B\n\tfor ;\n\tTue, 7 Nov 2023 08:50:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F24E629AE;\n\tTue, 7 Nov 2023 09:50:19 +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 3AEDC6299D\n\tfor ;\n\tTue, 7 Nov 2023 09:50:17 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3090F552;\n\tTue, 7 Nov 2023 09:49:56 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699347019;\n\tbh=kA0SMUvyyEKkEbSVKrc3qM3lMvz4ZlB05am9NSi+bYg=;\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=01Q05h0oUrLbTZ+QMdPGapxb0Epu6Q8/xIEghwFiDli7iIfBruQO0gUzed25ScZ5i\n\tL27lNOo6Cj5nhxEYnWCto3qGreqwazFvuEbuuOsfGef7JKGAGlx987Y5o7iy520egC\n\tmUNSHvFgQDc403ymUceqvGYzAr+X4lh4mK9drfZ0/EXhQale82tvx7yT5v3dWs2PiM\n\t543SbHRtkF2//0F/f/SfVyv0Ws8DVg6RD0meMQqTsHvJMgIIQ4CGUA3l11JNt6UhSd\n\tTGrlfI1bcPD8Ny3JYjuTu34IUAonjPcz/StZDL4pukBJuqgO4S8rrsAwKbaUDvhCNS\n\tFZoijphF0IBdg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1699346996;\n\tbh=kA0SMUvyyEKkEbSVKrc3qM3lMvz4ZlB05am9NSi+bYg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ri4YL1uorj3/O4lCA5PoUci3srqY0P6r31hkBGV6SBb3mmPVLW7c0MmWjSDa8ba2o\n\tIxQkoAkRQGWMgzm0pAwBNkQXoOBC0rZcZ1W9IANHyYVk1V3Ijh2cs39cWbi1eUW065\n\tr2CMWo0BBijrukBh84Fsdl2aJwxGFXcuQm6O7cCw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Ri4YL1uo\"; dkim-atps=neutral","Date":"Tue, 7 Nov 2023 10:50:24 +0200","To":"Kieran Bingham ","Message-ID":"<20231107085024.GA995@pendragon.ideasonboard.com>","References":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>\n\t\n\t<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28061,"web_url":"https://patchwork.libcamera.org/comment/28061/","msgid":"","date":"2023-11-07T08:53:53","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nOn Mon, 6 Nov 2023 at 22:18, Kieran Bingham\n wrote:\n>\n> Hi Naush,\n>\n> Quoting Naushir Patuck (2023-11-03 09:30:29)\n> > Hi Kieran,\n> >\n> > Thank you for the patch.\n> >\n> > On Thu, 2 Nov 2023 at 18:09, Kieran Bingham via libcamera-devel\n> > wrote:\n> > >\n> > > Provide support for the Sony IMX335 in both libipa and RaspberryPi IPA\n> > > modules.\n> >\n> > Without an imx335.json camera tuning file present, this will not work\n> > on the RPi platform. Was that file meant to be added in this patch?\n>\n> So far I've used only an unmodified 'uncalibrated.json' for both RPi4\n> and Pi5.\n>\n> I've now successfully tested the module and updated kernel driver with\n> both 2 and 4 lane operation (2 lane on RPi4, 4 lane on Pi5).\n>\n> I haven't yet gone through a full 'tuning' process on Raspberry Pi\n> though as it's not my target platform for this camera module. But it's\n> /very/ helpful that I can test on Raspberry Pi.\n>\n> It gives good results even using uncalibrated.json on both platforms\n> too! (Which I believe is a testament to Raspberry Pi's IPA development!)\n>\n>\n> I would always like to think that 'every camera' which can be connected\n> should be supported on all platforms. But I don't know if that means to\n> get a camera into libcamera it should be tuned for each platform\n> explicitly?\n\nT\n\n>\n>\n> I'm weary of providing a Raspberry Pi 'tuning' file that implies I have\n> performed any full tuning on the module. What are your thoughts about\n> defaulting to use the 'uncalibrated.json' when no tuning file is\n> identified (and printing a warning to report this?)\n>\n> Otherwise I would be submitting essentially\n> 'cp uncalibrated.json imx335.json'\n\nI would prefer not to do this. The uncalibrated.json file is intended\nto be used for bringup and get libcamera running enough to capture DNG\nfiles for tuning purposes. IMO, I would be hesitant to add partial\n(untuned in this case) support for sensors as inevitably someone is\ngoing to run this sensor, see bad IQ and complain to Raspberry Pi\nabout it.\n\n>\n> And that would perhaps give potential future users undue belief in the\n> tuning file. I'm going to go through the tuning process on a Pi now -\n> but even with that - this would only be a very 'home lab' basic effort.\n\nOur tuning process is meant to be simple enough that you should get a\nreasonable image without all the equipment. So you ought to get a\nreasonable output from the tuning tool, then we'd be able to merge the\ntuning file for full support of the sensor, even if the tuning is not\n\"optimal\" in some respects. Again, this is my opinion, David please\nchime in if you think otherwise.\n\nRegards,\nNaush\n\n>\n> --\n> Regards\n>\n> Kieran\n>\n>\n> >\n> > Regards,\n> > Naush\n> >\n> > >\n> > > Signed-off-by: Kieran Bingham \n> > > ---\n> > > src/ipa/libipa/camera_sensor_helper.cpp | 23 ++++++\n> > > src/ipa/rpi/cam_helper/cam_helper_imx335.cpp | 74 ++++++++++++++++++++\n> > > src/ipa/rpi/cam_helper/meson.build | 1 +\n> > > src/libcamera/camera_sensor_properties.cpp | 4 ++\n> > > 4 files changed, 102 insertions(+)\n> > > create mode 100644 src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > >\n> > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > index f0ecc3830115..ddab5af6eac2 100644\n> > > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > @@ -444,6 +444,29 @@ class CameraSensorHelperImx327 : public CameraSensorHelperImx290\n> > > };\n> > > REGISTER_CAMERA_SENSOR_HELPER(\"imx327\", CameraSensorHelperImx327)\n> > >\n> > > +class CameraSensorHelperImx335 : public CameraSensorHelper\n> > > +{\n> > > +public:\n> > > + uint32_t gainCode(double gain) const override;\n> > > + double gain(uint32_t gainCode) const override;\n> > > +private:\n> > > + static constexpr uint32_t maxGainCode_ = 240;\n> > > +};\n> > > +\n> > > +uint32_t CameraSensorHelperImx335::gainCode(double gain) const\n> > > +{\n> > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > +\n> > > + return std::min(code, maxGainCode_);\n> > > +}\n> > > +\n> > > +double CameraSensorHelperImx335::gain(uint32_t gainCode) const\n> > > +{\n> > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > +}\n> > > +\n> > > +REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> > > +\n> > > class CameraSensorHelperImx477 : public CameraSensorHelper\n> > > {\n> > > public:\n> > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > new file mode 100644\n> > > index 000000000000..659c69d6b6c7\n> > > --- /dev/null\n> > > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > @@ -0,0 +1,74 @@\n> > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > +/*\n> > > + * Copyright (C) 2023, Ideas on Board Oy.\n> > > + *\n> > > + * cam_helper_imx335.cpp - camera information for the Sony IMX335 sensor\n> > > + */\n> > > +\n> > > +#include \n> > > +\n> > > +#include \"cam_helper.h\"\n> > > +#include \"math.h\"\n> > > +\n> > > +using namespace RPiController;\n> > > +\n> > > +class CamHelperImx335 : public CamHelper\n> > > +{\n> > > +public:\n> > > + CamHelperImx335();\n> > > + uint32_t gainCode(double gain) const override;\n> > > + double gain(uint32_t gainCode) const override;\n> > > + void getDelays(int &exposureDelay, int &gainDelay,\n> > > + int &vblankDelay, int &hblankDelay) const override;\n> > > + unsigned int hideFramesModeSwitch() const override;\n> > > +\n> > > +private:\n> > > + /*\n> > > + * Smallest difference between the frame length and integration time,\n> > > + * in units of lines.\n> > > + */\n> > > + static constexpr int frameIntegrationDiff = 4;\n> > > + static constexpr uint32_t maxGainCode = 240;\n> > > +};\n> > > +\n> > > +/*\n> > > + * IMX335 Metadata isn't yet supported.\n> > > + */\n> > > +\n> > > +CamHelperImx335::CamHelperImx335()\n> > > + : CamHelper({}, frameIntegrationDiff)\n> > > +{\n> > > +}\n> > > +\n> > > +uint32_t CamHelperImx335::gainCode(double gain) const\n> > > +{\n> > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > + return std::min(code, maxGainCode);\n> > > +}\n> > > +\n> > > +double CamHelperImx335::gain(uint32_t gainCode) const\n> > > +{\n> > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > +}\n> > > +\n> > > +void CamHelperImx335::getDelays(int &exposureDelay, int &gainDelay,\n> > > + int &vblankDelay, int &hblankDelay) const\n> > > +{\n> > > + exposureDelay = 2;\n> > > + gainDelay = 2;\n> > > + vblankDelay = 2;\n> > > + hblankDelay = 2;\n> > > +}\n> > > +\n> > > +unsigned int CamHelperImx335::hideFramesModeSwitch() const\n> > > +{\n> > > + /* One bad frame can be expected after a mode switch. */\n> > > + return 1;\n> > > +}\n> > > +\n> > > +static CamHelper *create()\n> > > +{\n> > > + return new CamHelperImx335();\n> > > +}\n> > > +\n> > > +static RegisterCamHelper reg(\"imx335\", &create);\n> > > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build\n> > > index bdf2db8eb742..17c25cb0e4a6 100644\n> > > --- a/src/ipa/rpi/cam_helper/meson.build\n> > > +++ b/src/ipa/rpi/cam_helper/meson.build\n> > > @@ -6,6 +6,7 @@ rpi_ipa_cam_helper_sources = files([\n> > > 'cam_helper_imx219.cpp',\n> > > 'cam_helper_imx290.cpp',\n> > > 'cam_helper_imx296.cpp',\n> > > + 'cam_helper_imx335.cpp',\n> > > 'cam_helper_imx477.cpp',\n> > > 'cam_helper_imx519.cpp',\n> > > 'cam_helper_imx708.cpp',\n> > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > > index 27d6799a2686..dc76051fa349 100644\n> > > --- a/src/libcamera/camera_sensor_properties.cpp\n> > > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > > @@ -111,6 +111,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > .unitCellSize = { 2900, 2900 },\n> > > .testPatternModes = {},\n> > > } },\n> > > + { \"imx335\", {\n> > > + .unitCellSize = { 2000, 2000 },\n> > > + .testPatternModes = {},\n> > > + } },\n> > > { \"imx477\", {\n> > > .unitCellSize = { 1550, 1550 },\n> > > .testPatternModes = {},\n> > > --\n> > > 2.34.1\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 027D9BDE6B\n\tfor ;\n\tTue, 7 Nov 2023 08:54:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 48081629AB;\n\tTue, 7 Nov 2023 09:54:26 +0100 (CET)","from mail-yw1-x112a.google.com (mail-yw1-x112a.google.com\n\t[IPv6:2607:f8b0:4864:20::112a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4786D6299D\n\tfor ;\n\tTue, 7 Nov 2023 09:54:24 +0100 (CET)","by mail-yw1-x112a.google.com with SMTP id\n\t00721157ae682-5a7eef0b931so63717177b3.0\n\tfor ;\n\tTue, 07 Nov 2023 00:54:24 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699347266;\n\tbh=gm+swMckS3V6W2dzm8+rC7WyCMaxhQdFiIsgS62Y1YQ=;\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=fiAVO3SMNtEtQFxhleZgC/tX+E/GL40F/dPoF6kZS0+geoeSMBrWBkzFoQ2i3QG8B\n\tG/9NTk5TX8sZRlAh+Ayk7YBGaAo/FQHyzFr54j84MUIdqBAfkU2MsXop+WSp3/stLf\n\t0fFAU/5Ec2EN7KTttfK0DUI6L9Vt434UzUKxIkKKtR+wao4eUdlZHGT/+9jsBvSDx/\n\tDuNE2FtlBjTpV3R6Pdv6DAY3Lc0U+6OVT27MWA5VRpRb0Vxkr26Aqy4hFBeEwqbkqn\n\tsraOdqUM1cwSZ80ttQ8YmKStsw6Sf2e+OkGBGf0Ks7Z9VD8MsYun8IMhB8jmL81ipm\n\tZSVZcuRg3xl7g==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1699347263; x=1699952063;\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=vgVWJv86ZbqTP+qcx2zJ7rYIi5XTvQNq2wabbPrkgHE=;\n\tb=GDLGOtQY/JbyJ+qa7Ldc46/sPJFaKb0mkGHSbxo2DzHijZiS5e1u9d9TWI4ZhG+Fdd\n\tmqNqjdG/6XggPcUHIe9xMRrqrNvfhlWo3e2QMSYRzsvLGA97ZOSZ+zZNkTXiLxISZUgZ\n\tvW7mzWDJLROlEhl565BGYcy5sykKkFGvtSdbcfwaCBqk+UZ7oARMALBDrefskic9Jdxs\n\tuxgflbWDnLhiIdo7HtMlrXQtg+RND4UCUcpI5arlnyXwOsDnEJXBvfAOPcHLrBvNn7nr\n\tSWfjySQ6VLEtPq9wJ8p8OFnCCIRvMfiaie6w9Ua4/rx+4RG9dZblxR/eGMN0Mxd+56Z0\n\t2sKg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"GDLGOtQY\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1699347263; x=1699952063;\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=vgVWJv86ZbqTP+qcx2zJ7rYIi5XTvQNq2wabbPrkgHE=;\n\tb=j9NiGrG7ZkDmmqx8wTi5gjAEzjuyrL+trsDIK55wikkXEvP1x2+TVYxuT3kirwxUUx\n\tscTzc+wKTzly07aU/LVFyOU2slxD4UHuDyHKDkLTDa3wn/8ncI6rtVy6e1QzeFuQ4dzM\n\trBhKIDeJi/Fg5jtHJ9jMtRCRTqsdO8/451dmYiJ71baUydYdMWjYZNh1Eh+tBfUZXx0Q\n\t5MVpFGyZwDDhA9ekrzV1+xB1E8SXvhcYSfHFEn2xUAjhws+VLs84fg9QpaWamFp3OUlo\n\tqssX5cVK2RDkPeZMPeORv2wdxxzTytMwpeY7KpfHJwrCbtMjtbIIEZnDz7ZIgBFXB+U4\n\tGYog==","X-Gm-Message-State":"AOJu0Yw65vJbayHigeJZyg4m8oWosmjuhODi54sPbxGAgvgVlTJSSmEC\n\tGY14dpc3Qjmb9lHPumQoITTvyUYmd544Z77DWJbfv6CH/275NR5hdT64gA==","X-Google-Smtp-Source":"AGHT+IFXcJreAT48nFhwzceZw8fmZR+TIp9Q6hdlihzRo+0GHeoeR7Sl3krhK/fAE/gjU0h7AkdyaV+uFM2vPYR1Yps=","X-Received":"by 2002:a25:d186:0:b0:d9a:3bf1:35e9 with SMTP id\n\ti128-20020a25d186000000b00d9a3bf135e9mr30929702ybg.3.1699347262942;\n\tTue, 07 Nov 2023 00:54:22 -0800 (PST)","MIME-Version":"1.0","References":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>\n\t\n\t<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>","In-Reply-To":"<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>","Date":"Tue, 7 Nov 2023 08:53:53 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28064,"web_url":"https://patchwork.libcamera.org/comment/28064/","msgid":"<20231109103018.GA26402@pendragon.ideasonboard.com>","date":"2023-11-09T10:30:18","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Nov 07, 2023 at 10:50:24AM +0200, Laurent Pinchart via libcamera-devel wrote:\n> On Mon, Nov 06, 2023 at 10:17:58PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > Quoting Naushir Patuck (2023-11-03 09:30:29)\n> > > On Thu, 2 Nov 2023 at 18:09, Kieran Bingham via libcamera-devel wrote:\n> > > >\n> > > > Provide support for the Sony IMX335 in both libipa and RaspberryPi IPA\n> > > > modules.\n> > > \n> > > Without an imx335.json camera tuning file present, this will not work\n> > > on the RPi platform. Was that file meant to be added in this patch?\n> > \n> > So far I've used only an unmodified 'uncalibrated.json' for both RPi4\n> > and Pi5.\n> > \n> > I've now successfully tested the module and updated kernel driver with\n> > both 2 and 4 lane operation (2 lane on RPi4, 4 lane on Pi5).\n> > \n> > I haven't yet gone through a full 'tuning' process on Raspberry Pi\n> > though as it's not my target platform for this camera module. But it's\n> > /very/ helpful that I can test on Raspberry Pi.\n> > \n> > It gives good results even using uncalibrated.json on both platforms\n> > too! (Which I believe is a testament to Raspberry Pi's IPA development!)\n> > \n> > \n> > I would always like to think that 'every camera' which can be connected\n> > should be supported on all platforms. But I don't know if that means to\n> > get a camera into libcamera it should be tuned for each platform\n> > explicitly?\n> \n> I don't think that would be a reasonable expectation, it just wouldn't\n> scale.\n> \n> There's a difference between teaching libcamera about intrinsic\n> properties of a camera sensor, which by definition are not dependent on\n> the platform it is connected to, and providing tuning data that covers\n> the combination of a camera module and an ISP. The former should simply\n> be a matter of updating a shared implementation, without a need to test\n> on all platforms, while the latter requires per-platform work.\n> \n> One issue we have today is that there are, for historical reasons, two\n> sets of helpers for camera sensors, one in libipa, and one in the\n> Raspberry Pi IPA. It seems to be time to add missing information to the\n> former and drop the latter. I'm tempted to give a strong incentive in\n> that direction by refusing new additions to the Raspberry Pi-specific\n> camera helpers.\n\nRe-reading this, I realize the wording is more aggressive than I\nintended it to be. To clarify my point, I don't call for refusing new\nadditions right now, blocking this patch. I do believe that it's time to\nmerge the two sets of helpers in one, as if we have an agreement on\nthat, and can agree on a concrete plan, then I'll be happy.\n\n> > I'm weary of providing a Raspberry Pi 'tuning' file that implies I have\n> > performed any full tuning on the module. What are your thoughts about\n> > defaulting to use the 'uncalibrated.json' when no tuning file is\n> > identified (and printing a warning to report this?)\n> >\n> > Otherwise I would be submitting essentially\n> > 'cp uncalibrated.json imx335.json'\n> > \n> > And that would perhaps give potential future users undue belief in the\n> > tuning file.\n> \n> Defaulting to uncalibrated.json is what we do on some other platforms,\n> and I think it's better than pretending we have tuned a particular\n> camera module for a Pi 4 or Pi 5 board.\n> \n> > I'm going to go through the tuning process on a Pi now -\n> > but even with that - this would only be a very 'home lab' basic effort.\n> \n> That's of course useful too :-) It just shouldn't be a hard requirement.\n> \n> > > > Signed-off-by: Kieran Bingham \n> > > > ---\n> > > > src/ipa/libipa/camera_sensor_helper.cpp | 23 ++++++\n> > > > src/ipa/rpi/cam_helper/cam_helper_imx335.cpp | 74 ++++++++++++++++++++\n> > > > src/ipa/rpi/cam_helper/meson.build | 1 +\n> > > > src/libcamera/camera_sensor_properties.cpp | 4 ++\n> > > > 4 files changed, 102 insertions(+)\n> > > > create mode 100644 src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > >\n> > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > index f0ecc3830115..ddab5af6eac2 100644\n> > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > @@ -444,6 +444,29 @@ class CameraSensorHelperImx327 : public CameraSensorHelperImx290\n> > > > };\n> > > > REGISTER_CAMERA_SENSOR_HELPER(\"imx327\", CameraSensorHelperImx327)\n> > > >\n> > > > +class CameraSensorHelperImx335 : public CameraSensorHelper\n> > > > +{\n> > > > +public:\n> > > > + uint32_t gainCode(double gain) const override;\n> > > > + double gain(uint32_t gainCode) const override;\n> > > > +private:\n> > > > + static constexpr uint32_t maxGainCode_ = 240;\n> > > > +};\n> > > > +\n> > > > +uint32_t CameraSensorHelperImx335::gainCode(double gain) const\n> > > > +{\n> > > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > > +\n> > > > + return std::min(code, maxGainCode_);\n> > > > +}\n> > > > +\n> > > > +double CameraSensorHelperImx335::gain(uint32_t gainCode) const\n> > > > +{\n> > > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > > +}\n> > > > +\n> > > > +REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> > > > +\n> > > > class CameraSensorHelperImx477 : public CameraSensorHelper\n> > > > {\n> > > > public:\n> > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > > new file mode 100644\n> > > > index 000000000000..659c69d6b6c7\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > > @@ -0,0 +1,74 @@\n> > > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > > +/*\n> > > > + * Copyright (C) 2023, Ideas on Board Oy.\n> > > > + *\n> > > > + * cam_helper_imx335.cpp - camera information for the Sony IMX335 sensor\n> > > > + */\n> > > > +\n> > > > +#include \n> > > > +\n> > > > +#include \"cam_helper.h\"\n> > > > +#include \"math.h\"\n> > > > +\n> > > > +using namespace RPiController;\n> > > > +\n> > > > +class CamHelperImx335 : public CamHelper\n> > > > +{\n> > > > +public:\n> > > > + CamHelperImx335();\n> > > > + uint32_t gainCode(double gain) const override;\n> > > > + double gain(uint32_t gainCode) const override;\n> > > > + void getDelays(int &exposureDelay, int &gainDelay,\n> > > > + int &vblankDelay, int &hblankDelay) const override;\n> > > > + unsigned int hideFramesModeSwitch() const override;\n> > > > +\n> > > > +private:\n> > > > + /*\n> > > > + * Smallest difference between the frame length and integration time,\n> > > > + * in units of lines.\n> > > > + */\n> > > > + static constexpr int frameIntegrationDiff = 4;\n> > > > + static constexpr uint32_t maxGainCode = 240;\n> > > > +};\n> > > > +\n> > > > +/*\n> > > > + * IMX335 Metadata isn't yet supported.\n> > > > + */\n> > > > +\n> > > > +CamHelperImx335::CamHelperImx335()\n> > > > + : CamHelper({}, frameIntegrationDiff)\n> > > > +{\n> > > > +}\n> > > > +\n> > > > +uint32_t CamHelperImx335::gainCode(double gain) const\n> > > > +{\n> > > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > > + return std::min(code, maxGainCode);\n> > > > +}\n> > > > +\n> > > > +double CamHelperImx335::gain(uint32_t gainCode) const\n> > > > +{\n> > > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > > +}\n> > > > +\n> > > > +void CamHelperImx335::getDelays(int &exposureDelay, int &gainDelay,\n> > > > + int &vblankDelay, int &hblankDelay) const\n> > > > +{\n> > > > + exposureDelay = 2;\n> > > > + gainDelay = 2;\n> > > > + vblankDelay = 2;\n> > > > + hblankDelay = 2;\n> > > > +}\n> > > > +\n> > > > +unsigned int CamHelperImx335::hideFramesModeSwitch() const\n> > > > +{\n> > > > + /* One bad frame can be expected after a mode switch. */\n> > > > + return 1;\n> > > > +}\n> > > > +\n> > > > +static CamHelper *create()\n> > > > +{\n> > > > + return new CamHelperImx335();\n> > > > +}\n> > > > +\n> > > > +static RegisterCamHelper reg(\"imx335\", &create);\n> > > > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build\n> > > > index bdf2db8eb742..17c25cb0e4a6 100644\n> > > > --- a/src/ipa/rpi/cam_helper/meson.build\n> > > > +++ b/src/ipa/rpi/cam_helper/meson.build\n> > > > @@ -6,6 +6,7 @@ rpi_ipa_cam_helper_sources = files([\n> > > > 'cam_helper_imx219.cpp',\n> > > > 'cam_helper_imx290.cpp',\n> > > > 'cam_helper_imx296.cpp',\n> > > > + 'cam_helper_imx335.cpp',\n> > > > 'cam_helper_imx477.cpp',\n> > > > 'cam_helper_imx519.cpp',\n> > > > 'cam_helper_imx708.cpp',\n> > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > > > index 27d6799a2686..dc76051fa349 100644\n> > > > --- a/src/libcamera/camera_sensor_properties.cpp\n> > > > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > > > @@ -111,6 +111,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > > .unitCellSize = { 2900, 2900 },\n> > > > .testPatternModes = {},\n> > > > } },\n> > > > + { \"imx335\", {\n> > > > + .unitCellSize = { 2000, 2000 },\n> > > > + .testPatternModes = {},\n> > > > + } },\n> > > > { \"imx477\", {\n> > > > .unitCellSize = { 1550, 1550 },\n> > > > .testPatternModes = {},","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AF23CC3284\n\tfor ;\n\tThu, 9 Nov 2023 10:30:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 057B8629BB;\n\tThu, 9 Nov 2023 11:30:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 667E561DB8\n\tfor ;\n\tThu, 9 Nov 2023 11:30:12 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D32246EF;\n\tThu, 9 Nov 2023 11:29:49 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699525815;\n\tbh=F5oHTivgmBrfoyGmfXOqBzCYt4KZQTTVebBgOF9wNjw=;\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:\n\tFrom;\n\tb=1ei05tQwLGe3/1Lll2fF6yH39TZXuCwasvZI4299BUM9FEXg5Ebp0sOwW/GWKzjVe\n\tZyjNwXtyi1QMJYB1ZOEU4cc8ptapKy/0XLa5LxdV3ciDAwCy3YOhnxo9n+oICJFJ75\n\tw6I9jKFcWb7yaGRlfgnHgGmtosFnW78XYaS17IywlNwslohZBIgqO5b2Qe+Fz61cWS\n\tEdrroue4R6zVidkpiKMBIPCobD5OtKzS/zPoZBSR+NwtuwOtsS1tZds7gXIbtYJxa7\n\tDXIRrluOIuX4wROVeQz5efP8vOs0Y7DVOUR6I6SzXZTrfmyZfPDBTDZkQq2jXUkuWi\n\tCryVQrcK+01pA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1699525790;\n\tbh=F5oHTivgmBrfoyGmfXOqBzCYt4KZQTTVebBgOF9wNjw=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=TWK6uV+fwgdxauV0GmkM0dX+pxO4eTLkpqFSC8gu7vtljSqfQ0MKW2YF/Da79REU4\n\tIEZ2j12YzFYsYMInL0OKocYKerX/F8sgSyWGFkVz2SgFK2CuEBajr4/MN86mUZPfWX\n\tsP/3VexfpLCnrkUHouElfM+tXcgcGzARn0YD2sh8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TWK6uV+f\"; dkim-atps=neutral","Date":"Thu, 9 Nov 2023 12:30:18 +0200","To":"Kieran Bingham ,\n\tlibcamera devel ","Message-ID":"<20231109103018.GA26402@pendragon.ideasonboard.com>","References":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>\n\t\n\t<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>\n\t<20231107085024.GA995@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231107085024.GA995@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28065,"web_url":"https://patchwork.libcamera.org/comment/28065/","msgid":"","date":"2023-11-09T11:13:46","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 9 Nov 2023 at 10:30, Laurent Pinchart via libcamera-devel\n wrote:\n>\n> On Tue, Nov 07, 2023 at 10:50:24AM +0200, Laurent Pinchart via libcamera-devel wrote:\n> > On Mon, Nov 06, 2023 at 10:17:58PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > > Quoting Naushir Patuck (2023-11-03 09:30:29)\n> > > > On Thu, 2 Nov 2023 at 18:09, Kieran Bingham via libcamera-devel wrote:\n> > > > >\n> > > > > Provide support for the Sony IMX335 in both libipa and RaspberryPi IPA\n> > > > > modules.\n> > > >\n> > > > Without an imx335.json camera tuning file present, this will not work\n> > > > on the RPi platform. Was that file meant to be added in this patch?\n> > >\n> > > So far I've used only an unmodified 'uncalibrated.json' for both RPi4\n> > > and Pi5.\n> > >\n> > > I've now successfully tested the module and updated kernel driver with\n> > > both 2 and 4 lane operation (2 lane on RPi4, 4 lane on Pi5).\n> > >\n> > > I haven't yet gone through a full 'tuning' process on Raspberry Pi\n> > > though as it's not my target platform for this camera module. But it's\n> > > /very/ helpful that I can test on Raspberry Pi.\n> > >\n> > > It gives good results even using uncalibrated.json on both platforms\n> > > too! (Which I believe is a testament to Raspberry Pi's IPA development!)\n> > >\n> > >\n> > > I would always like to think that 'every camera' which can be connected\n> > > should be supported on all platforms. But I don't know if that means to\n> > > get a camera into libcamera it should be tuned for each platform\n> > > explicitly?\n> >\n> > I don't think that would be a reasonable expectation, it just wouldn't\n> > scale.\n> >\n> > There's a difference between teaching libcamera about intrinsic\n> > properties of a camera sensor, which by definition are not dependent on\n> > the platform it is connected to, and providing tuning data that covers\n> > the combination of a camera module and an ISP. The former should simply\n> > be a matter of updating a shared implementation, without a need to test\n> > on all platforms, while the latter requires per-platform work.\n> >\n> > One issue we have today is that there are, for historical reasons, two\n> > sets of helpers for camera sensors, one in libipa, and one in the\n> > Raspberry Pi IPA. It seems to be time to add missing information to the\n> > former and drop the latter. I'm tempted to give a strong incentive in\n> > that direction by refusing new additions to the Raspberry Pi-specific\n> > camera helpers.\n>\n> Re-reading this, I realize the wording is more aggressive than I\n> intended it to be. To clarify my point, I don't call for refusing new\n> additions right now, blocking this patch. I do believe that it's time to\n> merge the two sets of helpers in one, as if we have an agreement on\n> that, and can agree on a concrete plan, then I'll be happy.\n\nThank you for clarifying this!\n\nAgree, it would be nice to consolidate these 2 helpers - hopefully we\ncan find a reasonably painless path forward to doing this. They key\ntrouble I see is trying to keep our existing structures in-place since\nwe use them through all our IPA and algorithms source code.\n\nRegards,\nNaush\n\n\n\n>\n> > > I'm weary of providing a Raspberry Pi 'tuning' file that implies I have\n> > > performed any full tuning on the module. What are your thoughts about\n> > > defaulting to use the 'uncalibrated.json' when no tuning file is\n> > > identified (and printing a warning to report this?)\n> > >\n> > > Otherwise I would be submitting essentially\n> > > 'cp uncalibrated.json imx335.json'\n> > >\n> > > And that would perhaps give potential future users undue belief in the\n> > > tuning file.\n> >\n> > Defaulting to uncalibrated.json is what we do on some other platforms,\n> > and I think it's better than pretending we have tuned a particular\n> > camera module for a Pi 4 or Pi 5 board.\n> >\n> > > I'm going to go through the tuning process on a Pi now -\n> > > but even with that - this would only be a very 'home lab' basic effort.\n> >\n> > That's of course useful too :-) It just shouldn't be a hard requirement.\n> >\n> > > > > Signed-off-by: Kieran Bingham \n> > > > > ---\n> > > > > src/ipa/libipa/camera_sensor_helper.cpp | 23 ++++++\n> > > > > src/ipa/rpi/cam_helper/cam_helper_imx335.cpp | 74 ++++++++++++++++++++\n> > > > > src/ipa/rpi/cam_helper/meson.build | 1 +\n> > > > > src/libcamera/camera_sensor_properties.cpp | 4 ++\n> > > > > 4 files changed, 102 insertions(+)\n> > > > > create mode 100644 src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > > >\n> > > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > > index f0ecc3830115..ddab5af6eac2 100644\n> > > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > > > > @@ -444,6 +444,29 @@ class CameraSensorHelperImx327 : public CameraSensorHelperImx290\n> > > > > };\n> > > > > REGISTER_CAMERA_SENSOR_HELPER(\"imx327\", CameraSensorHelperImx327)\n> > > > >\n> > > > > +class CameraSensorHelperImx335 : public CameraSensorHelper\n> > > > > +{\n> > > > > +public:\n> > > > > + uint32_t gainCode(double gain) const override;\n> > > > > + double gain(uint32_t gainCode) const override;\n> > > > > +private:\n> > > > > + static constexpr uint32_t maxGainCode_ = 240;\n> > > > > +};\n> > > > > +\n> > > > > +uint32_t CameraSensorHelperImx335::gainCode(double gain) const\n> > > > > +{\n> > > > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > > > +\n> > > > > + return std::min(code, maxGainCode_);\n> > > > > +}\n> > > > > +\n> > > > > +double CameraSensorHelperImx335::gain(uint32_t gainCode) const\n> > > > > +{\n> > > > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > > > +}\n> > > > > +\n> > > > > +REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> > > > > +\n> > > > > class CameraSensorHelperImx477 : public CameraSensorHelper\n> > > > > {\n> > > > > public:\n> > > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > > > new file mode 100644\n> > > > > index 000000000000..659c69d6b6c7\n> > > > > --- /dev/null\n> > > > > +++ b/src/ipa/rpi/cam_helper/cam_helper_imx335.cpp\n> > > > > @@ -0,0 +1,74 @@\n> > > > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > > > +/*\n> > > > > + * Copyright (C) 2023, Ideas on Board Oy.\n> > > > > + *\n> > > > > + * cam_helper_imx335.cpp - camera information for the Sony IMX335 sensor\n> > > > > + */\n> > > > > +\n> > > > > +#include \n> > > > > +\n> > > > > +#include \"cam_helper.h\"\n> > > > > +#include \"math.h\"\n> > > > > +\n> > > > > +using namespace RPiController;\n> > > > > +\n> > > > > +class CamHelperImx335 : public CamHelper\n> > > > > +{\n> > > > > +public:\n> > > > > + CamHelperImx335();\n> > > > > + uint32_t gainCode(double gain) const override;\n> > > > > + double gain(uint32_t gainCode) const override;\n> > > > > + void getDelays(int &exposureDelay, int &gainDelay,\n> > > > > + int &vblankDelay, int &hblankDelay) const override;\n> > > > > + unsigned int hideFramesModeSwitch() const override;\n> > > > > +\n> > > > > +private:\n> > > > > + /*\n> > > > > + * Smallest difference between the frame length and integration time,\n> > > > > + * in units of lines.\n> > > > > + */\n> > > > > + static constexpr int frameIntegrationDiff = 4;\n> > > > > + static constexpr uint32_t maxGainCode = 240;\n> > > > > +};\n> > > > > +\n> > > > > +/*\n> > > > > + * IMX335 Metadata isn't yet supported.\n> > > > > + */\n> > > > > +\n> > > > > +CamHelperImx335::CamHelperImx335()\n> > > > > + : CamHelper({}, frameIntegrationDiff)\n> > > > > +{\n> > > > > +}\n> > > > > +\n> > > > > +uint32_t CamHelperImx335::gainCode(double gain) const\n> > > > > +{\n> > > > > + uint32_t code = 10 * std::log10(gain) * 10 / 3;\n> > > > > + return std::min(code, maxGainCode);\n> > > > > +}\n> > > > > +\n> > > > > +double CamHelperImx335::gain(uint32_t gainCode) const\n> > > > > +{\n> > > > > + return std::pow(10.0, gainCode / (10 * 10 / 3));\n> > > > > +}\n> > > > > +\n> > > > > +void CamHelperImx335::getDelays(int &exposureDelay, int &gainDelay,\n> > > > > + int &vblankDelay, int &hblankDelay) const\n> > > > > +{\n> > > > > + exposureDelay = 2;\n> > > > > + gainDelay = 2;\n> > > > > + vblankDelay = 2;\n> > > > > + hblankDelay = 2;\n> > > > > +}\n> > > > > +\n> > > > > +unsigned int CamHelperImx335::hideFramesModeSwitch() const\n> > > > > +{\n> > > > > + /* One bad frame can be expected after a mode switch. */\n> > > > > + return 1;\n> > > > > +}\n> > > > > +\n> > > > > +static CamHelper *create()\n> > > > > +{\n> > > > > + return new CamHelperImx335();\n> > > > > +}\n> > > > > +\n> > > > > +static RegisterCamHelper reg(\"imx335\", &create);\n> > > > > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build\n> > > > > index bdf2db8eb742..17c25cb0e4a6 100644\n> > > > > --- a/src/ipa/rpi/cam_helper/meson.build\n> > > > > +++ b/src/ipa/rpi/cam_helper/meson.build\n> > > > > @@ -6,6 +6,7 @@ rpi_ipa_cam_helper_sources = files([\n> > > > > 'cam_helper_imx219.cpp',\n> > > > > 'cam_helper_imx290.cpp',\n> > > > > 'cam_helper_imx296.cpp',\n> > > > > + 'cam_helper_imx335.cpp',\n> > > > > 'cam_helper_imx477.cpp',\n> > > > > 'cam_helper_imx519.cpp',\n> > > > > 'cam_helper_imx708.cpp',\n> > > > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> > > > > index 27d6799a2686..dc76051fa349 100644\n> > > > > --- a/src/libcamera/camera_sensor_properties.cpp\n> > > > > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > > > > @@ -111,6 +111,10 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > > > .unitCellSize = { 2900, 2900 },\n> > > > > .testPatternModes = {},\n> > > > > } },\n> > > > > + { \"imx335\", {\n> > > > > + .unitCellSize = { 2000, 2000 },\n> > > > > + .testPatternModes = {},\n> > > > > + } },\n> > > > > { \"imx477\", {\n> > > > > .unitCellSize = { 1550, 1550 },\n> > > > > .testPatternModes = {},\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 542DDBDE6B\n\tfor ;\n\tThu, 9 Nov 2023 11:14:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BDBC1629AF;\n\tThu, 9 Nov 2023 12:14:24 +0100 (CET)","from mail-yb1-xb36.google.com (mail-yb1-xb36.google.com\n\t[IPv6:2607:f8b0:4864:20::b36])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F3C6F629AD\n\tfor ;\n\tThu, 9 Nov 2023 12:14:22 +0100 (CET)","by mail-yb1-xb36.google.com with SMTP id\n\t3f1490d57ef6-dae7cc31151so758375276.3\n\tfor ;\n\tThu, 09 Nov 2023 03:14:22 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699528464;\n\tbh=t0+LNHYV5dmot6quBI9EB9QGboszEFVqcq7tK/YTBHI=;\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=YMoPb2Cyeo4xuySfm0OHLH+PkPh6F2r0ALOUaH2ph91eVlwjozMLAMjkkZiO+jWeG\n\tShrNYDjo75wD0+xnkv/5ORDwY+1jsm3cxMGECFKTl+84ssaEN3rCO3Mnvc3oHjtm5k\n\tew8K/dhrKfmtMfhzbrJbVAFe6+Do3JAOj0MBCDeQtvoACXS/cTxvnyI2fGGlCQSm5m\n\tWUsx5ltDSO1bqmv7/8UJX4sbhU20xaQBdxi0aARUM/LPFBV/sqz5F5zFByRRZfOl6x\n\tJbZeyZGSNUocIbY05QtCcFTvrgfQM0ZOaDj9uIdQ1VK5rtfnPHH+Is2MsuXXzmxkGf\n\tXWNJ8rz1NbE/A==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1699528462; x=1700133262;\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=7RRMSZmJeboDwrNBgY8jWaBQrzosCHCpp1kdPMJW0to=;\n\tb=iTv0A9mTBbFLrBYbQhAlO6wkBvcm6mtE8Ki0O6gj3SmUnvRdLb/87lk/U+eCa4S8Ub\n\tLu7qm4Mqug4F9Y6We27xaotcbew8SN2M7Gvpu3/2alhpJy4T9ByVcXkN0xgYWBoK8w+B\n\tKRPsj3VqUfv76eVqPYp1bwfP5zzlqeoNa9iSz1X0c6uhCETT+PukmhVaCSasutOx4hdl\n\tYKQYpDrhywi+QApuzGuTOYbUCbr7Rf8dXhEYEeYhNHbkXBJQkouHjeyoRnBGitOjFipi\n\tjt/UKwswq4zfn3c3pVNZ0sT9xRIlTRBokYO67+F/k+NTHlDvB6v4JRnCnTWT2E7w9LKj\n\tYRlw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"iTv0A9mT\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1699528462; x=1700133262;\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=7RRMSZmJeboDwrNBgY8jWaBQrzosCHCpp1kdPMJW0to=;\n\tb=MeFwJePJ9UQ+SYo0K3Z5+P4VaeJCHvl9jtQ7DBVTWGZB/q7kjgmj1P+biEdc2qwkr0\n\tQ8yhUL2JK13D+lsAWu6TwTmVbNXMjiXUrlYNCpEPnG/nM1mNIOrTcZGRyMqSzkcaHbUX\n\tJRSQj6TBMTunesqhKdnlcOZdSV5hlP7i0FFnz4JVytcp4DblbZKmXgjAnbrGm0BFoP16\n\tOLEMuQtzBC4o2ViTFSaXgZ95dLphORl8f4C0zVaBTXwTsraPVBFD4R8bNWVLVJJPqJQx\n\tq0kXyl2i1x6tyNVv1Aeb6mXt5JfL3WM5Vy7BLjpcP2Pg6AUvST6y25VQlS2B+8ctYP0j\n\tu75Q==","X-Gm-Message-State":"AOJu0YzjYSv6Jq8SPG6PxnecC44Ef5mgKaKzvL1gprbSWaEweTldOTdH\n\t4c2XDNrgQ4EqZXPn8xq8EKjitKEriJ2j4i+XUDCn6DNVZik0jvwH178=","X-Google-Smtp-Source":"AGHT+IG6lEVWmtW0foZgOgvpXoUh8rTNQS3BeSgQJ/+Fnwn+cuB0FGNlMP/p9rSe5LXiwSydMqQn7qkwd0EV1nAkXmE=","X-Received":"by 2002:a25:2653:0:b0:da0:c7f9:9fb6 with SMTP id\n\tm80-20020a252653000000b00da0c7f99fb6mr4753851ybm.62.1699528461653;\n\tThu, 09 Nov 2023 03:14:21 -0800 (PST)","MIME-Version":"1.0","References":"<20231102180916.3575006-1-kieran.bingham@ideasonboard.com>\n\t\n\t<169930907801.1817078.5037182619798984675@ping.linuxembedded.co.uk>\n\t<20231107085024.GA995@pendragon.ideasonboard.com>\n\t<20231109103018.GA26402@pendragon.ideasonboard.com>","In-Reply-To":"<20231109103018.GA26402@pendragon.ideasonboard.com>","Date":"Thu, 9 Nov 2023 11:13:46 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipa: Add IMX335 support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]