[Mesa-dev] [PATCH 1/2] i965: Throttle rendering to an fbo

[Chad Versace]

On 03/06/2015 06:56 AM, Chris Wilson wrote:
> When rendering to an fbo, even though it may be acting as a winsys
> frontbuffer or just generally, we never throttle. However, when rendering
> to an fbo, there is no natural frame boundary. Conventionally we use
> SwapBuffers and glFinish, but potential callers avoid often glFinish for
> being too heavy handed (waiting on all outstanding rendering to complete).
> The kernel provides a soft-throttling option for this case that waits for
> rendering older than 20ms to be complete (that's a little too lax to be
> used for swapbuffers, but is here a useful safety net). The remaining
> choice is then either never to throttle, throttle after every draw call,
> or at after intermediate user defined point such as glFlush and thus all the
> implied flushes. This patch opts for the latter as that is the current
> method used for flushing to front buffers.
> 
> v2: Defer the throttling from inside the flush to the next
> intel_prepare_render() and switch non-fbo frontbuffer throttling over to
> use the same lax method. The issuing being that
> glFlush()/intel_prepare_read() is just as likely to be called inside a
> tight loop and not at "frame" boundaries.

Thanks for the change. Moving the throttle to intel_prepare_render() makes
sense to me. Comments below.


> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 972e458..2ed5f16 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -232,8 +232,8 @@ intel_glFlush(struct gl_context *ctx)
>  
>     intel_batchbuffer_flush(brw);
>     intel_flush_front(ctx);
> -   if (brw_is_front_buffer_drawing(ctx->DrawBuffer))
> -      brw->need_throttle = true;
> +
> +   brw->need_front_throttle = true;
>  }

Because of the variable name need_front_throttle, this code looks incorrect for fbo
rendering, even thought it's not. Please add a comment here explaining that
need_front_throttle is being hijacked for fbo, back, and front buffer rendering.

With that little comment, patch 1 is
Reviewed-by: Chad Versace <chad.versace at intel.com>

I'm still looking at patch 2.