[Intel-gfx] [PATCH 2/2] drm/i915: Consolidate gen8_emit_pipe_control

[Chris Wilson]

On Wed, Feb 15, 2017 at 04:06:34PM +0000, Tvrtko Ursulin wrote:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index f7499829ecc2..a359527948b9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -334,35 +334,16 @@ gen7_render_ring_flush(struct drm_i915_gem_request *req, u32 mode)
>  }
>  
>  static int
> -gen8_emit_pipe_control(struct drm_i915_gem_request *req,
> -		       u32 flags, u32 scratch_addr)
> +gen8_render_ring_flush(struct drm_i915_gem_request *req, u32 mode)
>  {
> +	u32 flags;
>  	u32 *cs;
>  
> -	cs = intel_ring_begin(req, 6);
> +	cs = intel_ring_begin(req, mode & EMIT_INVALIDATE ? 12 : 6);
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  

> -static int
> -gen8_render_ring_flush(struct drm_i915_gem_request *req, u32 mode)
> -{
> -	u32 scratch_addr =
> -		i915_ggtt_offset(req->engine->scratch) + 2 * CACHELINE_BYTES;
> -	u32 flags = 0;
> -	int ret;
> -
> -	flags |= PIPE_CONTROL_CS_STALL;
> +	flags = PIPE_CONTROL_CS_STALL;
>  
>  	if (mode & EMIT_FLUSH) {
>  		flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> @@ -381,15 +362,19 @@ gen8_render_ring_flush(struct drm_i915_gem_request *req, u32 mode)
>  		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
>  
>  		/* WaCsStallBeforeStateCacheInvalidate:bdw,chv */
> -		ret = gen8_emit_pipe_control(req,
> -					     PIPE_CONTROL_CS_STALL |
> -					     PIPE_CONTROL_STALL_AT_SCOREBOARD,
> -					     0);
> -		if (ret)
> -			return ret;
> +		cs = gen8_emit_pipe_control(cs,
> +					    PIPE_CONTROL_CS_STALL |
> +					    PIPE_CONTROL_STALL_AT_SCOREBOARD,
> +					    0);
>  	}
>  
> -	return gen8_emit_pipe_control(req, flags, scratch_addr);
> +	cs = gen8_emit_pipe_control(cs, flags,
> +				    i915_ggtt_offset(req->engine->scratch) +
> +				    2 * CACHELINE_BYTES);
> +
> +	intel_ring_advance(req, cs);

That looks scary in the diff, but correct.

> +static inline u32 *gen8_emit_pipe_control(u32 *batch, u32 flags, u32 offset)
> +{
> +	static const u32 pc6[6] = { GFX_OP_PIPE_CONTROL(6), 0, 0, 0, 0, 0 };
> +
> +	memcpy(batch, pc6, sizeof(pc6));
> +
> +	batch[1] = flags;
> +	batch[2] = offset;
> +
> +	return batch + 6;
> +}

Other than quibbling over code generation here,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Coverage of pipecontrol side-effects is lax in igt, piglit is the best
bet to see if it has any unintentional impact.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre