All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset
@ 2019-04-19 23:00 Fernando Pacheco
  2019-04-19 23:00 ` [PATCH v3 1/5] drm/i915/uc: Rename uC firmware init/fini functions Fernando Pacheco
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Fernando Pacheco @ 2019-04-19 23:00 UTC (permalink / raw)
  To: intel-gfx

The intent is to move the GuC and HuC firmware images to the
top of the address space. This portion is inaccessible during
normal GuC operations and should be relatively safe to house
both firmware images. By making the move we can re-enable the
full gpu reset with GuC enabled.

Placing the firmware images above GUC_GGTT_TOP was discussed
previously here:
	https://patchwork.freedesktop.org/patch/273616/

v3: Bindings are only needed for xfer

v2:
The decision to rename both the uc_fw init and fini functions
made it easier to pull the bind/unbind operations out of
intel_guc_fw.* and intel_huc_fw.*. The bind/unbind will now
take place within the newly repurposed intel_uc_fw_init/fini.
All other changes should be called out in their respective patches
and should be the direct result of a review comment.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Fernando Pacheco (5):
  drm/i915/uc: Rename uC firmware init/fini functions
  drm/i915/uc: Reserve upper range of GGTT
  drm/i915/uc: Place uC firmware in upper range of GGTT
  Revert "drm/i915/guc: Disable global reset"
  drm/i915/selftests: Check that gpu reset is usable from atomic context

 drivers/gpu/drm/i915/i915_gem_gtt.c           |  25 ++--
 drivers/gpu/drm/i915/i915_gem_gtt.h           |   1 +
 drivers/gpu/drm/i915/i915_reset.c             |   3 -
 drivers/gpu/drm/i915/intel_guc.c              |  41 ++++++-
 drivers/gpu/drm/i915/intel_guc.h              |   2 +
 drivers/gpu/drm/i915/intel_guc_fw.c           |  20 ++--
 drivers/gpu/drm/i915/intel_huc.c              |  74 +++++++++---
 drivers/gpu/drm/i915/intel_huc.h              |   6 +-
 drivers/gpu/drm/i915/intel_huc_fw.c           |  49 +++++---
 drivers/gpu/drm/i915/intel_uc.c               |  23 +++-
 drivers/gpu/drm/i915/intel_uc_fw.c            | 107 +++++++++++++-----
 drivers/gpu/drm/i915/intel_uc_fw.h            |  10 +-
 .../gpu/drm/i915/selftests/intel_hangcheck.c  |   6 +-
 13 files changed, 267 insertions(+), 100 deletions(-)

-- 
2.21.0

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/5] drm/i915/uc: Rename uC firmware init/fini functions
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
@ 2019-04-19 23:00 ` Fernando Pacheco
  2019-04-20  6:45   ` Chris Wilson
  2019-04-19 23:00 ` [PATCH v3 2/5] drm/i915/uc: Reserve upper range of GGTT Fernando Pacheco
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Fernando Pacheco @ 2019-04-19 23:00 UTC (permalink / raw)
  To: intel-gfx

The uC firmware init function is called during
GuC/HuC init early phases. Rename to include "_early"
and properly reflect which phase we are at.

The uC firmware fini function is cleaning up the
state set/created on firmware fetch. Replace
"_fini" with "_cleanup_fetch".

v2: also rename uC fw fini function

Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c    | 6 +++---
 drivers/gpu/drm/i915/intel_guc_fw.c | 2 +-
 drivers/gpu/drm/i915/intel_huc.h    | 2 +-
 drivers/gpu/drm/i915/intel_huc_fw.c | 2 +-
 drivers/gpu/drm/i915/intel_uc_fw.c  | 4 ++--
 drivers/gpu/drm/i915/intel_uc_fw.h  | 5 +++--
 6 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 3aabfa2d9198..d81a02b0f525 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -154,7 +154,7 @@ int intel_guc_init_misc(struct intel_guc *guc)
 
 void intel_guc_fini_misc(struct intel_guc *guc)
 {
-	intel_uc_fw_fini(&guc->fw);
+	intel_uc_fw_cleanup_fetch(&guc->fw);
 	guc_fini_wq(guc);
 }
 
@@ -221,7 +221,7 @@ int intel_guc_init(struct intel_guc *guc)
 err_shared:
 	guc_shared_data_destroy(guc);
 err_fetch:
-	intel_uc_fw_fini(&guc->fw);
+	intel_uc_fw_cleanup_fetch(&guc->fw);
 	return ret;
 }
 
@@ -237,7 +237,7 @@ void intel_guc_fini(struct intel_guc *guc)
 	intel_guc_ads_destroy(guc);
 	intel_guc_log_destroy(&guc->log);
 	guc_shared_data_destroy(guc);
-	intel_uc_fw_fini(&guc->fw);
+	intel_uc_fw_cleanup_fetch(&guc->fw);
 }
 
 static u32 guc_ctl_debug_flags(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 792a551450c7..4385d9ef02bb 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -90,7 +90,7 @@ void intel_guc_fw_init_early(struct intel_guc *guc)
 {
 	struct intel_uc_fw *guc_fw = &guc->fw;
 
-	intel_uc_fw_init(guc_fw, INTEL_UC_FW_TYPE_GUC);
+	intel_uc_fw_init_early(guc_fw, INTEL_UC_FW_TYPE_GUC);
 	guc_fw_select(guc_fw);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 7e41d870b509..ce129e301961 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -42,7 +42,7 @@ int intel_huc_check_status(struct intel_huc *huc);
 
 static inline void intel_huc_fini_misc(struct intel_huc *huc)
 {
-	intel_uc_fw_fini(&huc->fw);
+	intel_uc_fw_cleanup_fetch(&huc->fw);
 }
 
 static inline int intel_huc_sanitize(struct intel_huc *huc)
diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c b/drivers/gpu/drm/i915/intel_huc_fw.c
index 68d47c105939..80a176d91edc 100644
--- a/drivers/gpu/drm/i915/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/intel_huc_fw.c
@@ -89,7 +89,7 @@ void intel_huc_fw_init_early(struct intel_huc *huc)
 {
 	struct intel_uc_fw *huc_fw = &huc->fw;
 
-	intel_uc_fw_init(huc_fw, INTEL_UC_FW_TYPE_HUC);
+	intel_uc_fw_init_early(huc_fw, INTEL_UC_FW_TYPE_HUC);
 	huc_fw_select(huc_fw);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index becf05ebae4d..e3e74207a102 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -274,13 +274,13 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 }
 
 /**
- * intel_uc_fw_fini - cleanup uC firmware
+ * intel_uc_fw_cleanup_fetch - cleanup uC firmware
  *
  * @uc_fw: uC firmware
  *
  * Cleans up uC firmware by releasing the firmware GEM obj.
  */
-void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
+void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
 {
 	struct drm_i915_gem_object *obj;
 
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index 0e3bd580e267..e6fa8599757c 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -102,7 +102,8 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
 }
 
 static inline
-void intel_uc_fw_init(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type)
+void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
+			    enum intel_uc_fw_type type)
 {
 	uc_fw->path = NULL;
 	uc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
@@ -144,10 +145,10 @@ static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
 
 void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 		       struct intel_uc_fw *uc_fw);
+void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 		       int (*xfer)(struct intel_uc_fw *uc_fw,
 				   struct i915_vma *vma));
