diff mbox

[4/9] drm/i915: Remove early l3-remap

Message ID 1461048560-31983-5-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson April 19, 2016, 6:49 a.m. UTC 
Since we do the l3-remap on context switch, and proceed to do a context
switch immediately after manually doing the l3-remap, we can remove the
redundant manual call.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 -
 drivers/gpu/drm/i915/i915_gem.c         | 39 +--------------------------------
 drivers/gpu/drm/i915/i915_gem_context.c | 31 +++++++++++++++++++++++++-
 3 files changed, 31 insertions(+), 40 deletions(-)

Comments

Tvrtko Ursulin April 19, 2016, 9:41 a.m. UTC | #1 
On 19/04/16 07:49, Chris Wilson wrote:
> Since we do the l3-remap on context switch, and proceed to do a context
> switch immediately after manually doing the l3-remap, we can remove the
> redundant manual call.

Looks like it should say it is removing the l3-remap on driver load, not 
"manual".

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h         |  1 -
>   drivers/gpu/drm/i915/i915_gem.c         | 39 +--------------------------------
>   drivers/gpu/drm/i915/i915_gem_context.c | 31 +++++++++++++++++++++++++-
>   3 files changed, 31 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 368fd7090189..4c55f480f60b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2983,7 +2983,6 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
>   int __must_check i915_gem_init(struct drm_device *dev);
>   int i915_gem_init_engines(struct drm_device *dev);
>   int __must_check i915_gem_init_hw(struct drm_device *dev);
> -int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice);
>   void i915_gem_init_swizzling(struct drm_device *dev);
>   void i915_gem_cleanup_engines(struct drm_device *dev);
>   int __must_check i915_gpu_idle(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index c9d7dfc8f80c..fd9a36badb45 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4077,35 +4077,6 @@ err:
>   	return ret;
>   }
>
> -int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice)
> -{
> -	struct intel_engine_cs *engine = req->engine;
> -	u32 *remap_info = to_i915(req)->l3_parity.remap_info[slice];

Wrong base.

> -	int i, ret;
> -
> -	if (!HAS_L3_DPF(req) || !remap_info)
> -		return 0;
> -
> -	ret = intel_ring_begin(req, GEN7_L3LOG_SIZE / 4 * 3);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * Note: We do not worry about the concurrent register cacheline hang
> -	 * here because no other code should access these registers other than
> -	 * at initialization time.
> -	 */
> -	for (i = 0; i < GEN7_L3LOG_SIZE / 4; i++) {
> -		intel_ring_emit(engine, MI_LOAD_REGISTER_IMM(1));
> -		intel_ring_emit_reg(engine, GEN7_L3LOG(slice, i));
> -		intel_ring_emit(engine, remap_info[i]);
> -	}
> -
> -	intel_ring_advance(engine);
> -
> -	return ret;
> -}
> -
>   void i915_gem_init_swizzling(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -4210,7 +4181,7 @@ i915_gem_init_hw(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_engine_cs *engine;
> -	int ret, j;
> +	int ret;
>
>   	if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
>   		return -EIO;
> @@ -4284,14 +4255,6 @@ i915_gem_init_hw(struct drm_device *dev)
>   			break;
>   		}
>
> -		if (engine->id == RCS) {
> -			for (j = 0; j < NUM_L3_SLICES(dev); j++) {
> -				ret = i915_gem_l3_remap(req, j);
> -				if (ret)
> -					goto err_request;
> -			}
> -		}
> -
>   		ret = i915_ppgtt_init_ring(req);
>   		if (ret)
>   			goto err_request;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index db53d811370d..6870556a180b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -691,6 +691,35 @@ needs_pd_load_post(struct intel_context *to, u32 hw_flags)
>   	return false;
>   }
>
> +static int remap_l3(struct drm_i915_gem_request *req, int slice)
> +{
> +	u32 *remap_info = to_i915(req)->l3_parity.remap_info[slice];
> +	struct intel_engine_cs *engine = req->engine;
> +	int i, ret;
> +
> +	if (!remap_info)
> +		return 0;

HAS_L3_DPF check is gone. Ok it looks correct from the call site, but 
best would be not to change it when moving around.

> +
> +	ret = intel_ring_begin(req, GEN7_L3LOG_SIZE/4 * 2 + 2);

More changes - should you perhaps add a patch which simplifies l3 remap 
to use since lri ahead of this one?

> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Note: We do not worry about the concurrent register cacheline hang
> +	 * here because no other code should access these registers other than
> +	 * at initialization time.
> +	 */
> +	intel_ring_emit(engine, MI_LOAD_REGISTER_IMM(GEN7_L3LOG_SIZE/4));
> +	for (i = 0; i < GEN7_L3LOG_SIZE/4; i++) {
> +		intel_ring_emit_reg(engine, GEN7_L3LOG(slice, i));
> +		intel_ring_emit(engine, remap_info[i]);
> +	}
> +	intel_ring_emit(engine, MI_NOOP);
> +	intel_ring_advance(engine);
> +
> +	return 0;
> +}
> +
>   static int do_rcs_switch(struct drm_i915_gem_request *req)
>   {
>   	struct intel_context *to = req->ctx;
> @@ -810,7 +839,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
>   		if (!(to->remap_slice & (1<<i)))
>   			continue;
>
> -		ret = i915_gem_l3_remap(req, i);
> +		ret = remap_l3(req, i);
>   		if (ret)
>   			return ret;
>
>

Otherwise looks safe, although I am not sure why it belongs in this series.

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 368fd7090189..4c55f480f60b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2983,7 +2983,6 @@  bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
 int __must_check i915_gem_init(struct drm_device *dev);
 int i915_gem_init_engines(struct drm_device *dev);
 int __must_check i915_gem_init_hw(struct drm_device *dev);
-int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice);
 void i915_gem_init_swizzling(struct drm_device *dev);
 void i915_gem_cleanup_engines(struct drm_device *dev);
 int __must_check i915_gpu_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c9d7dfc8f80c..fd9a36badb45 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4077,35 +4077,6 @@  err:
 	return ret;
 }
 
