From patchwork Thu Jun 19 16:12:32 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 24431 Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from ) id 1WxexW-0003EZ-AO; Thu, 19 Jun 2014 18:13:06 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.72/mailfrontend-7) with esmtp id 1WxexT-0002tJ-0I; Thu, 19 Jun 2014 18:13:05 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757912AbaFSQMj (ORCPT + 1 other); Thu, 19 Jun 2014 12:12:39 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:40171 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757405AbaFSQMi (ORCPT ); Thu, 19 Jun 2014 12:12:38 -0400 Received: from 5ed49945.cm-7-5c.dynamic.ziggo.nl ([94.212.153.69] helo=[192.168.1.128]) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1Wxewz-0000JU-OW; Thu, 19 Jun 2014 16:12:33 +0000 Message-ID: <53A30BF0.1060109@canonical.com> Date: Thu, 19 Jun 2014 18:12:32 +0200 From: Maarten Lankhorst User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Colin Cross , Greg KH , "open list:GENERIC INCLUDE/A..." , thellstrom@vmware.com, lkml , "dri-devel@lists.freedesktop.org" , "linaro-mm-sig@lists.linaro.org" , Rob Clark , thierry.reding@gmail.com, Sumit Semwal , "open list:DMA BUFFER SHARIN..." CC: daniel@ffwll.ch Subject: Re: [REPOST PATCH 4/8] android: convert sync to fence api, v5 References: <20140618102957.15728.43525.stgit@patser> <20140618103711.15728.97842.stgit@patser> <20140619011556.GE10921@kroah.com> <20140619063727.GL5821@phenom.ffwll.local> In-Reply-To: Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2014.6.19.160619 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, MSGID_ADDED_BY_MTA 0.05, BODY_SIZE_5000_5999 0, BODY_SIZE_7000_LESS 0, CT_TEXT_PLAIN_UTF8_CAPS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CP_URI_IN_BODY 0, __CT 0, __CTE 0, __CT_TEXT_PLAIN 0, __FORWARDED_MSG 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MOZILLA_MSGID 0, __MOZILLA_USER_AGENT 0, __MULTIPLE_RCPTS_TO_X5 0, __SANE_MSGID 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __URI_NO_WWW 0, __URI_NS , __USER_AGENT 0' op 19-06-14 17:22, Colin Cross schreef: > On Wed, Jun 18, 2014 at 11:37 PM, Daniel Vetter wrote: >> On Wed, Jun 18, 2014 at 06:15:56PM -0700, Greg KH wrote: >>> On Wed, Jun 18, 2014 at 12:37:11PM +0200, Maarten Lankhorst wrote: >>>> Just to show it's easy. >>>> >>>> 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. >>>> v5: >>>> - Fix small style issues pointed out by Thomas Hellstrom. >>>> >>>> Signed-off-by: Maarten Lankhorst >>>> Acked-by: John Stultz >>>> --- >>>> drivers/staging/android/Kconfig | 1 >>>> drivers/staging/android/Makefile | 2 >>>> drivers/staging/android/sw_sync.c | 6 >>>> drivers/staging/android/sync.c | 913 +++++++++++----------------------- >>>> drivers/staging/android/sync.h | 79 ++- >>>> drivers/staging/android/sync_debug.c | 247 +++++++++ >>>> drivers/staging/android/trace/sync.h | 12 >>>> 7 files changed, 609 insertions(+), 651 deletions(-) >>>> create mode 100644 drivers/staging/android/sync_debug.c >>> With these changes, can we pull the android sync logic out of >>> drivers/staging/ now? >> Afaik the google guys never really looked at this and acked it. So I'm not >> sure whether they'll follow along. The other issue I have as the >> maintainer of gfx driver is that I don't want to implement support for two >> different sync object primitives (once for dma-buf and once for android >> syncpts), and my impression thus far has been that even with this we're >> not there. > We have tested these patches to use dma fences to back the android > sync driver and not found any major issues. However, my understanding > is that dma fences are designed for implicit sync, and explicit sync > through the android sync driver is bolted on the side to share code. > Android is not moving away from explicit sync, but we do wrap all of > our userspace sync accesses through libsync > (https://android.googlesource.com/platform/system/core/+/master/libsync/sync.c, > ignore the sw_sync parts), so if the kernel supported a slightly > different userspace explicit sync interface we could adapt to it > fairly easily. All we require is that individual kernel drivers need > to be able to accept work alongisde an fd to wait on, and to return an > fd that will signal when the work is done, and that userspace has some > way to merge two of those fds, wait on an fd, and get some debugging > info from an fd. However, this patch set doesn't do that, it has no > way to export a dma fence as an fd except through the android sync > driver, so it is not yet ready to fully replace android sync. > Dma fences can be exported as android fences, just didn't see a need for it yet. :-) To wait on all implicit fences attached to a dma-buf one could simply poll the dma-buf directly, or use something like a android userspace fence. sync_fence_create takes a sync_pt as function argument, but I kept that to keep source code compatibility, not because it uses any sync_pt functions. Here's a patch to create a userspace fd for dma-fence instead of a android fence, applied on top of "android: convert sync to fence api". --- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c index a76db3ff87cb..afc3c63b0438 100644 --- a/drivers/staging/android/sw_sync.c +++ b/drivers/staging/android/sw_sync.c @@ -184,7 +184,7 @@ static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj, } data.name[sizeof(data.name) - 1] = '\0'; - fence = sync_fence_create(data.name, pt); + fence = sync_fence_create(data.name, &pt->base); if (fence == NULL) { sync_pt_free(pt); err = -ENOMEM; diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 70b09b5001ba..c89a6f954e41 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -188,7 +188,7 @@ static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) } /* TODO: implement a create which takes more that one sync_pt */ -struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt) +struct sync_fence *sync_fence_create(const char *name, struct fence *pt) { struct sync_fence *fence; @@ -199,10 +199,10 @@ struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt) fence->num_fences = 1; atomic_set(&fence->status, 1); - fence_get(&pt->base); - fence->cbs[0].sync_pt = &pt->base; + fence_get(pt); + fence->cbs[0].sync_pt = pt; fence->cbs[0].fence = fence; - if (fence_add_callback(&pt->base, &fence->cbs[0].cb, + if (fence_add_callback(pt, &fence->cbs[0].cb, fence_check_cb_func)) atomic_dec(&fence->status); diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 66b0f431f63e..3020751f3c5d 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -252,7 +252,7 @@ void sync_pt_free(struct sync_pt *pt); * Creates a fence containg @pt. Once this is called, the fence takes * ownership of @pt. */ -struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt); +struct sync_fence *sync_fence_create(const char *name, struct fence *fence); /* * API for sync_fence consumers