From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gustavo Padovan Subject: Re: [RFC 00/29] De-stage android's sync framework Date: Wed, 20 Jan 2016 16:28:17 -0200 Message-ID: <20160120182817.GA2502@joana> References: <1452869739-3304-1-git-send-email-gustavo@padovan.org> <569F614A.1070807@linux.intel.com> <20160120143250.GF8217@joana> <569FA187.2060601@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-yk0-f181.google.com (mail-yk0-f181.google.com [209.85.160.181]) by gabe.freedesktop.org (Postfix) with ESMTPS id DE1916E9D0 for ; Wed, 20 Jan 2016 10:28:22 -0800 (PST) Received: by mail-yk0-f181.google.com with SMTP id a85so19894934ykb.1 for ; Wed, 20 Jan 2016 10:28:22 -0800 (PST) Content-Disposition: inline In-Reply-To: <569FA187.2060601@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Maarten Lankhorst Cc: devel@driverdev.osuosl.org, daniels@collabora.com, Greg Kroah-Hartman , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Riley Andrews , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Daniel Vetter , Gustavo Padovan , John Harrison List-Id: dri-devel@lists.freedesktop.org MjAxNi0wMS0yMCBNYWFydGVuIExhbmtob3JzdCA8bWFhcnRlbi5sYW5raG9yc3RAbGludXguaW50 ZWwuY29tPjoKCj4gT3AgMjAtMDEtMTYgb20gMTU6MzIgc2NocmVlZiBHdXN0YXZvIFBhZG92YW46 Cj4gPiAyMDE2LTAxLTIwIE1hYXJ0ZW4gTGFua2hvcnN0IDxtYWFydGVuLmxhbmtob3JzdEBsaW51 eC5pbnRlbC5jb20+Ogo+ID4KPiA+PiBIZXksCj4gPj4KPiA+PiBPcCAxNS0wMS0xNiBvbSAxNTo1 NSBzY2hyZWVmIEd1c3Rhdm8gUGFkb3ZhbjoKPiA+Pj4gRnJvbTogR3VzdGF2byBQYWRvdmFuIDxn dXN0YXZvLnBhZG92YW5AY29sbGFib3JhLmNvLnVrPgo+ID4+Pgo+ID4+PiBUaGlzIHBhdGNoIHNl cmllcyBkZS1zdGFnZSB0aGUgc3luYyBmcmFtZXdvcmssIGFuZCBpbiBvcmRlciB0byBhY2NvbXBs aXNoIHRoYXQKPiA+Pj4gYSBidW5jaCBvZiBjbGVhbnVwcy9pbXByb3ZlbWVudHMgb24gdGhlIHN5 bmMgYW5kIGZlbmNlIHdlcmUgbWFkZS4KPiA+Pj4KPiA+Pj4gVGhlIHN5bmMgZnJhbWV3b3JrIGNv bnRhaW5lZCBzb21lIGFic3RyYWN0aW9ucyBhcm91bmQgc3RydWN0IGZlbmNlIGFuZCB0aG9zZQo+ ID4+PiB3ZXJlIHJlbW92ZWQgaW4gdGhlIGRlLXN0YWdpbmcgcHJvY2VzcyBhbW9uZyBvdGhlciBj aGFuZ2VzOgo+ID4+Pgo+ID4+PiBVc2Vyc3BhY2UgdmlzaWJsZSBjaGFuZ2VzCj4gPj4+IC0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0KPiA+Pj4KPiA+Pj4gICogVGhlIHN3X3N5bmMgZmlsZSB3YXMg bW92ZWQgZnJvbSAvZGV2L3N3X3N5bmMgdG8gPGRlYnVnZnM+L3N5bmMvc3dfc3luYy4gTm8KPiA+ Pj4gIG90aGVyIGNoYW5nZS4KPiA+Pj4KPiA+Pj4gS2VybmVsIEFQSSBjaGFuZ2VzCj4gPj4+IC0t LS0tLS0tLS0tLS0tLS0tLQo+ID4+Pgo+ID4+PiAgKiBzdHJ1Y3Qgc3luY190aW1lbGluZSBpcyBu b3cgc3RydWN0IGZlbmNlX3RpbWVsaW5lCj4gPj4+ICAqIHN5bmNfdGltZWxpbmVfb3BzIGlzIG5v dyBmZW5jZV90aW1lbGluZV9vcHMgYW5kIHRoZXkgbm93IGNhcnJ5IHN0cnVjdAo+ID4+PiAgZmVu Y2UgYXMgcGFyYW1ldGVyIGluc3RlYWQgb2Ygc3RydWN0IHN5bmNfcHQKPiA+Pj4gICogYSAuY2xl YW51cCgpIGZlbmNlIG9wIHdhcyBhZGRlZCB0byBhbGxvdyBzeW5jX2ZlbmNlIHRvIHJ1biBhIGNs ZWFudXAgd2hlbgo+ID4+PiAgdGhlIGZlbmNlX3RpbWVsaW5lIGlzIGRlc3Ryb3llZAo+ID4+PiAg KiBhZGRlZCBmZW5jZV9hZGRfdXNlZF9kYXRhKCkgdG8gcGFzcyBhIHByaXZhdGUgcG9pbnQgdG8g c3RydWN0IGZlbmNlLiBUaGlzCj4gPj4+ICBwb2ludGVyIGlzIHNlbnQgYmFjayBvbiB0aGUgLmNs ZWFudXAgb3AuCj4gPj4+ICAqIFRoZSBzeW5jIHRpbWVsaW5lIGZ1bmN0aW9uIHdlcmUgbW92ZWQg dG8gYmUgZmVuY2VfdGltZWxpbmUgZnVuY3Rpb25zOgo+ID4+PiAJIC0gc3luY190aW1lbGluZV9j cmVhdGUoKQktPiBmZW5jZV90aW1lbGluZV9jcmVhdGUoKQo+ID4+PiAJIC0gc3luY190aW1lbGlu ZV9nZXQoKQkJLT4gZmVuY2VfdGltZWxpbmVfZ2V0KCkKPiA+Pj4gCSAtIHN5bmNfdGltZWxpbmVf cHV0KCkJCS0+IGZlbmNlX3RpbWVsaW5lX3B1dCgpCj4gPj4+IAkgLSBzeW5jX3RpbWVsaW5lX2Rl c3Ryb3koKQktPiBmZW5jZV90aW1lbGluZV9kZXN0cm95KCkKPiA+Pj4gCSAtIHN5bmNfdGltZWxp bmVfc2lnbmFsKCkJLT4gZmVuY2VfdGltZWxpbmVfc2lnbmFsKCkKPiA+Pj4KPiA+Pj4gICAqIHN5 bmNfcHRfY3JlYXRlKCkgd2FzIHJlcGxhY2VkIGJlIGZlbmNlX2NyZWF0ZV9vbl90aW1lbGluZSgp Cj4gPj4+Cj4gPj4+IEludGVybmFsIGNoYW5nZXMKPiA+Pj4gLS0tLS0tLS0tLS0tLS0tLQo+ID4+ Pgo+ID4+PiAgKiBmZW5jZV90aW1lbGluZV9vcHMgd2FzIHJlbW92ZWQgaW4gZmF2b3Igb2YgZGly ZWN0IHVzZSBmZW5jZV9vcHMKPiA+Pj4gICogZmVuY2UgZGVmYXVsdCBmdW5jdGlvbnMgd2VyZSBj cmVhdGVkIGZvciBmZW5jZV9vcHMKPiA+Pj4gICogcmVtb3ZlZCBzdHJ1Y3RzIHN5bmNfcHQsIHN3 X3N5bmNfdGltZWxpbmUgYW5kIHN3X3N5bmNfcHQKPiA+Pj4KPiA+Pj4gR3VzdGF2byBQYWRvdmFu ICgyOSk6Cj4gPj4+ICAgc3RhZ2luZy9hbmRyb2lkOiBmaXggc3luYyBmcmFtZXdvcmsgZG9jdW1l bnRhdGlvbgo+ID4+PiAgIHN0YWdpbmcvYW5kcm9pZDogZml4IGNoZWNrcGF0Y2ggd2FybmluZwo+ ID4+PiAgIHN0YWdpbmcvYW5kcm9pZDogcmVuYW1lIHN5bmNfZmVuY2VfcmVsZWFzZQo+ID4+PiAg IHN0YWdpbmcvYW5kcm9pZDogcmVuYW1lICdhbmRyb2lkX2ZlbmNlJyB0byAnc3luY19mZW5jZScK PiA+Pj4gICBzdGFnaW5nL2FuZHJvaWQ6IHJlbW92ZSBub3QgdXNlZCBzeW5jX3RpbWVsaW5lIG9w cwo+ID4+PiAgIHN0YWdpbmcvYW5kcm9pZDogY3JlYXRlIGEgJ3N5bmMnIGRpciBmb3IgZGVidWdm cyBpbmZvcm1hdGlvbgo+ID4+PiAgIHN0YWdpbmcvYW5kcm9pZDogbW92ZSBzd19zeW5jIGZpbGUg dG8gZGVidWdmcyBmaWxlCj4gPj4+ICAgc3RhZ2luZy9hbmRyb2lkOiBSZW1vdmUgV0FSTl9PTl9P TkNFIHdoZW4gcmVsZWFzaW5nIHN5bmNfZmVuY2UKPiA+Pj4gICBzdGFnaW5nL2FuZHJvaWQ6IHJl bmFtZSBzdHJ1Y3Qgc3luY19mZW5jZSdzIHZhcmlhYmxlcyB0byAnc3luY19mZW5jZScKPiA+Pj4g ICBzdGFnaW5nL2FuZHJvaWQ6IHJlbmFtZSAnc3luY19wdCcgdG8gJ2ZlbmNlJyBpbiBzdHJ1Y3Qg c3luY19mZW5jZV9jYgo+ID4+PiAgIGRtYS1idWYvZmVuY2U6IG1vdmUgc3luY190aW1lbGluZSB0 byBmZW5jZV90aW1lbGluZQo+ID4+PiAgIHN0YWdpbmcvYW5kcm9pZDogcmVtb3ZlIHN0cnVjdCBz eW5jX3B0Cj4gPj4+ICAgZG1hLWJ1Zi9mZW5jZTogY3JlYXRlIGZlbmNlX2RlZmF1bHRfZW5hYmxl X3NpZ25hbGluZygpCj4gPj4+ICAgZG1hLWJ1Zi9mZW5jZTogY3JlYXRlIGZlbmNlX2RlZmF1bHRf cmVsZWFzZSgpCj4gPj4+ICAgZG1hLWJ1Zi9mZW5jZTogY3JlYXRlIGZlbmNlX2RlZmF1bHRfZ2V0 X2RyaXZlcl9uYW1lKCkKPiA+Pj4gICBkbWEtYnVmL2ZlbmNlOiBjcmVhdGUgZmVuY2VfZGVmYXVs dF90aW1lbGluZV9uYW1lKCkKPiA+PiBUaGlzIGlzIG1pc2xlYWRpbmcuIEkgdGhpbmsgdGltZWxp bmVfZmVuY2UgcHJlZml4IHdvdWxkIGJlIG1vcmUgYXBwcm9wcmlhdGUgaGVyZS4KPiA+IFdoeT8g VGhlc2UgZmVuY2VfZGVmYXVsdF8uLiBmdW5jdGlvbnMgYXJlIGZlbmNlX29wcyBhbmQgbm90IHJl bGF0ZWQgdG8KPiA+IGZlbmNlX3RpbWVsaW5lIGluIGFueSB3YXkuCj4gQmVjYXVzZSB0aGV5J3Jl IHVzaW5nIGZlbmNlX3BhcmVudCwgd2hpY2ggc2hvdWxkIHByb2JhYmx5IGJlIHJlbmFtZWQgdG8g ZmVuY2VfdG9fdGltZWxpbmUoKQo+IAo+IFRoZSBuYW1lIG1ha2VzIGl0IHNvdW5kIGFzIGlmIHRo ZXkgY291bGQgYXBwbHkgdG8gYWxsIHR5cGUgb2YgZmVuY2VzLCBJIGRvbid0IHRoaW5rIHRoaXMg aXMgdGhlIGNhc2UuCgpGYWlyIGVub3VnaC4gSSBjYW4gY2hhbmdlIHRoZSBuYW1pbmcuIEFsc28g dGhlIGZpbmFsIGlkZWEgaXMgdG8gbW92ZQpmZW5jZV9vcHMgdG8gYmUgZmVuY2VfdGltZWxpbmVf b3BzIHNvIHRoZSBuYW1pbmcgbWFrZXMgc2Vuc2UuICAKCj4gPj4gSSBhbHNvIGJlbGlldmUgdGhp cyBzaG91bGQgYmUgZG9uZSBpbiBtdWx0aXBsZSBzZXJpZXMuIEZpcnN0IHNlcmllcyBzaG91bGQg ZGUtc3RhZ2UgdGhlIHVzZXJzcGFjZSBmZW5jZSBmcmFtZXdvcmsuIFRoZSBuZXh0IHNlcmllcyBz aG91bGQgZml4IHVwIGFuZHJvaWRfZmVuY2UgYW5kIG1heWJlIHJlbmFtZSBpdCB0byB0aW1lbGlu ZV9mZW5jZSBzaW5jZSBzeW5jX2ZlbmNlIGlzIGFscmVhZHkgdXNlZCBmb3IgdGhlIHVzZXJzcGFj ZSBmZCwgd2hpY2ggd291bGQgYWRkIG1vcmUgY29uZnVzaW9uPwo+ID4gU3VyZS4gSSd2ZSBiZWVu IHRoaW5raW5nIG9uIGhvdyB0byBzcGxpdCB0aGlzIHByb3Blcmx5LiBJJ20gdHJ5aW5nIHRvCj4g PiBhZGQgYSBidW5jaCBvZiBjbGVhbiB1cC9yZW5hbWluZyBmaXJzdCwgZWcgdGhlIHN5bmNfZmVu Y2UgcmVuYW1lIHRvCj4gPiBzeW5jX2ZpbGUgdGhhdCBEYW5pZWwgVmV0dGVyIGFuZCBJIGRpc2N1 c3NlZC4gCj4gPgo+ID4gTmV4dCBteSBwbGFuIHdvdWxkIGJlIG1vdmUgc3luY190aW1lbGluZSB0 byBmZW5jZV90aW1lbGluZSwgYWRkIHRoZQo+ID4gZmVuY2VfZGVmYXVsdC4uIGZlbmNlX29wcywg Y2xlYW4gdXAgc3dfc3luYyBhbmQgZmluYWxseSBtZXJnZQo+ID4gZmVuY2VfY29udGV4dCBhbmQg ZmVuY2VfdGltZWxpbmUuCj4gPgo+ID4gTG9va2luZyBhdCBob3cgc3luYyBhbmQgZmVuY2UgSXQg bG9va3MgZWFzaWVyIHRvIG1lIHRvIGRlLXN0YWdlIHN5bmNfdGltZWxpbmUgZmlyc3QgdGhhbiB1 c2Vyc3BhY2UKPiA+IGZlbmNlLgo+IFRoZXJlJ3MgYWxyZWFkeSBjb2RlIHRvIGFkZCBhIHN5bmNf ZmVuY2VfY3JlYXRlX2RtYSBleHBvcnQgWzFdWzJdLiBTbyBpZiB5b3Ugd2FudCB0byBkZS1zdGFn ZSBpdCB0aGVuIHRoZXJlIHdpbGwgYmUgdXNlcnMgZm9yIGl0Lgo+IAo+IHN5bmNfcHQgb3RvaCBo YXMgbm8gdXBzdHJlYW0gaW4ta2VybmVsIHVzZXJzLiBJdCB3YXMgYSB3cmFwcGVyIHRvIGtlZXAg YW5kcm9pZCBkcml2ZXJzIGFwaSBjb21wYXRpYmxlIHdpdGggdGhlIGZlbmNlIGFwaS4KCk9uZSBv ZiB0aGUgZmlyc3QgcGF0Y2hlcyBvZiBteSBzZXJpZXMgaXMgcmVtb3Zpbmcgc3RydWN0IHN5bmNf cHQsIHRoYXQKbWVhbnMgSSBhbSBhbHNvIHJlbW92aW5nIHN5bmNfZmVuY2VfY3JlYXRlX2RtYSgp IGJlY2F1c2UKc3luY19mZW5jZV9jcmVhdGUoKSBub3cgYWNjZXB0cyBzdHJ1Y3QgZmVuY2UgYXMg cGFyYW1ldGVyLgpJIGRvbid0IHN5bmNfcHQgYmVpbmcgdXNlZnVsIGZvciBhbnkgaW4ta2VybmVs IGRyaXZlciBhcyBpdCBpcyBqdXN0IGEKd3JhcHBlciBvbiB0b3Agb2Ygc3RydWN0IGZlbmNlLgoK CUd1c3Rhdm8KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18K ZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0 dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935014AbcATS2Z (ORCPT ); Wed, 20 Jan 2016 13:28:25 -0500 Received: from mail-yk0-f170.google.com ([209.85.160.170]:32827 "EHLO mail-yk0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934679AbcATS2W (ORCPT ); Wed, 20 Jan 2016 13:28:22 -0500 Date: Wed, 20 Jan 2016 16:28:17 -0200 From: Gustavo Padovan To: Maarten Lankhorst Cc: Greg Kroah-Hartman , devel@driverdev.osuosl.org, daniels@collabora.com, Daniel Vetter , Riley Andrews , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Gustavo Padovan , John Harrison Subject: Re: [RFC 00/29] De-stage android's sync framework Message-ID: <20160120182817.GA2502@joana> Mail-Followup-To: Gustavo Padovan , Maarten Lankhorst , Greg Kroah-Hartman , devel@driverdev.osuosl.org, daniels@collabora.com, Daniel Vetter , Riley Andrews , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Gustavo Padovan , John Harrison References: <1452869739-3304-1-git-send-email-gustavo@padovan.org> <569F614A.1070807@linux.intel.com> <20160120143250.GF8217@joana> <569FA187.2060601@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <569FA187.2060601@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2016-01-20 Maarten Lankhorst : > 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. Fair enough. I can change the naming. Also the final idea is to move fence_ops to be fence_timeline_ops so the naming makes sense. > >> 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. One of the first patches of my series is removing struct sync_pt, that means I am also removing sync_fence_create_dma() because sync_fence_create() now accepts struct fence as parameter. I don't sync_pt being useful for any in-kernel driver as it is just a wrapper on top of struct fence. Gustavo