[v3,2/5] drm/i915/uc: Reserve upper range of GGTT
diff mbox series

Message ID 20190419230015.18121-3-fernando.pacheco@intel.com
State New
Headers show
Series
  • Perma-pin uC firmware and re-enable global reset
Related show

Commit Message

Fernando Pacheco April 19, 2019, 11 p.m. UTC
GuC and HuC depend on struct_mutex for device
reinitialization. Moving away from this dependency
requires perma-pinning the firmware images in GGTT.
The upper portion of the GuC address space has
a sizeable hole (several MB) that is inaccessible
by GuC. Reserve this range within GGTT as it can
comfortably hold GuC/HuC firmware images.

v2: Reserve node rather than insert (Chris)
    Simpler determination of node start/size (Daniele)
    Move reserve/release out to intel_guc.* files

v3: Reserve starting at GUC_GGTT_TOP only and bail if this
    fails (Chris)

Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++-------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
 drivers/gpu/drm/i915/intel_guc.c    | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h    |  2 ++
 4 files changed, 43 insertions(+), 13 deletions(-)

Comments

Chris Wilson April 20, 2019, 6:47 a.m. UTC | #1
Quoting Fernando Pacheco (2019-04-20 00:00:12)
> GuC and HuC depend on struct_mutex for device
> reinitialization. Moving away from this dependency
> requires perma-pinning the firmware images in GGTT.
> The upper portion of the GuC address space has
> a sizeable hole (several MB) that is inaccessible
> by GuC. Reserve this range within GGTT as it can
> comfortably hold GuC/HuC firmware images.
> 
> v2: Reserve node rather than insert (Chris)
>     Simpler determination of node start/size (Daniele)
>     Move reserve/release out to intel_guc.* files
> 
> v3: Reserve starting at GUC_GGTT_TOP only and bail if this
>     fails (Chris)
> 
> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++-------------
>  drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
>  drivers/gpu/drm/i915/intel_guc.c    | 28 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc.h    |  2 ++
>  4 files changed, 43 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8f460cc4cc1f..0b4c22e68574 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2752,6 +2752,12 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
>         if (ret)
>                 return ret;
>  
> +       if (USES_GUC(dev_priv)) {
> +               ret = intel_guc_reserve_ggtt_top(&dev_priv->guc);
> +               if (ret)
> +                       goto err_reserve;
> +       }
> +
>         /* Clear any non-preallocated blocks */
>         drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
>                 DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
> @@ -2766,12 +2772,14 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
>         if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) {
>                 ret = i915_gem_init_aliasing_ppgtt(dev_priv);
>                 if (ret)
> -                       goto err;
> +                       goto err_appgtt;
>         }
>  
>         return 0;
>  
> -err:
> +err_appgtt:
> +       intel_guc_release_ggtt_top(&dev_priv->guc);
> +err_reserve:
>         drm_mm_remove_node(&ggtt->error_capture);
>         return ret;
>  }
> @@ -2797,6 +2805,8 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
>         if (drm_mm_node_allocated(&ggtt->error_capture))
>                 drm_mm_remove_node(&ggtt->error_capture);
>  
> +       intel_guc_release_ggtt_top(&dev_priv->guc);
> +
>         if (drm_mm_initialized(&ggtt->vm.mm)) {
>                 intel_vgt_deballoon(dev_priv);
>                 i915_address_space_fini(&ggtt->vm);
> @@ -3369,17 +3379,6 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
>         if (ret)
>                 return ret;
>  
> -       /* Trim the GGTT to fit the GuC mappable upper range (when enabled).
> -        * This is easier than doing range restriction on the fly, as we
> -        * currently don't have any bits spare to pass in this upper
> -        * restriction!
> -        */
> -       if (USES_GUC(dev_priv)) {
> -               ggtt->vm.total = min_t(u64, ggtt->vm.total, GUC_GGTT_TOP);
> -               ggtt->mappable_end =
> -                       min_t(u64, ggtt->mappable_end, ggtt->vm.total);
> -       }
> -
>         if ((ggtt->vm.total - 1) >> 32) {
>                 DRM_ERROR("We never expected a Global GTT with more than 32bits"
>                           " of address space! Found %lldM!\n",
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index f597f35b109b..b51e779732c3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -384,6 +384,7 @@ struct i915_ggtt {
>         u32 pin_bias;
>  
>         struct drm_mm_node error_capture;
> +       struct drm_mm_node uc_fw;

I thought this might have been a better fit in with intel_uc if there's
a common spot for it. Likewise, I should move error_capture over to
i915_gpu_error.

>  };
>  
>  struct i915_hw_ppgtt {
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index d81a02b0f525..299b6aa4fe28 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -721,3 +721,31 @@ u32 intel_guc_reserved_gtt_size(struct intel_guc *guc)
>  {
>         return guc_to_i915(guc)->wopcm.guc.size;
>  }
> +
> +int intel_guc_reserve_ggtt_top(struct intel_guc *guc)
> +{
> +       struct drm_i915_private *i915 = guc_to_i915(guc);
> +       struct i915_ggtt *ggtt = &i915->ggtt;
> +       u64 size;
> +       int ret;
> +
> +       size = ggtt->vm.total - GUC_GGTT_TOP;
> +
> +       ret = i915_gem_gtt_reserve(&ggtt->vm, &ggtt->uc_fw, size,
> +                                  GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
> +                                  PIN_NOEVICT);
> +

We don't tend to leave newlines before conditions.

> +       if (ret)
> +               DRM_DEBUG_DRIVER("GuC: failed to reserve top of ggtt\n");
> +
> +       return ret;
> +}
> +
> +void intel_guc_release_ggtt_top(struct intel_guc *guc)
> +{
> +       struct drm_i915_private *i915 = guc_to_i915(guc);
> +       struct i915_ggtt *ggtt = &i915->ggtt;
> +
> +       if (drm_mm_node_allocated(&ggtt->uc_fw))
> +               drm_mm_remove_node(&ggtt->uc_fw);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 2c59ff8d9f39..2494e84831a2 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -173,6 +173,8 @@ int intel_guc_suspend(struct intel_guc *guc);
>  int intel_guc_resume(struct intel_guc *guc);
>  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
>  u32 intel_guc_reserved_gtt_size(struct intel_guc *guc);
> +int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
> +void intel_guc_release_ggtt_top(struct intel_guc *guc);

Much neater now that this is doing one job, ta.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8f460cc4cc1f..0b4c22e68574 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2752,6 +2752,12 @@  int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
+	if (USES_GUC(dev_priv)) {
+		ret = intel_guc_reserve_ggtt_top(&dev_priv->guc);
+		if (ret)
+			goto err_reserve;
+	}
+
 	/* Clear any non-preallocated blocks */
 	drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
 		DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
@@ -2766,12 +2772,14 @@  int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
 	if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) {
 		ret = i915_gem_init_aliasing_ppgtt(dev_priv);
 		if (ret)
-			goto err;
+			goto err_appgtt;
 	}
 
 	return 0;
 
-err:
+err_appgtt:
+	intel_guc_release_ggtt_top(&dev_priv->guc);
+err_reserve:
 	drm_mm_remove_node(&ggtt->error_capture);
 	return ret;
 }
@@ -2797,6 +2805,8 @@  void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
 	if (drm_mm_node_allocated(&ggtt->error_capture))
 		drm_mm_remove_node(&ggtt->error_capture);
 
+	intel_guc_release_ggtt_top(&dev_priv->guc);
+
 	if (drm_mm_initialized(&ggtt->vm.mm)) {
 		intel_vgt_deballoon(dev_priv);
 		i915_address_space_fini(&ggtt->vm);
@@ -3369,17 +3379,6 @@  int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		return ret;
 
-	/* Trim the GGTT to fit the GuC mappable upper range (when enabled).
-	 * This is easier than doing range restriction on the fly, as we
-	 * currently don't have any bits spare to pass in this upper
-	 * restriction!
-	 */
-	if (USES_GUC(dev_priv)) {
-		ggtt->vm.total = min_t(u64, ggtt->vm.total, GUC_GGTT_TOP);
-		ggtt->mappable_end =
-			min_t(u64, ggtt->mappable_end, ggtt->vm.total);
-	}
-
 	if ((ggtt->vm.total - 1) >> 32) {
 		DRM_ERROR("We never expected a Global GTT with more than 32bits"
 			  " of address space! Found %lldM!\n",
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f597f35b109b..b51e779732c3 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -384,6 +384,7 @@  struct i915_ggtt {
 	u32 pin_bias;
 
 	struct drm_mm_node error_capture;
+	struct drm_mm_node uc_fw;
 };
 
 struct i915_hw_ppgtt {
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index d81a02b0f525..299b6aa4fe28 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -721,3 +721,31 @@  u32 intel_guc_reserved_gtt_size(struct intel_guc *guc)
 {
 	return guc_to_i915(guc)->wopcm.guc.size;
 }
+
+int intel_guc_reserve_ggtt_top(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+	struct i915_ggtt *ggtt = &i915->ggtt;
+	u64 size;
+	int ret;
+
+	size = ggtt->vm.total - GUC_GGTT_TOP;
+
+	ret = i915_gem_gtt_reserve(&ggtt->vm, &ggtt->uc_fw, size,
+				   GUC_GGTT_TOP, I915_COLOR_UNEVICTABLE,
+				   PIN_NOEVICT);
+
+	if (ret)
+		DRM_DEBUG_DRIVER("GuC: failed to reserve top of ggtt\n");
+
+	return ret;
+}
+
+void intel_guc_release_ggtt_top(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+	struct i915_ggtt *ggtt = &i915->ggtt;
+
+	if (drm_mm_node_allocated(&ggtt->uc_fw))
+		drm_mm_remove_node(&ggtt->uc_fw);
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 2c59ff8d9f39..2494e84831a2 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -173,6 +173,8 @@  int intel_guc_suspend(struct intel_guc *guc);
 int intel_guc_resume(struct intel_guc *guc);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 u32 intel_guc_reserved_gtt_size(struct intel_guc *guc);
+int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
+void intel_guc_release_ggtt_top(struct intel_guc *guc);
 
 static inline int intel_guc_sanitize(struct intel_guc *guc)
 {