From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [RFC 00/29] De-stage android's sync framework Date: Wed, 20 Jan 2016 17:29:32 +0100 Message-ID: <20160120162932.GG19130@phenom.ffwll.local> 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-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by gabe.freedesktop.org (Postfix) with ESMTPS id 519586E989 for ; Wed, 20 Jan 2016 08:29:28 -0800 (PST) Received: by mail-wm0-f48.google.com with SMTP id n5so38240023wmn.0 for ; Wed, 20 Jan 2016 08:29:28 -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 T24gV2VkLCBKYW4gMjAsIDIwMTYgYXQgMDQ6MDI6MzFQTSArMDEwMCwgTWFhcnRlbiBMYW5raG9y c3Qgd3JvdGU6Cj4gT3AgMjAtMDEtMTYgb20gMTU6MzIgc2NocmVlZiBHdXN0YXZvIFBhZG92YW46 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 aXMgdGhlIGNhc2UuCj4gPj4gSSBhbHNvIGJlbGlldmUgdGhpcyBzaG91bGQgYmUgZG9uZSBpbiBt dWx0aXBsZSBzZXJpZXMuIEZpcnN0IHNlcmllcyBzaG91bGQgZGUtc3RhZ2UgdGhlIHVzZXJzcGFj ZSBmZW5jZSBmcmFtZXdvcmsuIFRoZSBuZXh0IHNlcmllcyBzaG91bGQgZml4IHVwIGFuZHJvaWRf ZmVuY2UgYW5kIG1heWJlIHJlbmFtZSBpdCB0byB0aW1lbGluZV9mZW5jZSBzaW5jZSBzeW5jX2Zl bmNlIGlzIGFscmVhZHkgdXNlZCBmb3IgdGhlIHVzZXJzcGFjZSBmZCwgd2hpY2ggd291bGQgYWRk IG1vcmUgY29uZnVzaW9uPwo+ID4gU3VyZS4gSSd2ZSBiZWVuIHRoaW5raW5nIG9uIGhvdyB0byBz cGxpdCB0aGlzIHByb3Blcmx5LiBJJ20gdHJ5aW5nIHRvCj4gPiBhZGQgYSBidW5jaCBvZiBjbGVh biB1cC9yZW5hbWluZyBmaXJzdCwgZWcgdGhlIHN5bmNfZmVuY2UgcmVuYW1lIHRvCj4gPiBzeW5j X2ZpbGUgdGhhdCBEYW5pZWwgVmV0dGVyIGFuZCBJIGRpc2N1c3NlZC4gCj4gPgo+ID4gTmV4dCBt eSBwbGFuIHdvdWxkIGJlIG1vdmUgc3luY190aW1lbGluZSB0byBmZW5jZV90aW1lbGluZSwgYWRk IHRoZQo+ID4gZmVuY2VfZGVmYXVsdC4uIGZlbmNlX29wcywgY2xlYW4gdXAgc3dfc3luYyBhbmQg ZmluYWxseSBtZXJnZQo+ID4gZmVuY2VfY29udGV4dCBhbmQgZmVuY2VfdGltZWxpbmUuCj4gPgo+ ID4gTG9va2luZyBhdCBob3cgc3luYyBhbmQgZmVuY2UgSXQgbG9va3MgZWFzaWVyIHRvIG1lIHRv IGRlLXN0YWdlIHN5bmNfdGltZWxpbmUgZmlyc3QgdGhhbiB1c2Vyc3BhY2UKPiA+IGZlbmNlLgo+ IFRoZXJlJ3MgYWxyZWFkeSBjb2RlIHRvIGFkZCBhIHN5bmNfZmVuY2VfY3JlYXRlX2RtYSBleHBv cnQgWzFdWzJdLiBTbyBpZiB5b3Ugd2FudCB0byBkZS1zdGFnZSBpdCB0aGVuIHRoZXJlIHdpbGwg YmUgdXNlcnMgZm9yIGl0Lgo+IAo+IHN5bmNfcHQgb3RvaCBoYXMgbm8gdXBzdHJlYW0gaW4ta2Vy bmVsIHVzZXJzLiBJdCB3YXMgYSB3cmFwcGVyIHRvIGtlZXAgYW5kcm9pZCBkcml2ZXJzIGFwaSBj b21wYXRpYmxlIHdpdGggdGhlIGZlbmNlIGFwaS4KPiAKPiBbMV0gaHR0cHM6Ly9wYXRjaHdvcmsu ZnJlZWRlc2t0b3Aub3JnL3BhdGNoLzY3ODQ1Lwo+IFsyXSBodHRwczovL3BhdGNod29yay5mcmVl ZGVza3RvcC5vcmcvcGF0Y2gvNjc4NDYvCgpXZSBraW5kYSBkb24ndCBoYXZlIG9wZW4tc291cmNl IHVzZXJzcGFjZSBmb3IgdGhpcyBzdHVmIHRob3VnaCAuLi4KLURhbmllbAotLSAKRGFuaWVsIFZl dHRlcgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KaHR0cDovL2Jsb2cuZmZ3 bGwuY2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6 Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934588AbcATQ3b (ORCPT ); Wed, 20 Jan 2016 11:29:31 -0500 Received: from mail-wm0-f43.google.com ([74.125.82.43]:37926 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934015AbcATQ32 (ORCPT ); Wed, 20 Jan 2016 11:29:28 -0500 Date: Wed, 20 Jan 2016 17:29:32 +0100 From: Daniel Vetter To: Maarten Lankhorst Cc: 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, Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Gustavo Padovan , John Harrison Subject: Re: [RFC 00/29] De-stage android's sync framework Message-ID: <20160120162932.GG19130@phenom.ffwll.local> Mail-Followup-To: Maarten Lankhorst , Gustavo Padovan , Greg Kroah-Hartman , devel@driverdev.osuosl.org, daniels@collabora.com, 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> X-Operating-System: Linux phenom 4.3.0-1-amd64 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 On Wed, Jan 20, 2016 at 04:02:31PM +0100, Maarten Lankhorst wrote: > 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/ We kinda don't have open-source userspace for this stuf though ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch