[{"id":20601,"web_url":"https://patchwork.libcamera.org/comment/20601/","msgid":"<CAHW6GYK2v1Lzq3LHTgbkVCSF3y-pjwRzVyOkaktDUnqv981YTw@mail.gmail.com>","date":"2021-10-28T10:56:48","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: ipu3: Extend ipu3 ipa\n\tinterface for sensor and lens controls","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi\n\nI'm very pleased to see some other teams start to think about autofocus!\n\nOne thing we need to do is decide what the libcamera API for autofocus\nlooks like, otherwise there is of course no way to drive it! I tried to\nstart this discussion a little while ago (check for emails from me to this\nmailing list on Thursday 21 October), so I'd be delighted to get some\nfeedback on this subject and hear other people's ideas.\n\nThanks!\n\nDavid\n\nOn Thu, 28 Oct 2021 at 11:03, Han-Lin Chen <hanlinchen@chromium.org> wrote:\n\n> IPU3Event and IPU3Action use single ControlList for both libcamera and V4L2\n> controls, and it's content could be either one based on the context.\n> Extend IPU3Event and IPU3Action for sensor and lens V4L2 controls, and\n> preserve\n> the original one for only libcamera Controls to make the content of an\n> event\n> more specific.\n>\n> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>\n> ---\n>  include/libcamera/ipa/ipu3.mojom     | 4 ++++\n>  src/ipa/ipu3/ipu3.cpp                | 2 +-\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n>  3 files changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/ipu3.mojom\n> b/include/libcamera/ipa/ipu3.mojom\n> index 2f254ed4..cc0d822f 100644\n> --- a/include/libcamera/ipa/ipu3.mojom\n> +++ b/include/libcamera/ipa/ipu3.mojom\n> @@ -23,11 +23,15 @@ struct IPU3Event {\n>         int64 frameTimestamp;\n>         uint32 bufferId;\n>         libcamera.ControlList controls;\n> +       libcamera.ControlList sensorControls;\n> +       libcamera.ControlList lensControls;\n>  };\n>\n>  struct IPU3Action {\n>         IPU3Operations op;\n>         libcamera.ControlList controls;\n> +       libcamera.ControlList sensorControls;\n> +       libcamera.ControlList lensControls;\n>  };\n>\n>  struct IPAConfigInfo {\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 5c51607d..6775570e 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -646,7 +646,7 @@ void IPAIPU3::setControls(unsigned int frame)\n>         ControlList ctrls(ctrls_);\n>         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n>         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> -       op.controls = ctrls;\n> +       op.sensorControls = ctrls;\n>\n>         queueFrameAction.emit(frame, op);\n>  }\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index eb714aa6..8816efc5 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1248,7 +1248,7 @@ void IPU3CameraData::queueFrameAction(unsigned int\n> id,\n>  {\n>         switch (action.op) {\n>         case ipa::ipu3::ActionSetSensorControls: {\n> -               const ControlList &controls = action.controls;\n> +               const ControlList &controls = action.sensorControls;\n>                 delayedCtrls_->push(controls);\n>                 break;\n>         }\n> --\n> 2.33.1.1089.g2158813163f-goog\n>\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 89C80BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Oct 2021 10:57:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE3AC600BC;\n\tThu, 28 Oct 2021 12:57:00 +0200 (CEST)","from mail-wm1-x336.google.com (mail-wm1-x336.google.com\n\t[IPv6:2a00:1450:4864:20::336])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C391600BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Oct 2021 12:56:59 +0200 (CEST)","by mail-wm1-x336.google.com with SMTP id\n\tb2-20020a1c8002000000b0032fb900951eso834317wmd.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Oct 2021 03:56:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"ZnavcqML\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=nm7+2OhAKiLDaRRXxcp8CDoW78Ls6vjDCW1hbKYX/M4=;\n\tb=ZnavcqMLhGBosfbIGuvumy8z+udmGca5Zx+rPNpcHdPIrow58InnGVRqAj2mAMrxZ9\n\tY8AwBdO/e4OpqzvDMGe7W00UyxDRfO0mFjiF6voV7SJ8qEn0lhfRhdti662RxJlPgQ45\n\tRz82RkGzohTxJGPmydqkSfhxQ/SLjMh6HL+UDHFUJPm+ZryhIj0xZyzMKD4B9mbucexo\n\tibZUXnIaB5l0Y21a0mLUR0Mw9hDKpNgFywRv830grZP8rUCXCrukb/HmMSSyvMpW4ldQ\n\t+O0E3h4NOYcckX6XP4ot+YXFn/spaEQRyNVL2CZq1g99KiMHzT66u0irN5t5aEYf2JxX\n\tEpgA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=nm7+2OhAKiLDaRRXxcp8CDoW78Ls6vjDCW1hbKYX/M4=;\n\tb=v3AoJuRB9D9iz+oDFVWD9Purbni0wTtZZO7b3dRH0yPTq3wXh8Q0LZupAnncBxGmkp\n\tcMcYpVnoRjMUKNyOpupgR/2dXfB7m1UcjuphaLWPTMnZtbTezS6XHFP/XkbxMOjCkLEf\n\tsVoTBhu2bcaQe115epny9Q3sYIZLIBhxABuyvqgfY8GluXFPhJZ/+dF4D+5mFiY4iKZS\n\tR6rubEA6H85JU/i+rINvCTl265wXWvyalncibrPrv+//OmA5U4cSnoI1OUUcNuJuP54C\n\tJVe9k7OLezaEm7AFQHLOaYvuuR28/fnZt3pHRjoU6nlw7qhsk/zzbs+R3W5nArjd5MpS\n\tZ/uQ==","X-Gm-Message-State":"AOAM533935tUNOmNhcP0+OLDTJDmIgD121nbWtsBa3SYzcuksOH5ndrl\n\tplMmJQLTYwIUyJwY5rtkLF/Z76e2GIc2tWuzkBtYzHaIkAg=","X-Google-Smtp-Source":"ABdhPJyMHjSszKs6hml/4YY1sJ764fojh3sdKEZLGZCy6lEosqsqgusmShgw6rBnJ3we2VNVUju5oCH6L0B94IkTdhg=","X-Received":"by 2002:a05:600c:1c21:: with SMTP id\n\tj33mr11456325wms.163.1635418618783; \n\tThu, 28 Oct 2021 03:56:58 -0700 (PDT)","MIME-Version":"1.0","References":"<20211028100319.1097720-1-hanlinchen@chromium.org>","In-Reply-To":"<20211028100319.1097720-1-hanlinchen@chromium.org>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Thu, 28 Oct 2021 11:56:48 +0100","Message-ID":"<CAHW6GYK2v1Lzq3LHTgbkVCSF3y-pjwRzVyOkaktDUnqv981YTw@mail.gmail.com>","To":"Han-Lin Chen <hanlinchen@chromium.org>","Content-Type":"multipart/alternative; boundary=\"000000000000d5022905cf6791b4\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: ipu3: Extend ipu3 ipa\n\tinterface for sensor and lens controls","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>,\n\tHan-Lin Chen <hanlinchen@chromium.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20608,"web_url":"https://patchwork.libcamera.org/comment/20608/","msgid":"<163542759035.2498920.10850556358743965699@Monstersaurus>","date":"2021-10-28T13:26:30","subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: ipa: Allow IPA to apply\n\tcontrols to the lens device","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Han-Lin Chen (2021-10-28 11:03:19)\n> Allow IPA to apply controls to the lens device.\n> \n> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>\n> ---\n>  meson.build                          |  6 ++++++\n>  src/libcamera/pipeline/ipu3/cio2.cpp | 30 ++++++++++++++++++++++++++++\n>  src/libcamera/pipeline/ipu3/cio2.h   |  3 +++\n>  src/libcamera/pipeline/ipu3/ipu3.cpp |  9 +++++++--\n>  4 files changed, 46 insertions(+), 2 deletions(-)\n> \n> diff --git a/meson.build b/meson.build\n> index 7892a9e3..2a4b68a2 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -108,6 +108,12 @@ if cc.has_argument('-Wno-c99-designator')\n>      ]\n>  endif\n>  \n> +if get_option('android_platform') == 'cros'\n> +    common_arguments += [\n> +        '-DOS_CHROMEOS',\n> +    ]\n> +endif\n> +\n>  c_arguments += common_arguments\n>  cpp_arguments += common_arguments\n>  \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 59dda56b..143e2a95 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -16,6 +16,7 @@\n>  #include <libcamera/geometry.h>\n>  #include <libcamera/stream.h>\n>  \n> +#include \"libcamera/internal/camera_lens.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/framebuffer.h\"\n>  #include \"libcamera/internal/media_device.h\"\n> @@ -159,6 +160,35 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n>                 return -EINVAL;\n>         }\n>  \n> +#if defined(OS_CHROMEOS)\n\nI'd feel very awkward letting such OS specifics into the pipeline\nhandler.\n\nThis really needs to come from the media-controller graph, but I fear\nthat the graph might not be currently telling us the association with\nthe sensor. So we must fix that as a priority.\n\nIf this was (very) temporary while the media-controller was being fixed,\nI might overlook this - but not unless I knew it was being actively\nfixed - or this would just end up sticking.\n\n> +       /*\n> +        * \\todo Read the lens model from the sensor itself or from a device database.\n> +        * For now use default values taken from ChromeOS.\n> +        */\n> +       static std::unordered_map<std::string, std::string> sensorLens = {\n> +               { \"ov13858\", \"dw9714\" },\n> +               { \"imx258\", \"dw9807\" },\n> +               { \"imx355\", \"ak7375\" }\n> +       };\n> +\n> +       auto it = sensorLens.find(sensor_->model());\n> +       if (it != sensorLens.end()) {\n> +               const std::vector<MediaEntity *> &entities = media->entities();\n> +               for (auto ent: entities) {\n> +                       if (ent->function() == MEDIA_ENT_F_LENS) {\n> +                               lens_ = std::make_unique<CameraLens>(ent);\n> +                               ret = lens_->init();\n> +                               if (!ret && lens_->model() == it->second) {\n> +                                       break;\n> +                               }\n> +                               lens_.reset();\n> +                       }\n> +                       if (!lens_)\n> +                               LOG(IPU3, Warning) << \"Lens device \" << it->second << \" not found\";\n> +               }\n> +       }\n> +#endif\n> +\n>         /*\n>          * \\todo Define when to open and close video device nodes, as they\n>          * might impact on power consumption.\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> index ba8f0052..635566c8 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.h\n> +++ b/src/libcamera/pipeline/ipu3/cio2.h\n> @@ -18,6 +18,7 @@\n>  \n>  namespace libcamera {\n>  \n> +class CameraLens;\n>  class CameraSensor;\n>  class FrameBuffer;\n>  class MediaDevice;\n> @@ -52,6 +53,7 @@ public:\n>         int stop();\n>  \n>         CameraSensor *sensor() { return sensor_.get(); }\n> +       CameraLens *lens() { return lens_.get(); }\n>         const CameraSensor *sensor() const { return sensor_.get(); }\n>  \n>         FrameBuffer *queueBuffer(Request *request, FrameBuffer *rawBuffer);\n> @@ -67,6 +69,7 @@ private:\n>         void cio2BufferReady(FrameBuffer *buffer);\n>  \n>         std::unique_ptr<CameraSensor> sensor_;\n> +       std::unique_ptr<CameraLens> lens_;\n>         std::unique_ptr<V4L2Subdevice> csi2_;\n>         std::unique_ptr<V4L2VideoDevice> output_;\n>  \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 6a7f5b9a..36e93fb0 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -24,6 +24,7 @@\n>  #include <libcamera/stream.h>\n>  \n>  #include \"libcamera/internal/camera.h\"\n> +#include \"libcamera/internal/camera_lens.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/delayed_controls.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n> @@ -1250,8 +1251,12 @@ void IPU3CameraData::queueFrameAction(unsigned int id,\n>  {\n>         switch (action.op) {\n>         case ipa::ipu3::ActionSetSensorControls: {\n> -               const ControlList &controls = action.sensorControls;\n> -               delayedCtrls_->push(controls);\n> +               const ControlList &sensorControls = action.sensorControls;\n> +               delayedCtrls_->push(sensorControls);\n> +               if (cio2_.lens()) {\n> +                       ControlList& lensControls = const_cast<ControlList&>(action.lensControls);\n\nWhy does this need a const_cast? Can it not be used in the same way as\nthe action.sensorControls?\n\n\t\t\tconst ControlList &lensControls = action.lensControls;\n\n> +                       cio2_.lens()->setControls(&lensControls);\n\nOr can it just be set directly here?\n\t\t\tcio2_.lens()->setControls(&action.lenscontrols);\n\n\n> +               }\n>                 break;\n>         }\n>         case ipa::ipu3::ActionParamFilled: {\n> -- \n> 2.33.1.1089.g2158813163f-goog\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 12807BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Oct 2021 13:26:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6CC16600BA;\n\tThu, 28 Oct 2021 15:26:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 55B5B600B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Oct 2021 15:26:33 +0200 (CEST)","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 CE31F513;\n\tThu, 28 Oct 2021 15:26:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mI4iBc7y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635427592;\n\tbh=lOhXTfVm+9TU8XsWWXY4nmRj6fev0H2D3Y2ZOPX/guw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=mI4iBc7yjJzrsOG1itnDi1edfwfPr8+gmoSVhNaQCnnGvwgP5oG4Nar3iUk4kw4An\n\tVepuBTRNjJD53gtQ2TLCoEF7G1zx94jqcSnGunuz71L6arYMOSV1CpxHpCAAPTozbj\n\t5sa8G7jPUGAKLsE8W8mcE+914EVGx3HoOSmz3Ln4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211028100319.1097720-4-hanlinchen@chromium.org>","References":"<20211028100319.1097720-1-hanlinchen@chromium.org>\n\t<20211028100319.1097720-4-hanlinchen@chromium.org>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Han-Lin Chen <hanlinchen@chromium.org>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 28 Oct 2021 14:26:30 +0100","Message-ID":"<163542759035.2498920.10850556358743965699@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: ipa: Allow IPA to apply\n\tcontrols to the lens device","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>","Cc":"Han-Lin Chen <hanlinchen@chromium.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20610,"web_url":"https://patchwork.libcamera.org/comment/20610/","msgid":"<163542789077.2498920.5101413435383192606@Monstersaurus>","date":"2021-10-28T13:31:30","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: ipu3: Extend ipu3 ipa\n\tinterface for sensor and lens controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Han-Lin Chen (2021-10-28 11:03:16)\n> IPU3Event and IPU3Action use single ControlList for both libcamera and V4L2\n> controls, and it's content could be either one based on the context.\n> Extend IPU3Event and IPU3Action for sensor and lens V4L2 controls, and preserve\n> the original one for only libcamera Controls to make the content of an event\n> more specific.\n\nI like this, it does make the controls clearer.\nI wonder if there's much impact on the IPC but when empty, I presume it\nwill be minimal cost.\n\n> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>\n> ---\n>  include/libcamera/ipa/ipu3.mojom     | 4 ++++\n>  src/ipa/ipu3/ipu3.cpp                | 2 +-\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n>  3 files changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> index 2f254ed4..cc0d822f 100644\n> --- a/include/libcamera/ipa/ipu3.mojom\n> +++ b/include/libcamera/ipa/ipu3.mojom\n> @@ -23,11 +23,15 @@ struct IPU3Event {\n>         int64 frameTimestamp;\n>         uint32 bufferId;\n>         libcamera.ControlList controls;\n> +       libcamera.ControlList sensorControls;\n> +       libcamera.ControlList lensControls;\n\nDo events need to send sensor controls and lens controls to the IPA? Or\nwill these always be unused?\n\nAha, I see at least the Sensor controls are going to get used in the\nnext patch... Ok - so keeping them aligned with IPU3Action seems to make\nsense.\n\n\n>  };\n>  \n>  struct IPU3Action {\n>         IPU3Operations op;\n>         libcamera.ControlList controls;\n> +       libcamera.ControlList sensorControls;\n> +       libcamera.ControlList lensControls;\n>  };\n>  \n>  struct IPAConfigInfo {\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 5c51607d..6775570e 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -646,7 +646,7 @@ void IPAIPU3::setControls(unsigned int frame)\n>         ControlList ctrls(ctrls_);\n>         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n>         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> -       op.controls = ctrls;\n> +       op.sensorControls = ctrls;\n>  \n>         queueFrameAction.emit(frame, op);\n>  }\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index eb714aa6..8816efc5 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1248,7 +1248,7 @@ void IPU3CameraData::queueFrameAction(unsigned int id,\n>  {\n>         switch (action.op) {\n>         case ipa::ipu3::ActionSetSensorControls: {\n> -               const ControlList &controls = action.controls;\n> +               const ControlList &controls = action.sensorControls;\n>                 delayedCtrls_->push(controls);\n>                 break;\n>         }\n> -- \n> 2.33.1.1089.g2158813163f-goog\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 DE37EBF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Oct 2021 13:31:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04788600BA;\n\tThu, 28 Oct 2021 15:31:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 66CC3600B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Oct 2021 15:31:33 +0200 (CEST)","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 E6D4F513;\n\tThu, 28 Oct 2021 15:31:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"osX6JEQN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635427893;\n\tbh=d8he9aw95Mp4LUtx3qlVovMhJeN20WRK/M4klHDWax0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=osX6JEQNGv9N/NHfcZwLrRs8J4/+mIbN3BYfMtUTMdkHg/0kO6PHziunxnqYKlF62\n\t7yH1ZeHqDXUNWEc09P/Itp1XGLqsEfxB1f9XLlfhERWlFLXKk1PmnAEmDRFpbuwQTb\n\tYZX7A+JhAjSBPW5nFN6qQjKFVfhSkOu0CI5yAHjk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211028100319.1097720-1-hanlinchen@chromium.org>","References":"<20211028100319.1097720-1-hanlinchen@chromium.org>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Han-Lin Chen <hanlinchen@chromium.org>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 28 Oct 2021 14:31:30 +0100","Message-ID":"<163542789077.2498920.5101413435383192606@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: ipu3: Extend ipu3 ipa\n\tinterface for sensor and lens controls","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>","Cc":"Han-Lin Chen <hanlinchen@chromium.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20627,"web_url":"https://patchwork.libcamera.org/comment/20627/","msgid":"<CAJAuwMmB1G9Rr9yPQDnhuGKcyq=dV+e6-Y2-F8agVNMyFZhY-A@mail.gmail.com>","date":"2021-10-29T08:57:00","subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: ipa: Allow IPA to apply\n\tcontrols to the lens device","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi Kieran,\nMany thanks for the review.\n\nOn Thu, Oct 28, 2021 at 9:26 PM Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Quoting Han-Lin Chen (2021-10-28 11:03:19)\n> > Allow IPA to apply controls to the lens device.\n> >\n> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>\n> > ---\n> >  meson.build                          |  6 ++++++\n> >  src/libcamera/pipeline/ipu3/cio2.cpp | 30 ++++++++++++++++++++++++++++\n> >  src/libcamera/pipeline/ipu3/cio2.h   |  3 +++\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp |  9 +++++++--\n> >  4 files changed, 46 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/meson.build b/meson.build\n> > index 7892a9e3..2a4b68a2 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -108,6 +108,12 @@ if cc.has_argument('-Wno-c99-designator')\n> >      ]\n> >  endif\n> >\n> > +if get_option('android_platform') == 'cros'\n> > +    common_arguments += [\n> > +        '-DOS_CHROMEOS',\n> > +    ]\n> > +endif\n> > +\n> >  c_arguments += common_arguments\n> >  cpp_arguments += common_arguments\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index 59dda56b..143e2a95 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -16,6 +16,7 @@\n> >  #include <libcamera/geometry.h>\n> >  #include <libcamera/stream.h>\n> >\n> > +#include \"libcamera/internal/camera_lens.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/framebuffer.h\"\n> >  #include \"libcamera/internal/media_device.h\"\n> > @@ -159,6 +160,35 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> >                 return -EINVAL;\n> >         }\n> >\n> > +#if defined(OS_CHROMEOS)\n>\n> I'd feel very awkward letting such OS specifics into the pipeline\n> handler.\n>\n> This really needs to come from the media-controller graph, but I fear\n> that the graph might not be currently telling us the association with\n> the sensor. So we must fix that as a priority.\n>\n> If this was (very) temporary while the media-controller was being fixed,\n> I might overlook this - but not unless I knew it was being actively\n> fixed - or this would just end up sticking.\n\nYes, we do have a discussion on how to fix media-controller on our\nweekly meeting.\nTomasz may have more insight on this.\n\n>\n> > +       /*\n> > +        * \\todo Read the lens model from the sensor itself or from a device database.\n> > +        * For now use default values taken from ChromeOS.\n> > +        */\n> > +       static std::unordered_map<std::string, std::string> sensorLens = {\n> > +               { \"ov13858\", \"dw9714\" },\n> > +               { \"imx258\", \"dw9807\" },\n> > +               { \"imx355\", \"ak7375\" }\n> > +       };\n> > +\n> > +       auto it = sensorLens.find(sensor_->model());\n> > +       if (it != sensorLens.end()) {\n> > +               const std::vector<MediaEntity *> &entities = media->entities();\n> > +               for (auto ent: entities) {\n> > +                       if (ent->function() == MEDIA_ENT_F_LENS) {\n> > +                               lens_ = std::make_unique<CameraLens>(ent);\n> > +                               ret = lens_->init();\n> > +                               if (!ret && lens_->model() == it->second) {\n> > +                                       break;\n> > +                               }\n> > +                               lens_.reset();\n> > +                       }\n> > +                       if (!lens_)\n> > +                               LOG(IPU3, Warning) << \"Lens device \" << it->second << \" not found\";\n> > +               }\n> > +       }\n> > +#endif\n> > +\n> >         /*\n> >          * \\todo Define when to open and close video device nodes, as they\n> >          * might impact on power consumption.\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> > index ba8f0052..635566c8 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.h\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.h\n> > @@ -18,6 +18,7 @@\n> >\n> >  namespace libcamera {\n> >\n> > +class CameraLens;\n> >  class CameraSensor;\n> >  class FrameBuffer;\n> >  class MediaDevice;\n> > @@ -52,6 +53,7 @@ public:\n> >         int stop();\n> >\n> >         CameraSensor *sensor() { return sensor_.get(); }\n> > +       CameraLens *lens() { return lens_.get(); }\n> >         const CameraSensor *sensor() const { return sensor_.get(); }\n> >\n> >         FrameBuffer *queueBuffer(Request *request, FrameBuffer *rawBuffer);\n> > @@ -67,6 +69,7 @@ private:\n> >         void cio2BufferReady(FrameBuffer *buffer);\n> >\n> >         std::unique_ptr<CameraSensor> sensor_;\n> > +       std::unique_ptr<CameraLens> lens_;\n> >         std::unique_ptr<V4L2Subdevice> csi2_;\n> >         std::unique_ptr<V4L2VideoDevice> output_;\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 6a7f5b9a..36e93fb0 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -24,6 +24,7 @@\n> >  #include <libcamera/stream.h>\n> >\n> >  #include \"libcamera/internal/camera.h\"\n> > +#include \"libcamera/internal/camera_lens.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/delayed_controls.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> > @@ -1250,8 +1251,12 @@ void IPU3CameraData::queueFrameAction(unsigned int id,\n> >  {\n> >         switch (action.op) {\n> >         case ipa::ipu3::ActionSetSensorControls: {\n> > -               const ControlList &controls = action.sensorControls;\n> > -               delayedCtrls_->push(controls);\n> > +               const ControlList &sensorControls = action.sensorControls;\n> > +               delayedCtrls_->push(sensorControls);\n> > +               if (cio2_.lens()) {\n> > +                       ControlList& lensControls = const_cast<ControlList&>(action.lensControls);\n>\n> Why does this need a const_cast? Can it not be used in the same way as\n> the action.sensorControls?\n>\n>                         const ControlList &lensControls = action.lensControls;\n>\n> > +                       cio2_.lens()->setControls(&lensControls);\n>\n> Or can it just be set directly here?\n>                         cio2_.lens()->setControls(&action.lenscontrols);\nCompiler would reject the implicit conversion from * to const*.\nDelayedControl implicitly copies it. We could do it too:\n\nControlList lensControls = action.lensControls;\ncio2_.lens()->setControls(&lensControls);\n\n Or change V4L2Device::setControls() to accept const ControlList*.\n>\n>\n> > +               }\n> >                 break;\n> >         }\n> >         case ipa::ipu3::ActionParamFilled: {\n> > --\n> > 2.33.1.1089.g2158813163f-goog\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 C6CDDBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 29 Oct 2021 08:57:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 498BC600B4;\n\tFri, 29 Oct 2021 10:57:15 +0200 (CEST)","from mail-oi1-x22a.google.com (mail-oi1-x22a.google.com\n\t[IPv6:2607:f8b0:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2915C600B4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 29 Oct 2021 10:57:13 +0200 (CEST)","by mail-oi1-x22a.google.com with SMTP id q124so12405280oig.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 29 Oct 2021 01:57:13 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"ocViFqvW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=g1FzSGQsfJ/YCHn1OgR2dXC8n/ExwNAq/wv5pMg4daE=;\n\tb=ocViFqvW4+GlYvnQl8rH78mvnU/D48Y3DYD2joNyClwbXaL62bF6D4WZq1bFmuBaqd\n\tqjByoQne3XnBP+pFIas+vtRTbbaRdN9D/vX2wQwEoyMcx92/EpAqaksWfLMiqZ2unuwC\n\tb76THAdUrTV0VGwdmrcV18iTxy49l3fqiOXyw=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=g1FzSGQsfJ/YCHn1OgR2dXC8n/ExwNAq/wv5pMg4daE=;\n\tb=OkIrt2w9LQmrZQ3llfm+hQqEO/Sr7KNJ1C0anHsYQTVdFbClxF21rsnIZYnJ0NqaMH\n\tVzcZ2Zro2S4Hj/j3jYufNqYLBJTvAZE6Tofj8wCUdvrvQj+5zVWEYxMrgH+LAFZquaU8\n\tzjuyrucj43vSc2gtbJe7pO3j8AKOrzEIaii4JB47gMCC/731rTRkOki/pns2b7Bs0o3T\n\tbWTUrSgPqhfjB7f9+jtofrdi//4M3vTV+Rt/xx16lneFuHJLLIERSkGxhM8uNQywDrLO\n\ttmt48RLhCE+jSh6rXrN0XJmzV32DntbioS6zRj+W/lhJzYUirVwyvjcSVKtaexnhgARk\n\tt7Kg==","X-Gm-Message-State":"AOAM5318mbKCEzyHw90whrSRJQT/zwel0NKK3JcxNGbn9GFwB1m5T3yU\n\ttGILbzNjbRW/RxMj033MOQzhKuyJZjF0NmHivHVM+g==","X-Google-Smtp-Source":"ABdhPJw2+Lz0BGinO9EE+jloQScmymm9rMX75niZkqIKC+xXDvgFKIeqf8hK6uN4iXQhrSInx8RJ+dX5So1x5944HzE=","X-Received":"by 2002:a05:6808:1827:: with SMTP id\n\tbh39mr1020905oib.142.1635497831659; \n\tFri, 29 Oct 2021 01:57:11 -0700 (PDT)","MIME-Version":"1.0","References":"<20211028100319.1097720-1-hanlinchen@chromium.org>\n\t<20211028100319.1097720-4-hanlinchen@chromium.org>\n\t<163542759035.2498920.10850556358743965699@Monstersaurus>","In-Reply-To":"<163542759035.2498920.10850556358743965699@Monstersaurus>","From":"Hanlin Chen <hanlinchen@chromium.org>","Date":"Fri, 29 Oct 2021 16:57:00 +0800","Message-ID":"<CAJAuwMmB1G9Rr9yPQDnhuGKcyq=dV+e6-Y2-F8agVNMyFZhY-A@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: ipa: Allow IPA to apply\n\tcontrols to the lens device","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20631,"web_url":"https://patchwork.libcamera.org/comment/20631/","msgid":"<163549887259.14824.6470630401730305936@Monstersaurus>","date":"2021-10-29T09:14:32","subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: ipa: Allow IPA to apply\n\tcontrols to the lens device","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Hanlin Chen (2021-10-29 09:57:00)\n> Hi Kieran,\n> Many thanks for the review.\n> \n> On Thu, Oct 28, 2021 at 9:26 PM Kieran Bingham\n> <kieran.bingham@ideasonboard.com> wrote:\n> >\n> > Quoting Han-Lin Chen (2021-10-28 11:03:19)\n> > > Allow IPA to apply controls to the lens device.\n> > >\n> > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>\n> > > ---\n> > >  meson.build                          |  6 ++++++\n> > >  src/libcamera/pipeline/ipu3/cio2.cpp | 30 ++++++++++++++++++++++++++++\n> > >  src/libcamera/pipeline/ipu3/cio2.h   |  3 +++\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp |  9 +++++++--\n> > >  4 files changed, 46 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/meson.build b/meson.build\n> > > index 7892a9e3..2a4b68a2 100644\n> > > --- a/meson.build\n> > > +++ b/meson.build\n> > > @@ -108,6 +108,12 @@ if cc.has_argument('-Wno-c99-designator')\n> > >      ]\n> > >  endif\n> > >\n> > > +if get_option('android_platform') == 'cros'\n> > > +    common_arguments += [\n> > > +        '-DOS_CHROMEOS',\n> > > +    ]\n> > > +endif\n> > > +\n> > >  c_arguments += common_arguments\n> > >  cpp_arguments += common_arguments\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > index 59dda56b..143e2a95 100644\n> > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > @@ -16,6 +16,7 @@\n> > >  #include <libcamera/geometry.h>\n> > >  #include <libcamera/stream.h>\n> > >\n> > > +#include \"libcamera/internal/camera_lens.h\"\n> > >  #include \"libcamera/internal/camera_sensor.h\"\n> > >  #include \"libcamera/internal/framebuffer.h\"\n> > >  #include \"libcamera/internal/media_device.h\"\n> > > @@ -159,6 +160,35 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> > >                 return -EINVAL;\n> > >         }\n> > >\n> > > +#if defined(OS_CHROMEOS)\n> >\n> > I'd feel very awkward letting such OS specifics into the pipeline\n> > handler.\n> >\n> > This really needs to come from the media-controller graph, but I fear\n> > that the graph might not be currently telling us the association with\n> > the sensor. So we must fix that as a priority.\n> >\n> > If this was (very) temporary while the media-controller was being fixed,\n> > I might overlook this - but not unless I knew it was being actively\n> > fixed - or this would just end up sticking.\n> \n> Yes, we do have a discussion on how to fix media-controller on our\n> weekly meeting.\n> Tomasz may have more insight on this.\n\nOk, I hope that makes good progress.\n\n> > > +       /*\n> > > +        * \\todo Read the lens model from the sensor itself or from a device database.\n> > > +        * For now use default values taken from ChromeOS.\n> > > +        */\n> > > +       static std::unordered_map<std::string, std::string> sensorLens = {\n> > > +               { \"ov13858\", \"dw9714\" },\n> > > +               { \"imx258\", \"dw9807\" },\n> > > +               { \"imx355\", \"ak7375\" }\n> > > +       };\n> > > +\n> > > +       auto it = sensorLens.find(sensor_->model());\n> > > +       if (it != sensorLens.end()) {\n> > > +               const std::vector<MediaEntity *> &entities = media->entities();\n> > > +               for (auto ent: entities) {\n> > > +                       if (ent->function() == MEDIA_ENT_F_LENS) {\n> > > +                               lens_ = std::make_unique<CameraLens>(ent);\n> > > +                               ret = lens_->init();\n> > > +                               if (!ret && lens_->model() == it->second) {\n> > > +                                       break;\n> > > +                               }\n> > > +                               lens_.reset();\n> > > +                       }\n> > > +                       if (!lens_)\n> > > +                               LOG(IPU3, Warning) << \"Lens device \" << it->second << \" not found\";\n> > > +               }\n> > > +       }\n> > > +#endif\n> > > +\n> > >         /*\n> > >          * \\todo Define when to open and close video device nodes, as they\n> > >          * might impact on power consumption.\n> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> > > index ba8f0052..635566c8 100644\n> > > --- a/src/libcamera/pipeline/ipu3/cio2.h\n> > > +++ b/src/libcamera/pipeline/ipu3/cio2.h\n> > > @@ -18,6 +18,7 @@\n> > >\n> > >  namespace libcamera {\n> > >\n> > > +class CameraLens;\n> > >  class CameraSensor;\n> > >  class FrameBuffer;\n> > >  class MediaDevice;\n> > > @@ -52,6 +53,7 @@ public:\n> > >         int stop();\n> > >\n> > >         CameraSensor *sensor() { return sensor_.get(); }\n> > > +       CameraLens *lens() { return lens_.get(); }\n> > >         const CameraSensor *sensor() const { return sensor_.get(); }\n> > >\n> > >         FrameBuffer *queueBuffer(Request *request, FrameBuffer *rawBuffer);\n> > > @@ -67,6 +69,7 @@ private:\n> > >         void cio2BufferReady(FrameBuffer *buffer);\n> > >\n> > >         std::unique_ptr<CameraSensor> sensor_;\n> > > +       std::unique_ptr<CameraLens> lens_;\n> > >         std::unique_ptr<V4L2Subdevice> csi2_;\n> > >         std::unique_ptr<V4L2VideoDevice> output_;\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 6a7f5b9a..36e93fb0 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -24,6 +24,7 @@\n> > >  #include <libcamera/stream.h>\n> > >\n> > >  #include \"libcamera/internal/camera.h\"\n> > > +#include \"libcamera/internal/camera_lens.h\"\n> > >  #include \"libcamera/internal/camera_sensor.h\"\n> > >  #include \"libcamera/internal/delayed_controls.h\"\n> > >  #include \"libcamera/internal/device_enumerator.h\"\n> > > @@ -1250,8 +1251,12 @@ void IPU3CameraData::queueFrameAction(unsigned int id,\n> > >  {\n> > >         switch (action.op) {\n> > >         case ipa::ipu3::ActionSetSensorControls: {\n> > > -               const ControlList &controls = action.sensorControls;\n> > > -               delayedCtrls_->push(controls);\n> > > +               const ControlList &sensorControls = action.sensorControls;\n> > > +               delayedCtrls_->push(sensorControls);\n> > > +               if (cio2_.lens()) {\n> > > +                       ControlList& lensControls = const_cast<ControlList&>(action.lensControls);\n> >\n> > Why does this need a const_cast? Can it not be used in the same way as\n> > the action.sensorControls?\n> >\n> >                         const ControlList &lensControls = action.lensControls;\n> >\n> > > +                       cio2_.lens()->setControls(&lensControls);\n> >\n> > Or can it just be set directly here?\n> >                         cio2_.lens()->setControls(&action.lenscontrols);\n> Compiler would reject the implicit conversion from * to const*.\n> DelayedControl implicitly copies it. We could do it too:\n> \n> ControlList lensControls = action.lensControls;\n> cio2_.lens()->setControls(&lensControls);\n> \n>  Or change V4L2Device::setControls() to accept const ControlList*.\n\nAh, I see. setControls states:\n\n\"All controls below that index are written and their values are updated in \\a ctrls,\"\n\nSo setControls /modifies/ the incoming controls to update them in case\nthey were updated by the kernel.\n\nWe could make a const version of setControls but I'm weary that would\nsilently lose updates - but perhaps that's not unexpected if the\nControlList is const, and not expected to be updated\n... Hrm.. one to think more about I guess.\n\n\n> >\n> >\n> > > +               }\n> > >                 break;\n> > >         }\n> > >         case ipa::ipu3::ActionParamFilled: {\n> > > --\n> > > 2.33.1.1089.g2158813163f-goog\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 8F5E3BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 29 Oct 2021 09:14:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 463B7600BB;\n\tFri, 29 Oct 2021 11:14:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 93EFD600B4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 29 Oct 2021 11:14:35 +0200 (CEST)","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 3B60F881;\n\tFri, 29 Oct 2021 11:14:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"efbVtrdm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635498875;\n\tbh=+0qefe6DGwU4SUkC7IXVhXPfhXKsUngMLAShypnF0yI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=efbVtrdmRQaOzA6NFkvh8AGf9hpzftOOFvEJam1G/x3yWPkHJmNDNja7VHatshfhB\n\tpIPCYqK32cwPVQ26wgNYoe1JE25fix+ji+w2P9hLj5/2P5nu9WPaqWXsZTCdkkJ7o9\n\tNoYsOECvd3hg5d6CVdWBIH0LWNf2ofvrktSu+O6w=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAJAuwMmB1G9Rr9yPQDnhuGKcyq=dV+e6-Y2-F8agVNMyFZhY-A@mail.gmail.com>","References":"<20211028100319.1097720-1-hanlinchen@chromium.org>\n\t<20211028100319.1097720-4-hanlinchen@chromium.org>\n\t<163542759035.2498920.10850556358743965699@Monstersaurus>\n\t<CAJAuwMmB1G9Rr9yPQDnhuGKcyq=dV+e6-Y2-F8agVNMyFZhY-A@mail.gmail.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Hanlin Chen <hanlinchen@chromium.org>","Date":"Fri, 29 Oct 2021 10:14:32 +0100","Message-ID":"<163549887259.14824.6470630401730305936@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipu3: ipa: Allow IPA to apply\n\tcontrols to the lens device","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]