[{"id":32329,"web_url":"https://patchwork.libcamera.org/comment/32329/","msgid":"<87ed35vt2u.fsf@redhat.com>","date":"2024-11-21T09:23:21","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Harvey Yang writes:\n\n> To synchronize CPU access with mmap and hardware access on DMA buffers,\n> using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> a helper class to allow users to sync buffers more easily.\n>\n> Signed-off-by: Han-Lin Chen \n> Co-developed-by: Harvey Yang \n> Signed-off-by: Harvey Yang \n\nOK for me.\n\nReviewed-by: Milan Zamazal \n\n> ---\n> .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> 2 files changed, 96 insertions(+)\n>\n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> index d2a0a0d19..e4073f668 100644\n> --- a/include/libcamera/internal/dma_buf_allocator.h\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -35,6 +36,26 @@ private:\n> \tDmaBufAllocatorFlag type_;\n> };\n> \n> +class DmaSyncer final\n> +{\n> +public:\n> +\tenum class SyncType {\n> +\t\tRead = 0,\n> +\t\tWrite,\n> +\t\tReadWrite,\n> +\t};\n> +\n> +\texplicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> +\n> +\t~DmaSyncer();\n> +\n> +private:\n> +\tvoid sync(uint64_t step);\n> +\n> +\tSharedFD fd_;\n> +\tuint64_t flags_ = 0;\n> +};\n> +\n> LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> index be6efb89f..c1c2103d6 100644\n> --- a/src/libcamera/dma_buf_allocator.cpp\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> \t\treturn allocFromHeap(name, size);\n> }\n> \n> +/**\n> + * \\class DmaSyncer\n> + * \\brief Helper class for dma-buf's synchronization\n> + *\n> + * This class wraps a userspace dma-buf's synchronization process with an\n> + * object's lifetime.\n> + *\n> + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> + * ISP.\n> + */\n> +\n> +/**\n> + * \\enum DmaSyncer::SyncType\n> + * \\brief Read and/or write access via the CPU map\n> + * \\var DmaSyncer::Read\n> + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> + * CPU map\n> + * \\var DmaSyncer::Write\n> + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> + * CPU map\n> + * \\var DmaSyncer::ReadWrite\n> + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> + * client via the CPU map\n> + */\n> +\n> +/**\n> + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n\nIs this line needed when the docstring is attached to the constructor?\n\n> + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> + * \\param[in] type Read and/or write access via the CPU map\n> + */\n> +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> +\t: fd_(fd)\n> +{\n> +\tswitch (type) {\n> +\tcase SyncType::Read:\n> +\t\tflags_ = DMA_BUF_SYNC_READ;\n> +\t\tbreak;\n> +\tcase SyncType::Write:\n> +\t\tflags_ = DMA_BUF_SYNC_WRITE;\n> +\t\tbreak;\n> +\tcase SyncType::ReadWrite:\n> +\t\tflags_ = DMA_BUF_SYNC_RW;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tsync(DMA_BUF_SYNC_START);\n> +}\n> +\n> +DmaSyncer::~DmaSyncer()\n> +{\n> +\tsync(DMA_BUF_SYNC_END);\n> +}\n> +\n> +void DmaSyncer::sync(uint64_t step)\n> +{\n> +\tstruct dma_buf_sync sync = {\n> +\t\t.flags = flags_ | step\n> +\t};\n> +\n> +\tint ret;\n> +\tdo {\n> +\t\tret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> +\t} while (ret && (errno == EINTR || errno == EAGAIN));\n> +\n> +\tif (ret) {\n> +\t\tret = errno;\n> +\t\tLOG(DmaBufAllocator, Error)\n> +\t\t\t<< \"Unable to sync dma fd: \" << fd_.get()\n> +\t\t\t<< \", err: \" << strerror(ret)\n> +\t\t\t<< \", flags: \" << sync.flags;\n> +\t}\n> +}\n> +\n> } /* namespace libcamera */","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 4955CC32F9\n\tfor ;\n\tThu, 21 Nov 2024 09:23:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 639E965FBC;\n\tThu, 21 Nov 2024 10:23:29 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F04A365F9F\n\tfor ;\n\tThu, 21 Nov 2024 10:23:26 +0100 (CET)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-295-CZtc7gEFPSyqiQosHHrFng-1; Thu, 21 Nov 2024 04:23:24 -0500","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-382450d158fso386747f8f.1\n\tfor ;\n\tThu, 21 Nov 2024 01:23:24 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-38254905328sm4465181f8f.2.2024.11.21.01.23.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 21 Nov 2024 01:23:22 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"KzsFAHQA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1732181006;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=oYhRgkTlVor4xOIe7rHw+2vMDUGtU8AYjdTgtfvIrcc=;\n\tb=KzsFAHQA8nTG+LMQiXiS+HY/oAInW3dNQFDfZbitiLmJvypH4UmLZkMft4TdoQwLcpRvBc\n\tCa15jNZ7vb+zpmWImOMpgV6doVrxUdpT0K7mik5jN+6ocfHAHkiJo3ruWE5GCZr7HEiNIs\n\tjDeIkig2vXciCVdWuMwCGHu3yq76TUo=","X-MC-Unique":"CZtc7gEFPSyqiQosHHrFng-1","X-Mimecast-MFC-AGG-ID":"CZtc7gEFPSyqiQosHHrFng","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1732181003; x=1732785803;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=oYhRgkTlVor4xOIe7rHw+2vMDUGtU8AYjdTgtfvIrcc=;\n\tb=ZRinOQ2Z8AVTMk3rNki8mZV9T++6xxB2BRM3ly86GWFttKfcQp5BeVHQX7q5Ask6o3\n\tw03tmGaSOj7+X/jm4Qv0wXuGqjCW8VDIiMOPvryacboEaaBh0O+xfZASSOVO48mSf1I+\n\t4Qrk9ODBCSOouCdu+0/rDZPMziNfqN4PU7+cWTB8TNHH+kCzT6e/i9FWOdcsw5opcooH\n\tVgjjBKyBIWT31V3sJ+c/VurkjpEMywMoqNvYQ7BEEqf06tierQSgdskMKU7A9dsabXz7\n\t+XAW185s7N/E+CpBTjK6EtIBNcHNs+idyjrsRUQKI598tWboLK9VEVyvjND73NVi/n+J\n\tTTbg==","X-Gm-Message-State":"AOJu0Yyxv0YklTUnigYudbfZqm5DXAnMHADCttPLfq1LE4mLtnCRexXu\n\t171WgerPJtGsvge5B85v7Ls7vFeliIL0MmuMtHNOdQtnDo/+p9+iKaFdF//uZT0na5facWuM2lO\n\tW/Cihhl47CZcfB1rSHXG+hy4vuuNSYIaFwAMfZJ+lOZXm7qmVwV/Wko5jO1K+m/yUkk2PlIA=","X-Received":["by 2002:a5d:47c9:0:b0:37d:51bc:3229 with SMTP id\n\tffacd0b85a97d-38254b18a85mr4365775f8f.51.1732181003119; \n\tThu, 21 Nov 2024 01:23:23 -0800 (PST)","by 2002:a5d:47c9:0:b0:37d:51bc:3229 with SMTP id\n\tffacd0b85a97d-38254b18a85mr4365760f8f.51.1732181002682; \n\tThu, 21 Nov 2024 01:23:22 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IGN9Z3o0UJiSt+65TP3plMyVnHGOmWDkKjksSRXgCAG3obi7WYwavFMmqLddubVVx6IXQPQgw==","From":"Milan Zamazal ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org, Han-Lin Chen\n\t","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","In-Reply-To":"<20241121055436.2502314-2-chenghaoyang@chromium.org> (Harvey\n\tYang's message of \"Thu, 21 Nov 2024 05:51:31 +0000\")","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>","Date":"Thu, 21 Nov 2024 10:23:21 +0100","Message-ID":"<87ed35vt2u.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"-2DZSZu77H4_78dpOg3n_yejJ7TyX5ILMCNjih1q3YU_1732181003","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32330,"web_url":"https://patchwork.libcamera.org/comment/32330/","msgid":"","date":"2024-11-21T09:27:31","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":148,"url":"https://patchwork.libcamera.org/api/people/148/","name":"Cheng-Hao Yang","email":"chenghaoyang@google.com"},"content":"Hi Milan,\n\nOn Thu, Nov 21, 2024 at 5:23 PM Milan Zamazal wrote:\n>\n> Harvey Yang writes:\n>\n> > To synchronize CPU access with mmap and hardware access on DMA buffers,\n> > using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> > a helper class to allow users to sync buffers more easily.\n> >\n> > Signed-off-by: Han-Lin Chen \n> > Co-developed-by: Harvey Yang \n> > Signed-off-by: Harvey Yang \n>\n> OK for me.\n>\n> Reviewed-by: Milan Zamazal \n>\n> > ---\n> > .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> > src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> > 2 files changed, 96 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> > index d2a0a0d19..e4073f668 100644\n> > --- a/include/libcamera/internal/dma_buf_allocator.h\n> > +++ b/include/libcamera/internal/dma_buf_allocator.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> >\n> > #include \n> > +#include \n> > #include \n> >\n> > namespace libcamera {\n> > @@ -35,6 +36,26 @@ private:\n> > DmaBufAllocatorFlag type_;\n> > };\n> >\n> > +class DmaSyncer final\n> > +{\n> > +public:\n> > + enum class SyncType {\n> > + Read = 0,\n> > + Write,\n> > + ReadWrite,\n> > + };\n> > +\n> > + explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> > +\n> > + ~DmaSyncer();\n> > +\n> > +private:\n> > + void sync(uint64_t step);\n> > +\n> > + SharedFD fd_;\n> > + uint64_t flags_ = 0;\n> > +};\n> > +\n> > LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> > index be6efb89f..c1c2103d6 100644\n> > --- a/src/libcamera/dma_buf_allocator.cpp\n> > +++ b/src/libcamera/dma_buf_allocator.cpp\n> > @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> > return allocFromHeap(name, size);\n> > }\n> >\n> > +/**\n> > + * \\class DmaSyncer\n> > + * \\brief Helper class for dma-buf's synchronization\n> > + *\n> > + * This class wraps a userspace dma-buf's synchronization process with an\n> > + * object's lifetime.\n> > + *\n> > + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> > + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> > + * ISP.\n> > + */\n> > +\n> > +/**\n> > + * \\enum DmaSyncer::SyncType\n> > + * \\brief Read and/or write access via the CPU map\n> > + * \\var DmaSyncer::Read\n> > + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> > + * CPU map\n> > + * \\var DmaSyncer::Write\n> > + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> > + * CPU map\n> > + * \\var DmaSyncer::ReadWrite\n> > + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> > + * client via the CPU map\n> > + */\n> > +\n> > +/**\n> > + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n>\n> Is this line needed when the docstring is attached to the constructor?\n\nNo, sorry...\nI'll remove it in the next version.\n\nLet's wait for others' comments though to prevent the spam.\n\nBR,\nHarvey\n\n>\n> > + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> > + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> > + * \\param[in] type Read and/or write access via the CPU map\n> > + */\n> > +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> > + : fd_(fd)\n> > +{\n> > + switch (type) {\n> > + case SyncType::Read:\n> > + flags_ = DMA_BUF_SYNC_READ;\n> > + break;\n> > + case SyncType::Write:\n> > + flags_ = DMA_BUF_SYNC_WRITE;\n> > + break;\n> > + case SyncType::ReadWrite:\n> > + flags_ = DMA_BUF_SYNC_RW;\n> > + break;\n> > + }\n> > +\n> > + sync(DMA_BUF_SYNC_START);\n> > +}\n> > +\n> > +DmaSyncer::~DmaSyncer()\n> > +{\n> > + sync(DMA_BUF_SYNC_END);\n> > +}\n> > +\n> > +void DmaSyncer::sync(uint64_t step)\n> > +{\n> > + struct dma_buf_sync sync = {\n> > + .flags = flags_ | step\n> > + };\n> > +\n> > + int ret;\n> > + do {\n> > + ret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> > + } while (ret && (errno == EINTR || errno == EAGAIN));\n> > +\n> > + if (ret) {\n> > + ret = errno;\n> > + LOG(DmaBufAllocator, Error)\n> > + << \"Unable to sync dma fd: \" << fd_.get()\n> > + << \", err: \" << strerror(ret)\n> > + << \", flags: \" << sync.flags;\n> > + }\n> > +}\n> > +\n> > } /* namespace libcamera */\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 6D394C326C\n\tfor ;\n\tThu, 21 Nov 2024 09:28:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DE9265FBF;\n\tThu, 21 Nov 2024 10:28:10 +0100 (CET)","from mail-ed1-x536.google.com (mail-ed1-x536.google.com\n\t[IPv6:2a00:1450:4864:20::536])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20F7B65FB0\n\tfor ;\n\tThu, 21 Nov 2024 10:28:08 +0100 (CET)","by mail-ed1-x536.google.com with SMTP id\n\t4fb4d7f45d1cf-5cfc18d5259so4832a12.1\n\tfor ;\n\tThu, 21 Nov 2024 01:28:08 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"TbWryh9z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20230601; t=1732181287; x=1732786087;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=0INzUd9G17kxtRwP8QA3V2FVCCSsoDF8a+Bl+lRRuKc=;\n\tb=TbWryh9zRKDYaml+3jTZnFxqOnoSmxAsOw7I0+9FN168JUGe0VgvMrWY4tDI6ISWGS\n\tOwL/4hwrbpi5KNQOiNWd0xcDnGE2aMHlg8UHykApslubtWK584lcK23VkZBIVCgekBcD\n\t3tcW3yoR0sTey2biMPxm6WM2tRiEaL6j/o/xZs1821Q+jeOM8rRXONwwrdAOCl5oL+cX\n\t+DlvQ7xg/OwL/ZQNzcoQRu4eT+HfzmcU5D0uasTOfGMkW+LMaK6Lj9JBIQzatcAexWVL\n\t8uBQc1Z9QuBkP7sT54WtDnMGoiSEsy4i1lPvcuqQS/cZIfkpOiuf4rCIjXkEXvW7zztU\n\tEBzA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1732181287; x=1732786087;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=0INzUd9G17kxtRwP8QA3V2FVCCSsoDF8a+Bl+lRRuKc=;\n\tb=Iz9ZDIYjD/L8IPafhxa6pN65U3/fLlsbSHja8FknxxfrboGDBX5mJakUgKju7aLIUi\n\toRHe+SqQukVOMwzBwEO8zPlTH5DbzrLa1LYUDUP+f2F9izBp2VfdFxi6u31YeEAmBCj/\n\tdaKwHAtfq2vj4vOjKHLnGnfPjBxu+GcQ89COZFAeFmdthSZSyBBnK8PD16WEv1ZGD3Xu\n\ttfwepOuthXDsLr+NIEzlRzcleipU41n98cDmba2htLpn2NMapfaxqt7IYt6OHVzz4S/A\n\ttWEOXOM7h700hKo/fOeT9cnBQUlCguq3tssGCkdj9bw985qxXfbRniTY3IMuz9OYrK7U\n\tHc1g==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXBlapMBoAi/wm1/O8lcDUGAoepEkbbfx2Q2hOLwap8GI/tMzfHzWAKvm/Jj62+hw+ynuJpUHyG0evCdiItWUI=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yw/aeVDq6wooLfjOkKG23N6DvsEGCJj5XdbkiW2nzqVfOpxqHth\n\ti+NBFcnUBVbNDYCKFdweCLtMvHBUEKLZHuKkqDV3iAdQoJBp/Iee0P7PgABIAiKBIXnSpAkg9Cz\n\tmy8uF4jfqUb/Fy3TGj685w0LC27TyGG/5XuZA","X-Gm-Gg":"ASbGncvYvF1h8DMRjH1/l3hMUhIcCoXNtc7QgHMUtnjSdIGRLCjKR9olXmSz+dUZ4FE\n\ttYf4fUpwCOXYd8RIhJq6BFBYQ8n+rqM9Ob/5n/VO4vPzhsMBuvO7bfp2qZiM=","X-Google-Smtp-Source":"AGHT+IE8dYO/MWPrM6xKubXxidyXbmRSUgY6GTCfIXFeaf2O2KREJ1/smqsNajwDxW0dNfAv3Dqe0o5kHo9Di/D+Ayg=","X-Received":"by 2002:aa7:c70d:0:b0:5d0:f39:9c7 with SMTP id\n\t4fb4d7f45d1cf-5d00f390a9cmr10005a12.7.1732181287418; \n\tThu, 21 Nov 2024 01:28:07 -0800 (PST)","MIME-Version":"1.0","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>\n\t<87ed35vt2u.fsf@redhat.com>","In-Reply-To":"<87ed35vt2u.fsf@redhat.com>","From":"Cheng-Hao Yang ","Date":"Thu, 21 Nov 2024 17:27:31 +0800","Message-ID":"","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","To":"Milan Zamazal ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32332,"web_url":"https://patchwork.libcamera.org/comment/32332/","msgid":"<173219151552.2888836.13318689374391887402@ping.linuxembedded.co.uk>","date":"2024-11-21T12:18:35","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Harvey Yang (2024-11-21 05:51:31)\n> To synchronize CPU access with mmap and hardware access on DMA buffers,\n> using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> a helper class to allow users to sync buffers more easily.\n> \n> Signed-off-by: Han-Lin Chen \n> Co-developed-by: Harvey Yang \n> Signed-off-by: Harvey Yang \n\nI think I'm biased here from the previous review, but I think this looks\nbetter ;-)\n\nReviewed-by: Kieran Bingham \n\n> ---\n> .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> 2 files changed, 96 insertions(+)\n> \n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> index d2a0a0d19..e4073f668 100644\n> --- a/include/libcamera/internal/dma_buf_allocator.h\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -35,6 +36,26 @@ private:\n> DmaBufAllocatorFlag type_;\n> };\n> \n> +class DmaSyncer final\n> +{\n> +public:\n> + enum class SyncType {\n> + Read = 0,\n> + Write,\n> + ReadWrite,\n> + };\n> +\n> + explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> +\n> + ~DmaSyncer();\n> +\n> +private:\n> + void sync(uint64_t step);\n> +\n> + SharedFD fd_;\n> + uint64_t flags_ = 0;\n> +};\n> +\n> LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> index be6efb89f..c1c2103d6 100644\n> --- a/src/libcamera/dma_buf_allocator.cpp\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> return allocFromHeap(name, size);\n> }\n> \n> +/**\n> + * \\class DmaSyncer\n> + * \\brief Helper class for dma-buf's synchronization\n> + *\n> + * This class wraps a userspace dma-buf's synchronization process with an\n> + * object's lifetime.\n> + *\n> + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> + * ISP.\n> + */\n> +\n> +/**\n> + * \\enum DmaSyncer::SyncType\n> + * \\brief Read and/or write access via the CPU map\n> + * \\var DmaSyncer::Read\n> + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> + * CPU map\n> + * \\var DmaSyncer::Write\n> + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> + * CPU map\n> + * \\var DmaSyncer::ReadWrite\n> + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> + * client via the CPU map\n> + */\n> +\n> +/**\n> + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n> + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> + * \\param[in] type Read and/or write access via the CPU map\n> + */\n> +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> + : fd_(fd)\n> +{\n> + switch (type) {\n> + case SyncType::Read:\n> + flags_ = DMA_BUF_SYNC_READ;\n> + break;\n> + case SyncType::Write:\n> + flags_ = DMA_BUF_SYNC_WRITE;\n> + break;\n> + case SyncType::ReadWrite:\n> + flags_ = DMA_BUF_SYNC_RW;\n> + break;\n> + }\n> +\n> + sync(DMA_BUF_SYNC_START);\n> +}\n> +\n> +DmaSyncer::~DmaSyncer()\n> +{\n> + sync(DMA_BUF_SYNC_END);\n> +}\n> +\n> +void DmaSyncer::sync(uint64_t step)\n> +{\n> + struct dma_buf_sync sync = {\n> + .flags = flags_ | step\n> + };\n> +\n> + int ret;\n> + do {\n> + ret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> + } while (ret && (errno == EINTR || errno == EAGAIN));\n> +\n> + if (ret) {\n> + ret = errno;\n> + LOG(DmaBufAllocator, Error)\n> + << \"Unable to sync dma fd: \" << fd_.get()\n> + << \", err: \" << strerror(ret)\n> + << \", flags: \" << sync.flags;\n> + }\n> +}\n> +\n> } /* namespace libcamera */\n> -- \n> 2.47.0.338.g60cca15819-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6E438C32F9\n\tfor ;\n\tThu, 21 Nov 2024 12:18:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 75FA165FC3;\n\tThu, 21 Nov 2024 13:18:40 +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 679A565F9F\n\tfor ;\n\tThu, 21 Nov 2024 13:18:38 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7118E219;\n\tThu, 21 Nov 2024 13:18:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BbFPzMUE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732191499;\n\tbh=J9stuCaisVtIvSiXzf6ICxfwpMC5QHKhNFKMhOVpndA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=BbFPzMUEtDkXZ1WCoylIlIwzDut1ram8wozmsG1JMX2EcBZeei88Q1ahaAHc3iyz1\n\tVYUTq4OycHj37Uutk37J8SFkUc39E3KIX1oZvn5pRoQR5hKnzr87w+7Pa9yEyX8g/2\n\tXn0bDDJkHd+cIyiBMW4enoD77fTI2AUTzxijq8fY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241121055436.2502314-2-chenghaoyang@chromium.org>","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","From":"Kieran Bingham ","Cc":"Harvey Yang ,\n\tHan-Lin Chen ","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 21 Nov 2024 12:18:35 +0000","Message-ID":"<173219151552.2888836.13318689374391887402@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32334,"web_url":"https://patchwork.libcamera.org/comment/32334/","msgid":"<173219160450.2888836.11562255598547424390@ping.linuxembedded.co.uk>","date":"2024-11-21T12:20:04","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Cheng-Hao Yang (2024-11-21 09:27:31)\n> Hi Milan,\n> \n> On Thu, Nov 21, 2024 at 5:23 PM Milan Zamazal wrote:\n> >\n> > Harvey Yang writes:\n> >\n> > > To synchronize CPU access with mmap and hardware access on DMA buffers,\n> > > using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> > > a helper class to allow users to sync buffers more easily.\n> > >\n> > > Signed-off-by: Han-Lin Chen \n> > > Co-developed-by: Harvey Yang \n> > > Signed-off-by: Harvey Yang \n> >\n> > OK for me.\n> >\n> > Reviewed-by: Milan Zamazal \n> >\n> > > ---\n> > > .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> > > src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> > > 2 files changed, 96 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> > > index d2a0a0d19..e4073f668 100644\n> > > --- a/include/libcamera/internal/dma_buf_allocator.h\n> > > +++ b/include/libcamera/internal/dma_buf_allocator.h\n> > > @@ -8,6 +8,7 @@\n> > > #pragma once\n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > >\n> > > namespace libcamera {\n> > > @@ -35,6 +36,26 @@ private:\n> > > DmaBufAllocatorFlag type_;\n> > > };\n> > >\n> > > +class DmaSyncer final\n> > > +{\n> > > +public:\n> > > + enum class SyncType {\n> > > + Read = 0,\n> > > + Write,\n> > > + ReadWrite,\n> > > + };\n> > > +\n> > > + explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> > > +\n> > > + ~DmaSyncer();\n> > > +\n> > > +private:\n> > > + void sync(uint64_t step);\n> > > +\n> > > + SharedFD fd_;\n> > > + uint64_t flags_ = 0;\n> > > +};\n> > > +\n> > > LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> > >\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> > > index be6efb89f..c1c2103d6 100644\n> > > --- a/src/libcamera/dma_buf_allocator.cpp\n> > > +++ b/src/libcamera/dma_buf_allocator.cpp\n> > > @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> > > return allocFromHeap(name, size);\n> > > }\n> > >\n> > > +/**\n> > > + * \\class DmaSyncer\n> > > + * \\brief Helper class for dma-buf's synchronization\n> > > + *\n> > > + * This class wraps a userspace dma-buf's synchronization process with an\n> > > + * object's lifetime.\n> > > + *\n> > > + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> > > + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> > > + * ISP.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\enum DmaSyncer::SyncType\n> > > + * \\brief Read and/or write access via the CPU map\n> > > + * \\var DmaSyncer::Read\n> > > + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> > > + * CPU map\n> > > + * \\var DmaSyncer::Write\n> > > + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> > > + * CPU map\n> > > + * \\var DmaSyncer::ReadWrite\n> > > + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> > > + * client via the CPU map\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n> >\n> > Is this line needed when the docstring is attached to the constructor?\n> \n> No, sorry...\n> I'll remove it in the next version.\n\nI do'nt think it breaks, but it could be redundant and a source of\nsomething that could bitrot so perhaps easier to remove the line.\n\n> \n> Let's wait for others' comments though to prevent the spam.\n\nIf there's no other comments, it could be removed while applying. Lets\ngive it a day or so to see.\n\n--\nKieran\n\n> \n> BR,\n> Harvey\n> \n> >\n> > > + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> > > + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> > > + * \\param[in] type Read and/or write access via the CPU map\n> > > + */\n> > > +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> > > + : fd_(fd)\n> > > +{\n> > > + switch (type) {\n> > > + case SyncType::Read:\n> > > + flags_ = DMA_BUF_SYNC_READ;\n> > > + break;\n> > > + case SyncType::Write:\n> > > + flags_ = DMA_BUF_SYNC_WRITE;\n> > > + break;\n> > > + case SyncType::ReadWrite:\n> > > + flags_ = DMA_BUF_SYNC_RW;\n> > > + break;\n> > > + }\n> > > +\n> > > + sync(DMA_BUF_SYNC_START);\n> > > +}\n> > > +\n> > > +DmaSyncer::~DmaSyncer()\n> > > +{\n> > > + sync(DMA_BUF_SYNC_END);\n> > > +}\n> > > +\n> > > +void DmaSyncer::sync(uint64_t step)\n> > > +{\n> > > + struct dma_buf_sync sync = {\n> > > + .flags = flags_ | step\n> > > + };\n> > > +\n> > > + int ret;\n> > > + do {\n> > > + ret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> > > + } while (ret && (errno == EINTR || errno == EAGAIN));\n> > > +\n> > > + if (ret) {\n> > > + ret = errno;\n> > > + LOG(DmaBufAllocator, Error)\n> > > + << \"Unable to sync dma fd: \" << fd_.get()\n> > > + << \", err: \" << strerror(ret)\n> > > + << \", flags: \" << sync.flags;\n> > > + }\n> > > +}\n> > > +\n> > > } /* namespace libcamera */\n> >\n> \n> \n> -- \n> BR,\n> Harvey Yang","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 D4EDBC32F9\n\tfor ;\n\tThu, 21 Nov 2024 12:20:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71B9D65FC7;\n\tThu, 21 Nov 2024 13:20:08 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E7FD65F9F\n\tfor ;\n\tThu, 21 Nov 2024 13:20:07 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 479A3219;\n\tThu, 21 Nov 2024 13:19:48 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VlPFKbYJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732191588;\n\tbh=wCUBUreWrLYrWzCceYJw8iEaXm04oYiyzS6mpRbHNJI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=VlPFKbYJzjZ9r9anIIDFfkOtwjBdxQqp5Aj3DZXviVYCUy6c66pygOC8cIw8S9vdS\n\t1cJ+l45/xfBxGFByONgbSWiElhMPK/wzOVpB2D1jxuoNZr05cVZ2fnAqV4HRxrVGxo\n\t+P1x4LvuXdq0bvAWN1I0zvZC1BoNfdDJ22WaexUA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>\n\t<87ed35vt2u.fsf@redhat.com>\n\t","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","From":"Kieran Bingham ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org,\n\tHan-Lin Chen ","To":"Cheng-Hao Yang ,\n\tMilan Zamazal ","Date":"Thu, 21 Nov 2024 12:20:04 +0000","Message-ID":"<173219160450.2888836.11562255598547424390@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32348,"web_url":"https://patchwork.libcamera.org/comment/32348/","msgid":"","date":"2024-11-25T06:15:45","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Kieran,\n\nOn Thu, Nov 21, 2024 at 8:20 PM Kieran Bingham\n wrote:\n>\n> Quoting Cheng-Hao Yang (2024-11-21 09:27:31)\n> > Hi Milan,\n> >\n> > On Thu, Nov 21, 2024 at 5:23 PM Milan Zamazal wrote:\n> > >\n> > > Harvey Yang writes:\n> > >\n> > > > To synchronize CPU access with mmap and hardware access on DMA buffers,\n> > > > using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> > > > a helper class to allow users to sync buffers more easily.\n> > > >\n> > > > Signed-off-by: Han-Lin Chen \n> > > > Co-developed-by: Harvey Yang \n> > > > Signed-off-by: Harvey Yang \n> > >\n> > > OK for me.\n> > >\n> > > Reviewed-by: Milan Zamazal \n> > >\n> > > > ---\n> > > > .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> > > > src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> > > > 2 files changed, 96 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> > > > index d2a0a0d19..e4073f668 100644\n> > > > --- a/include/libcamera/internal/dma_buf_allocator.h\n> > > > +++ b/include/libcamera/internal/dma_buf_allocator.h\n> > > > @@ -8,6 +8,7 @@\n> > > > #pragma once\n> > > >\n> > > > #include \n> > > > +#include \n> > > > #include \n> > > >\n> > > > namespace libcamera {\n> > > > @@ -35,6 +36,26 @@ private:\n> > > > DmaBufAllocatorFlag type_;\n> > > > };\n> > > >\n> > > > +class DmaSyncer final\n> > > > +{\n> > > > +public:\n> > > > + enum class SyncType {\n> > > > + Read = 0,\n> > > > + Write,\n> > > > + ReadWrite,\n> > > > + };\n> > > > +\n> > > > + explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> > > > +\n> > > > + ~DmaSyncer();\n> > > > +\n> > > > +private:\n> > > > + void sync(uint64_t step);\n> > > > +\n> > > > + SharedFD fd_;\n> > > > + uint64_t flags_ = 0;\n> > > > +};\n> > > > +\n> > > > LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> > > >\n> > > > } /* namespace libcamera */\n> > > > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> > > > index be6efb89f..c1c2103d6 100644\n> > > > --- a/src/libcamera/dma_buf_allocator.cpp\n> > > > +++ b/src/libcamera/dma_buf_allocator.cpp\n> > > > @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> > > > return allocFromHeap(name, size);\n> > > > }\n> > > >\n> > > > +/**\n> > > > + * \\class DmaSyncer\n> > > > + * \\brief Helper class for dma-buf's synchronization\n> > > > + *\n> > > > + * This class wraps a userspace dma-buf's synchronization process with an\n> > > > + * object's lifetime.\n> > > > + *\n> > > > + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> > > > + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> > > > + * ISP.\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\enum DmaSyncer::SyncType\n> > > > + * \\brief Read and/or write access via the CPU map\n> > > > + * \\var DmaSyncer::Read\n> > > > + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> > > > + * CPU map\n> > > > + * \\var DmaSyncer::Write\n> > > > + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> > > > + * CPU map\n> > > > + * \\var DmaSyncer::ReadWrite\n> > > > + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> > > > + * client via the CPU map\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n> > >\n> > > Is this line needed when the docstring is attached to the constructor?\n> >\n> > No, sorry...\n> > I'll remove it in the next version.\n>\n> I do'nt think it breaks, but it could be redundant and a source of\n> something that could bitrot so perhaps easier to remove the line.\n>\n> >\n> > Let's wait for others' comments though to prevent the spam.\n>\n> If there's no other comments, it could be removed while applying. Lets\n> give it a day or so to see.\n\nYeah, please help remove it while applying to prevent the spam,\nif there's no other comemnts. Thanks!\n\nBR,\nHarvey\n\n>\n> --\n> Kieran\n>\n> >\n> > BR,\n> > Harvey\n> >\n> > >\n> > > > + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> > > > + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> > > > + * \\param[in] type Read and/or write access via the CPU map\n> > > > + */\n> > > > +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> > > > + : fd_(fd)\n> > > > +{\n> > > > + switch (type) {\n> > > > + case SyncType::Read:\n> > > > + flags_ = DMA_BUF_SYNC_READ;\n> > > > + break;\n> > > > + case SyncType::Write:\n> > > > + flags_ = DMA_BUF_SYNC_WRITE;\n> > > > + break;\n> > > > + case SyncType::ReadWrite:\n> > > > + flags_ = DMA_BUF_SYNC_RW;\n> > > > + break;\n> > > > + }\n> > > > +\n> > > > + sync(DMA_BUF_SYNC_START);\n> > > > +}\n> > > > +\n> > > > +DmaSyncer::~DmaSyncer()\n> > > > +{\n> > > > + sync(DMA_BUF_SYNC_END);\n> > > > +}\n> > > > +\n> > > > +void DmaSyncer::sync(uint64_t step)\n> > > > +{\n> > > > + struct dma_buf_sync sync = {\n> > > > + .flags = flags_ | step\n> > > > + };\n> > > > +\n> > > > + int ret;\n> > > > + do {\n> > > > + ret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> > > > + } while (ret && (errno == EINTR || errno == EAGAIN));\n> > > > +\n> > > > + if (ret) {\n> > > > + ret = errno;\n> > > > + LOG(DmaBufAllocator, Error)\n> > > > + << \"Unable to sync dma fd: \" << fd_.get()\n> > > > + << \", err: \" << strerror(ret)\n> > > > + << \", flags: \" << sync.flags;\n> > > > + }\n> > > > +}\n> > > > +\n> > > > } /* namespace libcamera */\n> > >\n> >\n> >\n> > --\n> > BR,\n> > Harvey Yang","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 81475BD808\n\tfor ;\n\tMon, 25 Nov 2024 06:16:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5854F66008;\n\tMon, 25 Nov 2024 07:15:59 +0100 (CET)","from mail-lj1-x231.google.com (mail-lj1-x231.google.com\n\t[IPv6:2a00:1450:4864:20::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7371465FFA\n\tfor ;\n\tMon, 25 Nov 2024 07:15:57 +0100 (CET)","by mail-lj1-x231.google.com with SMTP id\n\t38308e7fff4ca-2ffc81cee68so707261fa.0\n\tfor ;\n\tSun, 24 Nov 2024 22:15:57 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"BHVwb3aT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1732515357; x=1733120157;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=GOtgRXsQ+FT4VpnvQghpLsKS3cUhF9+m8iEpUbKJ/h4=;\n\tb=BHVwb3aTuPdWP5itV3LKSh+ILL3Iuw7PjrKu+5xR9qHM3+cZxyjhlpiBZsoEdvV9FM\n\ttbTFaMJk8RWPEwG/6zHSswTmS1Hgg8f12xqcTdnatOolJBDDo6EJhq4pXWJ+5T0KKvuc\n\tSSAFK2i5gSiBlJEGycZB3itK41V9UpipMHvBQ=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1732515357; x=1733120157;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=GOtgRXsQ+FT4VpnvQghpLsKS3cUhF9+m8iEpUbKJ/h4=;\n\tb=X5CtnKysc486KAapxloKi/ARdLzXVhWA2q5rIORz2Jhekyex1FD1L2cpRi6znP+D/X\n\t8/SIdnATVJvUzClLp1z2S5o7391OCo6Pr9OQLK1CJ1tEVdCVg1EYyMiUbMUaPU5Z9PL/\n\t1lZ5OUTRKmPFZyK/QPu+cIkxfdxRS5rWSaOm4E4oMQctKhwPhbspykadtwRmrKNaTp2W\n\tXZviEuxB5fhQ1iS5YhvZnFMeAmacSGdMaOj14JjEhwF5IODQ1rZVFzZPm1e2RfbtHkQm\n\t3Dy+tYZpqUpKUYOTiUqUWzEphCWe6XA8JO1n/qauZ9DdVDQVw7o31IotTgvgT7jSOBU7\n\t4X9Q==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCX02kC0tFW9VoOOTB4B/Ddwo5Ufo7iqL/aMMEAHVkhvvsqgpxAhn01EKmZ/0zqL+uxfBHIc/xIQk7DPoANUqZA=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzWxeeY1n0x4Z40DUwWjslkhiJkzC43WClG5QGJaAgpMkx9RMo7\n\tcRwsZ+oY/nF7cjxz+BWJLaJMn54i+zFZnC0LduhqPXgo6n/PBpfi+nMhIlCAFfWRhpB6wjigSzS\n\txXPn3dKaJsVwXaJr9EoJ06T0h7oIxqHgX7e8FrZ0rEJ3pgN8=","X-Gm-Gg":"ASbGncvaoUXZD6sdD0K7Z70BE+ktmakIY5+rzCJKUINZ4eEOU7pHPEnYAH2+Bu/sRog\n\tpO2IXndac2eREyxhVlmC+xs44qD/bwIMs/VoU7iKZCnD9ZgTDSoPo+d7DPMQ=","X-Google-Smtp-Source":"AGHT+IFNQvIONU4w6LzLXioHbJdUVH39e8TAjx2KfTITI2C/pK1rlRBnLr7iBU5vmPA4upeCu4czUzwZmy61QvZJieE=","X-Received":"by 2002:a2e:bc8f:0:b0:2ff:c422:c52 with SMTP id\n\t38308e7fff4ca-2ffc4220e3bmr5567441fa.39.1732515356670;\n\tSun, 24 Nov 2024 22:15:56 -0800 (PST)","MIME-Version":"1.0","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>\n\t<87ed35vt2u.fsf@redhat.com>\n\t\n\t<173219160450.2888836.11562255598547424390@ping.linuxembedded.co.uk>","In-Reply-To":"<173219160450.2888836.11562255598547424390@ping.linuxembedded.co.uk>","From":"Cheng-Hao Yang ","Date":"Mon, 25 Nov 2024 14:15:45 +0800","Message-ID":"","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","To":"Kieran Bingham ","Cc":"Cheng-Hao Yang ,\n\tMilan Zamazal , \n\tlibcamera-devel@lists.libcamera.org,\n\tHan-Lin Chen ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32446,"web_url":"https://patchwork.libcamera.org/comment/32446/","msgid":"<20241128194856.GA22438@pendragon.ideasonboard.com>","date":"2024-11-28T19:48:56","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Nov 21, 2024 at 05:51:31AM +0000, Harvey Yang wrote:\n> To synchronize CPU access with mmap and hardware access on DMA buffers,\n> using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> a helper class to allow users to sync buffers more easily.\n> \n> Signed-off-by: Han-Lin Chen \n> Co-developed-by: Harvey Yang \n> Signed-off-by: Harvey Yang \n> ---\n> .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> 2 files changed, 96 insertions(+)\n> \n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> index d2a0a0d19..e4073f668 100644\n> --- a/include/libcamera/internal/dma_buf_allocator.h\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -35,6 +36,26 @@ private:\n> \tDmaBufAllocatorFlag type_;\n> };\n> \n> +class DmaSyncer final\n> +{\n> +public:\n> +\tenum class SyncType {\n> +\t\tRead = 0,\n> +\t\tWrite,\n> +\t\tReadWrite,\n> +\t};\n> +\n> +\texplicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n\nWhy is this calss copyable ?\n\n> +\n> +\t~DmaSyncer();\n> +\n> +private:\n> +\tvoid sync(uint64_t step);\n> +\n> +\tSharedFD fd_;\n> +\tuint64_t flags_ = 0;\n> +};\n> +\n> LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> index be6efb89f..c1c2103d6 100644\n> --- a/src/libcamera/dma_buf_allocator.cpp\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> \t\treturn allocFromHeap(name, size);\n> }\n> \n> +/**\n> + * \\class DmaSyncer\n> + * \\brief Helper class for dma-buf's synchronization\n> + *\n> + * This class wraps a userspace dma-buf's synchronization process with an\n> + * object's lifetime.\n> + *\n> + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> + * ISP.\n> + */\n> +\n> +/**\n> + * \\enum DmaSyncer::SyncType\n> + * \\brief Read and/or write access via the CPU map\n> + * \\var DmaSyncer::Read\n> + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> + * CPU map\n> + * \\var DmaSyncer::Write\n> + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> + * CPU map\n> + * \\var DmaSyncer::ReadWrite\n> + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> + * client via the CPU map\n> + */\n> +\n> +/**\n> + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n> + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> + * \\param[in] type Read and/or write access via the CPU map\n> + */\n> +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> +\t: fd_(fd)\n> +{\n> +\tswitch (type) {\n> +\tcase SyncType::Read:\n> +\t\tflags_ = DMA_BUF_SYNC_READ;\n> +\t\tbreak;\n> +\tcase SyncType::Write:\n> +\t\tflags_ = DMA_BUF_SYNC_WRITE;\n> +\t\tbreak;\n> +\tcase SyncType::ReadWrite:\n> +\t\tflags_ = DMA_BUF_SYNC_RW;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tsync(DMA_BUF_SYNC_START);\n> +}\n> +\n> +DmaSyncer::~DmaSyncer()\n> +{\n> +\tsync(DMA_BUF_SYNC_END);\n> +}\n> +\n> +void DmaSyncer::sync(uint64_t step)\n> +{\n> +\tstruct dma_buf_sync sync = {\n> +\t\t.flags = flags_ | step\n> +\t};\n> +\n> +\tint ret;\n> +\tdo {\n> +\t\tret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> +\t} while (ret && (errno == EINTR || errno == EAGAIN));\n> +\n> +\tif (ret) {\n> +\t\tret = errno;\n> +\t\tLOG(DmaBufAllocator, Error)\n> +\t\t\t<< \"Unable to sync dma fd: \" << fd_.get()\n> +\t\t\t<< \", err: \" << strerror(ret)\n> +\t\t\t<< \", flags: \" << sync.flags;\n> +\t}\n> +}\n> +\n> } /* namespace libcamera */","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 6E583BD78E\n\tfor ;\n\tThu, 28 Nov 2024 19:49:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5FADB65FD2;\n\tThu, 28 Nov 2024 20:49:09 +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 46A0265898\n\tfor ;\n\tThu, 28 Nov 2024 20:49:07 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 157D0526;\n\tThu, 28 Nov 2024 20:48:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XbJOR9Zy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732823323;\n\tbh=kyI1zVBN43ZEU2Wazr+rhqUPdyuNgLRFbxO9MiFfWd8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XbJOR9Zy+tZMfieVz0ODwpdDc6oUEy1+Fy3c0SR3gb7QtZLyvYT+C/BdORQ+LctLY\n\ti3h70iY2N/b92ZZ1adggzm0AjU0YZtLdQ5b/CcveDpCUMupMptKph89V4SmksVhK/c\n\tAcmAnd2t5MgpdNdTMWBmV+qnJ+2FtpM1jzKRA/bw=","Date":"Thu, 28 Nov 2024 21:48:56 +0200","From":"Laurent Pinchart ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org,\n\tHan-Lin Chen ","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","Message-ID":"<20241128194856.GA22438@pendragon.ideasonboard.com>","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241121055436.2502314-2-chenghaoyang@chromium.org>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32467,"web_url":"https://patchwork.libcamera.org/comment/32467/","msgid":"","date":"2024-11-29T18:56:27","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Laurent,\n\nOn Fri, Nov 29, 2024 at 3:49 AM Laurent Pinchart\n wrote:\n>\n> On Thu, Nov 21, 2024 at 05:51:31AM +0000, Harvey Yang wrote:\n> > To synchronize CPU access with mmap and hardware access on DMA buffers,\n> > using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> > a helper class to allow users to sync buffers more easily.\n> >\n> > Signed-off-by: Han-Lin Chen \n> > Co-developed-by: Harvey Yang \n> > Signed-off-by: Harvey Yang \n> > ---\n> > .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> > src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> > 2 files changed, 96 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> > index d2a0a0d19..e4073f668 100644\n> > --- a/include/libcamera/internal/dma_buf_allocator.h\n> > +++ b/include/libcamera/internal/dma_buf_allocator.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> >\n> > #include \n> > +#include \n> > #include \n> >\n> > namespace libcamera {\n> > @@ -35,6 +36,26 @@ private:\n> > DmaBufAllocatorFlag type_;\n> > };\n> >\n> > +class DmaSyncer final\n> > +{\n> > +public:\n> > + enum class SyncType {\n> > + Read = 0,\n> > + Write,\n> > + ReadWrite,\n> > + };\n> > +\n> > + explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n>\n> Why is this calss copyable ?\n\nThanks for the catch.\nUploaded patch `DmaBufAllocator: Make DmaSyncer non-copyable`.\nPlease take a look.\n\nBR,\nHarvey\n\n>\n> > +\n> > + ~DmaSyncer();\n> > +\n> > +private:\n> > + void sync(uint64_t step);\n> > +\n> > + SharedFD fd_;\n> > + uint64_t flags_ = 0;\n> > +};\n> > +\n> > LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> > index be6efb89f..c1c2103d6 100644\n> > --- a/src/libcamera/dma_buf_allocator.cpp\n> > +++ b/src/libcamera/dma_buf_allocator.cpp\n> > @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> > return allocFromHeap(name, size);\n> > }\n> >\n> > +/**\n> > + * \\class DmaSyncer\n> > + * \\brief Helper class for dma-buf's synchronization\n> > + *\n> > + * This class wraps a userspace dma-buf's synchronization process with an\n> > + * object's lifetime.\n> > + *\n> > + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> > + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> > + * ISP.\n> > + */\n> > +\n> > +/**\n> > + * \\enum DmaSyncer::SyncType\n> > + * \\brief Read and/or write access via the CPU map\n> > + * \\var DmaSyncer::Read\n> > + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> > + * CPU map\n> > + * \\var DmaSyncer::Write\n> > + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> > + * CPU map\n> > + * \\var DmaSyncer::ReadWrite\n> > + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> > + * client via the CPU map\n> > + */\n> > +\n> > +/**\n> > + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n> > + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> > + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> > + * \\param[in] type Read and/or write access via the CPU map\n> > + */\n> > +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> > + : fd_(fd)\n> > +{\n> > + switch (type) {\n> > + case SyncType::Read:\n> > + flags_ = DMA_BUF_SYNC_READ;\n> > + break;\n> > + case SyncType::Write:\n> > + flags_ = DMA_BUF_SYNC_WRITE;\n> > + break;\n> > + case SyncType::ReadWrite:\n> > + flags_ = DMA_BUF_SYNC_RW;\n> > + break;\n> > + }\n> > +\n> > + sync(DMA_BUF_SYNC_START);\n> > +}\n> > +\n> > +DmaSyncer::~DmaSyncer()\n> > +{\n> > + sync(DMA_BUF_SYNC_END);\n> > +}\n> > +\n> > +void DmaSyncer::sync(uint64_t step)\n> > +{\n> > + struct dma_buf_sync sync = {\n> > + .flags = flags_ | step\n> > + };\n> > +\n> > + int ret;\n> > + do {\n> > + ret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> > + } while (ret && (errno == EINTR || errno == EAGAIN));\n> > +\n> > + if (ret) {\n> > + ret = errno;\n> > + LOG(DmaBufAllocator, Error)\n> > + << \"Unable to sync dma fd: \" << fd_.get()\n> > + << \", err: \" << strerror(ret)\n> > + << \", flags: \" << sync.flags;\n> > + }\n> > +}\n> > +\n> > } /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 707E0C31E9\n\tfor ;\n\tFri, 29 Nov 2024 18:56:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 180856603D;\n\tFri, 29 Nov 2024 19:56:41 +0100 (CET)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 570CB66025\n\tfor ;\n\tFri, 29 Nov 2024 19:56:39 +0100 (CET)","by mail-lj1-x235.google.com with SMTP id\n\t38308e7fff4ca-2ffa8092e34so26103341fa.1\n\tfor ;\n\tFri, 29 Nov 2024 10:56:39 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"H1CQKBoC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1732906598; x=1733511398;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=zvzveXHOa9io8WAwvBm3mjKWOT7HLbsa9ATPxHIs3OE=;\n\tb=H1CQKBoCCDWYLRCHF0sg05jG5rjb16BlflHcPeU5xUykCr5jn0fE+brqDuV/5yyFod\n\t2AYNdEBxGx5T/ZSvOJT0iG7OzQzxgHZ4rwid8/lC2jfZUqepRl/jwt/2ZFAl0QZ0fK+L\n\tcyaPb7GJFe1b6RN5veh8R54EQlGP2vM+HYmuI=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1732906598; x=1733511398;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=zvzveXHOa9io8WAwvBm3mjKWOT7HLbsa9ATPxHIs3OE=;\n\tb=gFppH8r/wsEm41kb2+DVnR2eUgjQ2/e9VaWEZTxQzc5H/bknaSU1uMyAGoPj8ghAR5\n\tLNB835WnDwzAxsvGZVGhW5ytzJssdDhEvt9IA9JzNpMz5i1tTez/uu3ccFAlC+mBlME9\n\tux0kJBOI/onO/Gc5dt1+tAQPGhWGyfqucrmIQJwHY7zIcZ0/9Sy7oPOgNaA8chEajsTT\n\t5WuY6sq9coRlSkMxewpxYZjcUlSlrwCG6ER/sC2Wp81iUmwPIpNJMvMi7DPGElOGGwPq\n\tVoGyHtzcGNK4v2rZd5/5fRmOPIxnPj/LtCrA5FMcCN4CzCM6dFcWtLlK89HWhapRyISG\n\t+c/g==","X-Gm-Message-State":"AOJu0YziGN9Cag+d/9mNmv+X5jsf/qbd8PDwnieMv0O5wP16j7TueuQF\n\tYKtKtrGXWMjCrPKotMpPLBH/9Rbow2nduAUn76fpexejWTyFaZuzLV6hi4oFA0Tw66MbF7kqFpJ\n\tUyyn+KVxBXc8WRjzv2TxSF2hO7IeQGM3ProKf6/5+YsCIhxEv2g==","X-Gm-Gg":"ASbGncvdT6wyOJdi7B/Y8wAo09l3BJ9TOeWd6yYKk6ErT9KVvFHKk8qv8dPcCjuz3kr\n\tfjqaJF2AQgFiwArRPH3Biob9nQgP6C7CS2Fz6uzpW3A0nZ3K2gCkJGNw5BtXy3STY","X-Google-Smtp-Source":"AGHT+IF1452HqXLqoLgptQeslpRwGO0wsxHLk0xCMeB1jT53Z7DqKfoIMz6h0dC8uUjmBd9EDPsLBpvtDtX23UHLyqk=","X-Received":"by 2002:a05:651c:b25:b0:2ff:8f67:bc69 with SMTP id\n\t38308e7fff4ca-2ffd5eb0a5amr84145951fa.0.1732906598635;\n\tFri, 29 Nov 2024 10:56:38 -0800 (PST)","MIME-Version":"1.0","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>\n\t<20241128194856.GA22438@pendragon.ideasonboard.com>","In-Reply-To":"<20241128194856.GA22438@pendragon.ideasonboard.com>","From":"Cheng-Hao Yang ","Date":"Sat, 30 Nov 2024 02:56:27 +0800","Message-ID":"","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32469,"web_url":"https://patchwork.libcamera.org/comment/32469/","msgid":"<20241129212303.GA2652@pendragon.ideasonboard.com>","date":"2024-11-29T21:23:03","subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sat, Nov 30, 2024 at 02:56:27AM +0800, Cheng-Hao Yang wrote:\n> On Fri, Nov 29, 2024 at 3:49 AM Laurent Pinchart wrote:\n> > On Thu, Nov 21, 2024 at 05:51:31AM +0000, Harvey Yang wrote:\n> > > To synchronize CPU access with mmap and hardware access on DMA buffers,\n> > > using `DMA_BUF_IOCTL_SYNC` is required. This patch adds a function and\n> > > a helper class to allow users to sync buffers more easily.\n> > >\n> > > Signed-off-by: Han-Lin Chen \n> > > Co-developed-by: Harvey Yang \n> > > Signed-off-by: Harvey Yang \n> > > ---\n> > > .../libcamera/internal/dma_buf_allocator.h | 21 ++++++\n> > > src/libcamera/dma_buf_allocator.cpp | 75 +++++++++++++++++++\n> > > 2 files changed, 96 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> > > index d2a0a0d19..e4073f668 100644\n> > > --- a/include/libcamera/internal/dma_buf_allocator.h\n> > > +++ b/include/libcamera/internal/dma_buf_allocator.h\n> > > @@ -8,6 +8,7 @@\n> > > #pragma once\n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > >\n> > > namespace libcamera {\n> > > @@ -35,6 +36,26 @@ private:\n> > > DmaBufAllocatorFlag type_;\n> > > };\n> > >\n> > > +class DmaSyncer final\n> > > +{\n> > > +public:\n> > > + enum class SyncType {\n> > > + Read = 0,\n> > > + Write,\n> > > + ReadWrite,\n> > > + };\n> > > +\n> > > + explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> >\n> > Why is this calss copyable ?\n> \n> Thanks for the catch.\n> Uploaded patch `DmaBufAllocator: Make DmaSyncer non-copyable`.\n> Please take a look.\n\nThank you, I'll review that.\n\n> > > +\n> > > + ~DmaSyncer();\n> > > +\n> > > +private:\n> > > + void sync(uint64_t step);\n> > > +\n> > > + SharedFD fd_;\n> > > + uint64_t flags_ = 0;\n> > > +};\n> > > +\n> > > LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> > >\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> > > index be6efb89f..c1c2103d6 100644\n> > > --- a/src/libcamera/dma_buf_allocator.cpp\n> > > +++ b/src/libcamera/dma_buf_allocator.cpp\n> > > @@ -205,4 +205,79 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> > > return allocFromHeap(name, size);\n> > > }\n> > >\n> > > +/**\n> > > + * \\class DmaSyncer\n> > > + * \\brief Helper class for dma-buf's synchronization\n> > > + *\n> > > + * This class wraps a userspace dma-buf's synchronization process with an\n> > > + * object's lifetime.\n> > > + *\n> > > + * It's used when the user needs to access a dma-buf with CPU, mostly mapped\n> > > + * with MappedFrameBuffer, so that the buffer is synchronized between CPU and\n> > > + * ISP.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\enum DmaSyncer::SyncType\n> > > + * \\brief Read and/or write access via the CPU map\n> > > + * \\var DmaSyncer::Read\n> > > + * \\brief Indicates that the mapped dma-buf will be read by the client via the\n> > > + * CPU map\n> > > + * \\var DmaSyncer::Write\n> > > + * \\brief Indicates that the mapped dm-buf will be written by the client via the\n> > > + * CPU map\n> > > + * \\var DmaSyncer::ReadWrite\n> > > + * \\brief Indicates that the mapped dma-buf will be read and written by the\n> > > + * client via the CPU map\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn DmaSyncer::DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite)\n> > > + * \\brief Construct a DmaSyncer with a dma-buf's fd and the access type\n> > > + * \\param[in] fd The dma-buf's file descriptor to synchronize\n> > > + * \\param[in] type Read and/or write access via the CPU map\n> > > + */\n> > > +DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> > > + : fd_(fd)\n> > > +{\n> > > + switch (type) {\n> > > + case SyncType::Read:\n> > > + flags_ = DMA_BUF_SYNC_READ;\n> > > + break;\n> > > + case SyncType::Write:\n> > > + flags_ = DMA_BUF_SYNC_WRITE;\n> > > + break;\n> > > + case SyncType::ReadWrite:\n> > > + flags_ = DMA_BUF_SYNC_RW;\n> > > + break;\n> > > + }\n> > > +\n> > > + sync(DMA_BUF_SYNC_START);\n> > > +}\n> > > +\n> > > +DmaSyncer::~DmaSyncer()\n> > > +{\n> > > + sync(DMA_BUF_SYNC_END);\n> > > +}\n> > > +\n> > > +void DmaSyncer::sync(uint64_t step)\n> > > +{\n> > > + struct dma_buf_sync sync = {\n> > > + .flags = flags_ | step\n> > > + };\n> > > +\n> > > + int ret;\n> > > + do {\n> > > + ret = ioctl(fd_.get(), DMA_BUF_IOCTL_SYNC, &sync);\n> > > + } while (ret && (errno == EINTR || errno == EAGAIN));\n> > > +\n> > > + if (ret) {\n> > > + ret = errno;\n> > > + LOG(DmaBufAllocator, Error)\n> > > + << \"Unable to sync dma fd: \" << fd_.get()\n> > > + << \", err: \" << strerror(ret)\n> > > + << \", flags: \" << sync.flags;\n> > > + }\n> > > +}\n> > > +\n> > > } /* namespace libcamera */","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 199D4C31E9\n\tfor ;\n\tFri, 29 Nov 2024 21:23:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 44B7C6603E;\n\tFri, 29 Nov 2024 22:23:16 +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 DB66E66025\n\tfor ;\n\tFri, 29 Nov 2024 22:23:14 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DC9E2A57;\n\tFri, 29 Nov 2024 22:22:49 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OWOh0Yqo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732915370;\n\tbh=IsVNjJiBI2CoIuYfVukn7ajREVTXmQxQKqP2G9LuqPo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OWOh0Yqoxr9AjE6Evs0gFMASB6NanfXEvhg1myzR7P54AABnMgstxwfRbU6Z6Zd+a\n\tiBXj6XDUCQp0wwG+WMPSi0kFiwil8aNRS559wEAzAsm9OJTewuStmUa93XUBuErd3v\n\tCKoQTFZMJElL/UCfSsG+PmKPonIHnwUYNnjjUV1o=","Date":"Fri, 29 Nov 2024 23:23:03 +0200","From":"Laurent Pinchart ","To":"Cheng-Hao Yang ","Cc":"libcamera-devel@lists.libcamera.org,\n\tHan-Lin Chen ","Subject":"Re: [PATCH v3 1/2] DmaBufAllocator: Add Dma Buffer synchronization\n\tfunction & helper class","Message-ID":"<20241129212303.GA2652@pendragon.ideasonboard.com>","References":"<20241121055436.2502314-1-chenghaoyang@chromium.org>\n\t<20241121055436.2502314-2-chenghaoyang@chromium.org>\n\t<20241128194856.GA22438@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]