-void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
 void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);
 
 #endif
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/5] drm/i915/uc: Reserve upper range of GGTT
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
  2019-04-19 23:00 ` [PATCH v3 1/5] drm/i915/uc: Rename uC firmware init/fini functions Fernando Pacheco
@ 2019-04-19 23:00 ` Fernando Pacheco
  2019-04-20  6:47   ` Chris Wilson
  2019-04-19 23:00 ` [PATCH v3 3/5] drm/i915/uc: Place uC firmware in " Fernando Pacheco
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Fernando Pacheco @ 2019-04-19 23:00 UTC (permalink / raw)
  To: intel-gfx

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;
 };
 
 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)
 {
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/5] drm/i915/uc: Place uC firmware in upper range of GGTT
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
  2019-04-19 23:00 ` [PATCH v3 1/5] drm/i915/uc: Rename uC firmware init/fini functions Fernando Pacheco
  2019-04-19 23:00 ` [PATCH v3 2/5] drm/i915/uc: Reserve upper range of GGTT Fernando Pacheco
@ 2019-04-19 23:00 ` Fernando Pacheco
  2019-04-20  6:53   ` Chris Wilson
  2019-04-19 23:00 ` [PATCH v3 4/5] Revert "drm/i915/guc: Disable global reset" Fernando Pacheco
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Fernando Pacheco @ 2019-04-19 23:00 UTC (permalink / raw)
  To: intel-gfx

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:
- 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.

v2: Remove call to set to gtt domain
    Do not restore fw gtt mapping unconditionally
    Separate out pin/unpin functions and drop usage of pin/unpin
    Use uc_fw init/fini functions to bind/unbind fw object

v3: Bind is only needed during xfer (Chris)
    Remove attempts to bind outside of xfer (Chris)
    Mark fw bind/unbind static

Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c    |   9 ++-
 drivers/gpu/drm/i915/intel_guc_fw.c |  18 ++---
 drivers/gpu/drm/i915/intel_huc.c    |  74 +++++++++++++++-----
 drivers/gpu/drm/i915/intel_huc.h    |   4 ++
 drivers/gpu/drm/i915/intel_huc_fw.c |  47 +++++++++----
 drivers/gpu/drm/i915/intel_uc.c     |  23 +++++--
 drivers/gpu/drm/i915/intel_uc_fw.c  | 103 ++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_uc_fw.h  |   7 +-
 8 files changed, 212 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 299b6aa4fe28..3bbf45a3bf78 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -189,9 +189,13 @@ int intel_guc_init(struct intel_guc *guc)
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	int ret;
 
-	ret = guc_shared_data_create(guc);
+	ret = intel_uc_fw_init(&guc->fw);
 	if (ret)
 		goto err_fetch;
+
+	ret = guc_shared_data_create(guc);
+	if (ret)
+		goto err_fw;
 	GEM_BUG_ON(!guc->shared_data);
 
 	ret = intel_guc_log_create(&guc->log);
@@ -220,6 +224,8 @@ int intel_guc_init(struct intel_guc *guc)
 	intel_guc_log_destroy(&guc->log);
 err_shared:
 	guc_shared_data_destroy(guc);
+err_fw:
+	intel_uc_fw_fini(&guc->fw);
 err_fetch:
 	intel_uc_fw_cleanup_fetch(&guc->fw);
 	return ret;
@@ -237,6 +243,7 @@ void intel_guc_fini(struct intel_guc *guc)
 	intel_guc_ads_destroy(guc);
 	intel_guc_log_destroy(&guc->log);
 	guc_shared_data_destroy(guc);
+	intel_uc_fw_fini(&guc->fw);
 	intel_uc_fw_cleanup_fetch(&guc->fw);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 4385d9ef02bb..8b2dcc70b956 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -122,14 +122,16 @@ static void guc_prepare_xfer(struct intel_guc *guc)
 }
 
 /* Copy RSA signature from the fw image to HW for verification */
-static void guc_xfer_rsa(struct intel_guc *guc, struct i915_vma *vma)
+static void guc_xfer_rsa(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct intel_uc_fw *fw = &guc->fw;
+	struct sg_table *pages = fw->obj->mm.pages;
 	u32 rsa[UOS_RSA_SCRATCH_COUNT];
 	int i;
 
-	sg_pcopy_to_buffer(vma->pages->sgl, vma->pages->nents,
-			   rsa, sizeof(rsa), guc->fw.rsa_offset);
+	sg_pcopy_to_buffer(pages->sgl, pages->nents,
+			   rsa, sizeof(rsa), fw->rsa_offset);
 
 	for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
 		I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
@@ -201,7 +203,7 @@ static int guc_wait_ucode(struct intel_guc *guc)
  * transfer between GTT locations. This functionality is left out of the API
  * for now as there is no need for it.
  */
-static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
+static int guc_xfer_ucode(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct intel_uc_fw *guc_fw = &guc->fw;
@@ -214,7 +216,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
 	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
 
 	/* Set the source address for the new blob */
-	offset = intel_guc_ggtt_offset(guc, vma) + guc_fw->header_offset;
+	offset = intel_uc_fw_ggtt_offset(guc_fw) + guc_fw->header_offset;
 	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
 	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
 
@@ -233,7 +235,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
 /*
  * Load the GuC firmware blob into the MinuteIA.
  */
-static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
+static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
 {
 	struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -250,9 +252,9 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
 	 * by the DMA engine in one operation, whereas the RSA signature is
 	 * loaded via MMIO.
 	 */
-	guc_xfer_rsa(guc, vma);
+	guc_xfer_rsa(guc);
 
-	ret = guc_xfer_ucode(guc, vma);
+	ret = guc_xfer_ucode(guc);
 
 	intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL);
 
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 94c04f16a2ad..1ff1fb015e58 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -40,6 +40,61 @@ int intel_huc_init_misc(struct intel_huc *huc)
 	return 0;
 }
 
+static int intel_huc_rsa_data_create(struct intel_huc *huc)
+{
+	struct drm_i915_private *i915 = huc_to_i915(huc);
+	struct intel_guc *guc = &i915->guc;
+	struct i915_vma *vma;
+	void *vaddr;
+
+	/*
+	 * HuC firmware will sit above GUC_GGTT_TOP and will not map
+	 * through GTT. Unfortunately, this means GuC cannot perform
+	 * the HuC auth. as the rsa offset now falls within the GuC
+	 * inaccessible range. We resort to perma-pinning an additional
+	 * vma within the accessible range that only contains the rsa
+	 * signature. The GuC can use this extra pinning to perform
+	 * the authentication since its GGTT offset will be GuC
+	 * accessible.
+	 */
+	vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
+	if (IS_ERR(vma))
+		return PTR_ERR(vma);
+
+	vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
+	if (IS_ERR(vaddr)) {
+		i915_vma_unpin_and_release(&vma, 0);
+		return PTR_ERR(vaddr);
+	}
+
+	huc->rsa_data = vma;
+	huc->rsa_data_vaddr = vaddr;
+
+	return 0;
+}
+
+static void intel_huc_rsa_data_destroy(struct intel_huc *huc)
+{
+	i915_vma_unpin_and_release(&huc->rsa_data, I915_VMA_RELEASE_MAP);
+}
+
+int intel_huc_init(struct intel_huc *huc)
+{
+	int err;
+
+	err = intel_huc_rsa_data_create(huc);
+	if (err)
+		return err;
+
+	return intel_uc_fw_init(&huc->fw);
+}
+
+void intel_huc_fini(struct intel_huc *huc)
+{
+	intel_uc_fw_fini(&huc->fw);
+	intel_huc_rsa_data_destroy(huc);
+}
+
 /**
  * intel_huc_auth() - Authenticate HuC uCode
  * @huc: intel_huc structure
@@ -55,27 +110,17 @@ int intel_huc_auth(struct intel_huc *huc)
 {
 	struct drm_i915_private *i915 = huc_to_i915(huc);
 	struct intel_guc *guc = &i915->guc;
-	struct i915_vma *vma;
 	u32 status;
 	int ret;
 
 	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return -ENOEXEC;
 
-	vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
-				       PIN_OFFSET_BIAS | i915->ggtt.pin_bias);
-	if (IS_ERR(vma)) {
-		ret = PTR_ERR(vma);
-		DRM_ERROR("HuC: Failed to pin huc fw object %d\n", ret);
-		goto fail;
-	}
-
 	ret = intel_guc_auth_huc(guc,
-				 intel_guc_ggtt_offset(guc, vma) +
-				 huc->fw.rsa_offset);
+				 intel_guc_ggtt_offset(guc, huc->rsa_data));
 	if (ret) {
 		DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
-		goto fail_unpin;
+		goto fail;
 	}
 
 	/* Check authentication status, it should be done by now */
@@ -86,14 +131,11 @@ int intel_huc_auth(struct intel_huc *huc)
 					2, 50, &status);
 	if (ret) {
 		DRM_ERROR("HuC: Firmware not verified %#x\n", status);
-		goto fail_unpin;
+		goto fail;
 	}
 
-	i915_vma_unpin(vma);
 	return 0;
 
-fail_unpin:
-	i915_vma_unpin(vma);
 fail:
 	huc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
 
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index ce129e301961..a0c21ae02a99 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -33,10 +33,14 @@ struct intel_huc {
 	struct intel_uc_fw fw;
 
 	/* HuC-specific additions */
+	struct i915_vma *rsa_data;
+	void *rsa_data_vaddr;
 };
 
 void intel_huc_init_early(struct intel_huc *huc);
 int intel_huc_init_misc(struct intel_huc *huc);
+int intel_huc_init(struct intel_huc *huc);
+void intel_huc_fini(struct intel_huc *huc);
 int intel_huc_auth(struct intel_huc *huc);
 int intel_huc_check_status(struct intel_huc *huc);
 
diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c b/drivers/gpu/drm/i915/intel_huc_fw.c
index 80a176d91edc..44c559526072 100644
--- a/drivers/gpu/drm/i915/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/intel_huc_fw.c
@@ -93,18 +93,24 @@ void intel_huc_fw_init_early(struct intel_huc *huc)
 	huc_fw_select(huc_fw);
 }
 
-/**
- * huc_fw_xfer() - DMA's the firmware
- * @huc_fw: the firmware descriptor
- * @vma: the firmware image (bound into the GGTT)
- *
- * Transfer the firmware image to RAM for execution by the microcontroller.
- *
- * Return: 0 on success, non-zero on failure
- */
-static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
+static void huc_xfer_rsa(struct intel_huc *huc)
 {
-	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
+	struct intel_uc_fw *fw = &huc->fw;
+	struct sg_table *pages = fw->obj->mm.pages;
+
+	/*
+	 * HuC firmware image is outside GuC accessible range.
+	 * Copy the RSA signature out of the image into
+	 * the perma-pinned region set aside for it
+	 */
+	sg_pcopy_to_buffer(pages->sgl, pages->nents,
+			   huc->rsa_data_vaddr, fw->rsa_size,
+			   fw->rsa_offset);
+}
+
+static int huc_xfer_ucode(struct intel_huc *huc)
+{
+	struct intel_uc_fw *huc_fw = &huc->fw;
 	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	unsigned long offset = 0;
@@ -116,7 +122,7 @@ static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
 	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
 
 	/* Set the source address for the uCode */
-	offset = intel_guc_ggtt_offset(&dev_priv->guc, vma) +
+	offset = intel_uc_fw_ggtt_offset(huc_fw) +
 		 huc_fw->header_offset;
 	intel_uncore_write(uncore, DMA_ADDR_0_LOW,
 			   lower_32_bits(offset));
@@ -150,6 +156,23 @@ static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
 	return ret;
 }
 
+/**
+ * huc_fw_xfer() - DMA's the firmware
+ * @huc_fw: the firmware descriptor
+ *
+ * Transfer the firmware image to RAM for execution by the microcontroller.
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+static int huc_fw_xfer(struct intel_uc_fw *huc_fw)
+{
+	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
+
+	huc_xfer_rsa(huc);
+
+	return huc_xfer_ucode(huc);
+}
+
 /**
  * intel_huc_fw_upload() - load HuC uCode to device
  * @huc: intel_huc structure
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 25b80ffe71ad..488dffba04d2 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -280,6 +280,7 @@ void intel_uc_fini_misc(struct drm_i915_private *i915)
 int intel_uc_init(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
+	struct intel_huc *huc = &i915->huc;
 	int ret;
 
 	if (!USES_GUC(i915))
@@ -292,19 +293,30 @@ int intel_uc_init(struct drm_i915_private *i915)
 	if (ret)
 		return ret;
 
+	if (USES_HUC(i915)) {
+		ret = intel_huc_init(huc);
+		if (ret)
+			goto err_guc;
+	}
+
 	if (USES_GUC_SUBMISSION(i915)) {
 		/*
 		 * This is stuff we need to have available at fw load time
 		 * if we are planning to enable submission later
 		 */
 		ret = intel_guc_submission_init(guc);
-		if (ret) {
-			intel_guc_fini(guc);
-			return ret;
-		}
+		if (ret)
+			goto err_huc;
 	}
 
 	return 0;
+
+err_huc:
+	if (USES_HUC(i915))
+		intel_huc_fini(huc);
+err_guc:
+	intel_guc_fini(guc);
+	return ret;
 }
 
 void intel_uc_fini(struct drm_i915_private *i915)
@@ -319,6 +331,9 @@ void intel_uc_fini(struct drm_i915_private *i915)
 	if (USES_GUC_SUBMISSION(i915))
 		intel_guc_submission_fini(guc);
 
+	if (USES_HUC(i915))
+		intel_huc_fini(&i915->huc);
+
 	intel_guc_fini(guc);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index e3e74207a102..d1191682d285 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -191,6 +191,39 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	release_firmware(fw);		/* OK even if fw is NULL */
 }
 
+static void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw)
+{
+	struct drm_i915_gem_object *obj = uc_fw->obj;
+	struct i915_ggtt *ggtt;
+	struct i915_vma dummy;
+
+	GEM_BUG_ON(!obj);
+
+	ggtt = &to_i915(obj->base.dev)->ggtt;
+
+	dummy.node.start = intel_uc_fw_ggtt_offset(uc_fw);
+	dummy.node.size = obj->base.size;
+	dummy.pages = obj->mm.pages;
+	dummy.vm = &ggtt->vm;
+
+	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
+	ggtt->vm.insert_entries(&ggtt->vm, &dummy, obj->cache_level, 0);
+}
+
+static void intel_uc_fw_ggtt_unbind(struct intel_uc_fw *uc_fw)
+{
+	struct drm_i915_gem_object *obj = uc_fw->obj;
+	struct i915_ggtt *ggtt;
+	u64 start;
+
+	GEM_BUG_ON(!obj);
+
+	ggtt = &to_i915(obj->base.dev)->ggtt;
+	start = intel_uc_fw_ggtt_offset(uc_fw);
+
+	ggtt->vm.clear_range(&ggtt->vm, start, obj->base.size);
+}
+
 /**
  * intel_uc_fw_upload - load uC firmware using custom loader
  * @uc_fw: uC firmware
@@ -201,11 +234,8 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
  * Return: 0 on success, non-zero on failure.
  */
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
-		       int (*xfer)(struct intel_uc_fw *uc_fw,
-				   struct i915_vma *vma))
+		       int (*xfer)(struct intel_uc_fw *uc_fw))
 {
-	struct i915_vma *vma;
-	u32 ggtt_pin_bias;
 	int err;
 
 	DRM_DEBUG_DRIVER("%s fw load %s\n",
@@ -219,36 +249,15 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 			 intel_uc_fw_type_repr(uc_fw->type),
 			 intel_uc_fw_status_repr(uc_fw->load_status));
 
-	/* Pin object with firmware */
-	err = i915_gem_object_set_to_gtt_domain(uc_fw->obj, false);
-	if (err) {
-		DRM_DEBUG_DRIVER("%s fw set-domain err=%d\n",
-				 intel_uc_fw_type_repr(uc_fw->type), err);
-		goto fail;
-	}
-
-	ggtt_pin_bias = to_i915(uc_fw->obj->base.dev)->ggtt.pin_bias;
-	vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
-				       PIN_OFFSET_BIAS | ggtt_pin_bias);
-	if (IS_ERR(vma)) {
-		err = PTR_ERR(vma);
-		DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
-				 intel_uc_fw_type_repr(uc_fw->type), err);
-		goto fail;
-	}
+	intel_uc_fw_ggtt_bind(uc_fw);
 
 	/* Call custom loader */
-	err = xfer(uc_fw, vma);
-
-	/*
-	 * We keep the object pages for reuse during resume. But we can unpin it
-	 * now that DMA has completed, so it doesn't continue to take up space.
-	 */
-	i915_vma_unpin(vma);
-
+	err = xfer(uc_fw);
 	if (err)
 		goto fail;
 
+	intel_uc_fw_ggtt_unbind(uc_fw);
+
 	uc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
 	DRM_DEBUG_DRIVER("%s fw load %s\n",
 			 intel_uc_fw_type_repr(uc_fw->type),
@@ -273,6 +282,42 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 	return err;
 }
 
+int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
+{
+	int err;
+
+	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return -ENOEXEC;
+
+	err = i915_gem_object_pin_pages(uc_fw->obj);
+	if (err)
+		DRM_DEBUG_DRIVER("%s fw pin-pages err=%d\n",
+				 intel_uc_fw_type_repr(uc_fw->type), err);
+
+	return err;
+}
+
+void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
+{
+	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return;
+
+	i915_gem_object_unpin_pages(uc_fw->obj);
+}
+
+u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw)
+{
+	struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
+	struct i915_ggtt *ggtt = &i915->ggtt;
+	struct drm_mm_node *node = &ggtt->uc_fw;
+
+	GEM_BUG_ON(!node->allocated);
+	GEM_BUG_ON(upper_32_bits(node->start));
+	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
+
+	return lower_32_bits(node->start);
+}
+
 /**
  * intel_uc_fw_cleanup_fetch - cleanup uC firmware
  *
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index e6fa8599757c..ff98f8661d72 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -27,7 +27,6 @@
 
 struct drm_printer;
 struct drm_i915_private;
-struct i915_vma;
 
 /* Home of GuC, HuC and DMC firmwares */
 #define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
@@ -147,8 +146,10 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 		       struct intel_uc_fw *uc_fw);
 void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
-		       int (*xfer)(struct intel_uc_fw *uc_fw,
-				   struct i915_vma *vma));
+		       int (*xfer)(struct intel_uc_fw *uc_fw));
+int intel_uc_fw_init(struct intel_uc_fw *uc_fw);
+void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
+u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw);
 void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);
 
 #endif
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/5] Revert "drm/i915/guc: Disable global reset"
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
                   ` (2 preceding siblings ...)
  2019-04-19 23:00 ` [PATCH v3 3/5] drm/i915/uc: Place uC firmware in " Fernando Pacheco
@ 2019-04-19 23:00 ` Fernando Pacheco
  2019-04-20  6:53   ` Chris Wilson
  2019-04-19 23:00 ` [PATCH v3 5/5] drm/i915/selftests: Check that gpu reset is usable from atomic context Fernando Pacheco
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Fernando Pacheco @ 2019-04-19 23:00 UTC (permalink / raw)
  To: intel-gfx

