[{"id":17156,"web_url":"https://patchwork.libcamera.org/comment/17156/","msgid":"","date":"2021-05-24T02:52:23","subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: ipu3: Store requests\n\tin the case 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\nThank you for the patch.\n\nOn Thu, May 13, 2021 at 11:29:45AM +0900, Hirokazu Honda wrote:\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 IPU3CameraData to do that.\n> \n> Signed-off-by: Hirokazu Honda \n> Reviewed-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/pipeline/ipu3/cio2.cpp | 2 +-\n> src/libcamera/pipeline/ipu3/frames.cpp | 4 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 80 +++++++++++++++++++-------\n> 3 files changed, 63 insertions(+), 23 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 3cd777d1..8bbef174 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)\n> \t/* If no buffer is provided in the request, use an internal one. */\n> \tif (!buffer) {\n> \t\tif (availableBuffers_.empty()) {\n> -\t\t\tLOG(IPU3, Error) << \"CIO2 buffer underrun\";\n> +\t\t\tLOG(IPU3, Debug) << \"CIO2 buffer underrun\";\n> \t\t\treturn nullptr;\n> \t\t}\n> \n> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n> index d0f55ab9..29d9aafc 100644\n> --- a/src/libcamera/pipeline/ipu3/frames.cpp\n> +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n> \tunsigned int id = request->sequence();\n> \n> \tif (availableParamBuffers_.empty()) {\n> -\t\tLOG(IPU3, Error) << \"Parameters buffer underrun\";\n> +\t\tLOG(IPU3, Debug) << \"Parameters buffer underrun\";\n> \t\treturn nullptr;\n> \t}\n> \n> \tif (availableStatBuffers_.empty()) {\n> -\t\tLOG(IPU3, Error) << \"Statistics buffer underrun\";\n> +\t\tLOG(IPU3, Debug) << \"Statistics buffer underrun\";\n> \t\treturn nullptr;\n> \t}\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ade8ffbd..6961d498 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -66,6 +66,8 @@ public:\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> \tvoid paramBufferReady(FrameBuffer *buffer);\n> \tvoid statBufferReady(FrameBuffer *buffer);\n> +\tvoid queuePendingRequests();\n> +\tvoid cancelPendingRequests();\n> \n> \tCIO2Device cio2_;\n> \tImgUDevice *imgu_;\n> @@ -84,6 +86,8 @@ public:\n> \n> \tstd::unique_ptr ipa_;\n> \n> +\tstd::queue pendingRequests_;\n> +\n> private:\n> \tvoid queueFrameAction(unsigned int id,\n> \t\t\t const ipa::ipu3::IPU3Action &action);\n> @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> \tIPU3CameraData *data = cameraData(camera);\n> \tint ret = 0;\n> \n> +\tdata->cancelPendingRequests();\n> +\n> \tdata->ipa_->stop();\n> \n> \tret |= data->imgu_->stop();\n> @@ -774,32 +780,66 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> \tfreeBuffers(camera);\n> }\n> \n> -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> +void IPU3CameraData::cancelPendingRequests()\n> {\n> -\tIPU3CameraData *data = cameraData(camera);\n> +\twhile (!pendingRequests_.empty()) {\n> +\t\tRequest *request = pendingRequests_.front();\n> \n> -\tIPU3Frames::Info *info = data->frameInfos_.create(request);\n> -\tif (!info)\n> -\t\treturn -ENOBUFS;\n> +\t\tfor (auto it : request->buffers()) {\n> +\t\t\tFrameBuffer *buffer = it.second;\n> +\t\t\tbuffer->cancel();\n> +\t\t\tpipe_->completeBuffer(request, buffer);\n> +\t\t}\n> \n> -\t/*\n> -\t * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> -\t * the request, if any, or a CIO2 internal buffer otherwise.\n> -\t */\n> -\tFrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_);\n> -\tFrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer);\n> -\tif (!rawBuffer) {\n> -\t\tdata->frameInfos_.remove(info);\n> -\t\treturn -ENOMEM;\n> +\t\tpipe_->completeRequest(request);\n> +\t\tpendingRequests_.pop();\n> \t}\n> +}\n> \n> -\tinfo->rawBuffer = rawBuffer;\n> +void IPU3CameraData::queuePendingRequests()\n> +{\n> +\twhile (!pendingRequests_.empty()) {\n> +\t\tRequest *request = pendingRequests_.front();\n> \n> -\tipa::ipu3::IPU3Event ev;\n> -\tev.op = ipa::ipu3::EventProcessControls;\n> -\tev.frame = info->id;\n> -\tev.controls = request->controls();\n> -\tdata->ipa_->processEvent(ev);\n> +\t\tIPU3Frames::Info *info = frameInfos_.create(request);\n> +\t\tif (!info)\n> +\t\t\tbreak;\n> +\n> +\t\t/*\n> +\t\t * Queue a buffer on the CIO2, using the raw stream buffer\n> +\t\t * provided in the request, if any, or a CIO2 internal buffer\n> +\t\t * otherwise.\n> +\t\t */\n> +\t\tFrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_);\n> +\t\tFrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer);\n> +\t\t/*\n> +\t\t * \\todo If queueBuffer fails in queuing a buffer to the device,\n> +\t\t * report the request as error by cancelling the request and\n> +\t\t * calling PipelineHandler::completeRequest().\n> +\t\t */\n> +\t\tif (!rawBuffer) {\n> +\t\t\tframeInfos_.remove(info);\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tinfo->rawBuffer = rawBuffer;\n> +\n> +\t\tipa::ipu3::IPU3Event ev;\n> +\t\tev.op = ipa::ipu3::EventProcessControls;\n> +\t\tev.frame = info->id;\n> +\t\tev.controls = request->controls();\n> +\t\tipa_->processEvent(ev);\n> +\n> +\t\tpendingRequests_.pop();\n> +\t}\n> +}\n> +\n> +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> +{\n> +\tIPU3CameraData *data = cameraData(camera);\n> +\n> +\tdata->pendingRequests_.push(request);\n> +\tdata->queuePendingRequests();\n> \n> \treturn 0;\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 B3E26C3201\n\tfor ;\n\tMon, 24 May 2021 02:52:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 132FE6891B;\n\tMon, 24 May 2021 04:52:29 +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 3D825602AD\n\tfor ;\n\tMon, 24 May 2021 04:52:27 +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 A36ED476;\n\tMon, 24 May 2021 04:52:26 +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=\"Cn91PVPX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621824746;\n\tbh=JdrkQ9BB5PsviGlSzPDyVozLq4eE5BCOl7GyvfoY07I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Cn91PVPXuejXLt7+WN0mfQuTdKOiSsykuA3vChFBCM87D1BhylLNqc21AgTL5Brtz\n\tZhXnezSqY54efIayv4T9nOOR4wD8EA9qBEbANNaail4Kmqsz7AYe2Ny/939iac1Pi7\n\tBgpc4Q0DvIn6sc+5PIcAA1MSB7C03gJSc+g4geUE=","Date":"Mon, 24 May 2021 05:52:23 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210513022946.2194341-1-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210513022946.2194341-1-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v5 1/2] pipeline: ipu3: Store requests\n\tin the case 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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]