Quoting Fernando Pacheco (2019-04-09 22:31:01)
> Currently we pin the GuC or HuC firmware image just
> before uploading. Perma-pin during uC initialization
> instead and use the range reserved at the top of the
> address space.
>
> Moving the firmware resulted in needing to:
> - restore the ggtt mapping during the suspend/resume path.
> - use an additional pinning for the rsa signature which will
> be used during HuC auth as addresses above GUC_GGTT_TOP
> do not map through GTT.
>
> Signed-off-by: Fernando Pacheco <fernando.pacheco@xxxxxxxxx>
> ---
> @@ -315,3 +287,67 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p)
> drm_printf(p, "\tRSA: offset %u, size %u\n",
> uc_fw->rsa_offset, uc_fw->rsa_size);
> }
> +
> +void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw,
> + struct i915_ggtt *ggtt, u64 start)
> +{
> + struct drm_i915_gem_object *obj = uc_fw->obj;
> + struct i915_vma dummy = {
> + .node = { .start = start, .size = obj->base.size },
> + .size = obj->base.size,
> + .pages = obj->mm.pages,
> + .obj = obj,
Shouldn't need .size or .obj, but usually needs .vm.
> + };
> +
> + GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
> + ggtt->vm.insert_entries(&ggtt->vm, &dummy, obj->cache_level, 0);
> +}
> +
> +int intel_uc_fw_ggtt_pin(struct intel_uc_fw *uc_fw,
> + struct i915_ggtt *ggtt, u64 start)
> +{
> + struct drm_i915_gem_object *obj = uc_fw->obj;
> + int err;
> +
> + err = i915_gem_object_set_to_gtt_domain(obj, false);
Currently requires struct_mutex, and is not required as we can ensure
the pages are flushed on creation.
> + if (err) {
> + DRM_DEBUG_DRIVER("%s fw set-domain err=%d\n",
> + intel_uc_fw_type_repr(uc_fw->type), err);
> + return err;
> + }
> +
> + err = i915_gem_object_pin_pages(obj);
I'm pretty sure we don't need to pin the pages here, as the caller
should be holding the pages already for the duration of the bind.
So I think this should just reduce to the ggtt bind.
> + if (err) {
> + DRM_DEBUG_DRIVER("%s fw pin-pages err=%d\n",
> + intel_uc_fw_type_repr(uc_fw->type), err);
> + return err;
> + }
> +
> + intel_uc_fw_ggtt_bind(uc_fw, ggtt, start);
> +
> + return 0;
> +}
> +
> +void intel_uc_fw_ggtt_unpin(struct intel_uc_fw *uc_fw,
> + struct i915_ggtt *ggtt, u64 start)
> +{
> + struct drm_i915_gem_object *obj = uc_fw->obj;
> + u64 length = obj->base.size;
> +
> + ggtt->vm.clear_range(&ggtt->vm, start, length);
> +
> + if (i915_gem_object_has_pinned_pages(obj))
> + i915_gem_object_unpin_pages(obj);
No. You either own the pages, or you do not. Don't guess.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
