| Message ID | 20190419230015.18121-3-fernando.pacheco@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Perma-pin uC firmware and re-enable global reset | expand |
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
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) {
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(-)