-int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice)
-{
-	struct intel_engine_cs *engine = req->engine;
-	u32 *remap_info = to_i915(req)->l3_parity.remap_info[slice];
-	int i, ret;
-
-	if (!HAS_L3_DPF(req) || !remap_info)
-		return 0;
-
-	ret = intel_ring_begin(req, GEN7_L3LOG_SIZE / 4 * 3);
-	if (ret)
-		return ret;
-
-	/*
-	 * Note: We do not worry about the concurrent register cacheline hang
-	 * here because no other code should access these registers other than
-	 * at initialization time.
-	 */
-	for (i = 0; i < GEN7_L3LOG_SIZE / 4; i++) {
-		intel_ring_emit(engine, MI_LOAD_REGISTER_IMM(1));
-		intel_ring_emit_reg(engine, GEN7_L3LOG(slice, i));
-		intel_ring_emit(engine, remap_info[i]);
-	}
-
-	intel_ring_advance(engine);
-
-	return ret;
-}
-
 void i915_gem_init_swizzling(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -4210,7 +4181,7 @@  i915_gem_init_hw(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *engine;
-	int ret, j;
+	int ret;
 
 	if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
 		return -EIO;
@@ -4284,14 +4255,6 @@  i915_gem_init_hw(struct drm_device *dev)
 			break;
 		}
 
-		if (engine->id == RCS) {
-			for (j = 0; j < NUM_L3_SLICES(dev); j++) {
-				ret = i915_gem_l3_remap(req, j);
-				if (ret)
-					goto err_request;
-			}
-		}
-
 		ret = i915_ppgtt_init_ring(req);
 		if (ret)
 			goto err_request;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index db53d811370d..6870556a180b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -691,6 +691,35 @@  needs_pd_load_post(struct intel_context *to, u32 hw_flags)
 	return false;
 }
 
+static int remap_l3(struct drm_i915_gem_request *req, int slice)
+{
+	u32 *remap_info = to_i915(req)->l3_parity.remap_info[slice];
+	struct intel_engine_cs *engine = req->engine;
+	int i, ret;
+
+	if (!remap_info)
+		return 0;
+
+	ret = intel_ring_begin(req, GEN7_L3LOG_SIZE/4 * 2 + 2);
+	if (ret)
+		return ret;
+
+	/*
+	 * Note: We do not worry about the concurrent register cacheline hang
+	 * here because no other code should access these registers other than
+	 * at initialization time.
+	 */
+	intel_ring_emit(engine, MI_LOAD_REGISTER_IMM(GEN7_L3LOG_SIZE/4));
+	for (i = 0; i < GEN7_L3LOG_SIZE/4; i++) {
+		intel_ring_emit_reg(engine, GEN7_L3LOG(slice, i));
+		intel_ring_emit(engine, remap_info[i]);
+	}
+	intel_ring_emit(engine, MI_NOOP);
+	intel_ring_advance(engine);
+
+	return 0;
+}
+
 static int do_rcs_switch(struct drm_i915_gem_request *req)
 {
 	struct intel_context *to = req->ctx;
@@ -810,7 +839,7 @@  static int do_rcs_switch(struct drm_i915_gem_request *req)
 		if (!(to->remap_slice & (1<<i)))
 			continue;
 
-		ret = i915_gem_l3_remap(req, i);
+		ret = remap_l3(req, i);
 		if (ret)
 			return ret;