This reverts commit fe62365f9f80a1c1d438c54fba21f5108a182de8.

Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
---
 drivers/gpu/drm/i915/i915_reset.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
index 677d59304e78..1092d16c289c 100644
--- a/drivers/gpu/drm/i915/i915_reset.c
+++ b/drivers/gpu/drm/i915/i915_reset.c
@@ -641,9 +641,6 @@ int intel_gpu_reset(struct drm_i915_private *i915,
 
 bool intel_has_gpu_reset(struct drm_i915_private *i915)
 {
-	if (USES_GUC(i915))
-		return false;
-
 	if (!i915_modparams.reset)
 		return NULL;
 
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 5/5] drm/i915/selftests: Check that gpu reset is usable from atomic context
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
                   ` (3 preceding siblings ...)
  2019-04-19 23:00 ` [PATCH v3 4/5] Revert "drm/i915/guc: Disable global reset" Fernando Pacheco
@ 2019-04-19 23:00 ` Fernando Pacheco
  2019-04-20  6:54   ` Chris Wilson
  2019-04-19 23:22 ` ✗ Fi.CI.SPARSE: warning for Perma-pin uC firmware and re-enable global reset (rev3) Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Fernando Pacheco @ 2019-04-19 23:00 UTC (permalink / raw)
  To: intel-gfx

GPU reset is now available with GuC enabled,
so re-enable our check that this reset is usable
from atomic context.

Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
---
 drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
index 050bd1e19e02..2fd33aad8683 100644
--- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
+++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
@@ -1814,9 +1814,6 @@ static int igt_atomic_reset(void *arg)
 
 	/* Check that the resets are usable from atomic context */
 
-	if (USES_GUC_SUBMISSION(i915))
-		return 0; /* guc is dead; long live the guc */
-
 	igt_global_reset_lock(i915);
 	mutex_lock(&i915->drm.struct_mutex);
 	wakeref = intel_runtime_pm_get(i915);
@@ -1846,6 +1843,9 @@ static int igt_atomic_reset(void *arg)
 		force_reset(i915);
 	}
 
+	if (USES_GUC_SUBMISSION(i915))
+		goto unlock;
+
 	if (intel_has_reset_engine(i915)) {
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* ✗ Fi.CI.SPARSE: warning for Perma-pin uC firmware and re-enable global reset (rev3)
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
                   ` (4 preceding siblings ...)
  2019-04-19 23:00 ` [PATCH v3 5/5] drm/i915/selftests: Check that gpu reset is usable from atomic context Fernando Pacheco
@ 2019-04-19 23:22 ` Patchwork
  2019-04-20  0:16 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-04-20  2:28 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2019-04-19 23:22 UTC (permalink / raw)
  To: Fernando Pacheco; +Cc: intel-gfx

== Series Details ==

Series: Perma-pin uC firmware and re-enable global reset (rev3)
URL   : https://patchwork.freedesktop.org/series/59255/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/uc: Rename uC firmware init/fini functions
Okay!

Commit: drm/i915/uc: Reserve upper range of GGTT
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:3380:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:3382:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:3382:25: warning: expression using sizeof(void)

Commit: drm/i915/uc: Place uC firmware in upper range of GGTT
Okay!

Commit: Revert "drm/i915/guc: Disable global reset"
Okay!

Commit: drm/i915/selftests: Check that gpu reset is usable from atomic context
Okay!

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* ✓ Fi.CI.BAT: success for Perma-pin uC firmware and re-enable global reset (rev3)
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
                   ` (5 preceding siblings ...)
  2019-04-19 23:22 ` ✗ Fi.CI.SPARSE: warning for Perma-pin uC firmware and re-enable global reset (rev3) Patchwork
@ 2019-04-20  0:16 ` Patchwork
  2019-04-20  2:28 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2019-04-20  0:16 UTC (permalink / raw)
  To: Fernando Pacheco; +Cc: intel-gfx

== Series Details ==

Series: Perma-pin uC firmware and re-enable global reset (rev3)
URL   : https://patchwork.freedesktop.org/series/59255/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5961 -> Patchwork_12849
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59255/revisions/3/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12849 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-multi-fence:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +26

  * igt@amdgpu/amd_basic@userptr:
    - fi-whl-u:           NOTRUN -> SKIP [fdo#109271] +41

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@basic-bsd2:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_basic@basic-vebox:
    - fi-ivb-3770:        NOTRUN -> SKIP [fdo#109271] +48

  * igt@gem_exec_basic@readonly-bsd:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] +71

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_store@basic-bsd1:
    - fi-kbl-r:           NOTRUN -> SKIP [fdo#109271] +41

  * igt@gem_exec_store@basic-bsd2:
    - fi-hsw-4770:        NOTRUN -> SKIP [fdo#109271] +41

  * igt@gem_mmap_gtt@basic-write-cpu-read-gtt:
    - fi-apl-guc:         NOTRUN -> SKIP [fdo#109271] +46

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      PASS -> DMESG-FAIL [fdo#110235 ]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         NOTRUN -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-y:           PASS -> INCOMPLETE [fdo#107713] / [fdo#108569]

  * igt@kms_busy@basic-flip-c:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-bxt-dsi:         NOTRUN -> SKIP [fdo#109271] +47

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] +47
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_force_connector_basic@force-edid:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-whl-u:           NOTRUN -> FAIL [fdo#103375] +1

  * igt@kms_psr@cursor_plane_move:
    - fi-whl-u:           NOTRUN -> FAIL [fdo#107383] +3

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@i915_hangman@error-state-basic:
    - fi-kbl-guc:         SKIP [fdo#109271] -> PASS
    - fi-cfl-guc:         SKIP [fdo#109271] -> PASS +1

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-skl-guc:         SKIP [fdo#109271] -> PASS +1

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (34 -> 34)
------------------------------

  Additional (9): fi-bxt-dsi fi-byt-j1900 fi-apl-guc fi-hsw-4770 fi-whl-u fi-ivb-3770 fi-icl-u3 fi-pnv-d510 fi-kbl-r 
  Missing    (9): fi-ilk-m540 fi-bsw-n3050 fi-byt-squawks fi-ctg-p8600 fi-bxt-j4205 fi-gdg-551 fi-bsw-kefka fi-byt-n2820 fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_5961 -> Patchwork_12849

  CI_DRM_5961: c5fdcd8bd5c596bfc70b6be93e6f04d910c51308 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12849: 18f65425d76cf341a88c22d8cbd6d9473b97b8d8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

18f65425d76c drm/i915/selftests: Check that gpu reset is usable from atomic context
a891afc28d57 Revert "drm/i915/guc: Disable global reset"
4957b7613cc9 drm/i915/uc: Place uC firmware in upper range of GGTT
78bec8eafadd drm/i915/uc: Reserve upper range of GGTT
81daa5b62c99 drm/i915/uc: Rename uC firmware init/fini functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12849/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* ✓ Fi.CI.IGT: success for Perma-pin uC firmware and re-enable global reset (rev3)
  2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
                   ` (6 preceding siblings ...)
  2019-04-20  0:16 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-04-20  2:28 ` Patchwork
  7 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2019-04-20  2:28 UTC (permalink / raw)
  To: Fernando Pacheco; +Cc: intel-gfx

== Series Details ==

Series: Perma-pin uC firmware and re-enable global reset (rev3)
URL   : https://patchwork.freedesktop.org/series/59255/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5961_full -> Patchwork_12849_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12849_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@vecs0-s3:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#108566]

  * igt@gem_eio@in-flight-suspend:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +2

  * igt@i915_pm_rpm@gem-idle:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +2

  * igt@kms_busy@basic-flip-d:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_busy@basic-modeset-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-modeset-hang-oldfb-render-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +16

  * igt@kms_cursor_crc@cursor-64x64-onscreen:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_edge_walk@pipe-c-64x64-right-edge:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +5

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-xtiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_flip@dpms-vs-vblank-race-interruptible:
    - shard-apl:          PASS -> FAIL [fdo#103060]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#109507]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +6

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-wc:
    - shard-skl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +44

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-pwrite:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +23

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-skl:          NOTRUN -> FAIL [fdo#110279]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +5

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#108145] / [fdo#110403] +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         PASS -> SKIP [fdo#109441]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         PASS -> FAIL [fdo#100047]

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108]

  * igt@perf@blocking:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#107713]

  * igt@perf_pmu@busy-accuracy-98-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +126

  * igt@perf_pmu@busy-check-all-vecs0:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +29

  
#### Possible fixes ####

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +2

  * igt@i915_pm_rpm@gem-evict-pwrite:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> PASS

  * igt@kms_flip@dpms-vs-vblank-race-interruptible:
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr@no_drrs:
    - shard-iclb:         FAIL [fdo#108341] -> PASS

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +1

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  * igt@perf@oa-exponents:
    - shard-glk:          FAIL [fdo#105483] -> PASS

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

  * Linux: CI_DRM_5961 -> Patchwork_12849

  CI_DRM_5961: c5fdcd8bd5c596bfc70b6be93e6f04d910c51308 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12849: 18f65425d76cf341a88c22d8cbd6d9473b97b8d8 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12849/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/5] drm/i915/uc: Rename uC firmware init/fini functions
  2019-04-19 23:00 ` [PATCH v3 1/5] drm/i915/uc: Rename uC firmware init/fini functions Fernando Pacheco
@ 2019-04-20  6:45   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-04-20  6:45 UTC (permalink / raw)
  To: Fernando Pacheco, intel-gfx

Quoting Fernando Pacheco (2019-04-20 00:00:11)
> The uC firmware init function is called during
> GuC/HuC init early phases. Rename to include "_early"
> and properly reflect which phase we are at.
> 
> The uC firmware fini function is cleaning up the
> state set/created on firmware fetch. Replace
> "_fini" with "_cleanup_fetch".
> 
> v2: also rename uC fw fini function
> 
> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/5] drm/i915/uc: Reserve upper range of GGTT
  2019-04-19 23:00 ` [PATCH v3 2/5] drm/i915/uc: Reserve upper range of GGTT Fernando Pacheco
@ 2019-04-20  6:47   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-04-20  6:47 UTC (permalink / raw)
  To: Fernando Pacheco, intel-gfx

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/5] drm/i915/uc: Place uC firmware in upper range of GGTT
  2019-04-19 23:00 ` [PATCH v3 3/5] drm/i915/uc: Place uC firmware in " Fernando Pacheco
