[Intel-gfx] [PATCH 15/18] drm/i915: Reserve space in the global seqno during request allocation

[Joonas Lahtinen]

On ma, 2016-09-19 at 18:35 +0300, Jani Nikula wrote:
> > On Mon, 19 Sep 2016, Joonas Lahtinen <joonas.lahtinen at linux.intel.com> wrote:
> > 
> > On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> > > 
> > > +static int reserve_global_seqno(struct drm_i915_private *i915)
> > >  {
> > > -	struct i915_gem_timeline *tl = &dev_priv->gt.global_timeline;
> > > +	struct i915_gem_timeline *tl = &i915->gt.global_timeline;
> > > +	u32 next_seqno = atomic_read(&tl->next_seqno);
> > >  
> > > -	/* reserve 0 for non-seqno */
> > > -	if (unlikely(tl->next_seqno == 0)) {
> > 
> > Meh, do not hide the ++i915->gt.active_requests in if (unlikely(...)).
> 
> *shudder* it doesn't get any better by removing unlikely!
> 

Well, it gets twice as bad by adding it, though :P

My below recommendation would be preferred by lifting the increment
before the if ().

> > > 
> > > +	if (unlikely(next_seqno + ++i915->gt.active_requests <= next_seqno)) {
> > >  		int ret;
> > >  
> > 
> > Why not if (likely(next_seqno + active_requests > next_seqno))
> > > > 		return 0;

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation