[{"id":28458,"web_url":"https://patchwork.libcamera.org/comment/28458/","msgid":"<170518527196.3418233.36183636241911333@ping.linuxembedded.co.uk>","date":"2024-01-13T22:34:31","subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Hans de Goede via libcamera-devel (2024-01-13 14:22:02)\n> From: Andrey Konovalov <andrey.konovalov@linaro.org>\n> \n> DmaHeap class is useful outside the RPi pipeline handler too.\n> \n> Move dma_heaps.h and dma_heaps.cpp to common directories. Update\n> the build files and RPi vc4 pipeline handler accordingly.\n\nThis looks fine to me, but I'd like to see an Acked-by: or Reviewed-by:\nfrom Raspberry Pi to move this.\n\nNaush, David, - any objections to promoting the DmaHeap code to core\ncode?\n\nThe next patch also extends it with a new feature, but I don't believe\nit impacts the RPi code base specifically.\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n> Signed-off-by: Hans de Goede <hdegoede@redhat.com>\n> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek <pavel@ucw.cz>\n> ---\n>  .../libcamera/internal}/dma_heaps.h            |  4 ----\n>  include/libcamera/internal/meson.build         |  1 +\n>  .../{pipeline/rpi/vc4 => }/dma_heaps.cpp       | 18 +++++++-----------\n>  src/libcamera/meson.build                      |  1 +\n>  src/libcamera/pipeline/rpi/vc4/meson.build     |  1 -\n>  src/libcamera/pipeline/rpi/vc4/vc4.cpp         |  5 ++---\n>  6 files changed, 11 insertions(+), 19 deletions(-)\n>  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (92%)\n>  rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (83%)\n> \n> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h b/include/libcamera/internal/dma_heaps.h\n> similarity index 92%\n> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n> rename to include/libcamera/internal/dma_heaps.h\n> index 0a4a8d86..cff8f140 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n> +++ b/include/libcamera/internal/dma_heaps.h\n> @@ -13,8 +13,6 @@\n>  \n>  namespace libcamera {\n>  \n> -namespace RPi {\n> -\n>  class DmaHeap\n>  {\n>  public:\n> @@ -27,6 +25,4 @@ private:\n>         UniqueFD dmaHeapHandle_;\n>  };\n>  \n> -} /* namespace RPi */\n> -\n>  } /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 7f1f3440..33eb0fb3 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -25,6 +25,7 @@ libcamera_internal_headers = files([\n>      'device_enumerator.h',\n>      'device_enumerator_sysfs.h',\n>      'device_enumerator_udev.h',\n> +    'dma_heaps.h',\n>      'formats.h',\n>      'framebuffer.h',\n>      'ipa_manager.h',\n> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp\n> similarity index 83%\n> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n> rename to src/libcamera/dma_heaps.cpp\n> index 317b1fc1..7444d9c2 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n> +++ b/src/libcamera/dma_heaps.cpp\n> @@ -5,8 +5,6 @@\n>   * dma_heaps.h - Helper class for dma-heap allocations.\n>   */\n>  \n> -#include \"dma_heaps.h\"\n> -\n>  #include <array>\n>  #include <fcntl.h>\n>  #include <linux/dma-buf.h>\n> @@ -16,6 +14,8 @@\n>  \n>  #include <libcamera/base/log.h>\n>  \n> +#include \"libcamera/internal/dma_heaps.h\"\n> +\n>  /*\n>   * /dev/dma-heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma\n>   * to only have to worry about importing.\n> @@ -30,9 +30,7 @@ static constexpr std::array<const char *, 2> heapNames = {\n>  \n>  namespace libcamera {\n>  \n> -LOG_DECLARE_CATEGORY(RPI)\n> -\n> -namespace RPi {\n> +LOG_DEFINE_CATEGORY(DmaHeap)\n>  \n>  DmaHeap::DmaHeap()\n>  {\n> @@ -40,7 +38,7 @@ DmaHeap::DmaHeap()\n>                 int ret = ::open(name, O_RDWR | O_CLOEXEC, 0);\n>                 if (ret < 0) {\n>                         ret = errno;\n> -                       LOG(RPI, Debug) << \"Failed to open \" << name << \": \"\n> +                       LOG(DmaHeap, Debug) << \"Failed to open \" << name << \": \"\n>                                         << strerror(ret);\n>                         continue;\n>                 }\n> @@ -50,7 +48,7 @@ DmaHeap::DmaHeap()\n>         }\n>  \n>         if (!dmaHeapHandle_.isValid())\n> -               LOG(RPI, Error) << \"Could not open any dmaHeap device\";\n> +               LOG(DmaHeap, Error) << \"Could not open any dmaHeap device\";\n>  }\n>  \n>  DmaHeap::~DmaHeap() = default;\n> @@ -69,7 +67,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>  \n>         ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n>         if (ret < 0) {\n> -               LOG(RPI, Error) << \"dmaHeap allocation failure for \"\n> +               LOG(DmaHeap, Error) << \"dmaHeap allocation failure for \"\n>                                 << name;\n>                 return {};\n>         }\n> @@ -77,7 +75,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>         UniqueFD allocFd(alloc.fd);\n>         ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n>         if (ret < 0) {\n> -               LOG(RPI, Error) << \"dmaHeap naming failure for \"\n> +               LOG(DmaHeap, Error) << \"dmaHeap naming failure for \"\n>                                 << name;\n>                 return {};\n>         }\n> @@ -85,6 +83,4 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>         return allocFd;\n>  }\n>  \n> -} /* namespace RPi */\n> -\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 45f63e93..3c5e43df 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -17,6 +17,7 @@ libcamera_sources = files([\n>      'delayed_controls.cpp',\n>      'device_enumerator.cpp',\n>      'device_enumerator_sysfs.cpp',\n> +    'dma_heaps.cpp',\n>      'fence.cpp',\n>      'formats.cpp',\n>      'framebuffer.cpp',\n> diff --git a/src/libcamera/pipeline/rpi/vc4/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build\n> index cdb049c5..386e2296 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/meson.build\n> +++ b/src/libcamera/pipeline/rpi/vc4/meson.build\n> @@ -1,7 +1,6 @@\n>  # SPDX-License-Identifier: CC0-1.0\n>  \n>  libcamera_sources += files([\n> -    'dma_heaps.cpp',\n>      'vc4.cpp',\n>  ])\n>  \n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 26102ea7..3a42e75e 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -12,12 +12,11 @@\n>  #include <libcamera/formats.h>\n>  \n>  #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/dma_heaps.h\"\n>  \n>  #include \"../common/pipeline_base.h\"\n>  #include \"../common/rpi_stream.h\"\n>  \n> -#include \"dma_heaps.h\"\n> -\n>  using namespace std::chrono_literals;\n>  \n>  namespace libcamera {\n> @@ -87,7 +86,7 @@ public:\n>         RPi::Device<Isp, 4> isp_;\n>  \n>         /* DMAHEAP allocation helper. */\n> -       RPi::DmaHeap dmaHeap_;\n> +       DmaHeap dmaHeap_;\n>         SharedFD lsTable_;\n>  \n>         struct Config {\n> -- \n> 2.43.0\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 37234BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 13 Jan 2024 22:34:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D23E628B7;\n\tSat, 13 Jan 2024 23:34:37 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 380C261D57\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 13 Jan 2024 23:34:35 +0100 (CET)","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 0925F975;\n\tSat, 13 Jan 2024 23:33:27 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705185277;\n\tbh=b+ldUK7IO4lhmY1IAdJMauO5N5xr3efaiHfq9IbQ02k=;\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:Cc:\n\tFrom;\n\tb=CwdSFMhPCXQg8Bf/UqX1ylDVLiW4VTTI3qBeQzlO3nMz8EKfEMHbMpNUnj4C1Kjym\n\tS0ABTqgiqNUmGCfvUnlwMgccQ4VBcEmR0mIzKe7NV40zoOLwU4aavuFxwdoxWRzDM8\n\tTLYdDW4q1PyULemqt6CQxpR5abjm8bCQXw6LQSL8GE7NpTahXpINFDj+UbVknNZ2T+\n\t7tXTv3crl7kN1Ir82+eiMbAm9Ct3ZXE44UFWWZtlA1lbYDklwm+fxByNvUWlwNCpt7\n\tfPeji35w2Kjh1/Twtn3+qecRgaqlAAa7heNiKO47DmI1/5s7KnzRdcPbtgWCOwMIbW\n\tjzXmquw7phrSQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705185208;\n\tbh=b+ldUK7IO4lhmY1IAdJMauO5N5xr3efaiHfq9IbQ02k=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=RSRuwwCZQ/ax0BCt/jPyMBqZm+ITkJkCGTbj6HlV/MhwDa44MEbcAJPTxbbHhw1qs\n\tKSefyNWFctd9nePsPoYdhLfoPbWnFAr9rb7mWvjMyXK3ix9TKr+crLrWwSM5YjPipI\n\tlAfC+LJqdbO7czNSxdAlL+c39OSOryBosfIPSvJ0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RSRuwwCZ\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240113142218.28063-3-hdegoede@redhat.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-3-hdegoede@redhat.com>","To":"Andrey Konovalov <andrey.konovalov.ynk@gmail.com>,\n\tHans de Goede <hdegoede@redhat.com>, libcamera-devel@lists.libcamera.org","Date":"Sat, 13 Jan 2024 22:34:31 +0000","Message-ID":"<170518527196.3418233.36183636241911333@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","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>","Cc":"Maxime Ripard <mripard@redhat.com>, g.martti@gmail.com,\n\tt.langendam@gmail.com, srinivas.kandagatla@linaro.org,\n\tPavel Machek <pavel@ucw.cz>,\n\tBryan O'Donoghue <bryan.odonoghue@linaro.org>, admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28475,"web_url":"https://patchwork.libcamera.org/comment/28475/","msgid":"<CAEmqJPr+_s9X01bcuvapnP8v-JZSCc=5b44Sk0ASWjFNPYbMaw@mail.gmail.com>","date":"2024-01-15T09:08:42","subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi all,\n\nOn Sat, 13 Jan 2024 at 22:34, Kieran Bingham via libcamera-devel\n<libcamera-devel@lists.libcamera.org> wrote:\n>\n> Quoting Hans de Goede via libcamera-devel (2024-01-13 14:22:02)\n> > From: Andrey Konovalov <andrey.konovalov@linaro.org>\n> >\n> > DmaHeap class is useful outside the RPi pipeline handler too.\n> >\n> > Move dma_heaps.h and dma_heaps.cpp to common directories. Update\n> > the build files and RPi vc4 pipeline handler accordingly.\n>\n> This looks fine to me, but I'd like to see an Acked-by: or Reviewed-by:\n> from Raspberry Pi to move this.\n>\n> Naush, David, - any objections to promoting the DmaHeap code to core\n> code?\n\nNo objections from me.\n\nReviewed-by: Naushir Patuck <naush@raspberrypi.com>\n\n>\n> The next patch also extends it with a new feature, but I don't believe\n> it impacts the RPi code base specifically.\n>\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n> >\n> > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>\n> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s\n> > Tested-by: Pavel Machek <pavel@ucw.cz>\n> > ---\n> >  .../libcamera/internal}/dma_heaps.h            |  4 ----\n> >  include/libcamera/internal/meson.build         |  1 +\n> >  .../{pipeline/rpi/vc4 => }/dma_heaps.cpp       | 18 +++++++-----------\n> >  src/libcamera/meson.build                      |  1 +\n> >  src/libcamera/pipeline/rpi/vc4/meson.build     |  1 -\n> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp         |  5 ++---\n> >  6 files changed, 11 insertions(+), 19 deletions(-)\n> >  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (92%)\n> >  rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (83%)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h b/include/libcamera/internal/dma_heaps.h\n> > similarity index 92%\n> > rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n> > rename to include/libcamera/internal/dma_heaps.h\n> > index 0a4a8d86..cff8f140 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n> > +++ b/include/libcamera/internal/dma_heaps.h\n> > @@ -13,8 +13,6 @@\n> >\n> >  namespace libcamera {\n> >\n> > -namespace RPi {\n> > -\n> >  class DmaHeap\n> >  {\n> >  public:\n> > @@ -27,6 +25,4 @@ private:\n> >         UniqueFD dmaHeapHandle_;\n> >  };\n> >\n> > -} /* namespace RPi */\n> > -\n> >  } /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 7f1f3440..33eb0fb3 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -25,6 +25,7 @@ libcamera_internal_headers = files([\n> >      'device_enumerator.h',\n> >      'device_enumerator_sysfs.h',\n> >      'device_enumerator_udev.h',\n> > +    'dma_heaps.h',\n> >      'formats.h',\n> >      'framebuffer.h',\n> >      'ipa_manager.h',\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp\n> > similarity index 83%\n> > rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n> > rename to src/libcamera/dma_heaps.cpp\n> > index 317b1fc1..7444d9c2 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n> > +++ b/src/libcamera/dma_heaps.cpp\n> > @@ -5,8 +5,6 @@\n> >   * dma_heaps.h - Helper class for dma-heap allocations.\n> >   */\n> >\n> > -#include \"dma_heaps.h\"\n> > -\n> >  #include <array>\n> >  #include <fcntl.h>\n> >  #include <linux/dma-buf.h>\n> > @@ -16,6 +14,8 @@\n> >\n> >  #include <libcamera/base/log.h>\n> >\n> > +#include \"libcamera/internal/dma_heaps.h\"\n> > +\n> >  /*\n> >   * /dev/dma-heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma\n> >   * to only have to worry about importing.\n> > @@ -30,9 +30,7 @@ static constexpr std::array<const char *, 2> heapNames = {\n> >\n> >  namespace libcamera {\n> >\n> > -LOG_DECLARE_CATEGORY(RPI)\n> > -\n> > -namespace RPi {\n> > +LOG_DEFINE_CATEGORY(DmaHeap)\n> >\n> >  DmaHeap::DmaHeap()\n> >  {\n> > @@ -40,7 +38,7 @@ DmaHeap::DmaHeap()\n> >                 int ret = ::open(name, O_RDWR | O_CLOEXEC, 0);\n> >                 if (ret < 0) {\n> >                         ret = errno;\n> > -                       LOG(RPI, Debug) << \"Failed to open \" << name << \": \"\n> > +                       LOG(DmaHeap, Debug) << \"Failed to open \" << name << \": \"\n> >                                         << strerror(ret);\n> >                         continue;\n> >                 }\n> > @@ -50,7 +48,7 @@ DmaHeap::DmaHeap()\n> >         }\n> >\n> >         if (!dmaHeapHandle_.isValid())\n> > -               LOG(RPI, Error) << \"Could not open any dmaHeap device\";\n> > +               LOG(DmaHeap, Error) << \"Could not open any dmaHeap device\";\n> >  }\n> >\n> >  DmaHeap::~DmaHeap() = default;\n> > @@ -69,7 +67,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n> >\n> >         ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n> >         if (ret < 0) {\n> > -               LOG(RPI, Error) << \"dmaHeap allocation failure for \"\n> > +               LOG(DmaHeap, Error) << \"dmaHeap allocation failure for \"\n> >                                 << name;\n> >                 return {};\n> >         }\n> > @@ -77,7 +75,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n> >         UniqueFD allocFd(alloc.fd);\n> >         ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n> >         if (ret < 0) {\n> > -               LOG(RPI, Error) << \"dmaHeap naming failure for \"\n> > +               LOG(DmaHeap, Error) << \"dmaHeap naming failure for \"\n> >                                 << name;\n> >                 return {};\n> >         }\n> > @@ -85,6 +83,4 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n> >         return allocFd;\n> >  }\n> >\n> > -} /* namespace RPi */\n> > -\n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 45f63e93..3c5e43df 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -17,6 +17,7 @@ libcamera_sources = files([\n> >      'delayed_controls.cpp',\n> >      'device_enumerator.cpp',\n> >      'device_enumerator_sysfs.cpp',\n> > +    'dma_heaps.cpp',\n> >      'fence.cpp',\n> >      'formats.cpp',\n> >      'framebuffer.cpp',\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build\n> > index cdb049c5..386e2296 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/meson.build\n> > +++ b/src/libcamera/pipeline/rpi/vc4/meson.build\n> > @@ -1,7 +1,6 @@\n> >  # SPDX-License-Identifier: CC0-1.0\n> >\n> >  libcamera_sources += files([\n> > -    'dma_heaps.cpp',\n> >      'vc4.cpp',\n> >  ])\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > index 26102ea7..3a42e75e 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > @@ -12,12 +12,11 @@\n> >  #include <libcamera/formats.h>\n> >\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> > +#include \"libcamera/internal/dma_heaps.h\"\n> >\n> >  #include \"../common/pipeline_base.h\"\n> >  #include \"../common/rpi_stream.h\"\n> >\n> > -#include \"dma_heaps.h\"\n> > -\n> >  using namespace std::chrono_literals;\n> >\n> >  namespace libcamera {\n> > @@ -87,7 +86,7 @@ public:\n> >         RPi::Device<Isp, 4> isp_;\n> >\n> >         /* DMAHEAP allocation helper. */\n> > -       RPi::DmaHeap dmaHeap_;\n> > +       DmaHeap dmaHeap_;\n> >         SharedFD lsTable_;\n> >\n> >         struct Config {\n> > --\n> > 2.43.0\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 B8D63BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 15 Jan 2024 09:09:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E700628B6;\n\tMon, 15 Jan 2024 10:09:20 +0100 (CET)","from mail-yb1-xb2f.google.com (mail-yb1-xb2f.google.com\n\t[IPv6:2607:f8b0:4864:20::b2f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DB37628AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 15 Jan 2024 10:09:18 +0100 (CET)","by mail-yb1-xb2f.google.com with SMTP id\n\t3f1490d57ef6-d9b9adaf291so5703646276.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 15 Jan 2024 01:09:18 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705309760;\n\tbh=OWPo00pIl9YywUG8DYnC1R8Bp+I9peK41cBBBZKVhDA=;\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=GvXyU7PX3Oe4QaR9obQwLon7kRE4emREql95uYpFzCbxZ/eOneAcxCWkDzZ77Ofzz\n\txBt87tNZi+5LoqmGfN3WreTVsOTkh1n0mvqEN6h8Dm2vdnKJFJXOjOTckKpi3+G+yC\n\tH/PYPTq8pux+uXbsh8RmPO+m3kpjOnWdKLUeldaOzzkB8wuFrkfxyt4jYRx9H9Atcq\n\t0Tyd5uJAlrGXQFSipwYpNeRoSmxxDUp2H4g27JcFMtXaCPz/OIAzYZnd9mf9HzBIeT\n\tIkaPXsvj5E8DJ7OnW5qhFCalsNhDmk2NBlBAwpEeJiNgUazr5KQ7XXLG++Cw9Vv+K5\n\tsGD0dzKk7l/pg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1705309757; x=1705914557;\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=bG5pcIydiYJ+CNXTa96GnOS5YjTX+QN/Eq8Z6BoH/t0=;\n\tb=hhd+JfSiD36iE/5XJs3pf3OAMPAYahRE1Wo3xPl5mAlr1XrCMfQgSgvh7Gsqw6jUmL\n\t4z09j5HwqAq69s3T4f41VTeezmXe3ZZ6TVkv8qGku6TAeYL1xwslhL1Ss1KlJ64OGHeS\n\tnCf1uakdg2PrGT83xwaWu8pf/aAYXECTUuNK8uoe13PGPruw9y1grPBeM9tSWUeE4hlt\n\tm4YToOSoNOoB3abwvKFgdoqmfl6obvxv6IkXF05qJ7pgG/3nyolriLdau6+NHot2pEeu\n\tQlIIneZtaRVuQyIHI3EhWgNAAWe+xyLDU3RRNJP2eC0UmptsnmwOvsUilYMq0vNLFEC3\n\tQz5w=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"hhd+JfSi\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1705309757; x=1705914557;\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=bG5pcIydiYJ+CNXTa96GnOS5YjTX+QN/Eq8Z6BoH/t0=;\n\tb=LWyG23Xpey8sguO621230fNv/3BCYaneVjQm92Xss0C8pO8nXvNK1hbwht863p5kH3\n\teN6su3VDV0STdfAGU0rw41RhxWqzli9DFQJMMH/r9oJb56cMRw+U3WDdiLz5JY7MOr7k\n\tOiYOjaLSTDCcF/yFBtT6AXtonhSlbt9DXA4eUGDIbcFLzSV989U58NMI8SkcmuWX1jo6\n\tYJkLexy+1NUuXWVu+QmYSNFC49xEZDNhVB9dJ4EFmg2w0xndumNuytslRlRyCC1xcSH4\n\tGZfNeFDI9btzYl6fRP8qPIQUK815RGdTucmFEjW+tbLPUnEIhjfsoHC1K2cVbqIBCY8V\n\tyixw==","X-Gm-Message-State":"AOJu0YyYrFjiwWSk5+uUohxLUyNjw2H1o1FBVid1l+MCnq857zVyAQr9\n\tuZE/AL/G5YCBQ35LH2RcYyOsNQYkv4e1wxNiH+rrJCGwwIWwNA==","X-Google-Smtp-Source":"AGHT+IESXjeR0o72VascjC1OVc/x0xvQ3BcL8SzSpkAOHPAEMZ2F7zxHt1O/5VSHadgjCswvFGGB9bCg8g4tDYOU9Y0=","X-Received":"by 2002:a25:9248:0:b0:dbd:b8a7:af0a with SMTP id\n\te8-20020a259248000000b00dbdb8a7af0amr1671444ybo.8.1705309757153;\n\tMon, 15 Jan 2024 01:09:17 -0800 (PST)","MIME-Version":"1.0","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-3-hdegoede@redhat.com>\n\t<170518527196.3418233.36183636241911333@ping.linuxembedded.co.uk>","In-Reply-To":"<170518527196.3418233.36183636241911333@ping.linuxembedded.co.uk>","Date":"Mon, 15 Jan 2024 09:08:42 +0000","Message-ID":"<CAEmqJPr+_s9X01bcuvapnP8v-JZSCc=5b44Sk0ASWjFNPYbMaw@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","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":"Maxime Ripard <mripard@redhat.com>, g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek <pavel@ucw.cz>,\n\tBryan O'Donoghue <bryan.odonoghue@linaro.org>, admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28597,"web_url":"https://patchwork.libcamera.org/comment/28597/","msgid":"<20240123142016.GB10679@pendragon.ideasonboard.com>","date":"2024-01-23T14:20:16","subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hello Andrey,\n\nThank you for the patch.\n\nOn Sat, Jan 13, 2024 at 03:22:02PM +0100, 📷-dev wrote:\n> From: Andrey Konovalov <andrey.konovalov@linaro.org>\n> \n> DmaHeap class is useful outside the RPi pipeline handler too.\n> \n> Move dma_heaps.h and dma_heaps.cpp to common directories. Update\n> the build files and RPi vc4 pipeline handler accordingly.\n> \n> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n> Signed-off-by: Hans de Goede <hdegoede@redhat.com>\n> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek <pavel@ucw.cz>\n> ---\n>  .../libcamera/internal}/dma_heaps.h            |  4 ----\n>  include/libcamera/internal/meson.build         |  1 +\n>  .../{pipeline/rpi/vc4 => }/dma_heaps.cpp       | 18 +++++++-----------\n>  src/libcamera/meson.build                      |  1 +\n>  src/libcamera/pipeline/rpi/vc4/meson.build     |  1 -\n>  src/libcamera/pipeline/rpi/vc4/vc4.cpp         |  5 ++---\n>  6 files changed, 11 insertions(+), 19 deletions(-)\n>  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (92%)\n>  rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (83%)\n> \n> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h b/include/libcamera/internal/dma_heaps.h\n> similarity index 92%\n> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n> rename to include/libcamera/internal/dma_heaps.h\n> index 0a4a8d86..cff8f140 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n> +++ b/include/libcamera/internal/dma_heaps.h\n> @@ -13,8 +13,6 @@\n>  \n>  namespace libcamera {\n>  \n> -namespace RPi {\n> -\n>  class DmaHeap\n>  {\n>  public:\n> @@ -27,6 +25,4 @@ private:\n>  \tUniqueFD dmaHeapHandle_;\n>  };\n>  \n> -} /* namespace RPi */\n> -\n>  } /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 7f1f3440..33eb0fb3 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -25,6 +25,7 @@ libcamera_internal_headers = files([\n>      'device_enumerator.h',\n>      'device_enumerator_sysfs.h',\n>      'device_enumerator_udev.h',\n> +    'dma_heaps.h',\n>      'formats.h',\n>      'framebuffer.h',\n>      'ipa_manager.h',\n> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp\n> similarity index 83%\n> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n> rename to src/libcamera/dma_heaps.cpp\n> index 317b1fc1..7444d9c2 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n> +++ b/src/libcamera/dma_heaps.cpp\n> @@ -5,8 +5,6 @@\n>   * dma_heaps.h - Helper class for dma-heap allocations.\n>   */\n>  \n> -#include \"dma_heaps.h\"\n> -\n>  #include <array>\n>  #include <fcntl.h>\n>  #include <linux/dma-buf.h>\n> @@ -16,6 +14,8 @@\n>  \n>  #include <libcamera/base/log.h>\n>  \n> +#include \"libcamera/internal/dma_heaps.h\"\n> +\n\nThis should go first in the file, see \"Order of Includes\" in\nDocumentation/coding-style.rst.\n\n>  /*\n>   * /dev/dma-heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma\n>   * to only have to worry about importing.\n> @@ -30,9 +30,7 @@ static constexpr std::array<const char *, 2> heapNames = {\n>  \n>  namespace libcamera {\n>  \n> -LOG_DECLARE_CATEGORY(RPI)\n> -\n> -namespace RPi {\n> +LOG_DEFINE_CATEGORY(DmaHeap)\n>  \n>  DmaHeap::DmaHeap()\n\nNow that this is a shared class, it requires documentation.\n\n>  {\n> @@ -40,7 +38,7 @@ DmaHeap::DmaHeap()\n>  \t\tint ret = ::open(name, O_RDWR | O_CLOEXEC, 0);\n>  \t\tif (ret < 0) {\n>  \t\t\tret = errno;\n> -\t\t\tLOG(RPI, Debug) << \"Failed to open \" << name << \": \"\n> +\t\t\tLOG(DmaHeap, Debug) << \"Failed to open \" << name << \": \"\n>  \t\t\t\t\t<< strerror(ret);\n\nWhile at it, single extra tab for the last line:\n\n\t\t\tLOG(DmaHeap, Debug) << \"Failed to open \" << name << \": \"\n\t\t\t\t<< strerror(ret);\n\nbut we then usually wrap the whole message as\n\n\t\t\tLOG(DmaHeap, Debug)\n\t\t\t\t<< \"Failed to open \" << name << \": \"\n\t\t\t\t<< strerror(ret);\n\n>  \t\t\tcontinue;\n>  \t\t}\n> @@ -50,7 +48,7 @@ DmaHeap::DmaHeap()\n>  \t}\n>  \n>  \tif (!dmaHeapHandle_.isValid())\n> -\t\tLOG(RPI, Error) << \"Could not open any dmaHeap device\";\n> +\t\tLOG(DmaHeap, Error) << \"Could not open any dmaHeap device\";\n>  }\n>  \n>  DmaHeap::~DmaHeap() = default;\n> @@ -69,7 +67,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>  \n>  \tret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n>  \tif (ret < 0) {\n> -\t\tLOG(RPI, Error) << \"dmaHeap allocation failure for \"\n> +\t\tLOG(DmaHeap, Error) << \"dmaHeap allocation failure for \"\n>  \t\t\t\t<< name;\n\n\t\tLOG(DmaHeap, Error) << \"dmaHeap allocation failure for \" << name;\n\nIt can go a bit over the 80 columns limit, that's fine.\n\n>  \t\treturn {};\n>  \t}\n> @@ -77,7 +75,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>  \tUniqueFD allocFd(alloc.fd);\n>  \tret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n>  \tif (ret < 0) {\n> -\t\tLOG(RPI, Error) << \"dmaHeap naming failure for \"\n> +\t\tLOG(DmaHeap, Error) << \"dmaHeap naming failure for \"\n>  \t\t\t\t<< name;\n\nThis one doesn't even go over 80 columns :-)\n\n\t\tLOG(DmaHeap, Error) << \"dmaHeap naming failure for \" << name;\n\n>  \t\treturn {};\n>  \t}\n> @@ -85,6 +83,4 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>  \treturn allocFd;\n>  }\n>  \n> -} /* namespace RPi */\n> -\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 45f63e93..3c5e43df 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -17,6 +17,7 @@ libcamera_sources = files([\n>      'delayed_controls.cpp',\n>      'device_enumerator.cpp',\n>      'device_enumerator_sysfs.cpp',\n> +    'dma_heaps.cpp',\n>      'fence.cpp',\n>      'formats.cpp',\n>      'framebuffer.cpp',\n> diff --git a/src/libcamera/pipeline/rpi/vc4/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build\n> index cdb049c5..386e2296 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/meson.build\n> +++ b/src/libcamera/pipeline/rpi/vc4/meson.build\n> @@ -1,7 +1,6 @@\n>  # SPDX-License-Identifier: CC0-1.0\n>  \n>  libcamera_sources += files([\n> -    'dma_heaps.cpp',\n>      'vc4.cpp',\n>  ])\n>  \n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 26102ea7..3a42e75e 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -12,12 +12,11 @@\n>  #include <libcamera/formats.h>\n>  \n>  #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/dma_heaps.h\"\n>  \n>  #include \"../common/pipeline_base.h\"\n>  #include \"../common/rpi_stream.h\"\n>  \n> -#include \"dma_heaps.h\"\n> -\n>  using namespace std::chrono_literals;\n>  \n>  namespace libcamera {\n> @@ -87,7 +86,7 @@ public:\n>  \tRPi::Device<Isp, 4> isp_;\n>  \n>  \t/* DMAHEAP allocation helper. */\n> -\tRPi::DmaHeap dmaHeap_;\n> +\tDmaHeap dmaHeap_;\n>  \tSharedFD lsTable_;\n>  \n>  \tstruct Config {","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 D03C7BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Jan 2024 14:20:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E6E362944;\n\tTue, 23 Jan 2024 15:20:19 +0100 (CET)","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 EDB86628E9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jan 2024 15:20:17 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 015A01890;\n\tTue, 23 Jan 2024 15:19:03 +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=\"XLANCGBx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706019544;\n\tbh=ErnDFM3oSiWZUXj1BXx7OMCekSHEukPHAEUep1EGm5Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XLANCGBxEukii7zRNq4xixgd3Zh3yvZ7QyvnhbednliYub227K6JdpyuJmR3O5YyX\n\txD20Lp5CaPR2SBE0V5ELuE1dphsUUvOsaibmdikHN3VmqWOgYfv5w6jjBaqOjMYjZm\n\tHYLmYWnwEvlCrBpZyaMnJ5UTs5M9HJYExZt/QPls=","Date":"Tue, 23 Jan 2024 16:20:16 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Hans de Goede <hdegoede@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","Message-ID":"<20240123142016.GB10679@pendragon.ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-3-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240113142218.28063-3-hdegoede@redhat.com>","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":"Maxime Ripard <mripard@redhat.com>, g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek <pavel@ucw.cz>,\n\tBryan O'Donoghue <bryan.odonoghue@linaro.org>, admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28638,"web_url":"https://patchwork.libcamera.org/comment/28638/","msgid":"<6e667268-cfd4-485e-8d92-18d604b0f01b@redhat.com>","date":"2024-02-06T20:33:10","subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 1/15/24 10:08, Naushir Patuck wrote:\n> Hi all,\n> \n> On Sat, 13 Jan 2024 at 22:34, Kieran Bingham via libcamera-devel\n> <libcamera-devel@lists.libcamera.org> wrote:\n>>\n>> Quoting Hans de Goede via libcamera-devel (2024-01-13 14:22:02)\n>>> From: Andrey Konovalov <andrey.konovalov@linaro.org>\n>>>\n>>> DmaHeap class is useful outside the RPi pipeline handler too.\n>>>\n>>> Move dma_heaps.h and dma_heaps.cpp to common directories. Update\n>>> the build files and RPi vc4 pipeline handler accordingly.\n>>\n>> This looks fine to me, but I'd like to see an Acked-by: or Reviewed-by:\n>> from Raspberry Pi to move this.\n>>\n>> Naush, David, - any objections to promoting the DmaHeap code to core\n>> code?\n> \n> No objections from me.\n> \n> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> \n>>\n>> The next patch also extends it with a new feature, but I don't believe\n>> it impacts the RPi code base specifically.\n>>\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\nKieran, Naush, I'm adding your Reviewed-by to v3 of this series\nto indicate that you are ok with the move, but note that Andrey\nhas added documentation to the DmaHeap code for v3. So you\nmay want to review the new documentation parts when I post v3.\n\nRegards,\n\nHans\n\n\n\n\n>>\n>>>\n>>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>\n>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s\n>>> Tested-by: Pavel Machek <pavel@ucw.cz>\n>>> ---\n>>>  .../libcamera/internal}/dma_heaps.h            |  4 ----\n>>>  include/libcamera/internal/meson.build         |  1 +\n>>>  .../{pipeline/rpi/vc4 => }/dma_heaps.cpp       | 18 +++++++-----------\n>>>  src/libcamera/meson.build                      |  1 +\n>>>  src/libcamera/pipeline/rpi/vc4/meson.build     |  1 -\n>>>  src/libcamera/pipeline/rpi/vc4/vc4.cpp         |  5 ++---\n>>>  6 files changed, 11 insertions(+), 19 deletions(-)\n>>>  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (92%)\n>>>  rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (83%)\n>>>\n>>> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h b/include/libcamera/internal/dma_heaps.h\n>>> similarity index 92%\n>>> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n>>> rename to include/libcamera/internal/dma_heaps.h\n>>> index 0a4a8d86..cff8f140 100644\n>>> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h\n>>> +++ b/include/libcamera/internal/dma_heaps.h\n>>> @@ -13,8 +13,6 @@\n>>>\n>>>  namespace libcamera {\n>>>\n>>> -namespace RPi {\n>>> -\n>>>  class DmaHeap\n>>>  {\n>>>  public:\n>>> @@ -27,6 +25,4 @@ private:\n>>>         UniqueFD dmaHeapHandle_;\n>>>  };\n>>>\n>>> -} /* namespace RPi */\n>>> -\n>>>  } /* namespace libcamera */\n>>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n>>> index 7f1f3440..33eb0fb3 100644\n>>> --- a/include/libcamera/internal/meson.build\n>>> +++ b/include/libcamera/internal/meson.build\n>>> @@ -25,6 +25,7 @@ libcamera_internal_headers = files([\n>>>      'device_enumerator.h',\n>>>      'device_enumerator_sysfs.h',\n>>>      'device_enumerator_udev.h',\n>>> +    'dma_heaps.h',\n>>>      'formats.h',\n>>>      'framebuffer.h',\n>>>      'ipa_manager.h',\n>>> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp\n>>> similarity index 83%\n>>> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n>>> rename to src/libcamera/dma_heaps.cpp\n>>> index 317b1fc1..7444d9c2 100644\n>>> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp\n>>> +++ b/src/libcamera/dma_heaps.cpp\n>>> @@ -5,8 +5,6 @@\n>>>   * dma_heaps.h - Helper class for dma-heap allocations.\n>>>   */\n>>>\n>>> -#include \"dma_heaps.h\"\n>>> -\n>>>  #include <array>\n>>>  #include <fcntl.h>\n>>>  #include <linux/dma-buf.h>\n>>> @@ -16,6 +14,8 @@\n>>>\n>>>  #include <libcamera/base/log.h>\n>>>\n>>> +#include \"libcamera/internal/dma_heaps.h\"\n>>> +\n>>>  /*\n>>>   * /dev/dma-heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma\n>>>   * to only have to worry about importing.\n>>> @@ -30,9 +30,7 @@ static constexpr std::array<const char *, 2> heapNames = {\n>>>\n>>>  namespace libcamera {\n>>>\n>>> -LOG_DECLARE_CATEGORY(RPI)\n>>> -\n>>> -namespace RPi {\n>>> +LOG_DEFINE_CATEGORY(DmaHeap)\n>>>\n>>>  DmaHeap::DmaHeap()\n>>>  {\n>>> @@ -40,7 +38,7 @@ DmaHeap::DmaHeap()\n>>>                 int ret = ::open(name, O_RDWR | O_CLOEXEC, 0);\n>>>                 if (ret < 0) {\n>>>                         ret = errno;\n>>> -                       LOG(RPI, Debug) << \"Failed to open \" << name << \": \"\n>>> +                       LOG(DmaHeap, Debug) << \"Failed to open \" << name << \": \"\n>>>                                         << strerror(ret);\n>>>                         continue;\n>>>                 }\n>>> @@ -50,7 +48,7 @@ DmaHeap::DmaHeap()\n>>>         }\n>>>\n>>>         if (!dmaHeapHandle_.isValid())\n>>> -               LOG(RPI, Error) << \"Could not open any dmaHeap device\";\n>>> +               LOG(DmaHeap, Error) << \"Could not open any dmaHeap device\";\n>>>  }\n>>>\n>>>  DmaHeap::~DmaHeap() = default;\n>>> @@ -69,7 +67,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>>>\n>>>         ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n>>>         if (ret < 0) {\n>>> -               LOG(RPI, Error) << \"dmaHeap allocation failure for \"\n>>> +               LOG(DmaHeap, Error) << \"dmaHeap allocation failure for \"\n>>>                                 << name;\n>>>                 return {};\n>>>         }\n>>> @@ -77,7 +75,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>>>         UniqueFD allocFd(alloc.fd);\n>>>         ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n>>>         if (ret < 0) {\n>>> -               LOG(RPI, Error) << \"dmaHeap naming failure for \"\n>>> +               LOG(DmaHeap, Error) << \"dmaHeap naming failure for \"\n>>>                                 << name;\n>>>                 return {};\n>>>         }\n>>> @@ -85,6 +83,4 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n>>>         return allocFd;\n>>>  }\n>>>\n>>> -} /* namespace RPi */\n>>> -\n>>>  } /* namespace libcamera */\n>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>> index 45f63e93..3c5e43df 100644\n>>> --- a/src/libcamera/meson.build\n>>> +++ b/src/libcamera/meson.build\n>>> @@ -17,6 +17,7 @@ libcamera_sources = files([\n>>>      'delayed_controls.cpp',\n>>>      'device_enumerator.cpp',\n>>>      'device_enumerator_sysfs.cpp',\n>>> +    'dma_heaps.cpp',\n>>>      'fence.cpp',\n>>>      'formats.cpp',\n>>>      'framebuffer.cpp',\n>>> diff --git a/src/libcamera/pipeline/rpi/vc4/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build\n>>> index cdb049c5..386e2296 100644\n>>> --- a/src/libcamera/pipeline/rpi/vc4/meson.build\n>>> +++ b/src/libcamera/pipeline/rpi/vc4/meson.build\n>>> @@ -1,7 +1,6 @@\n>>>  # SPDX-License-Identifier: CC0-1.0\n>>>\n>>>  libcamera_sources += files([\n>>> -    'dma_heaps.cpp',\n>>>      'vc4.cpp',\n>>>  ])\n>>>\n>>> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n>>> index 26102ea7..3a42e75e 100644\n>>> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n>>> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n>>> @@ -12,12 +12,11 @@\n>>>  #include <libcamera/formats.h>\n>>>\n>>>  #include \"libcamera/internal/device_enumerator.h\"\n>>> +#include \"libcamera/internal/dma_heaps.h\"\n>>>\n>>>  #include \"../common/pipeline_base.h\"\n>>>  #include \"../common/rpi_stream.h\"\n>>>\n>>> -#include \"dma_heaps.h\"\n>>> -\n>>>  using namespace std::chrono_literals;\n>>>\n>>>  namespace libcamera {\n>>> @@ -87,7 +86,7 @@ public:\n>>>         RPi::Device<Isp, 4> isp_;\n>>>\n>>>         /* DMAHEAP allocation helper. */\n>>> -       RPi::DmaHeap dmaHeap_;\n>>> +       DmaHeap dmaHeap_;\n>>>         SharedFD lsTable_;\n>>>\n>>>         struct Config {\n>>> --\n>>> 2.43.0\n>>>\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 A3902BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Feb 2024 20:33:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B7BF362804;\n\tTue,  6 Feb 2024 21:33:19 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9A61261CBA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Feb 2024 21:33:17 +0100 (CET)","from mail-lj1-f199.google.com (mail-lj1-f199.google.com\n\t[209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-412-qdn0MoGCP3avtUUGuXKgXA-1; Tue, 06 Feb 2024 15:33:14 -0500","by mail-lj1-f199.google.com with SMTP id\n\t38308e7fff4ca-2d09fe39949so29961301fa.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 06 Feb 2024 12:33:14 -0800 (PST)","from ?IPV6:2001:1c00:2a07:3a01:6c4:9fb2:fbc:7029?\n\t(2001-1c00-2a07-3a01-06c4-9fb2-0fbc-7029.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:2a07:3a01:6c4:9fb2:fbc:7029])\n\tby smtp.gmail.com with ESMTPSA id\n\tp12-20020a17090628cc00b00a371c568978sm1517233ejd.150.2024.02.06.12.33.11\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 06 Feb 2024 12:33:12 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Fpib1M+b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1707251596;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=JDi6MU9cOTHWRKvlnRwB8/cP1/ZHiI9TTuLE7BgQJyk=;\n\tb=Fpib1M+bt/Hjxp2uUxvsCAcuAvxjEQ5Pbz4MXuzCgELslUzq7wGmdtx/HgA2k6Nddtx84G\n\tZ/JMXM876lvBWG54t99mkSlIRdb5SPkfrsk8gnRVlvOS7CbCzm+BrnsdJROjhRAtaJvb9p\n\t6+uaLXCMgXIUYPxtYKkebpNcg3sC/F4=","X-MC-Unique":"qdn0MoGCP3avtUUGuXKgXA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1707251593; x=1707856393;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=JDi6MU9cOTHWRKvlnRwB8/cP1/ZHiI9TTuLE7BgQJyk=;\n\tb=lxoEElAcYn/fXxlygaLLQ9wBumf4QwRz/ECVuLNzxDHO68JAi6UzcS0dzbWyfL3QC4\n\tpkDTggWig3sl7kX3AS8B/QOA/WM0d/kWMlUzsgGjEhQSl+JZ0LkQ3NBgf5P0I8A52HMK\n\tzUh0qjQRdZgrVCi/Aqv6NcRbrA9grzFvIuZL93jgVf3ChR7Vj9ExPHBFDnLRnfukvfNf\n\tGj9pvSh1hEzOaBpCdOYHD82QO13qcma8NjBYZLz/SQ2kbiv48Oys5NdCzh5HZjktgASM\n\tSMtEn1d+Ijq6F47BoGPwk+a7PlgWV1C2Msy0yd/KmMDJLBJWH0E5mSThPFwLgmlQRt9O\n\tYd3Q==","X-Gm-Message-State":"AOJu0YyErmiNUe1E/noKn8CFjg2Vg0iY2aSVhM6Wkpt+yYvqBMpWF0LB\n\tL9eNTj0i91MmwNmvwJYORwkeOyw72UUgXkwXXHZVGa9de34gjEzBxsgh53/iih5bdK9t/5hmkq2\n\tVdYfC4GhXD41WWzqAEX1sTB2G9RANlhjSaA3YktYz94I1EozxQ3SB3W6rKkPHkCMYLc/UEhE=","X-Received":["by 2002:a05:6512:33cc:b0:50e:a789:dd3b with SMTP id\n\td12-20020a05651233cc00b0050ea789dd3bmr3171307lfg.1.1707251592944; \n\tTue, 06 Feb 2024 12:33:12 -0800 (PST)","by 2002:a05:6512:33cc:b0:50e:a789:dd3b with SMTP id\n\td12-20020a05651233cc00b0050ea789dd3bmr3171295lfg.1.1707251592509; \n\tTue, 06 Feb 2024 12:33:12 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHhJtS5s9gko1PD24KTekI2PLFAcUt0EGriY8WGkS4VTAJ5B75i8YV1dqCKyTNbAXTTB8NU8w==","X-Forwarded-Encrypted":"i=0;\n\tAJvYcCWOAgJldUU5vFKfmBoDIJSBVLQs2Bi+6x8/7X9yOSlO5rGrafrY0imj56/HUynW6uwjvHmv6DxrjM7KXnrfu3vOR2zS+iXAR2iGHw3jtOP4pjUr3baxqwcsn9da0rITzW6b+SXMxQYznV/wa7xhXuaVwqNAZNypmfNf6c6dZybG6LNOxLZshFzNJ5F7rVNE30lv1rVOkV6i40luW7BpwfjfrXDp02RNObW7byH+r/1LDfOvk25TKxSWmuiRM4A6obaX/ugNBOpkwqcKcg9nHlDeNxjJi2XCEVgIOAq+Q20tzUpZ9PPBDiOXxAwEo9FPFrxXBbA2dPX9Z2U1QZBnuyHgHxpqouep8zT6XODETwsRSkpAV9Q=","Message-ID":"<6e667268-cfd4-485e-8d92-18d604b0f01b@redhat.com>","Date":"Tue, 6 Feb 2024 21:33:10 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move\n\tdma_heaps.[h, cpp] to common directories","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-3-hdegoede@redhat.com>\n\t<170518527196.3418233.36183636241911333@ping.linuxembedded.co.uk>\n\t<CAEmqJPr+_s9X01bcuvapnP8v-JZSCc=5b44Sk0ASWjFNPYbMaw@mail.gmail.com>","From":"Hans de Goede <hdegoede@redhat.com>","In-Reply-To":"<CAEmqJPr+_s9X01bcuvapnP8v-JZSCc=5b44Sk0ASWjFNPYbMaw@mail.gmail.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","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":"Maxime Ripard <mripard@redhat.com>, g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek <pavel@ucw.cz>,\n\tBryan O'Donoghue <bryan.odonoghue@linaro.org>, admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]