From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id 761EC10E1A4 for ; Tue, 25 Jul 2023 04:18:12 +0000 (UTC) Date: Tue, 25 Jul 2023 06:18:07 +0200 From: Zbigniew =?utf-8?Q?Kempczy=C5=84ski?= To: Erik Kurzinger Message-ID: <20230725041807.f4cydvkattmc7sie@zkempczy-mobl2> References: <2aafc4e6-0ba7-5d78-4adb-0a0368f74792@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2aafc4e6-0ba7-5d78-4adb-0a0368f74792@nvidia.com> Subject: Re: [igt-dev] [PATCH] tests/syncobj_sync_file: new test List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Simon Ser , James Jones , igt-dev@lists.freedesktop.org, Austin Shafer Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: Hi Erik. +Petri, +Kamil Please verify I'm not wrong with the uapi bumping procedure below. On Mon, Jul 24, 2023 at 03:33:25PM -0700, Erik Kurzinger wrote: > This test suite exercises the new DRM_IOCTL_IMPORT/EXPORT_SYNC_FILE > ioctl introduced in [1]. > > [1] https://lore.kernel.org/dri-devel/5e687ad8-78ad-0350-6052-a698b278cc8c@nvidia.com/ > > Signed-off-by: Erik Kurzinger > --- > include/drm-uapi/drm.h | 11 ++ > lib/igt_syncobj.c | 24 ++++ > lib/igt_syncobj.h | 2 + > tests/meson.build | 1 + > tests/syncobj_sync_file.c | 256 ++++++++++++++++++++++++++++++++++++++ > 5 files changed, 294 insertions(+) > create mode 100644 tests/syncobj_sync_file.c > > diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h > index 5e54c3aa4..389dc138a 100644 > --- a/include/drm-uapi/drm.h > +++ b/include/drm-uapi/drm.h > @@ -878,6 +878,12 @@ struct drm_syncobj_transfer { > __u32 pad; > }; > > +struct drm_syncobj_sync_file { > + __u32 handle; > + __u32 fd; > + __u64 point; > +}; > + > #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) > #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1) > #define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE (1 << 2) /* wait for time point to become available */ > @@ -1090,8 +1096,13 @@ extern "C" { > #define DRM_IOCTL_SYNCOBJ_TRANSFER DRM_IOWR(0xCC, struct drm_syncobj_transfer) > #define DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL DRM_IOWR(0xCD, struct drm_syncobj_timeline_array) > > + > + > #define DRM_IOCTL_MODE_GETFB2 DRM_IOWR(0xCE, struct drm_mode_fb_cmd2) > > +#define DRM_IOCTL_SYNCOBJ_IMPORT_SYNC_FILE DRM_IOWR(0xD0, struct drm_syncobj_sync_file) > +#define DRM_IOCTL_SYNCOBJ_EXPORT_SYNC_FILE DRM_IOWR(0xD1, struct drm_syncobj_sync_file) > + > /* > * Device specific ioctls should only be in their respective headers > * The device specific ioctl range is from 0x40 to 0x9f. We allow changes in uapi headers which were previously accepted and merged in the community. This is chicken-and-egg problem but prevent us from exposing uapi which might be not accepted upstream. > diff --git a/lib/igt_syncobj.c b/lib/igt_syncobj.c > index a24ed10b7..ef0e297e3 100644 > --- a/lib/igt_syncobj.c > +++ b/lib/igt_syncobj.c > @@ -163,6 +163,18 @@ syncobj_fd_to_handle(int fd, int syncobj_fd, uint32_t flags) > return args.handle; > } > > +int > +__syncobj_import_sync_file(int fd, struct drm_syncobj_sync_file *args) > +{ > + int err = 0; > + if (igt_ioctl(fd, DRM_IOCTL_SYNCOBJ_IMPORT_SYNC_FILE, args)) { > + err = -errno; > + igt_assume(err); > + errno = 0; > + } > + return err; > +} > + > /** > * syncobj_import_sync_file: > * @fd: The DRM file descriptor > @@ -181,6 +193,18 @@ syncobj_import_sync_file(int fd, uint32_t handle, int sync_file) > igt_assert_eq(__syncobj_fd_to_handle(fd, &args), 0); > } > > +int > +__syncobj_export_sync_file(int fd, struct drm_syncobj_sync_file *args) > +{ > + int err = 0; > + if (igt_ioctl(fd, DRM_IOCTL_SYNCOBJ_EXPORT_SYNC_FILE, args)) { > + err = -errno; > + igt_assume(err); > + errno = 0; > + } > + return err; > +} > + > int > __syncobj_wait(int fd, struct drm_syncobj_wait *args) > { At the moment (before uapi will be accepted) I would add locally in the test: - drm.h definitions naming them LOCAL_DRM_IOCTL_... - two functions locally in the test. In this case names can stay, when uapi will be accepted and merged just move them to igt_syncobj.c/h (use inside LOCAL_DRM_IOCTL_... until official merge). After uapi merge we do verbatim copy of header from the kernel include directory and remove "localization" described above. I'm sorry for inconvinience but that's the rules. -- Zbigniew > diff --git a/lib/igt_syncobj.h b/lib/igt_syncobj.h > index e6725671d..01f5f062b 100644 > --- a/lib/igt_syncobj.h > +++ b/lib/igt_syncobj.h > @@ -34,7 +34,9 @@ int __syncobj_handle_to_fd(int fd, struct drm_syncobj_handle *args); > int __syncobj_fd_to_handle(int fd, struct drm_syncobj_handle *args); > int syncobj_handle_to_fd(int fd, uint32_t handle, uint32_t flags); > uint32_t syncobj_fd_to_handle(int fd, int syncobj_fd, uint32_t flags); > +int __syncobj_import_sync_file(int fd, struct drm_syncobj_sync_file *args); > void syncobj_import_sync_file(int fd, uint32_t handle, int sync_file); > +int __syncobj_export_sync_file(int fd, struct drm_syncobj_sync_file *args); > int __syncobj_wait(int fd, struct drm_syncobj_wait *args); > int syncobj_wait_err(int fd, uint32_t *handles, uint32_t count, > uint64_t abs_timeout_nsec, uint32_t flags); > diff --git a/tests/meson.build b/tests/meson.build > index 944a0941f..1712c31ca 100644 > --- a/tests/meson.build > +++ b/tests/meson.build > @@ -80,6 +80,7 @@ test_progs = [ > 'syncobj_basic', > 'syncobj_wait', > 'syncobj_timeline', > + 'syncobj_sync_file', > 'sw_sync', > 'template', > 'testdisplay', > diff --git a/tests/syncobj_sync_file.c b/tests/syncobj_sync_file.c > new file mode 100644 > index 000000000..6cbcc9dcd > --- /dev/null > +++ b/tests/syncobj_sync_file.c > @@ -0,0 +1,256 @@ > +/* > + * Copyright © 2023 NVIDIA > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + */ > + > +#include "igt.h" > +#include "igt_syncobj.h" > +#include "sw_sync.h" > +#include > +#include > +#include "drm.h" > + > +/** > + * TEST: syncobj sync file > + * Category: Infrastructure > + * Description: Tests for DRM syncobj sync file import and export > + * Feature: synchronization > + * Functionality: semaphore > + * Run type: FULL > + * Sub-category: DRM > + * Test category: GEM_Legacy > + * > + * SUBTEST: binary-import-export > + * Description: Verifies importing and exporting sync files with a binary syncobj. > + * > + * SUBTEST: timeline-import-export > + * Description: Verifies importing and exporting sync files with a timeline syncobj. > + * > + * SUBTEST: invalid-handle-import > + * Description: Verifies that importing a sync file to an invalid syncobj fails. > + * > + * SUBTEST: invalid-handle-export > + * Description: Verifies that exporting a sync file from an invalid syncobj fails. > + * > + * SUBTEST: invalid-fd-import > + * Description: Verifies that importing an invalid sync file fails. > + * > + * SUBTEST: unsubmitted-export > + * Description: Verifies that exporting a sync file for an unsubmitted point fails. > + * > + */ > + > +IGT_TEST_DESCRIPTION("Tests for DRM syncobj sync file import and export"); > + > +const char *test_binary_import_export_desc = > + "Verifies importing and exporting a sync file with a binary syncobj."; > +static void > +test_binary_import_export(int fd) > +{ > + struct drm_syncobj_sync_file args = { 0 }; > + int timeline = sw_sync_timeline_create(); > + > + args.handle = syncobj_create(fd, 0); > + args.fd = sw_sync_timeline_create_fence(timeline, 1); > + args.point = 0; > + igt_assert_eq(__syncobj_import_sync_file(fd, &args), 0); > + close(args.fd); > + args.fd = -1; > + igt_assert_eq(__syncobj_export_sync_file(fd, &args), 0); > + > + igt_assert(!syncobj_wait(fd, &args.handle, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(args.fd), 0); > + > + sw_sync_timeline_inc(timeline, 1); > + igt_assert(syncobj_wait(fd, &args.handle, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(args.fd), 1); > + > + close(args.fd); > + close(timeline); > + syncobj_destroy(fd, args.handle); > +} > + > +const char *test_timeline_import_export_desc = > + "Verifies importing and exporting sync files with a timeline syncobj."; > +static void > +test_timeline_import_export(int fd) > +{ > + struct drm_syncobj_sync_file args = { 0 }; > + int timeline = sw_sync_timeline_create(); > + int fence1, fence2; > + uint64_t point1 = 1, point2 = 2; > + > + args.handle = syncobj_create(fd, 0); > + args.fd = sw_sync_timeline_create_fence(timeline, 1); > + args.point = 1; > + igt_assert_eq(__syncobj_import_sync_file(fd, &args), 0); > + close(args.fd); > + args.fd = -1; > + igt_assert_eq(__syncobj_export_sync_file(fd, &args), 0); > + fence1 = args.fd; > + > + args.fd = sw_sync_timeline_create_fence(timeline, 2); > + args.point = 2; > + igt_assert_eq(__syncobj_import_sync_file(fd, &args), 0); > + close(args.fd); > + args.fd = -1; > + igt_assert_eq(__syncobj_export_sync_file(fd, &args), 0); > + fence2 = args.fd; > + > + igt_assert(!syncobj_timeline_wait(fd, &args.handle, &point1, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(fence1), 0); > + igt_assert(!syncobj_timeline_wait(fd, &args.handle, &point2, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(fence2), 0); > + > + sw_sync_timeline_inc(timeline, 1); > + igt_assert(syncobj_timeline_wait(fd, &args.handle, &point1, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(fence1), 1); > + igt_assert(!syncobj_timeline_wait(fd, &args.handle, &point2, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(fence2), 0); > + > + sw_sync_timeline_inc(timeline, 1); > + igt_assert(syncobj_timeline_wait(fd, &args.handle, &point1, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(fence1), 1); > + igt_assert(syncobj_timeline_wait(fd, &args.handle, &point2, 1, 0, 0, NULL)); > + igt_assert_eq(sync_fence_status(fence2), 1); > + > + close(fence1); > + close(fence2); > + close(timeline); > + syncobj_destroy(fd, args.handle); > +} > + > +const char *test_invalid_handle_import_desc = > + "Verifies that importing a sync file to an invalid syncobj fails."; > +static void > +test_invalid_handle_import(int fd) > +{ > + struct drm_syncobj_sync_file args = { 0 }; > + int timeline = sw_sync_timeline_create(); > + > + args.handle = 0; > + args.point = 0; > + args.fd = sw_sync_timeline_create_fence(timeline, 1); > + igt_assert_eq(__syncobj_import_sync_file(fd, &args), -EINVAL); > + > + close(args.fd); > + close(timeline); > +} > + > +const char *test_invalid_handle_export_desc = > + "Verifies that exporting a sync file from an invalid syncobj fails."; > +static void > +test_invalid_handle_export(int fd) > +{ > + struct drm_syncobj_sync_file args = { 0 }; > + > + args.handle = 0; > + args.point = 0; > + args.fd = -1; > + igt_assert_eq(__syncobj_export_sync_file(fd, &args), -EINVAL); > +} > + > +const char *test_invalid_fd_import_desc = > + "Verifies that importing an invalid sync file fails."; > +static void > +test_invalid_fd_import(int fd) > +{ > + struct drm_syncobj_sync_file args = { 0 }; > + > + args.handle = syncobj_create(fd, 0); > + args.point = 0; > + args.fd = -1; > + igt_assert_eq(__syncobj_import_sync_file(fd, &args), -EINVAL); > + > + syncobj_destroy(fd, args.handle); > +} > + > +const char *test_unsubmitted_export_desc = > + "Verifies that exporting a sync file for an unsubmitted point fails."; > +static void > +test_unsubmitted_export(int fd) > +{ > + struct drm_syncobj_sync_file args = { 0 }; > + > + args.handle = syncobj_create(fd, 0); > + args.point = 0; > + args.fd = -1; > + igt_assert_eq(__syncobj_export_sync_file(fd, &args), -EINVAL); > + > + syncobj_destroy(fd, args.handle); > +} > + > +static bool has_syncobj_timeline(int fd) > +{ > + uint64_t value; > + return drmGetCap(fd, DRM_CAP_SYNCOBJ_TIMELINE, > + &value) == 0 && value; > +} > + > +static bool has_syncobj_sync_file_import_export(int fd) > +{ > + struct drm_syncobj_sync_file args = { 0 }; > + args.handle = 0xffffffff; > + /* if sync file import/export is supported this will fail with ENOENT, > + * otherwise it will fail with EINVAL */ > + return __syncobj_export_sync_file(fd, &args) == -ENOENT; > +} > + > +igt_main > +{ > + int fd = -1; > + > + igt_fixture { > + fd = drm_open_driver(DRIVER_ANY); > + igt_require(has_syncobj_timeline(fd)); > + igt_require(has_syncobj_sync_file_import_export(fd)); > + igt_require_sw_sync(); > + } > + > + igt_describe(test_binary_import_export_desc); > + igt_subtest("binary-import-export") > + test_binary_import_export(fd); > + > + igt_describe(test_timeline_import_export_desc); > + igt_subtest("timeline-import-export") > + test_timeline_import_export(fd); > + > + igt_describe(test_invalid_handle_import_desc); > + igt_subtest("invalid-handle-import") > + test_invalid_handle_import(fd); > + > + igt_describe(test_invalid_handle_export_desc); > + igt_subtest("invalid-handle-export") > + test_invalid_handle_export(fd); > + > + igt_describe(test_invalid_fd_import_desc); > + igt_subtest("invalid-fd-import") > + test_invalid_fd_import(fd); > + > + igt_describe(test_unsubmitted_export_desc); > + igt_subtest("unsubmitted-export") > + test_unsubmitted_export(fd); > + > + igt_fixture { > + drm_close_driver(fd); > + } > + > +} > -- > 2.41.0 > >