[Intel-gfx] [PATCH 3/3] drm/i915: Updating comments.

[Daniel Vetter]

On Mon, Jun 30, 2014 at 06:16:50PM -0700, Ben Widawsky wrote:
> On Mon, Jun 30, 2014 at 09:51:11AM -0700, Rodrigo Vivi wrote:
> > ring index calculation table was out of date after other rings were added,
> > although the formula is flexible and scale when adding new rings.
> > 
> > So this patch just update the comments and add a brief explanation
> > why to use sync_seqno[ring index].
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c         | 2 ++
> >  drivers/gpu/drm/i915/intel_ringbuffer.h | 8 +++++---
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index f6d1238..e85c85c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2842,6 +2842,8 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> >  	idx = intel_ring_sync_index(from, to);
> >  
> >  	seqno = obj->last_read_seqno;
> > +	/* Optimization: Avoid semaphore sync when we are sure we already
> > +	 * waited for an object with higher seqno */
> >  	if (seqno <= from->semaphore.sync_seqno[idx])
> >  		return 0;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index e72017b..2e8b516 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -238,9 +238,11 @@ intel_ring_sync_index(struct intel_engine_cs *ring,
> >  	int idx;
> >  
> >  	/*
> > -	 * cs -> 0 = vcs, 1 = bcs
> > -	 * vcs -> 0 = bcs, 1 = cs,
> > -	 * bcs -> 0 = cs, 1 = vcs.
> > +	 * rcs -> 0 = vcs, 1 = bcs, 2 = vecs, 3 = vcs2;
> > +	 * vcs -> 0 = bcs, 1 = vecs, 2 = vcs2, 3 = rcs;
> > +	 * bcs -> 0 = vecs, 1 = vcs2. 2 = rcs, 3 = vcs;
> > +	 * vecs -> 0 = vcs2, 1 = rcs, 2 = vcs, 3 = bcs;
> > +	 * vcs2 -> 0 = rcs, 1 = vcs, 2 = bcs, 3 = vecs;
> >  	 */
> 
> I'd be a favor of dropping this table, and instead explaining the goal
> of the math (to save the dword)

tbh I don't mind either way ...

> >  
> >  	idx = (other - ring) - 1;
> 
> I'm guessing this hunk is from your private branch?

Applied here without fuzz ...

> In any event, the topmost comment is a nice addition:

Indeed.

> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch