Re: [RFC 00/29] De-stage android's sync framework

[Tomeu Vizoso <tomeu@tomeuvizoso.net>]

On 19 January 2016 at 17:12, John Harrison <John.C.Harrison@intel.com> wrote:
> On 19/01/2016 15:23, Gustavo Padovan wrote:
>>
>> Hi Daniel,
>>
>> 2016-01-19 Daniel Vetter <daniel@ffwll.ch>:
>>
>>> On Fri, Jan 15, 2016 at 12:55:10PM -0200, Gustavo Padovan wrote:
>>>>
>>>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>>>
>>>> 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.

Hi John,

I'm working on making the tests in igt useful for drivers other than
i915 and would love to have tests for the fence functionality. Have
you made any progress since you posted that RFC?

Thanks,

Tomeu