[{"id":27619,"web_url":"https://patchwork.libcamera.org/comment/27619/","msgid":"<6njl7zajtsvo6me6op5thov7t4tincu6jnfhdkrqvxasoeaeha@k5ienyoenuhn>","date":"2023-07-27T09:33:42","subject":"Re: [libcamera-devel] [PATCH v2 2/4] pipeline: rpi: Remove\n\tadditional external dma buf handling logic","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, Jul 25, 2023 at 09:55:38AM +0100, Naushir Patuck via libcamera-devel wrote:\n> There is no need to distinguish between dma bufs allocated outside of\n> libcamera and internally allocated buffers. As such, remove all the\n> special case handling of such buffers.\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n> ---\n>  .../pipeline/rpi/common/pipeline_base.cpp        | 16 ----------------\n>  .../pipeline/rpi/common/pipeline_base.h          |  1 -\n>  src/libcamera/pipeline/rpi/common/rpi_stream.cpp | 11 +----------\n>  src/libcamera/pipeline/rpi/common/rpi_stream.h   |  2 --\n>  4 files changed, 1 insertion(+), 29 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 179a5b81a516..f244edc68a85 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1391,11 +1391,6 @@ void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n>  \t */\n>  \tRequest *request = requestQueue_.empty() ? nullptr : requestQueue_.front();\n>  \tif (!dropFrameCount_ && request && request->findBuffer(stream) == buffer) {\n> -\t\t/*\n> -\t\t * Check if this is an externally provided buffer, and if\n> -\t\t * so, we must stop tracking it in the pipeline handler.\n> -\t\t */\n> -\t\thandleExternalBuffer(buffer, stream);\n>  \t\t/*\n>  \t\t * Tag the buffer as completed, returning it to the\n>  \t\t * application.\n> @@ -1435,17 +1430,6 @@ void CameraData::handleState()\n>  \t}\n>  }\n>\n> -void CameraData::handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n> -{\n> -\tunsigned int id = stream->getBufferId(buffer);\n> -\n> -\tif (!(id & MaskExternalBuffer))\n> -\t\treturn;\n> -\n> -\t/* Stop the Stream object from tracking the buffer. */\n> -\tstream->removeExternalBuffer(buffer);\n> -}\n> -\n>  void CameraData::checkRequestCompleted()\n>  {\n>  \tbool requestCompleted = false;\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index f648e81054bb..8ee20a1b6d9b 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -193,7 +193,6 @@ protected:\n>  \tunsigned int ispOutputTotal_;\n>\n>  private:\n> -\tvoid handleExternalBuffer(FrameBuffer *buffer, Stream *stream);\n>  \tvoid checkRequestCompleted();\n>  };\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> index e9ad1e6f0848..74b5abf447c7 100644\n> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> @@ -78,16 +78,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const\n>\n>  void Stream::setExternalBuffer(FrameBuffer *buffer)\n>  {\n> -\tbufferMap_.emplace(BufferMask::MaskExternalBuffer | id_.get(), buffer);\n> -}\n> -\n> -void Stream::removeExternalBuffer(FrameBuffer *buffer)\n> -{\n> -\tunsigned int id = getBufferId(buffer);\n> -\n> -\t/* Ensure we have this buffer in the stream, and it is marked external. */\n> -\tASSERT(id & BufferMask::MaskExternalBuffer);\n> -\tbufferMap_.erase(id);\n> +\tbufferMap_.emplace(id_.get(), buffer);\n>  }\n>\n>  int Stream::prepareBuffers(unsigned int count)\n> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> index 6edd304bdfe2..ca591f99cc45 100644\n> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> @@ -28,7 +28,6 @@ enum BufferMask {\n>  \tMaskStats\t\t= 0x010000,\n>  \tMaskEmbeddedData\t= 0x020000,\n>  \tMaskBayerData\t\t= 0x040000,\n> -\tMaskExternalBuffer\t= 0x100000,\n>  };\n>\n>  /*\n> @@ -78,7 +77,6 @@ public:\n>  \tunsigned int getBufferId(FrameBuffer *buffer) const;\n>\n>  \tvoid setExternalBuffer(FrameBuffer *buffer);\n> -\tvoid removeExternalBuffer(FrameBuffer *buffer);\n>\n>  \tint prepareBuffers(unsigned int count);\n>  \tint queueBuffer(FrameBuffer *buffer);\n> --\n> 2.34.1\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 D6856BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 Jul 2023 09:33:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5360F628BC;\n\tThu, 27 Jul 2023 11:33:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 339B861E24\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 Jul 2023 11:33:46 +0200 (CEST)","from ideasonboard.com (mob-5-91-19-250.net.vodafone.it\n\t[5.91.19.250])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 74E8A4A9;\n\tThu, 27 Jul 2023 11:32:46 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1690450428;\n\tbh=rZTqzr2omNiWHr/P9ud5YUTxjA5RW5r+UaScls1RbTA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=yH7qbpeEVC3ojGCVRWjIhzYsHOEMJYChWMWVV9Bf2XATeucu+e1E+z1udDvIioEP5\n\tYSjQim3J+i9nj7ACCRunYLSJTB3kXi4sA3JjQUUWZTtgjVINE+9+UwLVn9AVLCJGGQ\n\tVcI8gX3kil3JvY1gp0o0Eecp4mgUvQl+hMUJvEA7R/Nz42HfhungS8trBXmgrrH5Hc\n\tkxKS0M8HL5nAdkOvW5DqGQGmN7cAFWh9hWA6tnRsuogAoyTEkd7ypa+vc9la+HZSJP\n\tWif8FjRDetJppx3d2LzSO/sjPvSuA+QD8q1TVIu8S11s6PpKwQzHF0PGzJOms8oSY4\n\tL73tR7miQCoIQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1690450366;\n\tbh=rZTqzr2omNiWHr/P9ud5YUTxjA5RW5r+UaScls1RbTA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oQ8vaMHmZYnXFBt1WpOUcvACCxu0o8XnKVvda48ns/3kfdzzfu9ySxnUnkx+qJTr6\n\tS/Y6iOdDbrLAq4kLce3fbTSb9fIXbH1jXE2nf10zkD/lK9XRrXUBaBo7/RAFnl6ryP\n\t30IelYshVgFNILbSqRig83zFVlgKB3iV2vrY+hTY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"oQ8vaMHm\"; dkim-atps=neutral","Date":"Thu, 27 Jul 2023 11:33:42 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<6njl7zajtsvo6me6op5thov7t4tincu6jnfhdkrqvxasoeaeha@k5ienyoenuhn>","References":"<20230725085540.24863-1-naush@raspberrypi.com>\n\t<20230725085540.24863-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230725085540.24863-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] pipeline: rpi: Remove\n\tadditional external dma buf handling logic","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27722,"web_url":"https://patchwork.libcamera.org/comment/27722/","msgid":"<169381729911.277971.17212445272436176911@ping.linuxembedded.co.uk>","date":"2023-09-04T08:48:19","subject":"Re: [libcamera-devel] [PATCH v2 2/4] pipeline: rpi: Remove\n\tadditional external dma buf handling logic","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-07-25 09:55:38)\n> There is no need to distinguish between dma bufs allocated outside of\n> libcamera and internally allocated buffers. As such, remove all the\n> special case handling of such buffers.\n\nHrm ... I now wonder why this was added in the first place?\n\nBut indeed - a dmabuf is a dmabuf - there shouldn't be explicit handling\nfor a buffer depending on where it was allocated - unless it was an\n'internal only' buffer ... that isn't part of the stream...\n\nWas this part of handling streams that had to have an internally\nallocated 'scratch' buffer to support when the user didn't provide a\n(required) buffer for a stream?\n\nIf you're sure it's safe to go then I'm happy...\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  .../pipeline/rpi/common/pipeline_base.cpp        | 16 ----------------\n>  .../pipeline/rpi/common/pipeline_base.h          |  1 -\n>  src/libcamera/pipeline/rpi/common/rpi_stream.cpp | 11 +----------\n>  src/libcamera/pipeline/rpi/common/rpi_stream.h   |  2 --\n>  4 files changed, 1 insertion(+), 29 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 179a5b81a516..f244edc68a85 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1391,11 +1391,6 @@ void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n>          */\n>         Request *request = requestQueue_.empty() ? nullptr : requestQueue_.front();\n>         if (!dropFrameCount_ && request && request->findBuffer(stream) == buffer) {\n> -               /*\n> -                * Check if this is an externally provided buffer, and if\n> -                * so, we must stop tracking it in the pipeline handler.\n> -                */\n> -               handleExternalBuffer(buffer, stream);\n>                 /*\n>                  * Tag the buffer as completed, returning it to the\n>                  * application.\n> @@ -1435,17 +1430,6 @@ void CameraData::handleState()\n>         }\n>  }\n>  \n> -void CameraData::handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n> -{\n> -       unsigned int id = stream->getBufferId(buffer);\n> -\n> -       if (!(id & MaskExternalBuffer))\n> -               return;\n> -\n> -       /* Stop the Stream object from tracking the buffer. */\n> -       stream->removeExternalBuffer(buffer);\n> -}\n> -\n>  void CameraData::checkRequestCompleted()\n>  {\n>         bool requestCompleted = false;\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index f648e81054bb..8ee20a1b6d9b 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -193,7 +193,6 @@ protected:\n>         unsigned int ispOutputTotal_;\n>  \n>  private:\n> -       void handleExternalBuffer(FrameBuffer *buffer, Stream *stream);\n>         void checkRequestCompleted();\n>  };\n>  \n> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> index e9ad1e6f0848..74b5abf447c7 100644\n> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> @@ -78,16 +78,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const\n>  \n>  void Stream::setExternalBuffer(FrameBuffer *buffer)\n>  {\n> -       bufferMap_.emplace(BufferMask::MaskExternalBuffer | id_.get(), buffer);\n> -}\n> -\n> -void Stream::removeExternalBuffer(FrameBuffer *buffer)\n> -{\n> -       unsigned int id = getBufferId(buffer);\n> -\n> -       /* Ensure we have this buffer in the stream, and it is marked external. */\n> -       ASSERT(id & BufferMask::MaskExternalBuffer);\n> -       bufferMap_.erase(id);\n> +       bufferMap_.emplace(id_.get(), buffer);\n>  }\n>  \n>  int Stream::prepareBuffers(unsigned int count)\n> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> index 6edd304bdfe2..ca591f99cc45 100644\n> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> @@ -28,7 +28,6 @@ enum BufferMask {\n>         MaskStats               = 0x010000,\n>         MaskEmbeddedData        = 0x020000,\n>         MaskBayerData           = 0x040000,\n> -       MaskExternalBuffer      = 0x100000,\n>  };\n>  \n>  /*\n> @@ -78,7 +77,6 @@ public:\n>         unsigned int getBufferId(FrameBuffer *buffer) const;\n>  \n>         void setExternalBuffer(FrameBuffer *buffer);\n> -       void removeExternalBuffer(FrameBuffer *buffer);\n>  \n>         int prepareBuffers(unsigned int count);\n>         int queueBuffer(FrameBuffer *buffer);\n> -- \n> 2.34.1\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 7D850C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  4 Sep 2023 08:48:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBF1E628EA;\n\tMon,  4 Sep 2023 10:48:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9404A61DF7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  4 Sep 2023 10:48:21 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B2AF775A;\n\tMon,  4 Sep 2023 10:46:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693817303;\n\tbh=ZrtZphbfY6Bz+ZhlC2tS/szDnB+rBrHogSh1gCFvsic=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=EnsjqJM3Qb7DEJMzLc3zLxFvE1OJsOXr2m4lEUfwxrh9NYL5DXYbrN0iGrBff3ztW\n\tAvzJpC4gmLIkLvEpfR7uvduHJs77IybRXqmrsBeRqAvJQnwSLaWkz6joUNlTqiZrP1\n\tV8+1j0NJadOTcKkUdl6dfZzYcDw2cqpHsLesU3XbiVv9GPxR0JKJGZmT5gQChGLwls\n\tmbmae8udbh9wD1uwmD3d1yOpKnHBOjCnqOhoHsK04DpVuDJUnPxCOIS3Y2VGEBPTVV\n\tDQaKyoPkASlnqX4zz2eVItPVx8RzalQnujo5T29MVEzORa6rCp5UOoaK77qRjDB6M1\n\t7BHJ0Xmsn2EUQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693817215;\n\tbh=ZrtZphbfY6Bz+ZhlC2tS/szDnB+rBrHogSh1gCFvsic=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=tZDMfAooIzhhBQ9vQaj8Fr//ZFB5DverQ3uGYxEQLH1oUNzW4FPuz4pAms6/6ikaf\n\tgthCGtT7bSM9pBiNopl8LcTq5kPZWy39UlD31pfbbOVz8QsevhHxSyiStwItDvYEtT\n\tu3x4ceGcFts4baqsW8/bZ3cGqJM6ldJY/fn8Aptc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"tZDMfAoo\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230725085540.24863-3-naush@raspberrypi.com>","References":"<20230725085540.24863-1-naush@raspberrypi.com>\n\t<20230725085540.24863-3-naush@raspberrypi.com>","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 04 Sep 2023 09:48:19 +0100","Message-ID":"<169381729911.277971.17212445272436176911@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] pipeline: rpi: Remove\n\tadditional external dma buf handling logic","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27725,"web_url":"https://patchwork.libcamera.org/comment/27725/","msgid":"<CAEmqJPre_zc+A98p3k643TW4wBEcgodiPb65-GrpB0PX7k1t=A@mail.gmail.com>","date":"2023-09-04T09:07:52","subject":"Re: [libcamera-devel] [PATCH v2 2/4] pipeline: rpi: Remove\n\tadditional external dma buf handling logic","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for the review.\n\nOn Mon, 4 Sept 2023 at 09:48, Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Quoting Naushir Patuck via libcamera-devel (2023-07-25 09:55:38)\n> > There is no need to distinguish between dma bufs allocated outside of\n> > libcamera and internally allocated buffers. As such, remove all the\n> > special case handling of such buffers.\n>\n> Hrm ... I now wonder why this was added in the first place?\n>\n>\n> But indeed - a dmabuf is a dmabuf - there shouldn't be explicit handling\n> for a buffer depending on where it was allocated - unless it was an\n> 'internal only' buffer ... that isn't part of the stream...\n>\n> Was this part of handling streams that had to have an internally\n> allocated 'scratch' buffer to support when the user didn't provide a\n> (required) buffer for a stream?\n>\n> If you're sure it's safe to go then I'm happy...\n\nAll this external dmabuf handling code was mostly speculative code as\nit had never been tested with a real application doing external buffer\nallocation.  Now that I've actually got an example of an application\ndoing this, I can weed out the unnecessary code (like this bit here).\n\nRegards,\nNaush\n\n>\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  .../pipeline/rpi/common/pipeline_base.cpp        | 16 ----------------\n> >  .../pipeline/rpi/common/pipeline_base.h          |  1 -\n> >  src/libcamera/pipeline/rpi/common/rpi_stream.cpp | 11 +----------\n> >  src/libcamera/pipeline/rpi/common/rpi_stream.h   |  2 --\n> >  4 files changed, 1 insertion(+), 29 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 179a5b81a516..f244edc68a85 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -1391,11 +1391,6 @@ void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n> >          */\n> >         Request *request = requestQueue_.empty() ? nullptr : requestQueue_.front();\n> >         if (!dropFrameCount_ && request && request->findBuffer(stream) == buffer) {\n> > -               /*\n> > -                * Check if this is an externally provided buffer, and if\n> > -                * so, we must stop tracking it in the pipeline handler.\n> > -                */\n> > -               handleExternalBuffer(buffer, stream);\n> >                 /*\n> >                  * Tag the buffer as completed, returning it to the\n> >                  * application.\n> > @@ -1435,17 +1430,6 @@ void CameraData::handleState()\n> >         }\n> >  }\n> >\n> > -void CameraData::handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n> > -{\n> > -       unsigned int id = stream->getBufferId(buffer);\n> > -\n> > -       if (!(id & MaskExternalBuffer))\n> > -               return;\n> > -\n> > -       /* Stop the Stream object from tracking the buffer. */\n> > -       stream->removeExternalBuffer(buffer);\n> > -}\n> > -\n> >  void CameraData::checkRequestCompleted()\n> >  {\n> >         bool requestCompleted = false;\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > index f648e81054bb..8ee20a1b6d9b 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > @@ -193,7 +193,6 @@ protected:\n> >         unsigned int ispOutputTotal_;\n> >\n> >  private:\n> > -       void handleExternalBuffer(FrameBuffer *buffer, Stream *stream);\n> >         void checkRequestCompleted();\n> >  };\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > index e9ad1e6f0848..74b5abf447c7 100644\n> > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp\n> > @@ -78,16 +78,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const\n> >\n> >  void Stream::setExternalBuffer(FrameBuffer *buffer)\n> >  {\n> > -       bufferMap_.emplace(BufferMask::MaskExternalBuffer | id_.get(), buffer);\n> > -}\n> > -\n> > -void Stream::removeExternalBuffer(FrameBuffer *buffer)\n> > -{\n> > -       unsigned int id = getBufferId(buffer);\n> > -\n> > -       /* Ensure we have this buffer in the stream, and it is marked external. */\n> > -       ASSERT(id & BufferMask::MaskExternalBuffer);\n> > -       bufferMap_.erase(id);\n> > +       bufferMap_.emplace(id_.get(), buffer);\n> >  }\n> >\n> >  int Stream::prepareBuffers(unsigned int count)\n> > diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > index 6edd304bdfe2..ca591f99cc45 100644\n> > --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h\n> > @@ -28,7 +28,6 @@ enum BufferMask {\n> >         MaskStats               = 0x010000,\n> >         MaskEmbeddedData        = 0x020000,\n> >         MaskBayerData           = 0x040000,\n> > -       MaskExternalBuffer      = 0x100000,\n> >  };\n> >\n> >  /*\n> > @@ -78,7 +77,6 @@ public:\n> >         unsigned int getBufferId(FrameBuffer *buffer) const;\n> >\n> >         void setExternalBuffer(FrameBuffer *buffer);\n> > -       void removeExternalBuffer(FrameBuffer *buffer);\n> >\n> >         int prepareBuffers(unsigned int count);\n> >         int queueBuffer(FrameBuffer *buffer);\n> > --\n> > 2.34.1\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 EE527C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  4 Sep 2023 09:08:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4416F628E6;\n\tMon,  4 Sep 2023 11:08:32 +0200 (CEST)","from mail-ot1-x32a.google.com (mail-ot1-x32a.google.com\n\t[IPv6:2607:f8b0:4864:20::32a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1FE69628E4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  4 Sep 2023 11:08:30 +0200 (CEST)","by mail-ot1-x32a.google.com with SMTP id\n\t46e09a7af769-6bf04263dc8so972682a34.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 04 Sep 2023 02:08:30 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693818512;\n\tbh=e+MJKaQNQHDwN53m/E+JB+yOV6PITUSQ1T26XjG2AHg=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=H6l20mFVGckFJMuBHxrPwSYQ7lFypjyx7f+WEy79Y31VEzuKhVX7J51dVyVjs20CW\n\tl9OyFAw0CyTn6pzDHa7fmRRfNV4rkp6tppIqFEodF0yp/iwHBwkY7PrzZft+G3MMP/\n\tuZPruNhvCyLgMIAut9PwlqFFe1xgBmtqNeJL6VDOMbT8ncWkK+msuF/9V18PM1KnvV\n\tOtfi9LTcOb+UlGagpBwyGR0w4eFPqFAoXC1qOh6L+6lyaQ7asEtvbK/9ztKWY2a9us\n\t4D2EjJo7/DsTjAJtMgBA+L/yllvM7u00xIUmSuuR55K1ChfKygUWb7DsExjV9a6jZM\n\t72FgmDm/YtOWg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1693818508; x=1694423308;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=QDUgqluMDhbYLzXMBgZkh6rh9E+/pcyeC+9+txj+TOE=;\n\tb=eYWTkxJOLlOr2NSLoFnqlesPlBOK0IiNy2Oz7AXevyuGDhELsD1LmHRbVN/QDZTM5F\n\tr3ctcDuywdpchqW984TdSO9oFtYAah2Vwgc7HRtWBN6PGfmzDvVWxd2z5gSHyJ4RrAll\n\tY8cpdz5yjT1iaiWPvcZX3ltXHiuVeBt3GHQraCRmCbu22HWoXQjUUH645rYGPrP8hPkW\n\tjpH9dt9UBXAuLmeaHu34w2wNfxaYerHPMTGPD5TJXjjiixEwOCC3jI5ltvJc+9AXq0Dr\n\tbgrJfuMPWzC6RJGrpbdGbrQJl7SwJQbuDEABN31SsrQxAreq8tKFQjFpZnT9C0ColcP4\n\tQvXg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"eYWTkxJO\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1693818508; x=1694423308;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=QDUgqluMDhbYLzXMBgZkh6rh9E+/pcyeC+9+txj+TOE=;\n\tb=VLMUtN2psRqPT3IJUCLljwQPLgGElPFty2rZFckIX8zUama79oDiVKs+JmOJpN2OAU\n\tfzPOfh++JHRdFWIz/xxAcI+wXNDHwwZmzasNY8sGTkqRbkkPdfTRjWTCs+ygVNtPh/Lt\n\tqS1rqxXME5ZOEF7lDkPDl9KiwrXicqnoHz0fR+xyyGRwss3UL8iQBBMLQgCxqVEehxnA\n\tCczx14QnRT3vuwNGS8e06Z15YGIpdvmVDoaQAFPAroa8bMZfdwoAYdgxVxLf3090eT4+\n\tbxzLT/lLVY5MUz3arz20siVLMwESQ90922jZTdI5Xh8YqDMGEOXVK/N1Fw9eGPHTgLNr\n\tJeoQ==","X-Gm-Message-State":"AOJu0YxlktNkVMCmVcZ9W1Qesvi05Jn7Zomi4X0NAoil9yoastxMvaZb\n\t28EJvioctWb4MnzZwuGTlLtHTCT7+kFnRMQQ0Gx14yIJKXbE+We1Tmc=","X-Google-Smtp-Source":"AGHT+IHVNNJ/db+EanEC5dzwGA+kB4ytz+ShobpkEjZWOdDmLXnffgRFd9bsX5Y2mHuwng6rSqqhyZ9HoPhB05KBqkk=","X-Received":"by 2002:a05:6358:70f:b0:134:d128:9f5f with SMTP id\n\te15-20020a056358070f00b00134d1289f5fmr10673748rwj.9.1693818508579;\n\tMon, 04 Sep 2023 02:08:28 -0700 (PDT)","MIME-Version":"1.0","References":"<20230725085540.24863-1-naush@raspberrypi.com>\n\t<20230725085540.24863-3-naush@raspberrypi.com>\n\t<169381729911.277971.17212445272436176911@ping.linuxembedded.co.uk>","In-Reply-To":"<169381729911.277971.17212445272436176911@ping.linuxembedded.co.uk>","Date":"Mon, 4 Sep 2023 10:07:52 +0100","Message-ID":"<CAEmqJPre_zc+A98p3k643TW4wBEcgodiPb65-GrpB0PX7k1t=A@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/4] pipeline: rpi: Remove\n\tadditional external dma buf handling logic","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]