[{"id":18884,"web_url":"https://patchwork.libcamera.org/comment/18884/","msgid":"","date":"2021-08-17T23:58:54","subject":"Re: [libcamera-devel] [RFC PATCH 05/10] ipa: rkisp1: Use offset in\n\tmapping IPABuffer","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 Mon, Aug 16, 2021 at 01:31:33PM +0900, Hirokazu Honda wrote:\n> IPABuffer is represented by FrameBuffer. FrameBuffer::Plane has\n> now an offset. This uses the offset variable to map the IPABuffer.\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> src/ipa/rkisp1/rkisp1.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 06fb9640..eb7a2705 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -165,7 +165,7 @@ void IPARkISP1::mapBuffers(const std::vector &buffers)\n> \t\t * to applications).\n> \t\t */\n> \t\tbuffersMemory_[buffer.id] = mmap(NULL,\n> -\t\t\t\t\t\t fb.planes()[0].length,\n> +\t\t\t\t\t\t fb.planes()[0].offset + fb.planes()[0].length,\n\nIs this right ? You're mapping offset + length bytes at offset 0 from\nthe beginning of the dmabuf. The munmap() call is updated accordingly,\nbut the code that uses the mapped memory isn't, and will look at offset\n0. Now, as those buffers, are used for statistics and ISP parameters,\nthe offset will always be zero, but the code isn't really correct.\n\nHow about updating the RkISP1 IPA to use MappedFrameBuffer instead, like\nthe other IPA modules ?\n\n> \t\t\t\t\t\t PROT_READ | PROT_WRITE,\n> \t\t\t\t\t\t MAP_SHARED,\n> \t\t\t\t\t\t fb.planes()[0].fd.fd(),\n> @@ -186,7 +186,7 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t\tif (fb == buffers_.end())\n> \t\t\tcontinue;\n> \n> -\t\tmunmap(buffersMemory_[id], fb->second.planes()[0].length);\n> +\t\tmunmap(buffersMemory_[id], fb->second.planes()[0].offset + fb->second.planes()[0].length);\n> \t\tbuffersMemory_.erase(id);\n> \t\tbuffers_.erase(id);\n> \t}","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 1DF8CBD87D\n\tfor ;\n\tTue, 17 Aug 2021 23:59:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DCD768894;\n\tWed, 18 Aug 2021 01:59:02 +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 CBA316025D\n\tfor ;\n\tWed, 18 Aug 2021 01:59:01 +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 42DFE499;\n\tWed, 18 Aug 2021 01:59:01 +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=\"JwqYTBfo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629244741;\n\tbh=Wj03iZyTgQHuvaSm40Oepx57CZ+zR1LaJ6odc/8uLCY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JwqYTBfodUAkHkWaR+XmM8QI1wUU9ShwxwrLqwP9LLTgbi9bG74Z27fwIkzFHlJFI\n\tWiYX15WNg+BmV9NVDgteufVp7obIts7wkDZjYKJhJqNgh8wi3LsrN7Uy7lJD6Io3oB\n\tsgVScjUC9w4xZgQUugdVMtZa5/21gI1LkdwTedS8=","Date":"Wed, 18 Aug 2021 02:58:54 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210816043138.957984-1-hiroh@chromium.org>\n\t<20210816043138.957984-6-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210816043138.957984-6-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 05/10] ipa: rkisp1: Use offset in\n\tmapping IPABuffer","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\" "}}]