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

From: John Harrison <John.C.Harrison@Intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Intel-GFX@Lists.FreeDesktop.Org
Subject: Re: [PATCH 03/13] staging/android/sync: Move sync framework out of staging
Date: Tue, 22 Dec 2015 12:14:25 +0000	[thread overview]
Message-ID: <56793EA1.1000506@Intel.com> (raw)
In-Reply-To: <20151221154643.GC30437@phenom.ffwll.local>

On 21/12/2015 15:46, Daniel Vetter wrote:
> 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

Okay, I've got a bunch of changes to make right the way through anyway 
as I forgot to run the style checker. Too used to our Android process 
which does it automatically.

Will repost both the struct fence and the scheduler patches as new 
series with all the staging stuff left until the very end as an 
independent set.

Thanks,
John.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx