Re: [RFC 00/29] De-stage android's sync framework

[Gustavo Padovan <gustavo@padovan.org>]

2016-01-20 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>:

> Hey,
> 
> Op 15-01-16 om 15:55 schreef Gustavo Padovan:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > This patch series de-stage the sync framework, and in order to accomplish that
> > a bunch of cleanups/improvements on the sync and fence were made.
> >
> > The sync framework contained some abstractions around struct fence and those
> > were removed in the de-staging process among other changes:
> >
> > Userspace visible changes
> > -------------------------
> >
> >  * The sw_sync file was moved from /dev/sw_sync to <debugfs>/sync/sw_sync. No
> >  other change.
> >
> > Kernel API changes
> > ------------------
> >
> >  * struct sync_timeline is now struct fence_timeline
> >  * sync_timeline_ops is now fence_timeline_ops and they now carry struct
> >  fence as parameter instead of struct sync_pt
> >  * a .cleanup() fence op was added to allow sync_fence to run a cleanup when
> >  the fence_timeline is destroyed
> >  * added fence_add_used_data() to pass a private point to struct fence. This
> >  pointer is sent back on the .cleanup op.
> >  * The sync timeline function were moved to be fence_timeline functions:
> > 	 - sync_timeline_create()	-> fence_timeline_create()
> > 	 - sync_timeline_get()		-> fence_timeline_get()
> > 	 - sync_timeline_put()		-> fence_timeline_put()
> > 	 - sync_timeline_destroy()	-> fence_timeline_destroy()
> > 	 - sync_timeline_signal()	-> fence_timeline_signal()
> >
> >   * sync_pt_create() was replaced be fence_create_on_timeline()
> >
> > Internal changes
> > ----------------
> >
> >  * fence_timeline_ops was removed in favor of direct use fence_ops
> >  * fence default functions were created for fence_ops
> >  * removed structs sync_pt, sw_sync_timeline and sw_sync_pt
> >
> > Gustavo Padovan (29):
> >   staging/android: fix sync framework documentation
> >   staging/android: fix checkpatch warning
> >   staging/android: rename sync_fence_release
> >   staging/android: rename 'android_fence' to 'sync_fence'
> >   staging/android: remove not used sync_timeline ops
> >   staging/android: create a 'sync' dir for debugfs information
> >   staging/android: move sw_sync file to debugfs file
> >   staging/android: Remove WARN_ON_ONCE when releasing sync_fence
> >   staging/android: rename struct sync_fence's variables to 'sync_fence'
> >   staging/android: rename 'sync_pt' to 'fence' in struct sync_fence_cb
> >   dma-buf/fence: move sync_timeline to fence_timeline
> >   staging/android: remove struct sync_pt
> >   dma-buf/fence: create fence_default_enable_signaling()
> >   dma-buf/fence: create fence_default_release()
> >   dma-buf/fence: create fence_default_get_driver_name()
> >   dma-buf/fence: create fence_default_timeline_name()
> This is misleading. I think timeline_fence prefix would be more appropriate here.

Why? These fence_default_.. functions are fence_ops and not related to
fence_timeline in any way.

> 
> I also believe this should be done in multiple series. First series should de-stage the userspace fence framework. The next series should fix up android_fence and maybe rename it to timeline_fence since sync_fence is already used for the userspace fd, which would add more confusion?

Sure. I've been thinking on how to split this properly. I'm trying to
add a bunch of clean up/renaming first, eg the sync_fence rename to
sync_file that Daniel Vetter and I discussed. 

Next my plan would be move sync_timeline to fence_timeline, add the
fence_default.. fence_ops, clean up sw_sync and finally merge
fence_context and fence_timeline.

Looking at how sync and fence It looks easier to me to de-stage sync_timeline first than userspace
fence.

	Gustavo