[Intel-gfx] [PATCH v9] drm/i915: Squash repeated awaits on the same fence

[Chris Wilson]

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