On Tue, Feb 14, 2017 at 01:13:58PM +0200, Joonas Lahtinen wrote:
> On to, 2017-02-02 at 15:12 +0000, Chris Wilson wrote:
> > -static int reserve_global_seqno(struct drm_i915_private *i915)
> > +static int reserve_global_seqno(struct intel_engine_cs *engine)
> > {
> > - u32 active_requests = ++i915->gt.active_requests;
> > - u32 seqno = atomic_read(&i915->gt.global_timeline.seqno);
> > + u32 active = ++engine->timeline->active_seqno;
> > + u32 seqno = engine->timeline->seqno;
> > int ret;
> >
> > /* Reservation is fine until we need to wrap around */
> > - if (likely(seqno + active_requests > seqno))
> > + if (likely(seqno + active > seqno))
> > return 0;
> >
>
> Make a comment here that we don't currently track the semaphores
> waiting on specific engine, so we instead choose to wrap all of them.
>
> > - ret = i915_gem_init_global_seqno(i915, 0);
> > + ret = i915_gem_init_global_seqno(engine->i915, 0);
>
> Also, we don't have a global seqno anymore, so the function name needs
> updating. Maybe "init_{all_}global_seqnos" to make it clear.
However, i915_gem_init_global_seqno() is acting globally. It's debug code
but I think the implication is important to keep. reserve_global_seqno()
is now local and that feels appropriate to rename.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