@ 2019-04-20  6:53   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-04-20  6:53 UTC (permalink / raw)
  To: Fernando Pacheco, intel-gfx

Quoting Fernando Pacheco (2019-04-20 00:00:13)
> 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:
> - 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.
> 
> v2: Remove call to set to gtt domain
>     Do not restore fw gtt mapping unconditionally
>     Separate out pin/unpin functions and drop usage of pin/unpin
>     Use uc_fw init/fini functions to bind/unbind fw object
> 
> v3: Bind is only needed during xfer (Chris)
>     Remove attempts to bind outside of xfer (Chris)
>     Mark fw bind/unbind static
> 
> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.c    |   9 ++-
>  drivers/gpu/drm/i915/intel_guc_fw.c |  18 ++---
>  drivers/gpu/drm/i915/intel_huc.c    |  74 +++++++++++++++-----
>  drivers/gpu/drm/i915/intel_huc.h    |   4 ++
>  drivers/gpu/drm/i915/intel_huc_fw.c |  47 +++++++++----
>  drivers/gpu/drm/i915/intel_uc.c     |  23 +++++--
>  drivers/gpu/drm/i915/intel_uc_fw.c  | 103 ++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_uc_fw.h  |   7 +-
>  8 files changed, 212 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 299b6aa4fe28..3bbf45a3bf78 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -189,9 +189,13 @@ int intel_guc_init(struct intel_guc *guc)
>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
>         int ret;
>  
> -       ret = guc_shared_data_create(guc);
> +       ret = intel_uc_fw_init(&guc->fw);
>         if (ret)
>                 goto err_fetch;
> +
> +       ret = guc_shared_data_create(guc);
> +       if (ret)
> +               goto err_fw;
>         GEM_BUG_ON(!guc->shared_data);
>  
>         ret = intel_guc_log_create(&guc->log);
> @@ -220,6 +224,8 @@ int intel_guc_init(struct intel_guc *guc)
>         intel_guc_log_destroy(&guc->log);
>  err_shared:
>         guc_shared_data_destroy(guc);
> +err_fw:
> +       intel_uc_fw_fini(&guc->fw);
>  err_fetch:
>         intel_uc_fw_cleanup_fetch(&guc->fw);
>         return ret;
> @@ -237,6 +243,7 @@ void intel_guc_fini(struct intel_guc *guc)
>         intel_guc_ads_destroy(guc);
>         intel_guc_log_destroy(&guc->log);
>         guc_shared_data_destroy(guc);
> +       intel_uc_fw_fini(&guc->fw);
>         intel_uc_fw_cleanup_fetch(&guc->fw);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
> index 4385d9ef02bb..8b2dcc70b956 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -122,14 +122,16 @@ static void guc_prepare_xfer(struct intel_guc *guc)
>  }
>  
>  /* Copy RSA signature from the fw image to HW for verification */
> -static void guc_xfer_rsa(struct intel_guc *guc, struct i915_vma *vma)
> +static void guc_xfer_rsa(struct intel_guc *guc)
>  {
>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +       struct intel_uc_fw *fw = &guc->fw;
> +       struct sg_table *pages = fw->obj->mm.pages;
>         u32 rsa[UOS_RSA_SCRATCH_COUNT];
>         int i;
>  
> -       sg_pcopy_to_buffer(vma->pages->sgl, vma->pages->nents,
> -                          rsa, sizeof(rsa), guc->fw.rsa_offset);
> +       sg_pcopy_to_buffer(pages->sgl, pages->nents,
> +                          rsa, sizeof(rsa), fw->rsa_offset);
>  
>         for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
>                 I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
> @@ -201,7 +203,7 @@ static int guc_wait_ucode(struct intel_guc *guc)
>   * transfer between GTT locations. This functionality is left out of the API
>   * for now as there is no need for it.
>   */
> -static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
> +static int guc_xfer_ucode(struct intel_guc *guc)
>  {
>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
>         struct intel_uc_fw *guc_fw = &guc->fw;
> @@ -214,7 +216,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
>         I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
>  
>         /* Set the source address for the new blob */
> -       offset = intel_guc_ggtt_offset(guc, vma) + guc_fw->header_offset;
> +       offset = intel_uc_fw_ggtt_offset(guc_fw) + guc_fw->header_offset;
>         I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
>         I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
>  
> @@ -233,7 +235,7 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
>  /*
>   * Load the GuC firmware blob into the MinuteIA.
>   */
> -static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
> +static int guc_fw_xfer(struct intel_uc_fw *guc_fw)
>  {
>         struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
> @@ -250,9 +252,9 @@ static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
>          * by the DMA engine in one operation, whereas the RSA signature is
>          * loaded via MMIO.
>          */
> -       guc_xfer_rsa(guc, vma);
> +       guc_xfer_rsa(guc);
>  
> -       ret = guc_xfer_ucode(guc, vma);
> +       ret = guc_xfer_ucode(guc);
>  
>         intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_ALL);
>  
> diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
> index 94c04f16a2ad..1ff1fb015e58 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -40,6 +40,61 @@ int intel_huc_init_misc(struct intel_huc *huc)
>         return 0;
>  }
>  
> +static int intel_huc_rsa_data_create(struct intel_huc *huc)
> +{
> +       struct drm_i915_private *i915 = huc_to_i915(huc);
> +       struct intel_guc *guc = &i915->guc;
> +       struct i915_vma *vma;
> +       void *vaddr;
> +
> +       /*
> +        * HuC firmware will sit above GUC_GGTT_TOP and will not map
> +        * through GTT. Unfortunately, this means GuC cannot perform
> +        * the HuC auth. as the rsa offset now falls within the GuC
> +        * inaccessible range. We resort to perma-pinning an additional
> +        * vma within the accessible range that only contains the rsa
> +        * signature. The GuC can use this extra pinning to perform
> +        * the authentication since its GGTT offset will be GuC
> +        * accessible.
> +        */
> +       vma = intel_guc_allocate_vma(guc, PAGE_SIZE);
> +       if (IS_ERR(vma))
> +               return PTR_ERR(vma);
> +
> +       vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> +       if (IS_ERR(vaddr)) {
> +               i915_vma_unpin_and_release(&vma, 0);
> +               return PTR_ERR(vaddr);
> +       }
> +
> +       huc->rsa_data = vma;
> +       huc->rsa_data_vaddr = vaddr;
> +
> +       return 0;
> +}
> +
> +static void intel_huc_rsa_data_destroy(struct intel_huc *huc)
> +{
> +       i915_vma_unpin_and_release(&huc->rsa_data, I915_VMA_RELEASE_MAP);
> +}
> +
> +int intel_huc_init(struct intel_huc *huc)
> +{
> +       int err;
> +
> +       err = intel_huc_rsa_data_create(huc);
> +       if (err)
> +               return err;
> +
> +       return intel_uc_fw_init(&huc->fw);
> +}
> +
> +void intel_huc_fini(struct intel_huc *huc)
> +{
> +       intel_uc_fw_fini(&huc->fw);
> +       intel_huc_rsa_data_destroy(huc);
> +}
> +
>  /**
>   * intel_huc_auth() - Authenticate HuC uCode
>   * @huc: intel_huc structure
> @@ -55,27 +110,17 @@ int intel_huc_auth(struct intel_huc *huc)
>  {
>         struct drm_i915_private *i915 = huc_to_i915(huc);
>         struct intel_guc *guc = &i915->guc;
> -       struct i915_vma *vma;
>         u32 status;
>         int ret;
>  
>         if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>                 return -ENOEXEC;
>  
> -       vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
> -                                      PIN_OFFSET_BIAS | i915->ggtt.pin_bias);
> -       if (IS_ERR(vma)) {
> -               ret = PTR_ERR(vma);
> -               DRM_ERROR("HuC: Failed to pin huc fw object %d\n", ret);
> -               goto fail;
> -       }
> -
>         ret = intel_guc_auth_huc(guc,
> -                                intel_guc_ggtt_offset(guc, vma) +
> -                                huc->fw.rsa_offset);
> +                                intel_guc_ggtt_offset(guc, huc->rsa_data));
>         if (ret) {
>                 DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
> -               goto fail_unpin;
> +               goto fail;
>         }
>  
>         /* Check authentication status, it should be done by now */
> @@ -86,14 +131,11 @@ int intel_huc_auth(struct intel_huc *huc)
>                                         2, 50, &status);
>         if (ret) {
>                 DRM_ERROR("HuC: Firmware not verified %#x\n", status);
> -               goto fail_unpin;
> +               goto fail;
>         }
>  
> -       i915_vma_unpin(vma);
>         return 0;
>  
> -fail_unpin:
> -       i915_vma_unpin(vma);
>  fail:
>         huc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
>  
> diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
> index ce129e301961..a0c21ae02a99 100644
> --- a/drivers/gpu/drm/i915/intel_huc.h
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -33,10 +33,14 @@ struct intel_huc {
>         struct intel_uc_fw fw;
>  
>         /* HuC-specific additions */
> +       struct i915_vma *rsa_data;
> +       void *rsa_data_vaddr;
>  };
>  
>  void intel_huc_init_early(struct intel_huc *huc);
>  int intel_huc_init_misc(struct intel_huc *huc);
> +int intel_huc_init(struct intel_huc *huc);
> +void intel_huc_fini(struct intel_huc *huc);
>  int intel_huc_auth(struct intel_huc *huc);
>  int intel_huc_check_status(struct intel_huc *huc);
>  
> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c b/drivers/gpu/drm/i915/intel_huc_fw.c
> index 80a176d91edc..44c559526072 100644
> --- a/drivers/gpu/drm/i915/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_huc_fw.c
> @@ -93,18 +93,24 @@ void intel_huc_fw_init_early(struct intel_huc *huc)
>         huc_fw_select(huc_fw);
>  }
>  
> -/**
> - * huc_fw_xfer() - DMA's the firmware
> - * @huc_fw: the firmware descriptor
> - * @vma: the firmware image (bound into the GGTT)
> - *
> - * Transfer the firmware image to RAM for execution by the microcontroller.
> - *
> - * Return: 0 on success, non-zero on failure
> - */
> -static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
> +static void huc_xfer_rsa(struct intel_huc *huc)
>  {
> -       struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> +       struct intel_uc_fw *fw = &huc->fw;
> +       struct sg_table *pages = fw->obj->mm.pages;
> +
> +       /*
> +        * HuC firmware image is outside GuC accessible range.
> +        * Copy the RSA signature out of the image into
> +        * the perma-pinned region set aside for it
> +        */
> +       sg_pcopy_to_buffer(pages->sgl, pages->nents,
> +                          huc->rsa_data_vaddr, fw->rsa_size,
> +                          fw->rsa_offset);

The sneaky part is that as we have to manage caches ourselves (to avoid
locks) we need a drm_clflush_sg() here to ensure it is flushed to memory
prior to the xfer. (We may be lucky that the xfer snoops or is on the
right side of llc, but is it worth taking the risk?)

> +}
> +
> +static int huc_xfer_ucode(struct intel_huc *huc)
> +{
> +       struct intel_uc_fw *huc_fw = &huc->fw;
>         struct drm_i915_private *dev_priv = huc_to_i915(huc);
>         struct intel_uncore *uncore = &dev_priv->uncore;
>         unsigned long offset = 0;
> @@ -116,7 +122,7 @@ static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
>         intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
>  
>         /* Set the source address for the uCode */
> -       offset = intel_guc_ggtt_offset(&dev_priv->guc, vma) +
> +       offset = intel_uc_fw_ggtt_offset(huc_fw) +
>                  huc_fw->header_offset;
>         intel_uncore_write(uncore, DMA_ADDR_0_LOW,
>                            lower_32_bits(offset));
> @@ -150,6 +156,23 @@ static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
>         return ret;
>  }
>  
> +/**
> + * huc_fw_xfer() - DMA's the firmware
> + * @huc_fw: the firmware descriptor
> + *
> + * Transfer the firmware image to RAM for execution by the microcontroller.
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +static int huc_fw_xfer(struct intel_uc_fw *huc_fw)
> +{
> +       struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
> +
> +       huc_xfer_rsa(huc);
> +
> +       return huc_xfer_ucode(huc);
> +}
> +
>  /**
>   * intel_huc_fw_upload() - load HuC uCode to device
>   * @huc: intel_huc structure
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 25b80ffe71ad..488dffba04d2 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -280,6 +280,7 @@ void intel_uc_fini_misc(struct drm_i915_private *i915)
>  int intel_uc_init(struct drm_i915_private *i915)
>  {
>         struct intel_guc *guc = &i915->guc;
> +       struct intel_huc *huc = &i915->huc;
>         int ret;
>  
>         if (!USES_GUC(i915))
> @@ -292,19 +293,30 @@ int intel_uc_init(struct drm_i915_private *i915)
>         if (ret)
>                 return ret;
>  
> +       if (USES_HUC(i915)) {
> +               ret = intel_huc_init(huc);
> +               if (ret)
> +                       goto err_guc;
> +       }
> +
>         if (USES_GUC_SUBMISSION(i915)) {
>                 /*
>                  * This is stuff we need to have available at fw load time
>                  * if we are planning to enable submission later
>                  */
>                 ret = intel_guc_submission_init(guc);
> -               if (ret) {
> -                       intel_guc_fini(guc);
> -                       return ret;
> -               }
> +               if (ret)
> +                       goto err_huc;
>         }
>  
>         return 0;
> +
> +err_huc:
> +       if (USES_HUC(i915))
> +               intel_huc_fini(huc);
> +err_guc:
> +       intel_guc_fini(guc);
> +       return ret;
>  }
>  
>  void intel_uc_fini(struct drm_i915_private *i915)
> @@ -319,6 +331,9 @@ void intel_uc_fini(struct drm_i915_private *i915)
>         if (USES_GUC_SUBMISSION(i915))
>                 intel_guc_submission_fini(guc);
>  
> +       if (USES_HUC(i915))
> +               intel_huc_fini(&i915->huc);
> +
>         intel_guc_fini(guc);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
> index e3e74207a102..d1191682d285 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -191,6 +191,39 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>         release_firmware(fw);           /* OK even if fw is NULL */
>  }
>  
> +static void intel_uc_fw_ggtt_bind(struct intel_uc_fw *uc_fw)
> +{
> +       struct drm_i915_gem_object *obj = uc_fw->obj;
> +       struct i915_ggtt *ggtt;
> +       struct i915_vma dummy;
> +
> +       GEM_BUG_ON(!obj);

About as useful as the GPF oops report, so we tend to just use that oops
and make the code tidier.

> +
> +       ggtt = &to_i915(obj->base.dev)->ggtt;
> +
> +       dummy.node.start = intel_uc_fw_ggtt_offset(uc_fw);
> +       dummy.node.size = obj->base.size;
> +       dummy.pages = obj->mm.pages;
> +       dummy.vm = &ggtt->vm;
> +
> +       GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
> +       ggtt->vm.insert_entries(&ggtt->vm, &dummy, obj->cache_level, 0);
> +}
> +
> +static void intel_uc_fw_ggtt_unbind(struct intel_uc_fw *uc_fw)
> +{
> +       struct drm_i915_gem_object *obj = uc_fw->obj;
> +       struct i915_ggtt *ggtt;
> +       u64 start;
> +
> +       GEM_BUG_ON(!obj);
> +
> +       ggtt = &to_i915(obj->base.dev)->ggtt;


> +       start = intel_uc_fw_ggtt_offset(uc_fw);
> +
> +       ggtt->vm.clear_range(&ggtt->vm, start, obj->base.size);
> +}
> +
>  /**
>   * intel_uc_fw_upload - load uC firmware using custom loader
>   * @uc_fw: uC firmware
> @@ -201,11 +234,8 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>   * Return: 0 on success, non-zero on failure.
>   */
>  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> -                      int (*xfer)(struct intel_uc_fw *uc_fw,
> -                                  struct i915_vma *vma))
> +                      int (*xfer)(struct intel_uc_fw *uc_fw))
>  {
> -       struct i915_vma *vma;
> -       u32 ggtt_pin_bias;
>         int err;
>  
>         DRM_DEBUG_DRIVER("%s fw load %s\n",
> @@ -219,36 +249,15 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
>                          intel_uc_fw_type_repr(uc_fw->type),
>                          intel_uc_fw_status_repr(uc_fw->load_status));
>  
> -       /* Pin object with firmware */
> -       err = i915_gem_object_set_to_gtt_domain(uc_fw->obj, false);
> -       if (err) {
> -               DRM_DEBUG_DRIVER("%s fw set-domain err=%d\n",
> -                                intel_uc_fw_type_repr(uc_fw->type), err);
> -               goto fail;
> -       }
> -
> -       ggtt_pin_bias = to_i915(uc_fw->obj->base.dev)->ggtt.pin_bias;
> -       vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
> -                                      PIN_OFFSET_BIAS | ggtt_pin_bias);
> -       if (IS_ERR(vma)) {
> -               err = PTR_ERR(vma);
> -               DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
> -                                intel_uc_fw_type_repr(uc_fw->type), err);
> -               goto fail;
> -       }
> +       intel_uc_fw_ggtt_bind(uc_fw);
>  
>         /* Call custom loader */
> -       err = xfer(uc_fw, vma);
> -
> -       /*
> -        * We keep the object pages for reuse during resume. But we can unpin it
> -        * now that DMA has completed, so it doesn't continue to take up space.
> -        */
> -       i915_vma_unpin(vma);
> -
> +       err = xfer(uc_fw);
>         if (err)
>                 goto fail;
>  
> +       intel_uc_fw_ggtt_unbind(uc_fw);
> +
>         uc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
>         DRM_DEBUG_DRIVER("%s fw load %s\n",
>                          intel_uc_fw_type_repr(uc_fw->type),
> @@ -273,6 +282,42 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
>         return err;
>  }
>  
> +int intel_uc_fw_init(struct intel_uc_fw *uc_fw)
> +{
> +       int err;
> +
> +       if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
> +               return -ENOEXEC;
> +
> +       err = i915_gem_object_pin_pages(uc_fw->obj);
> +       if (err)
> +               DRM_DEBUG_DRIVER("%s fw pin-pages err=%d\n",
> +                                intel_uc_fw_type_repr(uc_fw->type), err);
> +
> +       return err;
> +}
> +
> +void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
> +{
> +       if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
> +               return;
> +
> +       i915_gem_object_unpin_pages(uc_fw->obj);
> +}
> +
> +u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw)
> +{
> +       struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
> +       struct i915_ggtt *ggtt = &i915->ggtt;
> +       struct drm_mm_node *node = &ggtt->uc_fw;
> +
> +       GEM_BUG_ON(!node->allocated);
> +       GEM_BUG_ON(upper_32_bits(node->start));
> +       GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
> +
> +       return lower_32_bits(node->start);
> +}
> +
>  /**
>   * intel_uc_fw_cleanup_fetch - cleanup uC firmware
>   *
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
> index e6fa8599757c..ff98f8661d72 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
> @@ -27,7 +27,6 @@
>  
>  struct drm_printer;
>  struct drm_i915_private;
> -struct i915_vma;
>  
>  /* Home of GuC, HuC and DMC firmwares */
>  #define INTEL_UC_FIRMWARE_URL "https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/i915"
> @@ -147,8 +146,10 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>                        struct intel_uc_fw *uc_fw);
>  void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);
>  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
> -                      int (*xfer)(struct intel_uc_fw *uc_fw,
> -                                  struct i915_vma *vma));
> +                      int (*xfer)(struct intel_uc_fw *uc_fw));
> +int intel_uc_fw_init(struct intel_uc_fw *uc_fw);
> +void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
> +u32 intel_uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw);
>  void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p);

