On Thu, Apr 27, 2017 at 09:34:10PM +0100, Chris Wilson wrote:
> 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?
Went with i915_syncmap (struct, .c, .h)
There's some knowlege of seqno built in (i.e the is_later function).
-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
