RE: [PATCH 1/3] [RFC]drm: add syncobj timeline support v4

["Zhou, David(ChunMing)" <David1.Zhou-5C7GfCeVMHo@public.gmane.org>]

> -----Original Message-----
> From: Koenig, Christian
> Sent: Thursday, September 13, 2018 2:56 PM
> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Zhou,
> David(ChunMing) <David1.Zhou@amd.com>; dri-
> devel@lists.freedesktop.org
> Cc: Dave Airlie <airlied@redhat.com>; Rakos, Daniel
> <Daniel.Rakos@amd.com>; amd-gfx@lists.freedesktop.org
> 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? Advanced wait pt is bad?

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