[PATCH 1/3] [RFC]drm: add syncobj timeline support v4

[Christian König]

Am 13.09.2018 um 09:43 schrieb Zhou, David(ChunMing):
>
>> -----Original Message-----
>> From: Koenig, Christian
>> Sent: Thursday, September 13, 2018 2:56 PM
>> To: Zhou, David(ChunMing) <David1.Zhou at amd.com>; Zhou,
>> David(ChunMing) <David1.Zhou at amd.com>; dri-
>> devel at lists.freedesktop.org
>> Cc: Dave Airlie <airlied at redhat.com>; Rakos, Daniel
>> <Daniel.Rakos at amd.com>; amd-gfx at lists.freedesktop.org
>> Subject: Re: [PATCH 1/3] [RFC]drm: add syncobj timeline support v4
>>
>> Am 13.09.2018 um 04:15 schrieb zhoucm1:
>>> On 2018年09月12日 19:05, Christian König wrote:
>>>>>>>> [SNIP]
>>>>>>>> +static void drm_syncobj_find_signal_pt_for_wait_pt(struct
>>>>>>>> drm_syncobj *syncobj,
>>>>>>>> +                           struct drm_syncobj_wait_pt *wait_pt)
>>>>>>>> +{
>>>>>>> That whole approach still looks horrible complicated to me.
>>>>> It's already very close to what you said before.
>>>>>
>>>>>>> Especially the separation of signal and wait pt is completely
>>>>>>> unnecessary as far as I can see.
>>>>>>> When a wait pt is requested we just need to search for the signal
>>>>>>> point which it will trigger.
>>>>> Yeah, I tried this, but when I implement cpu wait ioctl on specific
>>>>> point, we need a advanced wait pt fence, otherwise, we could still
>>>>> need old syncobj cb.
>>>> Why? I mean you just need to call drm_syncobj_find_fence() and when
>>>> that one returns NULL you use wait_event_*() to wait for a signal
>>>> point >= your wait point to appear and try again.
>>> e.g. when there are 3 syncobjs(A,B,C) to wait, all syncobjABC have no
>>> fence yet, as you said, during drm_syncobj_find_fence(A) is working on
>>> wait_event, syncobjB and syncobjC could already be signaled, then we
>>> don't know which one is first signaled, which is need when wait ioctl
>>> returns.
>> I don't really see a problem with that. When you wait for the first one you
>> need to wait for A,B,C at the same time anyway.
>>
>> So what you do is to register a fence callback on the fences you already have
>> and for the syncobj which doesn't yet have a fence you make sure that they
>> wake up your thread when they get one.
>>
>> So essentially exactly what drm_syncobj_fence_get_or_add_callback()
>> already does today.
> So do you mean we need still use old syncobj CB for that?

Yes, as far as I can see it should work.

>   Advanced wait pt is bad?

Well it isn't bad, I just don't see any advantage in it. The existing 
mechanism should already be able to handle that.

Christian.

>
> Thanks,
> David Zhou
>> Regards,
>> Christian.
>>
>>> Back to my implementation, it already fixes all your concerns before,
>>> and can be able to easily used in wait_ioctl. When you feel that is
>>> complicated, I guess that is because we merged all logic to that and
>>> much clean up in one patch. In fact, it already is very simple,
>>> timeline_init/fini, create signal/wait_pt, find signal_pt for wait_pt,
>>> garbage collection, just them.
>>>
>>> Thanks,
>>> David Zhou
>>>> Regards,
>>>> Christian.
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx