[Intel-gfx] [PATCH 17/31] drm/i915: Remove obsolete engine->gpu_caches_dirty

Dave Gordon david.s.gordon at intel.com
Wed Jul 27 09:49:46 UTC 2016


On 25/07/16 08:44, Chris Wilson wrote:
> Space for flushing the GPU cache prior to completing the request is
> preallocated and so cannot fail.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c    |  2 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  9 +---
>  drivers/gpu/drm/i915/i915_gem_gtt.c        | 11 +++--
>  drivers/gpu/drm/i915/i915_gem_request.c    |  7 ++-
>  drivers/gpu/drm/i915/intel_lrc.c           | 47 +++----------------
>  drivers/gpu/drm/i915/intel_lrc.h           |  2 -
>  drivers/gpu/drm/i915/intel_ringbuffer.c    | 72 +++++++-----------------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |  7 ---
>  8 files changed, 37 insertions(+), 120 deletions(-)

[snip]

> -static int logical_ring_invalidate_all_caches(struct drm_i915_gem_request *req)
> -{
> -	struct intel_engine_cs *engine = req->engine;
> -	uint32_t flush_domains;
> -	int ret;
> -
> -	flush_domains = 0;
> -	if (engine->gpu_caches_dirty)
> -		flush_domains = I915_GEM_GPU_DOMAINS;
> -
> -	ret = engine->emit_flush(req, I915_GEM_GPU_DOMAINS, flush_domains);
> -	if (ret)
> -		return ret;
> -
> -	engine->gpu_caches_dirty = false;
> -	return 0;
> -}
> -
>  static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
>  				 struct list_head *vmas)
>  {
> @@ -690,7 +672,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req,
>  	/* Unconditionally invalidate gpu caches and ensure that we do flush
>  	 * any residual writes from the previous batch.
>  	 */
> -	return logical_ring_invalidate_all_caches(req);
> +	return req->engine->emit_flush(req, I915_GEM_GPU_DOMAINS, 0);
>  }

I don't think the direct call to the vfunc is as clear as to what we're 
trying to achieve here. I'd like some flavour of flush_caches() and 
invalidate_caches() reinstated, even if they're just trivial wrappers 
round the ->emit_flush().

While we're here, could we simplify the parameters? AFAICT we need only 
three permutations: FLUSH (only), INVALIDATE (only) or FLUSH and 
INVALIDATE; and in each case each parameter is either GEM_GPU_DOMAINS or 0.

.Dave.


More information about the Intel-gfx mailing list