Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-03-04 Thread Daniel Vetter 
On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:
 op 03-03-14 22:11, Daniel Vetter schreef:
 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.lankho...@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 ...
 Oops.
*/
   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
 
 No, the idea is that because android syncpoint is simply another type of
 dma-fence, that if you deal with normal fences then android can
 automatically be handled too. The userspace fence api android exposes
 could be very easily made to work for dma-fence, just pass a dma-fence
 to sync_fence_create.
 So exposing dma-fence would probably work for android too.

Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-03-04 Thread Maarten Lankhorst 

op 04-03-14 09:14, Daniel Vetter schreef:

On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:

op 03-03-14 22:11, Daniel Vetter schreef:

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.lankho...@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 ...

Oops.

   */
  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


No, the idea is that because android syncpoint is simply another type of
dma-fence, that if you deal with normal fences then android can
automatically be handled too. The userspace fence api android exposes
could be very easily made to work for dma-fence, just pass a dma-fence
to sync_fence_create.
So exposing dma-fence would probably work for android too.

Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel

To keep compatibility with the android api. I think that gradually converting 
them is going to be
more useful than to force all drivers to use a new api all at once. They could 
keep android
syncpoint api for exporting, as long as they accept dma-fence for 
importing/waiting.

~Maarten
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-03-04 Thread Daniel Vetter 
On Tue, Mar 04, 2014 at 09:20:58AM +0100, Maarten Lankhorst wrote:
 op 04-03-14 09:14, Daniel Vetter schreef:
 On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:
 op 03-03-14 22:11, Daniel Vetter schreef:
 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.lankho...@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 ...
 Oops.
*/
   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
 
 No, the idea is that because android syncpoint is simply another type of
 dma-fence, that if you deal with normal fences then android can
 automatically be handled too. The userspace fence api android exposes
 could be very easily made to work for dma-fence, just pass a dma-fence
 to sync_fence_create.
 So exposing dma-fence would probably work for android too.
 Hm, then why do we still have struct sync_pt around? Since it's just the
 internal bit, with the userspace facing object being struct sync_fence,
 I'd opt to shuffle any useful features into the core struct fence.
 -Daniel
 To keep compatibility with the android api. I think that gradually converting 
 them is going to be
 more useful than to force all drivers to use a new api all at once. They 
 could keep android
 syncpoint api for exporting, as long as they accept dma-fence for 
 importing/waiting.

We don't have any users of the android sync_pt stuff (outside of the
framework itself). So any considerations for existing drivers for
upstreaming are imo moot. At least for the in-kernel interfaces used. For
the actual

Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-03-04 Thread Maarten Lankhorst 

op 04-03-14 11:00, Daniel Vetter schreef:

On Tue, Mar 04, 2014 at 09:20:58AM +0100, Maarten Lankhorst wrote:

op 04-03-14 09:14, Daniel Vetter schreef:

On Tue, Mar 04, 2014 at 08:50:38AM +0100, Maarten Lankhorst wrote:

op 03-03-14 22:11, Daniel Vetter schreef:

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.lankho...@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 ...

Oops.

   */
  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


No, the idea is that because android syncpoint is simply another type of
dma-fence, that if you deal with normal fences then android can
automatically be handled too. The userspace fence api android exposes
could be very easily made to work for dma-fence, just pass a dma-fence
to sync_fence_create.
So exposing dma-fence would probably work for android too.

Hm, then why do we still have struct sync_pt around? Since it's just the
internal bit, with the userspace facing object being struct sync_fence,
I'd opt to shuffle any useful features into the core struct fence.
-Daniel

To keep compatibility with the android api. I think that gradually converting 
them is going to be
more useful than to force all drivers to use a new api all at once. They could 
keep android
syncpoint api for exporting, as long as they accept dma-fence for 
importing/waiting.

We don't have any users of the android sync_pt stuff (outside of the
framework itself). So any considerations for existing drivers for
upstreaming are imo moot. At least for the in-kernel interfaces used. For
the actual userspace interface I guess keeping the android syncpt ioctls
as-is has value,

Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-03-03 Thread Daniel Vetter 
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.lankho...@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

Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-03-03 Thread Maarten Lankhorst 

op 03-03-14 22:11, Daniel Vetter schreef:

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.lankho...@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 ...

Oops.

   */
  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


No, the idea is that because android syncpoint is simply another type of 
dma-fence, that if you deal with normal fences then android can automatically
be handled too. The userspace fence api android exposes could be very easily 
made to work for dma-fence, just pass a dma-fence to sync_fence_create.
So exposing dma-fence would probably work for android too.

~Maarten
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-02-24 Thread Maarten Lankhorst 

op 19-02-14 14:56, Thomas Hellstrom schreef:

+static void fence_check_cb_func(struct fence *f, struct fence_cb *cb)
+{
+   struct sync_fence_cb *check = container_of(cb, struct sync_fence_cb, cb);
+   struct sync_fence *fence = check-fence;
+
+   // TODO: Add a fence-status member and check it

Hmm, C++ / C99 style comments makes checkpatch.pl complain. Did you run
this series through checkpatch?

/Thomas


Actually I used c99 here because it shouldn't have been in the sent patch. ;-)

Right below that comment I use fence-status, so the right thing to do was to 
zap the comment.

Thanks for catching it!

~Maarten\
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] android: convert sync to fence api, v4