With clflush,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/5] Revert "drm/i915/guc: Disable global reset"
  2019-04-19 23:00 ` [PATCH v3 4/5] Revert "drm/i915/guc: Disable global reset" Fernando Pacheco
@ 2019-04-20  6:53   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-04-20  6:53 UTC (permalink / raw)
  To: Fernando Pacheco, intel-gfx

Quoting Fernando Pacheco (2019-04-20 00:00:14)
> This reverts commit fe62365f9f80a1c1d438c54fba21f5108a182de8.
> 
> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>

Many thanks,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 5/5] drm/i915/selftests: Check that gpu reset is usable from atomic context
  2019-04-19 23:00 ` [PATCH v3 5/5] drm/i915/selftests: Check that gpu reset is usable from atomic context Fernando Pacheco
@ 2019-04-20  6:54   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2019-04-20  6:54 UTC (permalink / raw)
  To: Fernando Pacheco, intel-gfx

Quoting Fernando Pacheco (2019-04-20 00:00:15)
> GPU reset is now available with GuC enabled,
> so re-enable our check that this reset is usable
> from atomic context.
> 
> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-04-20  6:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-19 23:00 [PATCH v3 0/5] Perma-pin uC firmware and re-enable global reset Fernando Pacheco
2019-04-19 23:00 ` [PATCH v3 1/5] drm/i915/uc: Rename uC firmware init/fini functions Fernando Pacheco
2019-04-20  6:45   ` Chris Wilson
2019-04-19 23:00 ` [PATCH v3 2/5] drm/i915/uc: Reserve upper range of GGTT Fernando Pacheco
2019-04-20  6:47   ` Chris Wilson
2019-04-19 23:00 ` [PATCH v3 3/5] drm/i915/uc: Place uC firmware in " Fernando Pacheco
2019-04-20  6:53   ` Chris Wilson
2019-04-19 23:00 ` [PATCH v3 4/5] Revert "drm/i915/guc: Disable global reset" Fernando Pacheco
2019-04-20  6:53   ` Chris Wilson
2019-04-19 23:00 ` [PATCH v3 5/5] drm/i915/selftests: Check that gpu reset is usable from atomic context Fernando Pacheco
2019-04-20  6:54   ` Chris Wilson
2019-04-19 23:22 ` ✗ Fi.CI.SPARSE: warning for Perma-pin uC firmware and re-enable global reset (rev3) Patchwork
2019-04-20  0:16 ` ✓ Fi.CI.BAT: success " Patchwork
2019-04-20  2:28 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.