[Mesa-dev] [PATCH] i965: Defer the throttle until we submit new commands

[Chad Versace]

On 03/11/2015 05:36 AM, Chris Wilson wrote:
> Currently, we throttle before the user begins preparing commands for the
> next frame when we acquire the draw/read buffers. However, construction
> of the command buffer can itself take significant time relative to the
> frame time. If we move the throttle from the buffer acquire to the
> command submit phase we can allow the user to improve concurrency
> between the CPU and GPU (i.e. reduce the amount of time we waste inside
> the throttle).

Yes, more concurrency please. If Mesa is going to throttle, then throttling should
happen definitely immediately before submitting the batch, not immediately before
the user wants to use the cpu to build a new batch.

This patch solves my major complaint with the original patches you sent.
Reviewed-by: Chad Versace <chad.versace at intel.com>

But... please fix the style conventions in my comments below. Either way,
my r-b still stands.

> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Ben Widawsky <ben at bwidawsk.net>
> Cc: Kristian Høgsberg <krh at bitplanet.net>
> Cc: Chad Versace <chad.versace at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_context.c       | 34 ----------------------
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 41 +++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 34 deletions(-)
> 



> @@ -226,6 +229,43 @@ brw_finish_batch(struct brw_context *brw)
>     brw->cache.bo_used_by_gpu = true;
>  }
>  
> +static void throttle(struct brw_context *brw)

Convention in this file is:

static void
throttle(struct brw_context *brw)


>  static int
> @@ -260,6 +300,7 @@ do_flush_locked(struct brw_context *brw)
>        if (ret == 0) {
>           if (unlikely(INTEL_DEBUG & DEBUG_AUB))
>              brw_annotate_aub(brw);
> +	 throttle(brw);
>  	 if (brw->hw_ctx == NULL || batch->ring != RENDER_RING) {
>  	    ret = drm_intel_bo_mrb_exec(batch->bo, 4 * batch->used, NULL, 0, 0,
>  					flags);

Please put a newline above and below throttle(). Most i965 code is not so compact.