Re: [PATCH 4/6] android: convert sync to fence api, v4
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
---
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
#include
#include
+#include
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, at least if we conclude that their not badly broken. In
which case we
Re: [PATCH 4/6] android: convert sync to fence api, v4
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
> ---
> >>>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
> #include
> #include
> +#include
>
> 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 and
Re: [PATCH 4/6] android: convert sync to fence api, v4
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
---
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
#include
#include
+#include
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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] android: convert sync to fence api, v4
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
> >>---
> >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
> >> #include
> >> #include
> >>+#include
> >>
> >> 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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] android: convert sync to fence api, v4
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
---
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
#include
#include
+#include
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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] android: convert sync to fence api, v4
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
> ---
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
> #include
> #include
> +#include
>
> 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 sta
Re: [PATCH 4/6] android: convert sync to fence api, v4
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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/6] android: convert sync to fence api, v4
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
> ---
> 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_
