Re: [PATCH 22/37] drm/i915: Move common scratch allocation/destroy to intel_engine_cs.c

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On pe, 2016-08-12 at 07:54 +0100, Chris Wilson wrote:
> Since the scratch allocation and cleanup is shared by all engine
> submission backends, move it out of the legacy intel_ringbuffer.c and
> into the new home for common routines, intel_engine_cs.c
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>

Hmm, did not get my e-mail?

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c  | 50 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c        |  1 -
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 50 ---------------------------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  4 +--
>  4 files changed, 51 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 186c12d07f99..7104dec5e893 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -195,6 +195,54 @@ void intel_engine_setup_common(struct intel_engine_cs *engine)
>  	i915_gem_batch_pool_init(engine, &engine->batch_pool);
>  }
>  
> +int intel_engine_create_scratch(struct intel_engine_cs *engine, int size)
> +{
> +	struct drm_i915_gem_object *obj;
> +	struct i915_vma *vma;
> +	int ret;
> +
> +	WARN_ON(engine->scratch);
> +
> +	obj = i915_gem_object_create_stolen(&engine->i915->drm, size);
> +	if (!obj)
> +		obj = i915_gem_object_create(&engine->i915->drm, size);
> +	if (IS_ERR(obj)) {
> +		DRM_ERROR("Failed to allocate scratch page\n");
> +		return PTR_ERR(obj);
> +	}
> +
> +	vma = i915_vma_create(obj, &engine->i915->ggtt.base, NULL);
> +	if (IS_ERR(vma)) {
> +		ret = PTR_ERR(vma);
> +		goto err_unref;
> +	}
> +
> +	ret = i915_vma_pin(vma, 0, 4096, PIN_GLOBAL | PIN_HIGH);
> +	if (ret)
> +		goto err_unref;
> +
> +	engine->scratch = vma;
> +	DRM_DEBUG_DRIVER("%s pipe control offset: 0x%08llx\n",
> +			 engine->name, vma->node.start);
> +	return 0;
> +
> +err_unref:
> +	i915_gem_object_put(obj);
> +	return ret;
> +}
> +
> +static void intel_engine_cleanup_scratch(struct intel_engine_cs *engine)
> +{
> +	struct i915_vma *vma;
> +
> +	vma = fetch_and_zero(&engine->scratch);
> +	if (!vma)
> +		return;
> +
> +	i915_vma_unpin(vma);
> +	i915_vma_put(vma);
> +}
> +
>  /**
>   * intel_engines_init_common - initialize cengine state which might require hw access
>   * @engine: Engine to initialize.
> @@ -226,6 +274,8 @@ int intel_engine_init_common(struct intel_engine_cs *engine)
>   */
>  void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>  {
> +	intel_engine_cleanup_scratch(engine);
> +
>  	intel_engine_cleanup_cmd_parser(engine);
>  	intel_engine_fini_breadcrumbs(engine);
>  	i915_gem_batch_pool_fini(&engine->batch_pool);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index cdcdec83d640..88bc5d4ce243 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1844,7 +1844,6 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
>  	else
>  		engine->init_hw = gen8_init_render_ring;
>  	engine->init_context = gen8_init_rcs_context;
> -	engine->cleanup = intel_engine_cleanup_scratch;
>  	engine->emit_flush = gen8_emit_flush_render;
>  	engine->emit_request = gen8_emit_request_render;
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 939632229320..0ecdd452a7f7 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -613,54 +613,6 @@ out:
>  	return ret;
>  }
>  
> -void intel_engine_cleanup_scratch(struct intel_engine_cs *engine)
> -{
> -	struct i915_vma *vma;
> -
> -	vma = fetch_and_zero(&engine->scratch);
> -	if (!vma)
> -		return;
> -
> -	i915_vma_unpin(vma);
> -	i915_vma_put(vma);
> -}
> -
> -int intel_engine_create_scratch(struct intel_engine_cs *engine, int size)
> -{
> -	struct drm_i915_gem_object *obj;
> -	struct i915_vma *vma;
> -	int ret;
> -
> -	WARN_ON(engine->scratch);
> -
> -	obj = i915_gem_object_create_stolen(&engine->i915->drm, size);
> -	if (!obj)
> -		obj = i915_gem_object_create(&engine->i915->drm, size);
> -	if (IS_ERR(obj)) {
> -		DRM_ERROR("Failed to allocate scratch page\n");
> -		return PTR_ERR(obj);
> -	}
> -
> -	vma = i915_vma_create(obj, &engine->i915->ggtt.base, NULL);
> -	if (IS_ERR(vma)) {
> -		ret = PTR_ERR(vma);
> -		goto err_unref;
> -	}
> -
> -	ret = i915_vma_pin(vma, 0, 4096, PIN_GLOBAL | PIN_HIGH);
> -	if (ret)
> -		goto err_unref;
> -
> -	engine->scratch = vma;
> -	DRM_DEBUG_DRIVER("%s pipe control offset: 0x%08llx\n",
> -			 engine->name, vma->node.start);
> -	return 0;
> -
> -err_unref:
> -	i915_gem_object_put(obj);
> -	return ret;
> -}
> -
>  static int intel_ring_workarounds_emit(struct drm_i915_gem_request *req)
>  {
>  	struct intel_ring *ring = req->ring;
> @@ -1311,8 +1263,6 @@ static void render_ring_cleanup(struct intel_engine_cs *engine)
>  		i915_gem_object_put(dev_priv->semaphore_obj);
>  		dev_priv->semaphore_obj = NULL;
>  	}
> -
> -	intel_engine_cleanup_scratch(engine);
>  }
>  
>  static int gen8_rcs_signal(struct drm_i915_gem_request *req)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index c6024db75f93..dca84258be16 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -472,11 +472,9 @@ void intel_ring_update_space(struct intel_ring *ring);
>  
>  void intel_engine_init_seqno(struct intel_engine_cs *engine, u32 seqno);
>  
> -int intel_engine_create_scratch(struct intel_engine_cs *engine, int size);
> -void intel_engine_cleanup_scratch(struct intel_engine_cs *engine);
> -
>  void intel_engine_setup_common(struct intel_engine_cs *engine);
>  int intel_engine_init_common(struct intel_engine_cs *engine);
> +int intel_engine_create_scratch(struct intel_engine_cs *engine, int size);
>  void intel_engine_cleanup_common(struct intel_engine_cs *engine);
>  
>  static inline int intel_engine_idle(struct intel_engine_cs *engine,
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx