[PATCH 2/7] drm: add syncobj timeline support v8

[Christian König]

Am 18.10.18 um 05:11 schrieb zhoucm1:
>
>
> On 2018年10月17日 18:24, Daniel Vetter wrote:
>> On Wed, Oct 17, 2018 at 11:29 AM Koenig, Christian
>> <Christian.Koenig at amd.com> wrote:
>>> Am 17.10.18 um 11:17 schrieb zhoucm1:
>>>> [SNIP]
>>>>>>    +struct drm_syncobj_signal_pt {
>>>>>> +    struct dma_fence_array *base;
>>>>> Out of curiosity, why the pointer and not embedding? base is kinda
>>>>> misleading for a pointer.
>>>> Yeah, Christian doesn't like signal_pt lifecycle same as fence, so
>>>> it's a pointer.
>>>> If you don't like 'base' name, I can change it.
>>> Well I never said that you can't embed the fence array into the 
>>> signal_pt.
>>>
>>> You just need to make sure that we don't affect the drm_syncobj
>>> lilecycle as well, e.g. that we don't also need to keep that around.
>> I don't see a problem with that, as long as drm_syncobj keeps a
>> reference to the fence while it's on the timeline list. Which it
>> already does. And embedding would avoid that 2nd separate allocation,
>> aside from making base less confusing.
> That's indeed my initial implementation for signal_pt/wait_pt with 
> fence based, but after long and many discussions, we get current 
> solution, as you see, the version is up to v8 :).
>
> For here  why the pointer and not embedding?
> Two reasons:
> 1. their lifecycles are not same.
> 2. It is a fence array usage, which always needs separate allocation, 
> seems which is mandatory.
> So it is a pointer.
>
> But the name is historical from initial, and indeed be kinda 
> misleading for a pointer, I will change it to fence_array instead in 
> coming v9.

To avoid running into a v10 I've just pushed this version upstream :)

The rest in the series looks good to me as well, but I certainly want 
the radv/anv developers to take a look as well as Daniel suggested.

Christian.

>
> Thanks,
> David Zhou
>
>> -Daniel
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx