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

[Chris Wilson]

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