[PATCH 7/7] drm/syncobj: use the timeline point in drm_syncobj_find_fence

[zhoucm1]

On 2018年11月22日 19:30, Christian König wrote:
> Am 22.11.18 um 07:52 schrieb zhoucm1:
>>
>>
>> On 2018年11月15日 19:12, Christian König wrote:
>>> Implement finding the right timeline point in drm_syncobj_find_fence.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 10 +++++++++-
>>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index 589d884ccd58..d42c51520da4 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -307,9 +307,17 @@ int drm_syncobj_find_fence(struct drm_file 
>>> *file_private,
>>>           return -ENOENT;
>>>         *fence = drm_syncobj_fence_get(syncobj);
>>> -    if (!*fence) {
>>> +    if (!*fence)
>>>           ret = -EINVAL;
>>> +
>>> +    if (!ret && point) {
>>> +        dma_fence_chain_for_each(*fence) {
>>> +            if (!to_dma_fence_chain(*fence) ||
>>> +                (*fence)->seqno <= point)
>>> +                break;
>> This condition isn't enough to find proper point.
>> For two examples:
>> a. No garbage collection happens, the points in chain are 
>> 1----3----6----9----12---18---20, if user wants to get point17, then 
>> we should return node 18.
>
> And that is exactly what's wrong in the original logic. In this case 
> we need to return 12, not 18 because point 17 could have already been 
> garbage collected.
I don't think so, the 'a' case I already assume there isn't garbage 
collection. If user wants to get point17, then we should return node 18.
timeline means point[N]  must be signaled later than point[N-1].
Point[12] just can make sure point[1] ~point[12] are signaled.
Point[18] signal can make sure point[17] is signaled.
So this case we need to return 18, not 12, which is key timeline concept.

-David
>
>> b. garbage collection happens on point6, chain would be updated to 
>> 1---3---9---12---18---20, if user wants to get point5, then we should 
>> return node 3, but if user wants to get point 7, then we should 
>> return node 9.
>
> Why? That doesn't seem to make any sense to me.
>
>> I still have no idea how to satisfy all these requirements with your 
>> current chain-fence. All these logic just are same we encountered 
>> before, we're walking them again. After solving these problems, I 
>> guess all design is similar as before.
>>
>> In fact, I don't know what problem previous design has, maybe there 
>> are some bugs, can't we fix these bugs by time going? Who can make 
>> sure his implementation never have bugs?
>
> Well there where numerous problems with the original design. For 
> example we need to reject the requirement that timeline fences are in 
> order because that doesn't make sense in the kernel.
>
> When userspace does something like submitting fences in the order 1, 
> 5, 3 then it is broken and can keep the pieces. In other words the 
> kernel should not care about that, but rather make sure that it never 
> looses any synchronization no matter what.
>
> Regards,
> Christian.
>
>>
>>
>> -David
>>> +        }
>>>       }
>>> +
>>>       drm_syncobj_put(syncobj);
>>>       return ret;
>>>   }
>>
>