diff mbox series

drm/i915/guc: enable GuC GGTT invalidation from the start

Message ID 20221110175823.3867135-1-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/guc: enable GuC GGTT invalidation from the start | expand

Commit Message

Daniele Ceraolo Spurio Nov. 10, 2022, 5:58 p.m. UTC
Invalidating the GuC TLBs while GuC is not loaded does not have negative
consequences, so if we're starting the driver with GuC enabled we can
use the GGTT invalidation function from the get-go, iinstead of switching
to it when we initialize the GuC objects.

In MTL, this fixes and issue where we try to overwrite the invalidation
function twice (once for each GuC), due to the GGTT being shared between
the primary and media GTs

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_ggtt.c   | 28 ++++----------------------
 drivers/gpu/drm/i915/gt/intel_gtt.h    |  2 --
 drivers/gpu/drm/i915/gt/uc/intel_guc.c |  7 -------
 3 files changed, 4 insertions(+), 33 deletions(-)

Comments

John Harrison Dec. 2, 2022, 6:47 p.m. UTC | #1
On 11/10/2022 09:58, Daniele Ceraolo Spurio wrote:
> Invalidating the GuC TLBs while GuC is not loaded does not have negative
> consequences, so if we're starting the driver with GuC enabled we can
> use the GGTT invalidation function from the get-go, iinstead of switching
> to it when we initialize the GuC objects.
>
> In MTL, this fixes and issue where we try to overwrite the invalidation
> function twice (once for each GuC), due to the GGTT being shared between
> the primary and media GTs
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

> ---
>   drivers/gpu/drm/i915/gt/intel_ggtt.c   | 28 ++++----------------------
>   drivers/gpu/drm/i915/gt/intel_gtt.h    |  2 --
>   drivers/gpu/drm/i915/gt/uc/intel_guc.c |  7 -------
>   3 files changed, 4 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index 2518cebbf931..2dbe6ad5c900 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -979,7 +979,10 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>   			I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
>   	}
>   
> -	ggtt->invalidate = gen8_ggtt_invalidate;
> +	if (intel_uc_wants_guc(&ggtt->vm.gt->uc))
> +		ggtt->invalidate = guc_ggtt_invalidate;
> +	else
> +		ggtt->invalidate = gen8_ggtt_invalidate;
>   
>   	ggtt->vm.vma_ops.bind_vma    = intel_ggtt_bind_vma;
>   	ggtt->vm.vma_ops.unbind_vma  = intel_ggtt_unbind_vma;
> @@ -1216,29 +1219,6 @@ int i915_ggtt_enable_hw(struct drm_i915_private *i915)
>   	return 0;
>   }
>   
> -void i915_ggtt_enable_guc(struct i915_ggtt *ggtt)
> -{
> -	GEM_BUG_ON(ggtt->invalidate != gen8_ggtt_invalidate);
> -
> -	ggtt->invalidate = guc_ggtt_invalidate;
> -
> -	ggtt->invalidate(ggtt);
> -}
> -
> -void i915_ggtt_disable_guc(struct i915_ggtt *ggtt)
> -{
> -	/* XXX Temporary pardon for error unload */
> -	if (ggtt->invalidate == gen8_ggtt_invalidate)
> -		return;
> -
> -	/* We should only be called after i915_ggtt_enable_guc() */
> -	GEM_BUG_ON(ggtt->invalidate != guc_ggtt_invalidate);
> -
> -	ggtt->invalidate = gen8_ggtt_invalidate;
> -
> -	ggtt->invalidate(ggtt);
> -}
> -
>   /**
>    * i915_ggtt_resume_vm - Restore the memory mappings for a GGTT or DPT VM
>    * @vm: The VM to restore the mappings for
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 4d75ba4bb41d..fcbbfed79f15 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -579,8 +579,6 @@ void intel_ggtt_unbind_vma(struct i915_address_space *vm,
>   int i915_ggtt_probe_hw(struct drm_i915_private *i915);
>   int i915_ggtt_init_hw(struct drm_i915_private *i915);
>   int i915_ggtt_enable_hw(struct drm_i915_private *i915);
> -void i915_ggtt_enable_guc(struct i915_ggtt *ggtt);
> -void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
>   int i915_init_ggtt(struct drm_i915_private *i915);
>   void i915_ggtt_driver_release(struct drm_i915_private *i915);
>   void i915_ggtt_driver_late_release(struct drm_i915_private *i915);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 1bcd61bb50f8..4ec954b6b4e8 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -421,9 +421,6 @@ int intel_guc_init(struct intel_guc *guc)
>   	/* now that everything is perma-pinned, initialize the parameters */
>   	guc_init_params(guc);
>   
> -	/* We need to notify the guc whenever we change the GGTT */
> -	i915_ggtt_enable_guc(gt->ggtt);
> -
>   	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_LOADABLE);
>   
>   	return 0;
> @@ -448,13 +445,9 @@ int intel_guc_init(struct intel_guc *guc)
>   
>   void intel_guc_fini(struct intel_guc *guc)
>   {
> -	struct intel_gt *gt = guc_to_gt(guc);
> -
>   	if (!intel_uc_fw_is_loadable(&guc->fw))
>   		return;
>   
> -	i915_ggtt_disable_guc(gt->ggtt);
> -
>   	if (intel_guc_slpc_is_used(guc))
>   		intel_guc_slpc_fini(&guc->slpc);
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 2518cebbf931..2dbe6ad5c900 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -979,7 +979,10 @@  static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 			I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
 	}
 
