On 4/9/19 2:41 PM, Chris Wilson wrote:
> Quoting Fernando Pacheco (2019-04-09 22:31:00)
>> 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.
>>
>> Signed-off-by: Fernando Pacheco <fernando.pacheco@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++++++++++++---
>> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
>> drivers/gpu/drm/i915/intel_guc.h | 11 +++++++++++
>> 3 files changed, 34 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 736c845eb77f..30f294a07e6d 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -2747,6 +2747,18 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
>> if (ret)
>> return ret;
>>
>> + /* Reserve a mappable slot for our lockless uC firmware load */
>> + if (USES_GUC(dev_priv)) {
>> + ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->uc_fw,
>> + GUC_GGTT_FW_SIZE, 0,
>> + I915_COLOR_UNEVICTABLE,
>> + GUC_GGTT_FW_START,
>> + GUC_GGTT_FW_END,
>> + DRM_MM_INSERT_LOW);
> Use drm_mm_reserve_node() as you want an explicit address.
>
> We should be able to push this to guc init, with appropriate bailing if
> something already took the high range.
I tried pushing this to guc init, but this bailed immediately as there are
pinnings between the calls to ggtt init and guc init. Did I misinterpret what
you had in mind?
Fernando
>
>> + if (ret)
>> + goto err_node;
>> + }
>> +
>> /* 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",
>> @@ -2761,12 +2773,15 @@ 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:
>> + if (USES_GUC(dev_priv))
>> + drm_mm_remove_node(&ggtt->uc_fw);
>> +err_node:
>> drm_mm_remove_node(&ggtt->error_capture);
>> return ret;
>> }
>> @@ -2792,6 +2807,9 @@ 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);
>>
>> + if (drm_mm_node_allocated(&ggtt->uc_fw))
>> + drm_mm_remove_node(&ggtt->uc_fw);
>> +
>> if (drm_mm_initialized(&ggtt->vm.mm)) {
>> intel_vgt_deballoon(dev_priv);
>> i915_address_space_fini(&ggtt->vm);
>> @@ -3370,7 +3388,8 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
>> * restriction!
>> */
>> if (USES_GUC(dev_priv)) {
>> - ggtt->vm.total = min_t(u64, ggtt->vm.total, GUC_GGTT_TOP);
>> + ggtt->vm.total = min_t(u64, ggtt->vm.total,
>> + GUC_GGTT_RESERVED_END);
> Should not be required now as you should have completely reserved the range
> for the guc, by the guc.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
