2016-01-19 John Harrison <John.C.Harrison@xxxxxxxxx>: > On 19/01/2016 15:23, Gustavo Padovan wrote: > >Hi Daniel, > > > >2016-01-19 Daniel Vetter <daniel@xxxxxxxx>: > > > >>On Fri, Jan 15, 2016 at 12:55:10PM -0200, Gustavo Padovan wrote: > >>>From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx> > >>> > >>>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 <debugfs>/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. Gustavo _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
