[Intel-gfx] [PATCH 1/2] drm/i915: Pull the unconditional GPU cache invalidation into request construction
Mika Kuoppala
mika.kuoppala at linux.intel.com
Mon Nov 20 15:26:57 UTC 2017
Chris Wilson <chris at chris-wilson.co.uk> writes:
> As the request now may implicitly invoke a context-switch, we should
> follow that with a GPU TLB invalidation. Also even before using GGTT, we
s/follow/preample? As it is the context-switch you are preampling.
Also point that the request allocation will invote a context-switch
but only after next patch.
So with some confusion untangled on these first lines,
Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> should invalidate the TLBs for any updates (as well as the ppgtt
> invalidates that are unconditionally applied by execbuf). Since we
> almost always require the TLB invalidate, do it unconditionally on
> request allocation and so we can remove it from all other paths.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +------
> drivers/gpu/drm/i915/i915_gem_render_state.c | 4 ----
> drivers/gpu/drm/i915/i915_gem_request.c | 24 ++++++++++++++++++-----
> drivers/gpu/drm/i915/selftests/huge_pages.c | 4 ----
> drivers/gpu/drm/i915/selftests/i915_gem_context.c | 4 ----
> drivers/gpu/drm/i915/selftests/i915_gem_request.c | 10 ----------
> drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 4 ----
> 7 files changed, 20 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 53ccb27bfe91..b7895788bc75 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1111,10 +1111,6 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
> if (err)
> goto err_request;
>
> - err = eb->engine->emit_flush(rq, EMIT_INVALIDATE);
> - if (err)
> - goto err_request;
> -
> err = i915_switch_context(rq);
> if (err)
> goto err_request;
> @@ -1818,8 +1814,7 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
> /* Unconditionally flush any chipset caches (for streaming writes). */
> i915_gem_chipset_flush(eb->i915);
>
> - /* Unconditionally invalidate GPU caches and TLBs. */
> - return eb->engine->emit_flush(eb->request, EMIT_INVALIDATE);
> + return 0;
> }
>
> static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> index c2723a06fbb4..f7fc0df251ac 100644
> --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -208,10 +208,6 @@ int i915_gem_render_state_emit(struct drm_i915_gem_request *rq)
> if (err)
> goto err_unpin;
>
> - err = engine->emit_flush(rq, EMIT_INVALIDATE);
> - if (err)
> - goto err_unpin;
> -
> err = engine->emit_bb_start(rq,
> so.batch_offset, so.batch_size,
> I915_DISPATCH_SECURE);
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index e0d6221022a8..91eae1b20c42 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -703,17 +703,31 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
> req->reserved_space = MIN_SPACE_FOR_ADD_REQUEST;
> GEM_BUG_ON(req->reserved_space < engine->emit_breadcrumb_sz);
>
> - ret = engine->request_alloc(req);
> - if (ret)
> - goto err_ctx;
> -
> - /* Record the position of the start of the request so that
> + /*
> + * Record the position of the start of the request so that
> * should we detect the updated seqno part-way through the
> * GPU processing the request, we never over-estimate the
> * position of the head.
> */
> req->head = req->ring->emit;
>
> + /* Unconditionally invalidate GPU caches and TLBs. */
> + ret = engine->emit_flush(req, EMIT_INVALIDATE);
> + if (ret)
> + goto err_ctx;
> +
> + ret = engine->request_alloc(req);
> + if (ret) {
> + /*
> + * Past the point-of-no-return. Since we may have updated
> + * global state after partially completing the request alloc,
> + * we need to commit any commands so far emitted in the
> + * request to the HW.
> + */
> + __i915_add_request(req, false);
> + return ERR_PTR(ret);
> + }
> +
> /* Check that we didn't interrupt ourselves with a new request */
> GEM_BUG_ON(req->timeline->seqno != req->fence.seqno);
> return req;
> diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c
> index 01af540b6ef9..159a2cb68765 100644
> --- a/drivers/gpu/drm/i915/selftests/huge_pages.c
> +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
> @@ -989,10 +989,6 @@ static int gpu_write(struct i915_vma *vma,
> i915_vma_unpin(batch);
> i915_vma_close(batch);
>
> - err = rq->engine->emit_flush(rq, EMIT_INVALIDATE);
> - if (err)
> - goto err_request;
> -
> err = i915_switch_context(rq);
> if (err)
> goto err_request;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index c82780a9d455..4ff30b9af1fe 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -158,10 +158,6 @@ static int gpu_fill(struct drm_i915_gem_object *obj,
> goto err_batch;
> }
>
> - err = engine->emit_flush(rq, EMIT_INVALIDATE);
> - if (err)
> - goto err_request;
> -
> err = i915_switch_context(rq);
> if (err)
> goto err_request;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_request.c b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> index 6bce99050e94..d7bf53ff8f84 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> @@ -459,10 +459,6 @@ empty_request(struct intel_engine_cs *engine,
> if (IS_ERR(request))
> return request;
>
> - err = engine->emit_flush(request, EMIT_INVALIDATE);
> - if (err)
> - goto out_request;
> -
> err = i915_switch_context(request);
> if (err)
> goto out_request;
> @@ -675,9 +671,6 @@ static int live_all_engines(void *arg)
> goto out_request;
> }
>
> - err = engine->emit_flush(request[id], EMIT_INVALIDATE);
> - GEM_BUG_ON(err);
> -
> err = i915_switch_context(request[id]);
> GEM_BUG_ON(err);
>
> @@ -797,9 +790,6 @@ static int live_sequential_engines(void *arg)
> }
> }
>
> - err = engine->emit_flush(request[id], EMIT_INVALIDATE);
> - GEM_BUG_ON(err);
> -
> err = i915_switch_context(request[id]);
> GEM_BUG_ON(err);
>
> diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> index 71ce06680d66..145bdc26553c 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> @@ -114,10 +114,6 @@ static int emit_recurse_batch(struct hang *h,
> if (err)
> goto unpin_vma;
>
> - err = rq->engine->emit_flush(rq, EMIT_INVALIDATE);
> - if (err)
> - goto unpin_hws;
> -
> err = i915_switch_context(rq);
> if (err)
> goto unpin_hws;
> --
> 2.15.0
More information about the Intel-gfx
mailing list