As proposed yesterday, here's the Android sync driver patches for staging.
I've preserved the commit history, but moved all the changes over to be against the staging directory (instead of drivers/base).
The goal of submitting this driver to staging is to try to get more collaberation, as there are some similar efforts going on in the community with dmabuf-fences. My email from yesterday with more details for how I hope this goes is here: http://comments.gmane.org/gmane.linux.kernel/1448420
Erik also provided a nice background on the patch set in his reply yesterday, which I'll quote here:
"In Honeycomb where we introduced the Hardware Composer HAL. This is a userspace layer that allows composition acceleration on a per platform basis. Different SoC vendors have implemented this using overlays, 2d blitters, a combinations of both, or other clever/disgusting means. Along with the HWC we consolidated a lot of our camera and media pipeline to allow their input to be fed into the GPU or display(overlay.) In order to exploit parallelism the the graphics pipeline, this introduced lots of implicit synchronization dependancies. After a couple years of working with many different SoC vendors, we found that it was really difficult to communicate our system's expectations of the implicit contract and it was difficult for the SoC vendors to properly implement the implicit contract in each of their IP blocks (display, gpu, camera, video codecs). It was also incredibly difficult to debug when problems/deadlocks arose.
In an effort to clean up the situation we decided to create set of simple synchronization primitives and have our compositor (SurfaceFlinger) manage the synchronization contract explicitly. We designed these primitives so that they can be passed across processes (much like ion/dma_buf handles), can be backed by hardware synchronization primitives, and can be combined with other sync dependancies in a heterogeneous manner. We also added enough debugging information to make pinpointing a synchronization deadlock bug easier. There are also OpenGL extensions added (which I believe have been ratified by Khronos) to convert a "native" sync object to a gl fence object and vise versa.
So far shipped this system on two products (the Nexus 10 and 4) with two different SoCs (Samsung Exynos5250 and Qualcomm MSM8064.) These two projects were much easier to work out the kinks in the graphics/compositing pipelines. In addition we were able to use the telemetry and tracing features to track down the causes of dropped frames aka "jank."
As for the implementation, I started with having the main driver op primitive be a wait() op. I quickly noticed that most of the tricky race condition prone code was ending up in the drivers wait() op. It also made handling asynchronous waits of more than one type of sync_pt difficult to manage. In the end I opted for something roughly like poll() where all the heavy lifting is done at the high level and the drivers only need to implement a simple check function."
Anyway, let me know what you think of the patches, and hopefully this is something that could be considered for staging for 3.10
thanks -john
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Cc: Android Kernel Team kernel-team@android.com
Erik Gilling (26): staging: sync: Add synchronization framework staging: sw_sync: Add cpu based sync driver staging: sync: Add timestamps to sync_pts staging: sync: Add debugfs support staging: sw_sync: Add debug support staging: sync: Add ioctl to get fence data staging: sw_sync: Add fill_driver_data support staging: sync: Add poll support staging: sync: Allow async waits to be canceled staging: sync: Export sync API symbols staging: sw_sync: Export sw_sync API staging: sync: Reorder sync_fence_release staging: sync: Optimize fence merges staging: sync: Add internal refcounting to fences staging: sync: Add reference counting to timelines staging: sync: Change wait timeout to mirror poll semantics staging: sync: Dump sync state to console on timeout staging: sync: Improve timeout dump messages staging: sync: Dump sync state on fence errors staging: sync: Protect unlocked access to fence status staging: sync: Update new fence status with sync_fence_signal_pt staging: sync: Use proper barriers when waiting indefinitely staging: sync: Refactor sync debug printing staging: sw_sync: Convert to use new value_str debug ops staging: sync: Add tracepoint support staging: sync: Don't log wait timeouts when timeout = 0
Jamie Gennis (1): staging: sync: Fix timeout = 0 wait behavior
Rebecca Schultz Zavin (2): staging: sync: Fix error paths staging: sw_sync: Fix error paths
Ørjan Eide (1): staging: sync: Fix race condition between merge and signal
drivers/staging/android/Kconfig | 27 + drivers/staging/android/Makefile | 2 + drivers/staging/android/sw_sync.c | 263 +++++++++ drivers/staging/android/sw_sync.h | 58 ++ drivers/staging/android/sync.c | 1016 ++++++++++++++++++++++++++++++++++ drivers/staging/android/sync.h | 426 ++++++++++++++ drivers/staging/android/trace/sync.h | 82 +++ 7 files changed, 1874 insertions(+) create mode 100644 drivers/staging/android/sw_sync.c create mode 100644 drivers/staging/android/sw_sync.h create mode 100644 drivers/staging/android/sync.c create mode 100644 drivers/staging/android/sync.h create mode 100644 drivers/staging/android/trace/sync.h
From: Erik Gilling konkers@android.com
Sync is a framework for synchronization between multiple drivers. Sync implementations can take advantage of hardware synchronization built into devices like GPUs.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Added commit message, moved to staging, squished minor fix in] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/Kconfig | 9 + drivers/staging/android/Makefile | 1 + drivers/staging/android/sync.c | 525 ++++++++++++++++++++++++++++++++++++++ drivers/staging/android/sync.h | 314 +++++++++++++++++++++++ 4 files changed, 849 insertions(+) create mode 100644 drivers/staging/android/sync.c create mode 100644 drivers/staging/android/sync.h
diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig index 465a28c..c95aede 100644 --- a/drivers/staging/android/Kconfig +++ b/drivers/staging/android/Kconfig @@ -72,6 +72,15 @@ config ANDROID_INTF_ALARM_DEV elapsed realtime, and a non-wakeup alarm on the monotonic clock. Also exports the alarm interface to user-space.
+config SYNC + bool "Synchronization framework" + default n + select ANON_INODES + help + This option enables the framework for synchronization between multiple + drivers. Sync implementations can take advantage of hardware + synchronization built into devices like GPUs. + endif # if ANDROID
endmenu diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index b35a631..22c656c 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_ANDROID_TIMED_OUTPUT) += timed_output.o obj-$(CONFIG_ANDROID_TIMED_GPIO) += timed_gpio.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER) += lowmemorykiller.o obj-$(CONFIG_ANDROID_INTF_ALARM_DEV) += alarm-dev.o +obj-$(CONFIG_SYNC) += sync.o diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c new file mode 100644 index 0000000..4a9e63d --- /dev/null +++ b/drivers/staging/android/sync.c @@ -0,0 +1,525 @@ +/* + * drivers/base/sync.c + * + * Copyright (C) 2012 Google, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/kernel.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/uaccess.h> +#include <linux/anon_inodes.h> + +#include "sync.h" + +static void sync_fence_signal_pt(struct sync_pt *pt); +static int _sync_pt_has_signaled(struct sync_pt *pt); + +struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, + int size, const char *name) +{ + struct sync_timeline *obj; + + if (size < sizeof(struct sync_timeline)) + return NULL; + + obj = kzalloc(size, GFP_KERNEL); + if (obj == NULL) + return NULL; + + obj->ops = ops; + strlcpy(obj->name, name, sizeof(obj->name)); + + INIT_LIST_HEAD(&obj->child_list_head); + spin_lock_init(&obj->child_list_lock); + + INIT_LIST_HEAD(&obj->active_list_head); + spin_lock_init(&obj->active_list_lock); + + return obj; +} + +void sync_timeline_destroy(struct sync_timeline *obj) +{ + unsigned long flags; + bool needs_freeing; + + spin_lock_irqsave(&obj->child_list_lock, flags); + obj->destroyed = true; + needs_freeing = list_empty(&obj->child_list_head); + spin_unlock_irqrestore(&obj->child_list_lock, flags); + + if (needs_freeing) + kfree(obj); + else + sync_timeline_signal(obj); +} + +static void sync_timeline_add_pt(struct sync_timeline *obj, struct sync_pt *pt) +{ + unsigned long flags; + + pt->parent = obj; + + spin_lock_irqsave(&obj->child_list_lock, flags); + list_add_tail(&pt->child_list, &obj->child_list_head); + spin_unlock_irqrestore(&obj->child_list_lock, flags); +} + +static void sync_timeline_remove_pt(struct sync_pt *pt) +{ + struct sync_timeline *obj = pt->parent; + unsigned long flags; + bool needs_freeing; + + spin_lock_irqsave(&obj->active_list_lock, flags); + if (!list_empty(&pt->active_list)) + list_del_init(&pt->active_list); + spin_unlock_irqrestore(&obj->active_list_lock, flags); + + spin_lock_irqsave(&obj->child_list_lock, flags); + list_del(&pt->child_list); + needs_freeing = obj->destroyed && list_empty(&obj->child_list_head); + spin_unlock_irqrestore(&obj->child_list_lock, flags); + + if (needs_freeing) + kfree(obj); +} + +void sync_timeline_signal(struct sync_timeline *obj) +{ + unsigned long flags; + LIST_HEAD(signaled_pts); + struct list_head *pos, *n; + + spin_lock_irqsave(&obj->active_list_lock, flags); + + list_for_each_safe(pos, n, &obj->active_list_head) { + struct sync_pt *pt = + container_of(pos, struct sync_pt, active_list); + + if (_sync_pt_has_signaled(pt)) + list_move(pos, &signaled_pts); + } + + spin_unlock_irqrestore(&obj->active_list_lock, flags); + + list_for_each_safe(pos, n, &signaled_pts) { + struct sync_pt *pt = + container_of(pos, struct sync_pt, active_list); + + list_del_init(pos); + sync_fence_signal_pt(pt); + } +} + +struct sync_pt *sync_pt_create(struct sync_timeline *parent, int size) +{ + struct sync_pt *pt; + + if (size < sizeof(struct sync_pt)) + return NULL; + + pt = kzalloc(size, GFP_KERNEL); + if (pt == NULL) + return NULL; + + INIT_LIST_HEAD(&pt->active_list); + sync_timeline_add_pt(parent, pt); + + return pt; +} + +void sync_pt_free(struct sync_pt *pt) +{ + if (pt->parent->ops->free_pt) + pt->parent->ops->free_pt(pt); + + sync_timeline_remove_pt(pt); + + kfree(pt); +} + +/* call with pt->parent->active_list_lock held */ +static int _sync_pt_has_signaled(struct sync_pt *pt) +{ + if (!pt->status) + pt->status = pt->parent->ops->has_signaled(pt); + + if (!pt->status && pt->parent->destroyed) + pt->status = -ENOENT; + + return pt->status; +} + +static struct sync_pt *sync_pt_dup(struct sync_pt *pt) +{ + return pt->parent->ops->dup(pt); +} + +/* Adds a sync pt to the active queue. Called when added to a fence */ +static void sync_pt_activate(struct sync_pt *pt) +{ + struct sync_timeline *obj = pt->parent; + unsigned long flags; + int err; + + spin_lock_irqsave(&obj->active_list_lock, flags); + + err = _sync_pt_has_signaled(pt); + if (err != 0) + goto out; + + list_add_tail(&pt->active_list, &obj->active_list_head); + +out: + spin_unlock_irqrestore(&obj->active_list_lock, flags); +} + +static int sync_fence_release(struct inode *inode, struct file *file); +static long sync_fence_ioctl(struct file *file, unsigned int cmd, + unsigned long arg); + + +static const struct file_operations sync_fence_fops = { + .release = sync_fence_release, + .unlocked_ioctl = sync_fence_ioctl, +}; + +static struct sync_fence *sync_fence_alloc(const char *name) +{ + struct sync_fence *fence; + + fence = kzalloc(sizeof(struct sync_fence), GFP_KERNEL); + if (fence == NULL) + return NULL; + + fence->file = anon_inode_getfile("sync_fence", &sync_fence_fops, + fence, 0); + if (fence->file == NULL) + goto err; + + strlcpy(fence->name, name, sizeof(fence->name)); + + INIT_LIST_HEAD(&fence->pt_list_head); + INIT_LIST_HEAD(&fence->waiter_list_head); + spin_lock_init(&fence->waiter_list_lock); + + init_waitqueue_head(&fence->wq); + return fence; + +err: + kfree(fence); + return NULL; +} + +/* TODO: implement a create which takes more that one sync_pt */ +struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt) +{ + struct sync_fence *fence; + + if (pt->fence) + return NULL; + + fence = sync_fence_alloc(name); + if (fence == NULL) + return NULL; + + pt->fence = fence; + list_add(&pt->pt_list, &fence->pt_list_head); + sync_pt_activate(pt); + + return fence; +} + +static int sync_fence_copy_pts(struct sync_fence *dst, struct sync_fence *src) +{ + struct list_head *pos; + + list_for_each(pos, &src->pt_list_head) { + struct sync_pt *orig_pt = + container_of(pos, struct sync_pt, pt_list); + struct sync_pt *new_pt = sync_pt_dup(orig_pt); + + if (new_pt == NULL) + return -ENOMEM; + + new_pt->fence = dst; + list_add(&new_pt->pt_list, &dst->pt_list_head); + sync_pt_activate(new_pt); + } + + return 0; +} + +static void sync_fence_free_pts(struct sync_fence *fence) +{ + struct list_head *pos, *n; + + list_for_each_safe(pos, n, &fence->pt_list_head) { + struct sync_pt *pt = container_of(pos, struct sync_pt, pt_list); + sync_pt_free(pt); + } +} + +struct sync_fence *sync_fence_fdget(int fd) +{ + struct file *file = fget(fd); + + if (file == NULL) + return NULL; + + if (file->f_op != &sync_fence_fops) + goto err; + + return file->private_data; + +err: + fput(file); + return NULL; +} + +void sync_fence_put(struct sync_fence *fence) +{ + fput(fence->file); +} + +void sync_fence_install(struct sync_fence *fence, int fd) +{ + fd_install(fd, fence->file); +} + +static int sync_fence_get_status(struct sync_fence *fence) +{ + struct list_head *pos; + int status = 1; + + list_for_each(pos, &fence->pt_list_head) { + struct sync_pt *pt = container_of(pos, struct sync_pt, pt_list); + int pt_status = pt->status; + + if (pt_status < 0) { + status = pt_status; + break; + } else if (status == 1) { + status = pt_status; + } + } + + return status; +} + +struct sync_fence *sync_fence_merge(const char *name, + struct sync_fence *a, struct sync_fence *b) +{ + struct sync_fence *fence; + int err; + + fence = sync_fence_alloc(name); + if (fence == NULL) + return NULL; + + err = sync_fence_copy_pts(fence, a); + if (err < 0) + goto err; + + err = sync_fence_copy_pts(fence, b); + if (err < 0) + goto err; + + fence->status = sync_fence_get_status(fence); + + return fence; +err: + sync_fence_free_pts(fence); + kfree(fence); + return NULL; +} + +static void sync_fence_signal_pt(struct sync_pt *pt) +{ + LIST_HEAD(signaled_waiters); + struct sync_fence *fence = pt->fence; + struct list_head *pos; + struct list_head *n; + unsigned long flags; + int status; + + status = sync_fence_get_status(fence); + + spin_lock_irqsave(&fence->waiter_list_lock, flags); + /* + * this should protect against two threads racing on the signaled + * false -> true transition + */ + if (status && !fence->status) { + list_for_each_safe(pos, n, &fence->waiter_list_head) + list_move(pos, &signaled_waiters); + + fence->status = status; + } else { + status = 0; + } + spin_unlock_irqrestore(&fence->waiter_list_lock, flags); + + if (status) { + list_for_each_safe(pos, n, &signaled_waiters) { + struct sync_fence_waiter *waiter = + container_of(pos, struct sync_fence_waiter, + waiter_list); + + waiter->callback(fence, waiter->callback_data); + list_del(pos); + kfree(waiter); + } + wake_up(&fence->wq); + } +} + +int sync_fence_wait_async(struct sync_fence *fence, + void (*callback)(struct sync_fence *, void *data), + void *callback_data) +{ + struct sync_fence_waiter *waiter; + unsigned long flags; + int err = 0; + + waiter = kzalloc(sizeof(struct sync_fence_waiter), GFP_KERNEL); + if (waiter == NULL) + return -ENOMEM; + + waiter->callback = callback; + waiter->callback_data = callback_data; + + spin_lock_irqsave(&fence->waiter_list_lock, flags); + + if (fence->status) { + kfree(waiter); + err = fence->status; + goto out; + } + + list_add_tail(&waiter->waiter_list, &fence->waiter_list_head); +out: + spin_unlock_irqrestore(&fence->waiter_list_lock, flags); + + return err; +} + +int sync_fence_wait(struct sync_fence *fence, long timeout) +{ + int err; + + if (timeout) { + timeout = msecs_to_jiffies(timeout); + err = wait_event_interruptible_timeout(fence->wq, + fence->status != 0, + timeout); + } else { + err = wait_event_interruptible(fence->wq, fence->status != 0); + } + + if (err < 0) + return err; + + if (fence->status < 0) + return fence->status; + + if (fence->status == 0) + return -ETIME; + + return 0; +} + +static int sync_fence_release(struct inode *inode, struct file *file) +{ + struct sync_fence *fence = file->private_data; + + sync_fence_free_pts(fence); + kfree(fence); + + return 0; +} + +static long sync_fence_ioctl_wait(struct sync_fence *fence, unsigned long arg) +{ + __s32 value; + + if (copy_from_user(&value, (void __user *)arg, sizeof(value))) + return -EFAULT; + + return sync_fence_wait(fence, value); +} + +static long sync_fence_ioctl_merge(struct sync_fence *fence, unsigned long arg) +{ + int fd = get_unused_fd(); + int err; + struct sync_fence *fence2, *fence3; + struct sync_merge_data data; + + if (copy_from_user(&data, (void __user *)arg, sizeof(data))) + return -EFAULT; + + fence2 = sync_fence_fdget(data.fd2); + if (fence2 == NULL) { + err = -ENOENT; + goto err_put_fd; + } + + data.name[sizeof(data.name) - 1] = '\0'; + fence3 = sync_fence_merge(data.name, fence, fence2); + if (fence3 == NULL) { + err = -ENOMEM; + goto err_put_fence2; + } + + data.fence = fd; + if (copy_to_user((void __user *)arg, &data, sizeof(data))) { + err = -EFAULT; + goto err_put_fence3; + } + + sync_fence_install(fence3, fd); + sync_fence_put(fence2); + return 0; + +err_put_fence3: + sync_fence_put(fence3); + +err_put_fence2: + sync_fence_put(fence2); + +err_put_fd: + put_unused_fd(fd); + return err; +} + + +static long sync_fence_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct sync_fence *fence = file->private_data; + switch (cmd) { + case SYNC_IOC_WAIT: + return sync_fence_ioctl_wait(fence, arg); + + case SYNC_IOC_MERGE: + return sync_fence_ioctl_merge(fence, arg); + default: + return -ENOTTY; + } +} + diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h new file mode 100644 index 0000000..388acd1 --- /dev/null +++ b/drivers/staging/android/sync.h @@ -0,0 +1,314 @@ +/* + * include/linux/sync.h + * + * Copyright (C) 2012 Google, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#ifndef _LINUX_SYNC_H +#define _LINUX_SYNC_H + +#include <linux/types.h> +#ifdef __KERNEL__ + +#include <linux/list.h> +#include <linux/spinlock.h> +#include <linux/wait.h> + +struct sync_timeline; +struct sync_pt; +struct sync_fence; + +/** + * struct sync_timeline_ops - sync object implementation ops + * @driver_name: name of the implentation + * @dup: duplicate a sync_pt + * @has_signaled: returns: + * 1 if pt has signaled + * 0 if pt has not signaled + * <0 on error + * @compare: returns: + * 1 if b will signal before a + * 0 if a and b will signal at the same time + * -1 if a will signabl before b + * @free_pt: called before sync_pt is freed + * @release_obj: called before sync_timeline is freed + */ +struct sync_timeline_ops { + const char *driver_name; + + /* required */ + struct sync_pt *(*dup)(struct sync_pt *pt); + + /* required */ + int (*has_signaled)(struct sync_pt *pt); + + /* required */ + int (*compare)(struct sync_pt *a, struct sync_pt *b); + + /* optional */ + void (*free_pt)(struct sync_pt *sync_pt); + + /* optional */ + void (*release_obj)(struct sync_timeline *sync_timeline); +}; + +/** + * struct sync_timeline - sync object + * @ops: ops that define the implementaiton of the sync_timeline + * @name: name of the sync_timeline. Useful for debugging + * @destoryed: set when sync_timeline is destroyed + * @child_list_head: list of children sync_pts for this sync_timeline + * @child_list_lock: lock protecting @child_list_head, destroyed, and + * sync_pt.status + * @active_list_head: list of active (unsignaled/errored) sync_pts + */ +struct sync_timeline { + const struct sync_timeline_ops *ops; + char name[32]; + + /* protected by child_list_lock */ + bool destroyed; + + struct list_head child_list_head; + spinlock_t child_list_lock; + + struct list_head active_list_head; + spinlock_t active_list_lock; +}; + +/** + * struct sync_pt - sync point + * @parent: sync_timeline to which this sync_pt belongs + * @child_list: membership in sync_timeline.child_list_head + * @active_list: membership in sync_timeline.active_list_head + * @fence: sync_fence to which the sync_pt belongs + * @pt_list: membership in sync_fence.pt_list_head + * @status: 1: signaled, 0:active, <0: error + */ +struct sync_pt { + struct sync_timeline *parent; + struct list_head child_list; + + struct list_head active_list; + + struct sync_fence *fence; + struct list_head pt_list; + + /* protected by parent->active_list_lock */ + int status; +}; + +/** + * struct sync_fence - sync fence + * @file: file representing this fence + * @name: name of sync_fence. Useful for debugging + * @pt_list_head: list of sync_pts in ths fence. immutable once fence + * is created + * @waiter_list_head: list of asynchronous waiters on this fence + * @waiter_list_lock: lock protecting @waiter_list_head and @status + * @status: 1: signaled, 0:active, <0: error + * + * @wq: wait queue for fence signaling + */ +struct sync_fence { + struct file *file; + char name[32]; + + /* this list is immutable once the fence is created */ + struct list_head pt_list_head; + + struct list_head waiter_list_head; + spinlock_t waiter_list_lock; /* also protects status */ + int status; + + wait_queue_head_t wq; +}; + +/** + * struct sync_fence_waiter - metadata for asynchronous waiter on a fence + * @waiter_list: membership in sync_fence.waiter_list_head + * @callback: function pointer to call when fence signals + * @callback_data: pointer to pass to @callback + */ +struct sync_fence_waiter { + struct list_head waiter_list; + + void (*callback)(struct sync_fence *fence, void *data); + void *callback_data; +}; + +/* + * API for sync_timeline implementers + */ + +/** + * sync_timeline_create() - creates a sync object + * @ops: specifies the implemention ops for the object + * @size: size to allocate for this obj + * @name: sync_timeline name + * + * Creates a new sync_timeline which will use the implemetation specified by + * @ops. @size bytes will be allocated allowing for implemntation specific + * data to be kept after the generic sync_timeline stuct. + */ +struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, + int size, const char *name); + +/** + * sync_timeline_destory() - destorys a sync object + * @obj: sync_timeline to destroy + * + * A sync implemntation should call this when the @obj is going away + * (i.e. module unload.) @obj won't actually be freed until all its childern + * sync_pts are freed. + */ +void sync_timeline_destroy(struct sync_timeline *obj); + +/** + * sync_timeline_signal() - signal a status change on a sync_timeline + * @obj: sync_timeline to signal + * + * A sync implemntation should call this any time one of it's sync_pts + * has signaled or has an error condition. + */ +void sync_timeline_signal(struct sync_timeline *obj); + +/** + * sync_pt_create() - creates a sync pt + * @parent: sync_pt's parent sync_timeline + * @size: size to allocate for this pt + * + * Creates a new sync_pt as a chiled of @parent. @size bytes will be + * allocated allowing for implemntation specific data to be kept after + * the generic sync_timeline struct. + */ +struct sync_pt *sync_pt_create(struct sync_timeline *parent, int size); + +/** + * sync_pt_free() - frees a sync pt + * @pt: sync_pt to free + * + * This should only be called on sync_pts which have been created but + * not added to a fence. + */ +void sync_pt_free(struct sync_pt *pt); + +/** + * sync_fence_create() - creates a sync fence + * @name: name of fence to create + * @pt: sync_pt to add to the fence + * + * Creates a fence containg @pt. Once this is called, the fence takes + * ownership of @pt. + */ +struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt); + +/* + * API for sync_fence consumers + */ + +/** + * sync_fence_merge() - merge two fences + * @name: name of new fence + * @a: fence a + * @b: fence b + * + * Creates a new fence which contains copies of all the sync_pts in both + * @a and @b. @a and @b remain valid, independent fences. + */ +struct sync_fence *sync_fence_merge(const char *name, + struct sync_fence *a, struct sync_fence *b); + +/** + * sync_fence_fdget() - get a fence from an fd + * @fd: fd referencing a fence + * + * Ensures @fd references a valid fence, increments the refcount of the backing + * file, and returns the fence. + */ +struct sync_fence *sync_fence_fdget(int fd); + +/** + * sync_fence_put() - puts a refernnce of a sync fence + * @fence: fence to put + * + * Puts a reference on @fence. If this is the last reference, the fence and + * all it's sync_pts will be freed + */ +void sync_fence_put(struct sync_fence *fence); + +/** + * sync_fence_install() - installs a fence into a file descriptor + * @fence: fence to instal + * @fd: file descriptor in which to install the fence + * + * Installs @fence into @fd. @fd's should be acquired through get_unused_fd(). + */ +void sync_fence_install(struct sync_fence *fence, int fd); + +/** + * sync_fence_wait_async() - registers and async wait on the fence + * @fence: fence to wait on + * @callback: callback + * @callback_data data to pass to the callback + * + * Returns 1 if @fence has already signaled. + * + * Registers a callback to be called when @fence signals or has an error + */ +int sync_fence_wait_async(struct sync_fence *fence, + void (*callback)(struct sync_fence *, void *data), + void *callback_data); + +/** + * sync_fence_wait() - wait on fence + * @fence: fence to wait on + * @tiemout: timeout in ms + * + * Wait for @fence to be signaled or have an error. Waits indefintly + * if @timeout = 0 + */ +int sync_fence_wait(struct sync_fence *fence, long timeout); + +/* useful for sync driver's debug print handlers */ +const char *sync_status_str(int status); + +#endif /* __KERNEL__ */ + +/** + * struct sync_merge_data - data passed to merge ioctl + * @fd2: file descriptor of second fence + * @name: name of new fence + * @fence: returns the fd of the new fence to userspace + */ +struct sync_merge_data { + __s32 fd2; /* fd of second fence */ + char name[32]; /* name of new fence */ + __s32 fence; /* fd on newly created fence */ +}; + +#define SYNC_IOC_MAGIC '>' + +/** + * DOC: SYNC_IOC_WAIT - wait for a fence to signal + * + * pass timeout in milliseconds. + */ +#define SYNC_IOC_WAIT _IOW(SYNC_IOC_MAGIC, 0, __u32) + +/** + * DOC: SYNC_IOC_MERGE - merge two fences + * + * Takes a struct sync_merge_data. Creates a new fence containing copies of + * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the + * new fence's fd in sync_merge_data.fence + */ +#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data) + +#endif /* _LINUX_SYNC_H */
From: Erik Gilling konkers@android.com
Adds a base sync driver that uses the cpu for serialization.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Add commit message, whitespace fixes and move to staging directory] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/Kconfig | 18 +++ drivers/staging/android/Makefile | 1 + drivers/staging/android/sw_sync.c | 224 +++++++++++++++++++++++++++++++++++++ drivers/staging/android/sw_sync.h | 58 ++++++++++ 4 files changed, 301 insertions(+) create mode 100644 drivers/staging/android/sw_sync.c create mode 100644 drivers/staging/android/sw_sync.h
diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig index c95aede..cc406cc 100644 --- a/drivers/staging/android/Kconfig +++ b/drivers/staging/android/Kconfig @@ -81,6 +81,24 @@ config SYNC drivers. Sync implementations can take advantage of hardware synchronization built into devices like GPUs.
+config SW_SYNC + bool "Software synchronization objects" + default n + depends on SYNC + help + A sync object driver that uses a 32bit counter to coordinate + syncrhronization. Useful when there is no hardware primitive backing + the synchronization. + +config SW_SYNC_USER + bool "Userspace API for SW_SYNC" + default n + depends on SW_SYNC + help + Provides a user space API to the sw sync object. + *WARNING* improper use of this can result in deadlocking kernel + drivers from userspace. + endif # if ANDROID
endmenu diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile index 22c656c..c136299 100644 --- a/drivers/staging/android/Makefile +++ b/drivers/staging/android/Makefile @@ -8,3 +8,4 @@ obj-$(CONFIG_ANDROID_TIMED_GPIO) += timed_gpio.o obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER) += lowmemorykiller.o obj-$(CONFIG_ANDROID_INTF_ALARM_DEV) += alarm-dev.o obj-$(CONFIG_SYNC) += sync.o +obj-$(CONFIG_SW_SYNC) += sw_sync.o diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c new file mode 100644 index 0000000..c27004c --- /dev/null +++ b/drivers/staging/android/sw_sync.c @@ -0,0 +1,224 @@ +/* + * drivers/base/sw_sync.c + * + * Copyright (C) 2012 Google, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/kernel.h> +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/miscdevice.h> +#include <linux/module.h> +#include <linux/syscalls.h> +#include <linux/uaccess.h> + +#include "sw_sync.h" + +static int sw_sync_cmp(u32 a, u32 b) +{ + if (a == b) + return 0; + + return ((s32)a - (s32)b) < 0 ? -1 : 1; +} + +struct sync_pt *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value) +{ + struct sw_sync_pt *pt; + + pt = (struct sw_sync_pt *) + sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt)); + + pt->value = value; + + return (struct sync_pt *)pt; +} + +static struct sync_pt *sw_sync_pt_dup(struct sync_pt *sync_pt) +{ + struct sw_sync_pt *pt = (struct sw_sync_pt *) sync_pt; + struct sw_sync_timeline *obj = + (struct sw_sync_timeline *)sync_pt->parent; + + return (struct sync_pt *) sw_sync_pt_create(obj, pt->value); +} + +static int sw_sync_pt_has_signaled(struct sync_pt *sync_pt) +{ + struct sw_sync_pt *pt = (struct sw_sync_pt *)sync_pt; + struct sw_sync_timeline *obj = + (struct sw_sync_timeline *)sync_pt->parent; + + return sw_sync_cmp(obj->value, pt->value) >= 0; +} + +static int sw_sync_pt_compare(struct sync_pt *a, struct sync_pt *b) +{ + struct sw_sync_pt *pt_a = (struct sw_sync_pt *)a; + struct sw_sync_pt *pt_b = (struct sw_sync_pt *)b; + + return sw_sync_cmp(pt_a->value, pt_b->value); +} + +struct sync_timeline_ops sw_sync_timeline_ops = { + .driver_name = "sw_sync", + .dup = sw_sync_pt_dup, + .has_signaled = sw_sync_pt_has_signaled, + .compare = sw_sync_pt_compare, +}; + + +struct sw_sync_timeline *sw_sync_timeline_create(const char *name) +{ + struct sw_sync_timeline *obj = (struct sw_sync_timeline *) + sync_timeline_create(&sw_sync_timeline_ops, + sizeof(struct sw_sync_timeline), + name); + + return obj; +} + +void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) +{ + obj->value += inc; + + sync_timeline_signal(&obj->obj); +} + + +#ifdef CONFIG_SW_SYNC_USER +/* *WARNING* + * + * improper use of this can result in deadlocking kernel drivers from userspace. + */ + +/* opening sw_sync create a new sync obj */ +int sw_sync_open(struct inode *inode, struct file *file) +{ + struct sw_sync_timeline *obj; + char task_comm[TASK_COMM_LEN]; + + get_task_comm(task_comm, current); + + obj = sw_sync_timeline_create(task_comm); + if (obj == NULL) + return -ENOMEM; + + file->private_data = obj; + + return 0; +} + +int sw_sync_release(struct inode *inode, struct file *file) +{ + struct sw_sync_timeline *obj = file->private_data; + sync_timeline_destroy(&obj->obj); + return 0; +} + +long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, unsigned long arg) +{ + int fd = get_unused_fd(); + int err; + struct sync_pt *pt; + struct sync_fence *fence; + struct sw_sync_create_fence_data data; + + if (copy_from_user(&data, (void __user *)arg, sizeof(data))) + return -EFAULT; + + pt = sw_sync_pt_create(obj, data.value); + if (pt == NULL) { + err = -ENOMEM; + goto err; + } + + data.name[sizeof(data.name) - 1] = '\0'; + fence = sync_fence_create(data.name, pt); + if (fence == NULL) { + sync_pt_free(pt); + err = -ENOMEM; + goto err; + } + + data.fence = fd; + if (copy_to_user((void __user *)arg, &data, sizeof(data))) { + sync_fence_put(fence); + err = -EFAULT; + goto err; + } + + sync_fence_install(fence, fd); + + return 0; + +err: + put_unused_fd(fd); + return err; +} + +long sw_sync_ioctl_inc(struct sw_sync_timeline *obj, unsigned long arg) +{ + u32 value; + + if (copy_from_user(&value, (void __user *)arg, sizeof(value))) + return -EFAULT; + + sw_sync_timeline_inc(obj, value); + + return 0; +} + +long sw_sync_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct sw_sync_timeline *obj = file->private_data; + + switch (cmd) { + case SW_SYNC_IOC_CREATE_FENCE: + return sw_sync_ioctl_create_fence(obj, arg); + + case SW_SYNC_IOC_INC: + return sw_sync_ioctl_inc(obj, arg); + + default: + return -ENOTTY; + } +} + +static const struct file_operations sw_sync_fops = { + .owner = THIS_MODULE, + .open = sw_sync_open, + .release = sw_sync_release, + .unlocked_ioctl = sw_sync_ioctl, +}; + +static struct miscdevice sw_sync_dev = { + .minor = MISC_DYNAMIC_MINOR, + .name = "sw_sync", + .fops = &sw_sync_fops, +}; + +int __init sw_sync_device_init(void) +{ + return misc_register(&sw_sync_dev); +} + +void __exit sw_sync_device_remove(void) +{ + misc_deregister(&sw_sync_dev); +} + +module_init(sw_sync_device_init); +module_exit(sw_sync_device_remove); + +#endif /* CONFIG_SW_SYNC_USER */ diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h new file mode 100644 index 0000000..585040b --- /dev/null +++ b/drivers/staging/android/sw_sync.h @@ -0,0 +1,58 @@ +/* + * include/linux/sw_sync.h + * + * Copyright (C) 2012 Google, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#ifndef _LINUX_SW_SYNC_H +#define _LINUX_SW_SYNC_H + +#include <linux/types.h> + +#ifdef __KERNEL__ + +#include "sync.h" + +struct sw_sync_timeline { + struct sync_timeline obj; + + u32 value; +}; + +struct sw_sync_pt { + struct sync_pt pt; + + u32 value; +}; + +struct sw_sync_timeline *sw_sync_timeline_create(const char *name); +void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc); + +struct sync_pt *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value); + +#endif /* __KERNEL __ */ + +struct sw_sync_create_fence_data { + __u32 value; + char name[32]; + __s32 fence; /* fd of new 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 /* _LINUX_SW_SYNC_H */
From: Erik Gilling konkers@android.com
Add ktime timestamps to sync_pt structure and update them when signaled
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Added commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 5 +++++ drivers/staging/android/sync.h | 5 +++++ 2 files changed, 10 insertions(+)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 4a9e63d..88d7e66 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -155,12 +155,17 @@ void sync_pt_free(struct sync_pt *pt) /* call with pt->parent->active_list_lock held */ static int _sync_pt_has_signaled(struct sync_pt *pt) { + int old_status = pt->status; + if (!pt->status) pt->status = pt->parent->ops->has_signaled(pt);
if (!pt->status && pt->parent->destroyed) pt->status = -ENOENT;
+ if (pt->status != old_status) + pt->timestamp = ktime_get(); + return pt->status; }
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 388acd1..a8e289d 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -16,6 +16,7 @@ #include <linux/types.h> #ifdef __KERNEL__
+#include <linux/ktime.h> #include <linux/list.h> #include <linux/spinlock.h> #include <linux/wait.h> @@ -90,6 +91,8 @@ struct sync_timeline { * @fence: sync_fence to which the sync_pt belongs * @pt_list: membership in sync_fence.pt_list_head * @status: 1: signaled, 0:active, <0: error + * @timestamp: time which sync_pt status transitioned from active to + * singaled or error. */ struct sync_pt { struct sync_timeline *parent; @@ -102,6 +105,8 @@ struct sync_pt {
/* protected by parent->active_list_lock */ int status; + + ktime_t timestamp; };
/**
From: Erik Gilling konkers@android.com
Add support for debugfs
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Add commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 176 +++++++++++++++++++++++++++++++++++++++- drivers/staging/android/sync.h | 20 ++++- 2 files changed, 191 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 88d7e66..4ab55a3 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -14,10 +14,12 @@ * */
+#include <linux/debugfs.h> #include <linux/file.h> #include <linux/fs.h> #include <linux/kernel.h> #include <linux/sched.h> +#include <linux/seq_file.h> #include <linux/slab.h> #include <linux/uaccess.h> #include <linux/anon_inodes.h> @@ -27,10 +29,17 @@ static void sync_fence_signal_pt(struct sync_pt *pt); static int _sync_pt_has_signaled(struct sync_pt *pt);
+static LIST_HEAD(sync_timeline_list_head); +static DEFINE_SPINLOCK(sync_timeline_list_lock); + +static LIST_HEAD(sync_fence_list_head); +static DEFINE_SPINLOCK(sync_fence_list_lock); + struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, int size, const char *name) { struct sync_timeline *obj; + unsigned long flags;
if (size < sizeof(struct sync_timeline)) return NULL; @@ -48,9 +57,27 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, INIT_LIST_HEAD(&obj->active_list_head); spin_lock_init(&obj->active_list_lock);
+ spin_lock_irqsave(&sync_timeline_list_lock, flags); + list_add_tail(&obj->sync_timeline_list, &sync_timeline_list_head); + spin_unlock_irqrestore(&sync_timeline_list_lock, flags); + return obj; }
+static void sync_timeline_free(struct sync_timeline *obj) +{ + unsigned long flags; + + if (obj->ops->release_obj) + obj->ops->release_obj(obj); + + spin_lock_irqsave(&sync_timeline_list_lock, flags); + list_del(&obj->sync_timeline_list); + spin_unlock_irqrestore(&sync_timeline_list_lock, flags); + + kfree(obj); +} + void sync_timeline_destroy(struct sync_timeline *obj) { unsigned long flags; @@ -62,7 +89,7 @@ void sync_timeline_destroy(struct sync_timeline *obj) spin_unlock_irqrestore(&obj->child_list_lock, flags);
if (needs_freeing) - kfree(obj); + sync_timeline_free(obj); else sync_timeline_signal(obj); } @@ -95,7 +122,7 @@ static void sync_timeline_remove_pt(struct sync_pt *pt) spin_unlock_irqrestore(&obj->child_list_lock, flags);
if (needs_freeing) - kfree(obj); + sync_timeline_free(obj); }
void sync_timeline_signal(struct sync_timeline *obj) @@ -206,6 +233,7 @@ static const struct file_operations sync_fence_fops = { static struct sync_fence *sync_fence_alloc(const char *name) { struct sync_fence *fence; + unsigned long flags;
fence = kzalloc(sizeof(struct sync_fence), GFP_KERNEL); if (fence == NULL) @@ -223,6 +251,11 @@ static struct sync_fence *sync_fence_alloc(const char *name) spin_lock_init(&fence->waiter_list_lock);
init_waitqueue_head(&fence->wq); + + spin_lock_irqsave(&sync_fence_list_lock, flags); + list_add_tail(&fence->sync_fence_list, &sync_fence_list_head); + spin_unlock_irqrestore(&sync_fence_list_lock, flags); + return fence;
err: @@ -451,8 +484,14 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) static int sync_fence_release(struct inode *inode, struct file *file) { struct sync_fence *fence = file->private_data; + unsigned long flags;
sync_fence_free_pts(fence); + + spin_lock_irqsave(&sync_fence_list_lock, flags); + list_del(&fence->sync_fence_list); + spin_unlock_irqrestore(&sync_fence_list_lock, flags); + kfree(fence);
return 0; @@ -523,8 +562,141 @@ static long sync_fence_ioctl(struct file *file, unsigned int cmd,
case SYNC_IOC_MERGE: return sync_fence_ioctl_merge(fence, arg); + default: return -ENOTTY; } }
+#ifdef CONFIG_DEBUG_FS +static const char *sync_status_str(int status) +{ + if (status > 0) + return "signaled"; + else if (status == 0) + return "active"; + else + return "error"; +} + +static void sync_print_pt(struct seq_file *s, struct sync_pt *pt, bool fence) +{ + int status = pt->status; + seq_printf(s, " %s%spt %s", + fence ? pt->parent->name : "", + fence ? "_" : "", + sync_status_str(status)); + if (pt->status) { + struct timeval tv = ktime_to_timeval(pt->timestamp); + seq_printf(s, "@%ld.%06ld", tv.tv_sec, tv.tv_usec); + } + + if (pt->parent->ops->print_pt) { + seq_printf(s, ": "); + pt->parent->ops->print_pt(s, pt); + } + + seq_printf(s, "\n"); +} + +static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj) +{ + struct list_head *pos; + unsigned long flags; + + seq_printf(s, "%s %s", obj->name, obj->ops->driver_name); + + if (obj->ops->print_obj) { + seq_printf(s, ": "); + obj->ops->print_obj(s, obj); + } + + seq_printf(s, "\n"); + + spin_lock_irqsave(&obj->child_list_lock, flags); + list_for_each(pos, &obj->child_list_head) { + struct sync_pt *pt = + container_of(pos, struct sync_pt, child_list); + sync_print_pt(s, pt, false); + } + spin_unlock_irqrestore(&obj->child_list_lock, flags); +} + +static void sync_print_fence(struct seq_file *s, struct sync_fence *fence) +{ + struct list_head *pos; + unsigned long flags; + + seq_printf(s, "%s: %s\n", fence->name, sync_status_str(fence->status)); + + list_for_each(pos, &fence->pt_list_head) { + struct sync_pt *pt = + container_of(pos, struct sync_pt, pt_list); + sync_print_pt(s, pt, true); + } + + spin_lock_irqsave(&fence->waiter_list_lock, flags); + list_for_each(pos, &fence->waiter_list_head) { + struct sync_fence_waiter *waiter = + container_of(pos, struct sync_fence_waiter, + waiter_list); + + seq_printf(s, "waiter %pF %p\n", waiter->callback, + waiter->callback_data); + } + spin_unlock_irqrestore(&fence->waiter_list_lock, flags); +} + +static int sync_debugfs_show(struct seq_file *s, void *unused) +{ + unsigned long flags; + struct list_head *pos; + + seq_printf(s, "objs:\n--------------\n"); + + spin_lock_irqsave(&sync_timeline_list_lock, flags); + list_for_each(pos, &sync_timeline_list_head) { + struct sync_timeline *obj = + container_of(pos, struct sync_timeline, + sync_timeline_list); + + sync_print_obj(s, obj); + seq_printf(s, "\n"); + } + spin_unlock_irqrestore(&sync_timeline_list_lock, flags); + + seq_printf(s, "fences:\n--------------\n"); + + spin_lock_irqsave(&sync_fence_list_lock, flags); + list_for_each(pos, &sync_fence_list_head) { + struct sync_fence *fence = + container_of(pos, struct sync_fence, sync_fence_list); + + sync_print_fence(s, fence); + seq_printf(s, "\n"); + } + spin_unlock_irqrestore(&sync_fence_list_lock, flags); + return 0; +} + +static int sync_debugfs_open(struct inode *inode, struct file *file) +{ + return single_open(file, sync_debugfs_show, inode->i_private); +} + +static const struct file_operations sync_debugfs_fops = { + .open = sync_debugfs_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static __init int sync_debugfs_init(void) +{ + debugfs_create_file("sync", S_IRUGO, NULL, NULL, &sync_debugfs_fops); + return 0; +} + +late_initcall(sync_debugfs_init); + +#endif diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index a8e289d..d64271b 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -39,6 +39,10 @@ struct sync_fence; * -1 if a will signabl before b * @free_pt: called before sync_pt is freed * @release_obj: called before sync_timeline is freed + * @print_obj: print aditional debug information about sync_timeline. + * should not print a newline + * @print_pt: print aditional debug information about sync_pt. + * should not print a newline */ struct sync_timeline_ops { const char *driver_name; @@ -57,6 +61,13 @@ struct sync_timeline_ops {
/* optional */ void (*release_obj)(struct sync_timeline *sync_timeline); + + /* optional */ + void (*print_obj)(struct seq_file *s, + struct sync_timeline *sync_timeline); + + /* optional */ + void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt); };
/** @@ -68,6 +79,7 @@ struct sync_timeline_ops { * @child_list_lock: lock protecting @child_list_head, destroyed, and * sync_pt.status * @active_list_head: list of active (unsignaled/errored) sync_pts + * @sync_timeline_list: membership in global sync_timeline_list */ struct sync_timeline { const struct sync_timeline_ops *ops; @@ -81,6 +93,8 @@ struct sync_timeline {
struct list_head active_list_head; spinlock_t active_list_lock; + + struct list_head sync_timeline_list; };
/** @@ -120,6 +134,7 @@ struct sync_pt { * @status: 1: signaled, 0:active, <0: error * * @wq: wait queue for fence signaling + * @sync_fence_list: membership in global fence list */ struct sync_fence { struct file *file; @@ -133,6 +148,8 @@ struct sync_fence { int status;
wait_queue_head_t wq; + + struct list_head sync_fence_list; };
/** @@ -281,9 +298,6 @@ int sync_fence_wait_async(struct sync_fence *fence, */ int sync_fence_wait(struct sync_fence *fence, long timeout);
-/* useful for sync driver's debug print handlers */ -const char *sync_status_str(int status); - #endif /* __KERNEL__ */
/**
From: Erik Gilling konkers@android.com
Add debugfs support hooks.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Add commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sw_sync.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index c27004c..64c5ebb 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -70,11 +70,30 @@ static int sw_sync_pt_compare(struct sync_pt *a, struct sync_pt *b) return sw_sync_cmp(pt_a->value, pt_b->value); }
+static void sw_sync_print_obj(struct seq_file *s, + struct sync_timeline *sync_timeline) +{ + struct sw_sync_timeline *obj = (struct sw_sync_timeline *)sync_timeline; + + seq_printf(s, "%d", obj->value); +} + +static void sw_sync_print_pt(struct seq_file *s, struct sync_pt *sync_pt) +{ + struct sw_sync_pt *pt = (struct sw_sync_pt *)sync_pt; + struct sw_sync_timeline *obj = + (struct sw_sync_timeline *)sync_pt->parent; + + seq_printf(s, "%d / %d", pt->value, obj->value); +} + struct sync_timeline_ops sw_sync_timeline_ops = { .driver_name = "sw_sync", .dup = sw_sync_pt_dup, .has_signaled = sw_sync_pt_has_signaled, .compare = sw_sync_pt_compare, + .print_obj = sw_sync_print_obj, + .print_pt = sw_sync_print_pt, };
From: Erik Gilling konkers@android.com
Add ioctl to get fence data
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Commit message tweaks] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 81 ++++++++++++++++++++++++++++++++++++++++ drivers/staging/android/sync.h | 57 ++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 4ab55a3..f84caad 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -551,6 +551,84 @@ err_put_fd: return err; }
+static int sync_fill_pt_info(struct sync_pt *pt, void *data, int size) +{ + struct sync_pt_info *info = data; + int ret; + + if (size < sizeof(struct sync_pt_info)) + return -ENOMEM; + + info->len = sizeof(struct sync_pt_info); + + if (pt->parent->ops->fill_driver_data) { + ret = pt->parent->ops->fill_driver_data(pt, info->driver_data, + size - sizeof(*info)); + if (ret < 0) + return ret; + + info->len += ret; + } + + strlcpy(info->obj_name, pt->parent->name, sizeof(info->obj_name)); + strlcpy(info->driver_name, pt->parent->ops->driver_name, + sizeof(info->driver_name)); + info->status = pt->status; + info->timestamp_ns = ktime_to_ns(pt->timestamp); + + return info->len; +} + +static long sync_fence_ioctl_fence_info(struct sync_fence *fence, + unsigned long arg) +{ + struct sync_fence_info_data *data; + struct list_head *pos; + __u32 size; + __u32 len = 0; + int ret; + + if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + return -EFAULT; + + if (size < sizeof(struct sync_fence_info_data)) + return -EINVAL; + + if (size > 4096) + size = 4096; + + data = kzalloc(size, GFP_KERNEL); + if (data == NULL) + return -ENOMEM; + + strlcpy(data->name, fence->name, sizeof(data->name)); + data->status = fence->status; + len = sizeof(struct sync_fence_info_data); + + list_for_each(pos, &fence->pt_list_head) { + struct sync_pt *pt = + container_of(pos, struct sync_pt, pt_list); + + ret = sync_fill_pt_info(pt, (u8 *)data + len, size - len); + + if (ret < 0) + goto out; + + len += ret; + } + + data->len = len; + + if (copy_to_user((void __user *)arg, data, len)) + ret = -EFAULT; + else + ret = 0; + +out: + kfree(data); + + return ret; +}
static long sync_fence_ioctl(struct file *file, unsigned int cmd, unsigned long arg) @@ -563,6 +641,9 @@ static long sync_fence_ioctl(struct file *file, unsigned int cmd, case SYNC_IOC_MERGE: return sync_fence_ioctl_merge(fence, arg);
+ case SYNC_IOC_FENCE_INFO: + return sync_fence_ioctl_fence_info(fence, arg); + default: return -ENOTTY; } diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index d64271b..4f19938 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -43,6 +43,10 @@ struct sync_fence; * should not print a newline * @print_pt: print aditional debug information about sync_pt. * should not print a newline + * @fill_driver_data: write implmentation specific driver data to data. + * should return an error if there is not enough room + * as specified by size. This information is returned + * to userspace by SYNC_IOC_FENCE_INFO. */ struct sync_timeline_ops { const char *driver_name; @@ -68,6 +72,9 @@ struct sync_timeline_ops {
/* optional */ void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt); + + /* optional */ + int (*fill_driver_data)(struct sync_pt *syncpt, void *data, int size); };
/** @@ -312,6 +319,42 @@ struct sync_merge_data { __s32 fence; /* fd on newly created fence */ };
+/** + * struct sync_pt_info - detailed sync_pt information + * @len: length of sync_pt_info including any driver_data + * @obj_name: name of parent sync_timeline + * @driver_name: name of driver implmenting the parent + * @status: status of the sync_pt 0:active 1:signaled <0:error + * @timestamp_ns: timestamp of status change in nanoseconds + * @driver_data: any driver dependant data + */ +struct sync_pt_info { + __u32 len; + char obj_name[32]; + char driver_name[32]; + __s32 status; + __u64 timestamp_ns; + + __u8 driver_data[0]; +}; + +/** + * struct sync_fence_info_data - data returned from fence info ioctl + * @len: ioctl caller writes the size of the buffer its passing in. + * ioctl returns length of sync_fence_data reutnred to userspace + * including pt_info. + * @name: name of fence + * @status: status of fence. 1: signaled 0:active <0:error + * @pt_info: a sync_pt_info struct for every sync_pt in the fence + */ +struct sync_fence_info_data { + __u32 len; + char name[32]; + __s32 status; + + __u8 pt_info[0]; +}; + #define SYNC_IOC_MAGIC '>'
/** @@ -330,4 +373,18 @@ struct sync_merge_data { */ #define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data)
+/** + * DOC: SYNC_IOC_FENCE_INFO - get detailed information on a fence + * + * Takes a struct sync_fence_info_data with extra space allocated for pt_info. + * Caller should write the size of the buffer into len. On return, len is + * updated to reflect the total size of the sync_fence_info_data including + * pt_info. + * + * pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence. + * To itterate over the sync_pt_infos, use the sync_pt_info.len field. + */ +#define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2,\ + struct sync_fence_info_data) + #endif /* _LINUX_SYNC_H */
From: Erik Gilling konkers@android.com
Add fill_driver_data support to export fence data to ioctl
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Add commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sw_sync.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 64c5ebb..081e4d1 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -87,6 +87,19 @@ static void sw_sync_print_pt(struct seq_file *s, struct sync_pt *sync_pt) seq_printf(s, "%d / %d", pt->value, obj->value); }
+static int sw_sync_fill_driver_data(struct sync_pt *sync_pt, + void *data, int size) +{ + struct sw_sync_pt *pt = (struct sw_sync_pt *)sync_pt; + + if (size < sizeof(pt->value)) + return -ENOMEM; + + memcpy(data, &pt->value, sizeof(pt->value)); + + return sizeof(pt->value); +} + struct sync_timeline_ops sw_sync_timeline_ops = { .driver_name = "sw_sync", .dup = sw_sync_pt_dup, @@ -94,6 +107,7 @@ struct sync_timeline_ops sw_sync_timeline_ops = { .compare = sw_sync_pt_compare, .print_obj = sw_sync_print_obj, .print_pt = sw_sync_print_pt, + .fill_driver_data = sw_sync_fill_driver_data, };
From: Erik Gilling konkers@android.com
Support poll on sync fence
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Add commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index f84caad..9e35ea3 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -18,6 +18,7 @@ #include <linux/file.h> #include <linux/fs.h> #include <linux/kernel.h> +#include <linux/poll.h> #include <linux/sched.h> #include <linux/seq_file.h> #include <linux/slab.h> @@ -221,12 +222,14 @@ out: }
static int sync_fence_release(struct inode *inode, struct file *file); +static unsigned int sync_fence_poll(struct file *file, poll_table *wait); static long sync_fence_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
static const struct file_operations sync_fence_fops = { .release = sync_fence_release, + .poll = sync_fence_poll, .unlocked_ioctl = sync_fence_ioctl, };
@@ -497,6 +500,20 @@ static int sync_fence_release(struct inode *inode, struct file *file) return 0; }
+static unsigned int sync_fence_poll(struct file *file, poll_table *wait) +{ + struct sync_fence *fence = file->private_data; + + poll_wait(file, &fence->wq, wait); + + if (fence->status == 1) + return POLLIN; + else if (fence->status < 0) + return POLLERR; + else + return 0; +} + static long sync_fence_ioctl_wait(struct sync_fence *fence, unsigned long arg) { __s32 value;
From: Erik Gilling konkers@android.com
In order to allow drivers to cleanly handled teardown we need to allow them to cancel pending async waits. To do this cleanly, we move allocation of sync_fence_waiter to the driver calling sync_async_wait().
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 46 +++++++++++++++++++++++++++------------- drivers/staging/android/sync.h | 36 +++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 9e35ea3..54f84d9 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -421,33 +421,22 @@ static void sync_fence_signal_pt(struct sync_pt *pt) container_of(pos, struct sync_fence_waiter, waiter_list);
- waiter->callback(fence, waiter->callback_data); list_del(pos); - kfree(waiter); + waiter->callback(fence, waiter); } wake_up(&fence->wq); } }
int sync_fence_wait_async(struct sync_fence *fence, - void (*callback)(struct sync_fence *, void *data), - void *callback_data) + struct sync_fence_waiter *waiter) { - struct sync_fence_waiter *waiter; unsigned long flags; int err = 0;
- waiter = kzalloc(sizeof(struct sync_fence_waiter), GFP_KERNEL); - if (waiter == NULL) - return -ENOMEM; - - waiter->callback = callback; - waiter->callback_data = callback_data; - spin_lock_irqsave(&fence->waiter_list_lock, flags);
if (fence->status) { - kfree(waiter); err = fence->status; goto out; } @@ -459,6 +448,34 @@ out: return err; }
+int sync_fence_cancel_async(struct sync_fence *fence, + struct sync_fence_waiter *waiter) +{ + struct list_head *pos; + struct list_head *n; + unsigned long flags; + int ret = -ENOENT; + + spin_lock_irqsave(&fence->waiter_list_lock, flags); + /* + * Make sure waiter is still in waiter_list because it is possible for + * the waiter to be removed from the list while the callback is still + * pending. + */ + list_for_each_safe(pos, n, &fence->waiter_list_head) { + struct sync_fence_waiter *list_waiter = + container_of(pos, struct sync_fence_waiter, + waiter_list); + if (list_waiter == waiter) { + list_del(pos); + ret = 0; + break; + } + } + spin_unlock_irqrestore(&fence->waiter_list_lock, flags); + return ret; +} + int sync_fence_wait(struct sync_fence *fence, long timeout) { int err; @@ -739,8 +756,7 @@ static void sync_print_fence(struct seq_file *s, struct sync_fence *fence) container_of(pos, struct sync_fence_waiter, waiter_list);
- seq_printf(s, "waiter %pF %p\n", waiter->callback, - waiter->callback_data); + seq_printf(s, "waiter %pF\n", waiter->callback); } spin_unlock_irqrestore(&fence->waiter_list_lock, flags); } diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 4f19938..943f414 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -159,6 +159,10 @@ struct sync_fence { struct list_head sync_fence_list; };
+struct sync_fence_waiter; +typedef void (*sync_callback_t)(struct sync_fence *fence, + struct sync_fence_waiter *waiter); + /** * struct sync_fence_waiter - metadata for asynchronous waiter on a fence * @waiter_list: membership in sync_fence.waiter_list_head @@ -168,10 +172,15 @@ struct sync_fence { struct sync_fence_waiter { struct list_head waiter_list;
- void (*callback)(struct sync_fence *fence, void *data); - void *callback_data; + sync_callback_t callback; };
+static inline void sync_fence_waiter_init(struct sync_fence_waiter *waiter, + sync_callback_t callback) +{ + waiter->callback = callback; +} + /* * API for sync_timeline implementers */ @@ -284,16 +293,29 @@ void sync_fence_install(struct sync_fence *fence, int fd); /** * sync_fence_wait_async() - registers and async wait on the fence * @fence: fence to wait on - * @callback: callback - * @callback_data data to pass to the callback + * @waiter: waiter callback struck * * Returns 1 if @fence has already signaled. * - * Registers a callback to be called when @fence signals or has an error + * Registers a callback to be called when @fence signals or has an error. + * @waiter should be initialized with sync_fence_waiter_init(). */ int sync_fence_wait_async(struct sync_fence *fence, - void (*callback)(struct sync_fence *, void *data), - void *callback_data); + struct sync_fence_waiter *waiter); + +/** + * sync_fence_cancel_async() - cancels an async wait + * @fence: fence to wait on + * @waiter: waiter callback struck + * + * returns 0 if waiter was removed from fence's async waiter list. + * returns -ENOENT if waiter was not found on fence's async waiter list. + * + * Cancels a previously registered async wait. Will fail gracefully if + * @waiter was never registered or if @fence has already signaled @waiter. + */ +int sync_fence_cancel_async(struct sync_fence *fence, + struct sync_fence_waiter *waiter);
/** * sync_fence_wait() - wait on fence
From: Erik Gilling konkers@android.com
This is needed to allow modules to link against the sync subsystem
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 54f84d9..6739a84 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -15,6 +15,7 @@ */
#include <linux/debugfs.h> +#include <linux/export.h> #include <linux/file.h> #include <linux/fs.h> #include <linux/kernel.h> @@ -64,6 +65,7 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops,
return obj; } +EXPORT_SYMBOL(sync_timeline_create);
static void sync_timeline_free(struct sync_timeline *obj) { @@ -94,6 +96,7 @@ void sync_timeline_destroy(struct sync_timeline *obj) else sync_timeline_signal(obj); } +EXPORT_SYMBOL(sync_timeline_destroy);
static void sync_timeline_add_pt(struct sync_timeline *obj, struct sync_pt *pt) { @@ -152,6 +155,7 @@ void sync_timeline_signal(struct sync_timeline *obj) sync_fence_signal_pt(pt); } } +EXPORT_SYMBOL(sync_timeline_signal);
struct sync_pt *sync_pt_create(struct sync_timeline *parent, int size) { @@ -169,6 +173,7 @@ struct sync_pt *sync_pt_create(struct sync_timeline *parent, int size)
return pt; } +EXPORT_SYMBOL(sync_pt_create);
void sync_pt_free(struct sync_pt *pt) { @@ -179,6 +184,7 @@ void sync_pt_free(struct sync_pt *pt)
kfree(pt); } +EXPORT_SYMBOL(sync_pt_free);
/* call with pt->parent->active_list_lock held */ static int _sync_pt_has_signaled(struct sync_pt *pt) @@ -284,6 +290,7 @@ struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
return fence; } +EXPORT_SYMBOL(sync_fence_create);
static int sync_fence_copy_pts(struct sync_fence *dst, struct sync_fence *src) { @@ -331,16 +338,19 @@ err: fput(file); return NULL; } +EXPORT_SYMBOL(sync_fence_fdget);
void sync_fence_put(struct sync_fence *fence) { fput(fence->file); } +EXPORT_SYMBOL(sync_fence_put);
void sync_fence_install(struct sync_fence *fence, int fd) { fd_install(fd, fence->file); } +EXPORT_SYMBOL(sync_fence_install);
static int sync_fence_get_status(struct sync_fence *fence) { @@ -388,6 +398,7 @@ err: kfree(fence); return NULL; } +EXPORT_SYMBOL(sync_fence_merge);
static void sync_fence_signal_pt(struct sync_pt *pt) { @@ -447,6 +458,7 @@ out:
return err; } +EXPORT_SYMBOL(sync_fence_wait_async);
int sync_fence_cancel_async(struct sync_fence *fence, struct sync_fence_waiter *waiter) @@ -475,6 +487,7 @@ int sync_fence_cancel_async(struct sync_fence *fence, spin_unlock_irqrestore(&fence->waiter_list_lock, flags); return ret; } +EXPORT_SYMBOL(sync_fence_cancel_async);
int sync_fence_wait(struct sync_fence *fence, long timeout) { @@ -500,6 +513,7 @@ int sync_fence_wait(struct sync_fence *fence, long timeout)
return 0; } +EXPORT_SYMBOL(sync_fence_wait);
static int sync_fence_release(struct inode *inode, struct file *file) {
On Thu, Feb 28, 2013 at 04:43:06PM -0800, John Stultz wrote:
From: Erik Gilling konkers@android.com
This is needed to allow modules to link against the sync subsystem
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org
drivers/staging/android/sync.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 54f84d9..6739a84 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -15,6 +15,7 @@ */
#include <linux/debugfs.h> +#include <linux/export.h> #include <linux/file.h> #include <linux/fs.h> #include <linux/kernel.h> @@ -64,6 +65,7 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops,
return obj; } +EXPORT_SYMBOL(sync_timeline_create);
As these are now global, should they be a bit more "specific"? "sync_" seems pretty broad.
Also, EXPORT_SYMBOL_GPL() perhaps?
And who is using these exports?
thanks,
greg k-h
On 02/28/2013 06:00 PM, Greg KH wrote:
On Thu, Feb 28, 2013 at 04:43:06PM -0800, John Stultz wrote:
From: Erik Gilling konkers@android.com
This is needed to allow modules to link against the sync subsystem
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org
drivers/staging/android/sync.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 54f84d9..6739a84 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -15,6 +15,7 @@ */
#include <linux/debugfs.h> +#include <linux/export.h> #include <linux/file.h> #include <linux/fs.h> #include <linux/kernel.h> @@ -64,6 +65,7 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops,
return obj; } +EXPORT_SYMBOL(sync_timeline_create);
As these are now global, should they be a bit more "specific"? "sync_" seems pretty broad.
Given its the sync driver, its most obvious choice, but I agree its likely to collide with filesystem related or other sync_ named functions that don't have a subsystem prefix.
Any suggestions?
The only good alternative I can think of is that in some private conversations with DanielV, he referred to Android using "sync-points".
Erik: Would syncpoint_ be an ok prefix? Or do you have other ideas?
Also, EXPORT_SYMBOL_GPL() perhaps?
And who is using these exports?
From some quick git grepping...
In the android exynos tree: https://android.googlesource.com/kernel/exynos.git android-exynos-manta-3.4-jb-mr1.1
drivers/gpu/arm/t6xx/kbase/src/linux/mali_kbase_sync.c: tl = sync_timeline_creat drivers/media/video/videobuf2-core.c: q->timeline = sw_sync_timeline_create(q- drivers/video/s3c-fb.c: sfb->timeline = sw_sync_timeline_create("s3c-fb");
In the android msm tree: https://android.googlesource.com/kernel/msm.git android-msm-mako-3.4-jb-mr1.1
drivers/gpu/msm/kgsl_sync.c: context->timeline = sync_timeline_create(&kgsl_s drivers/video/msm/msm_fb.c: mfd->timeline = sw_sync_timeline_create(
thanks -john
On Thu, Feb 28, 2013 at 7:59 PM, John Stultz john.stultz@linaro.org wrote:
+EXPORT_SYMBOL(sync_timeline_create);
As these are now global, should they be a bit more "specific"? "sync_" seems pretty broad.
Given its the sync driver, its most obvious choice, but I agree its likely to collide with filesystem related or other sync_ named functions that don't have a subsystem prefix.
Any suggestions?
The only good alternative I can think of is that in some private conversations with DanielV, he referred to Android using "sync-points".
Erik: Would syncpoint_ be an ok prefix? Or do you have other ideas?
syncpoint would be semantically weird when you end up with struct syncpoint_pt. I'm open to suggestions as long as it works with XXXX_pt, XXXX_timeline, and XXXX_fence. I'll ask around the office and see if someone has a good idea.
Also, EXPORT_SYMBOL_GPL() perhaps?
And who is using these exports?
From some quick git grepping...
As John pointed out, the exynos and msm display and code uses them. I know nvidia is working on adding suport to their tegra tree. My knee jerk reaction is to make the export as permissible as possible. That being said, all of the ARM SoC vendors I've worked with have GPL kernel drivers even if their user space is closed. I'll reach out to them and ask for their opinions. Are there any issues with keeping them EXPORT_SYMBOL?
Cheers, Erik
On Fri, Mar 01, 2013 at 12:21:24AM -0800, Erik Gilling wrote:
On Thu, Feb 28, 2013 at 7:59 PM, John Stultz john.stultz@linaro.org wrote:
Also, EXPORT_SYMBOL_GPL() perhaps?
And who is using these exports?
From some quick git grepping...
As John pointed out, the exynos and msm display and code uses them. I know nvidia is working on adding suport to their tegra tree. My knee jerk reaction is to make the export as permissible as possible. That being said, all of the ARM SoC vendors I've worked with have GPL kernel drivers even if their user space is closed. I'll reach out to them and ask for their opinions. Are there any issues with keeping them EXPORT_SYMBOL?
There's no "issues", it's just that it is preferred by some people and companies to add new symbols to the kernel in this manner. It's really up to you / Google as you are the ones contributing the code.
thanks,
greg k-h
On Fri, Mar 1, 2013 at 5:55 AM, Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Mar 01, 2013 at 12:21:24AM -0800, Erik Gilling wrote:
As John pointed out, the exynos and msm display and code uses them. I know nvidia is working on adding suport to their tegra tree. My knee jerk reaction is to make the export as permissible as possible. That being said, all of the ARM SoC vendors I've worked with have GPL kernel drivers even if their user space is closed. I'll reach out to them and ask for their opinions. Are there any issues with keeping them EXPORT_SYMBOL?
There's no "issues", it's just that it is preferred by some people and companies to add new symbols to the kernel in this manner. It's really up to you / Google as you are the ones contributing the code.
Ok. Lets keep it EXPORT_SYMBOL then.
Cheers, Erik
On Fri, Mar 1, 2013 at 12:21 AM, Erik Gilling konkers@android.com wrote:
On Thu, Feb 28, 2013 at 7:59 PM, John Stultz john.stultz@linaro.org wrote:
Given its the sync driver, its most obvious choice, but I agree its likely to collide with filesystem related or other sync_ named functions that don't have a subsystem prefix.
Any suggestions?
The only good alternative I can think of is that in some private conversations with DanielV, he referred to Android using "sync-points".
Erik: Would syncpoint_ be an ok prefix? Or do you have other ideas?
syncpoint would be semantically weird when you end up with struct syncpoint_pt. I'm open to suggestions as long as it works with XXXX_pt, XXXX_timeline, and XXXX_fence. I'll ask around the office and see if someone has a good idea.
Colin Cross pointed out that this is limited to sync_fence_*, sync_pt_*, and sync_timeline_* and not sync_* so it's much less likely to have naming collisions.
Cheers, Erik
From: Erik Gilling konkers@android.com
Needed to let modules link against sw_sync.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sw_sync.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index 081e4d1..d689760 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -15,6 +15,7 @@ */
#include <linux/kernel.h> +#include <linux/export.h> #include <linux/file.h> #include <linux/fs.h> #include <linux/miscdevice.h> @@ -43,6 +44,7 @@ struct sync_pt *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value)
return (struct sync_pt *)pt; } +EXPORT_SYMBOL(sw_sync_pt_create);
static struct sync_pt *sw_sync_pt_dup(struct sync_pt *sync_pt) { @@ -120,6 +122,7 @@ struct sw_sync_timeline *sw_sync_timeline_create(const char *name)
return obj; } +EXPORT_SYMBOL(sw_sync_timeline_create);
void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc) { @@ -127,7 +130,7 @@ void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc)
sync_timeline_signal(&obj->obj); } - +EXPORT_SYMBOL(sw_sync_timeline_inc);
#ifdef CONFIG_SW_SYNC_USER /* *WARNING*
From: Erik Gilling konkers@android.com
Previously fence's pts were freed before the were the fence was removed from the global fence list. This led to a race with the debugfs support where it would iterate over sync_pts that had been freed.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 6739a84..2afbd69 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -520,12 +520,12 @@ static int sync_fence_release(struct inode *inode, struct file *file) struct sync_fence *fence = file->private_data; unsigned long flags;
- sync_fence_free_pts(fence); - spin_lock_irqsave(&sync_fence_list_lock, flags); list_del(&fence->sync_fence_list); spin_unlock_irqrestore(&sync_fence_list_lock, flags);
+ sync_fence_free_pts(fence); + kfree(fence);
return 0;
From: Erik Gilling konkers@android.com
If the two fences being merged contain sync_pts from the same timeline, those two pts will be collapsed into a single pt representing the latter of the two.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Whitespace fixes] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 52 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 2afbd69..6cb7c88 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -312,6 +312,56 @@ static int sync_fence_copy_pts(struct sync_fence *dst, struct sync_fence *src) return 0; }
+static int sync_fence_merge_pts(struct sync_fence *dst, struct sync_fence *src) +{ + struct list_head *src_pos, *dst_pos, *n; + + list_for_each(src_pos, &src->pt_list_head) { + struct sync_pt *src_pt = + container_of(src_pos, struct sync_pt, pt_list); + bool collapsed = false; + + list_for_each_safe(dst_pos, n, &dst->pt_list_head) { + struct sync_pt *dst_pt = + container_of(dst_pos, struct sync_pt, pt_list); + /* collapse two sync_pts on the same timeline + * to a single sync_pt that will signal at + * the later of the two + */ + if (dst_pt->parent == src_pt->parent) { + if (dst_pt->parent->ops->compare(dst_pt, src_pt) + == -1) { + struct sync_pt *new_pt = + sync_pt_dup(src_pt); + if (new_pt == NULL) + return -ENOMEM; + + new_pt->fence = dst; + list_replace(&dst_pt->pt_list, + &new_pt->pt_list); + sync_pt_activate(new_pt); + sync_pt_free(dst_pt); + } + collapsed = true; + break; + } + } + + if (!collapsed) { + struct sync_pt *new_pt = sync_pt_dup(src_pt); + + if (new_pt == NULL) + return -ENOMEM; + + new_pt->fence = dst; + list_add(&new_pt->pt_list, &dst->pt_list_head); + sync_pt_activate(new_pt); + } + } + + return 0; +} + static void sync_fence_free_pts(struct sync_fence *fence) { struct list_head *pos, *n; @@ -386,7 +436,7 @@ struct sync_fence *sync_fence_merge(const char *name, if (err < 0) goto err;
- err = sync_fence_copy_pts(fence, b); + err = sync_fence_merge_pts(fence, b); if (err < 0) goto err;
From: Erik Gilling konkers@android.com
If a fence is released while a timeline that one of it's pts is on is being signaled, it is possible for that fence to be deleted before it is signaled. This patch adds a refcount for internal references such as signaled pt processing.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 54 ++++++++++++++++++++++++++++++++++------ drivers/staging/android/sync.h | 5 ++++ 2 files changed, 51 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 6cb7c88..7d4e9aa 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -30,6 +30,7 @@
static void sync_fence_signal_pt(struct sync_pt *pt); static int _sync_pt_has_signaled(struct sync_pt *pt); +static void sync_fence_free(struct kref *kref);
static LIST_HEAD(sync_timeline_list_head); static DEFINE_SPINLOCK(sync_timeline_list_lock); @@ -113,7 +114,7 @@ static void sync_timeline_remove_pt(struct sync_pt *pt) { struct sync_timeline *obj = pt->parent; unsigned long flags; - bool needs_freeing; + bool needs_freeing = false;
spin_lock_irqsave(&obj->active_list_lock, flags); if (!list_empty(&pt->active_list)) @@ -121,8 +122,11 @@ static void sync_timeline_remove_pt(struct sync_pt *pt) spin_unlock_irqrestore(&obj->active_list_lock, flags);
spin_lock_irqsave(&obj->child_list_lock, flags); - list_del(&pt->child_list); - needs_freeing = obj->destroyed && list_empty(&obj->child_list_head); + if (!list_empty(&pt->child_list)) { + list_del_init(&pt->child_list); + needs_freeing = obj->destroyed && + list_empty(&obj->child_list_head); + } spin_unlock_irqrestore(&obj->child_list_lock, flags);
if (needs_freeing) @@ -141,18 +145,22 @@ void sync_timeline_signal(struct sync_timeline *obj) struct sync_pt *pt = container_of(pos, struct sync_pt, active_list);
- if (_sync_pt_has_signaled(pt)) - list_move(pos, &signaled_pts); + if (_sync_pt_has_signaled(pt)) { + list_del_init(pos); + list_add(&pt->signaled_list, &signaled_pts); + kref_get(&pt->fence->kref); + } }
spin_unlock_irqrestore(&obj->active_list_lock, flags);
list_for_each_safe(pos, n, &signaled_pts) { struct sync_pt *pt = - container_of(pos, struct sync_pt, active_list); + container_of(pos, struct sync_pt, signaled_list);
list_del_init(pos); sync_fence_signal_pt(pt); + kref_put(&pt->fence->kref, sync_fence_free); } } EXPORT_SYMBOL(sync_timeline_signal); @@ -253,6 +261,7 @@ static struct sync_fence *sync_fence_alloc(const char *name) if (fence->file == NULL) goto err;
+ kref_init(&fence->kref); strlcpy(fence->name, name, sizeof(fence->name));
INIT_LIST_HEAD(&fence->pt_list_head); @@ -362,6 +371,16 @@ static int sync_fence_merge_pts(struct sync_fence *dst, struct sync_fence *src) return 0; }
+static void sync_fence_detach_pts(struct sync_fence *fence) +{ + struct list_head *pos, *n; + + list_for_each_safe(pos, n, &fence->pt_list_head) { + struct sync_pt *pt = container_of(pos, struct sync_pt, pt_list); + sync_timeline_remove_pt(pt); + } +} + static void sync_fence_free_pts(struct sync_fence *fence) { struct list_head *pos, *n; @@ -565,18 +584,37 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) } EXPORT_SYMBOL(sync_fence_wait);
+static void sync_fence_free(struct kref *kref) +{ + struct sync_fence *fence = container_of(kref, struct sync_fence, kref); + + sync_fence_free_pts(fence); + + kfree(fence); +} + static int sync_fence_release(struct inode *inode, struct file *file) { struct sync_fence *fence = file->private_data; unsigned long flags;
+ /* + * We need to remove all ways to access this fence before droping + * our ref. + * + * start with its membership in the global fence list + */ spin_lock_irqsave(&sync_fence_list_lock, flags); list_del(&fence->sync_fence_list); spin_unlock_irqrestore(&sync_fence_list_lock, flags);
- sync_fence_free_pts(fence); + /* + * remove its pts from their parents so that sync_timeline_signal() + * can't reference the fence. + */ + sync_fence_detach_pts(fence);
- kfree(fence); + kref_put(&fence->kref, sync_fence_free);
return 0; } diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 943f414..00c9bae 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -16,6 +16,7 @@ #include <linux/types.h> #ifdef __KERNEL__
+#include <linux/kref.h> #include <linux/ktime.h> #include <linux/list.h> #include <linux/spinlock.h> @@ -109,6 +110,7 @@ struct sync_timeline { * @parent: sync_timeline to which this sync_pt belongs * @child_list: membership in sync_timeline.child_list_head * @active_list: membership in sync_timeline.active_list_head + * @signaled_list: membership in temorary signaled_list on stack * @fence: sync_fence to which the sync_pt belongs * @pt_list: membership in sync_fence.pt_list_head * @status: 1: signaled, 0:active, <0: error @@ -120,6 +122,7 @@ struct sync_pt { struct list_head child_list;
struct list_head active_list; + struct list_head signaled_list;
struct sync_fence *fence; struct list_head pt_list; @@ -133,6 +136,7 @@ struct sync_pt { /** * struct sync_fence - sync fence * @file: file representing this fence + * @kref: referenace count on fence. * @name: name of sync_fence. Useful for debugging * @pt_list_head: list of sync_pts in ths fence. immutable once fence * is created @@ -145,6 +149,7 @@ struct sync_pt { */ struct sync_fence { struct file *file; + struct kref kref; char name[32];
/* this list is immutable once the fence is created */
From: Erik Gilling konkers@android.com
If a timeline is destroyed while fences still hold pts on it, the reworked fence release handler can cause the timeline to be freed before all it's points are freed.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Squished in compiler warning fix] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 29 +++++++++++++---------------- drivers/staging/android/sync.h | 2 ++ 2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 7d4e9aa..61c27bd 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -51,6 +51,7 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, if (obj == NULL) return NULL;
+ kref_init(&obj->kref); obj->ops = ops; strlcpy(obj->name, name, sizeof(obj->name));
@@ -68,8 +69,10 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops, } EXPORT_SYMBOL(sync_timeline_create);
-static void sync_timeline_free(struct sync_timeline *obj) +static void sync_timeline_free(struct kref *kref) { + struct sync_timeline *obj = + container_of(kref, struct sync_timeline, kref); unsigned long flags;
if (obj->ops->release_obj) @@ -84,17 +87,14 @@ static void sync_timeline_free(struct sync_timeline *obj)
void sync_timeline_destroy(struct sync_timeline *obj) { - unsigned long flags; - bool needs_freeing; - - spin_lock_irqsave(&obj->child_list_lock, flags); obj->destroyed = true; - needs_freeing = list_empty(&obj->child_list_head); - spin_unlock_irqrestore(&obj->child_list_lock, flags);
- if (needs_freeing) - sync_timeline_free(obj); - else + /* + * If this is not the last reference, signal any children + * that their parent is going away. + */ + + if (!kref_put(&obj->kref, sync_timeline_free)) sync_timeline_signal(obj); } EXPORT_SYMBOL(sync_timeline_destroy); @@ -114,7 +114,6 @@ static void sync_timeline_remove_pt(struct sync_pt *pt) { struct sync_timeline *obj = pt->parent; unsigned long flags; - bool needs_freeing = false;
spin_lock_irqsave(&obj->active_list_lock, flags); if (!list_empty(&pt->active_list)) @@ -124,13 +123,8 @@ static void sync_timeline_remove_pt(struct sync_pt *pt) spin_lock_irqsave(&obj->child_list_lock, flags); if (!list_empty(&pt->child_list)) { list_del_init(&pt->child_list); - needs_freeing = obj->destroyed && - list_empty(&obj->child_list_head); } spin_unlock_irqrestore(&obj->child_list_lock, flags); - - if (needs_freeing) - sync_timeline_free(obj); }
void sync_timeline_signal(struct sync_timeline *obj) @@ -177,6 +171,7 @@ struct sync_pt *sync_pt_create(struct sync_timeline *parent, int size) return NULL;
INIT_LIST_HEAD(&pt->active_list); + kref_get(&parent->kref); sync_timeline_add_pt(parent, pt);
return pt; @@ -190,6 +185,8 @@ void sync_pt_free(struct sync_pt *pt)
sync_timeline_remove_pt(pt);
+ kref_put(&pt->parent->kref, sync_timeline_free); + kfree(pt); } EXPORT_SYMBOL(sync_pt_free); diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 00c9bae..15863a6 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -80,6 +80,7 @@ struct sync_timeline_ops {
/** * struct sync_timeline - sync object + * @kref: reference count on fence. * @ops: ops that define the implementaiton of the sync_timeline * @name: name of the sync_timeline. Useful for debugging * @destoryed: set when sync_timeline is destroyed @@ -90,6 +91,7 @@ struct sync_timeline_ops { * @sync_timeline_list: membership in global sync_timeline_list */ struct sync_timeline { + struct kref kref; const struct sync_timeline_ops *ops; char name[32];
From: Rebecca Schultz Zavin rebecca@android.com
Check the return value of get_unused_fd to make sure a valid file descriptor is returned.
Make sure to call put_unused_fd even if an error occurs before the fd can be used.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Rebecca Schultz Zavin rebecca@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 61c27bd..c4a3c1d 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -647,8 +647,13 @@ static long sync_fence_ioctl_merge(struct sync_fence *fence, unsigned long arg) struct sync_fence *fence2, *fence3; struct sync_merge_data data;
- if (copy_from_user(&data, (void __user *)arg, sizeof(data))) - return -EFAULT; + if (fd < 0) + return fd; + + if (copy_from_user(&data, (void __user *)arg, sizeof(data))) { + err = -EFAULT; + goto err_put_fd; + }
fence2 = sync_fence_fdget(data.fd2); if (fence2 == NULL) {
From: Rebecca Schultz Zavin rebecca@android.com
Check the return value of get_unused_fd to make sure a valid file descriptor is returned.
Make sure to call put_unused_fd even if an error occurs before the fd can be used.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Rebecca Schultz Zavin rebecca@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sw_sync.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index d689760..d768893 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -170,8 +170,13 @@ long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, unsigned long arg) struct sync_fence *fence; struct sw_sync_create_fence_data data;
- if (copy_from_user(&data, (void __user *)arg, sizeof(data))) - return -EFAULT; + if (fd < 0) + return fd; + + if (copy_from_user(&data, (void __user *)arg, sizeof(data))) { + err = -EFAULT; + goto err; + }
pt = sw_sync_pt_create(obj, data.value); if (pt == NULL) {
From: Erik Gilling konkers@android.com
Change wait timeout to act like poll
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Added commit message, squished typo-fix] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 6 +++--- drivers/staging/android/sync.h | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index c4a3c1d..7fccfcd 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -557,14 +557,14 @@ EXPORT_SYMBOL(sync_fence_cancel_async);
int sync_fence_wait(struct sync_fence *fence, long timeout) { - int err; + int err = 0;
- if (timeout) { + if (timeout > 0) { timeout = msecs_to_jiffies(timeout); err = wait_event_interruptible_timeout(fence->wq, fence->status != 0, timeout); - } else { + } else if (timeout < 0) { err = wait_event_interruptible(fence->wq, fence->status != 0); }
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 15863a6..75ed5f1 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -329,8 +329,8 @@ int sync_fence_cancel_async(struct sync_fence *fence, * @fence: fence to wait on * @tiemout: timeout in ms * - * Wait for @fence to be signaled or have an error. Waits indefintly - * if @timeout = 0 + * Wait for @fence to be signaled or have an error. Waits indefinitely + * if @timeout < 0 */ int sync_fence_wait(struct sync_fence *fence, long timeout);
@@ -389,9 +389,9 @@ struct sync_fence_info_data { /** * DOC: SYNC_IOC_WAIT - wait for a fence to signal * - * pass timeout in milliseconds. + * pass timeout in milliseconds. Waits indefinitely timeout < 0. */ -#define SYNC_IOC_WAIT _IOW(SYNC_IOC_MAGIC, 0, __u32) +#define SYNC_IOC_WAIT _IOW(SYNC_IOC_MAGIC, 0, __s32)
/** * DOC: SYNC_IOC_MERGE - merge two fences
From: Erik Gilling konkers@android.com
If we hit a timeout, dump sync state to console
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Add commit message, whitespace fixups] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 7fccfcd..d54fa8d 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -31,6 +31,7 @@ static void sync_fence_signal_pt(struct sync_pt *pt); static int _sync_pt_has_signaled(struct sync_pt *pt); static void sync_fence_free(struct kref *kref); +static void sync_dump(void);
static LIST_HEAD(sync_timeline_list_head); static DEFINE_SPINLOCK(sync_timeline_list_lock); @@ -574,8 +575,10 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) if (fence->status < 0) return fence->status;
- if (fence->status == 0) + if (fence->status == 0) { + sync_dump(); return -ETIME; + }
return 0; } @@ -914,7 +917,34 @@ static __init int sync_debugfs_init(void) debugfs_create_file("sync", S_IRUGO, NULL, NULL, &sync_debugfs_fops); return 0; } - late_initcall(sync_debugfs_init);
+#define DUMP_CHUNK 256 +static char sync_dump_buf[64 * 1024]; +void sync_dump(void) +{ + struct seq_file s = { + .buf = sync_dump_buf, + .size = sizeof(sync_dump_buf) - 1, + }; + int i; + + sync_debugfs_show(&s, NULL); + + for (i = 0; i < s.count; i += DUMP_CHUNK) { + if ((s.count - i) > DUMP_CHUNK) { + char c = s.buf[i + DUMP_CHUNK]; + s.buf[i + DUMP_CHUNK] = 0; + pr_cont("%s", s.buf + i); + s.buf[i + DUMP_CHUNK] = c; + } else { + s.buf[s.count] = 0; + pr_cont("%s", s.buf + i); + } + } +} +#else +static void sync_dump(void) +{ +} #endif
From: Erik Gilling konkers@android.com
Improve the output of the timeout dumps, including the fence pointer.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Added commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index d54fa8d..92cdfe8 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -576,6 +576,8 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) return fence->status;
if (fence->status == 0) { + pr_info("fence timeout on [%p] after %dms\n", fence, + jiffies_to_msecs(timeout)); sync_dump(); return -ETIME; } @@ -849,7 +851,8 @@ static void sync_print_fence(struct seq_file *s, struct sync_fence *fence) struct list_head *pos; unsigned long flags;
- seq_printf(s, "%s: %s\n", fence->name, sync_status_str(fence->status)); + seq_printf(s, "[%p] %s: %s\n", fence, fence->name, + sync_status_str(fence->status));
list_for_each(pos, &fence->pt_list_head) { struct sync_pt *pt =
From: Erik Gilling konkers@android.com
When we get a bad status, dump sync state
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Added commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 92cdfe8..36ffa20 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -572,8 +572,11 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) if (err < 0) return err;
- if (fence->status < 0) + if (fence->status < 0) { + pr_info("fence error %d on [%p]\n", fence->status, fence); + sync_dump(); return fence->status; + }
if (fence->status == 0) { pr_info("fence timeout on [%p] after %dms\n", fence,
From: Erik Gilling konkers@android.com
Fence status is checked outside of locks in both sync_fence_wait and sync_fence_poll. This patch adds propper barrier protection in these cases to avoid seeing stale status.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 36ffa20..2394189 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -556,6 +556,16 @@ int sync_fence_cancel_async(struct sync_fence *fence, } EXPORT_SYMBOL(sync_fence_cancel_async);
+static bool sync_fence_check(struct sync_fence *fence) +{ + /* + * Make sure that reads to fence->status are ordered with the + * wait queue event triggering + */ + smp_rmb(); + return fence->status != 0; +} + int sync_fence_wait(struct sync_fence *fence, long timeout) { int err = 0; @@ -563,7 +573,7 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) if (timeout > 0) { timeout = msecs_to_jiffies(timeout); err = wait_event_interruptible_timeout(fence->wq, - fence->status != 0, + sync_fence_check(fence), timeout); } else if (timeout < 0) { err = wait_event_interruptible(fence->wq, fence->status != 0); @@ -630,6 +640,12 @@ static unsigned int sync_fence_poll(struct file *file, poll_table *wait)
poll_wait(file, &fence->wq, wait);
+ /* + * Make sure that reads to fence->status are ordered with the + * wait queue event triggering + */ + smp_rmb(); + if (fence->status == 1) return POLLIN; else if (fence->status < 0)
From: Erik Gilling konkers@android.com
If a fence's pt is signaled before sync_fence_create is called, the fence will never transition into the signaled state. This also address a tiny race if a merged fence's pt after sync_fence_get_status checks it's status and before fence->status is updated.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 2394189..889ca6e 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -295,6 +295,12 @@ struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt) list_add(&pt->pt_list, &fence->pt_list_head); sync_pt_activate(pt);
+ /* + * signal the fence in case pt was activated before + * sync_pt_activate(pt) was called + */ + sync_fence_signal_pt(pt); + return fence; } EXPORT_SYMBOL(sync_fence_create); @@ -457,7 +463,13 @@ struct sync_fence *sync_fence_merge(const char *name, if (err < 0) goto err;
- fence->status = sync_fence_get_status(fence); + /* + * signal the fence in case one of it's pts were activated before + * they were activated + */ + sync_fence_signal_pt(list_first_entry(&fence->pt_list_head, + struct sync_pt, + pt_list));
return fence; err:
From: Erik Gilling konkers@android.com
The previous fix only addressed waiting with a timeout.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 889ca6e..811cf65 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -588,7 +588,8 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) sync_fence_check(fence), timeout); } else if (timeout < 0) { - err = wait_event_interruptible(fence->wq, fence->status != 0); + err = wait_event_interruptible(fence->wq, + sync_fence_check(fence)); }
if (err < 0)
From: Erik Gilling konkers@android.com
Move driver callbacks to fill strings instead of using seq_files. This will allow those values to be used in a future tracepoint patch.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 18 ++++++++++++++++-- drivers/staging/android/sync.h | 19 +++++++++++++------ 2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 811cf65..988f233 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -847,7 +847,17 @@ static void sync_print_pt(struct seq_file *s, struct sync_pt *pt, bool fence) seq_printf(s, "@%ld.%06ld", tv.tv_sec, tv.tv_usec); }
- if (pt->parent->ops->print_pt) { + if (pt->parent->ops->timeline_value_str && + pt->parent->ops->pt_value_str) { + char value[64]; + pt->parent->ops->pt_value_str(pt, value, sizeof(value)); + seq_printf(s, ": %s", value); + if (fence) { + pt->parent->ops->timeline_value_str(pt->parent, value, + sizeof(value)); + seq_printf(s, " / %s", value); + } + } else if (pt->parent->ops->print_pt) { seq_printf(s, ": "); pt->parent->ops->print_pt(s, pt); } @@ -862,7 +872,11 @@ static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj)
seq_printf(s, "%s %s", obj->name, obj->ops->driver_name);
- if (obj->ops->print_obj) { + if (obj->ops->timeline_value_str) { + char value[64]; + obj->ops->timeline_value_str(obj, value, sizeof(value)); + seq_printf(s, ": %s", value); + } else if (obj->ops->print_obj) { seq_printf(s, ": "); obj->ops->print_obj(s, obj); } diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 75ed5f1..38ea986 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -40,14 +40,14 @@ struct sync_fence; * -1 if a will signabl before b * @free_pt: called before sync_pt is freed * @release_obj: called before sync_timeline is freed - * @print_obj: print aditional debug information about sync_timeline. - * should not print a newline - * @print_pt: print aditional debug information about sync_pt. - * should not print a newline + * @print_obj: deprecated + * @print_pt: deprecated * @fill_driver_data: write implmentation specific driver data to data. * should return an error if there is not enough room * as specified by size. This information is returned * to userspace by SYNC_IOC_FENCE_INFO. + * @timeline_value_str: fill str with the value of the sync_timeline's counter + * @pt_value_str: fill str with the value of the sync_pt */ struct sync_timeline_ops { const char *driver_name; @@ -67,15 +67,22 @@ struct sync_timeline_ops { /* optional */ void (*release_obj)(struct sync_timeline *sync_timeline);
- /* optional */ + /* deprecated */ void (*print_obj)(struct seq_file *s, struct sync_timeline *sync_timeline);
- /* optional */ + /* deprecated */ void (*print_pt)(struct seq_file *s, struct sync_pt *sync_pt);
/* optional */ int (*fill_driver_data)(struct sync_pt *syncpt, void *data, int size); + + /* optional */ + void (*timeline_value_str)(struct sync_timeline *timeline, char *str, + int size); + + /* optional */ + void (*pt_value_str)(struct sync_pt *pt, char *str, int size); };
/**
From: Erik Gilling konkers@android.com
Switch from print_obj/print_pt to the new timeline_value_str and pt_value_str ops.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Add commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sw_sync.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index d768893..68025a5 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -72,23 +72,6 @@ static int sw_sync_pt_compare(struct sync_pt *a, struct sync_pt *b) return sw_sync_cmp(pt_a->value, pt_b->value); }
-static void sw_sync_print_obj(struct seq_file *s, - struct sync_timeline *sync_timeline) -{ - struct sw_sync_timeline *obj = (struct sw_sync_timeline *)sync_timeline; - - seq_printf(s, "%d", obj->value); -} - -static void sw_sync_print_pt(struct seq_file *s, struct sync_pt *sync_pt) -{ - struct sw_sync_pt *pt = (struct sw_sync_pt *)sync_pt; - struct sw_sync_timeline *obj = - (struct sw_sync_timeline *)sync_pt->parent; - - seq_printf(s, "%d / %d", pt->value, obj->value); -} - static int sw_sync_fill_driver_data(struct sync_pt *sync_pt, void *data, int size) { @@ -102,14 +85,29 @@ static int sw_sync_fill_driver_data(struct sync_pt *sync_pt, return sizeof(pt->value); }
+static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline, + char *str, int size) +{ + struct sw_sync_timeline *timeline = + (struct sw_sync_timeline *)sync_timeline; + snprintf(str, size, "%d", timeline->value); +} + +static void sw_sync_pt_value_str(struct sync_pt *sync_pt, + char *str, int size) +{ + struct sw_sync_pt *pt = (struct sw_sync_pt *)sync_pt; + snprintf(str, size, "%d", pt->value); +} + struct sync_timeline_ops sw_sync_timeline_ops = { .driver_name = "sw_sync", .dup = sw_sync_pt_dup, .has_signaled = sw_sync_pt_has_signaled, .compare = sw_sync_pt_compare, - .print_obj = sw_sync_print_obj, - .print_pt = sw_sync_print_pt, .fill_driver_data = sw_sync_fill_driver_data, + .timeline_value_str = sw_sync_timeline_value_str, + .pt_value_str = sw_sync_pt_value_str, };
From: Erik Gilling konkers@android.com
Add support for tracepoints
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Whitespace changes, add commit message, move to staging] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 11 +++++ drivers/staging/android/trace/sync.h | 82 ++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 drivers/staging/android/trace/sync.h
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 988f233..1ddc404 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -28,6 +28,9 @@
#include "sync.h"
+#define CREATE_TRACE_POINTS +#include "trace/sync.h" + static void sync_fence_signal_pt(struct sync_pt *pt); static int _sync_pt_has_signaled(struct sync_pt *pt); static void sync_fence_free(struct kref *kref); @@ -134,6 +137,8 @@ void sync_timeline_signal(struct sync_timeline *obj) LIST_HEAD(signaled_pts); struct list_head *pos, *n;
+ trace_sync_timeline(obj); + spin_lock_irqsave(&obj->active_list_lock, flags);
list_for_each_safe(pos, n, &obj->active_list_head) { @@ -581,6 +586,11 @@ static bool sync_fence_check(struct sync_fence *fence) int sync_fence_wait(struct sync_fence *fence, long timeout) { int err = 0; + struct sync_pt *pt; + + trace_sync_wait(fence, 1); + list_for_each_entry(pt, &fence->pt_list_head, pt_list) + trace_sync_pt(pt);
if (timeout > 0) { timeout = msecs_to_jiffies(timeout); @@ -591,6 +601,7 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) err = wait_event_interruptible(fence->wq, sync_fence_check(fence)); } + trace_sync_wait(fence, 0);
if (err < 0) return err; diff --git a/drivers/staging/android/trace/sync.h b/drivers/staging/android/trace/sync.h new file mode 100644 index 0000000..9546235 --- /dev/null +++ b/drivers/staging/android/trace/sync.h @@ -0,0 +1,82 @@ +#undef TRACE_SYSTEM +#define TRACE_INCLUDE_PATH ../../drivers/staging/android/trace +#define TRACE_SYSTEM sync + +#if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_SYNC_H + +#include "../sync.h" +#include <linux/tracepoint.h> + +TRACE_EVENT(sync_timeline, + TP_PROTO(struct sync_timeline *timeline), + + TP_ARGS(timeline), + + TP_STRUCT__entry( + __string(name, timeline->name) + __array(char, value, 32) + ), + + TP_fast_assign( + __assign_str(name, timeline->name); + if (timeline->ops->timeline_value_str) { + timeline->ops->timeline_value_str(timeline, + __entry->value, + sizeof(__entry->value)); + } else { + __entry->value[0] = '\0'; + } + ), + + TP_printk("name=%s value=%s", __get_str(name), __entry->value) +); + +TRACE_EVENT(sync_wait, + TP_PROTO(struct sync_fence *fence, int begin), + + TP_ARGS(fence, begin), + + TP_STRUCT__entry( + __string(name, fence->name) + __field(s32, status) + __field(u32, begin) + ), + + TP_fast_assign( + __assign_str(name, fence->name); + __entry->status = fence->status; + __entry->begin = begin; + ), + + TP_printk("%s name=%s state=%d", __entry->begin ? "begin" : "end", + __get_str(name), __entry->status) +); + +TRACE_EVENT(sync_pt, + TP_PROTO(struct sync_pt *pt), + + TP_ARGS(pt), + + TP_STRUCT__entry( + __string(timeline, pt->parent->name) + __array(char, value, 32) + ), + + TP_fast_assign( + __assign_str(timeline, pt->parent->name); + if (pt->parent->ops->pt_value_str) { + pt->parent->ops->pt_value_str(pt, __entry->value, + sizeof(__entry->value)); + } else { + __entry->value[0] = '\0'; + } + ), + + TP_printk("name=%s value=%s", __get_str(timeline), __entry->value) +); + +#endif /* if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) */ + +/* This part must be outside protection */ +#include <trace/define_trace.h>
From: Ørjan Eide orjan.eide@arm.com
The copied sync_pt was activated immediately. If the sync_pt was signaled before the entire merge was completed, the new fence's pt_list could be iterated over while it is still in the process of being created.
Moving the the sync_pt_activate call for all new sync_pts to after both the sync_fence_copy_pts and the sync_fence_merge_pts calls ensure that the pt_list is complete and immutable before it can be reached from the timeline's active list.
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 1ddc404..bd18c75 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -324,7 +324,6 @@ static int sync_fence_copy_pts(struct sync_fence *dst, struct sync_fence *src)
new_pt->fence = dst; list_add(&new_pt->pt_list, &dst->pt_list_head); - sync_pt_activate(new_pt); }
return 0; @@ -357,7 +356,6 @@ static int sync_fence_merge_pts(struct sync_fence *dst, struct sync_fence *src) new_pt->fence = dst; list_replace(&dst_pt->pt_list, &new_pt->pt_list); - sync_pt_activate(new_pt); sync_pt_free(dst_pt); } collapsed = true; @@ -373,7 +371,6 @@ static int sync_fence_merge_pts(struct sync_fence *dst, struct sync_fence *src)
new_pt->fence = dst; list_add(&new_pt->pt_list, &dst->pt_list_head); - sync_pt_activate(new_pt); } }
@@ -454,6 +451,7 @@ struct sync_fence *sync_fence_merge(const char *name, struct sync_fence *a, struct sync_fence *b) { struct sync_fence *fence; + struct list_head *pos; int err;
fence = sync_fence_alloc(name); @@ -468,6 +466,12 @@ struct sync_fence *sync_fence_merge(const char *name, if (err < 0) goto err;
+ list_for_each(pos, &fence->pt_list_head) { + struct sync_pt *pt = + container_of(pos, struct sync_pt, pt_list); + sync_pt_activate(pt); + } + /* * signal the fence in case one of it's pts were activated before * they were activated
From: Erik Gilling konkers@android.com
If the timeout is zero, don't trip the timeout debugging
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Erik Gilling konkers@android.com [jstultz: Added commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index bd18c75..9b8b0e9 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -616,7 +616,7 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) return fence->status; }
- if (fence->status == 0) { + if (fence->status == 0 && timeout > 0) { pr_info("fence timeout on [%p] after %dms\n", fence, jiffies_to_msecs(timeout)); sync_dump();
From: Jamie Gennis jgennis@google.com
Fix wait behavior on timeout == 0 case
Cc: Maarten Lankhorst maarten.lankhorst@canonical.com Cc: Erik Gilling konkers@android.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Rob Clark robclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Greg KH gregkh@linuxfoundation.org Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team kernel-team@android.com Signed-off-by: Jamie Gennis jgennis@google.com [jstultz: Added commit message] Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/staging/android/sync.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 9b8b0e9..b9bb974 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -616,10 +616,12 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) return fence->status; }
- if (fence->status == 0 && timeout > 0) { - pr_info("fence timeout on [%p] after %dms\n", fence, - jiffies_to_msecs(timeout)); - sync_dump(); + if (fence->status == 0) { + if (timeout > 0) { + pr_info("fence timeout on [%p] after %dms\n", fence, + jiffies_to_msecs(timeout)); + sync_dump(); + } return -ETIME; }
On Thu, Feb 28, 2013 at 04:42:56PM -0800, John Stultz wrote:
As proposed yesterday, here's the Android sync driver patches for staging.
I've preserved the commit history, but moved all the changes over to be against the staging directory (instead of drivers/base).
The goal of submitting this driver to staging is to try to get more collaberation, as there are some similar efforts going on in the community with dmabuf-fences. My email from yesterday with more details for how I hope this goes is here: http://comments.gmane.org/gmane.linux.kernel/1448420
Erik also provided a nice background on the patch set in his reply yesterday, which I'll quote here:
<snip>
Mind if I put that in the 1/30 changelog body for future people to see?
Other than that, at first glance, I only have one minor question, which I'll make in the patch itself. Otherwise, if there are no objections, I'll queue these up in my tree after 3.9-rc1 is out.
thanks for doing this work,
greg k-h
On Thu, Feb 28, 2013 at 5:59 PM, Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Feb 28, 2013 at 04:42:56PM -0800, John Stultz wrote:
Erik also provided a nice background on the patch set in his reply yesterday, which I'll quote here:
Mind if I put that in the 1/30 changelog body for future people to see?
Please do.
Other than that, at first glance, I only have one minor question, which I'll make in the patch itself. Otherwise, if there are no objections, I'll queue these up in my tree after 3.9-rc1 is out.
thanks for doing this work,
Indeed, thanks John
Cheers, Erik
On Fri, Mar 1, 2013 at 1:42 AM, John Stultz john.stultz@linaro.org wrote:
As proposed yesterday, here's the Android sync driver patches for staging.
I've preserved the commit history, but moved all the changes over to be against the staging directory (instead of drivers/base).
The goal of submitting this driver to staging is to try to get more collaberation, as there are some similar efforts going on in the community with dmabuf-fences. My email from yesterday with more details for how I hope this goes is here: http://comments.gmane.org/gmane.linux.kernel/1448420
I've been offline in a week of snowboarding, but I'll throw my late Ack - I've discussed this a bit with John offline and I agree with his general plan for integrating android sync points into mainline.
Erik also provided a nice background on the patch set in his reply yesterday, which I'll quote here:
"In Honeycomb where we introduced the Hardware Composer HAL. This is a userspace layer that allows composition acceleration on a per platform basis. Different SoC vendors have implemented this using overlays, 2d blitters, a combinations of both, or other clever/disgusting means. Along with the HWC we consolidated a lot of our camera and media pipeline to allow their input to be fed into the GPU or display(overlay.) In order to exploit parallelism the the graphics pipeline, this introduced lots of implicit synchronization dependancies. After a couple years of working with many different SoC vendors, we found that it was really difficult to communicate our system's expectations of the implicit contract and it was difficult for the SoC vendors to properly implement the implicit contract in each of their IP blocks (display, gpu, camera, video codecs). It was also incredibly difficult to debug when problems/deadlocks arose.
dma_buf fences should be tons easier to debug thanks to integration with lockdep. Also their design fundamentally excludes deadlock-loops in the fences themselves. And I also think that we should be able to hide the complexity from most drivers in e.g. drm/ttm or the v2l core. So I'm still bullish on implicit fencing (and will keep on pushing that for all things intel).
But I guess the simpler programming model afforded by that for userspace isn't of much use for the google guys now that they've pushed the effort to convert SurfaceFlinger to explicit fence handling ...
Cheers, Daniel
dri-devel@lists.freedesktop.org