[Intel-gfx] [PATCH v4 1/6] drm/i915/gen8: Add infrastructure to initialize WA batch buffers

[Chris Wilson]

On Tue, Jun 16, 2015 at 08:25:20PM +0100, Arun Siluvery wrote:
> +static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *ring, u32 size)
> +{
> +	int ret;
> +	struct drm_device *dev = ring->dev;

You only use it once, keeping it as a local seems counter-intuitive.

> +	WARN_ON(ring->id != RCS);
> +
> +	size = roundup(size, PAGE_SIZE);

Out of curiousity is gcc smart enough to turn this into an ALIGN()?

> +	ring->wa_ctx.obj = i915_gem_alloc_object(dev, size);
> +	if (!ring->wa_ctx.obj) {
> +		DRM_DEBUG_DRIVER("alloc LRC WA ctx backing obj failed.\n");
> +		return -ENOMEM;
> +	}
> +
> +	ret = i915_gem_obj_ggtt_pin(ring->wa_ctx.obj, GEN8_LR_CONTEXT_ALIGN, 0);

Strange choice of alignment since we pass around cacheline offsets.

> +	if (ret) {
> +		DRM_DEBUG_DRIVER("pin LRC WA ctx backing obj failed: %d\n",
> +				 ret);
> +		drm_gem_object_unreference(&ring->wa_ctx.obj->base);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void lrc_destroy_wa_ctx_obj(struct intel_engine_cs *ring)
> +{
> +	WARN_ON(ring->id != RCS);
> +
> +	i915_gem_object_ggtt_unpin(ring->wa_ctx.obj);
> +	drm_gem_object_unreference(&ring->wa_ctx.obj->base);
> +	ring->wa_ctx.obj = NULL;
> +}
> +
>  /**
>   * intel_logical_ring_cleanup() - deallocate the Engine Command Streamer
>   *
> @@ -1474,7 +1612,29 @@ static int logical_render_ring_init(struct drm_device *dev)
>  	if (ret)
>  		return ret;
>  
> -	return intel_init_pipe_control(ring);
> +	if (INTEL_INFO(ring->dev)->gen >= 8) {
> +		ret = lrc_setup_wa_ctx_obj(ring, PAGE_SIZE);
> +		if (ret) {
> +			DRM_DEBUG_DRIVER("Failed to setup context WA page: %d\n",
> +					 ret);
> +			return ret;
> +		}
> +
> +		ret = intel_init_workaround_bb(ring);
> +		if (ret) {
> +			lrc_destroy_wa_ctx_obj(ring);
> +			DRM_ERROR("WA batch buffers are not initialized: %d\n",
> +				  ret);
> +		}
> +	}
> +
> +	ret = intel_init_pipe_control(ring);

Did you consider stuffing it into the spare are of the pipe control
scatch bo? :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre