diff mbox

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

Message ID 1464970133-29859-12-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson June 3, 2016, 4:08 p.m. UTC 
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@chris-wilson.co.uk>
---
 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(-)

Comments

Tvrtko Ursulin June 6, 2016, 3:09 p.m. UTC | #1 
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@chris-wilson.co.uk>
> ---
>   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@intel.com>

Regards,

Tvrtko
Chris Wilson June 8, 2016, 9:27 a.m. UTC | #2 
On Mon, Jun 06, 2016 at 04:09:12PM +0100, Tvrtko Ursulin wrote:
> 
> On 03/06/16 17:08, Chris Wilson wrote:
> >  	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.

GTT_PAGE_SIZE, or GTT_PAGE_4k for when we do large pages from yonks ago.

In this case it would be GEN5_SCRATCH_SIZE I guess. Is that worth it?
-Chris
diff mbox

Patch

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);
+		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);