[{"id":36267,"web_url":"https://patchwork.libcamera.org/comment/36267/","msgid":"<176056637877.162040.8522881115519080997@ping.linuxembedded.co.uk>","date":"2025-10-15T22:12:58","subject":"Re: [PATCH v3 13/39] libcamera: software_isp: Make output DMA sync\n\tcontingent","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Bryan O'Donoghue (2025-10-15 02:22:25)\n> The DMA sync output buffer from the GPU need only have its cache\n> invalidated if the CPU is going to modify the buffer. Right now this is\n> not required for gpuisp so only act on the output buffer if it is\n> non-null.\n> \n> Suggested-by: Robert Mader <robert.mader@collabora.com>\n> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>\n> ---\n>  src/libcamera/software_isp/debayer.cpp | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\n> index 847067aa..96737c45 100644\n> --- a/src/libcamera/software_isp/debayer.cpp\n> +++ b/src/libcamera/software_isp/debayer.cpp\n> @@ -223,8 +223,10 @@ void Debayer::dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *inpu\n>         for (const FrameBuffer::Plane &plane : input->planes())\n>                 dmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Read);\n>  \n\nCompletly nit-picking (and what you have is fine here so don't change\nunless /you/ prefer this) but here I might have done:\n\n\n{\n\tfor (const FrameBuffer::Plane &plane : input->planes())\n\t\tdmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Read);\n\n\t/* \n\t * Output buffer handling is optional as cache operations are expensive\n\t * and unnecessary on GPU ISP where the CPU does not write to\n\t * the output buffer.\n\t */\n\tif (!output)\n\t\treturn\n\n\tfor (const FrameBuffer::Plane &plane : output->planes())\n\t\tdmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Write);\n}\n\nEither way:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> -       for (const FrameBuffer::Plane &plane : output->planes())\n> -               dmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Write);\n> +       if (output) {\n> +               for (const FrameBuffer::Plane &plane : output->planes())\n> +                       dmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Write);\n> +       }\n>  }\n>  \n>  } /* namespace libcamera */\n> -- \n> 2.51.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 A5774BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 15 Oct 2025 22:13:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C797C605F3;\n\tThu, 16 Oct 2025 00:13:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B4CE605F3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Oct 2025 00:13:01 +0200 (CEST)","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 DD3E6558;\n\tThu, 16 Oct 2025 00:11:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"J6fGRdP7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760566282;\n\tbh=QVBwUWDPFI8pE6r+Spr7Pu1HZPgKMG0HuaFLjPvge98=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=J6fGRdP7fa/9ZnORRQyvC78AsOe7Sx+M2Ul4aH5JS0A4VP9G+ikGPcGbypp8JPohl\n\t9jYce5o4o0kFLG/i01af6LxT2WACFEEelg/NfS+RoOS7q1w1/5CmwiLI9zhhknc0FH\n\td3vAFhns9yyVlP6Ff5mRNk6dCQ76PUxa1g262kyc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251015012251.17508-14-bryan.odonoghue@linaro.org>","References":"<20251015012251.17508-1-bryan.odonoghue@linaro.org>\n\t<20251015012251.17508-14-bryan.odonoghue@linaro.org>","Subject":"Re: [PATCH v3 13/39] libcamera: software_isp: Make output DMA sync\n\tcontingent","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"hdegoede@redhat.com, mzamazal@redhat.com, bryan.odonoghue@linaro.org,\n\tbod.linux@nxsw.ie, Robert Mader <robert.mader@collabora.com>","To":"Bryan O'Donoghue <bryan.odonoghue@linaro.org>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 15 Oct 2025 23:12:58 +0100","Message-ID":"<176056637877.162040.8522881115519080997@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36299,"web_url":"https://patchwork.libcamera.org/comment/36299/","msgid":"<855xcfgs23.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-10-16T08:38:12","subject":"Re: [PATCH v3 13/39] libcamera: software_isp: Make output DMA sync\n\tcontingent","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Kieran Bingham <kieran.bingham@ideasonboard.com> writes:\n\n> Quoting Bryan O'Donoghue (2025-10-15 02:22:25)\n>> The DMA sync output buffer from the GPU need only have its cache\n>> invalidated if the CPU is going to modify the buffer. Right now this is\n>\n>> not required for gpuisp so only act on the output buffer if it is\n>> non-null.\n>> \n>> Suggested-by: Robert Mader <robert.mader@collabora.com>\n>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>\n>> ---\n>>  src/libcamera/software_isp/debayer.cpp | 6 ++++--\n>>  1 file changed, 4 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\n>> index 847067aa..96737c45 100644\n>> --- a/src/libcamera/software_isp/debayer.cpp\n>> +++ b/src/libcamera/software_isp/debayer.cpp\n>> @@ -223,8 +223,10 @@ void Debayer::dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *inpu\n>>         for (const FrameBuffer::Plane &plane : input->planes())\n>>                 dmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Read);\n>>  \n>\n> Completly nit-picking (and what you have is fine here so don't change\n> unless /you/ prefer this) but here I might have done:\n>\n>\n> {\n> \tfor (const FrameBuffer::Plane &plane : input->planes())\n> \t\tdmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Read);\n>\n> \t/* \n> \t * Output buffer handling is optional as cache operations are expensive\n> \t * and unnecessary on GPU ISP where the CPU does not write to\n> \t * the output buffer.\n> \t */\n\nSuch comments in the code are very useful, at least for non-experts\n(like me).  Voting for such a change.  With that:\n\nReviewed-by: Milan Zamazal <mzamazal@redhat.com>\n\n> \tif (!output)\n> \t\treturn\n>\n> \tfor (const FrameBuffer::Plane &plane : output->planes())\n> \t\tdmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Write);\n> }\n>\n> Either way:\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n>> -       for (const FrameBuffer::Plane &plane : output->planes())\n>> -               dmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Write);\n>> +       if (output) {\n>> +               for (const FrameBuffer::Plane &plane : output->planes())\n>> +                       dmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Write);\n>> +       }\n>>  }\n>>  \n>>  } /* namespace libcamera */\n>> -- \n>> 2.51.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 6ADE7C3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 16 Oct 2025 08:38:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4617C6066D;\n\tThu, 16 Oct 2025 10:38:21 +0200 (CEST)","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 9CF15600CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Oct 2025 10:38:18 +0200 (CEST)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-222-lumYoQqlMxmRI1NBdhNZ9w-1; Thu, 16 Oct 2025 04:38:16 -0400","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-47106720618so8625195e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Oct 2025 01:38:16 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-47114428dedsm12346795e9.7.2025.10.16.01.38.13\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 16 Oct 2025 01:38:13 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"MpHJdOas\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1760603897;\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=OsIkELFHd54j46Aw/OY50ua7FWWW93iJxH5yxuMBSTY=;\n\tb=MpHJdOasXUNzLGX0ARS+86GxP9fytw2HUyz6QVvnwpA02FaBYFUCYewXUg/CX+zbYF30f5\n\tEze/pdpo1ba3MHahLdTYAPJLWl792Ii6lOdXHupArvIg0kIScik36H4ttqzZSHFh51X49s\n\tFC0JI340HKqfaOEiUYhtnI87UZ/PzXQ=","X-MC-Unique":"lumYoQqlMxmRI1NBdhNZ9w-1","X-Mimecast-MFC-AGG-ID":"lumYoQqlMxmRI1NBdhNZ9w_1760603895","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1760603895; x=1761208695;\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=OsIkELFHd54j46Aw/OY50ua7FWWW93iJxH5yxuMBSTY=;\n\tb=ZKkQ9dxZWfKiDSSBZCtYeV/xwb4pOh137JHVJlvPDWIzmY7RZKbs4Bd6LrVa3cSjdf\n\t5bijb60UIuQpjj3mQd+cOzrMmZct2LgLzhdqkOnyrszRLk+rOu4ecDnjJAPeLnIsVroU\n\txISGipYZnIHlpElWKUE4ArEQyDYCwJtpkg2bBbZj02YfZnxbRgvnWG52tKALv888EYIN\n\t17CrFGEnpr+O5RFk9klmtE7JZiX6uAsPXuRaD7u/lJP2vkilrKmBoFC3hA7Q5U3/RqVK\n\t4541bEYKShomVYlhnKXiJJ20r1j0XsOp3RJOzyotR2REgtOWKUhpslQgXtvbkpLd2Idx\n\tJIzg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCU2Xe8sjeytT6Kz9QixchAS9lFvT0xkzOGOZGst6s9r1/a2efV+ELNOGiBytxn77/if1ZJpQ2hr44IDDbhu8g0=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxzwORfV/V7Cn4PED/Hq+JoVpSD9cjm/IorV+j6BCQL5I7yja+F\n\tcqtQAg6V0gl1xha8Sw+6/ViEd2j72ABG/F+Zbj2SVrYsxwDEY1sgdEqy7RYlssfkLn8DXvwZjup\n\tBuwEOIONMHZtY5zhfhR7PaPMiryrjlhBz2VIUlR0/DuwZr6MW203jRvnfuF/z+iBrxFk4pDSKQf\n\tg=","X-Gm-Gg":"ASbGncvBcY7z3iVlCd5c8Ryi61Oh0Wl3J32vybS1vbKyxlniKUMjsKBVF4wyjleuUxg\n\t3vvbqm6NeEBoYVvig5zrMUQBxSWHJbv28+liyimcP9I0fS5p0dAxlRqtSkx0pbpB7wASK9DCkob\n\tErbyPT2oRoT0GDIoa6t0CT/aJ7HQzbTwoc+SnxIM+wzpMq6Ip/lFiPEyUSaSsCWH18Jl3eDwrmE\n\th448gAbXhfpAYBvC3YtM4kBKApp8wMEyDcTv/wNWUj0yNvKVOPWd416SkbycQXEjYFbL9e3yTet\n\taPu2oDFn153unoEhdVh3xOy8qM7o35CJA9atUeaexY38MqQqm1fqyTJOcNZH3924BQjTkdsVFnz\n\t0K80/4I4KbcDDnMuKB11oG3xpBmo6g2TAFY6lwsxyHlQctmk8sAUS","X-Received":["by 2002:a05:600c:354f:b0:471:115e:624b with SMTP id\n\t5b1f17b1804b1-471115e6678mr9374875e9.17.1760603894914; \n\tThu, 16 Oct 2025 01:38:14 -0700 (PDT)","by 2002:a05:600c:354f:b0:471:115e:624b with SMTP id\n\t5b1f17b1804b1-471115e6678mr9374585e9.17.1760603894463; \n\tThu, 16 Oct 2025 01:38:14 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IFISC6sUS08QWt6NwpRoKo/IRhl0IW+GemRG34A83wqThVInKc6dzdC1JjSHPOJvHAEXdZjnQ==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Bryan O'Donoghue <bryan.odonoghue@linaro.org>,\n\tlibcamera-devel@lists.libcamera.org,  hdegoede@redhat.com,\n\tbod.linux@nxsw.ie,  Robert Mader <robert.mader@collabora.com>","Subject":"Re: [PATCH v3 13/39] libcamera: software_isp: Make output DMA sync\n\tcontingent","In-Reply-To":"<176056637877.162040.8522881115519080997@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Wed, 15 Oct 2025 23:12:58 +0100\")","References":"<20251015012251.17508-1-bryan.odonoghue@linaro.org>\n\t<20251015012251.17508-14-bryan.odonoghue@linaro.org>\n\t<176056637877.162040.8522881115519080997@ping.linuxembedded.co.uk>","Date":"Thu, 16 Oct 2025 10:38:12 +0200","Message-ID":"<855xcfgs23.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"2X2wQE4XUCvAgirqqvAd5i-rkmx2cD-Vz9S43H4f8_M_1760603895","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":"<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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]