On Fri, Jun 16, 2017 at 11:59:31AM -0700, Eric Anholt wrote: > Rafael Antognolli <[email protected]> writes: > > > Add a small library that helps manipulating software fences. They are > > useful for testing EGL Android fences, since the latter can be created > > out of them. > > > > Signed-off-by: Rafael Antognolli <[email protected]> > > --- > > .../spec/egl_android_native_fence_sync/sw_sync.c | 211 > > +++++++++++++++++++++ > > .../spec/egl_android_native_fence_sync/sw_sync.h | 50 +++++ > > 2 files changed, 261 insertions(+) > > create mode 100644 tests/egl/spec/egl_android_native_fence_sync/sw_sync.c > > create mode 100644 tests/egl/spec/egl_android_native_fence_sync/sw_sync.h > > > > diff --git a/tests/egl/spec/egl_android_native_fence_sync/sw_sync.c > > b/tests/egl/spec/egl_android_native_fence_sync/sw_sync.c > > new file mode 100644 > > index 0000000..4e8117f > > --- /dev/null > > +++ b/tests/egl/spec/egl_android_native_fence_sync/sw_sync.c > > @@ -0,0 +1,211 @@ > > +/* > > + * 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 <[email protected]> > > + */ > > + > > +#include <fcntl.h> > > +#include <poll.h> > > +#include <stdint.h> > > +#include <stdlib.h> > > +#include <unistd.h> > > +#include <linux/sync_file.h> > > +#include <sys/ioctl.h> > > + > > +#include "sw_sync.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" > > + > > +bool sw_sync_is_supported(void) > > +{ > > + if(access(DEVFS_SW_SYNC, R_OK | W_OK) != -1) { > > + return true; > > space after "if" > > > + } else if (access(DEBUGFS_SW_SYNC, R_OK | W_OK) != -1 ) { > > + return true; > > + } > > + > > + return false; > > +} > > + > > +int sw_sync_fd_is_valid(int fd) > > +{ > > + int status; > > + > > + if (fd < 0) > > + return 0; > > + > > + status = fcntl(fd, F_GETFD, 0); > > + return status >= 0; > > +} > > Do we need to F_GETFD? Couldn't all callers of this just "if (fd < 0)"? > > > + > > +static > > +void sw_sync_fd_close(int fd) > > +{ > > + if (!sw_sync_fd_is_valid(fd)) > > + return; > > + > > + close(fd); > > Is there a reason to call is_valid() before close()? I think you could > drop this entire function and just close(fd) from > sw_sync_timeline_destroy() and fence_destroy(). > > > +} > > + > > +int sw_sync_timeline_create(void) > > +{ > > + int fd = open(DEVFS_SW_SYNC, O_RDWR); > > + > > + if (!sw_sync_fd_is_valid(fd)) > > + fd = open(DEBUGFS_SW_SYNC, O_RDWR); > > + > > + return fd; > > +} > > + > > +void sw_sync_timeline_destroy(int fd) > > +{ > > + return sw_sync_fd_close(fd); > > +} > > + > > +void sw_sync_fence_destroy(int fd) > > +{ > > + return sw_sync_fd_close(fd); > > +} > > + > > +int sw_sync_fence_create(int fd, int32_t seqno) > > +{ > > + struct sw_sync_create_fence_data data = {}; > > + data.value = seqno; > > + > > + if (fd >= 0) { > > + ioctl(fd, SW_SYNC_IOC_CREATE_FENCE, &data); > > + return data.fence; > > + } else { > > + ioctl(fd, SW_SYNC_IOC_CREATE_FENCE, &data); > > + return -1; > > + } > > Calling an ioctl on a negative fd? Shouldn't you just early return -1 > with no ioctl call? > > > +static struct sync_file_info *sync_file_info(int fd) > > +{ > > + struct sync_file_info *info; > > + struct sync_fence_info *fence_info; > > + int err, num_fences; > > + > > + info = calloc(1, sizeof(*info)); > > + if (info == NULL) > > + return NULL; > > + > > + err = ioctl(fd, SYNC_IOC_FILE_INFO, info); > > + if (err < 0) { > > + free(info); > > + return NULL; > > + } > > + > > + num_fences = info->num_fences; > > + > > + if (num_fences) { > > + info->flags = 0; > > + info->num_fences = num_fences; > > + > > + fence_info = calloc(num_fences, sizeof(*fence_info)); > > + if (!fence_info) { > > + free(info); > > + return NULL; > > + } > > + > > + info->sync_fence_info = (uint64_t)(unsigned long) (fence_info); > > + > > + err = ioctl(fd, SYNC_IOC_FILE_INFO, info); > > + if (err < 0) { > > + free(fence_info); > > + free(info); > > + return NULL; > > + } > > + } > > + > > + return info; > > +} > > + > > +static void sync_file_info_free(struct sync_file_info *info) > > +{ > > + free((void *)(uintptr_t)info->sync_fence_info); > > + free(info); > > +} > > + > > +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); > > + > > + return count; > > +} > > + > > +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; > > +} > > These functions seem to be unused. Do we expect them to get used, or > should we drop them?
Yeah, I'll remove the unused ones. _______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
