From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: Re: [RFC 00/29] De-stage android's sync framework Date: Wed, 20 Jan 2016 16:02:31 +0100 Message-ID: <569FA187.2060601@linux.intel.com> References: <1452869739-3304-1-git-send-email-gustavo@padovan.org> <569F614A.1070807@linux.intel.com> <20160120143250.GF8217@joana> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTP id 4FE7D6E960 for ; Wed, 20 Jan 2016 07:02:48 -0800 (PST) In-Reply-To: <20160120143250.GF8217@joana> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Gustavo Padovan , Greg Kroah-Hartman , devel@driverdev.osuosl.org, daniels@collabora.com, Daniel Vetter , Riley Andrews , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, =?UTF-8?Q?Arve_Hj=c3=b8nnev=c3=a5g?= , Gustavo Padovan , John Harrison List-Id: dri-devel@lists.freedesktop.org T3AgMjAtMDEtMTYgb20gMTU6MzIgc2NocmVlZiBHdXN0YXZvIFBhZG92YW46Cj4gMjAxNi0wMS0y MCBNYWFydGVuIExhbmtob3JzdCA8bWFhcnRlbi5sYW5raG9yc3RAbGludXguaW50ZWwuY29tPjoK Pgo+PiBIZXksCj4+Cj4+IE9wIDE1LTAxLTE2IG9tIDE1OjU1IHNjaHJlZWYgR3VzdGF2byBQYWRv dmFuOgo+Pj4gRnJvbTogR3VzdGF2byBQYWRvdmFuIDxndXN0YXZvLnBhZG92YW5AY29sbGFib3Jh LmNvLnVrPgo+Pj4KPj4+IFRoaXMgcGF0Y2ggc2VyaWVzIGRlLXN0YWdlIHRoZSBzeW5jIGZyYW1l d29yaywgYW5kIGluIG9yZGVyIHRvIGFjY29tcGxpc2ggdGhhdAo+Pj4gYSBidW5jaCBvZiBjbGVh bnVwcy9pbXByb3ZlbWVudHMgb24gdGhlIHN5bmMgYW5kIGZlbmNlIHdlcmUgbWFkZS4KPj4+Cj4+ PiBUaGUgc3luYyBmcmFtZXdvcmsgY29udGFpbmVkIHNvbWUgYWJzdHJhY3Rpb25zIGFyb3VuZCBz dHJ1Y3QgZmVuY2UgYW5kIHRob3NlCj4+PiB3ZXJlIHJlbW92ZWQgaW4gdGhlIGRlLXN0YWdpbmcg cHJvY2VzcyBhbW9uZyBvdGhlciBjaGFuZ2VzOgo+Pj4KPj4+IFVzZXJzcGFjZSB2aXNpYmxlIGNo YW5nZXMKPj4+IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPj4+Cj4+PiAgKiBUaGUgc3dfc3lu YyBmaWxlIHdhcyBtb3ZlZCBmcm9tIC9kZXYvc3dfc3luYyB0byA8ZGVidWdmcz4vc3luYy9zd19z eW5jLiBObwo+Pj4gIG90aGVyIGNoYW5nZS4KPj4+Cj4+PiBLZXJuZWwgQVBJIGNoYW5nZXMKPj4+ IC0tLS0tLS0tLS0tLS0tLS0tLQo+Pj4KPj4+ICAqIHN0cnVjdCBzeW5jX3RpbWVsaW5lIGlzIG5v dyBzdHJ1Y3QgZmVuY2VfdGltZWxpbmUKPj4+ICAqIHN5bmNfdGltZWxpbmVfb3BzIGlzIG5vdyBm ZW5jZV90aW1lbGluZV9vcHMgYW5kIHRoZXkgbm93IGNhcnJ5IHN0cnVjdAo+Pj4gIGZlbmNlIGFz IHBhcmFtZXRlciBpbnN0ZWFkIG9mIHN0cnVjdCBzeW5jX3B0Cj4+PiAgKiBhIC5jbGVhbnVwKCkg ZmVuY2Ugb3Agd2FzIGFkZGVkIHRvIGFsbG93IHN5bmNfZmVuY2UgdG8gcnVuIGEgY2xlYW51cCB3 aGVuCj4+PiAgdGhlIGZlbmNlX3RpbWVsaW5lIGlzIGRlc3Ryb3llZAo+Pj4gICogYWRkZWQgZmVu Y2VfYWRkX3VzZWRfZGF0YSgpIHRvIHBhc3MgYSBwcml2YXRlIHBvaW50IHRvIHN0cnVjdCBmZW5j ZS4gVGhpcwo+Pj4gIHBvaW50ZXIgaXMgc2VudCBiYWNrIG9uIHRoZSAuY2xlYW51cCBvcC4KPj4+ ICAqIFRoZSBzeW5jIHRpbWVsaW5lIGZ1bmN0aW9uIHdlcmUgbW92ZWQgdG8gYmUgZmVuY2VfdGlt ZWxpbmUgZnVuY3Rpb25zOgo+Pj4gCSAtIHN5bmNfdGltZWxpbmVfY3JlYXRlKCkJLT4gZmVuY2Vf dGltZWxpbmVfY3JlYXRlKCkKPj4+IAkgLSBzeW5jX3RpbWVsaW5lX2dldCgpCQktPiBmZW5jZV90 aW1lbGluZV9nZXQoKQo+Pj4gCSAtIHN5bmNfdGltZWxpbmVfcHV0KCkJCS0+IGZlbmNlX3RpbWVs aW5lX3B1dCgpCj4+PiAJIC0gc3luY190aW1lbGluZV9kZXN0cm95KCkJLT4gZmVuY2VfdGltZWxp bmVfZGVzdHJveSgpCj4+PiAJIC0gc3luY190aW1lbGluZV9zaWduYWwoKQktPiBmZW5jZV90aW1l bGluZV9zaWduYWwoKQo+Pj4KPj4+ICAgKiBzeW5jX3B0X2NyZWF0ZSgpIHdhcyByZXBsYWNlZCBi ZSBmZW5jZV9jcmVhdGVfb25fdGltZWxpbmUoKQo+Pj4KPj4+IEludGVybmFsIGNoYW5nZXMKPj4+ IC0tLS0tLS0tLS0tLS0tLS0KPj4+Cj4+PiAgKiBmZW5jZV90aW1lbGluZV9vcHMgd2FzIHJlbW92 ZWQgaW4gZmF2b3Igb2YgZGlyZWN0IHVzZSBmZW5jZV9vcHMKPj4+ICAqIGZlbmNlIGRlZmF1bHQg ZnVuY3Rpb25zIHdlcmUgY3JlYXRlZCBmb3IgZmVuY2Vfb3BzCj4+PiAgKiByZW1vdmVkIHN0cnVj dHMgc3luY19wdCwgc3dfc3luY190aW1lbGluZSBhbmQgc3dfc3luY19wdAo+Pj4KPj4+IEd1c3Rh dm8gUGFkb3ZhbiAoMjkpOgo+Pj4gICBzdGFnaW5nL2FuZHJvaWQ6IGZpeCBzeW5jIGZyYW1ld29y ayBkb2N1bWVudGF0aW9uCj4+PiAgIHN0YWdpbmcvYW5kcm9pZDogZml4IGNoZWNrcGF0Y2ggd2Fy bmluZwo+Pj4gICBzdGFnaW5nL2FuZHJvaWQ6IHJlbmFtZSBzeW5jX2ZlbmNlX3JlbGVhc2UKPj4+ ICAgc3RhZ2luZy9hbmRyb2lkOiByZW5hbWUgJ2FuZHJvaWRfZmVuY2UnIHRvICdzeW5jX2ZlbmNl Jwo+Pj4gICBzdGFnaW5nL2FuZHJvaWQ6IHJlbW92ZSBub3QgdXNlZCBzeW5jX3RpbWVsaW5lIG9w cwo+Pj4gICBzdGFnaW5nL2FuZHJvaWQ6IGNyZWF0ZSBhICdzeW5jJyBkaXIgZm9yIGRlYnVnZnMg aW5mb3JtYXRpb24KPj4+ICAgc3RhZ2luZy9hbmRyb2lkOiBtb3ZlIHN3X3N5bmMgZmlsZSB0byBk ZWJ1Z2ZzIGZpbGUKPj4+ICAgc3RhZ2luZy9hbmRyb2lkOiBSZW1vdmUgV0FSTl9PTl9PTkNFIHdo ZW4gcmVsZWFzaW5nIHN5bmNfZmVuY2UKPj4+ICAgc3RhZ2luZy9hbmRyb2lkOiByZW5hbWUgc3Ry dWN0IHN5bmNfZmVuY2UncyB2YXJpYWJsZXMgdG8gJ3N5bmNfZmVuY2UnCj4+PiAgIHN0YWdpbmcv YW5kcm9pZDogcmVuYW1lICdzeW5jX3B0JyB0byAnZmVuY2UnIGluIHN0cnVjdCBzeW5jX2ZlbmNl X2NiCj4+PiAgIGRtYS1idWYvZmVuY2U6IG1vdmUgc3luY190aW1lbGluZSB0byBmZW5jZV90aW1l bGluZQo+Pj4gICBzdGFnaW5nL2FuZHJvaWQ6IHJlbW92ZSBzdHJ1Y3Qgc3luY19wdAo+Pj4gICBk bWEtYnVmL2ZlbmNlOiBjcmVhdGUgZmVuY2VfZGVmYXVsdF9lbmFibGVfc2lnbmFsaW5nKCkKPj4+ ICAgZG1hLWJ1Zi9mZW5jZTogY3JlYXRlIGZlbmNlX2RlZmF1bHRfcmVsZWFzZSgpCj4+PiAgIGRt YS1idWYvZmVuY2U6IGNyZWF0ZSBmZW5jZV9kZWZhdWx0X2dldF9kcml2ZXJfbmFtZSgpCj4+PiAg IGRtYS1idWYvZmVuY2U6IGNyZWF0ZSBmZW5jZV9kZWZhdWx0X3RpbWVsaW5lX25hbWUoKQo+PiBU aGlzIGlzIG1pc2xlYWRpbmcuIEkgdGhpbmsgdGltZWxpbmVfZmVuY2UgcHJlZml4IHdvdWxkIGJl IG1vcmUgYXBwcm9wcmlhdGUgaGVyZS4KPiBXaHk/IFRoZXNlIGZlbmNlX2RlZmF1bHRfLi4gZnVu Y3Rpb25zIGFyZSBmZW5jZV9vcHMgYW5kIG5vdCByZWxhdGVkIHRvCj4gZmVuY2VfdGltZWxpbmUg aW4gYW55IHdheS4KQmVjYXVzZSB0aGV5J3JlIHVzaW5nIGZlbmNlX3BhcmVudCwgd2hpY2ggc2hv dWxkIHByb2JhYmx5IGJlIHJlbmFtZWQgdG8gZmVuY2VfdG9fdGltZWxpbmUoKQoKVGhlIG5hbWUg bWFrZXMgaXQgc291bmQgYXMgaWYgdGhleSBjb3VsZCBhcHBseSB0byBhbGwgdHlwZSBvZiBmZW5j ZXMsIEkgZG9uJ3QgdGhpbmsgdGhpcyBpcyB0aGUgY2FzZS4KPj4gSSBhbHNvIGJlbGlldmUgdGhp cyBzaG91bGQgYmUgZG9uZSBpbiBtdWx0aXBsZSBzZXJpZXMuIEZpcnN0IHNlcmllcyBzaG91bGQg ZGUtc3RhZ2UgdGhlIHVzZXJzcGFjZSBmZW5jZSBmcmFtZXdvcmsuIFRoZSBuZXh0IHNlcmllcyBz aG91bGQgZml4IHVwIGFuZHJvaWRfZmVuY2UgYW5kIG1heWJlIHJlbmFtZSBpdCB0byB0aW1lbGlu ZV9mZW5jZSBzaW5jZSBzeW5jX2ZlbmNlIGlzIGFscmVhZHkgdXNlZCBmb3IgdGhlIHVzZXJzcGFj ZSBmZCwgd2hpY2ggd291bGQgYWRkIG1vcmUgY29uZnVzaW9uPwo+IFN1cmUuIEkndmUgYmVlbiB0 aGlua2luZyBvbiBob3cgdG8gc3BsaXQgdGhpcyBwcm9wZXJseS4gSSdtIHRyeWluZyB0bwo+IGFk ZCBhIGJ1bmNoIG9mIGNsZWFuIHVwL3JlbmFtaW5nIGZpcnN0LCBlZyB0aGUgc3luY19mZW5jZSBy ZW5hbWUgdG8KPiBzeW5jX2ZpbGUgdGhhdCBEYW5pZWwgVmV0dGVyIGFuZCBJIGRpc2N1c3NlZC4g Cj4KPiBOZXh0IG15IHBsYW4gd291bGQgYmUgbW92ZSBzeW5jX3RpbWVsaW5lIHRvIGZlbmNlX3Rp bWVsaW5lLCBhZGQgdGhlCj4gZmVuY2VfZGVmYXVsdC4uIGZlbmNlX29wcywgY2xlYW4gdXAgc3df c3luYyBhbmQgZmluYWxseSBtZXJnZQo+IGZlbmNlX2NvbnRleHQgYW5kIGZlbmNlX3RpbWVsaW5l Lgo+Cj4gTG9va2luZyBhdCBob3cgc3luYyBhbmQgZmVuY2UgSXQgbG9va3MgZWFzaWVyIHRvIG1l IHRvIGRlLXN0YWdlIHN5bmNfdGltZWxpbmUgZmlyc3QgdGhhbiB1c2Vyc3BhY2UKPiBmZW5jZS4K VGhlcmUncyBhbHJlYWR5IGNvZGUgdG8gYWRkIGEgc3luY19mZW5jZV9jcmVhdGVfZG1hIGV4cG9y dCBbMV1bMl0uIFNvIGlmIHlvdSB3YW50IHRvIGRlLXN0YWdlIGl0IHRoZW4gdGhlcmUgd2lsbCBi ZSB1c2VycyBmb3IgaXQuCgpzeW5jX3B0IG90b2ggaGFzIG5vIHVwc3RyZWFtIGluLWtlcm5lbCB1 c2Vycy4gSXQgd2FzIGEgd3JhcHBlciB0byBrZWVwIGFuZHJvaWQgZHJpdmVycyBhcGkgY29tcGF0 aWJsZSB3aXRoIHRoZSBmZW5jZSBhcGkuCgpbMV0gaHR0cHM6Ly9wYXRjaHdvcmsuZnJlZWRlc2t0 b3Aub3JnL3BhdGNoLzY3ODQ1LwpbMl0gaHR0cHM6Ly9wYXRjaHdvcmsuZnJlZWRlc2t0b3Aub3Jn L3BhdGNoLzY3ODQ2LwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZl bAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934317AbcATPDN (ORCPT ); Wed, 20 Jan 2016 10:03:13 -0500 Received: from mga01.intel.com ([192.55.52.88]:31712 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933718AbcATPDL (ORCPT ); Wed, 20 Jan 2016 10:03:11 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,321,1449561600"; d="scan'208";a="897376012" Subject: Re: [RFC 00/29] De-stage android's sync framework To: Gustavo Padovan , Greg Kroah-Hartman , devel@driverdev.osuosl.org, daniels@collabora.com, Daniel Vetter , Riley Andrews , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, =?UTF-8?Q?Arve_Hj=c3=b8nnev=c3=a5g?= , Gustavo Padovan , John Harrison References: <1452869739-3304-1-git-send-email-gustavo@padovan.org> <569F614A.1070807@linux.intel.com> <20160120143250.GF8217@joana> From: Maarten Lankhorst Message-ID: <569FA187.2060601@linux.intel.com> Date: Wed, 20 Jan 2016 16:02:31 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20160120143250.GF8217@joana> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Op 20-01-16 om 15:32 schreef Gustavo Padovan: > 2016-01-20 Maarten Lankhorst : > >> Hey, >> >> Op 15-01-16 om 15:55 schreef Gustavo Padovan: >>> From: Gustavo Padovan >>> >>> 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 /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. Because they're using fence_parent, which should probably be renamed to fence_to_timeline() The name makes it sound as if they could apply to all type of fences, I don't think this is the case. >> 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. There's already code to add a sync_fence_create_dma export [1][2]. So if you want to de-stage it then there will be users for it. sync_pt otoh has no upstream in-kernel users. It was a wrapper to keep android drivers api compatible with the fence api. [1] https://patchwork.freedesktop.org/patch/67845/ [2] https://patchwork.freedesktop.org/patch/67846/