[{"id":22706,"web_url":"https://patchwork.libcamera.org/comment/22706/","msgid":"<164992654540.22830.14972184798922345085@Monstersaurus>","date":"2022-04-14T08:55:45","subject":"Re: [libcamera-devel] [RFC v1 5/5] ipa: ipu3: af: Remove hardcoded\n\tmaximum VCM steps","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kate Hsuan via libcamera-devel (2022-04-14 08:43:42)\n> The hardcoded VCM step variable was removed and was replaced by the\n> context configuration.\n> \n> Signed-off-by: Kate Hsuan\n> ---\n> src/ipa/ipu3/algorithms/af.cpp | 6 +++---\n> src/ipa/ipu3/ipu3.cpp | 1 +\n> 2 files changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index addf98af..95fb39f5 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -179,7 +179,7 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> grid.y_start |= IPU3_UAPI_GRID_Y_START_EN;\n> \n> /* Initial max focus step */\n> - maxStep_ = kMaxFocusSteps;\n> + maxStep_ = context.configuration.af.maxVcmSteps;\n\nAha I see (from below) you're setting\ncontext.configuration.af.maxVcmSteps just to assign it here, but you\nhave direct access to configInfo.sensorInfo.maxVcmSteps.\n\nSeems like the af.maxVcmSteps addition might not be needed?\n\n> \n> /* Initial focus value */\n> context.frameContext.af.focus = 0;\n> @@ -214,7 +214,7 @@ void Af::afCoarseScan(IPAContext &context)\n> context.frameContext.af.focus = focus_;\n> previousVariance_ = 0;\n> maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n> - 0U, kMaxFocusSteps);\n> + 0U, static_cast(context.configuration.af.maxVcmSteps));\n\nThe static_cast shouldn't be needed here. I think maxVcmSteps should be\nuint32_t.\n\n> }\n> }\n> \n> @@ -259,7 +259,7 @@ void Af::afReset(IPAContext &context)\n> previousVariance_ = 0.0;\n> coarseCompleted_ = false;\n> fineCompleted_ = false;\n> - maxStep_ = kMaxFocusSteps;\n> + maxStep_ = context.configuration.af.maxVcmSteps;\n> }\n> \n> /**\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index dd6cfd79..d46f7853 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -456,6 +456,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \n> /* Clean frameContext at each reconfiguration. */\n> context_.frameContext = {};\n> + context_.configuration.af.maxVcmSteps = configInfo.sensorInfo.maxVcmSteps;\n\nThis should be in the configure call of the AF, which will then get\ncalled by the \"for (auto const &algo : algorithms_)\" loop at the bottom\nof this function.\n\n> if (!validateSensorControls()) {\n> LOG(IPAIPU3, Error) << \"Sensor control validation failed.\";\n> -- \n> 2.35.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 4863CC3256\n\tfor ;\n\tThu, 14 Apr 2022 08:55:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 78ECF6564D;\n\tThu, 14 Apr 2022 10:55:49 +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 02D3C65645\n\tfor ;\n\tThu, 14 Apr 2022 10:55:47 +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 8F30525B;\n\tThu, 14 Apr 2022 10:55:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649926549;\n\tbh=HAyVE8M85MqplCBXs2MiiRxdXPPopLU8Jiy2Xv/z5Vs=;\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:\n\tFrom;\n\tb=oTBYd9Wgndn2ZZOQMsDdzQRQVziZLJYFxIQrifHFLdDwB8x9zq33su4xRIl9MsFwI\n\tH4LQ3giHzMuwV0Deq11KKbRKvJT2jG00ShOEp2ny1Qx8iBrfMNzt0yVsMGYYJpO2/Q\n\t9kj/HeKhRAfhUZOyEjoDxr4t0McTVpJgEhHZZYdfKwFX2CIp51vabBeVSg+NPpG760\n\tpOdkMoFLL9ITeeFCtXyUA9AChCnGhS8mEuVPCeLxzBAbMkE5lQW3a0hVjQbWxz5cBL\n\tQbpi1RZ3DBm4qIDfM3yT16H29kJAEORjQ2CR9cxf6PRFbCEhnmtg+6quguLHBwZ4Ty\n\tR65CqONmrTNdQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649926547;\n\tbh=HAyVE8M85MqplCBXs2MiiRxdXPPopLU8Jiy2Xv/z5Vs=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=RMAn2S+kyquMoNlstOvKP65vs3vDW/ZkBZziapBDRYQjvtQH8VtgnAw880MfHiyxX\n\tz2+NTknOM/WeHKZ1qo7TIIphflwOBbKWhp8FtPGJtxEEKqcbxAnWJ529wkZLqPrfIb\n\tP/SgU3KFKcuNczpFR9z49dcrV1nMiHcP6ptSH5fs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RMAn2S+k\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220414074342.7455-6-hpa@redhat.com>","References":"<20220414074342.7455-1-hpa@redhat.com>\n\t<20220414074342.7455-6-hpa@redhat.com>","To":"Kate Hsuan ,\n\tlibcamera devel ","Date":"Thu, 14 Apr 2022 09:55:45 +0100","Message-ID":"<164992654540.22830.14972184798922345085@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC v1 5/5] ipa: ipu3: af: Remove hardcoded\n\tmaximum VCM steps","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22717,"web_url":"https://patchwork.libcamera.org/comment/22717/","msgid":"","date":"2022-04-15T04:01:51","subject":"Re: [libcamera-devel] [RFC v1 5/5] ipa: ipu3: af: Remove hardcoded\n\tmaximum VCM steps","submitter":{"id":105,"url":"https://patchwork.libcamera.org/api/people/105/","name":"Kate Hsuan","email":"hpa@redhat.com"},"content":"Hi Kieran,\n\nOn Thu, Apr 14, 2022 at 4:55 PM Kieran Bingham\n wrote:\n>\n> Quoting Kate Hsuan via libcamera-devel (2022-04-14 08:43:42)\n> > The hardcoded VCM step variable was removed and was replaced by the\n> > context configuration.\n> >\n> > Signed-off-by: Kate Hsuan\n> > ---\n> > src/ipa/ipu3/algorithms/af.cpp | 6 +++---\n> > src/ipa/ipu3/ipu3.cpp | 1 +\n> > 2 files changed, 4 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> > index addf98af..95fb39f5 100644\n> > --- a/src/ipa/ipu3/algorithms/af.cpp\n> > +++ b/src/ipa/ipu3/algorithms/af.cpp\n> > @@ -179,7 +179,7 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> > grid.y_start |= IPU3_UAPI_GRID_Y_START_EN;\n> >\n> > /* Initial max focus step */\n> > - maxStep_ = kMaxFocusSteps;\n> > + maxStep_ = context.configuration.af.maxVcmSteps;\n>\n> Aha I see (from below) you're setting\n> context.configuration.af.maxVcmSteps just to assign it here, but you\n> have direct access to configInfo.sensorInfo.maxVcmSteps.\n>\n> Seems like the af.maxVcmSteps addition might not be needed?\n\nSeems can be removed. I could get the value in configure() of the\nalgorithm. I'll update them in v2.\n\n\nThank you :)\n\n>\n> >\n> > /* Initial focus value */\n> > context.frameContext.af.focus = 0;\n> > @@ -214,7 +214,7 @@ void Af::afCoarseScan(IPAContext &context)\n> > context.frameContext.af.focus = focus_;\n> > previousVariance_ = 0;\n> > maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n> > - 0U, kMaxFocusSteps);\n> > + 0U, static_cast(context.configuration.af.maxVcmSteps));\n>\n> The static_cast shouldn't be needed here. I think maxVcmSteps should be\n> uint32_t.\n\n\n>\n> > }\n> > }\n> >\n> > @@ -259,7 +259,7 @@ void Af::afReset(IPAContext &context)\n> > previousVariance_ = 0.0;\n> > coarseCompleted_ = false;\n> > fineCompleted_ = false;\n> > - maxStep_ = kMaxFocusSteps;\n> > + maxStep_ = context.configuration.af.maxVcmSteps;\n> > }\n> >\n> > /**\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index dd6cfd79..d46f7853 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -456,6 +456,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> >\n> > /* Clean frameContext at each reconfiguration. */\n> > context_.frameContext = {};\n> > + context_.configuration.af.maxVcmSteps = configInfo.sensorInfo.maxVcmSteps;\n>\n> This should be in the configure call of the AF, which will then get\n> called by the \"for (auto const &algo : algorithms_)\" loop at the bottom\n> of this function.\n>\n> > if (!validateSensorControls()) {\n> > LOG(IPAIPU3, Error) << \"Sensor control validation failed.\";\n> > --\n> > 2.35.1\n> >\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 4CA5BC3256\n\tfor ;\n\tFri, 15 Apr 2022 04:02:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71E9365648;\n\tFri, 15 Apr 2022 06:02:09 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E0FB65640\n\tfor ;\n\tFri, 15 Apr 2022 06:02:07 +0200 (CEST)","from mail-lf1-f72.google.com (mail-lf1-f72.google.com\n\t[209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n\tus-mta-489-saJU3BZuPWO7lOhNSIGkfA-1; Fri, 15 Apr 2022 00:02:04 -0400","by mail-lf1-f72.google.com with SMTP id\n\tw34-20020a0565120b2200b0044adfdd1570so3081148lfu.23\n\tfor ;\n\tThu, 14 Apr 2022 21:02:04 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649995329;\n\tbh=pY8QJAk4VOQbqAw68pFAntE4Efy8CriZWsGzdZhPNdw=;\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=046/+LVOBCeBtNFdtT0x1i3PfsFvo9RvHMgGZpXX2h2kZtQjBQ6FwSfUHptYF/2nF\n\tWlzADSVMuG0SKl1KuYWvxqkoSfc2FMNvMhX5HfgIrPG3U9XNsWBuX4rv9FBB2+kc/Y\n\tVa0Tjj12AYZ6msMjnpreNzuKZF0vu+iutf1TEViXLqt5LlRUQtOOJIbnrNyf2eFEYY\n\tDEYLbqoYWFcHh8YoNiENizznJjXTtt68QuVs1g5ceEHJxIt7cWGDngn46D33OAcu/G\n\tsVEPabXSIJPBDNQLpzChu10r/b20sBStpjbeCTCPUYdk47vOEbLnwA5DFgrImmU0uQ\n\tfjo/qGiPU/Mxw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1649995326;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=FIMDUjPMa9pm3QxBndsh8OOm5B8zd56lQ/JCxEpavD4=;\n\tb=GuyjG0HxHO8+NJEeOLTFYARE707Ay97HjHLvykH/2mv6fdkZxaJcN0X43b09dXLHe3kqSQ\n\t1Nko9dOeu+MBBnAnjqIF5WcKE1nUI150Pi88rXItWPGoTccvAdNlv3TEOfKkVW+N6XjZOm\n\t89ERdrdWDzFt6fsVaUGCj56coL72xdw="],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=redhat.com\n\theader.i=@redhat.com header.b=\"GuyjG0Hx\"; \n\tdkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hpa@redhat.com"],"X-MC-Unique":"saJU3BZuPWO7lOhNSIGkfA-1","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=FIMDUjPMa9pm3QxBndsh8OOm5B8zd56lQ/JCxEpavD4=;\n\tb=iPryFA+kvC7z8TNrrcFMTpeqPwWFPP6z8nXGcNNsH74M/QMNCUWOQA/6FLDQ54hYWo\n\tC7PdyUtT3VDatWwynQKOhDirCNVwMaGT7INKwE79W4jcgUR5lLkOY4Efcj+DMFdc6F+n\n\tbazaPcsXclI2ZPJOyPwQCbUZpvzdSEejQPsFWsMUNl6E/3bURxe/G5sk9U/ZqANptFx4\n\tMP96lDqcBqcZyy9bvwBAURp+bNiCpd57c4389oCJGNH3mB/dsKkZ81e5KXlAmfHBzi1k\n\t2RPzUdLWVAYS8Xg8qEsaAzal4ziNfqt7lrifrd2nunv6Bslii9ZxS6MxIEliLHEwMl/T\n\tr0sA==","X-Gm-Message-State":"AOAM530S47noQPWdO3qbuodCp9B0aPtB+28pu/VFNNgftWka0aYpgt8h\n\tGi0rVfnvMeTPqYhpCPPBqysszt/Amyeh743ZBgFvo9voP5ksKuqqBjIlnNex+e8VTOgOcdA0YEM\n\te8+qcJLrqwVJHPXMjh0nZae6SMvoVPmm9Pa7BpeUaFWmRbXRxow==","X-Received":["by 2002:a2e:a783:0:b0:24b:6927:7460 with SMTP id\n\tc3-20020a2ea783000000b0024b69277460mr3394124ljf.104.1649995323034; \n\tThu, 14 Apr 2022 21:02:03 -0700 (PDT)","by 2002:a2e:a783:0:b0:24b:6927:7460 with SMTP id\n\tc3-20020a2ea783000000b0024b69277460mr3394110ljf.104.1649995322682;\n\tThu, 14 Apr 2022 21:02:02 -0700 (PDT)"],"X-Google-Smtp-Source":"ABdhPJxfWTyLbs2Q7zfbB1nT4fH0DTWwKyPXfAk/XuPGCtRmW11Jt1vtFY4bEQPYInxZZQvlvsoF1a2ZsCX6pTdjVd4=","MIME-Version":"1.0","References":"<20220414074342.7455-1-hpa@redhat.com>\n\t<20220414074342.7455-6-hpa@redhat.com>\n\t<164992654540.22830.14972184798922345085@Monstersaurus>","In-Reply-To":"<164992654540.22830.14972184798922345085@Monstersaurus>","Date":"Fri, 15 Apr 2022 12:01:51 +0800","Message-ID":"","To":"Kieran Bingham ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC v1 5/5] ipa: ipu3: af: Remove hardcoded\n\tmaximum VCM steps","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":"Kate Hsuan via libcamera-devel ","Reply-To":"Kate Hsuan ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]