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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2018年10月19日 17:20, zhoucm1 wrote:


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@xxxxxxx> 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.
In fact, the bug is already caught and fixed, just the fix part isn't in patch#1, but in patch#2:

Have you reverted? If not, I can send that fix in one minute.


Regards,
David Zhou

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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux