Quoting Tvrtko Ursulin (2018-04-24 11:14:21)
>
> On 23/04/2018 19:08, Chris Wilson wrote:
> > -static int reserve_engine(struct intel_engine_cs *engine)
> > +static int reserve_gt(struct drm_i915_private *i915)
> > {
> > - struct drm_i915_private *i915 = engine->i915;
> > - u32 active = ++engine->timeline->inflight_seqnos;
> > - u32 seqno = engine->timeline->seqno;
> > int ret;
> >
> > - /* Reservation is fine until we need to wrap around */
> > - if (unlikely(add_overflows(seqno, active))) {
> > + /*
> > + * Reservation is fine until we may need to wrap around
> > + *
> > + * By incrementing the serial for every request, we know that no
> > + * individual engine may exceed that serial (as each is reset to 0
> > + * on any wrap). This protects even the most pessimistic of migrations
> > + * of every request from all engines onto just one.
> > + */
>
> I didn't really figure out what was wrong with v1? Neither could handle
> more than four billion of simultaneously active requests - but I thought
> that should not concern us. :)
It was still using the local engine->timeline.seqno as it's base. If we
swapped from one at 0 to another at U32_MAX, we would overflow much
later in submission; after the point of no return.
> I don't quite get wrapping based on otherwise unused counter.
>
> v1 made sense to wrap then any engine timeline couldn't handle all
> active requests without wrapping.
Emphasis on *any*, not just the current engine.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
