[{"id":14472,"web_url":"https://patchwork.libcamera.org/comment/14472/","msgid":"<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>","date":"2021-01-07T16:37:01","subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Dec 29, 2020 at 05:03:16PM +0100, Niklas Söderlund wrote:\n> Map and unmap the parameters and statistic buffers in the IPA when the\n> pipeline handler allocates and frees the buffers.\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++++++\n>  1 file changed, 26 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 95f1b75dc8be5d40..141066c528890c8e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -138,6 +138,8 @@ private:\n>  \tImgUDevice imgu1_;\n>  \tMediaDevice *cio2MediaDev_;\n>  \tMediaDevice *imguMediaDev_;\n> +\n> +\tstd::vector<IPABuffer> ipaBuffers_;\n>  };\n>\n>  IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> @@ -583,6 +585,23 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n>  \tif (ret < 0)\n>  \t\treturn ret;\n>\n> +\t/* Map buffers to the IPA. */\n> +\tunsigned int ipaBufferId = 1;\n> +\n> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> +\t\tbuffer->setCookie(ipaBufferId++);\n> +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n\nYou can reserve space in ipaBuffers_\n\n> +\t\t\t\t\t.planes = buffer->planes() });\n> +\t}\n> +\n> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> +\t\tbuffer->setCookie(ipaBufferId++);\n> +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> +\t\t\t\t\t.planes = buffer->planes() });\n> +\t}\n> +\n> +\tdata->ipa_->mapBuffers(ipaBuffers_);\n> +\n>  \treturn 0;\n>  }\n>\n> @@ -590,6 +609,13 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n>\n> +\tstd::vector<unsigned int> ids;\n\nYou can reserve(ipaBuffers_.size())\n\n> +\tfor (IPABuffer &ipabuf : ipaBuffers_)\n> +\t\tids.push_back(ipabuf.id);\n> +\n> +\tdata->ipa_->unmapBuffers(ids);\n> +\tipaBuffers_.clear();\n> +\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n>  \tdata->imgu_->freeBuffers();\n>\n>  \treturn 0;\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 07CC6C3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 Jan 2021 16:36:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F66563494;\n\tThu,  7 Jan 2021 17:36:48 +0100 (CET)","from relay13.mail.gandi.net (relay13.mail.gandi.net\n\t[217.70.178.233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2987C62013\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 Jan 2021 17:36:47 +0100 (CET)","from uno.localdomain (unknown [93.56.78.48])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay13.mail.gandi.net (Postfix) with ESMTPSA id 6A5D78000B;\n\tThu,  7 Jan 2021 16:36:46 +0000 (UTC)"],"Date":"Thu, 7 Jan 2021 17:37:01 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-10-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201229160318.77536-10-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14498,"web_url":"https://patchwork.libcamera.org/comment/14498/","msgid":"<X/tUkn1Z6tducIAZ@pendragon.ideasonboard.com>","date":"2021-01-10T19:25:06","subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Thu, Jan 07, 2021 at 05:37:01PM +0100, Jacopo Mondi wrote:\n> On Tue, Dec 29, 2020 at 05:03:16PM +0100, Niklas Söderlund wrote:\n> > Map and unmap the parameters and statistic buffers in the IPA when the\n> > pipeline handler allocates and frees the buffers.\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++++++\n> >  1 file changed, 26 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 95f1b75dc8be5d40..141066c528890c8e 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -138,6 +138,8 @@ private:\n> >  \tImgUDevice imgu1_;\n> >  \tMediaDevice *cio2MediaDev_;\n> >  \tMediaDevice *imguMediaDev_;\n> > +\n> > +\tstd::vector<IPABuffer> ipaBuffers_;\n> >  };\n> >\n> >  IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> > @@ -583,6 +585,23 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> >\n> > +\t/* Map buffers to the IPA. */\n> > +\tunsigned int ipaBufferId = 1;\n> > +\n> > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > +\t\tbuffer->setCookie(ipaBufferId++);\n> > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> \n> You can reserve space in ipaBuffers_\n> \n> > +\t\t\t\t\t.planes = buffer->planes() });\n> > +\t}\n> > +\n> > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> > +\t\tbuffer->setCookie(ipaBufferId++);\n> > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> > +\t\t\t\t\t.planes = buffer->planes() });\n\n\t\tipaBuffers_.push_back({\n\t\t\t.id = buffer->cookie(),\n\t\t\t.planes = buffer->planes()\n\t\t});\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> > +\t}\n> > +\n> > +\tdata->ipa_->mapBuffers(ipaBuffers_);\n> > +\n> >  \treturn 0;\n> >  }\n> >\n> > @@ -590,6 +609,13 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> >  {\n> >  \tIPU3CameraData *data = cameraData(camera);\n> >\n> > +\tstd::vector<unsigned int> ids;\n> \n> You can reserve(ipaBuffers_.size())\n> \n> > +\tfor (IPABuffer &ipabuf : ipaBuffers_)\n> > +\t\tids.push_back(ipabuf.id);\n> > +\n> > +\tdata->ipa_->unmapBuffers(ids);\n> > +\tipaBuffers_.clear();\n> > +\n> \n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> >  \tdata->imgu_->freeBuffers();\n> >\n> >  \treturn 0;","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 440AFBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 10 Jan 2021 19:25:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B2EE568072;\n\tSun, 10 Jan 2021 20:25:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A1F960523\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 10 Jan 2021 20:25:22 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 86E73DA;\n\tSun, 10 Jan 2021 20:25:21 +0100 (CET)"],"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=\"N6D0511y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610306721;\n\tbh=+pfPjcXBeM8YhdBKbHM5GpDRPCw5IZ9mZkUWppTIofA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=N6D0511y1Kyq6/5N6OX5rAAXTg98vryDZmHiPtb9/LQZNL3XT7T+jFPJpcWYuHie9\n\tOq+3FSliF/7rwdFbLrGcvAOHlx6cDzJbO4de5XbesH6J0C6CRwMn1/wLz2wiPqfZWw\n\tAExA/UGgxZxTXWn9ZLTfAn5XdnSqLFoC9JgPq9co=","Date":"Sun, 10 Jan 2021 21:25:06 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<X/tUkn1Z6tducIAZ@pendragon.ideasonboard.com>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-10-niklas.soderlund@ragnatech.se>\n\t<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14917,"web_url":"https://patchwork.libcamera.org/comment/14917/","msgid":"<YBp7IJSy2qhYJRPE@bismarck.dyn.berto.se>","date":"2021-02-03T10:29:52","subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your comments.\n\nOn 2021-01-07 17:37:01 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Tue, Dec 29, 2020 at 05:03:16PM +0100, Niklas Söderlund wrote:\n> > Map and unmap the parameters and statistic buffers in the IPA when the\n> > pipeline handler allocates and frees the buffers.\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++++++\n> >  1 file changed, 26 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 95f1b75dc8be5d40..141066c528890c8e 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -138,6 +138,8 @@ private:\n> >  \tImgUDevice imgu1_;\n> >  \tMediaDevice *cio2MediaDev_;\n> >  \tMediaDevice *imguMediaDev_;\n> > +\n> > +\tstd::vector<IPABuffer> ipaBuffers_;\n> >  };\n> >\n> >  IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> > @@ -583,6 +585,23 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> >  \tif (ret < 0)\n> >  \t\treturn ret;\n> >\n> > +\t/* Map buffers to the IPA. */\n> > +\tunsigned int ipaBufferId = 1;\n> > +\n> > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > +\t\tbuffer->setCookie(ipaBufferId++);\n> > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> \n> You can reserve space in ipaBuffers_\n\nI could but I think it is a case of premature optimization as it makes \nthe code harder to understand and as we know this will be redone with \nthe new IPC framework I would like to have it as simple as possible \nuntil then.\n\n> \n> > +\t\t\t\t\t.planes = buffer->planes() });\n> > +\t}\n> > +\n> > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> > +\t\tbuffer->setCookie(ipaBufferId++);\n> > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> > +\t\t\t\t\t.planes = buffer->planes() });\n> > +\t}\n> > +\n> > +\tdata->ipa_->mapBuffers(ipaBuffers_);\n> > +\n> >  \treturn 0;\n> >  }\n> >\n> > @@ -590,6 +609,13 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> >  {\n> >  \tIPU3CameraData *data = cameraData(camera);\n> >\n> > +\tstd::vector<unsigned int> ids;\n> \n> You can reserve(ipaBuffers_.size())\n> \n> > +\tfor (IPABuffer &ipabuf : ipaBuffers_)\n> > +\t\tids.push_back(ipabuf.id);\n> > +\n> > +\tdata->ipa_->unmapBuffers(ids);\n> > +\tipaBuffers_.clear();\n> > +\n> \n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> Thanks\n>   j\n> \n> >  \tdata->imgu_->freeBuffers();\n> >\n> >  \treturn 0;\n> > --\n> > 2.29.2\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","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 DD7C5BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Feb 2021 10:29:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F3D06843F;\n\tWed,  3 Feb 2021 11:29:56 +0100 (CET)","from mail-ej1-x630.google.com (mail-ej1-x630.google.com\n\t[IPv6:2a00:1450:4864:20::630])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 69A1E60305\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Feb 2021 11:29:54 +0100 (CET)","by mail-ej1-x630.google.com with SMTP id w1so34766338ejf.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 03 Feb 2021 02:29:54 -0800 (PST)","from localhost (p4fca2458.dip0.t-ipconnect.de. [79.202.36.88])\n\tby smtp.gmail.com with ESMTPSA id\n\tsb6sm506812ejb.54.2021.02.03.02.29.53\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 03 Feb 2021 02:29:53 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"SzQhMaRb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=0jAM49DaMgQKndvfR8oRpU2SAWybDNUE6Skc2PludpQ=;\n\tb=SzQhMaRb/dp7E4YdowEwCv32QTbN0hyW0MoRYoA4RuTA2+DyiWOriz1hMpT4hG8NPm\n\t+3WU6+Hse43vst03ErTq4cgGnNo2FC+ZHZs5j81XyshQ3SJLYkQ6vbSS+OR42HCItDee\n\tcHt1v/Uk9wQvLdX7trtc+sdCRL1kOvwaQWbVn+bZkVAQZ8fboXhgidx+1GUnDQiOsp9e\n\twiExANpjOh/5QmALOolIi+a0mDBtR9slHOCiR5iTcw4KblvCkADY96ooPwH89r6vP6AB\n\tiT5q/juGHUbzo0vwI3ty7zzpoHDOuOxXAb5wjCt3txm8pWvXMcUxAYz0IaNxaSZQM20h\n\t+gKA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=0jAM49DaMgQKndvfR8oRpU2SAWybDNUE6Skc2PludpQ=;\n\tb=PMxcjvhCYD7HvDzUD5zYLLVS5gn1GiqvRW1Ahfe3juAjnOc7bnOtWhYSd6r29OsL1G\n\tMy39WArWy/x141nIW+lw7tOxW3Dc1ADiuaL0qbd/hTz2tu9EE4JymahHPdFu3B2NbTcq\n\trFUp8b8UvB7FAtXp/PWWRf6UPFsvI6lAUGrQrw/fJCKEFmqNQ7RwjhmlUgo2oy5ZWA7W\n\tS0j56T2Q0vEQ1OHDwy8n8/Yl6Pn50NR6rCBkN9vsYC21uEapPufLXGPvd4QgBCSoGt7N\n\trkzhNXFZGb7i+Tjcn4R+ZwJ44fRfrQ0BBS0Scq/c3PQdlUyQ747HSXVJ3ZBh+ZBdelyy\n\tlCeQ==","X-Gm-Message-State":"AOAM533DDj4JTtiV434ItS1C7hUrggbxZnUJxsKiQRWvVfIY84Cr+Q0t\n\t3ckQWs5UwFJadQshwKDzEmBEgjRkZAF59Yq3","X-Google-Smtp-Source":"ABdhPJxrrgVWEDeOBmz/lxqiZr585zhes7zl5RJ+cZQi+8FrUZF0jITU4eBSwNfZStm/cuCCrVqSfw==","X-Received":"by 2002:a17:906:b50:: with SMTP id\n\tv16mr2383978ejg.298.1612348194013; \n\tWed, 03 Feb 2021 02:29:54 -0800 (PST)","Date":"Wed, 3 Feb 2021 11:29:52 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YBp7IJSy2qhYJRPE@bismarck.dyn.berto.se>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-10-niklas.soderlund@ragnatech.se>\n\t<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","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","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14918,"web_url":"https://patchwork.libcamera.org/comment/14918/","msgid":"<20210203112602.yyplqzsxvxtdtr24@uno.localdomain>","date":"2021-02-03T11:26:02","subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Feb 03, 2021 at 11:29:52AM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your comments.\n>\n> On 2021-01-07 17:37:01 +0100, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Tue, Dec 29, 2020 at 05:03:16PM +0100, Niklas Söderlund wrote:\n> > > Map and unmap the parameters and statistic buffers in the IPA when the\n> > > pipeline handler allocates and frees the buffers.\n> > >\n> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > ---\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++++++\n> > >  1 file changed, 26 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 95f1b75dc8be5d40..141066c528890c8e 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -138,6 +138,8 @@ private:\n> > >  \tImgUDevice imgu1_;\n> > >  \tMediaDevice *cio2MediaDev_;\n> > >  \tMediaDevice *imguMediaDev_;\n> > > +\n> > > +\tstd::vector<IPABuffer> ipaBuffers_;\n> > >  };\n> > >\n> > >  IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> > > @@ -583,6 +585,23 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > >  \tif (ret < 0)\n> > >  \t\treturn ret;\n> > >\n> > > +\t/* Map buffers to the IPA. */\n> > > +\tunsigned int ipaBufferId = 1;\n> > > +\n> > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> >\n> > You can reserve space in ipaBuffers_\n>\n> I could but I think it is a case of premature optimization as it makes\n> the code harder to understand and as we know this will be redone with\n> the new IPC framework I would like to have it as simple as possible\n> until then.\n>\n\nMy understanding is that all it takes is a:\n\n        ipaBuffers_.reserve(imgu->paramBuffers_.size());\n\nbefore the for loop. I might be mistaken as from your words I get it's\nmore complex than what I think it might be.\n\n> >\n> > > +\t\t\t\t\t.planes = buffer->planes() });\n> > > +\t}\n> > > +\n> > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> > > +\t\t\t\t\t.planes = buffer->planes() });\n> > > +\t}\n> > > +\n> > > +\tdata->ipa_->mapBuffers(ipaBuffers_);\n> > > +\n> > >  \treturn 0;\n> > >  }\n> > >\n> > > @@ -590,6 +609,13 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> > >  {\n> > >  \tIPU3CameraData *data = cameraData(camera);\n> > >\n> > > +\tstd::vector<unsigned int> ids;\n> >\n> > You can reserve(ipaBuffers_.size())\n> >\n> > > +\tfor (IPABuffer &ipabuf : ipaBuffers_)\n> > > +\t\tids.push_back(ipabuf.id);\n> > > +\n> > > +\tdata->ipa_->unmapBuffers(ids);\n> > > +\tipaBuffers_.clear();\n> > > +\n> >\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > Thanks\n> >   j\n> >\n> > >  \tdata->imgu_->freeBuffers();\n> > >\n> > >  \treturn 0;\n> > > --\n> > > 2.29.2\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","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 8D28FBD161\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Feb 2021 11:25:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 17FA86843F;\n\tWed,  3 Feb 2021 12:25:43 +0100 (CET)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C8C0C683FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Feb 2021 12:25:41 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 3D6102000C;\n\tWed,  3 Feb 2021 11:25:40 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 3 Feb 2021 12:26:02 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20210203112602.yyplqzsxvxtdtr24@uno.localdomain>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-10-niklas.soderlund@ragnatech.se>\n\t<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>\n\t<YBp7IJSy2qhYJRPE@bismarck.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YBp7IJSy2qhYJRPE@bismarck.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14923,"web_url":"https://patchwork.libcamera.org/comment/14923/","msgid":"<YBqUJC3selTsuRbA@oden.dyn.berto.se>","date":"2021-02-03T12:16:36","subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nOn 2021-02-03 12:26:02 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Wed, Feb 03, 2021 at 11:29:52AM +0100, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for your comments.\n> >\n> > On 2021-01-07 17:37:01 +0100, Jacopo Mondi wrote:\n> > > Hi Niklas,\n> > >\n> > > On Tue, Dec 29, 2020 at 05:03:16PM +0100, Niklas Söderlund wrote:\n> > > > Map and unmap the parameters and statistic buffers in the IPA when the\n> > > > pipeline handler allocates and frees the buffers.\n> > > >\n> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > ---\n> > > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++++++\n> > > >  1 file changed, 26 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > index 95f1b75dc8be5d40..141066c528890c8e 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > @@ -138,6 +138,8 @@ private:\n> > > >  \tImgUDevice imgu1_;\n> > > >  \tMediaDevice *cio2MediaDev_;\n> > > >  \tMediaDevice *imguMediaDev_;\n> > > > +\n> > > > +\tstd::vector<IPABuffer> ipaBuffers_;\n> > > >  };\n> > > >\n> > > >  IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> > > > @@ -583,6 +585,23 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > > >  \tif (ret < 0)\n> > > >  \t\treturn ret;\n> > > >\n> > > > +\t/* Map buffers to the IPA. */\n> > > > +\tunsigned int ipaBufferId = 1;\n> > > > +\n> > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> > >\n> > > You can reserve space in ipaBuffers_\n> >\n> > I could but I think it is a case of premature optimization as it makes\n> > the code harder to understand and as we know this will be redone with\n> > the new IPC framework I would like to have it as simple as possible\n> > until then.\n> >\n> \n> My understanding is that all it takes is a:\n> \n>         ipaBuffers_.reserve(imgu->paramBuffers_.size());\n> \n> before the for loop. I might be mistaken as from your words I get it's\n> more complex than what I think it might be.\n\nI think you are correct, the line you suggest is all that is needed. But \nwhenever I read code that calls reserve() my internal red warning light \nturns on as I think the reserved() is invoked as one wish to directly \naccess the continues memory of the object and not as a minor \noptimization. So to keep the re-workability of this code high I'm not \nkeen to optimize it as-is. I do not feel strongly about it tho and will \nbow to public opinion.\n\n> \n> > >\n> > > > +\t\t\t\t\t.planes = buffer->planes() });\n> > > > +\t}\n> > > > +\n> > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> > > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> > > > +\t\t\t\t\t.planes = buffer->planes() });\n> > > > +\t}\n> > > > +\n> > > > +\tdata->ipa_->mapBuffers(ipaBuffers_);\n> > > > +\n> > > >  \treturn 0;\n> > > >  }\n> > > >\n> > > > @@ -590,6 +609,13 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> > > >  {\n> > > >  \tIPU3CameraData *data = cameraData(camera);\n> > > >\n> > > > +\tstd::vector<unsigned int> ids;\n> > >\n> > > You can reserve(ipaBuffers_.size())\n> > >\n> > > > +\tfor (IPABuffer &ipabuf : ipaBuffers_)\n> > > > +\t\tids.push_back(ipabuf.id);\n> > > > +\n> > > > +\tdata->ipa_->unmapBuffers(ids);\n> > > > +\tipaBuffers_.clear();\n> > > > +\n> > >\n> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > >\n> > > Thanks\n> > >   j\n> > >\n> > > >  \tdata->imgu_->freeBuffers();\n> > > >\n> > > >  \treturn 0;\n> > > > --\n> > > > 2.29.2\n> > > >\n> > > > _______________________________________________\n> > > > libcamera-devel mailing list\n> > > > libcamera-devel@lists.libcamera.org\n> > > > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n> > --\n> > Regards,\n> > Niklas Söderlund","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 36239BD161\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Feb 2021 12:16:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B4B9868446;\n\tWed,  3 Feb 2021 13:16:40 +0100 (CET)","from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com\n\t[IPv6:2a00:1450:4864:20::12f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF972683FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Feb 2021 13:16:39 +0100 (CET)","by mail-lf1-x12f.google.com with SMTP id b2so33030382lfq.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 03 Feb 2021 04:16:39 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tz11sm223381lfd.98.2021.02.03.04.16.37\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 03 Feb 2021 04:16:38 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"CR8GviJE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=Wv2QSeq34h87+4+IaCaPBwnwW9BdtzrwkOfFwJS50Iw=;\n\tb=CR8GviJE4tFqX6iEU8+ta8q4j1pOg64O9ESaUQVi0hKOnZxtI15mcBXCrTLXpz2wf3\n\tdAUKPIC/eBY6d6NDQc8H8C1RrprCfumC4rDBCknYGlOxGcMFOd39vFSIS52YVCmPLLHw\n\t12i0kcuCBZaQrj9Bz0lwzLWBONpaLHWmgxx5q4bzb+GheW+BAx2xBjL4vtqEV6Ga+I0w\n\tQCtZE+HiA4iufGCwblSNCA4XYWxjnGskz4ncnW+s9yrKPXrRbicyKFRxxODgO94JRz6K\n\tDq/t84iHxddld8VULmTFRCb+Nn4U4WsM3x0R2jR+7QRFHVv3rgKavOwE3Ue3KopacnS4\n\t4eaQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=Wv2QSeq34h87+4+IaCaPBwnwW9BdtzrwkOfFwJS50Iw=;\n\tb=TThmnt00MuiPmbeIzzehAuLKnC1dnjDEK90BzUibtdR5lrguXRQ5zD/9Bzh/GTL/nL\n\tDFP2jVszPT74YlDO8oEvW2MzT2zti+xMFQZa83pBKAKIzqXL40OsjmyHFdLS3Ao3TQaR\n\tXBR9IsCihKTGaropbTSuOBkleTeyX6Ilaa1zMxeuiSWCWBdx+on3h1BfHx1o8TuWz1en\n\tkwUwQ2u3mZfwPBO/tymS1WrTfqeCnGVdJfV1M8Lqmho8YMwjunZCh6fEMLr00F98hnsk\n\t+vSRcJjLg5navOlYr1RDpS9vpulYYSruHhpPSQz5EnNtYwEHU4WUGuSXdqP7OrltrfXO\n\tbXhA==","X-Gm-Message-State":"AOAM530igLjW/tSkSOrsETLZmhoOhe6URVItciwQtEwnLX5XJeiKg6bC\n\tUBHOMJjXvqMJrUQrrtwmK5Ag7w==","X-Google-Smtp-Source":"ABdhPJzjtDkxwfJ9GdslS0Cp9spTrrfjcVRVbKzea1Uw42Q7vun/DyYiMCDCkhZ8Dy64JLvHFe2m1w==","X-Received":"by 2002:a19:40d4:: with SMTP id\n\tn203mr1696061lfa.350.1612354599038; \n\tWed, 03 Feb 2021 04:16:39 -0800 (PST)","Date":"Wed, 3 Feb 2021 13:16:36 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YBqUJC3selTsuRbA@oden.dyn.berto.se>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-10-niklas.soderlund@ragnatech.se>\n\t<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>\n\t<YBp7IJSy2qhYJRPE@bismarck.dyn.berto.se>\n\t<20210203112602.yyplqzsxvxtdtr24@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210203112602.yyplqzsxvxtdtr24@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","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","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14925,"web_url":"https://patchwork.libcamera.org/comment/14925/","msgid":"<20210203130919.k3enqfqegbwvujxk@uno.localdomain>","date":"2021-02-03T13:09:19","subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Feb 03, 2021 at 01:16:36PM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> On 2021-02-03 12:26:02 +0100, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Wed, Feb 03, 2021 at 11:29:52AM +0100, Niklas Söderlund wrote:\n> > > Hi Jacopo,\n> > >\n> > > Thanks for your comments.\n> > >\n> > > On 2021-01-07 17:37:01 +0100, Jacopo Mondi wrote:\n> > > > Hi Niklas,\n> > > >\n> > > > On Tue, Dec 29, 2020 at 05:03:16PM +0100, Niklas Söderlund wrote:\n> > > > > Map and unmap the parameters and statistic buffers in the IPA when the\n> > > > > pipeline handler allocates and frees the buffers.\n> > > > >\n> > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > > ---\n> > > > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++++++++++++++\n> > > > >  1 file changed, 26 insertions(+)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > index 95f1b75dc8be5d40..141066c528890c8e 100644\n> > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > @@ -138,6 +138,8 @@ private:\n> > > > >  \tImgUDevice imgu1_;\n> > > > >  \tMediaDevice *cio2MediaDev_;\n> > > > >  \tMediaDevice *imguMediaDev_;\n> > > > > +\n> > > > > +\tstd::vector<IPABuffer> ipaBuffers_;\n> > > > >  };\n> > > > >\n> > > > >  IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> > > > > @@ -583,6 +585,23 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > > > >  \tif (ret < 0)\n> > > > >  \t\treturn ret;\n> > > > >\n> > > > > +\t/* Map buffers to the IPA. */\n> > > > > +\tunsigned int ipaBufferId = 1;\n> > > > > +\n> > > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > > > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> > > >\n> > > > You can reserve space in ipaBuffers_\n> > >\n> > > I could but I think it is a case of premature optimization as it makes\n> > > the code harder to understand and as we know this will be redone with\n> > > the new IPC framework I would like to have it as simple as possible\n> > > until then.\n> > >\n> >\n> > My understanding is that all it takes is a:\n> >\n> >         ipaBuffers_.reserve(imgu->paramBuffers_.size());\n> >\n> > before the for loop. I might be mistaken as from your words I get it's\n> > more complex than what I think it might be.\n>\n> I think you are correct, the line you suggest is all that is needed. But\n> whenever I read code that calls reserve() my internal red warning light\n> turns on as I think the reserved() is invoked as one wish to directly\n> access the continues memory of the object and not as a minor\n> optimization. So to keep the re-workability of this code high I'm not\n> keen to optimize it as-is. I do not feel strongly about it tho and will\n> bow to public opinion.\n>\n\nNot sure I got entirely what you mean, but reserve() it's there mainly\nto avoid relocations while adding elements to a vector of a known\nsize.\n\nUp to you\n\n> >\n> > > >\n> > > > > +\t\t\t\t\t.planes = buffer->planes() });\n> > > > > +\t}\n> > > > > +\n> > > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> > > > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > +\t\tipaBuffers_.push_back({ .id = buffer->cookie(),\n> > > > > +\t\t\t\t\t.planes = buffer->planes() });\n> > > > > +\t}\n> > > > > +\n> > > > > +\tdata->ipa_->mapBuffers(ipaBuffers_);\n> > > > > +\n> > > > >  \treturn 0;\n> > > > >  }\n> > > > >\n> > > > > @@ -590,6 +609,13 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> > > > >  {\n> > > > >  \tIPU3CameraData *data = cameraData(camera);\n> > > > >\n> > > > > +\tstd::vector<unsigned int> ids;\n> > > >\n> > > > You can reserve(ipaBuffers_.size())\n> > > >\n> > > > > +\tfor (IPABuffer &ipabuf : ipaBuffers_)\n> > > > > +\t\tids.push_back(ipabuf.id);\n> > > > > +\n> > > > > +\tdata->ipa_->unmapBuffers(ids);\n> > > > > +\tipaBuffers_.clear();\n> > > > > +\n> > > >\n> > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > >\n> > > > Thanks\n> > > >   j\n> > > >\n> > > > >  \tdata->imgu_->freeBuffers();\n> > > > >\n> > > > >  \treturn 0;\n> > > > > --\n> > > > > 2.29.2\n> > > > >\n> > > > > _______________________________________________\n> > > > > libcamera-devel mailing list\n> > > > > libcamera-devel@lists.libcamera.org\n> > > > > https://lists.libcamera.org/listinfo/libcamera-devel\n> > >\n> > > --\n> > > Regards,\n> > > Niklas Söderlund\n>\n> --\n> Regards,\n> Niklas Söderlund","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 F1B7CBD161\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Feb 2021 13:09:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E5BB68445;\n\tWed,  3 Feb 2021 14:09:00 +0100 (CET)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2BD80683FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Feb 2021 14:08:59 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 9661D240013;\n\tWed,  3 Feb 2021 13:08:58 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 3 Feb 2021 14:09:19 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20210203130919.k3enqfqegbwvujxk@uno.localdomain>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-10-niklas.soderlund@ragnatech.se>\n\t<20210107163701.ssjs5hcien6nw7tu@uno.localdomain>\n\t<YBp7IJSy2qhYJRPE@bismarck.dyn.berto.se>\n\t<20210203112602.yyplqzsxvxtdtr24@uno.localdomain>\n\t<YBqUJC3selTsuRbA@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YBqUJC3selTsuRbA@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v2 09/11] libcamera: ipu3: Map buffers\n\tin IPA","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]