* [Intel-gfx] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
@ 2022-12-02 12:02 ` Matthew Auld
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2022-12-02 12:02 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Andrzej Hajda, Nirmoy Das
So we can use this across different tests.
v2
- Add docs for everything (Petri)
- Add missing copyright and fix headers slightly (Kamil)
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
---
.../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
lib/dmabuf_sync_file.h | 26 +++
lib/meson.build | 1 +
tests/dmabuf_sync_file.c | 133 +----------
5 files changed, 243 insertions(+), 129 deletions(-)
create mode 100644 lib/dmabuf_sync_file.c
create mode 100644 lib/dmabuf_sync_file.h
diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
index 1ce842f4..102c8a89 100644
--- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
+++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
@@ -15,6 +15,7 @@
<chapter>
<title>API Reference</title>
+ <xi:include href="xml/dmabuf_sync_file.xml"/>
<xi:include href="xml/drmtest.xml"/>
<xi:include href="xml/igt_alsa.xml"/>
<xi:include href="xml/igt_audio.xml"/>
diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
new file mode 100644
index 00000000..bcce2486
--- /dev/null
+++ b/lib/dmabuf_sync_file.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include "igt.h"
+#include "igt_vgem.h"
+#include "sw_sync.h"
+
+#include "dmabuf_sync_file.h"
+
+/**
+ * SECTION: dmabuf_sync_file
+ * @short_description: DMABUF importing/exporting fencing support library
+ * @title: DMABUF Sync File
+ * @include: dmabuf_sync_file.h
+ */
+
+struct igt_dma_buf_sync_file {
+ __u32 flags;
+ __s32 fd;
+};
+
+#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
+#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
+
+/**
+ * has_dmabuf_export_sync_file:
+ * @fd: The open drm fd
+ *
+ * Check if the kernel supports exporting a sync file from dmabuf.
+ */
+bool has_dmabuf_export_sync_file(int fd)
+{
+ struct vgem_bo bo;
+ int dmabuf, ret;
+ struct igt_dma_buf_sync_file arg;
+
+ bo.width = 1;
+ bo.height = 1;
+ bo.bpp = 32;
+ vgem_create(fd, &bo);
+
+ dmabuf = prime_handle_to_fd(fd, bo.handle);
+ gem_close(fd, bo.handle);
+
+ arg.flags = DMA_BUF_SYNC_WRITE;
+ arg.fd = -1;
+
+ ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
+ close(dmabuf);
+ igt_assert(ret == 0 || errno == ENOTTY);
+
+ return ret == 0;
+}
+
+/**
+ * dmabuf_export_sync_file:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ *
+ * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
+ * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
+ * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
+ */
+int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
+{
+ struct igt_dma_buf_sync_file arg;
+
+ arg.flags = flags;
+ arg.fd = -1;
+ do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
+
+ return arg.fd;
+}
+
+/**
+ * has_dmabuf_import_sync_file:
+ * @fd: The open drm fd
+ *
+ * Check if the kernel supports importing a sync file into a dmabuf.
+ */
+bool has_dmabuf_import_sync_file(int fd)
+{
+ struct vgem_bo bo;
+ int dmabuf, timeline, fence, ret;
+ struct igt_dma_buf_sync_file arg;
+
+ bo.width = 1;
+ bo.height = 1;
+ bo.bpp = 32;
+ vgem_create(fd, &bo);
+
+ dmabuf = prime_handle_to_fd(fd, bo.handle);
+ gem_close(fd, bo.handle);
+
+ timeline = sw_sync_timeline_create();
+ fence = sw_sync_timeline_create_fence(timeline, 1);
+ sw_sync_timeline_inc(timeline, 1);
+
+ arg.flags = DMA_BUF_SYNC_RW;
+ arg.fd = fence;
+
+ ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
+ close(dmabuf);
+ close(fence);
+ igt_assert(ret == 0 || errno == ENOTTY);
+
+ return ret == 0;
+}
+
+/**
+ * dmabuf_import_sync_file:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ * @sync_fd: The sync file (i.e our fence) to import
+ *
+ * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
+ * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
+ * importing the @sync_fd as a read or write fence.
+ */
+void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
+{
+ struct igt_dma_buf_sync_file arg;
+
+ arg.flags = flags;
+ arg.fd = sync_fd;
+ do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
+}
+
+/**
+ * dmabuf_import_timeline_fence:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ * @timeline: The sync file timeline where the new fence is created
+ * @seqno: The sequence number to use for the fence
+ *
+ * Create a new fence as part of @timeline, and import as a sync file into the
+ * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
+ * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
+ * or write fence.
+ */
+void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
+ int timeline, uint32_t seqno)
+{
+ int fence;
+
+ fence = sw_sync_timeline_create_fence(timeline, seqno);
+ dmabuf_import_sync_file(dmabuf, flags, fence);
+ close(fence);
+}
+
+/**
+ * dmabuf_busy:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ *
+ * Check if the fences in the dmabuf are still busy. The @flags should at least
+ * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
+ * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
+ * we care about both.
+ */
+bool dmabuf_busy(int dmabuf, uint32_t flags)
+{
+ struct pollfd pfd = { .fd = dmabuf };
+
+ /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
+ * else poll() may return a non-zero value if there are only read
+ * fences because POLLIN is ready even if POLLOUT isn't.
+ */
+ if (flags & DMA_BUF_SYNC_WRITE)
+ pfd.events |= POLLOUT;
+ else if (flags & DMA_BUF_SYNC_READ)
+ pfd.events |= POLLIN;
+
+ return poll(&pfd, 1, 0) == 0;
+}
+
+/**
+ * sync_file_busy:
+ * @sync_file: The sync file to check
+ *
+ * Check if the @sync_file is still busy or not.
+ */
+bool sync_file_busy(int sync_file)
+{
+ struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
+ return poll(&pfd, 1, 0) == 0;
+}
+
+/**
+ * dmabuf_sync_file_busy:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ *
+ * Export the current fences in @dmabuf as a sync file and check if still busy.
+ * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
+ * to specify which fences are to be exported from the @dmabuf and checked if
+ * busy. Or DMA_BUF_SYNC_RW if we care about both.
+ */
+bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
+{
+ int sync_file;
+ bool busy;
+
+ sync_file = dmabuf_export_sync_file(dmabuf, flags);
+ busy = sync_file_busy(sync_file);
+ close(sync_file);
+
+ return busy;
+}
diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
new file mode 100644
index 00000000..d642ff30
--- /dev/null
+++ b/lib/dmabuf_sync_file.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef DMABUF_SYNC_FILE_H
+#define DMABUF_SYNC_FILE_H
+
+#ifdef __linux__
+#include <linux/dma-buf.h>
+#endif
+#include <sys/poll.h>
+#include <stdbool.h>
+#include <stdint.h>
+
+bool has_dmabuf_export_sync_file(int fd);
+bool has_dmabuf_import_sync_file(int fd);
+int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
+void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
+void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
+ int timeline, uint32_t seqno);
+bool dmabuf_busy(int dmabuf, uint32_t flags);
+bool sync_file_busy(int sync_file);
+bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
+
+#endif
diff --git a/lib/meson.build b/lib/meson.build
index cef2d0ff..896d5733 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -1,5 +1,6 @@
lib_sources = [
'drmtest.c',
+ 'dmabuf_sync_file.c',
'huc_copy.c',
'i915/gem.c',
'i915/gem_context.c',
diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
index 2179a76d..25bb6ad7 100644
--- a/tests/dmabuf_sync_file.c
+++ b/tests/dmabuf_sync_file.c
@@ -1,140 +1,15 @@
// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
#include "igt.h"
#include "igt_vgem.h"
#include "sw_sync.h"
-
-#include <linux/dma-buf.h>
-#include <sys/poll.h>
+#include "dmabuf_sync_file.h"
IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
-struct igt_dma_buf_sync_file {
- __u32 flags;
- __s32 fd;
-};
-
-#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
-#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
-
-static bool has_dmabuf_export_sync_file(int fd)
-{
- struct vgem_bo bo;
- int dmabuf, ret;
- struct igt_dma_buf_sync_file arg;
-
- bo.width = 1;
- bo.height = 1;
- bo.bpp = 32;
- vgem_create(fd, &bo);
-
- dmabuf = prime_handle_to_fd(fd, bo.handle);
- gem_close(fd, bo.handle);
-
- arg.flags = DMA_BUF_SYNC_WRITE;
- arg.fd = -1;
-
- ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
- close(dmabuf);
- igt_assert(ret == 0 || errno == ENOTTY);
-
- return ret == 0;
-}
-
-static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
-{
- struct igt_dma_buf_sync_file arg;
-
- arg.flags = flags;
- arg.fd = -1;
- do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
-
- return arg.fd;
-}
-
-static bool has_dmabuf_import_sync_file(int fd)
-{
- struct vgem_bo bo;
- int dmabuf, timeline, fence, ret;
- struct igt_dma_buf_sync_file arg;
-
- bo.width = 1;
- bo.height = 1;
- bo.bpp = 32;
- vgem_create(fd, &bo);
-
- dmabuf = prime_handle_to_fd(fd, bo.handle);
- gem_close(fd, bo.handle);
-
- timeline = sw_sync_timeline_create();
- fence = sw_sync_timeline_create_fence(timeline, 1);
- sw_sync_timeline_inc(timeline, 1);
-
- arg.flags = DMA_BUF_SYNC_RW;
- arg.fd = fence;
-
- ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
- close(dmabuf);
- close(fence);
- igt_assert(ret == 0 || errno == ENOTTY);
-
- return ret == 0;
-}
-
-static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
-{
- struct igt_dma_buf_sync_file arg;
-
- arg.flags = flags;
- arg.fd = sync_fd;
- do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
-}
-
-static void
-dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
- int timeline, uint32_t seqno)
-{
- int fence;
-
- fence = sw_sync_timeline_create_fence(timeline, seqno);
- dmabuf_import_sync_file(dmabuf, flags, fence);
- close(fence);
-}
-
-static bool dmabuf_busy(int dmabuf, uint32_t flags)
-{
- struct pollfd pfd = { .fd = dmabuf };
-
- /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
- * else poll() may return a non-zero value if there are only read
- * fences because POLLIN is ready even if POLLOUT isn't.
- */
- if (flags & DMA_BUF_SYNC_WRITE)
- pfd.events |= POLLOUT;
- else if (flags & DMA_BUF_SYNC_READ)
- pfd.events |= POLLIN;
-
- return poll(&pfd, 1, 0) == 0;
-}
-
-static bool sync_file_busy(int sync_file)
-{
- struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
- return poll(&pfd, 1, 0) == 0;
-}
-
-static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
-{
- int sync_file;
- bool busy;
-
- sync_file = dmabuf_export_sync_file(dmabuf, flags);
- busy = sync_file_busy(sync_file);
- close(sync_file);
-
- return busy;
-}
-
static void test_export_basic(int fd)
{
struct vgem_bo bo;
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
@ 2022-12-02 12:02 ` Matthew Auld
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2022-12-02 12:02 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Petri Latvala, Nirmoy Das
So we can use this across different tests.
v2
- Add docs for everything (Petri)
- Add missing copyright and fix headers slightly (Kamil)
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
---
.../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
lib/dmabuf_sync_file.h | 26 +++
lib/meson.build | 1 +
tests/dmabuf_sync_file.c | 133 +----------
5 files changed, 243 insertions(+), 129 deletions(-)
create mode 100644 lib/dmabuf_sync_file.c
create mode 100644 lib/dmabuf_sync_file.h
diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
index 1ce842f4..102c8a89 100644
--- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
+++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
@@ -15,6 +15,7 @@
<chapter>
<title>API Reference</title>
+ <xi:include href="xml/dmabuf_sync_file.xml"/>
<xi:include href="xml/drmtest.xml"/>
<xi:include href="xml/igt_alsa.xml"/>
<xi:include href="xml/igt_audio.xml"/>
diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
new file mode 100644
index 00000000..bcce2486
--- /dev/null
+++ b/lib/dmabuf_sync_file.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include "igt.h"
+#include "igt_vgem.h"
+#include "sw_sync.h"
+
+#include "dmabuf_sync_file.h"
+
+/**
+ * SECTION: dmabuf_sync_file
+ * @short_description: DMABUF importing/exporting fencing support library
+ * @title: DMABUF Sync File
+ * @include: dmabuf_sync_file.h
+ */
+
+struct igt_dma_buf_sync_file {
+ __u32 flags;
+ __s32 fd;
+};
+
+#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
+#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
+
+/**
+ * has_dmabuf_export_sync_file:
+ * @fd: The open drm fd
+ *
+ * Check if the kernel supports exporting a sync file from dmabuf.
+ */
+bool has_dmabuf_export_sync_file(int fd)
+{
+ struct vgem_bo bo;
+ int dmabuf, ret;
+ struct igt_dma_buf_sync_file arg;
+
+ bo.width = 1;
+ bo.height = 1;
+ bo.bpp = 32;
+ vgem_create(fd, &bo);
+
+ dmabuf = prime_handle_to_fd(fd, bo.handle);
+ gem_close(fd, bo.handle);
+
+ arg.flags = DMA_BUF_SYNC_WRITE;
+ arg.fd = -1;
+
+ ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
+ close(dmabuf);
+ igt_assert(ret == 0 || errno == ENOTTY);
+
+ return ret == 0;
+}
+
+/**
+ * dmabuf_export_sync_file:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ *
+ * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
+ * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
+ * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
+ */
+int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
+{
+ struct igt_dma_buf_sync_file arg;
+
+ arg.flags = flags;
+ arg.fd = -1;
+ do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
+
+ return arg.fd;
+}
+
+/**
+ * has_dmabuf_import_sync_file:
+ * @fd: The open drm fd
+ *
+ * Check if the kernel supports importing a sync file into a dmabuf.
+ */
+bool has_dmabuf_import_sync_file(int fd)
+{
+ struct vgem_bo bo;
+ int dmabuf, timeline, fence, ret;
+ struct igt_dma_buf_sync_file arg;
+
+ bo.width = 1;
+ bo.height = 1;
+ bo.bpp = 32;
+ vgem_create(fd, &bo);
+
+ dmabuf = prime_handle_to_fd(fd, bo.handle);
+ gem_close(fd, bo.handle);
+
+ timeline = sw_sync_timeline_create();
+ fence = sw_sync_timeline_create_fence(timeline, 1);
+ sw_sync_timeline_inc(timeline, 1);
+
+ arg.flags = DMA_BUF_SYNC_RW;
+ arg.fd = fence;
+
+ ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
+ close(dmabuf);
+ close(fence);
+ igt_assert(ret == 0 || errno == ENOTTY);
+
+ return ret == 0;
+}
+
+/**
+ * dmabuf_import_sync_file:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ * @sync_fd: The sync file (i.e our fence) to import
+ *
+ * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
+ * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
+ * importing the @sync_fd as a read or write fence.
+ */
+void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
+{
+ struct igt_dma_buf_sync_file arg;
+
+ arg.flags = flags;
+ arg.fd = sync_fd;
+ do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
+}
+
+/**
+ * dmabuf_import_timeline_fence:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ * @timeline: The sync file timeline where the new fence is created
+ * @seqno: The sequence number to use for the fence
+ *
+ * Create a new fence as part of @timeline, and import as a sync file into the
+ * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
+ * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
+ * or write fence.
+ */
+void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
+ int timeline, uint32_t seqno)
+{
+ int fence;
+
+ fence = sw_sync_timeline_create_fence(timeline, seqno);
+ dmabuf_import_sync_file(dmabuf, flags, fence);
+ close(fence);
+}
+
+/**
+ * dmabuf_busy:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ *
+ * Check if the fences in the dmabuf are still busy. The @flags should at least
+ * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
+ * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
+ * we care about both.
+ */
+bool dmabuf_busy(int dmabuf, uint32_t flags)
+{
+ struct pollfd pfd = { .fd = dmabuf };
+
+ /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
+ * else poll() may return a non-zero value if there are only read
+ * fences because POLLIN is ready even if POLLOUT isn't.
+ */
+ if (flags & DMA_BUF_SYNC_WRITE)
+ pfd.events |= POLLOUT;
+ else if (flags & DMA_BUF_SYNC_READ)
+ pfd.events |= POLLIN;
+
+ return poll(&pfd, 1, 0) == 0;
+}
+
+/**
+ * sync_file_busy:
+ * @sync_file: The sync file to check
+ *
+ * Check if the @sync_file is still busy or not.
+ */
+bool sync_file_busy(int sync_file)
+{
+ struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
+ return poll(&pfd, 1, 0) == 0;
+}
+
+/**
+ * dmabuf_sync_file_busy:
+ * @dmabuf: The dmabuf fd
+ * @flags: The flags to control the behaviour
+ *
+ * Export the current fences in @dmabuf as a sync file and check if still busy.
+ * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
+ * to specify which fences are to be exported from the @dmabuf and checked if
+ * busy. Or DMA_BUF_SYNC_RW if we care about both.
+ */
+bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
+{
+ int sync_file;
+ bool busy;
+
+ sync_file = dmabuf_export_sync_file(dmabuf, flags);
+ busy = sync_file_busy(sync_file);
+ close(sync_file);
+
+ return busy;
+}
diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
new file mode 100644
index 00000000..d642ff30
--- /dev/null
+++ b/lib/dmabuf_sync_file.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef DMABUF_SYNC_FILE_H
+#define DMABUF_SYNC_FILE_H
+
+#ifdef __linux__
+#include <linux/dma-buf.h>
+#endif
+#include <sys/poll.h>
+#include <stdbool.h>
+#include <stdint.h>
+
+bool has_dmabuf_export_sync_file(int fd);
+bool has_dmabuf_import_sync_file(int fd);
+int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
+void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
+void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
+ int timeline, uint32_t seqno);
+bool dmabuf_busy(int dmabuf, uint32_t flags);
+bool sync_file_busy(int sync_file);
+bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
+
+#endif
diff --git a/lib/meson.build b/lib/meson.build
index cef2d0ff..896d5733 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -1,5 +1,6 @@
lib_sources = [
'drmtest.c',
+ 'dmabuf_sync_file.c',
'huc_copy.c',
'i915/gem.c',
'i915/gem_context.c',
diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
index 2179a76d..25bb6ad7 100644
--- a/tests/dmabuf_sync_file.c
+++ b/tests/dmabuf_sync_file.c
@@ -1,140 +1,15 @@
// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
#include "igt.h"
#include "igt_vgem.h"
#include "sw_sync.h"
-
-#include <linux/dma-buf.h>
-#include <sys/poll.h>
+#include "dmabuf_sync_file.h"
IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
-struct igt_dma_buf_sync_file {
- __u32 flags;
- __s32 fd;
-};
-
-#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
-#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
-
-static bool has_dmabuf_export_sync_file(int fd)
-{
- struct vgem_bo bo;
- int dmabuf, ret;
- struct igt_dma_buf_sync_file arg;
-
- bo.width = 1;
- bo.height = 1;
- bo.bpp = 32;
- vgem_create(fd, &bo);
-
- dmabuf = prime_handle_to_fd(fd, bo.handle);
- gem_close(fd, bo.handle);
-
- arg.flags = DMA_BUF_SYNC_WRITE;
- arg.fd = -1;
-
- ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
- close(dmabuf);
- igt_assert(ret == 0 || errno == ENOTTY);
-
- return ret == 0;
-}
-
-static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
-{
- struct igt_dma_buf_sync_file arg;
-
- arg.flags = flags;
- arg.fd = -1;
- do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
-
- return arg.fd;
-}
-
-static bool has_dmabuf_import_sync_file(int fd)
-{
- struct vgem_bo bo;
- int dmabuf, timeline, fence, ret;
- struct igt_dma_buf_sync_file arg;
-
- bo.width = 1;
- bo.height = 1;
- bo.bpp = 32;
- vgem_create(fd, &bo);
-
- dmabuf = prime_handle_to_fd(fd, bo.handle);
- gem_close(fd, bo.handle);
-
- timeline = sw_sync_timeline_create();
- fence = sw_sync_timeline_create_fence(timeline, 1);
- sw_sync_timeline_inc(timeline, 1);
-
- arg.flags = DMA_BUF_SYNC_RW;
- arg.fd = fence;
-
- ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
- close(dmabuf);
- close(fence);
- igt_assert(ret == 0 || errno == ENOTTY);
-
- return ret == 0;
-}
-
-static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
-{
- struct igt_dma_buf_sync_file arg;
-
- arg.flags = flags;
- arg.fd = sync_fd;
- do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
-}
-
-static void
-dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
- int timeline, uint32_t seqno)
-{
- int fence;
-
- fence = sw_sync_timeline_create_fence(timeline, seqno);
- dmabuf_import_sync_file(dmabuf, flags, fence);
- close(fence);
-}
-
-static bool dmabuf_busy(int dmabuf, uint32_t flags)
-{
- struct pollfd pfd = { .fd = dmabuf };
-
- /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
- * else poll() may return a non-zero value if there are only read
- * fences because POLLIN is ready even if POLLOUT isn't.
- */
- if (flags & DMA_BUF_SYNC_WRITE)
- pfd.events |= POLLOUT;
- else if (flags & DMA_BUF_SYNC_READ)
- pfd.events |= POLLIN;
-
- return poll(&pfd, 1, 0) == 0;
-}
-
-static bool sync_file_busy(int sync_file)
-{
- struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
- return poll(&pfd, 1, 0) == 0;
-}
-
-static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
-{
- int sync_file;
- bool busy;
-
- sync_file = dmabuf_export_sync_file(dmabuf, flags);
- busy = sync_file_busy(sync_file);
- close(sync_file);
-
- return busy;
-}
-
static void test_export_basic(int fd)
{
struct vgem_bo bo;
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH i-g-t v2 2/2] tests/i915/gem_exec_balancer: exercise dmabuf import
2022-12-02 12:02 ` [igt-dev] " Matthew Auld
@ 2022-12-02 12:02 ` Matthew Auld
-1 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2022-12-02 12:02 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Andrzej Hajda, Nirmoy Das
With parallel submission it should be easy to get a fence array as the
output fence. Try importing this into dma-buf reservation object, to see
if anything explodes.
v2: (Kamil)
- Use ifdef __linux__ for linux headers
- Add igt_describe() for new test
References: https://gitlab.freedesktop.org/drm/intel/-/issues/7532
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
tests/i915/gem_exec_balancer.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index 4300dbd1..d7acdca1 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -27,6 +27,7 @@
#include <sys/signal.h>
#include <poll.h>
+#include "dmabuf_sync_file.h"
#include "i915/gem.h"
#include "i915/gem_engine_topology.h"
#include "i915/gem_create.h"
@@ -2856,6 +2857,7 @@ static void logical_sort_siblings(int i915,
#define PARALLEL_SUBMIT_FENCE (0x1 << 3)
#define PARALLEL_CONTEXTS (0x1 << 4)
#define PARALLEL_VIRTUAL (0x1 << 5)
+#define PARALLEL_OUT_FENCE_DMABUF (0x1 << 6)
static void parallel_thread(int i915, unsigned int flags,
struct i915_engine_class_instance *siblings,
@@ -2871,6 +2873,8 @@ static void parallel_thread(int i915, unsigned int flags,
uint32_t target_bo_idx = 0;
uint32_t first_bb_idx = 1;
intel_ctx_cfg_t cfg;
+ uint32_t dmabuf_handle;
+ int dmabuf;
igt_assert(bb_per_execbuf < 32);
@@ -2924,11 +2928,20 @@ static void parallel_thread(int i915, unsigned int flags,
execbuf.buffers_ptr = to_user_pointer(obj);
execbuf.rsvd1 = ctx->id;
+ if (flags & PARALLEL_OUT_FENCE_DMABUF) {
+ dmabuf_handle = gem_create(i915, 4096);
+ dmabuf = prime_handle_to_fd(i915, dmabuf_handle);
+ }
+
for (n = 0; n < PARALLEL_BB_LOOP_COUNT; ++n) {
execbuf.flags &= ~0x3full;
gem_execbuf_wr(i915, &execbuf);
if (flags & PARALLEL_OUT_FENCE) {
+ if (flags & PARALLEL_OUT_FENCE_DMABUF)
+ dmabuf_import_sync_file(dmabuf, DMA_BUF_SYNC_WRITE,
+ execbuf.rsvd2 >> 32);
+
igt_assert_eq(sync_fence_wait(execbuf.rsvd2 >> 32,
1000), 0);
igt_assert_eq(sync_fence_status(execbuf.rsvd2 >> 32), 1);
@@ -2959,6 +2972,11 @@ static void parallel_thread(int i915, unsigned int flags,
if (fence)
close(fence);
+ if (flags & PARALLEL_OUT_FENCE_DMABUF) {
+ gem_close(i915, dmabuf_handle);
+ close(dmabuf);
+ }
+
check_bo(i915, obj[target_bo_idx].handle,
bb_per_execbuf * PARALLEL_BB_LOOP_COUNT, true);
@@ -3420,6 +3438,11 @@ igt_main
igt_subtest("parallel-out-fence")
parallel(i915, PARALLEL_OUT_FENCE);
+ igt_describe("Regression test to check that dmabuf imported sync file can handle fence array");
+ igt_subtest("parallel-dmabuf-import-out-fence")
+ parallel(i915, PARALLEL_OUT_FENCE |
+ PARALLEL_OUT_FENCE_DMABUF);
+
igt_subtest("parallel-keep-in-fence")
parallel(i915, PARALLEL_OUT_FENCE | PARALLEL_IN_FENCE);
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH i-g-t v2 2/2] tests/i915/gem_exec_balancer: exercise dmabuf import
@ 2022-12-02 12:02 ` Matthew Auld
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2022-12-02 12:02 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Nirmoy Das
With parallel submission it should be easy to get a fence array as the
output fence. Try importing this into dma-buf reservation object, to see
if anything explodes.
v2: (Kamil)
- Use ifdef __linux__ for linux headers
- Add igt_describe() for new test
References: https://gitlab.freedesktop.org/drm/intel/-/issues/7532
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
tests/i915/gem_exec_balancer.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index 4300dbd1..d7acdca1 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -27,6 +27,7 @@
#include <sys/signal.h>
#include <poll.h>
+#include "dmabuf_sync_file.h"
#include "i915/gem.h"
#include "i915/gem_engine_topology.h"
#include "i915/gem_create.h"
@@ -2856,6 +2857,7 @@ static void logical_sort_siblings(int i915,
#define PARALLEL_SUBMIT_FENCE (0x1 << 3)
#define PARALLEL_CONTEXTS (0x1 << 4)
#define PARALLEL_VIRTUAL (0x1 << 5)
+#define PARALLEL_OUT_FENCE_DMABUF (0x1 << 6)
static void parallel_thread(int i915, unsigned int flags,
struct i915_engine_class_instance *siblings,
@@ -2871,6 +2873,8 @@ static void parallel_thread(int i915, unsigned int flags,
uint32_t target_bo_idx = 0;
uint32_t first_bb_idx = 1;
intel_ctx_cfg_t cfg;
+ uint32_t dmabuf_handle;
+ int dmabuf;
igt_assert(bb_per_execbuf < 32);
@@ -2924,11 +2928,20 @@ static void parallel_thread(int i915, unsigned int flags,
execbuf.buffers_ptr = to_user_pointer(obj);
execbuf.rsvd1 = ctx->id;
+ if (flags & PARALLEL_OUT_FENCE_DMABUF) {
+ dmabuf_handle = gem_create(i915, 4096);
+ dmabuf = prime_handle_to_fd(i915, dmabuf_handle);
+ }
+
for (n = 0; n < PARALLEL_BB_LOOP_COUNT; ++n) {
execbuf.flags &= ~0x3full;
gem_execbuf_wr(i915, &execbuf);
if (flags & PARALLEL_OUT_FENCE) {
+ if (flags & PARALLEL_OUT_FENCE_DMABUF)
+ dmabuf_import_sync_file(dmabuf, DMA_BUF_SYNC_WRITE,
+ execbuf.rsvd2 >> 32);
+
igt_assert_eq(sync_fence_wait(execbuf.rsvd2 >> 32,
1000), 0);
igt_assert_eq(sync_fence_status(execbuf.rsvd2 >> 32), 1);
@@ -2959,6 +2972,11 @@ static void parallel_thread(int i915, unsigned int flags,
if (fence)
close(fence);
+ if (flags & PARALLEL_OUT_FENCE_DMABUF) {
+ gem_close(i915, dmabuf_handle);
+ close(dmabuf);
+ }
+
check_bo(i915, obj[target_bo_idx].handle,
bb_per_execbuf * PARALLEL_BB_LOOP_COUNT, true);
@@ -3420,6 +3438,11 @@ igt_main
igt_subtest("parallel-out-fence")
parallel(i915, PARALLEL_OUT_FENCE);
+ igt_describe("Regression test to check that dmabuf imported sync file can handle fence array");
+ igt_subtest("parallel-dmabuf-import-out-fence")
+ parallel(i915, PARALLEL_OUT_FENCE |
+ PARALLEL_OUT_FENCE_DMABUF);
+
igt_subtest("parallel-keep-in-fence")
parallel(i915, PARALLEL_OUT_FENCE | PARALLEL_IN_FENCE);
--
2.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v2,1/2] lib/dmabuf_sync_file: move common stuff into lib
2022-12-02 12:02 ` [igt-dev] " Matthew Auld
(?)
(?)
@ 2022-12-02 13:27 ` Patchwork
2022-12-05 14:32 ` Kamil Konieczny
-1 siblings, 1 reply; 12+ messages in thread
From: Patchwork @ 2022-12-02 13:27 UTC (permalink / raw)
To: Matthew Auld; +Cc: igt-dev
[-- Attachment #1: Type: text/plain, Size: 5407 bytes --]
== Series Details ==
Series: series starting with [i-g-t,v2,1/2] lib/dmabuf_sync_file: move common stuff into lib
URL : https://patchwork.freedesktop.org/series/111582/
State : failure
== Summary ==
CI Bug Log - changes from IGT_7079 -> IGTPW_8187
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_8187 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_8187, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/index.html
Participating hosts (42 -> 40)
------------------------------
Additional (1): bat-atsm-1
Missing (3): fi-ilk-m540 fi-tgl-dsi bat-dg1-6
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_8187:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live@execlists:
- fi-apl-guc: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-apl-guc/igt@i915_selftest@live@execlists.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-apl-guc/igt@i915_selftest@live@execlists.html
Known issues
------------
Here are the changes found in IGTPW_8187 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- {bat-rpls-2}: [WARN][3] ([i915#7346]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/bat-rpls-2/igt@i915_pm_rpm@module-reload.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/bat-rpls-2/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-cfl-guc: [DMESG-FAIL][5] ([i915#5334]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
- fi-apl-guc: [DMESG-FAIL][7] ([i915#5334]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@migrate:
- {bat-dg2-11}: [DMESG-WARN][9] ([i915#7359]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/bat-dg2-11/igt@i915_selftest@live@migrate.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/bat-dg2-11/igt@i915_selftest@live@migrate.html
* igt@i915_selftest@live@requests:
- fi-kbl-soraka: [INCOMPLETE][11] ([i915#7640]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-kbl-soraka/igt@i915_selftest@live@requests.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-kbl-soraka/igt@i915_selftest@live@requests.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6077]: https://gitlab.freedesktop.org/drm/intel/issues/6077
[i915#6078]: https://gitlab.freedesktop.org/drm/intel/issues/6078
[i915#6093]: https://gitlab.freedesktop.org/drm/intel/issues/6093
[i915#6094]: https://gitlab.freedesktop.org/drm/intel/issues/6094
[i915#6166]: https://gitlab.freedesktop.org/drm/intel/issues/6166
[i915#6311]: https://gitlab.freedesktop.org/drm/intel/issues/6311
[i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
[i915#7346]: https://gitlab.freedesktop.org/drm/intel/issues/7346
[i915#7357]: https://gitlab.freedesktop.org/drm/intel/issues/7357
[i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359
[i915#7640]: https://gitlab.freedesktop.org/drm/intel/issues/7640
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_7079 -> IGTPW_8187
CI-20190529: 20190529
CI_DRM_12462: 00b10bdfd8b9edc9b2c681d806fbb6ae2e5f31a3 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_8187: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/index.html
IGT_7079: 272354f8e1bbeb56df1663c42a9958f2ff9b8f54 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Testlist changes
----------------
+igt@gem_exec_balancer@parallel-dmabuf-import-out-fence
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/index.html
[-- Attachment #2: Type: text/html, Size: 4935 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v2,1/2] lib/dmabuf_sync_file: move common stuff into lib
2022-12-02 13:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v2,1/2] lib/dmabuf_sync_file: move common stuff into lib Patchwork
@ 2022-12-05 14:32 ` Kamil Konieczny
0 siblings, 0 replies; 12+ messages in thread
From: Kamil Konieczny @ 2022-12-05 14:32 UTC (permalink / raw)
To: igt-dev; +Cc: Lakshminarayana Vudum
Hi Lakshmi,
these are unrelated to the change.
--
Kamil
On 2022-12-02 at 13:27:47 -0000, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [i-g-t,v2,1/2] lib/dmabuf_sync_file: move common stuff into lib
> URL : https://patchwork.freedesktop.org/series/111582/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from IGT_7079 -> IGTPW_8187
> ====================================================
>
> Summary
> -------
>
> **FAILURE**
>
> Serious unknown changes coming with IGTPW_8187 absolutely need to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in IGTPW_8187, please notify your bug team to allow them
> to document this new failure mode, which will reduce false positives in CI.
>
> External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/index.html
>
> Participating hosts (42 -> 40)
> ------------------------------
>
> Additional (1): bat-atsm-1
> Missing (3): fi-ilk-m540 fi-tgl-dsi bat-dg1-6
>
> Possible new issues
> -------------------
>
> Here are the unknown changes that may have been introduced in IGTPW_8187:
>
> ### IGT changes ###
>
> #### Possible regressions ####
>
> * igt@i915_selftest@live@execlists:
> - fi-apl-guc: [PASS][1] -> [INCOMPLETE][2]
> [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-apl-guc/igt@i915_selftest@live@execlists.html
> [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-apl-guc/igt@i915_selftest@live@execlists.html
>
>
> Known issues
> ------------
>
> Here are the changes found in IGTPW_8187 that come from known issues:
>
> ### IGT changes ###
>
> #### Possible fixes ####
>
> * igt@i915_pm_rpm@module-reload:
> - {bat-rpls-2}: [WARN][3] ([i915#7346]) -> [PASS][4]
> [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/bat-rpls-2/igt@i915_pm_rpm@module-reload.html
> [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/bat-rpls-2/igt@i915_pm_rpm@module-reload.html
>
> * igt@i915_selftest@live@gt_heartbeat:
> - fi-cfl-guc: [DMESG-FAIL][5] ([i915#5334]) -> [PASS][6]
> [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
> [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
> - fi-apl-guc: [DMESG-FAIL][7] ([i915#5334]) -> [PASS][8]
> [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
> [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
>
> * igt@i915_selftest@live@migrate:
> - {bat-dg2-11}: [DMESG-WARN][9] ([i915#7359]) -> [PASS][10]
> [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/bat-dg2-11/igt@i915_selftest@live@migrate.html
> [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/bat-dg2-11/igt@i915_selftest@live@migrate.html
>
> * igt@i915_selftest@live@requests:
> - fi-kbl-soraka: [INCOMPLETE][11] ([i915#7640]) -> [PASS][12]
> [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7079/fi-kbl-soraka/igt@i915_selftest@live@requests.html
> [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/fi-kbl-soraka/igt@i915_selftest@live@requests.html
>
>
> {name}: This element is suppressed. This means it is ignored when computing
> the status of the difference (SUCCESS, WARNING, or FAILURE).
>
> [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
> [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
> [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
> [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
> [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
> [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
> [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
> [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
> [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
> [i915#6077]: https://gitlab.freedesktop.org/drm/intel/issues/6077
> [i915#6078]: https://gitlab.freedesktop.org/drm/intel/issues/6078
> [i915#6093]: https://gitlab.freedesktop.org/drm/intel/issues/6093
> [i915#6094]: https://gitlab.freedesktop.org/drm/intel/issues/6094
> [i915#6166]: https://gitlab.freedesktop.org/drm/intel/issues/6166
> [i915#6311]: https://gitlab.freedesktop.org/drm/intel/issues/6311
> [i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
> [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
> [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
> [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
> [i915#7346]: https://gitlab.freedesktop.org/drm/intel/issues/7346
> [i915#7357]: https://gitlab.freedesktop.org/drm/intel/issues/7357
> [i915#7359]: https://gitlab.freedesktop.org/drm/intel/issues/7359
> [i915#7640]: https://gitlab.freedesktop.org/drm/intel/issues/7640
>
>
> Build changes
> -------------
>
> * CI: CI-20190529 -> None
> * IGT: IGT_7079 -> IGTPW_8187
>
> CI-20190529: 20190529
> CI_DRM_12462: 00b10bdfd8b9edc9b2c681d806fbb6ae2e5f31a3 @ git://anongit.freedesktop.org/gfx-ci/linux
> IGTPW_8187: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/index.html
> IGT_7079: 272354f8e1bbeb56df1663c42a9958f2ff9b8f54 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
>
>
> Testlist changes
> ----------------
>
> +igt@gem_exec_balancer@parallel-dmabuf-import-out-fence
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8187/index.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
2022-12-02 12:02 ` [igt-dev] " Matthew Auld
@ 2022-12-05 16:31 ` Kamil Konieczny
-1 siblings, 0 replies; 12+ messages in thread
From: Kamil Konieczny @ 2022-12-05 16:31 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Matthew Auld, Andrzej Hajda, Nirmoy Das
Hi Matt,
after re-reading I have few more nits.
On 2022-12-02 at 12:02:41 +0000, Matthew Auld wrote:
> So we can use this across different tests.
>
> v2
> - Add docs for everything (Petri)
> - Add missing copyright and fix headers slightly (Kamil)
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Nirmoy Das <nirmoy.das@intel.com>
> ---
> .../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
> lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
> lib/dmabuf_sync_file.h | 26 +++
> lib/meson.build | 1 +
> tests/dmabuf_sync_file.c | 133 +----------
> 5 files changed, 243 insertions(+), 129 deletions(-)
> create mode 100644 lib/dmabuf_sync_file.c
> create mode 100644 lib/dmabuf_sync_file.h
>
> diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> index 1ce842f4..102c8a89 100644
> --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> @@ -15,6 +15,7 @@
>
> <chapter>
> <title>API Reference</title>
> + <xi:include href="xml/dmabuf_sync_file.xml"/>
> <xi:include href="xml/drmtest.xml"/>
> <xi:include href="xml/igt_alsa.xml"/>
> <xi:include href="xml/igt_audio.xml"/>
> diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
> new file mode 100644
> index 00000000..bcce2486
> --- /dev/null
> +++ b/lib/dmabuf_sync_file.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "igt.h"
> +#include "igt_vgem.h"
> +#include "sw_sync.h"
> +
> +#include "dmabuf_sync_file.h"
> +
> +/**
> + * SECTION: dmabuf_sync_file
> + * @short_description: DMABUF importing/exporting fencing support library
> + * @title: DMABUF Sync File
> + * @include: dmabuf_sync_file.h
> + */
> +
> +struct igt_dma_buf_sync_file {
> + __u32 flags;
> + __s32 fd;
> +};
> +
> +#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> +#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> +
> +/**
> + * has_dmabuf_export_sync_file:
> + * @fd: The open drm fd
> + *
> + * Check if the kernel supports exporting a sync file from dmabuf.
> + */
> +bool has_dmabuf_export_sync_file(int fd)
> +{
> + struct vgem_bo bo;
> + int dmabuf, ret;
> + struct igt_dma_buf_sync_file arg;
> +
> + bo.width = 1;
> + bo.height = 1;
> + bo.bpp = 32;
> + vgem_create(fd, &bo);
> +
> + dmabuf = prime_handle_to_fd(fd, bo.handle);
> + gem_close(fd, bo.handle);
> +
> + arg.flags = DMA_BUF_SYNC_WRITE;
> + arg.fd = -1;
> +
> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> + close(dmabuf);
> + igt_assert(ret == 0 || errno == ENOTTY);
imho we should not explode here, it was ok in test but in lib
we should just return false in case of unexpected error, you can
add igt_debug if you think it will help.
This may lead to change in place where you use it, like
igt_require(has_dmabuf_export_sync_file(fd));
> +
> + return ret == 0;
> +}
> +
> +/**
> + * dmabuf_export_sync_file:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + *
> + * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
> + * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
> + * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
> + */
> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
As you do not check for errors so this should be
int __dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> +{
> + struct igt_dma_buf_sync_file arg;
> +
> + arg.flags = flags;
> + arg.fd = -1;
> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> +
> + return arg.fd;
> +}
> +
> +/**
> + * has_dmabuf_import_sync_file:
> + * @fd: The open drm fd
> + *
> + * Check if the kernel supports importing a sync file into a dmabuf.
> + */
> +bool has_dmabuf_import_sync_file(int fd)
> +{
> + struct vgem_bo bo;
> + int dmabuf, timeline, fence, ret;
> + struct igt_dma_buf_sync_file arg;
> +
> + bo.width = 1;
> + bo.height = 1;
> + bo.bpp = 32;
> + vgem_create(fd, &bo);
> +
> + dmabuf = prime_handle_to_fd(fd, bo.handle);
> + gem_close(fd, bo.handle);
> +
> + timeline = sw_sync_timeline_create();
> + fence = sw_sync_timeline_create_fence(timeline, 1);
> + sw_sync_timeline_inc(timeline, 1);
> +
> + arg.flags = DMA_BUF_SYNC_RW;
> + arg.fd = fence;
> +
> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> + close(dmabuf);
> + close(fence);
> + igt_assert(ret == 0 || errno == ENOTTY);
Same here, return false instead of assert.
> +
> + return ret == 0;
> +}
> +
> +/**
> + * dmabuf_import_sync_file:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + * @sync_fd: The sync file (i.e our fence) to import
> + *
> + * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> + * importing the @sync_fd as a read or write fence.
> + */
> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
Same here, no error checks so use __ like:
void __dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> +{
> + struct igt_dma_buf_sync_file arg;
> +
> + arg.flags = flags;
> + arg.fd = sync_fd;
> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> +}
> +
> +/**
> + * dmabuf_import_timeline_fence:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + * @timeline: The sync file timeline where the new fence is created
> + * @seqno: The sequence number to use for the fence
> + *
> + * Create a new fence as part of @timeline, and import as a sync file into the
> + * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
> + * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
> + * or write fence.
> + */
> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
Same here, no error checks so use __ before function name.
> + int timeline, uint32_t seqno)
> +{
> + int fence;
> +
> + fence = sw_sync_timeline_create_fence(timeline, seqno);
> + dmabuf_import_sync_file(dmabuf, flags, fence);
> + close(fence);
> +}
> +
> +/**
> + * dmabuf_busy:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + *
> + * Check if the fences in the dmabuf are still busy. The @flags should at least
> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> + * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
> + * we care about both.
> + */
> +bool dmabuf_busy(int dmabuf, uint32_t flags)
> +{
> + struct pollfd pfd = { .fd = dmabuf };
> +
> + /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> + * else poll() may return a non-zero value if there are only read
> + * fences because POLLIN is ready even if POLLOUT isn't.
> + */
> + if (flags & DMA_BUF_SYNC_WRITE)
> + pfd.events |= POLLOUT;
> + else if (flags & DMA_BUF_SYNC_READ)
> + pfd.events |= POLLIN;
> +
> + return poll(&pfd, 1, 0) == 0;
> +}
> +
> +/**
> + * sync_file_busy:
> + * @sync_file: The sync file to check
> + *
> + * Check if the @sync_file is still busy or not.
> + */
> +bool sync_file_busy(int sync_file)
> +{
> + struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> + return poll(&pfd, 1, 0) == 0;
> +}
> +
> +/**
> + * dmabuf_sync_file_busy:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + *
> + * Export the current fences in @dmabuf as a sync file and check if still busy.
> + * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
> + * to specify which fences are to be exported from the @dmabuf and checked if
> + * busy. Or DMA_BUF_SYNC_RW if we care about both.
> + */
> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> +{
> + int sync_file;
> + bool busy;
> +
> + sync_file = dmabuf_export_sync_file(dmabuf, flags);
Check for error here.
Regards,
Kamil
> + busy = sync_file_busy(sync_file);
> + close(sync_file);
> +
> + return busy;
> +}
> diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
> new file mode 100644
> index 00000000..d642ff30
> --- /dev/null
> +++ b/lib/dmabuf_sync_file.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef DMABUF_SYNC_FILE_H
> +#define DMABUF_SYNC_FILE_H
> +
> +#ifdef __linux__
> +#include <linux/dma-buf.h>
> +#endif
> +#include <sys/poll.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +bool has_dmabuf_export_sync_file(int fd);
> +bool has_dmabuf_import_sync_file(int fd);
> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> + int timeline, uint32_t seqno);
> +bool dmabuf_busy(int dmabuf, uint32_t flags);
> +bool sync_file_busy(int sync_file);
> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
> +
> +#endif
> diff --git a/lib/meson.build b/lib/meson.build
> index cef2d0ff..896d5733 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -1,5 +1,6 @@
> lib_sources = [
> 'drmtest.c',
> + 'dmabuf_sync_file.c',
> 'huc_copy.c',
> 'i915/gem.c',
> 'i915/gem_context.c',
> diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
> index 2179a76d..25bb6ad7 100644
> --- a/tests/dmabuf_sync_file.c
> +++ b/tests/dmabuf_sync_file.c
> @@ -1,140 +1,15 @@
> // SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
>
> #include "igt.h"
> #include "igt_vgem.h"
> #include "sw_sync.h"
> -
> -#include <linux/dma-buf.h>
> -#include <sys/poll.h>
> +#include "dmabuf_sync_file.h"
>
> IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
>
> -struct igt_dma_buf_sync_file {
> - __u32 flags;
> - __s32 fd;
> -};
> -
> -#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> -#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> -
> -static bool has_dmabuf_export_sync_file(int fd)
> -{
> - struct vgem_bo bo;
> - int dmabuf, ret;
> - struct igt_dma_buf_sync_file arg;
> -
> - bo.width = 1;
> - bo.height = 1;
> - bo.bpp = 32;
> - vgem_create(fd, &bo);
> -
> - dmabuf = prime_handle_to_fd(fd, bo.handle);
> - gem_close(fd, bo.handle);
> -
> - arg.flags = DMA_BUF_SYNC_WRITE;
> - arg.fd = -1;
> -
> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> - close(dmabuf);
> - igt_assert(ret == 0 || errno == ENOTTY);
> -
> - return ret == 0;
> -}
> -
> -static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> -{
> - struct igt_dma_buf_sync_file arg;
> -
> - arg.flags = flags;
> - arg.fd = -1;
> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> -
> - return arg.fd;
> -}
> -
> -static bool has_dmabuf_import_sync_file(int fd)
> -{
> - struct vgem_bo bo;
> - int dmabuf, timeline, fence, ret;
> - struct igt_dma_buf_sync_file arg;
> -
> - bo.width = 1;
> - bo.height = 1;
> - bo.bpp = 32;
> - vgem_create(fd, &bo);
> -
> - dmabuf = prime_handle_to_fd(fd, bo.handle);
> - gem_close(fd, bo.handle);
> -
> - timeline = sw_sync_timeline_create();
> - fence = sw_sync_timeline_create_fence(timeline, 1);
> - sw_sync_timeline_inc(timeline, 1);
> -
> - arg.flags = DMA_BUF_SYNC_RW;
> - arg.fd = fence;
> -
> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> - close(dmabuf);
> - close(fence);
> - igt_assert(ret == 0 || errno == ENOTTY);
> -
> - return ret == 0;
> -}
> -
> -static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> -{
> - struct igt_dma_buf_sync_file arg;
> -
> - arg.flags = flags;
> - arg.fd = sync_fd;
> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> -}
> -
> -static void
> -dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> - int timeline, uint32_t seqno)
> -{
> - int fence;
> -
> - fence = sw_sync_timeline_create_fence(timeline, seqno);
> - dmabuf_import_sync_file(dmabuf, flags, fence);
> - close(fence);
> -}
> -
> -static bool dmabuf_busy(int dmabuf, uint32_t flags)
> -{
> - struct pollfd pfd = { .fd = dmabuf };
> -
> - /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> - * else poll() may return a non-zero value if there are only read
> - * fences because POLLIN is ready even if POLLOUT isn't.
> - */
> - if (flags & DMA_BUF_SYNC_WRITE)
> - pfd.events |= POLLOUT;
> - else if (flags & DMA_BUF_SYNC_READ)
> - pfd.events |= POLLIN;
> -
> - return poll(&pfd, 1, 0) == 0;
> -}
> -
> -static bool sync_file_busy(int sync_file)
> -{
> - struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> - return poll(&pfd, 1, 0) == 0;
> -}
> -
> -static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> -{
> - int sync_file;
> - bool busy;
> -
> - sync_file = dmabuf_export_sync_file(dmabuf, flags);
> - busy = sync_file_busy(sync_file);
> - close(sync_file);
> -
> - return busy;
> -}
> -
> static void test_export_basic(int fd)
> {
> struct vgem_bo bo;
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
@ 2022-12-05 16:31 ` Kamil Konieczny
0 siblings, 0 replies; 12+ messages in thread
From: Kamil Konieczny @ 2022-12-05 16:31 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Matthew Auld, Petri Latvala, Nirmoy Das
Hi Matt,
after re-reading I have few more nits.
On 2022-12-02 at 12:02:41 +0000, Matthew Auld wrote:
> So we can use this across different tests.
>
> v2
> - Add docs for everything (Petri)
> - Add missing copyright and fix headers slightly (Kamil)
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Nirmoy Das <nirmoy.das@intel.com>
> ---
> .../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
> lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
> lib/dmabuf_sync_file.h | 26 +++
> lib/meson.build | 1 +
> tests/dmabuf_sync_file.c | 133 +----------
> 5 files changed, 243 insertions(+), 129 deletions(-)
> create mode 100644 lib/dmabuf_sync_file.c
> create mode 100644 lib/dmabuf_sync_file.h
>
> diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> index 1ce842f4..102c8a89 100644
> --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> @@ -15,6 +15,7 @@
>
> <chapter>
> <title>API Reference</title>
> + <xi:include href="xml/dmabuf_sync_file.xml"/>
> <xi:include href="xml/drmtest.xml"/>
> <xi:include href="xml/igt_alsa.xml"/>
> <xi:include href="xml/igt_audio.xml"/>
> diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
> new file mode 100644
> index 00000000..bcce2486
> --- /dev/null
> +++ b/lib/dmabuf_sync_file.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "igt.h"
> +#include "igt_vgem.h"
> +#include "sw_sync.h"
> +
> +#include "dmabuf_sync_file.h"
> +
> +/**
> + * SECTION: dmabuf_sync_file
> + * @short_description: DMABUF importing/exporting fencing support library
> + * @title: DMABUF Sync File
> + * @include: dmabuf_sync_file.h
> + */
> +
> +struct igt_dma_buf_sync_file {
> + __u32 flags;
> + __s32 fd;
> +};
> +
> +#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> +#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> +
> +/**
> + * has_dmabuf_export_sync_file:
> + * @fd: The open drm fd
> + *
> + * Check if the kernel supports exporting a sync file from dmabuf.
> + */
> +bool has_dmabuf_export_sync_file(int fd)
> +{
> + struct vgem_bo bo;
> + int dmabuf, ret;
> + struct igt_dma_buf_sync_file arg;
> +
> + bo.width = 1;
> + bo.height = 1;
> + bo.bpp = 32;
> + vgem_create(fd, &bo);
> +
> + dmabuf = prime_handle_to_fd(fd, bo.handle);
> + gem_close(fd, bo.handle);
> +
> + arg.flags = DMA_BUF_SYNC_WRITE;
> + arg.fd = -1;
> +
> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> + close(dmabuf);
> + igt_assert(ret == 0 || errno == ENOTTY);
imho we should not explode here, it was ok in test but in lib
we should just return false in case of unexpected error, you can
add igt_debug if you think it will help.
This may lead to change in place where you use it, like
igt_require(has_dmabuf_export_sync_file(fd));
> +
> + return ret == 0;
> +}
> +
> +/**
> + * dmabuf_export_sync_file:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + *
> + * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
> + * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
> + * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
> + */
> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
As you do not check for errors so this should be
int __dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> +{
> + struct igt_dma_buf_sync_file arg;
> +
> + arg.flags = flags;
> + arg.fd = -1;
> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> +
> + return arg.fd;
> +}
> +
> +/**
> + * has_dmabuf_import_sync_file:
> + * @fd: The open drm fd
> + *
> + * Check if the kernel supports importing a sync file into a dmabuf.
> + */
> +bool has_dmabuf_import_sync_file(int fd)
> +{
> + struct vgem_bo bo;
> + int dmabuf, timeline, fence, ret;
> + struct igt_dma_buf_sync_file arg;
> +
> + bo.width = 1;
> + bo.height = 1;
> + bo.bpp = 32;
> + vgem_create(fd, &bo);
> +
> + dmabuf = prime_handle_to_fd(fd, bo.handle);
> + gem_close(fd, bo.handle);
> +
> + timeline = sw_sync_timeline_create();
> + fence = sw_sync_timeline_create_fence(timeline, 1);
> + sw_sync_timeline_inc(timeline, 1);
> +
> + arg.flags = DMA_BUF_SYNC_RW;
> + arg.fd = fence;
> +
> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> + close(dmabuf);
> + close(fence);
> + igt_assert(ret == 0 || errno == ENOTTY);
Same here, return false instead of assert.
> +
> + return ret == 0;
> +}
> +
> +/**
> + * dmabuf_import_sync_file:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + * @sync_fd: The sync file (i.e our fence) to import
> + *
> + * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> + * importing the @sync_fd as a read or write fence.
> + */
> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
Same here, no error checks so use __ like:
void __dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> +{
> + struct igt_dma_buf_sync_file arg;
> +
> + arg.flags = flags;
> + arg.fd = sync_fd;
> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> +}
> +
> +/**
> + * dmabuf_import_timeline_fence:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + * @timeline: The sync file timeline where the new fence is created
> + * @seqno: The sequence number to use for the fence
> + *
> + * Create a new fence as part of @timeline, and import as a sync file into the
> + * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
> + * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
> + * or write fence.
> + */
> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
Same here, no error checks so use __ before function name.
> + int timeline, uint32_t seqno)
> +{
> + int fence;
> +
> + fence = sw_sync_timeline_create_fence(timeline, seqno);
> + dmabuf_import_sync_file(dmabuf, flags, fence);
> + close(fence);
> +}
> +
> +/**
> + * dmabuf_busy:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + *
> + * Check if the fences in the dmabuf are still busy. The @flags should at least
> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> + * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
> + * we care about both.
> + */
> +bool dmabuf_busy(int dmabuf, uint32_t flags)
> +{
> + struct pollfd pfd = { .fd = dmabuf };
> +
> + /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> + * else poll() may return a non-zero value if there are only read
> + * fences because POLLIN is ready even if POLLOUT isn't.
> + */
> + if (flags & DMA_BUF_SYNC_WRITE)
> + pfd.events |= POLLOUT;
> + else if (flags & DMA_BUF_SYNC_READ)
> + pfd.events |= POLLIN;
> +
> + return poll(&pfd, 1, 0) == 0;
> +}
> +
> +/**
> + * sync_file_busy:
> + * @sync_file: The sync file to check
> + *
> + * Check if the @sync_file is still busy or not.
> + */
> +bool sync_file_busy(int sync_file)
> +{
> + struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> + return poll(&pfd, 1, 0) == 0;
> +}
> +
> +/**
> + * dmabuf_sync_file_busy:
> + * @dmabuf: The dmabuf fd
> + * @flags: The flags to control the behaviour
> + *
> + * Export the current fences in @dmabuf as a sync file and check if still busy.
> + * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
> + * to specify which fences are to be exported from the @dmabuf and checked if
> + * busy. Or DMA_BUF_SYNC_RW if we care about both.
> + */
> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> +{
> + int sync_file;
> + bool busy;
> +
> + sync_file = dmabuf_export_sync_file(dmabuf, flags);
Check for error here.
Regards,
Kamil
> + busy = sync_file_busy(sync_file);
> + close(sync_file);
> +
> + return busy;
> +}
> diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
> new file mode 100644
> index 00000000..d642ff30
> --- /dev/null
> +++ b/lib/dmabuf_sync_file.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef DMABUF_SYNC_FILE_H
> +#define DMABUF_SYNC_FILE_H
> +
> +#ifdef __linux__
> +#include <linux/dma-buf.h>
> +#endif
> +#include <sys/poll.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +bool has_dmabuf_export_sync_file(int fd);
> +bool has_dmabuf_import_sync_file(int fd);
> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> + int timeline, uint32_t seqno);
> +bool dmabuf_busy(int dmabuf, uint32_t flags);
> +bool sync_file_busy(int sync_file);
> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
> +
> +#endif
> diff --git a/lib/meson.build b/lib/meson.build
> index cef2d0ff..896d5733 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -1,5 +1,6 @@
> lib_sources = [
> 'drmtest.c',
> + 'dmabuf_sync_file.c',
> 'huc_copy.c',
> 'i915/gem.c',
> 'i915/gem_context.c',
> diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
> index 2179a76d..25bb6ad7 100644
> --- a/tests/dmabuf_sync_file.c
> +++ b/tests/dmabuf_sync_file.c
> @@ -1,140 +1,15 @@
> // SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
>
> #include "igt.h"
> #include "igt_vgem.h"
> #include "sw_sync.h"
> -
> -#include <linux/dma-buf.h>
> -#include <sys/poll.h>
> +#include "dmabuf_sync_file.h"
>
> IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
>
> -struct igt_dma_buf_sync_file {
> - __u32 flags;
> - __s32 fd;
> -};
> -
> -#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> -#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> -
> -static bool has_dmabuf_export_sync_file(int fd)
> -{
> - struct vgem_bo bo;
> - int dmabuf, ret;
> - struct igt_dma_buf_sync_file arg;
> -
> - bo.width = 1;
> - bo.height = 1;
> - bo.bpp = 32;
> - vgem_create(fd, &bo);
> -
> - dmabuf = prime_handle_to_fd(fd, bo.handle);
> - gem_close(fd, bo.handle);
> -
> - arg.flags = DMA_BUF_SYNC_WRITE;
> - arg.fd = -1;
> -
> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> - close(dmabuf);
> - igt_assert(ret == 0 || errno == ENOTTY);
> -
> - return ret == 0;
> -}
> -
> -static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> -{
> - struct igt_dma_buf_sync_file arg;
> -
> - arg.flags = flags;
> - arg.fd = -1;
> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> -
> - return arg.fd;
> -}
> -
> -static bool has_dmabuf_import_sync_file(int fd)
> -{
> - struct vgem_bo bo;
> - int dmabuf, timeline, fence, ret;
> - struct igt_dma_buf_sync_file arg;
> -
> - bo.width = 1;
> - bo.height = 1;
> - bo.bpp = 32;
> - vgem_create(fd, &bo);
> -
> - dmabuf = prime_handle_to_fd(fd, bo.handle);
> - gem_close(fd, bo.handle);
> -
> - timeline = sw_sync_timeline_create();
> - fence = sw_sync_timeline_create_fence(timeline, 1);
> - sw_sync_timeline_inc(timeline, 1);
> -
> - arg.flags = DMA_BUF_SYNC_RW;
> - arg.fd = fence;
> -
> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> - close(dmabuf);
> - close(fence);
> - igt_assert(ret == 0 || errno == ENOTTY);
> -
> - return ret == 0;
> -}
> -
> -static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> -{
> - struct igt_dma_buf_sync_file arg;
> -
> - arg.flags = flags;
> - arg.fd = sync_fd;
> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> -}
> -
> -static void
> -dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> - int timeline, uint32_t seqno)
> -{
> - int fence;
> -
> - fence = sw_sync_timeline_create_fence(timeline, seqno);
> - dmabuf_import_sync_file(dmabuf, flags, fence);
> - close(fence);
> -}
> -
> -static bool dmabuf_busy(int dmabuf, uint32_t flags)
> -{
> - struct pollfd pfd = { .fd = dmabuf };
> -
> - /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> - * else poll() may return a non-zero value if there are only read
> - * fences because POLLIN is ready even if POLLOUT isn't.
> - */
> - if (flags & DMA_BUF_SYNC_WRITE)
> - pfd.events |= POLLOUT;
> - else if (flags & DMA_BUF_SYNC_READ)
> - pfd.events |= POLLIN;
> -
> - return poll(&pfd, 1, 0) == 0;
> -}
> -
> -static bool sync_file_busy(int sync_file)
> -{
> - struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> - return poll(&pfd, 1, 0) == 0;
> -}
> -
> -static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> -{
> - int sync_file;
> - bool busy;
> -
> - sync_file = dmabuf_export_sync_file(dmabuf, flags);
> - busy = sync_file_busy(sync_file);
> - close(sync_file);
> -
> - return busy;
> -}
> -
> static void test_export_basic(int fd)
> {
> struct vgem_bo bo;
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
2022-12-05 16:31 ` [igt-dev] " Kamil Konieczny
@ 2022-12-05 17:11 ` Matthew Auld
-1 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2022-12-05 17:11 UTC (permalink / raw)
To: Kamil Konieczny, igt-dev, intel-gfx, Petri Latvala,
Andrzej Hajda, Nirmoy Das
On 05/12/2022 16:31, Kamil Konieczny wrote:
> Hi Matt,
>
> after re-reading I have few more nits.
>
> On 2022-12-02 at 12:02:41 +0000, Matthew Auld wrote:
>> So we can use this across different tests.
>>
>> v2
>> - Add docs for everything (Petri)
>> - Add missing copyright and fix headers slightly (Kamil)
>>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>> Cc: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>> .../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
>> lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
>> lib/dmabuf_sync_file.h | 26 +++
>> lib/meson.build | 1 +
>> tests/dmabuf_sync_file.c | 133 +----------
>> 5 files changed, 243 insertions(+), 129 deletions(-)
>> create mode 100644 lib/dmabuf_sync_file.c
>> create mode 100644 lib/dmabuf_sync_file.h
>>
>> diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>> index 1ce842f4..102c8a89 100644
>> --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>> +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>> @@ -15,6 +15,7 @@
>>
>> <chapter>
>> <title>API Reference</title>
>> + <xi:include href="xml/dmabuf_sync_file.xml"/>
>> <xi:include href="xml/drmtest.xml"/>
>> <xi:include href="xml/igt_alsa.xml"/>
>> <xi:include href="xml/igt_audio.xml"/>
>> diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
>> new file mode 100644
>> index 00000000..bcce2486
>> --- /dev/null
>> +++ b/lib/dmabuf_sync_file.c
>> @@ -0,0 +1,211 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#include "igt.h"
>> +#include "igt_vgem.h"
>> +#include "sw_sync.h"
>> +
>> +#include "dmabuf_sync_file.h"
>> +
>> +/**
>> + * SECTION: dmabuf_sync_file
>> + * @short_description: DMABUF importing/exporting fencing support library
>> + * @title: DMABUF Sync File
>> + * @include: dmabuf_sync_file.h
>> + */
>> +
>> +struct igt_dma_buf_sync_file {
>> + __u32 flags;
>> + __s32 fd;
>> +};
>> +
>> +#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
>> +#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
>> +
>> +/**
>> + * has_dmabuf_export_sync_file:
>> + * @fd: The open drm fd
>> + *
>> + * Check if the kernel supports exporting a sync file from dmabuf.
>> + */
>> +bool has_dmabuf_export_sync_file(int fd)
>> +{
>> + struct vgem_bo bo;
>> + int dmabuf, ret;
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + bo.width = 1;
>> + bo.height = 1;
>> + bo.bpp = 32;
>> + vgem_create(fd, &bo);
>> +
>> + dmabuf = prime_handle_to_fd(fd, bo.handle);
>> + gem_close(fd, bo.handle);
>> +
>> + arg.flags = DMA_BUF_SYNC_WRITE;
>> + arg.fd = -1;
>> +
>> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> + close(dmabuf);
>> + igt_assert(ret == 0 || errno == ENOTTY);
>
> imho we should not explode here, it was ok in test but in lib
> we should just return false in case of unexpected error, you can
> add igt_debug if you think it will help.
> This may lead to change in place where you use it, like
> igt_require(has_dmabuf_export_sync_file(fd));
Yup, makes sense. Will fix.
>
>> +
>> + return ret == 0;
>> +}
>> +
>> +/**
>> + * dmabuf_export_sync_file:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + *
>> + * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
>> + * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
>> + * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
>> + */
>> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
>
> As you do not check for errors so this should be
> int __dmabuf_export_sync_file(int dmabuf, uint32_t flags)
do_ioctl() is doing an igt_assert_eq(ioctl(...), 0). Or what do you mean
by not checking for errors?
>
>> +{
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + arg.flags = flags;
>> + arg.fd = -1;
>> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> +
>> + return arg.fd;
>> +}
>> +
>> +/**
>> + * has_dmabuf_import_sync_file:
>> + * @fd: The open drm fd
>> + *
>> + * Check if the kernel supports importing a sync file into a dmabuf.
>> + */
>> +bool has_dmabuf_import_sync_file(int fd)
>> +{
>> + struct vgem_bo bo;
>> + int dmabuf, timeline, fence, ret;
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + bo.width = 1;
>> + bo.height = 1;
>> + bo.bpp = 32;
>> + vgem_create(fd, &bo);
>> +
>> + dmabuf = prime_handle_to_fd(fd, bo.handle);
>> + gem_close(fd, bo.handle);
>> +
>> + timeline = sw_sync_timeline_create();
>> + fence = sw_sync_timeline_create_fence(timeline, 1);
>> + sw_sync_timeline_inc(timeline, 1);
>> +
>> + arg.flags = DMA_BUF_SYNC_RW;
>> + arg.fd = fence;
>> +
>> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> + close(dmabuf);
>> + close(fence);
>> + igt_assert(ret == 0 || errno == ENOTTY);
>
> Same here, return false instead of assert.
>
>> +
>> + return ret == 0;
>> +}
>> +
>> +/**
>> + * dmabuf_import_sync_file:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + * @sync_fd: The sync file (i.e our fence) to import
>> + *
>> + * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
>> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
>> + * importing the @sync_fd as a read or write fence.
>> + */
>> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
>
> Same here, no error checks so use __ like:
> void __dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
>
>> +{
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + arg.flags = flags;
>> + arg.fd = sync_fd;
>> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> +}
>> +
>> +/**
>> + * dmabuf_import_timeline_fence:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + * @timeline: The sync file timeline where the new fence is created
>> + * @seqno: The sequence number to use for the fence
>> + *
>> + * Create a new fence as part of @timeline, and import as a sync file into the
>> + * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
>> + * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
>> + * or write fence.
>> + */
>> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
>
> Same here, no error checks so use __ before function name.
>
>> + int timeline, uint32_t seqno)
>> +{
>> + int fence;
>> +
>> + fence = sw_sync_timeline_create_fence(timeline, seqno);
>> + dmabuf_import_sync_file(dmabuf, flags, fence);
>> + close(fence);
>> +}
>> +
>> +/**
>> + * dmabuf_busy:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + *
>> + * Check if the fences in the dmabuf are still busy. The @flags should at least
>> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
>> + * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
>> + * we care about both.
>> + */
>> +bool dmabuf_busy(int dmabuf, uint32_t flags)
>> +{
>> + struct pollfd pfd = { .fd = dmabuf };
>> +
>> + /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
>> + * else poll() may return a non-zero value if there are only read
>> + * fences because POLLIN is ready even if POLLOUT isn't.
>> + */
>> + if (flags & DMA_BUF_SYNC_WRITE)
>> + pfd.events |= POLLOUT;
>> + else if (flags & DMA_BUF_SYNC_READ)
>> + pfd.events |= POLLIN;
>> +
>> + return poll(&pfd, 1, 0) == 0;
>> +}
>> +
>> +/**
>> + * sync_file_busy:
>> + * @sync_file: The sync file to check
>> + *
>> + * Check if the @sync_file is still busy or not.
>> + */
>> +bool sync_file_busy(int sync_file)
>> +{
>> + struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
>> + return poll(&pfd, 1, 0) == 0;
>> +}
>> +
>> +/**
>> + * dmabuf_sync_file_busy:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + *
>> + * Export the current fences in @dmabuf as a sync file and check if still busy.
>> + * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
>> + * to specify which fences are to be exported from the @dmabuf and checked if
>> + * busy. Or DMA_BUF_SYNC_RW if we care about both.
>> + */
>> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
>> +{
>> + int sync_file;
>> + bool busy;
>> +
>> + sync_file = dmabuf_export_sync_file(dmabuf, flags);
>
> Check for error here.
>
> Regards,
> Kamil
>
>> + busy = sync_file_busy(sync_file);
>> + close(sync_file);
>> +
>> + return busy;
>> +}
>> diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
>> new file mode 100644
>> index 00000000..d642ff30
>> --- /dev/null
>> +++ b/lib/dmabuf_sync_file.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#ifndef DMABUF_SYNC_FILE_H
>> +#define DMABUF_SYNC_FILE_H
>> +
>> +#ifdef __linux__
>> +#include <linux/dma-buf.h>
>> +#endif
>> +#include <sys/poll.h>
>> +#include <stdbool.h>
>> +#include <stdint.h>
>> +
>> +bool has_dmabuf_export_sync_file(int fd);
>> +bool has_dmabuf_import_sync_file(int fd);
>> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
>> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
>> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
>> + int timeline, uint32_t seqno);
>> +bool dmabuf_busy(int dmabuf, uint32_t flags);
>> +bool sync_file_busy(int sync_file);
>> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
>> +
>> +#endif
>> diff --git a/lib/meson.build b/lib/meson.build
>> index cef2d0ff..896d5733 100644
>> --- a/lib/meson.build
>> +++ b/lib/meson.build
>> @@ -1,5 +1,6 @@
>> lib_sources = [
>> 'drmtest.c',
>> + 'dmabuf_sync_file.c',
>> 'huc_copy.c',
>> 'i915/gem.c',
>> 'i915/gem_context.c',
>> diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
>> index 2179a76d..25bb6ad7 100644
>> --- a/tests/dmabuf_sync_file.c
>> +++ b/tests/dmabuf_sync_file.c
>> @@ -1,140 +1,15 @@
>> // SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>>
>> #include "igt.h"
>> #include "igt_vgem.h"
>> #include "sw_sync.h"
>> -
>> -#include <linux/dma-buf.h>
>> -#include <sys/poll.h>
>> +#include "dmabuf_sync_file.h"
>>
>> IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
>>
>> -struct igt_dma_buf_sync_file {
>> - __u32 flags;
>> - __s32 fd;
>> -};
>> -
>> -#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
>> -#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
>> -
>> -static bool has_dmabuf_export_sync_file(int fd)
>> -{
>> - struct vgem_bo bo;
>> - int dmabuf, ret;
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - bo.width = 1;
>> - bo.height = 1;
>> - bo.bpp = 32;
>> - vgem_create(fd, &bo);
>> -
>> - dmabuf = prime_handle_to_fd(fd, bo.handle);
>> - gem_close(fd, bo.handle);
>> -
>> - arg.flags = DMA_BUF_SYNC_WRITE;
>> - arg.fd = -1;
>> -
>> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> - close(dmabuf);
>> - igt_assert(ret == 0 || errno == ENOTTY);
>> -
>> - return ret == 0;
>> -}
>> -
>> -static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
>> -{
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - arg.flags = flags;
>> - arg.fd = -1;
>> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> -
>> - return arg.fd;
>> -}
>> -
>> -static bool has_dmabuf_import_sync_file(int fd)
>> -{
>> - struct vgem_bo bo;
>> - int dmabuf, timeline, fence, ret;
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - bo.width = 1;
>> - bo.height = 1;
>> - bo.bpp = 32;
>> - vgem_create(fd, &bo);
>> -
>> - dmabuf = prime_handle_to_fd(fd, bo.handle);
>> - gem_close(fd, bo.handle);
>> -
>> - timeline = sw_sync_timeline_create();
>> - fence = sw_sync_timeline_create_fence(timeline, 1);
>> - sw_sync_timeline_inc(timeline, 1);
>> -
>> - arg.flags = DMA_BUF_SYNC_RW;
>> - arg.fd = fence;
>> -
>> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> - close(dmabuf);
>> - close(fence);
>> - igt_assert(ret == 0 || errno == ENOTTY);
>> -
>> - return ret == 0;
>> -}
>> -
>> -static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
>> -{
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - arg.flags = flags;
>> - arg.fd = sync_fd;
>> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> -}
>> -
>> -static void
>> -dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
>> - int timeline, uint32_t seqno)
>> -{
>> - int fence;
>> -
>> - fence = sw_sync_timeline_create_fence(timeline, seqno);
>> - dmabuf_import_sync_file(dmabuf, flags, fence);
>> - close(fence);
>> -}
>> -
>> -static bool dmabuf_busy(int dmabuf, uint32_t flags)
>> -{
>> - struct pollfd pfd = { .fd = dmabuf };
>> -
>> - /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
>> - * else poll() may return a non-zero value if there are only read
>> - * fences because POLLIN is ready even if POLLOUT isn't.
>> - */
>> - if (flags & DMA_BUF_SYNC_WRITE)
>> - pfd.events |= POLLOUT;
>> - else if (flags & DMA_BUF_SYNC_READ)
>> - pfd.events |= POLLIN;
>> -
>> - return poll(&pfd, 1, 0) == 0;
>> -}
>> -
>> -static bool sync_file_busy(int sync_file)
>> -{
>> - struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
>> - return poll(&pfd, 1, 0) == 0;
>> -}
>> -
>> -static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
>> -{
>> - int sync_file;
>> - bool busy;
>> -
>> - sync_file = dmabuf_export_sync_file(dmabuf, flags);
>> - busy = sync_file_busy(sync_file);
>> - close(sync_file);
>> -
>> - return busy;
>> -}
>> -
>> static void test_export_basic(int fd)
>> {
>> struct vgem_bo bo;
>> --
>> 2.38.1
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
@ 2022-12-05 17:11 ` Matthew Auld
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2022-12-05 17:11 UTC (permalink / raw)
To: Kamil Konieczny, igt-dev, intel-gfx, Petri Latvala,
Andrzej Hajda, Nirmoy Das
On 05/12/2022 16:31, Kamil Konieczny wrote:
> Hi Matt,
>
> after re-reading I have few more nits.
>
> On 2022-12-02 at 12:02:41 +0000, Matthew Auld wrote:
>> So we can use this across different tests.
>>
>> v2
>> - Add docs for everything (Petri)
>> - Add missing copyright and fix headers slightly (Kamil)
>>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
>> Cc: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>> .../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
>> lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
>> lib/dmabuf_sync_file.h | 26 +++
>> lib/meson.build | 1 +
>> tests/dmabuf_sync_file.c | 133 +----------
>> 5 files changed, 243 insertions(+), 129 deletions(-)
>> create mode 100644 lib/dmabuf_sync_file.c
>> create mode 100644 lib/dmabuf_sync_file.h
>>
>> diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>> index 1ce842f4..102c8a89 100644
>> --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>> +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>> @@ -15,6 +15,7 @@
>>
>> <chapter>
>> <title>API Reference</title>
>> + <xi:include href="xml/dmabuf_sync_file.xml"/>
>> <xi:include href="xml/drmtest.xml"/>
>> <xi:include href="xml/igt_alsa.xml"/>
>> <xi:include href="xml/igt_audio.xml"/>
>> diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
>> new file mode 100644
>> index 00000000..bcce2486
>> --- /dev/null
>> +++ b/lib/dmabuf_sync_file.c
>> @@ -0,0 +1,211 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#include "igt.h"
>> +#include "igt_vgem.h"
>> +#include "sw_sync.h"
>> +
>> +#include "dmabuf_sync_file.h"
>> +
>> +/**
>> + * SECTION: dmabuf_sync_file
>> + * @short_description: DMABUF importing/exporting fencing support library
>> + * @title: DMABUF Sync File
>> + * @include: dmabuf_sync_file.h
>> + */
>> +
>> +struct igt_dma_buf_sync_file {
>> + __u32 flags;
>> + __s32 fd;
>> +};
>> +
>> +#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
>> +#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
>> +
>> +/**
>> + * has_dmabuf_export_sync_file:
>> + * @fd: The open drm fd
>> + *
>> + * Check if the kernel supports exporting a sync file from dmabuf.
>> + */
>> +bool has_dmabuf_export_sync_file(int fd)
>> +{
>> + struct vgem_bo bo;
>> + int dmabuf, ret;
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + bo.width = 1;
>> + bo.height = 1;
>> + bo.bpp = 32;
>> + vgem_create(fd, &bo);
>> +
>> + dmabuf = prime_handle_to_fd(fd, bo.handle);
>> + gem_close(fd, bo.handle);
>> +
>> + arg.flags = DMA_BUF_SYNC_WRITE;
>> + arg.fd = -1;
>> +
>> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> + close(dmabuf);
>> + igt_assert(ret == 0 || errno == ENOTTY);
>
> imho we should not explode here, it was ok in test but in lib
> we should just return false in case of unexpected error, you can
> add igt_debug if you think it will help.
> This may lead to change in place where you use it, like
> igt_require(has_dmabuf_export_sync_file(fd));
Yup, makes sense. Will fix.
>
>> +
>> + return ret == 0;
>> +}
>> +
>> +/**
>> + * dmabuf_export_sync_file:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + *
>> + * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
>> + * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
>> + * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
>> + */
>> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
>
> As you do not check for errors so this should be
> int __dmabuf_export_sync_file(int dmabuf, uint32_t flags)
do_ioctl() is doing an igt_assert_eq(ioctl(...), 0). Or what do you mean
by not checking for errors?
>
>> +{
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + arg.flags = flags;
>> + arg.fd = -1;
>> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> +
>> + return arg.fd;
>> +}
>> +
>> +/**
>> + * has_dmabuf_import_sync_file:
>> + * @fd: The open drm fd
>> + *
>> + * Check if the kernel supports importing a sync file into a dmabuf.
>> + */
>> +bool has_dmabuf_import_sync_file(int fd)
>> +{
>> + struct vgem_bo bo;
>> + int dmabuf, timeline, fence, ret;
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + bo.width = 1;
>> + bo.height = 1;
>> + bo.bpp = 32;
>> + vgem_create(fd, &bo);
>> +
>> + dmabuf = prime_handle_to_fd(fd, bo.handle);
>> + gem_close(fd, bo.handle);
>> +
>> + timeline = sw_sync_timeline_create();
>> + fence = sw_sync_timeline_create_fence(timeline, 1);
>> + sw_sync_timeline_inc(timeline, 1);
>> +
>> + arg.flags = DMA_BUF_SYNC_RW;
>> + arg.fd = fence;
>> +
>> + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> + close(dmabuf);
>> + close(fence);
>> + igt_assert(ret == 0 || errno == ENOTTY);
>
> Same here, return false instead of assert.
>
>> +
>> + return ret == 0;
>> +}
>> +
>> +/**
>> + * dmabuf_import_sync_file:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + * @sync_fd: The sync file (i.e our fence) to import
>> + *
>> + * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
>> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
>> + * importing the @sync_fd as a read or write fence.
>> + */
>> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
>
> Same here, no error checks so use __ like:
> void __dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
>
>> +{
>> + struct igt_dma_buf_sync_file arg;
>> +
>> + arg.flags = flags;
>> + arg.fd = sync_fd;
>> + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> +}
>> +
>> +/**
>> + * dmabuf_import_timeline_fence:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + * @timeline: The sync file timeline where the new fence is created
>> + * @seqno: The sequence number to use for the fence
>> + *
>> + * Create a new fence as part of @timeline, and import as a sync file into the
>> + * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
>> + * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
>> + * or write fence.
>> + */
>> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
>
> Same here, no error checks so use __ before function name.
>
>> + int timeline, uint32_t seqno)
>> +{
>> + int fence;
>> +
>> + fence = sw_sync_timeline_create_fence(timeline, seqno);
>> + dmabuf_import_sync_file(dmabuf, flags, fence);
>> + close(fence);
>> +}
>> +
>> +/**
>> + * dmabuf_busy:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + *
>> + * Check if the fences in the dmabuf are still busy. The @flags should at least
>> + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
>> + * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
>> + * we care about both.
>> + */
>> +bool dmabuf_busy(int dmabuf, uint32_t flags)
>> +{
>> + struct pollfd pfd = { .fd = dmabuf };
>> +
>> + /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
>> + * else poll() may return a non-zero value if there are only read
>> + * fences because POLLIN is ready even if POLLOUT isn't.
>> + */
>> + if (flags & DMA_BUF_SYNC_WRITE)
>> + pfd.events |= POLLOUT;
>> + else if (flags & DMA_BUF_SYNC_READ)
>> + pfd.events |= POLLIN;
>> +
>> + return poll(&pfd, 1, 0) == 0;
>> +}
>> +
>> +/**
>> + * sync_file_busy:
>> + * @sync_file: The sync file to check
>> + *
>> + * Check if the @sync_file is still busy or not.
>> + */
>> +bool sync_file_busy(int sync_file)
>> +{
>> + struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
>> + return poll(&pfd, 1, 0) == 0;
>> +}
>> +
>> +/**
>> + * dmabuf_sync_file_busy:
>> + * @dmabuf: The dmabuf fd
>> + * @flags: The flags to control the behaviour
>> + *
>> + * Export the current fences in @dmabuf as a sync file and check if still busy.
>> + * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
>> + * to specify which fences are to be exported from the @dmabuf and checked if
>> + * busy. Or DMA_BUF_SYNC_RW if we care about both.
>> + */
>> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
>> +{
>> + int sync_file;
>> + bool busy;
>> +
>> + sync_file = dmabuf_export_sync_file(dmabuf, flags);
>
> Check for error here.
>
> Regards,
> Kamil
>
>> + busy = sync_file_busy(sync_file);
>> + close(sync_file);
>> +
>> + return busy;
>> +}
>> diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
>> new file mode 100644
>> index 00000000..d642ff30
>> --- /dev/null
>> +++ b/lib/dmabuf_sync_file.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#ifndef DMABUF_SYNC_FILE_H
>> +#define DMABUF_SYNC_FILE_H
>> +
>> +#ifdef __linux__
>> +#include <linux/dma-buf.h>
>> +#endif
>> +#include <sys/poll.h>
>> +#include <stdbool.h>
>> +#include <stdint.h>
>> +
>> +bool has_dmabuf_export_sync_file(int fd);
>> +bool has_dmabuf_import_sync_file(int fd);
>> +int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
>> +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
>> +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
>> + int timeline, uint32_t seqno);
>> +bool dmabuf_busy(int dmabuf, uint32_t flags);
>> +bool sync_file_busy(int sync_file);
>> +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
>> +
>> +#endif
>> diff --git a/lib/meson.build b/lib/meson.build
>> index cef2d0ff..896d5733 100644
>> --- a/lib/meson.build
>> +++ b/lib/meson.build
>> @@ -1,5 +1,6 @@
>> lib_sources = [
>> 'drmtest.c',
>> + 'dmabuf_sync_file.c',
>> 'huc_copy.c',
>> 'i915/gem.c',
>> 'i915/gem_context.c',
>> diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
>> index 2179a76d..25bb6ad7 100644
>> --- a/tests/dmabuf_sync_file.c
>> +++ b/tests/dmabuf_sync_file.c
>> @@ -1,140 +1,15 @@
>> // SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>>
>> #include "igt.h"
>> #include "igt_vgem.h"
>> #include "sw_sync.h"
>> -
>> -#include <linux/dma-buf.h>
>> -#include <sys/poll.h>
>> +#include "dmabuf_sync_file.h"
>>
>> IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
>>
>> -struct igt_dma_buf_sync_file {
>> - __u32 flags;
>> - __s32 fd;
>> -};
>> -
>> -#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
>> -#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
>> -
>> -static bool has_dmabuf_export_sync_file(int fd)
>> -{
>> - struct vgem_bo bo;
>> - int dmabuf, ret;
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - bo.width = 1;
>> - bo.height = 1;
>> - bo.bpp = 32;
>> - vgem_create(fd, &bo);
>> -
>> - dmabuf = prime_handle_to_fd(fd, bo.handle);
>> - gem_close(fd, bo.handle);
>> -
>> - arg.flags = DMA_BUF_SYNC_WRITE;
>> - arg.fd = -1;
>> -
>> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> - close(dmabuf);
>> - igt_assert(ret == 0 || errno == ENOTTY);
>> -
>> - return ret == 0;
>> -}
>> -
>> -static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
>> -{
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - arg.flags = flags;
>> - arg.fd = -1;
>> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
>> -
>> - return arg.fd;
>> -}
>> -
>> -static bool has_dmabuf_import_sync_file(int fd)
>> -{
>> - struct vgem_bo bo;
>> - int dmabuf, timeline, fence, ret;
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - bo.width = 1;
>> - bo.height = 1;
>> - bo.bpp = 32;
>> - vgem_create(fd, &bo);
>> -
>> - dmabuf = prime_handle_to_fd(fd, bo.handle);
>> - gem_close(fd, bo.handle);
>> -
>> - timeline = sw_sync_timeline_create();
>> - fence = sw_sync_timeline_create_fence(timeline, 1);
>> - sw_sync_timeline_inc(timeline, 1);
>> -
>> - arg.flags = DMA_BUF_SYNC_RW;
>> - arg.fd = fence;
>> -
>> - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> - close(dmabuf);
>> - close(fence);
>> - igt_assert(ret == 0 || errno == ENOTTY);
>> -
>> - return ret == 0;
>> -}
>> -
>> -static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
>> -{
>> - struct igt_dma_buf_sync_file arg;
>> -
>> - arg.flags = flags;
>> - arg.fd = sync_fd;
>> - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
>> -}
>> -
>> -static void
>> -dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
>> - int timeline, uint32_t seqno)
>> -{
>> - int fence;
>> -
>> - fence = sw_sync_timeline_create_fence(timeline, seqno);
>> - dmabuf_import_sync_file(dmabuf, flags, fence);
>> - close(fence);
>> -}
>> -
>> -static bool dmabuf_busy(int dmabuf, uint32_t flags)
>> -{
>> - struct pollfd pfd = { .fd = dmabuf };
>> -
>> - /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
>> - * else poll() may return a non-zero value if there are only read
>> - * fences because POLLIN is ready even if POLLOUT isn't.
>> - */
>> - if (flags & DMA_BUF_SYNC_WRITE)
>> - pfd.events |= POLLOUT;
>> - else if (flags & DMA_BUF_SYNC_READ)
>> - pfd.events |= POLLIN;
>> -
>> - return poll(&pfd, 1, 0) == 0;
>> -}
>> -
>> -static bool sync_file_busy(int sync_file)
>> -{
>> - struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
>> - return poll(&pfd, 1, 0) == 0;
>> -}
>> -
>> -static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
>> -{
>> - int sync_file;
>> - bool busy;
>> -
>> - sync_file = dmabuf_export_sync_file(dmabuf, flags);
>> - busy = sync_file_busy(sync_file);
>> - close(sync_file);
>> -
>> - return busy;
>> -}
>> -
>> static void test_export_basic(int fd)
>> {
>> struct vgem_bo bo;
>> --
>> 2.38.1
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
2022-12-05 17:11 ` [igt-dev] " Matthew Auld
@ 2022-12-07 11:50 ` Kamil Konieczny
-1 siblings, 0 replies; 12+ messages in thread
From: Kamil Konieczny @ 2022-12-07 11:50 UTC (permalink / raw)
Cc: intel-gfx, igt-dev, Matthew Auld, Andrzej Hajda, Nirmoy Das
Hi Matt,
On 2022-12-05 at 17:11:44 +0000, Matthew Auld wrote:
> On 05/12/2022 16:31, Kamil Konieczny wrote:
> > Hi Matt,
> >
> > after re-reading I have few more nits.
> >
> > On 2022-12-02 at 12:02:41 +0000, Matthew Auld wrote:
> > > So we can use this across different tests.
> > >
> > > v2
> > > - Add docs for everything (Petri)
> > > - Add missing copyright and fix headers slightly (Kamil)
> > >
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > > Cc: Nirmoy Das <nirmoy.das@intel.com>
> > > ---
> > > .../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
> > > lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
> > > lib/dmabuf_sync_file.h | 26 +++
> > > lib/meson.build | 1 +
> > > tests/dmabuf_sync_file.c | 133 +----------
> > > 5 files changed, 243 insertions(+), 129 deletions(-)
> > > create mode 100644 lib/dmabuf_sync_file.c
> > > create mode 100644 lib/dmabuf_sync_file.h
> > >
> > > diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> > > index 1ce842f4..102c8a89 100644
> > > --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> > > +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> > > @@ -15,6 +15,7 @@
> > > <chapter>
> > > <title>API Reference</title>
> > > + <xi:include href="xml/dmabuf_sync_file.xml"/>
> > > <xi:include href="xml/drmtest.xml"/>
> > > <xi:include href="xml/igt_alsa.xml"/>
> > > <xi:include href="xml/igt_audio.xml"/>
> > > diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
> > > new file mode 100644
> > > index 00000000..bcce2486
> > > --- /dev/null
> > > +++ b/lib/dmabuf_sync_file.c
> > > @@ -0,0 +1,211 @@
> > > +// SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > +
> > > +#include "igt.h"
> > > +#include "igt_vgem.h"
> > > +#include "sw_sync.h"
> > > +
> > > +#include "dmabuf_sync_file.h"
> > > +
> > > +/**
> > > + * SECTION: dmabuf_sync_file
> > > + * @short_description: DMABUF importing/exporting fencing support library
> > > + * @title: DMABUF Sync File
> > > + * @include: dmabuf_sync_file.h
> > > + */
> > > +
> > > +struct igt_dma_buf_sync_file {
> > > + __u32 flags;
> > > + __s32 fd;
> > > +};
> > > +
> > > +#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> > > +#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> > > +
> > > +/**
> > > + * has_dmabuf_export_sync_file:
> > > + * @fd: The open drm fd
> > > + *
> > > + * Check if the kernel supports exporting a sync file from dmabuf.
> > > + */
> > > +bool has_dmabuf_export_sync_file(int fd)
> > > +{
> > > + struct vgem_bo bo;
> > > + int dmabuf, ret;
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + bo.width = 1;
> > > + bo.height = 1;
> > > + bo.bpp = 32;
> > > + vgem_create(fd, &bo);
> > > +
> > > + dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > + gem_close(fd, bo.handle);
> > > +
> > > + arg.flags = DMA_BUF_SYNC_WRITE;
> > > + arg.fd = -1;
> > > +
> > > + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > + close(dmabuf);
> > > + igt_assert(ret == 0 || errno == ENOTTY);
> >
> > imho we should not explode here, it was ok in test but in lib
> > we should just return false in case of unexpected error, you can
> > add igt_debug if you think it will help.
> > This may lead to change in place where you use it, like
> > igt_require(has_dmabuf_export_sync_file(fd));
>
> Yup, makes sense. Will fix.
>
> >
> > > +
> > > + return ret == 0;
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_export_sync_file:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + *
> > > + * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
> > > + * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
> > > + * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
> > > + */
> > > +int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> >
> > As you do not check for errors so this should be
> > int __dmabuf_export_sync_file(int dmabuf, uint32_t flags)
>
> do_ioctl() is doing an igt_assert_eq(ioctl(...), 0). Or what do you mean by
> not checking for errors?
You are right, sorry, I readed it as igt_ioctl so
please do not follow my comment here.
Regards,
Kamil
>
> >
> > > +{
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + arg.flags = flags;
> > > + arg.fd = -1;
> > > + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > +
> > > + return arg.fd;
> > > +}
> > > +
> > > +/**
> > > + * has_dmabuf_import_sync_file:
> > > + * @fd: The open drm fd
> > > + *
> > > + * Check if the kernel supports importing a sync file into a dmabuf.
> > > + */
> > > +bool has_dmabuf_import_sync_file(int fd)
> > > +{
> > > + struct vgem_bo bo;
> > > + int dmabuf, timeline, fence, ret;
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + bo.width = 1;
> > > + bo.height = 1;
> > > + bo.bpp = 32;
> > > + vgem_create(fd, &bo);
> > > +
> > > + dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > + gem_close(fd, bo.handle);
> > > +
> > > + timeline = sw_sync_timeline_create();
> > > + fence = sw_sync_timeline_create_fence(timeline, 1);
> > > + sw_sync_timeline_inc(timeline, 1);
> > > +
> > > + arg.flags = DMA_BUF_SYNC_RW;
> > > + arg.fd = fence;
> > > +
> > > + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > + close(dmabuf);
> > > + close(fence);
> > > + igt_assert(ret == 0 || errno == ENOTTY);
> >
> > Same here, return false instead of assert.
> >
> > > +
> > > + return ret == 0;
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_import_sync_file:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + * @sync_fd: The sync file (i.e our fence) to import
> > > + *
> > > + * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
> > > + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> > > + * importing the @sync_fd as a read or write fence.
> > > + */
> > > +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> >
> > Same here, no error checks so use __ like:
> > void __dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> >
> > > +{
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + arg.flags = flags;
> > > + arg.fd = sync_fd;
> > > + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_import_timeline_fence:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + * @timeline: The sync file timeline where the new fence is created
> > > + * @seqno: The sequence number to use for the fence
> > > + *
> > > + * Create a new fence as part of @timeline, and import as a sync file into the
> > > + * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
> > > + * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
> > > + * or write fence.
> > > + */
> > > +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> >
> > Same here, no error checks so use __ before function name.
> >
> > > + int timeline, uint32_t seqno)
> > > +{
> > > + int fence;
> > > +
> > > + fence = sw_sync_timeline_create_fence(timeline, seqno);
> > > + dmabuf_import_sync_file(dmabuf, flags, fence);
> > > + close(fence);
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_busy:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + *
> > > + * Check if the fences in the dmabuf are still busy. The @flags should at least
> > > + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> > > + * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
> > > + * we care about both.
> > > + */
> > > +bool dmabuf_busy(int dmabuf, uint32_t flags)
> > > +{
> > > + struct pollfd pfd = { .fd = dmabuf };
> > > +
> > > + /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> > > + * else poll() may return a non-zero value if there are only read
> > > + * fences because POLLIN is ready even if POLLOUT isn't.
> > > + */
> > > + if (flags & DMA_BUF_SYNC_WRITE)
> > > + pfd.events |= POLLOUT;
> > > + else if (flags & DMA_BUF_SYNC_READ)
> > > + pfd.events |= POLLIN;
> > > +
> > > + return poll(&pfd, 1, 0) == 0;
> > > +}
> > > +
> > > +/**
> > > + * sync_file_busy:
> > > + * @sync_file: The sync file to check
> > > + *
> > > + * Check if the @sync_file is still busy or not.
> > > + */
> > > +bool sync_file_busy(int sync_file)
> > > +{
> > > + struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> > > + return poll(&pfd, 1, 0) == 0;
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_sync_file_busy:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + *
> > > + * Export the current fences in @dmabuf as a sync file and check if still busy.
> > > + * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
> > > + * to specify which fences are to be exported from the @dmabuf and checked if
> > > + * busy. Or DMA_BUF_SYNC_RW if we care about both.
> > > + */
> > > +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> > > +{
> > > + int sync_file;
> > > + bool busy;
> > > +
> > > + sync_file = dmabuf_export_sync_file(dmabuf, flags);
> >
> > Check for error here.
> >
> > Regards,
> > Kamil
> >
> > > + busy = sync_file_busy(sync_file);
> > > + close(sync_file);
> > > +
> > > + return busy;
> > > +}
> > > diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
> > > new file mode 100644
> > > index 00000000..d642ff30
> > > --- /dev/null
> > > +++ b/lib/dmabuf_sync_file.h
> > > @@ -0,0 +1,26 @@
> > > +/* SPDX-License-Identifier: MIT */
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > +
> > > +#ifndef DMABUF_SYNC_FILE_H
> > > +#define DMABUF_SYNC_FILE_H
> > > +
> > > +#ifdef __linux__
> > > +#include <linux/dma-buf.h>
> > > +#endif
> > > +#include <sys/poll.h>
> > > +#include <stdbool.h>
> > > +#include <stdint.h>
> > > +
> > > +bool has_dmabuf_export_sync_file(int fd);
> > > +bool has_dmabuf_import_sync_file(int fd);
> > > +int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
> > > +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
> > > +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> > > + int timeline, uint32_t seqno);
> > > +bool dmabuf_busy(int dmabuf, uint32_t flags);
> > > +bool sync_file_busy(int sync_file);
> > > +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
> > > +
> > > +#endif
> > > diff --git a/lib/meson.build b/lib/meson.build
> > > index cef2d0ff..896d5733 100644
> > > --- a/lib/meson.build
> > > +++ b/lib/meson.build
> > > @@ -1,5 +1,6 @@
> > > lib_sources = [
> > > 'drmtest.c',
> > > + 'dmabuf_sync_file.c',
> > > 'huc_copy.c',
> > > 'i915/gem.c',
> > > 'i915/gem_context.c',
> > > diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
> > > index 2179a76d..25bb6ad7 100644
> > > --- a/tests/dmabuf_sync_file.c
> > > +++ b/tests/dmabuf_sync_file.c
> > > @@ -1,140 +1,15 @@
> > > // SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > #include "igt.h"
> > > #include "igt_vgem.h"
> > > #include "sw_sync.h"
> > > -
> > > -#include <linux/dma-buf.h>
> > > -#include <sys/poll.h>
> > > +#include "dmabuf_sync_file.h"
> > > IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
> > > -struct igt_dma_buf_sync_file {
> > > - __u32 flags;
> > > - __s32 fd;
> > > -};
> > > -
> > > -#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> > > -#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> > > -
> > > -static bool has_dmabuf_export_sync_file(int fd)
> > > -{
> > > - struct vgem_bo bo;
> > > - int dmabuf, ret;
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - bo.width = 1;
> > > - bo.height = 1;
> > > - bo.bpp = 32;
> > > - vgem_create(fd, &bo);
> > > -
> > > - dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > - gem_close(fd, bo.handle);
> > > -
> > > - arg.flags = DMA_BUF_SYNC_WRITE;
> > > - arg.fd = -1;
> > > -
> > > - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > - close(dmabuf);
> > > - igt_assert(ret == 0 || errno == ENOTTY);
> > > -
> > > - return ret == 0;
> > > -}
> > > -
> > > -static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> > > -{
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - arg.flags = flags;
> > > - arg.fd = -1;
> > > - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > -
> > > - return arg.fd;
> > > -}
> > > -
> > > -static bool has_dmabuf_import_sync_file(int fd)
> > > -{
> > > - struct vgem_bo bo;
> > > - int dmabuf, timeline, fence, ret;
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - bo.width = 1;
> > > - bo.height = 1;
> > > - bo.bpp = 32;
> > > - vgem_create(fd, &bo);
> > > -
> > > - dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > - gem_close(fd, bo.handle);
> > > -
> > > - timeline = sw_sync_timeline_create();
> > > - fence = sw_sync_timeline_create_fence(timeline, 1);
> > > - sw_sync_timeline_inc(timeline, 1);
> > > -
> > > - arg.flags = DMA_BUF_SYNC_RW;
> > > - arg.fd = fence;
> > > -
> > > - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > - close(dmabuf);
> > > - close(fence);
> > > - igt_assert(ret == 0 || errno == ENOTTY);
> > > -
> > > - return ret == 0;
> > > -}
> > > -
> > > -static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> > > -{
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - arg.flags = flags;
> > > - arg.fd = sync_fd;
> > > - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > -}
> > > -
> > > -static void
> > > -dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> > > - int timeline, uint32_t seqno)
> > > -{
> > > - int fence;
> > > -
> > > - fence = sw_sync_timeline_create_fence(timeline, seqno);
> > > - dmabuf_import_sync_file(dmabuf, flags, fence);
> > > - close(fence);
> > > -}
> > > -
> > > -static bool dmabuf_busy(int dmabuf, uint32_t flags)
> > > -{
> > > - struct pollfd pfd = { .fd = dmabuf };
> > > -
> > > - /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> > > - * else poll() may return a non-zero value if there are only read
> > > - * fences because POLLIN is ready even if POLLOUT isn't.
> > > - */
> > > - if (flags & DMA_BUF_SYNC_WRITE)
> > > - pfd.events |= POLLOUT;
> > > - else if (flags & DMA_BUF_SYNC_READ)
> > > - pfd.events |= POLLIN;
> > > -
> > > - return poll(&pfd, 1, 0) == 0;
> > > -}
> > > -
> > > -static bool sync_file_busy(int sync_file)
> > > -{
> > > - struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> > > - return poll(&pfd, 1, 0) == 0;
> > > -}
> > > -
> > > -static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> > > -{
> > > - int sync_file;
> > > - bool busy;
> > > -
> > > - sync_file = dmabuf_export_sync_file(dmabuf, flags);
> > > - busy = sync_file_busy(sync_file);
> > > - close(sync_file);
> > > -
> > > - return busy;
> > > -}
> > > -
> > > static void test_export_basic(int fd)
> > > {
> > > struct vgem_bo bo;
> > > --
> > > 2.38.1
> > >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib
@ 2022-12-07 11:50 ` Kamil Konieczny
0 siblings, 0 replies; 12+ messages in thread
From: Kamil Konieczny @ 2022-12-07 11:50 UTC (permalink / raw)
Cc: Petri Latvala, intel-gfx, igt-dev, Matthew Auld, Nirmoy Das
Hi Matt,
On 2022-12-05 at 17:11:44 +0000, Matthew Auld wrote:
> On 05/12/2022 16:31, Kamil Konieczny wrote:
> > Hi Matt,
> >
> > after re-reading I have few more nits.
> >
> > On 2022-12-02 at 12:02:41 +0000, Matthew Auld wrote:
> > > So we can use this across different tests.
> > >
> > > v2
> > > - Add docs for everything (Petri)
> > > - Add missing copyright and fix headers slightly (Kamil)
> > >
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > > Cc: Nirmoy Das <nirmoy.das@intel.com>
> > > ---
> > > .../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
> > > lib/dmabuf_sync_file.c | 211 ++++++++++++++++++
> > > lib/dmabuf_sync_file.h | 26 +++
> > > lib/meson.build | 1 +
> > > tests/dmabuf_sync_file.c | 133 +----------
> > > 5 files changed, 243 insertions(+), 129 deletions(-)
> > > create mode 100644 lib/dmabuf_sync_file.c
> > > create mode 100644 lib/dmabuf_sync_file.h
> > >
> > > diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> > > index 1ce842f4..102c8a89 100644
> > > --- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> > > +++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
> > > @@ -15,6 +15,7 @@
> > > <chapter>
> > > <title>API Reference</title>
> > > + <xi:include href="xml/dmabuf_sync_file.xml"/>
> > > <xi:include href="xml/drmtest.xml"/>
> > > <xi:include href="xml/igt_alsa.xml"/>
> > > <xi:include href="xml/igt_audio.xml"/>
> > > diff --git a/lib/dmabuf_sync_file.c b/lib/dmabuf_sync_file.c
> > > new file mode 100644
> > > index 00000000..bcce2486
> > > --- /dev/null
> > > +++ b/lib/dmabuf_sync_file.c
> > > @@ -0,0 +1,211 @@
> > > +// SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > +
> > > +#include "igt.h"
> > > +#include "igt_vgem.h"
> > > +#include "sw_sync.h"
> > > +
> > > +#include "dmabuf_sync_file.h"
> > > +
> > > +/**
> > > + * SECTION: dmabuf_sync_file
> > > + * @short_description: DMABUF importing/exporting fencing support library
> > > + * @title: DMABUF Sync File
> > > + * @include: dmabuf_sync_file.h
> > > + */
> > > +
> > > +struct igt_dma_buf_sync_file {
> > > + __u32 flags;
> > > + __s32 fd;
> > > +};
> > > +
> > > +#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> > > +#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> > > +
> > > +/**
> > > + * has_dmabuf_export_sync_file:
> > > + * @fd: The open drm fd
> > > + *
> > > + * Check if the kernel supports exporting a sync file from dmabuf.
> > > + */
> > > +bool has_dmabuf_export_sync_file(int fd)
> > > +{
> > > + struct vgem_bo bo;
> > > + int dmabuf, ret;
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + bo.width = 1;
> > > + bo.height = 1;
> > > + bo.bpp = 32;
> > > + vgem_create(fd, &bo);
> > > +
> > > + dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > + gem_close(fd, bo.handle);
> > > +
> > > + arg.flags = DMA_BUF_SYNC_WRITE;
> > > + arg.fd = -1;
> > > +
> > > + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > + close(dmabuf);
> > > + igt_assert(ret == 0 || errno == ENOTTY);
> >
> > imho we should not explode here, it was ok in test but in lib
> > we should just return false in case of unexpected error, you can
> > add igt_debug if you think it will help.
> > This may lead to change in place where you use it, like
> > igt_require(has_dmabuf_export_sync_file(fd));
>
> Yup, makes sense. Will fix.
>
> >
> > > +
> > > + return ret == 0;
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_export_sync_file:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + *
> > > + * Take a snapshot of the current dma-resv fences in the dmabuf, and export as a
> > > + * syncfile. The @flags should at least specify either DMA_BUF_SYNC_WRITE or
> > > + * DMA_BUF_SYNC_READ, depending on if we care about the read or write fences.
> > > + */
> > > +int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> >
> > As you do not check for errors so this should be
> > int __dmabuf_export_sync_file(int dmabuf, uint32_t flags)
>
> do_ioctl() is doing an igt_assert_eq(ioctl(...), 0). Or what do you mean by
> not checking for errors?
You are right, sorry, I readed it as igt_ioctl so
please do not follow my comment here.
Regards,
Kamil
>
> >
> > > +{
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + arg.flags = flags;
> > > + arg.fd = -1;
> > > + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > +
> > > + return arg.fd;
> > > +}
> > > +
> > > +/**
> > > + * has_dmabuf_import_sync_file:
> > > + * @fd: The open drm fd
> > > + *
> > > + * Check if the kernel supports importing a sync file into a dmabuf.
> > > + */
> > > +bool has_dmabuf_import_sync_file(int fd)
> > > +{
> > > + struct vgem_bo bo;
> > > + int dmabuf, timeline, fence, ret;
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + bo.width = 1;
> > > + bo.height = 1;
> > > + bo.bpp = 32;
> > > + vgem_create(fd, &bo);
> > > +
> > > + dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > + gem_close(fd, bo.handle);
> > > +
> > > + timeline = sw_sync_timeline_create();
> > > + fence = sw_sync_timeline_create_fence(timeline, 1);
> > > + sw_sync_timeline_inc(timeline, 1);
> > > +
> > > + arg.flags = DMA_BUF_SYNC_RW;
> > > + arg.fd = fence;
> > > +
> > > + ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > + close(dmabuf);
> > > + close(fence);
> > > + igt_assert(ret == 0 || errno == ENOTTY);
> >
> > Same here, return false instead of assert.
> >
> > > +
> > > + return ret == 0;
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_import_sync_file:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + * @sync_fd: The sync file (i.e our fence) to import
> > > + *
> > > + * Import the sync file @sync_fd, into the dmabuf. The @flags should at least
> > > + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> > > + * importing the @sync_fd as a read or write fence.
> > > + */
> > > +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> >
> > Same here, no error checks so use __ like:
> > void __dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> >
> > > +{
> > > + struct igt_dma_buf_sync_file arg;
> > > +
> > > + arg.flags = flags;
> > > + arg.fd = sync_fd;
> > > + do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_import_timeline_fence:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + * @timeline: The sync file timeline where the new fence is created
> > > + * @seqno: The sequence number to use for the fence
> > > + *
> > > + * Create a new fence as part of @timeline, and import as a sync file into the
> > > + * dmabuf. The @flags should at least specify DMA_BUF_SYNC_WRITE or
> > > + * DMA_BUF_SYNC_READ, depending on if we are importing the new fence as a read
> > > + * or write fence.
> > > + */
> > > +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> >
> > Same here, no error checks so use __ before function name.
> >
> > > + int timeline, uint32_t seqno)
> > > +{
> > > + int fence;
> > > +
> > > + fence = sw_sync_timeline_create_fence(timeline, seqno);
> > > + dmabuf_import_sync_file(dmabuf, flags, fence);
> > > + close(fence);
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_busy:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + *
> > > + * Check if the fences in the dmabuf are still busy. The @flags should at least
> > > + * specify DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ, depending on if we are
> > > + * checking if the read or read fences have all signalled. Or DMA_BUF_SYNC_RW if
> > > + * we care about both.
> > > + */
> > > +bool dmabuf_busy(int dmabuf, uint32_t flags)
> > > +{
> > > + struct pollfd pfd = { .fd = dmabuf };
> > > +
> > > + /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> > > + * else poll() may return a non-zero value if there are only read
> > > + * fences because POLLIN is ready even if POLLOUT isn't.
> > > + */
> > > + if (flags & DMA_BUF_SYNC_WRITE)
> > > + pfd.events |= POLLOUT;
> > > + else if (flags & DMA_BUF_SYNC_READ)
> > > + pfd.events |= POLLIN;
> > > +
> > > + return poll(&pfd, 1, 0) == 0;
> > > +}
> > > +
> > > +/**
> > > + * sync_file_busy:
> > > + * @sync_file: The sync file to check
> > > + *
> > > + * Check if the @sync_file is still busy or not.
> > > + */
> > > +bool sync_file_busy(int sync_file)
> > > +{
> > > + struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> > > + return poll(&pfd, 1, 0) == 0;
> > > +}
> > > +
> > > +/**
> > > + * dmabuf_sync_file_busy:
> > > + * @dmabuf: The dmabuf fd
> > > + * @flags: The flags to control the behaviour
> > > + *
> > > + * Export the current fences in @dmabuf as a sync file and check if still busy.
> > > + * The @flags should at least contain DMA_BUF_SYNC_WRITE or DMA_BUF_SYNC_READ,
> > > + * to specify which fences are to be exported from the @dmabuf and checked if
> > > + * busy. Or DMA_BUF_SYNC_RW if we care about both.
> > > + */
> > > +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> > > +{
> > > + int sync_file;
> > > + bool busy;
> > > +
> > > + sync_file = dmabuf_export_sync_file(dmabuf, flags);
> >
> > Check for error here.
> >
> > Regards,
> > Kamil
> >
> > > + busy = sync_file_busy(sync_file);
> > > + close(sync_file);
> > > +
> > > + return busy;
> > > +}
> > > diff --git a/lib/dmabuf_sync_file.h b/lib/dmabuf_sync_file.h
> > > new file mode 100644
> > > index 00000000..d642ff30
> > > --- /dev/null
> > > +++ b/lib/dmabuf_sync_file.h
> > > @@ -0,0 +1,26 @@
> > > +/* SPDX-License-Identifier: MIT */
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > +
> > > +#ifndef DMABUF_SYNC_FILE_H
> > > +#define DMABUF_SYNC_FILE_H
> > > +
> > > +#ifdef __linux__
> > > +#include <linux/dma-buf.h>
> > > +#endif
> > > +#include <sys/poll.h>
> > > +#include <stdbool.h>
> > > +#include <stdint.h>
> > > +
> > > +bool has_dmabuf_export_sync_file(int fd);
> > > +bool has_dmabuf_import_sync_file(int fd);
> > > +int dmabuf_export_sync_file(int dmabuf, uint32_t flags);
> > > +void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd);
> > > +void dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> > > + int timeline, uint32_t seqno);
> > > +bool dmabuf_busy(int dmabuf, uint32_t flags);
> > > +bool sync_file_busy(int sync_file);
> > > +bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags);
> > > +
> > > +#endif
> > > diff --git a/lib/meson.build b/lib/meson.build
> > > index cef2d0ff..896d5733 100644
> > > --- a/lib/meson.build
> > > +++ b/lib/meson.build
> > > @@ -1,5 +1,6 @@
> > > lib_sources = [
> > > 'drmtest.c',
> > > + 'dmabuf_sync_file.c',
> > > 'huc_copy.c',
> > > 'i915/gem.c',
> > > 'i915/gem_context.c',
> > > diff --git a/tests/dmabuf_sync_file.c b/tests/dmabuf_sync_file.c
> > > index 2179a76d..25bb6ad7 100644
> > > --- a/tests/dmabuf_sync_file.c
> > > +++ b/tests/dmabuf_sync_file.c
> > > @@ -1,140 +1,15 @@
> > > // SPDX-License-Identifier: MIT
> > > +/*
> > > + * Copyright © 2022 Intel Corporation
> > > + */
> > > #include "igt.h"
> > > #include "igt_vgem.h"
> > > #include "sw_sync.h"
> > > -
> > > -#include <linux/dma-buf.h>
> > > -#include <sys/poll.h>
> > > +#include "dmabuf_sync_file.h"
> > > IGT_TEST_DESCRIPTION("Tests for sync_file support in dma-buf");
> > > -struct igt_dma_buf_sync_file {
> > > - __u32 flags;
> > > - __s32 fd;
> > > -};
> > > -
> > > -#define IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE _IOWR(DMA_BUF_BASE, 2, struct igt_dma_buf_sync_file)
> > > -#define IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE _IOW(DMA_BUF_BASE, 3, struct igt_dma_buf_sync_file)
> > > -
> > > -static bool has_dmabuf_export_sync_file(int fd)
> > > -{
> > > - struct vgem_bo bo;
> > > - int dmabuf, ret;
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - bo.width = 1;
> > > - bo.height = 1;
> > > - bo.bpp = 32;
> > > - vgem_create(fd, &bo);
> > > -
> > > - dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > - gem_close(fd, bo.handle);
> > > -
> > > - arg.flags = DMA_BUF_SYNC_WRITE;
> > > - arg.fd = -1;
> > > -
> > > - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > - close(dmabuf);
> > > - igt_assert(ret == 0 || errno == ENOTTY);
> > > -
> > > - return ret == 0;
> > > -}
> > > -
> > > -static int dmabuf_export_sync_file(int dmabuf, uint32_t flags)
> > > -{
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - arg.flags = flags;
> > > - arg.fd = -1;
> > > - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_EXPORT_SYNC_FILE, &arg);
> > > -
> > > - return arg.fd;
> > > -}
> > > -
> > > -static bool has_dmabuf_import_sync_file(int fd)
> > > -{
> > > - struct vgem_bo bo;
> > > - int dmabuf, timeline, fence, ret;
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - bo.width = 1;
> > > - bo.height = 1;
> > > - bo.bpp = 32;
> > > - vgem_create(fd, &bo);
> > > -
> > > - dmabuf = prime_handle_to_fd(fd, bo.handle);
> > > - gem_close(fd, bo.handle);
> > > -
> > > - timeline = sw_sync_timeline_create();
> > > - fence = sw_sync_timeline_create_fence(timeline, 1);
> > > - sw_sync_timeline_inc(timeline, 1);
> > > -
> > > - arg.flags = DMA_BUF_SYNC_RW;
> > > - arg.fd = fence;
> > > -
> > > - ret = igt_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > - close(dmabuf);
> > > - close(fence);
> > > - igt_assert(ret == 0 || errno == ENOTTY);
> > > -
> > > - return ret == 0;
> > > -}
> > > -
> > > -static void dmabuf_import_sync_file(int dmabuf, uint32_t flags, int sync_fd)
> > > -{
> > > - struct igt_dma_buf_sync_file arg;
> > > -
> > > - arg.flags = flags;
> > > - arg.fd = sync_fd;
> > > - do_ioctl(dmabuf, IGT_DMA_BUF_IOCTL_IMPORT_SYNC_FILE, &arg);
> > > -}
> > > -
> > > -static void
> > > -dmabuf_import_timeline_fence(int dmabuf, uint32_t flags,
> > > - int timeline, uint32_t seqno)
> > > -{
> > > - int fence;
> > > -
> > > - fence = sw_sync_timeline_create_fence(timeline, seqno);
> > > - dmabuf_import_sync_file(dmabuf, flags, fence);
> > > - close(fence);
> > > -}
> > > -
> > > -static bool dmabuf_busy(int dmabuf, uint32_t flags)
> > > -{
> > > - struct pollfd pfd = { .fd = dmabuf };
> > > -
> > > - /* If DMA_BUF_SYNC_WRITE is set, we don't want to set POLLIN or
> > > - * else poll() may return a non-zero value if there are only read
> > > - * fences because POLLIN is ready even if POLLOUT isn't.
> > > - */
> > > - if (flags & DMA_BUF_SYNC_WRITE)
> > > - pfd.events |= POLLOUT;
> > > - else if (flags & DMA_BUF_SYNC_READ)
> > > - pfd.events |= POLLIN;
> > > -
> > > - return poll(&pfd, 1, 0) == 0;
> > > -}
> > > -
> > > -static bool sync_file_busy(int sync_file)
> > > -{
> > > - struct pollfd pfd = { .fd = sync_file, .events = POLLIN };
> > > - return poll(&pfd, 1, 0) == 0;
> > > -}
> > > -
> > > -static bool dmabuf_sync_file_busy(int dmabuf, uint32_t flags)
> > > -{
> > > - int sync_file;
> > > - bool busy;
> > > -
> > > - sync_file = dmabuf_export_sync_file(dmabuf, flags);
> > > - busy = sync_file_busy(sync_file);
> > > - close(sync_file);
> > > -
> > > - return busy;
> > > -}
> > > -
> > > static void test_export_basic(int fd)
> > > {
> > > struct vgem_bo bo;
> > > --
> > > 2.38.1
> > >
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-12-07 11:50 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-02 12:02 [Intel-gfx] [PATCH i-g-t v2 1/2] lib/dmabuf_sync_file: move common stuff into lib Matthew Auld
2022-12-02 12:02 ` [igt-dev] " Matthew Auld
2022-12-02 12:02 ` [Intel-gfx] [PATCH i-g-t v2 2/2] tests/i915/gem_exec_balancer: exercise dmabuf import Matthew Auld
2022-12-02 12:02 ` [igt-dev] " Matthew Auld
2022-12-02 13:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v2,1/2] lib/dmabuf_sync_file: move common stuff into lib Patchwork
2022-12-05 14:32 ` Kamil Konieczny
2022-12-05 16:31 ` [Intel-gfx] [PATCH i-g-t v2 1/2] " Kamil Konieczny
2022-12-05 16:31 ` [igt-dev] " Kamil Konieczny
2022-12-05 17:11 ` [Intel-gfx] " Matthew Auld
2022-12-05 17:11 ` [igt-dev] " Matthew Auld
2022-12-07 11:50 ` [Intel-gfx] " Kamil Konieczny
2022-12-07 11:50 ` [igt-dev] " Kamil Konieczny
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.