On 03/06/2015 06:56 AM, Chris Wilson wrote:
> In order to facilitate the concurrency offered by triple buffering and to
> offset the latency induced by swapping via an external process, which
> may incur extra rendering itself, only throttle to the previous frame
> and not the last. This doubles the maximum possible latency at the
> benefit of improving throughput and reducing jitter.
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> Cc: Kenneth Graunke <kenn...@whitecape.org>
> Cc: Ben Widawsky <b...@bwidawsk.net>
> Cc: Kristian Høgsberg <k...@bitplanet.net>
> Cc: Chad Versace <chad.vers...@linux.intel.com
> ---
> src/mesa/drivers/dri/i965/brw_context.c | 19 ++++++++++++-------
> src/mesa/drivers/dri/i965/brw_context.h | 2 +-
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 7 ++++---
> 3 files changed, 17 insertions(+), 11 deletions(-)
I'm ok with the patch's idea. Just please rename the variable. It's badly named
now. The batches are used for more than just "swapbuffers". And it holds two
batches,
but how can there be two "first" batches?
How about brw->post_throttle_batches? Any name without "first" or "swap" would
be fine.
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 2ed5f16..6897c2c 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -928,8 +928,10 @@ intelDestroyContext(__DRIcontext * driContextPriv)
>
> intel_batchbuffer_free(brw);
>
> - drm_intel_bo_unreference(brw->first_post_swapbuffers_batch);
> - brw->first_post_swapbuffers_batch = NULL;
> + drm_intel_bo_unreference(brw->first_post_swapbuffers_batch[1]);
> + drm_intel_bo_unreference(brw->first_post_swapbuffers_batch[0]);
> + brw->first_post_swapbuffers_batch[1] = NULL;
> + brw->first_post_swapbuffers_batch[0] = NULL;
>
> driDestroyOptionCache(&brw->optionCache);
>
> @@ -1238,11 +1240,14 @@ intel_prepare_render(struct brw_context *brw)
> * the swap, and getting our hands on that doesn't seem worth it,
> * so we just us the first batch we emitted after the last swap.
> */
> - if (brw->need_swap_throttle && brw->first_post_swapbuffers_batch) {
> - if (!brw->disable_throttling)
> - drm_intel_bo_wait_rendering(brw->first_post_swapbuffers_batch);
> - drm_intel_bo_unreference(brw->first_post_swapbuffers_batch);
> - brw->first_post_swapbuffers_batch = NULL;
> + if (brw->need_swap_throttle && brw->first_post_swapbuffers_batch[0]) {
> + if (brw->first_post_swapbuffers_batch[1]) {
> + if (!brw->disable_throttling)
> +
> drm_intel_bo_wait_rendering(brw->first_post_swapbuffers_batch[1]);
> + drm_intel_bo_unreference(brw->first_post_swapbuffers_batch[1]);
> + }
> + brw->first_post_swapbuffers_batch[1] =
> brw->first_post_swapbuffers_batch[0];
> + brw->first_post_swapbuffers_batch[0] = NULL;
> brw->need_swap_throttle = false;
> brw->need_front_throttle = false;
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index b90e050..e347f26 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1030,7 +1030,7 @@ struct brw_context
> bool front_buffer_dirty;
>
> /** Framerate throttling: @{ */
> - drm_intel_bo *first_post_swapbuffers_batch;
> + drm_intel_bo *first_post_swapbuffers_batch[2];
> bool need_swap_throttle;
> bool need_front_throttle;
> /** @} */
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 5ac4d18..460b4b9 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -168,6 +168,7 @@ static void
> brw_new_batch(struct brw_context *brw)
> {
> /* Create a new batchbuffer and reset the associated state: */
> + drm_intel_gem_bo_clear_relocs(brw->batch.bo, 0);
> intel_batchbuffer_reset(brw);
>
> /* If the kernel supports hardware contexts, then most hardware state is
> @@ -289,9 +290,9 @@ _intel_batchbuffer_flush(struct brw_context *brw,
> if (brw->batch.used == 0)
> return 0;
>
> - if (brw->first_post_swapbuffers_batch == NULL) {
> - brw->first_post_swapbuffers_batch = brw->batch.bo;
> - drm_intel_bo_reference(brw->first_post_swapbuffers_batch);
> + if (brw->first_post_swapbuffers_batch[0] == NULL) {
> + brw->first_post_swapbuffers_batch[0] = brw->batch.bo;
> + drm_intel_bo_reference(brw->first_post_swapbuffers_batch[0]);
> }
>
> if (unlikely(INTEL_DEBUG & DEBUG_BATCH)) {
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