2014-02-19 Thread Thomas Hellstrom 
On 02/17/2014 04:57 PM, 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.lankho...@canonical.com
 ---
  drivers/staging/android/Kconfig  |1 
  drivers/staging/android/Makefile |2 
  drivers/staging/android/sw_sync.c|4 
  drivers/staging/android/sync.c   |  892 
 +++---
  drivers/staging/android/sync.h   |   80 ++-
  drivers/staging/android/sync_debug.c |  245 +
  drivers/staging/android/trace/sync.h |   12 
  7 files changed, 592 insertions(+), 644 deletions(-)
  create mode 100644 drivers/staging/android/sync_debug.c

 diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
 index b91c758883bf..ecc8194242b5 100644
 --- a/drivers/staging/android/Kconfig
 +++ b/drivers/staging/android/Kconfig
 @@ -77,6 +77,7 @@ config SYNC
   bool Synchronization framework
   default n
   select ANON_INODES
 + select DMA_SHARED_BUFFER
   ---help---
 This option enables the framework for synchronization between multiple
 drivers.  Sync implementations can take advantage of hardware
 diff --git a/drivers/staging/android/Makefile 
 b/drivers/staging/android/Makefile
 index 0a01e1914905..517ad5ffa429 100644
 --- a/drivers/staging/android/Makefile
 +++ b/drivers/staging/android/Makefile
 @@ -9,5 +9,5 @@ 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
 +obj-$(CONFIG_SYNC)   += sync.o sync_debug.o
  obj-$(CONFIG_SW_SYNC)+= sw_sync.o
 diff --git a/drivers/staging/android/sw_sync.c 
 b/drivers/staging/android/sw_sync.c
 index f24493ac65e3..a76db3ff87cb 100644
 --- a/drivers/staging/android/sw_sync.c
 +++ b/drivers/staging/android/sw_sync.c
 @@ -50,7 +50,7 @@ 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;
 + (struct sw_sync_timeline *)sync_pt_parent(sync_pt);
  
   return (struct sync_pt *) sw_sync_pt_create(obj, pt-value);
  }
 @@ -59,7 +59,7 @@ 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;
 + (struct sw_sync_timeline *)sync_pt_parent(sync_pt);
  
   return sw_sync_cmp(obj-value, pt-value) = 0;
  }
 diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
 index 3d05f662110b..8e77cd73b739 100644
 --- a/drivers/staging/android/sync.c
 +++ b/drivers/staging/android/sync.c
 @@ -31,22 +31,13 @@
  #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);
 -static void sync_dump(void);
 -
 -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);
 +static const struct fence_ops android_fence_ops;
 +static const struct file_operations sync_fence_fops;
  
  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;
 @@ -57,17 +48,14 @@ struct sync_timeline *sync_timeline_create(const struct 
 sync_timeline_ops *ops,
  
   kref_init(obj-kref);
   obj-ops = ops;
 + obj-context = fence_context_alloc(1);
   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);
 + spin_lock_init(obj-child_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);
 + sync_timeline_debug_add(obj);
  
   return obj;
  }
 @@ -77,11 +65,8 @@ static void

[PATCH 4/6] android: convert sync to fence api, v4

2014-02-17 Thread Maarten Lankhorst 
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.lankho...@canonical.com
---
 drivers/staging/android/Kconfig  |1 
 drivers/staging/android/Makefile |2 
 drivers/staging/android/sw_sync.c|4 
 drivers/staging/android/sync.c   |  892 +++---
 drivers/staging/android/sync.h   |   80 ++-
 drivers/staging/android/sync_debug.c |  245 +
 drivers/staging/android/trace/sync.h |   12 
 7 files changed, 592 insertions(+), 644 deletions(-)
 create mode 100644 drivers/staging/android/sync_debug.c

diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
index b91c758883bf..ecc8194242b5 100644
--- a/drivers/staging/android/Kconfig
+++ b/drivers/staging/android/Kconfig
@@ -77,6 +77,7 @@ config SYNC
bool Synchronization framework
default n
select ANON_INODES
+   select DMA_SHARED_BUFFER
---help---
  This option enables the framework for synchronization between multiple
  drivers.  Sync implementations can take advantage of hardware
diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile
index 0a01e1914905..517ad5ffa429 100644
--- a/drivers/staging/android/Makefile
+++ b/drivers/staging/android/Makefile
@@ -9,5 +9,5 @@ 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
+obj-$(CONFIG_SYNC) += sync.o sync_debug.o
 obj-$(CONFIG_SW_SYNC)  += sw_sync.o
diff --git a/drivers/staging/android/sw_sync.c 
b/drivers/staging/android/sw_sync.c
index f24493ac65e3..a76db3ff87cb 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -50,7 +50,7 @@ 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;
+   (struct sw_sync_timeline *)sync_pt_parent(sync_pt);
 
return (struct sync_pt *) sw_sync_pt_create(obj, pt-value);
 }
@@ -59,7 +59,7 @@ 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;
+   (struct sw_sync_timeline *)sync_pt_parent(sync_pt);
 
return sw_sync_cmp(obj-value, pt-value) = 0;
 }
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 3d05f662110b..8e77cd73b739 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -31,22 +31,13 @@
 #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);
-static void sync_dump(void);
-
-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);
+static const struct fence_ops android_fence_ops;
+static const struct file_operations sync_fence_fops;
 
 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;
@@ -57,17 +48,14 @@ struct sync_timeline *sync_timeline_create(const struct 
sync_timeline_ops *ops,
 
kref_init(obj-kref);
obj-ops = ops;
+   obj-context = fence_context_alloc(1);
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);
+   spin_lock_init(obj-child_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);
+   sync_timeline_debug_add(obj);
 
return obj;
 }
@@ -77,11 +65,8 @@ static void sync_timeline_free(struct kref *kref)
 {
struct sync_timeline *obj =