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

[zhoucm1 <zhoucm1@amd.com>]

On 2018年10月19日 16:55, Daniel Vetter wrote:
> On Fri, Oct 19, 2018 at 10:29:55AM +0800, zhoucm1 wrote:
>>
>> On 2018年10月18日 19:50, Christian König wrote:
>>> 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@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 :)
>> Thanks a lot.
> (This time reply to the right patch, silly me)
>
> Went boom:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=108490
>
> Can we revert pls?
Sorry for bug, please.
>
> Also, can we please have igts for this stuff so that intel-gfx-ci could
> test this properly before it's all fireworks?
Seems we cannot avoid igt now and vulkan CTS isn't enough, I will find 
some time next week to lean IGT, looks v10 is need.

Regards,
David Zhou
>
> Thanks, Daniel
>
>>> The rest in the series looks good to me as well,
>> Can I get your RB on them first?
>>
>>> but I certainly want the radv/anv developers to take a look as well as
>>> Daniel suggested.
>> Ping @Dave/Bas/Jason or other radv/anv developers, Could anyone of you take
>> a look the rest of series for u/k interface? So that we can move to next
>> step for libdrm patches?
>>
>> Thanks,
>> David
>>> Christian.
>>>
>>>> Thanks,
>>>> David Zhou
>>>>
>>>>> -Daniel
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel