Am 13.09.2018 um 11:35 schrieb Zhou, David(ChunMing):
-----Original Message-----
From: Koenig, Christian
Sent: Thursday, September 13, 2018 5:20 PM
To: Zhou, David(ChunMing) <David1.Zhou@xxxxxxx>; dri-
devel@xxxxxxxxxxxxxxxxxxxxx
Cc: Dave Airlie <airlied@xxxxxxxxxx>; Rakos, Daniel
<Daniel.Rakos@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 1/3] [RFC]drm: add syncobj timeline support v4
Am 13.09.2018 um 11:11 schrieb Zhou, David(ChunMing):
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Thursday, September 13, 2018 4:50 PM
To: Zhou, David(ChunMing) <David1.Zhou@xxxxxxx>; Koenig, Christian
<Christian.Koenig@xxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx
Cc: Dave Airlie <airlied@xxxxxxxxxx>; Rakos, Daniel
<Daniel.Rakos@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 1/3] [RFC]drm: add syncobj timeline support v4
Am 13.09.2018 um 09:43 schrieb Zhou, David(ChunMing):
-----Original Message-----
From: Koenig, Christian
Sent: Thursday, September 13, 2018 2:56 PM
To: Zhou, David(ChunMing) <David1.Zhou@xxxxxxx>; Zhou,
David(ChunMing) <David1.Zhou@xxxxxxx>; dri-
devel@xxxxxxxxxxxxxxxxxxxxx
Cc: Dave Airlie <airlied@xxxxxxxxxx>; Rakos, Daniel
<Daniel.Rakos@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 1/3] [RFC]drm: add syncobj timeline support v4
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.
So do you mean we need still use old syncobj CB for that?
Yes, as far as I can see it should work.
Advanced wait pt is bad?
Well it isn't bad, I just don't see any advantage in it.
The advantage is to replace old syncobj cb.
The existing mechanism
should already be able to handle that.
I thought more a bit, we don't that mechanism at all, if use advanced wait
pt, we can easily use fence array to achieve it for wait ioctl, we should use
kernel existing feature as much as possible, not invent another, shouldn't we?
I remember you said it before.
Yeah, but the syncobj cb is an existing feature.
This is obviously a workaround when doing for wait ioctl, Do you see it used in other place?
And I absolutely don't see a
need to modify that and replace it with something far more complex.
The wait ioctl is simplified much more by fence array, not complex, and we just need to allocate a wait pt. If keeping old syncobj cb workaround, all wait pt logic still is there, just save allocation and wait pt handling, in fact, which part isn't complex at all. But compare with ugly syncobj cb, which is simpler.
I strongly disagree on that. You just need to extend the syncobj cb with
the sequence number and you are done.
We could clean that up in the long term by adding some wait_multi event
macro, but for now just adding the sequence number should do the trick.
Regards,
Christian.
Thanks,
David Zhou
Regards,
Christian.
Thanks,
David Zhou
Christian.
Thanks,
David Zhou
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.
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