-	ggtt->invalidate = gen8_ggtt_invalidate;
+	if (intel_uc_wants_guc(&ggtt->vm.gt->uc))
+		ggtt->invalidate = guc_ggtt_invalidate;
+	else
+		ggtt->invalidate = gen8_ggtt_invalidate;
 
 	ggtt->vm.vma_ops.bind_vma    = intel_ggtt_bind_vma;
 	ggtt->vm.vma_ops.unbind_vma  = intel_ggtt_unbind_vma;
@@ -1216,29 +1219,6 @@  int i915_ggtt_enable_hw(struct drm_i915_private *i915)
 	return 0;
 }
 
-void i915_ggtt_enable_guc(struct i915_ggtt *ggtt)
-{
-	GEM_BUG_ON(ggtt->invalidate != gen8_ggtt_invalidate);
-
-	ggtt->invalidate = guc_ggtt_invalidate;
-
-	ggtt->invalidate(ggtt);
-}
-
-void i915_ggtt_disable_guc(struct i915_ggtt *ggtt)
-{
-	/* XXX Temporary pardon for error unload */
-	if (ggtt->invalidate == gen8_ggtt_invalidate)
-		return;
-
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(ggtt->invalidate != guc_ggtt_invalidate);
-
-	ggtt->invalidate = gen8_ggtt_invalidate;
-
-	ggtt->invalidate(ggtt);
-}
-
 /**
  * i915_ggtt_resume_vm - Restore the memory mappings for a GGTT or DPT VM
  * @vm: The VM to restore the mappings for
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 4d75ba4bb41d..fcbbfed79f15 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -579,8 +579,6 @@  void intel_ggtt_unbind_vma(struct i915_address_space *vm,
 int i915_ggtt_probe_hw(struct drm_i915_private *i915);
 int i915_ggtt_init_hw(struct drm_i915_private *i915);
 int i915_ggtt_enable_hw(struct drm_i915_private *i915);
-void i915_ggtt_enable_guc(struct i915_ggtt *ggtt);
-void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
 int i915_init_ggtt(struct drm_i915_private *i915);
 void i915_ggtt_driver_release(struct drm_i915_private *i915);
 void i915_ggtt_driver_late_release(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 1bcd61bb50f8..4ec954b6b4e8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -421,9 +421,6 @@  int intel_guc_init(struct intel_guc *guc)
 	/* now that everything is perma-pinned, initialize the parameters */
 	guc_init_params(guc);
 
-	/* We need to notify the guc whenever we change the GGTT */
-	i915_ggtt_enable_guc(gt->ggtt);
-
 	intel_uc_fw_change_status(&guc->fw, INTEL_UC_FIRMWARE_LOADABLE);
 
 	return 0;
@@ -448,13 +445,9 @@  int intel_guc_init(struct intel_guc *guc)
 
 void intel_guc_fini(struct intel_guc *guc)
 {
-	struct intel_gt *gt = guc_to_gt(guc);
-
 	if (!intel_uc_fw_is_loadable(&guc->fw))
 		return;
 
-	i915_ggtt_disable_guc(gt->ggtt);
-
 	if (intel_guc_slpc_is_used(guc))
 		intel_guc_slpc_fini(&guc->slpc);