[PATCH v4] libcamera: debayer_cpu: Sync DMABUFs

Robert Mader robert.mader at collabora.com
Mon Sep 16 18:12:31 CEST 2024


Using `DMA_BUF_IOCTL_SYNC` is required for DMABUFs in order to ensure
correct output. Not doing so currently results in occasional tearing
and/or backlashes in GL/VK clients that use the buffers directly for
rendering.

An alternative approach to have the sync code in `MappedFrameBuffer` was
considered but rejected for now, in order to allow clients more
flexibility.

Signed-off-by: Robert Mader <robert.mader at collabora.com>

---

Changes in v4:
 - Fixed errno handling
 - Added sync flags to error message
 - Wrapped syncBufferForCPU() and timeDiff() into an anonymous namespace

Changes in v3:
 - add syncBufferForCPU() helper function

Changes in v2:
 - sync input buffer as well
 - update commit title accordingly
 - small changes to the commit message
 - linting fixes, should pass now
---
 src/libcamera/software_isp/debayer_cpu.cpp | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index 077f7f4b..96cf2c71 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -12,8 +12,11 @@
 #include "debayer_cpu.h"
 
 #include <stdlib.h>
+#include <sys/ioctl.h>
 #include <time.h>
 
+#include <linux/dma-buf.h>
+
 #include <libcamera/formats.h>
 
 #include "libcamera/internal/bayer_format.h"
@@ -718,12 +721,33 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)
 	}
 }
 
+namespace {
+
+static void syncBufferForCPU(FrameBuffer *buffer, uint64_t syncFlags)
+{
+	for (const FrameBuffer::Plane &plane : buffer->planes()) {
+		const int fd = plane.fd.get();
+		struct dma_buf_sync sync = { syncFlags };
+		int ret;
+
+		ret = ioctl(fd, DMA_BUF_IOCTL_SYNC, &sync);
+		if (ret < 0) {
+			ret = errno;
+			LOG(Debayer, Error)
+				<< "Syncing buffer FD " << fd << " with flags "
+				<< syncFlags << " failed: " << strerror(-ret);
+		}
+	}
+}
+
 static inline int64_t timeDiff(timespec &after, timespec &before)
 {
 	return (after.tv_sec - before.tv_sec) * 1000000000LL +
 	       (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;
 }
 
+} /* namespace */
+
 void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)
 {
 	timespec frameStartTime;
@@ -733,6 +757,9 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
 		clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);
 	}
 
+	syncBufferForCPU(input, DMA_BUF_SYNC_START | DMA_BUF_SYNC_READ);
+	syncBufferForCPU(output, DMA_BUF_SYNC_START | DMA_BUF_SYNC_WRITE);
+
 	green_ = params.green;
 	red_ = swapRedBlueGains_ ? params.blue : params.red;
 	blue_ = swapRedBlueGains_ ? params.red : params.blue;
@@ -760,6 +787,9 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
 
 	metadata.planes()[0].bytesused = out.planes()[0].size();
 
+	syncBufferForCPU(output, DMA_BUF_SYNC_END | DMA_BUF_SYNC_WRITE);
+	syncBufferForCPU(input, DMA_BUF_SYNC_END | DMA_BUF_SYNC_READ);
+
 	/* Measure before emitting signals */
 	if (measuredFrames_ < DebayerCpu::kLastFrameToMeasure &&
 	    ++measuredFrames_ > DebayerCpu::kFramesToSkip) {
-- 
2.46.0



More information about the libcamera-devel mailing list