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: Tue, 19 Jan 2016 16:15:10 -0200 Message-ID: <20160119181510.GC8217@joana> References: <1452869739-3304-1-git-send-email-gustavo@padovan.org> <20160119110017.GZ19130@phenom.ffwll.local> <20160119152309.GA8217@joana> <569E6062.6030309@Intel.com> <20160119175226.GB8217@joana> <20160119180414.GK19130@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20160119180414.GK19130@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: John Harrison , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, daniels@collabora.com, Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Rob Clark , Greg Hackmann , Maarten Lankhorst , Gustavo Padovan List-Id: dri-devel@lists.freedesktop.org 2016-01-19 Daniel Vetter : > On Tue, Jan 19, 2016 at 03:52:26PM -0200, Gustavo Padovan wrote: > > 2016-01-19 John Harrison : > > > > > On 19/01/2016 15:23, Gustavo Padovan wrote: > > > >Hi Daniel, > > > > > > > >2016-01-19 Daniel Vetter : > > > > > > > >>On Fri, Jan 15, 2016 at 12:55:10PM -0200, Gustavo Padovan wrote: > > > >>>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 > > > >>Bunch of fairly random comments all over: > > > >> > > > >>- include/uapi/linux/sw_sync.h imo should be dropped, it's just a private > > > >> debugfs interface between fence fds and the testsuite. Since the plan is > > > >> to have the testcases integrated into the kernel tree too we don't need > > > >> a public header. > > > >> > > > >>- similar for include/linux/sw_sync.h Imo that should all be moved into > > > >> sync_debug.c. Same for sw_sync.c, that should all land in sync_debug > > > >> imo, and made optional with a Kconfig option. At least we should reuse > > > >> CONFIG_DEBUGFS. > > > >These two items sounds reasonable to me. > > > > > > I have just posted our in-progress IGT for testing i915 syncs (with a CC of > > > Gustavo). It uses the sw_sync mechanisms. Can you take a quick look and see > > > if it is the kind of thing you would expect us to be doing? Or is it using > > > interfaces that you are planning to remove and/or make kernel only? > > > > > > I'm not sure having a kernel only test is the best way to go. Having user > > > land tests like IGT would be much more versatile. > > > > I agree with you, we should allow IGT and other test tools to access > > sw_sync. include/linux/sw_sync.h can be kept private, but the uapi one > > needs wil be needed for testing, unless we replicate the header file > > inside IGT, but not sure if it is a good idea. > > We replicate all the debugfs stuff in igt that igt needs. uapi really only > should be stuff that's guaranteed to stick around, not debug interfaces we > are ok with breaking (if needed). Okay, that sounds quite good for me. Gustavo