Michal,

On 11/06/2019 13:23, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>

These functions operate on ggtt so make them take that directly as
parameter.

At the same time move the USES_GUC conditional down to
intel_guc_reserve_ggtt_top for symmetry with
intel_guc_reserved_gtt_size.

v2:
  * Rename and move functions to be static in i915_gem_gtt.c (Michal)

v3:
  * Add comment explaining reason for reservation, add assert and fix
    error message. (Michal)

v4:
  * Fix checkpatch error.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdec...@intel.com>

I've assumed I would be able to implement your small suggestions correctly so I've kept the r-b. Could you just give it a quick glance over to see if that is indeed true.

Regards,

Tvrtko

---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 43 ++++++++++++++++++++++++-----
  drivers/gpu/drm/i915/intel_guc.c    | 27 ------------------
  drivers/gpu/drm/i915/intel_guc.h    |  2 --
  3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5b5125ee49f3..c13b52b66ef1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2807,6 +2807,32 @@ static void fini_aliasing_ppgtt(struct drm_i915_private 
*i915)
        ggtt->vm.vma_ops.unbind_vma = ggtt_unbind_vma;
  }
+static int ggtt_reserve_guc_top(struct i915_ggtt *ggtt)
+{
+       u64 size;
+       int ret;
+
+       if (!USES_GUC(ggtt->vm.i915))
+               return 0;
+
+       GEM_BUG_ON(ggtt->vm.total <= GUC_GGTT_TOP);
+       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("Failed to reserve top of GGTT for GuC\n");
+
+       return ret;
+}
+
+static void ggtt_release_guc_top(struct i915_ggtt *ggtt)
+{
+       if (drm_mm_node_allocated(&ggtt->uc_fw))
+               drm_mm_remove_node(&ggtt->uc_fw);
+}
+
  int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
  {
        /* Let GEM Manage all of the aperture.
@@ -2844,11 +2870,14 @@ 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;
-       }
+       /*
+        * 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.
+        */
+       ret = ggtt_reserve_guc_top(ggtt);
+       if (ret)
+               goto err_reserve;
/* Clear any non-preallocated blocks */
        drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
@@ -2870,7 +2899,7 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
        return 0;
err_appgtt:
-       intel_guc_release_ggtt_top(&dev_priv->guc);
+       ggtt_release_guc_top(ggtt);
  err_reserve:
        drm_mm_remove_node(&ggtt->error_capture);
        return ret;
@@ -2897,7 +2926,7 @@ 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);
+       ggtt_release_guc_top(ggtt);
if (drm_mm_initialized(&ggtt->vm.mm)) {
                intel_vgt_deballoon(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index d45d97624402..c40a6efdd33a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -685,30 +685,3 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc 
*guc, u32 size)
        i915_gem_object_put(obj);
        return vma;
  }
-
-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 85c3b02a0c08..08c906abdfa2 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -172,8 +172,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 
rsa_offset);
  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);
-int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
-void intel_guc_release_ggtt_top(struct intel_guc *guc);
static inline bool intel_guc_is_loaded(struct intel_guc *guc)
  {

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

Reply via email to