On Mon, Feb 17, 2014 at 04:57:19PM +0100, Maarten Lankhorst wrote:
Android syncpoints can be mapped to a timeline. This removes the need to maintain a separate api for synchronization. I've left the android trace events in place, but the core fence events should already be sufficient for debugging.
v2:
- Call fence_remove_callback in sync_fence_free if not all fences have fired.
v3:
- Merge Colin Cross' bugfixes, and the android fence merge optimization.
v4:
- Merge with the upstream fixes.
Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com
Snipped everything but headers - Ian Lister from our android team is signed up to have a more in-depth look at proper integration with android syncpoints. Adding him to cc.
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 62e2255b1c1e..6036dbdc8e6f 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -21,6 +21,7 @@ #include <linux/list.h> #include <linux/spinlock.h> #include <linux/wait.h> +#include <linux/fence.h>
struct sync_timeline; struct sync_pt; @@ -40,8 +41,6 @@ struct sync_fence;
- -1 if a will signal before b
- @free_pt: called before sync_pt is freed
- @release_obj: called before sync_timeline is freed
- @print_obj: deprecated
- @print_pt: deprecated
- @fill_driver_data: write implementation specific driver data to data.
- should return an error if there is not enough room
- as specified by size. This information is returned
@@ -67,13 +66,6 @@ struct sync_timeline_ops { /* optional */ void (*release_obj)(struct sync_timeline *sync_timeline);
- /* deprecated */
- void (*print_obj)(struct seq_file *s,
- struct sync_timeline *sync_timeline);
- /* 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);
@@ -104,42 +96,48 @@ struct sync_timeline {
/* protected by child_list_lock */ bool destroyed;
int context, value;
struct list_head child_list_head; spinlock_t child_list_lock;
struct list_head active_list_head;
- spinlock_t active_list_lock;
+#ifdef CONFIG_DEBUG_FS struct list_head sync_timeline_list; +#endif };
/**
- struct sync_pt - sync point
- @parent: sync_timeline to which this sync_pt belongs
- @fence: base fence class
- @child_list: membership in sync_timeline.child_list_head
- @active_list: membership in sync_timeline.active_list_head
+<<<<<<< current
- @signaled_list: membership in temporary 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
- @timestamp: time which sync_pt status transitioned from active to
- signaled or error.
+======= +>>>>>>> patched
Conflict markers ...
*/ struct sync_pt {
- struct sync_timeline *parent;
- struct list_head child_list;
- struct fence base;
Hm, embedding feels wrong, since that still means that I'll need to implement two kinds of fences in i915 - one using the seqno fence to make dma-buf sync work, and one to implmenent sync_pt to make the android folks happy.
If I can dream I think we should have a pointer to an underlying fence here, i.e. a struct sync_pt would just be a userspace interface wrapper to do explicit syncing using native fences, instead of implicit syncing like with dma-bufs. But this is all drive-by comments from a very cursory high-level look. I might be full of myself again ;-) -Daniel
- struct list_head child_list; struct list_head active_list;
- struct list_head signaled_list;
- struct sync_fence *fence;
- struct list_head pt_list;
+};
- /* protected by parent->active_list_lock */
- int status;
+static inline struct sync_timeline *sync_pt_parent(struct sync_pt *pt) {
- return container_of(pt->base.lock, struct sync_timeline, child_list_lock);
+}
- ktime_t timestamp;
+struct sync_fence_cb {
- struct fence_cb cb;
- struct fence *sync_pt;
- struct sync_fence *fence;
};
/** @@ -149,9 +147,7 @@ struct sync_pt {
- @name: name of sync_fence. Useful for debugging
- @pt_list_head: list of sync_pts in the 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
- @status: 0: signaled, >0:active, <0: error
- @wq: wait queue for fence signaling
- @sync_fence_list: membership in global fence list
@@ -160,17 +156,15 @@ struct sync_fence { struct file *file; struct kref kref; 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;
+#ifdef CONFIG_DEBUG_FS
- struct list_head sync_fence_list;
+#endif
int num_fences;
wait_queue_head_t wq;
atomic_t status;
- struct list_head sync_fence_list;
- struct sync_fence_cb cbs[];
};
struct sync_fence_waiter; @@ -184,14 +178,14 @@ typedef void (*sync_callback_t)(struct sync_fence *fence,
- @callback_data: pointer to pass to @callback
*/ struct sync_fence_waiter {
- struct list_head waiter_list;
- sync_callback_t callback;
- wait_queue_t work;
- sync_callback_t callback;
};
static inline void sync_fence_waiter_init(struct sync_fence_waiter *waiter, sync_callback_t callback) {
- INIT_LIST_HEAD(&waiter->work.task_list); waiter->callback = callback;
}
@@ -423,4 +417,22 @@ struct sync_fence_info_data { #define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2,\ struct sync_fence_info_data)
+#ifdef CONFIG_DEBUG_FS
+extern void sync_timeline_debug_add(struct sync_timeline *obj); +extern void sync_timeline_debug_remove(struct sync_timeline *obj); +extern void sync_fence_debug_add(struct sync_fence *fence); +extern void sync_fence_debug_remove(struct sync_fence *fence); +extern void sync_dump(void);
+#else +# define sync_timeline_debug_add(obj) +# define sync_timeline_debug_remove(obj) +# define sync_fence_debug_add(fence) +# define sync_fence_debug_remove(fence) +# define sync_dump() +#endif +int sync_fence_wake_up_wq(wait_queue_t *curr, unsigned mode,
- int wake_flags, void *key);
#endif /* _LINUX_SYNC_H */