Re: [PATCH 03/13] staging/android/sync: Move sync framework out of staging

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Dec 21, 2015 at 02:20:59PM +0000, John Harrison wrote:
> On 21/12/2015 10:03, Daniel Vetter wrote:
> >On Thu, Dec 17, 2015 at 09:35:03AM -0800, Jesse Barnes wrote:
> >>On 12/11/2015 05:11 AM, John.C.Harrison@Intel.com wrote:
> >>>From: John Harrison <John.C.Harrison@Intel.com>
> >>>
> >>>The sync framework is now used by the i915 driver. Therefore it can be
> >>>moved out of staging and into the regular tree. Also, the public
> >>>interfaces can actually be made public and exported.
> >>>
> >>>v3: New patch for series.
> >>>
> >>>Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> >>>Signed-off-by: Geoff Miller <geoff.miller@intel.com>
> >>>---
> >>>  drivers/android/Kconfig                |  28 ++
> >>>  drivers/android/Makefile               |   2 +
> >>>  drivers/android/sw_sync.c              | 260 ++++++++++++
> >>>  drivers/android/sw_sync.h              |  59 +++
> >>>  drivers/android/sync.c                 | 734 +++++++++++++++++++++++++++++++++
> >>>  drivers/android/sync.h                 | 366 ++++++++++++++++
> >>>  drivers/android/sync_debug.c           | 256 ++++++++++++
> >>>  drivers/android/trace/sync.h           |  82 ++++
> >>>  drivers/staging/android/Kconfig        |  28 --
> >>>  drivers/staging/android/Makefile       |   2 -
> >>>  drivers/staging/android/sw_sync.c      | 260 ------------
> >>>  drivers/staging/android/sw_sync.h      |  59 ---
> >>>  drivers/staging/android/sync.c         | 734 ---------------------------------
> >>>  drivers/staging/android/sync.h         | 366 ----------------
> >>>  drivers/staging/android/sync_debug.c   | 256 ------------
> >>>  drivers/staging/android/trace/sync.h   |  82 ----
> >>>  drivers/staging/android/uapi/sw_sync.h |  32 --
> >>>  drivers/staging/android/uapi/sync.h    |  97 -----
> >>>  include/uapi/Kbuild                    |   1 +
> >>>  include/uapi/sync/Kbuild               |   3 +
> >>>  include/uapi/sync/sw_sync.h            |  32 ++
> >>>  include/uapi/sync/sync.h               |  97 +++++
> >>>  22 files changed, 1920 insertions(+), 1916 deletions(-)
> >>>  create mode 100644 drivers/android/sw_sync.c
> >>>  create mode 100644 drivers/android/sw_sync.h
> >>>  create mode 100644 drivers/android/sync.c
> >>>  create mode 100644 drivers/android/sync.h
> >>>  create mode 100644 drivers/android/sync_debug.c
> >>>  create mode 100644 drivers/android/trace/sync.h
> >>>  delete mode 100644 drivers/staging/android/sw_sync.c
> >>>  delete mode 100644 drivers/staging/android/sw_sync.h
> >>>  delete mode 100644 drivers/staging/android/sync.c
> >>>  delete mode 100644 drivers/staging/android/sync.h
> >>>  delete mode 100644 drivers/staging/android/sync_debug.c
> >>>  delete mode 100644 drivers/staging/android/trace/sync.h
> >>>  delete mode 100644 drivers/staging/android/uapi/sw_sync.h
> >>>  delete mode 100644 drivers/staging/android/uapi/sync.h
> >>>  create mode 100644 include/uapi/sync/Kbuild
> >>>  create mode 100644 include/uapi/sync/sw_sync.h
> >>>  create mode 100644 include/uapi/sync/sync.h
> >>>
> >>>diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
> >>>index bdfc6c6..9edcd8f 100644
> >>>--- a/drivers/android/Kconfig
> >>>+++ b/drivers/android/Kconfig
> >>>@@ -32,6 +32,34 @@ config ANDROID_BINDER_IPC_32BIT
> >>>  	  Note that enabling this will break newer Android user-space.
> >>>+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
> >>>+	  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
> >>>+	  synchronization.  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
> >>IIRC we wanted to drop the user ABI altogether?  I think we can de-stage
> >>this even before we push the new ABI on the i915 side to expose the sync
> >>points (since we'll need an open source userspace for that), and any
> >>changes/cleanups can happen outside of staging.
> >Just a head-up: Gustavo Padovan from Collabora is working to de-stage all
> >the syncpt stuff. Greg KH merged the TODO update for that work for 4.5,
> >which covers consensus (including ack from Google's Greg Hackmann on the
> >plan). Given that I think it'd be best to freeload on that effort. But
> >that means we need to push all the android/syncpt stuff down in the
> >series. I hope that works, and there's no functional depencies in the
> >fence conversion/scheduler core?
> >
> >Thanks, Daniel
> 
> Do you have any idea on the timescale for their destaging? Is it definitely,
> definitely happening or still just a 'some point in the future it would be
> nice to...'? The Android driver certainly needs it and I believe Jesse's
> bufferless work will require it too. So sooner is greatly preferable to
> later.

He's working on it, under a contract to make it happen. Afaik a bunch of
the work is done already, big bits left are cleaning up some of the
internals and then also doing some review on the ABI & test coverage.

> As long as the reset of the fence conversion still goes in then it's not all
> that much effort to extract the sync code. It was all previously '#if
> CONFIG_SYNC' anyway so the system can definitely be made to work without it.
> There are two main patches that add it in, one in the fence series and
> another in the scheduler series. However, if you just drop those there might
> well be a big bunch of merge conflicts to resolve in the subsequent patches.

Rebasing to avoid the depency would be good I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx