On Thu, Apr 27, 2017 at 06:25:47PM +0100, Chris Wilson wrote:
> On Thu, Apr 27, 2017 at 05:47:32PM +0100, Tvrtko Ursulin wrote:
> > >+int intel_timeline_sync_set(struct intel_timeline *tl, u64 id, u32 seqno)
> > >+{
> > >+ struct intel_timeline_sync *p = tl->sync;
> > >+
> > >+ /* We expect to be called in sequence following a _get(id), which
> > >+ * should have preloaded the tl->sync hint for us.
> > >+ */
> > >+ if (likely(p && (id >> SHIFT) == p->prefix)) {
> > >+ unsigned int idx = id & MASK;
> > >+
> > >+ __sync_seqno(p)[idx] = seqno;
> > >+ p->bitmap |= BIT(idx);
> > >+ return 0;
> > >+ }
> > >+
> > >+ return __intel_timeline_sync_set(tl, id, seqno);
> >
> > Could pass in p and set tl->sync = p at this level. That would
> > decouple the algorithm from the timeline better. With equivalent
> > treatment for the query, and renaming of struct intel_timeline_sync,
> > algorithm would be ready for moving out of drm/i915/ :)
>
> I really did want to keep this as a tail call to keep the fast path neat
> and tidy with minimal stack manipulation.
Happier with
_intel_timeline_sync_set(struct intel_timeline_sync **root,
u64 id, u32 seqno)
{
struct intel_timeline_sync *p = *root;
...
*root = p;
return 0;
}
return __intel_timeline_sync_set(&tl->sync, id, seqno);
A little step towards abstraction. Works equally well for
intel_timeline_sync_is_later().
Hmm. i915_seqmap.c ? Too cryptic?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
