[{"id":16071,"web_url":"https://patchwork.libcamera.org/comment/16071/","msgid":"","date":"2021-03-31T08:51:48","subject":"Re: [libcamera-devel] [PATCH] pipeline: ipu3: Store requests in the\n\tcase a buffer shortage","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"I should still work for this patch series.\nThe missing part is to trigger queuePendingRequests() appropriately\nwhen buffers are available.\nI couldn't find a good way as availableBuffers notification functions\nare separated and trying queuePendingRequests() is not likely\nsuccessful and thus inefficient.\nI would like to ask anyone for a good idea for this.\n\nThanks in advance,\n-Hiro\n\nOn Wed, Mar 31, 2021 at 5:45 PM Hirokazu Honda wrote:\n>\n> PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a\n> request when there are not sufficient buffers for the request.\n> Since the request will be successful if it is queued later when\n> enough buffers are available. The requests failed due to a buffer\n> shortage should be stored and retried later in the FIFO order.\n> This introduces the queue in PipelineHandlerIPU3 to do that.\n>\n> Signed-off-by: Hirokazu Honda \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 62 ++++++++++++++++++----------\n> 1 file changed, 41 insertions(+), 21 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 519cad4f..71dd311f 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -153,12 +153,16 @@ private:\n> int allocateBuffers(Camera *camera);\n> int freeBuffers(Camera *camera);\n>\n> + int queuePendingRequests();\n> +\n> ImgUDevice imgu0_;\n> ImgUDevice imgu1_;\n> MediaDevice *cio2MediaDev_;\n> MediaDevice *imguMediaDev_;\n>\n> std::vector ipaBuffers_;\n> +\n> + std::queue> pendingRequests_;\n> };\n>\n> IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> IPU3CameraData *data = cameraData(camera);\n> int ret = 0;\n>\n> + pendingRequests_ = {};\n> +\n> data->ipa_->stop();\n>\n> ret |= data->imgu_->stop();\n> @@ -774,36 +780,50 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> freeBuffers(camera);\n> }\n>\n> -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> +int PipelineHandlerIPU3::queuePendingRequests()\n> {\n> - IPU3CameraData *data = cameraData(camera);\n> + while (!pendingRequests_.empty()) {\n> + IPU3CameraData *data = pendingRequests_.front().first;\n> + Request *request = pendingRequests_.front().second;\n>\n> - IPU3Frames::Info *info = data->frameInfos_.create(request);\n> - if (!info)\n> - return -ENOBUFS;\n> + IPU3Frames::Info *info = data->frameInfos_.create(request);\n> + if (!info)\n> + break;\n>\n> - /*\n> - * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> - * the request, if any, or a CIO2 internal buffer otherwise.\n> - */\n> - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_);\n> - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer);\n> - if (!rawBuffer) {\n> - data->frameInfos_.remove(info);\n> - return -ENOMEM;\n> - }\n> + /*\n> + * Queue a buffer on the CIO2, using the raw stream buffer\n> + * provided in the request, if any, or a CIO2 internal buffer\n> + * otherwise.\n> + */\n> + FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_);\n> + FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer);\n> + if (!rawBuffer) {\n> + data->frameInfos_.remove(info);\n> + break;\n> + }\n>\n> - info->rawBuffer = rawBuffer;\n> + info->rawBuffer = rawBuffer;\n>\n> - ipa::ipu3::IPU3Event ev;\n> - ev.op = ipa::ipu3::EventProcessControls;\n> - ev.frame = info->id;\n> - ev.controls = request->controls();\n> - data->ipa_->processEvent(ev);\n> + ipa::ipu3::IPU3Event ev;\n> + ev.op = ipa::ipu3::EventProcessControls;\n> + ev.frame = info->id;\n> + ev.controls = request->controls();\n> + data->ipa_->processEvent(ev);\n> +\n> + pendingRequests_.pop();\n> + }\n>\n> return 0;\n> }\n>\n> +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> +{\n> + IPU3CameraData *data = cameraData(camera);\n> +\n> + pendingRequests_.emplace(data, request);\n> + return queuePendingRequests();\n> +}\n> +\n> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> {\n> int ret;\n> --\n> 2.31.0.291.g576ba9dcdaf-goog\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 87410C0DA4\n\tfor ;\n\tWed, 31 Mar 2021 08:52:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECB6868781;\n\tWed, 31 Mar 2021 10:51:59 +0200 (CEST)","from mail-ej1-x631.google.com (mail-ej1-x631.google.com\n\t[IPv6:2a00:1450:4864:20::631])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E364A602D7\n\tfor ;\n\tWed, 31 Mar 2021 10:51:58 +0200 (CEST)","by mail-ej1-x631.google.com with SMTP id ce10so28895479ejb.6\n\tfor ;\n\tWed, 31 Mar 2021 01:51:58 -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=\"JwZEQQiN\"; 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\tbh=RJXC7ZqiV7NjemPzSiBBRyERaaLXKr8VM0kBc0CKeAg=;\n\tb=JwZEQQiNtIg1osaluQO9TYUU3PYs5KkJJfwvT419MAiOQjDkS0CJQHKdzRKsecDIrC\n\tC15xJB+wyWmaQZt/CASCYbtiISBz1MrJlCHE0cLAQ0auYDdPK3P9qBaI+i5bZ1Zim2Ek\n\tTEahKUDnfPkKoL37ZhqiD7hexjT2B05N0qojQ=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=RJXC7ZqiV7NjemPzSiBBRyERaaLXKr8VM0kBc0CKeAg=;\n\tb=A2UFxW1yEvC9U5dh9VJxjPMhTa/gjIQ4kPLMjBARXr8wbtWyJ4xlZ6Te/BFy3wQWc/\n\tWUh1/GEhy+RYY3wxHIpACRwxrDMCPtXIgPM7BC2cqQxzAsDJmllNpgVGOV9ZivvYni+G\n\t6eLMb7az4hjZnGBUNcBNZNjHwsrZi0ij4pNVDgNdY+8vVHYiKBSNfqrb7/Ns8fJbHVio\n\trNmy0s45qx5IfbNxOBlwmDA4MLwdm6o9bv4Ur4gzm7NZHg8aNEZdU6eIeLgeLWOoVRvP\n\tWyiOwmQ1mNYn0qL+XWV0/HcRpefXS68g3xJKrCMJ1YRQC8NWIolDpZxcVxeGszZ7mVhT\n\t5Rnw==","X-Gm-Message-State":"AOAM530S7HWxfMdV79XkQjRSFdbfSvT+poshqnHyulYd8wMqViGbR5a6\n\tGFsFgVuOil4pAnTpR2mtmTdEpZM+xC9hdWSQVYNFzME4OHk=","X-Google-Smtp-Source":"ABdhPJwZphwYk4u+ahZrvxcc/djwbiYtqhD5hzA8Dp8O28lbpfqFJYfvYXvJrGgOPK6O3Q/23iN4Qk89o/DiD7mWCYM=","X-Received":"by 2002:a17:906:701:: with SMTP id\n\ty1mr2305273ejb.243.1617180718406; \n\tWed, 31 Mar 2021 01:51:58 -0700 (PDT)","MIME-Version":"1.0","References":"<20210331084539.930233-1-hiroh@chromium.org>","In-Reply-To":"<20210331084539.930233-1-hiroh@chromium.org>","From":"Hirokazu Honda ","Date":"Wed, 31 Mar 2021 17:51:48 +0900","Message-ID":"","To":"libcamera devel ","Subject":"Re: [libcamera-devel] [PATCH] pipeline: ipu3: Store requests in the\n\tcase a buffer shortage","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16104,"web_url":"https://patchwork.libcamera.org/comment/16104/","msgid":"","date":"2021-04-03T02:08:53","subject":"Re: [libcamera-devel] [PATCH] pipeline: ipu3: Store requests in the\n\tcase a buffer shortage","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nOn Wed, Mar 31, 2021 at 05:51:48PM +0900, Hirokazu Honda wrote:\n> I should still work for this patch series.\n> The missing part is to trigger queuePendingRequests() appropriately\n> when buffers are available.\n> I couldn't find a good way as availableBuffers notification functions\n> are separated and trying queuePendingRequests() is not likely\n> successful and thus inefficient.\n> I would like to ask anyone for a good idea for this.\n\nRequest queuing to the kernel can fail if we are out of either ImgU\nstats/params buffers, or internal CIO2 raw buffers. New buffers become\navailable for the ImgU in IPU3Frames::remove(), and for the CIO2 in\nCIO2Device::tryReturnBuffer(). Both could be modified to emit a signal,\nwhich could be connected to queuePendingRequests().\n\n> On Wed, Mar 31, 2021 at 5:45 PM Hirokazu Honda wrote:\n> >\n> > PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a\n> > request when there are not sufficient buffers for the request.\n> > Since the request will be successful if it is queued later when\n> > enough buffers are available. The requests failed due to a buffer\n> > shortage should be stored and retried later in the FIFO order.\n> > This introduces the queue in PipelineHandlerIPU3 to do that.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 62 ++++++++++++++++++----------\n> > 1 file changed, 41 insertions(+), 21 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 519cad4f..71dd311f 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -153,12 +153,16 @@ private:\n> > int allocateBuffers(Camera *camera);\n> > int freeBuffers(Camera *camera);\n> >\n> > + int queuePendingRequests();\n> > +\n> > ImgUDevice imgu0_;\n> > ImgUDevice imgu1_;\n> > MediaDevice *cio2MediaDev_;\n> > MediaDevice *imguMediaDev_;\n> >\n> > std::vector ipaBuffers_;\n> > +\n> > + std::queue> pendingRequests_;\n\nAs requests are per-camera, shouldn't the queue be stored in\nIPU3CameraData ? queuePendingRequests() should also be moved to the\nIPU3CameraData class.\n\n> > };\n> >\n> > IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> > @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> > IPU3CameraData *data = cameraData(camera);\n> > int ret = 0;\n> >\n> > + pendingRequests_ = {};\n> > +\n> > data->ipa_->stop();\n> >\n> > ret |= data->imgu_->stop();\n> > @@ -774,36 +780,50 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> > freeBuffers(camera);\n> > }\n> >\n> > -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> > +int PipelineHandlerIPU3::queuePendingRequests()\n> > {\n> > - IPU3CameraData *data = cameraData(camera);\n> > + while (!pendingRequests_.empty()) {\n> > + IPU3CameraData *data = pendingRequests_.front().first;\n> > + Request *request = pendingRequests_.front().second;\n> >\n> > - IPU3Frames::Info *info = data->frameInfos_.create(request);\n> > - if (!info)\n> > - return -ENOBUFS;\n> > + IPU3Frames::Info *info = data->frameInfos_.create(request);\n> > + if (!info)\n> > + break;\n> >\n> > - /*\n> > - * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> > - * the request, if any, or a CIO2 internal buffer otherwise.\n> > - */\n> > - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_);\n> > - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer);\n> > - if (!rawBuffer) {\n> > - data->frameInfos_.remove(info);\n> > - return -ENOMEM;\n> > - }\n> > + /*\n> > + * Queue a buffer on the CIO2, using the raw stream buffer\n> > + * provided in the request, if any, or a CIO2 internal buffer\n> > + * otherwise.\n> > + */\n> > + FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_);\n> > + FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer);\n> > + if (!rawBuffer) {\n> > + data->frameInfos_.remove(info);\n> > + break;\n> > + }\n> >\n> > - info->rawBuffer = rawBuffer;\n> > + info->rawBuffer = rawBuffer;\n> >\n> > - ipa::ipu3::IPU3Event ev;\n> > - ev.op = ipa::ipu3::EventProcessControls;\n> > - ev.frame = info->id;\n> > - ev.controls = request->controls();\n> > - data->ipa_->processEvent(ev);\n> > + ipa::ipu3::IPU3Event ev;\n> > + ev.op = ipa::ipu3::EventProcessControls;\n> > + ev.frame = info->id;\n> > + ev.controls = request->controls();\n> > + data->ipa_->processEvent(ev);\n> > +\n> > + pendingRequests_.pop();\n> > + }\n> >\n> > return 0;\n> > }\n> >\n> > +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> > +{\n> > + IPU3CameraData *data = cameraData(camera);\n> > +\n> > + pendingRequests_.emplace(data, request);\n> > + return queuePendingRequests();\n> > +}\n> > +\n> > bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > {\n> > int ret;","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 10610C0DA3\n\tfor ;\n\tSat, 3 Apr 2021 02:09:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 89CEA68786;\n\tSat, 3 Apr 2021 04:09:39 +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 5DAAA602CF\n\tfor ;\n\tSat, 3 Apr 2021 04:09:38 +0200 (CEST)","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 C55CE3D7;\n\tSat, 3 Apr 2021 04:09:37 +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=\"TOAz0t7H\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1617415778;\n\tbh=eznGQat6+NwtjOHzae7W49U/8jF3qAG1YfjrdkAPQDY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TOAz0t7HnGTQDP44lpM8I6/ZK8ml7loWIHPRF9a7fDU3wFvyD1JnlKPyHMsYNdjIL\n\tOzWFUTzXZCBttJU2oNiJna0x8vLBKgJS7yjhWC2UOA4rai+eukaCfRWVshThS4gZ0K\n\tqTA3XBK7gZFMLUN9LhlUxUeUMNs7Z+PGgJDvspcM=","Date":"Sat, 3 Apr 2021 05:08:53 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210331084539.930233-1-hiroh@chromium.org>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: ipu3: Store requests in the\n\tcase a buffer shortage","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]