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

[Ben Widawsky]

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)
>  
>  	idx = (other - ring) - 1;

I'm guessing this hunk is from your private branch?

In any event, the topmost comment is a nice addition:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

-- 
Ben Widawsky, Intel Open Source Technology Center