[{"id":20710,"web_url":"https://patchwork.libcamera.org/comment/20710/","msgid":"<163637937556.275423.13353397859826368268@Monstersaurus>","date":"2021-11-08T13:49:35","subject":"Re: [libcamera-devel] [PATCH 10/22] ipu3: Move delayedControls\n\treset after IPA is started","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-08 13:13:38)\n> We can call configure multiple times and we don't need to reset\n> delayedControls for each of the configure calls. Reset it after the IPA\n> is started.\n> \n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 6a7f5b9a..3fcfa777 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -667,8 +667,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n>                 return ret;\n>         }\n>  \n> -       data->delayedCtrls_->reset();\n> -\n>         return updateControls(data);\n>  }\n>  \n> @@ -769,6 +767,12 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n>         if (ret)\n>                 goto error;\n>  \n> +       /*\n> +        * Reset the delayed controls with the gain and exposure values set by\n> +        * the IPA.\n> +        */\n> +       data->delayedCtrls_->reset();\n> +\n\nShould this be squashed into 2/22?\n\nHan-Lin - Does this affect your use case at all? (I suspect it's ok)\n--\nKieran\n\n\n>         /*\n>          * Start the ImgU video devices, buffers will be queued to the\n>          * ImgU output and viewfinder when requests will be queued.\n> -- \n> 2.32.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CC8E8BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  8 Nov 2021 13:49:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 85A3860362;\n\tMon,  8 Nov 2021 14:49:40 +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 75F7F6035D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  8 Nov 2021 14:49:38 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F7991853;\n\tMon,  8 Nov 2021 14:49:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Jjv7gpIs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636379378;\n\tbh=W1APS/Kf4iYUUccXIamKcRQ4nGzgQWs5qY/XA9F7ccw=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Jjv7gpIsJXniME7LbkZLhwu36e0rXjeXfG1u/ytrcPLp1QgytBmbEhk9EnhlrfFgm\n\t80U3d5Zxw7yn6oZ9tXJfnV+RJB1ZFYFAOxkL9v+h0K7UW0LPtbBDhzroS8whNlDUTV\n\twBEqBlGr3xczkG7w5WtD+Ttd+WQJkwO/3JFDrXlI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211108131350.130665-11-jeanmichel.hautbois@ideasonboard.com>","References":"<20211108131350.130665-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211108131350.130665-11-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 08 Nov 2021 13:49:35 +0000","Message-ID":"<163637937556.275423.13353397859826368268@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 10/22] ipu3: Move delayedControls\n\treset after IPA is started","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20711,"web_url":"https://patchwork.libcamera.org/comment/20711/","msgid":"<163637958889.275423.9544544314414985053@Monstersaurus>","date":"2021-11-08T13:53:08","subject":"Re: [libcamera-devel] [PATCH 10/22] ipu3: Move delayedControls\n\treset after IPA is started","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Perhaps put \n\nlibcamera: pipeline: ipu3: in the $SUBJECT?\n\n$ git log --oneline ./src/libcamera/pipeline\n\nshows a bit of a mix of styles, but I think we can be more specific than\njust ipu3: as that doesn't make it clear which side is being changed.\n\n\nQuoting Kieran Bingham (2021-11-08 13:49:35)\n> Quoting Jean-Michel Hautbois (2021-11-08 13:13:38)\n> > We can call configure multiple times and we don't need to reset\n> > delayedControls for each of the configure calls. Reset it after the IPA\n> > is started.\n> > \n> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++++--\n> >  1 file changed, 6 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 6a7f5b9a..3fcfa777 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -667,8 +667,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> >                 return ret;\n> >         }\n> >  \n> > -       data->delayedCtrls_->reset();\n> > -\n> >         return updateControls(data);\n> >  }\n> >  \n> > @@ -769,6 +767,12 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> >         if (ret)\n> >                 goto error;\n> >  \n> > +       /*\n> > +        * Reset the delayed controls with the gain and exposure values set by\n> > +        * the IPA.\n> > +        */\n> > +       data->delayedCtrls_->reset();\n> > +\n> \n> Should this be squashed into 2/22?\n\nIn fact, it probably stands alone from 2/22 and could be 'before' it.\n(With the addition of the ->reset in current 2/22 not being required in\nthat patch).\n\n\n\n> \n> Han-Lin - Does this affect your use case at all? (I suspect it's ok)\n> --\n> Kieran\n> \n> \n> >         /*\n> >          * Start the ImgU video devices, buffers will be queued to the\n> >          * ImgU output and viewfinder when requests will be queued.\n> > -- \n> > 2.32.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D19CCBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  8 Nov 2021 13:53:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36F6160371;\n\tMon,  8 Nov 2021 14:53:13 +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 AB98660362\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  8 Nov 2021 14:53:11 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4B9421853;\n\tMon,  8 Nov 2021 14:53:11 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ct6x5bpD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636379591;\n\tbh=+X1WX1Z7qx/kr7ZfHZcmY/vV5jTtpv79CdOJ7mq8sMs=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Ct6x5bpDv1kk9ihXEugOqUoI7ZmZFH1at9SS3tPTDJykenD5zalUK3y6FA/Vj4LT+\n\tKEH//m5YJvKkqN+1Om7jkRUhezFPpUce96fl534qbZbbJSv+vZ2U9rt0Ty65evP2FR\n\tiJNPDbyTT/+sMOAzNRhhWT3Rp9x8P/VMQq0E77Sg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<163637937556.275423.13353397859826368268@Monstersaurus>","References":"<20211108131350.130665-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211108131350.130665-11-jeanmichel.hautbois@ideasonboard.com>\n\t<163637937556.275423.13353397859826368268@Monstersaurus>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 08 Nov 2021 13:53:08 +0000","Message-ID":"<163637958889.275423.9544544314414985053@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 10/22] ipu3: Move delayedControls\n\treset after IPA is started","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":20746,"web_url":"https://patchwork.libcamera.org/comment/20746/","msgid":"<CAJAuwM=FQVtVP2f7Q5KPTdX65g2+g3FAL1xRW7mSfZK1ZqeM=w@mail.gmail.com>","date":"2021-11-09T10:54:37","subject":"Re: [libcamera-devel] [PATCH 10/22] ipu3: Move delayedControls\n\treset after IPA is started","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Thanks for the patch Jean-Michel,\nOn Mon, Nov 8, 2021 at 9:49 PM Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Quoting Jean-Michel Hautbois (2021-11-08 13:13:38)\n> > We can call configure multiple times and we don't need to reset\n> > delayedControls for each of the configure calls. Reset it after the IPA\n> > is started.\n> >\n> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++++--\n> >  1 file changed, 6 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 6a7f5b9a..3fcfa777 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -667,8 +667,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> >                 return ret;\n> >         }\n> >\n> > -       data->delayedCtrls_->reset();\n> > -\n> >         return updateControls(data);\n> >  }\n> >\n> > @@ -769,6 +767,12 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> >         if (ret)\n> >                 goto error;\n> >\n> > +       /*\n> > +        * Reset the delayed controls with the gain and exposure values set by\n> > +        * the IPA.\n> > +        */\n> > +       data->delayedCtrls_->reset();\n> > +\n>\n> Should this be squashed into 2/22?\n>\n> Han-Lin - Does this affect your use case at all? (I suspect it's ok)\nI think it should be okay as long as we reset it before the first\nframe queuing into the sensor node.\n> --\n> Kieran\n>\n>\n> >         /*\n> >          * Start the ImgU video devices, buffers will be queued to the\n> >          * ImgU output and viewfinder when requests will be queued.\n> > --\n> > 2.32.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1167EBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  9 Nov 2021 10:54:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 67CA5600B5;\n\tTue,  9 Nov 2021 11:54:51 +0100 (CET)","from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com\n\t[IPv6:2607:f8b0:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CBA4600B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Nov 2021 11:54:49 +0100 (CET)","by mail-oi1-x22f.google.com with SMTP id u2so32908758oiu.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 09 Nov 2021 02:54:49 -0800 (PST)"],"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=\"KlR+lykB\"; 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=t7UXaKRTpeEj6hKeRmDJHrFAvSvK/uwCWGYbpmpS8Ho=;\n\tb=KlR+lykBYsJ1R+JHzNzT0hKkhM2JGluhIAUhKUxZJzt7cTRrzKnafSAv5UwT8lYbRa\n\t0qbMjtahwUw6DMRiU+xB7Dc+v2giNxfbgYxs3tO1HpHFlGGUcPrPjJQw9oeNaAUrlZXE\n\tbRSjyvXQpjmm/2zkPgCU+zxATWBgETzkQjSgI=","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=t7UXaKRTpeEj6hKeRmDJHrFAvSvK/uwCWGYbpmpS8Ho=;\n\tb=z0T3CWQfSnVZ8z7rfI4OIjSN4IB2VSEBFo1ru9zcuN2H+6MRGnW0eaMNKwW7B0zhIA\n\tNc9YO5WHwhhDRpuPLm8K5qUvx29bdXWF+/DbGUZwn+uplDdVp3Uw4wf/fgXmn8SSaLkF\n\tEl2zAfxgSkRQaLeQd9tnE2qT49xvakDKIEiOWgzaOFjIvGauiIk6HkO/P2sIlMFMODyr\n\t32aA3KoUNmEmGmjnmQAcAOuKONcf30keuhZSIbv+U0PEd3kLqcVvTjZC51JPr+mfnz+P\n\t4BOEv7CSD3nw2JuRMX6GjAY24VrL2jK35F+DEBxGCxTGHQrKryeGWULgDNQNEiFe491t\n\tBB6Q==","X-Gm-Message-State":"AOAM533IM2xhOeImgbMH7q6xIq0beAr6r8OgKW4Oa0gB5hcjVPjGIBLs\n\t2MNAeMP8YV0AWlomFkavGpU2pEN/vgbMhm5uUhYjIg==","X-Google-Smtp-Source":"ABdhPJwhOc39nUnLR3NndMXju2UtPApvRdekjtbcyYPuJL/Vy1u13NNP6wDuLLjRaNedjCiweNuRu/1QREmAgLu1PKU=","X-Received":"by 2002:a05:6808:1823:: with SMTP id\n\tbh35mr4913301oib.142.1636455288179; \n\tTue, 09 Nov 2021 02:54:48 -0800 (PST)","MIME-Version":"1.0","References":"<20211108131350.130665-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211108131350.130665-11-jeanmichel.hautbois@ideasonboard.com>\n\t<163637937556.275423.13353397859826368268@Monstersaurus>","In-Reply-To":"<163637937556.275423.13353397859826368268@Monstersaurus>","From":"Hanlin Chen <hanlinchen@chromium.org>","Date":"Tue, 9 Nov 2021 18:54:37 +0800","Message-ID":"<CAJAuwM=FQVtVP2f7Q5KPTdX65g2+g3FAL1xRW7mSfZK1ZqeM=w@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 10/22] ipu3: Move delayedControls\n\treset after IPA is started","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>"}}]