Re: [PATCH 11/21] drm/i915: Refactor scratch object allocation for gen2 w/a buffer

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 03/06/16 17:08, Chris Wilson wrote:
> The gen2 w/a buffer is stuffed into the same slot as the gen5+ scratch
> buffer. If we pass in the size we want to allocate for the scratch
> buffer, both callers can use the same routine.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c        |  2 +-
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 31 ++++++++-----------------------
>   drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
>   3 files changed, 10 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index e48687837a95..32b5eae7dd11 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2078,7 +2078,7 @@ static int logical_render_ring_init(struct drm_device *dev)
>   	engine->emit_flush = gen8_emit_flush_render;
>   	engine->emit_request = gen8_emit_request_render;
>
> -	ret = intel_init_pipe_control(engine);
> +	ret = intel_init_pipe_control(engine, 4096);
>   	if (ret)
>   		return ret;
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b7eebbed945d..ca2e59405998 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -653,16 +653,16 @@ void intel_fini_pipe_control(struct intel_engine_cs *engine)
>   	engine->scratch.obj = NULL;
>   }
>
> -int intel_init_pipe_control(struct intel_engine_cs *engine)
> +int intel_init_pipe_control(struct intel_engine_cs *engine, int size)
>   {
>   	struct drm_i915_gem_object *obj;
>   	int ret;
>
>   	WARN_ON(engine->scratch.obj);
>
> -	obj = i915_gem_object_create_stolen(engine->i915->dev, 4096);
> +	obj = i915_gem_object_create_stolen(engine->i915->dev, size);
>   	if (obj == NULL)
> -		obj = i915_gem_object_create(engine->i915->dev, 4096);
> +		obj = i915_gem_object_create(engine->i915->dev, size);
>   	if (IS_ERR(obj)) {
>   		DRM_ERROR("Failed to allocate scratch page\n");
>   		ret = PTR_ERR(obj);
> @@ -2863,31 +2863,16 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   	engine->init_hw = init_render_ring;
>   	engine->cleanup = render_ring_cleanup;
>
> -	/* Workaround batchbuffer to combat CS tlb bug. */
> -	if (HAS_BROKEN_CS_TLB(dev_priv)) {
> -		obj = i915_gem_object_create(dev, I830_WA_SIZE);
> -		if (IS_ERR(obj)) {
> -			DRM_ERROR("Failed to allocate batch bo\n");
> -			return PTR_ERR(obj);
> -		}
> -
> -		ret = i915_gem_obj_ggtt_pin(obj, 0, 0);
> -		if (ret != 0) {
> -			drm_gem_object_unreference(&obj->base);
> -			DRM_ERROR("Failed to ping batch bo\n");
> -			return ret;
> -		}
> -
> -		engine->scratch.obj = obj;
> -		engine->scratch.gtt_offset = i915_gem_obj_ggtt_offset(obj);
> -	}
> -
>   	ret = intel_init_ring_buffer(dev, engine);
>   	if (ret)
>   		return ret;
>
>   	if (INTEL_GEN(dev_priv) >= 5) {
> -		ret = intel_init_pipe_control(engine);
> +		ret = intel_init_pipe_control(engine, 4096);

Could be cool to define this size with a descriptive name at this point.

> +		if (ret)
> +			return ret;
> +	} else if (HAS_BROKEN_CS_TLB(dev_priv)) {
> +		ret = intel_init_pipe_control(engine, I830_WA_SIZE);
>   		if (ret)
>   			return ret;
>   	}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 4b2f19decb30..cb599a54931a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -483,8 +483,8 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno);
>   int intel_ring_flush_all_caches(struct drm_i915_gem_request *req);
>   int intel_ring_invalidate_all_caches(struct drm_i915_gem_request *req);
>
> +int intel_init_pipe_control(struct intel_engine_cs *engine, int size);
>   void intel_fini_pipe_control(struct intel_engine_cs *engine);
> -int intel_init_pipe_control(struct intel_engine_cs *engine);
>
>   int intel_init_render_ring_buffer(struct drm_device *dev);
>   int intel_init_bsd_ring_buffer(struct drm_device *dev);

Anyway,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx