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

[Christian König]

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.

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.