On Mon, Sep 12, 2016 at 06:08:30PM -0400, robert.foss@xxxxxxxxxxxxx wrote: > From: Robert Foss <robert.foss@xxxxxxxxxxxxx> > > Base functions to help testing the Sync File Framework (explicit fencing > mechanism ported from Android). > These functions allow you to create, use and destroy timelines and fences. > > Signed-off-by: Robert Foss <robert.foss@xxxxxxxxxxxxx> > Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx> > Reviewed-by: Eric Engestrom <eric@xxxxxxxxxxxx> > --- > lib/Makefile.sources | 2 + > lib/sw_sync.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++++++ > lib/sw_sync.h | 49 +++++++++++ > 3 files changed, 288 insertions(+) > create mode 100644 lib/sw_sync.c > create mode 100644 lib/sw_sync.h > > diff --git a/lib/Makefile.sources b/lib/Makefile.sources > index bac9a7f..3dc7c3c 100644 > --- a/lib/Makefile.sources > +++ b/lib/Makefile.sources > @@ -61,6 +61,8 @@ lib_source_list = \ > rendercopy_gen8.c \ > rendercopy_gen9.c \ > rendercopy.h \ > + sw_sync.c \ > + sw_sync.h \ > intel_reg_map.c \ > intel_iosf.c \ > igt_kms.c \ > diff --git a/lib/sw_sync.c b/lib/sw_sync.c > new file mode 100644 > index 0000000..179a473 > --- /dev/null > +++ b/lib/sw_sync.c > @@ -0,0 +1,237 @@ > +/* > + * Copyright 2012 Google, Inc > + * Copyright © 2016 Collabora, Ltd. > + * > + * Based on the implementation from the Android Open Source Project > + * > + * 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. > + * > + * Authors: > + * Robert Foss <robert.foss@xxxxxxxxxxxxx> > + */ > + > +#ifndef ANDROID > +#define _GNU_SOURCE > +#else > +#include <libgen.h> > +#endif > +#include <fcntl.h> > +#include <poll.h> > +#include <stdint.h> > +#include <linux/sync_file.h> > +#include <sys/ioctl.h> > + > +#include "sw_sync.h" > +#include "drmtest.h" > +#include "ioctl_wrappers.h" > + > +#ifndef SW_SYNC_IOC_INC > +struct sw_sync_create_fence_data { > + __u32 value; > + char name[32]; > + __s32 fence; > +}; > + > +#define SW_SYNC_IOC_MAGIC 'W' > +#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\ > + struct sw_sync_create_fence_data) > +#define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32) > +#endif > + > +#define DEVFS_SW_SYNC "/dev/sw_sync" > +#define DEBUGFS_SW_SYNC "/sys/kernel/debug/sync/sw_sync" > + > + > +int sw_sync_fd_is_valid(int fd) > +{ > + int status; > + > + if (fd < 0) > + return 0; > + > + status = fcntl(fd, F_GETFD, 0); > + return status >= 0; > +} > + > +static > +void sw_sync_fd_close(int fd) > +{ > + if (!sw_sync_fd_is_valid(fd)) > + return; > + > + close(fd); What bug are you trying to prevent? This is the same as just calling close(fd) and seeing EINVAL. > +} > + > +int sw_sync_timeline_create(void) > +{ > + int fd = open(DEVFS_SW_SYNC, O_RDWR); if (fd < 0) fd = open(DEBUGFS_SW_SYNC, O_RDWR); igt_require(fd >= 0); > + > + fd = open(DEBUGFS_SW_SYNC, O_RDWR); > + > + igt_assert(sw_sync_fd_is_valid(fd)); > + > + return fd; > +} > + > +void sw_sync_timeline_destroy(int fd) > +{ > + return sw_sync_fd_close(fd); Now just close(fd); and might as well not obfuscate that these are plain fd. > +int sw_sync_fence_create(int fd, int32_t seqno) > +{ > + struct sw_sync_create_fence_data data = {}; > + data.value = seqno; > + > + if (fd >= 0) { > + do_ioctl(fd, SW_SYNC_IOC_CREATE_FENCE, &data); > + return data.fence; > + } else { > + do_ioctl_err(fd, SW_SYNC_IOC_CREATE_FENCE, &data, EBADF); > + return -1; Try: int __sw_sync_fence_create(int fd, int32_t seqno) /* int32_t not unsigned ? */ { struct sw_sync_create_fence_data data; memset(&data, 0, sizeof(data)); data.value = seqno; if (igt_ioctl(fd, SW_SYNC_IOCT_CREATE_FENCE, &data)) return -errno; return data.fence; } int sw_sync_fence_create(int fd, int32_t seqno) { int fence = __sw_sync_fence_create(fd, seqno); igt_assert(fence >= 0); return fence; } Then only in the test code do you send garbage and check for the expected errno. > + } > +} > + > +void sw_sync_timeline_inc(int fd, uint32_t count) > +{ > + uint32_t arg = count; > + > + if (fd == 0 || fd == -1) > + return; What's wrong with fd == 0? See above. By default assert that we don't fail, then have a function for negative tests. > + > + do_ioctl(fd, SW_SYNC_IOC_INC, &arg); Try not to use do_ioctl(), it generates horrible messages from the macro expansions of the ioctl numbers. > +} > + > +int sw_sync_merge(int fd1, int fd2) > +{ > + struct sync_merge_data data = {}; > + int err; > + > + data.fd2 = fd2; > + > + err = ioctl(fd1, SYNC_IOC_MERGE, &data); > + if (err < 0) > + return err; Our convention is to return -errno. > + > + sw_sync_fd_is_valid(data.fence); > + > + return data.fence; > +} > + > +int sw_sync_wait(int fence, int timeout) > +{ > + struct pollfd fds; > + int ret; > + > + fds.fd = fence; > + fds.events = POLLIN | POLLERR; > + > + ret = poll(&fds, 1, timeout); > + > + return ret; > +} > + > +int sw_sync_fence_size(int fd) > +{ > + int count; > + struct sync_file_info *info = sync_file_info(fd); > + > + if (!info) > + return 0; > + > + count = info->num_fences; > + > + sync_file_info_free(info); Querying all those fences seems like a waste of time now. sw_sync_fence_count not size. struct sync_file_info info; memset(&info, 0, sizeof(info)); if (ioctl(fd, SYNC_IOC_FILE_INFO, &info)) return -errno; return info.num_fences; > +int sw_sync_fence_count_status(int fd, int status) > +{ > + int i, count = 0; > + struct sync_fence_info *fence_info = NULL; > + struct sync_file_info *info = sync_file_info(fd); > + > + if (!info) > + return -1; > + > + fence_info = (struct sync_fence_info *)(uintptr_t)info->sync_fence_info; > + for (i = 0 ; i < info->num_fences ; i++) { > + if (fence_info[i].status == status) > + count++; > + } > + > + sync_file_info_free(info); > + > + return count; > +} int __sw_sync_fence_count_status(int fd, int status) { struct sync_file_info info; struct sync_fence_info *fence_info; int count; memset(&info, 0, sizeof(info)); if (ioctl(fd, SYNC_IOC_FILE_INFO, &info)) return -errno; fence_info = calloc(info.num_fences, sizeof(*fence_info)); if (!fence_info) return -ENOMEM; info.fence_info = (uintptr_t)fence_info; if (ioctl(fd, SYNC_IOC_FILE_INFO, &info)) { count = -errno; } else { count = 0; for (i = 0 ; i < info.num_fences ; i++) count += fence_info[i].status == status; } free(fence_info); return count; } int sw_sync_fence_count_status(int fd, int status) { int count = __sw_sync_fence_count_status(fd, status); igt_assert(count >= 0); return count; } So that regular callers don't have to worry about errors. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx