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

Reply via email to