Re: [PATCH 13/18] drm/i915: Move the global sync optimisation to the timeline

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> @@ -262,6 +263,12 @@ static int i915_gem_init_global_seqno(struct drm_i915_private *dev_priv,
>  	for_each_engine(engine, dev_priv)
>  		intel_engine_init_global_seqno(engine, seqno);
>  
> +	list_for_each_entry(tl, &dev_priv->gt.timelines, link) {
> +		for (i = 0; i < ARRAY_SIZE(tl->engine); i++)
> +			memset(tl->engine[i].sync_seqno, 0,
> +			       sizeof(tl->engine[i].sync_seqno));
> +	}
> +

I think have braces in both loops or neither, looks extremely odd
otherwise.

> @@ -454,7 +461,7 @@ static int
>  i915_gem_request_await_request(struct drm_i915_gem_request *to,
> >  			       struct drm_i915_gem_request *from)
>  {
> > -	int idx, ret;
> > +	int ret;
>  
> >  	GEM_BUG_ON(to == from);
>  
> @@ -474,8 +481,7 @@ i915_gem_request_await_request(struct drm_i915_gem_request *to,
>  		return ret < 0 ? ret : 0;
>  	}
>  
> -	idx = intel_engine_sync_index(from->engine, to->engine);

You remove this here, and the only place it remains used
is i915_gpu_error.c, maybe make it static in there?

> @@ -263,16 +263,13 @@ static void error_print_engine(struct drm_i915_error_state_buf *m,
>  	if (INTEL_GEN(m->i915) >= 6) {
>  		err_printf(m, "  RC PSMI: 0x%08x\n", ee->rc_psmi);
>  		err_printf(m, "  FAULT_REG: 0x%08x\n", ee->fault_reg);
> -		err_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
> -			   ee->semaphore_mboxes[0],
> -			   ee->semaphore_seqno[0]);
> -		err_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
> -			   ee->semaphore_mboxes[1],
> -			   ee->semaphore_seqno[1]);
> +		err_printf(m, "  SYNC_0: 0x%08x\n",
> +			   ee->semaphore_mboxes[0]);
> +		err_printf(m, "  SYNC_1: 0x%08x\n",
> +			   ee->semaphore_mboxes[1]);
>  		if (HAS_VEBOX(m->i915)) {
> -			err_printf(m, "  SYNC_2: 0x%08x [last synced 0x%08x]\n",
> -				   ee->semaphore_mboxes[2],
> -				   ee->semaphore_seqno[2]);
> +			err_printf(m, "  SYNC_2: 0x%08x\n",
> +				   ee->semaphore_mboxes[2]);
>  		}

You can drop these braces too.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx