[PATCH v2 1/5] drm/i915/uc: Don't enable communication twice on resume

[PATCH v2 1/5] drm/i915/uc: Don't enable communication twice on resume

[Daniele Ceraolo Spurio]

When coming out of S3/S4 we sanitize and re-init the HW, which includes
enabling communication during uc_init_hw. We therefore don't want to do
that again in uc_resume and can just tell GuC to reload its state.

v2: split uc_resume and uc_runtime_resume to match the suspend
    functions and to better differentiate the expected state in the 2
    scenarios (Chris)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 35 +++++++++++++++++++++++++--
 drivers/gpu/drm/i915/gt/uc/intel_uc.h |  1 +
 drivers/gpu/drm/i915/i915_drv.c       |  4 +--
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 6eb8bb3fa252..657fdcb70d00 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -233,11 +233,20 @@ static void guc_disable_interrupts(struct intel_guc *guc)
 	guc->interrupts.disable(guc);
 }
 
+#ifdef CONFIG_DRM_I915_DEBUG_GEM
+static bool guc_communication_enabled(struct intel_guc *guc)
+{
+	return guc->send != intel_guc_send_nop;
+}
+#endif
+
 static int guc_enable_communication(struct intel_guc *guc)
 {
 	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
 	int ret;
 
+	GEM_BUG_ON(guc_communication_enabled(guc));
+
 	ret = intel_guc_ct_enable(&guc->ct);
 	if (ret)
 		return ret;
@@ -550,7 +559,7 @@ void intel_uc_suspend(struct intel_uc *uc)
 		intel_uc_runtime_suspend(uc);
 }
 
-int intel_uc_resume(struct intel_uc *uc)
+static int __uc_resume(struct intel_uc *uc, bool enable_communication)
 {
 	struct intel_guc *guc = &uc->guc;
 	int err;
@@ -558,7 +567,11 @@ int intel_uc_resume(struct intel_uc *uc)
 	if (!intel_guc_is_running(guc))
 		return 0;
 
-	guc_enable_communication(guc);
+	/* Make sure we enable communication if and only if it's disabled */
+	GEM_BUG_ON(enable_communication == guc_communication_enabled(guc));
+
+	if (enable_communication)
+		guc_enable_communication(guc);
 
 	err = intel_guc_resume(guc);
 	if (err) {
@@ -568,3 +581,21 @@ int intel_uc_resume(struct intel_uc *uc)
 
 	return 0;
 }
+
+int intel_uc_resume(struct intel_uc *uc)
+{
+	/*
+	 * When coming out of S3/S4 we sanitize and re-init the HW, so
+	 * communication is already re-enabled at this point.
+	 */
+	return __uc_resume(uc, false);
+}
+
+int intel_uc_runtime_resume(struct intel_uc *uc)
+{
+	/*
+	 * During runtime resume we don't sanitize, so we need to re-init
+	 * communication as well.
+	 */
+	return __uc_resume(uc, true);
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index fe3362fd7706..25da51e95417 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -47,6 +47,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc);
 void intel_uc_suspend(struct intel_uc *uc);
 void intel_uc_runtime_suspend(struct intel_uc *uc);
 int intel_uc_resume(struct intel_uc *uc);
+int intel_uc_runtime_resume(struct intel_uc *uc);
 
 static inline bool intel_uc_is_using_guc(struct intel_uc *uc)
 {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f2d3d754af37..761726818a22 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2950,7 +2950,7 @@ static int intel_runtime_suspend(struct device *kdev)
 
 		intel_runtime_pm_enable_interrupts(dev_priv);
 
-		intel_uc_resume(&dev_priv->gt.uc);
+		intel_uc_runtime_resume(&dev_priv->gt.uc);
 
 		intel_gt_init_swizzling(&dev_priv->gt);
 		i915_gem_restore_fences(dev_priv);
@@ -3047,7 +3047,7 @@ static int intel_runtime_resume(struct device *kdev)
 
 	intel_runtime_pm_enable_interrupts(dev_priv);
 
-	intel_uc_resume(&dev_priv->gt.uc);
+	intel_uc_runtime_resume(&dev_priv->gt.uc);
 
 	/*
 	 * No point of rolling back things in case of an error, as the best
-- 
2.22.0

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

[PATCH v2 2/5] drm/i915/uc: Move uC WOPCM setup in uc_init_hw

[Daniele Ceraolo Spurio]

The register we write are not WOPCM regs but uC ones related to how
GuC and HuC are going to use the WOPCM, so it makes logical sense
for them to be programmed as part of uc_init_hw. The WOPCM map on the
other side is not uC-specific (although that is our main use-case), so
keep that separate.

v2: move write_and_verify to uncore, fix log, re-use err_out tag,
    add intel_wopcm_guc_base, fix log

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 47 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h       |  3 +-
 drivers/gpu/drm/i915/i915_gem.c       |  8 +---
 drivers/gpu/drm/i915/intel_uncore.h   | 12 +++++
 drivers/gpu/drm/i915/intel_wopcm.c    | 68 ---------------------------
 drivers/gpu/drm/i915/intel_wopcm.h    | 18 +++++--
 6 files changed, 76 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 657fdcb70d00..7794a6a1f932 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -399,6 +399,49 @@ void intel_uc_sanitize(struct intel_uc *uc)
 	__uc_sanitize(uc);
 }
 
+/* Initialize and verify the uC regs related to uC positioning in WOPCM */
+static int uc_init_wopcm(struct intel_uc *uc)
+{
+	struct intel_gt *gt = uc_to_gt(uc);
+	struct intel_uncore *uncore = gt->uncore;
+	u32 base = intel_wopcm_guc_base(&gt->i915->wopcm);
+	u32 size = intel_wopcm_guc_size(&gt->i915->wopcm);
+	u32 huc_agent = intel_uc_is_using_huc(uc) ? HUC_LOADING_AGENT_GUC : 0;
+	u32 mask;
+	int err;
+
+	GEM_BUG_ON(!intel_uc_is_using_guc(uc));
+	GEM_BUG_ON(!(base & GUC_WOPCM_OFFSET_MASK));
+	GEM_BUG_ON(base & ~GUC_WOPCM_OFFSET_MASK);
+	GEM_BUG_ON(!(size & GUC_WOPCM_SIZE_MASK));
+	GEM_BUG_ON(size & ~GUC_WOPCM_SIZE_MASK);
+
+	mask = GUC_WOPCM_SIZE_MASK | GUC_WOPCM_SIZE_LOCKED;
+	err = intel_uncore_write_and_verify(uncore, GUC_WOPCM_SIZE, size, mask,
+					    size | GUC_WOPCM_SIZE_LOCKED);
+	if (err)
+		goto err_out;
+
+	mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
+	err = intel_uncore_write_and_verify(uncore, DMA_GUC_WOPCM_OFFSET,
+					    base | huc_agent, mask,
+					    base | huc_agent |
+					    GUC_WOPCM_OFFSET_VALID);
+	if (err)
+		goto err_out;
+
+	return 0;
+
+err_out:
+	DRM_ERROR("Failed to init uC WOPCM registers:\n");
+	DRM_ERROR("DMA_GUC_WOPCM_OFFSET=%#x\n",
+		  intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
+	DRM_ERROR("GUC_WOPCM_SIZE=%#x\n",
+		  intel_uncore_read(uncore, GUC_WOPCM_SIZE));
+
+	return err;
+}
+
 int intel_uc_init_hw(struct intel_uc *uc)
 {
 	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
@@ -411,6 +454,10 @@ int intel_uc_init_hw(struct intel_uc *uc)
 
 	GEM_BUG_ON(!intel_uc_fw_supported(&guc->fw));
 
+	ret = uc_init_wopcm(uc);
+	if (ret)
+		goto err_out;
+
 	guc_reset_interrupts(guc);
 
 	/* WaEnableuKernelHeaderValidFix:skl */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3eeb21ff04c2..bdede6325d87 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2274,10 +2274,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 
 #define HAS_GT_UC(dev_priv)	(INTEL_INFO(dev_priv)->has_gt_uc)
 
-/* Having GuC/HuC is not the same as using GuC/HuC */
+/* Having GuC is not the same as using GuC */
 #define USES_GUC(dev_priv)		intel_uc_is_using_guc(&(dev_priv)->gt.uc)
 #define USES_GUC_SUBMISSION(dev_priv)	intel_uc_is_using_guc_submission(&(dev_priv)->gt.uc)
-#define USES_HUC(dev_priv)		intel_uc_is_using_huc(&(dev_priv)->gt.uc)
 
 #define HAS_POOLED_EU(dev_priv)	(INTEL_INFO(dev_priv)->has_pooled_eu)
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 65863e955f40..f681152d27fa 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1240,14 +1240,8 @@ int i915_gem_init_hw(struct drm_i915_private *i915)
 		goto out;
 	}
 
-	ret = intel_wopcm_init_hw(&i915->wopcm, gt);
-	if (ret) {
-		DRM_ERROR("Enabling WOPCM failed (%d)\n", ret);
-		goto out;
-	}
-
 	/* We can't enable contexts until all firmware is loaded */
-	ret = intel_uc_init_hw(&i915->gt.uc);
+	ret = intel_uc_init_hw(&gt->uc);
 	if (ret) {
 		DRM_ERROR("Enabling uc failed (%d)\n", ret);
 		goto out;
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 2f6ffa309669..e603d210a34d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -393,6 +393,18 @@ static inline void intel_uncore_rmw_fw(struct intel_uncore *uncore,
 	intel_uncore_write_fw(uncore, reg, val);
 }
 
+static inline int intel_uncore_write_and_verify(struct intel_uncore *uncore,
+						i915_reg_t reg, u32 val,
+						u32 mask, u32 expected_val)
+{
+	u32 reg_val;
+
+	intel_uncore_write(uncore, reg, val);
+	reg_val = intel_uncore_read(uncore, reg);
+
+	return (reg_val & mask) != expected_val ? -EINVAL : 0;
+}
+
 #define raw_reg_read(base, reg) \
 	readl(base + i915_mmio_reg_offset(reg))
 #define raw_reg_write(base, reg, value) \
diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 0e86a9e85b49..d9973c0b0384 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -224,71 +224,3 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
 
 	return 0;
 }
-
-static int
-write_and_verify(struct intel_gt *gt,
-		 i915_reg_t reg, u32 val, u32 mask, u32 locked_bit)
-{
-	struct intel_uncore *uncore = gt->uncore;
-	u32 reg_val;
-
-	GEM_BUG_ON(val & ~mask);
-
-	intel_uncore_write(uncore, reg, val);
-
-	reg_val = intel_uncore_read(uncore, reg);
-
-	return (reg_val & mask) != (val | locked_bit) ? -EIO : 0;
-}
-
-/**
- * intel_wopcm_init_hw() - Setup GuC WOPCM registers.
- * @wopcm: pointer to intel_wopcm.
- * @gt: pointer to the containing GT
- *
- * Setup the GuC WOPCM size and offset registers with the calculated values. It
- * will verify the register values to make sure the registers are locked with
- * correct values.
- *
- * Return: 0 on success. -EIO if registers were locked with incorrect values.
- */
-int intel_wopcm_init_hw(struct intel_wopcm *wopcm, struct intel_gt *gt)
-{
-	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
-	struct intel_uncore *uncore = gt->uncore;
-	u32 huc_agent;
-	u32 mask;
-	int err;
-
-	if (!USES_GUC(i915))
-		return 0;
-
-	GEM_BUG_ON(!HAS_GT_UC(i915));
-	GEM_BUG_ON(!wopcm->guc.size);
-	GEM_BUG_ON(!wopcm->guc.base);
-
-	err = write_and_verify(gt, GUC_WOPCM_SIZE, wopcm->guc.size,
-			       GUC_WOPCM_SIZE_MASK | GUC_WOPCM_SIZE_LOCKED,
-			       GUC_WOPCM_SIZE_LOCKED);
-	if (err)
-		goto err_out;
-
-	huc_agent = USES_HUC(i915) ? HUC_LOADING_AGENT_GUC : 0;
-	mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
-	err = write_and_verify(gt, DMA_GUC_WOPCM_OFFSET,
-			       wopcm->guc.base | huc_agent, mask,
-			       GUC_WOPCM_OFFSET_VALID);
-	if (err)
-		goto err_out;
-
-	return 0;
-
-err_out:
-	DRM_ERROR("Failed to init WOPCM registers:\n");
-	DRM_ERROR("DMA_GUC_WOPCM_OFFSET=%#x\n",
-		  intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
-	DRM_ERROR("GUC_WOPCM_SIZE=%#x\n",
-		  intel_uncore_read(uncore, GUC_WOPCM_SIZE));
-
-	return err;
-}
diff --git a/drivers/gpu/drm/i915/intel_wopcm.h b/drivers/gpu/drm/i915/intel_wopcm.h
index 56aaed4d64ff..f9b603205bb1 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.h
+++ b/drivers/gpu/drm/i915/intel_wopcm.h
@@ -9,8 +9,6 @@
 
 #include <linux/types.h>
 
-struct intel_gt;
-
 /**
  * struct intel_wopcm - Overall WOPCM info and WOPCM regions.
  * @size: Size of overall WOPCM.
@@ -26,6 +24,21 @@ struct intel_wopcm {
 	} guc;
 };
 
+/**
+ * intel_wopcm_guc_base()
+ * @wopcm:	intel_wopcm structure
+ *
+ * Returns the base of the WOPCM shadowed region.
+ *
+ * Returns:
+ * 0 if GuC is not present or not in use.
+ * Otherwise, the GuC WOPCM base.
+ */
+static inline u32 intel_wopcm_guc_base(struct intel_wopcm *wopcm)
+{
+	return wopcm->guc.base;
+}
+
 /**
  * intel_wopcm_guc_size()
  * @wopcm:	intel_wopcm structure
@@ -43,6 +56,5 @@ static inline u32 intel_wopcm_guc_size(struct intel_wopcm *wopcm)
 
 void intel_wopcm_init_early(struct intel_wopcm *wopcm);
 int intel_wopcm_init(struct intel_wopcm *wopcm);
-int intel_wopcm_init_hw(struct intel_wopcm *wopcm, struct intel_gt *gt);
 
 #endif
-- 
2.22.0

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

[PATCH v2 3/5] drm/i915/gt: Move gt_cleanup_early out of gem_cleanup_early

[Daniele Ceraolo Spurio]

We don't call the init_early function from within the gem code, so we
shouldn't do it for the cleanup either.

v2: while at it, s/gt_cleanup_early/gt_late_release (Chris)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v1
---
 drivers/gpu/drm/i915/gt/intel_gt.c | 2 +-
 drivers/gpu/drm/i915/gt/intel_gt.h | 2 +-
 drivers/gpu/drm/i915/i915_drv.c    | 2 ++
 drivers/gpu/drm/i915/i915_gem.c    | 2 --
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index f7e69db4019d..5e76a426994a 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -244,7 +244,7 @@ void intel_gt_fini_scratch(struct intel_gt *gt)
 	i915_vma_unpin_and_release(&gt->scratch, 0);
 }
 
-void intel_gt_cleanup_early(struct intel_gt *gt)
+void intel_gt_late_release(struct intel_gt *gt)
 {
 	intel_gt_fini_reset(gt);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index 640bb0531f5b..123247ad32b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -30,7 +30,7 @@ static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
 void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915);
 void intel_gt_init_hw(struct drm_i915_private *i915);
 
-void intel_gt_cleanup_early(struct intel_gt *gt);
+void intel_gt_late_release(struct intel_gt *gt);
 
 void intel_gt_check_and_clear_faults(struct intel_gt *gt);
 void intel_gt_clear_error_registers(struct intel_gt *gt,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 761726818a22..4b1bba018df7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -951,6 +951,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	intel_uc_cleanup_early(&dev_priv->gt.uc);
 	i915_gem_cleanup_early(dev_priv);
 err_workqueues:
+	intel_gt_late_release(&dev_priv->gt);
 	i915_workqueues_cleanup(dev_priv);
 	return ret;
 }
@@ -966,6 +967,7 @@ static void i915_driver_late_release(struct drm_i915_private *dev_priv)
 	intel_power_domains_cleanup(dev_priv);
 	intel_uc_cleanup_early(&dev_priv->gt.uc);
 	i915_gem_cleanup_early(dev_priv);
+	intel_gt_late_release(&dev_priv->gt);
 	i915_workqueues_cleanup(dev_priv);
 
 	pm_qos_remove_request(&dev_priv->sb_qos);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f681152d27fa..8438ce037e4b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1689,8 +1689,6 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
 	GEM_BUG_ON(atomic_read(&dev_priv->mm.free_count));
 	WARN_ON(dev_priv->mm.shrink_count);
 
-	intel_gt_cleanup_early(&dev_priv->gt);
-
 	i915_gemfs_fini(dev_priv);
 }
 
-- 
2.22.0

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

[PATCH v2 4/5] drm/i915/uc: Move uC early functions inside the GT ones

[Daniele Ceraolo Spurio]

uC is a subcomponent of GT, so initialize/clean it as part of it. The
wopcm_init_early doesn't have to be happen before the uC one, but since
in other parts of the code we consider WOPCM first do the same for
consistency.

v2: s/cleanup_early/late_release to match the caller

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> #v1
---
 drivers/gpu/drm/i915/gt/intel_gt.c    |  2 ++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c |  2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc.h |  2 +-
 drivers/gpu/drm/i915/i915_drv.c       | 10 ++++------
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 5e76a426994a..362cac779afb 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -22,6 +22,7 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
 	intel_gt_init_hangcheck(gt);
 	intel_gt_init_reset(gt);
 	intel_gt_pm_init_early(gt);
+	intel_uc_init_early(&gt->uc);
 }
 
 void intel_gt_init_hw(struct drm_i915_private *i915)
@@ -246,5 +247,6 @@ void intel_gt_fini_scratch(struct intel_gt *gt)
 
 void intel_gt_late_release(struct intel_gt *gt)
 {
+	intel_uc_late_release(&gt->uc);
 	intel_gt_fini_reset(gt);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 7794a6a1f932..ecd245cb6311 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -137,7 +137,7 @@ void intel_uc_init_early(struct intel_uc *uc)
 	sanitize_options_early(uc);
 }
 
-void intel_uc_cleanup_early(struct intel_uc *uc)
+void intel_uc_late_release(struct intel_uc *uc)
 {
 	guc_free_load_err_log(&uc->guc);
 }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 25da51e95417..3ba2ac74c1a8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -34,7 +34,7 @@ struct intel_uc {
 };
 
 void intel_uc_init_early(struct intel_uc *uc);
-void intel_uc_cleanup_early(struct intel_uc *uc);
+void intel_uc_late_release(struct intel_uc *uc);
 void intel_uc_init_mmio(struct intel_uc *uc);
 void intel_uc_fetch_firmwares(struct intel_uc *uc);
 void intel_uc_cleanup_firmwares(struct intel_uc *uc);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4b1bba018df7..86e024d063f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -921,6 +921,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		return ret;
 
+	intel_wopcm_init_early(&dev_priv->wopcm);
+
 	intel_gt_init_early(&dev_priv->gt, dev_priv);
 
 	ret = i915_gem_init_early(dev_priv);
@@ -930,13 +932,11 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 	/* This must be called before any calls to HAS_PCH_* */
 	intel_detect_pch(dev_priv);
 
-	intel_wopcm_init_early(&dev_priv->wopcm);
-	intel_uc_init_early(&dev_priv->gt.uc);
 	intel_pm_setup(dev_priv);
 	intel_init_dpio(dev_priv);
 	ret = intel_power_domains_init(dev_priv);
 	if (ret < 0)
-		goto err_uc;
+		goto err_gem;
 	intel_irq_init(dev_priv);
 	intel_init_display_hooks(dev_priv);
 	intel_init_clock_gating_hooks(dev_priv);
@@ -947,8 +947,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 
 	return 0;
 
-err_uc:
-	intel_uc_cleanup_early(&dev_priv->gt.uc);
+err_gem:
 	i915_gem_cleanup_early(dev_priv);
 err_workqueues:
 	intel_gt_late_release(&dev_priv->gt);
@@ -965,7 +964,6 @@ static void i915_driver_late_release(struct drm_i915_private *dev_priv)
 {
 	intel_irq_fini(dev_priv);
 	intel_power_domains_cleanup(dev_priv);
-	intel_uc_cleanup_early(&dev_priv->gt.uc);
 	i915_gem_cleanup_early(dev_priv);
 	intel_gt_late_release(&dev_priv->gt);
 	i915_workqueues_cleanup(dev_priv);
-- 
2.22.0

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

[PATCH v2 5/5] drm/i915/gt: Introduce intel_gt_runtime_suspend/resume

[Daniele Ceraolo Spurio]

To be called from the top level runtime functions, to hide the
gt-specific bits (mainly related to intel_uc).

v2: rebased

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm.c | 12 ++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_pm.h |  2 ++
 drivers/gpu/drm/i915/i915_drv.c       |  9 +++------
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 65c0d0c9d543..6c8970271a7f 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -164,3 +164,15 @@ int intel_gt_resume(struct intel_gt *gt)
 
 	return err;
 }
+
+void intel_gt_runtime_suspend(struct intel_gt *gt)
+{
+	intel_uc_runtime_suspend(&gt->uc);
+}
+
+int intel_gt_runtime_resume(struct intel_gt *gt)
+{
+	intel_gt_init_swizzling(gt);
+
+	return intel_uc_runtime_resume(&gt->uc);
+}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
index ba960e1fc209..527894fe1345 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
@@ -23,5 +23,7 @@ void intel_gt_pm_init_early(struct intel_gt *gt);
 
 void intel_gt_sanitize(struct intel_gt *gt, bool force);
 int intel_gt_resume(struct intel_gt *gt);
+void intel_gt_runtime_suspend(struct intel_gt *gt);
+int intel_gt_runtime_resume(struct intel_gt *gt);
 
 #endif /* INTEL_GT_PM_H */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 86e024d063f3..2c68ebd7284b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2925,7 +2925,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	 */
 	i915_gem_runtime_suspend(dev_priv);
 
-	intel_uc_runtime_suspend(&dev_priv->gt.uc);
+	intel_gt_runtime_suspend(&dev_priv->gt);
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
@@ -2950,9 +2950,8 @@ static int intel_runtime_suspend(struct device *kdev)
 
 		intel_runtime_pm_enable_interrupts(dev_priv);
 
-		intel_uc_runtime_resume(&dev_priv->gt.uc);
+		intel_gt_runtime_resume(&dev_priv->gt);
 
-		intel_gt_init_swizzling(&dev_priv->gt);
 		i915_gem_restore_fences(dev_priv);
 
 		enable_rpm_wakeref_asserts(rpm);
@@ -3047,13 +3046,11 @@ static int intel_runtime_resume(struct device *kdev)
 
 	intel_runtime_pm_enable_interrupts(dev_priv);
 
-	intel_uc_runtime_resume(&dev_priv->gt.uc);
-
 	/*
 	 * No point of rolling back things in case of an error, as the best
 	 * we can do is to hope that things will still work (and disable RPM).
 	 */
-	intel_gt_init_swizzling(&dev_priv->gt);
+	intel_gt_runtime_resume(&dev_priv->gt);
 	i915_gem_restore_fences(dev_priv);
 
 	/*
-- 
2.22.0

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

✗ Fi.CI.BAT: failure for series starting with [v2,1/5] drm/i915/uc: Don't enable communication twice on resume

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/5] drm/i915/uc: Don't enable communication twice on resume
URL   : https://patchwork.freedesktop.org/series/64459/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6586 -> Patchwork_13814
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13814 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13814, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13814:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@module-reload:
    - fi-whl-u:           [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-whl-u/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-whl-u/igt@i915_pm_rpm@module-reload.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-ilk-650:         [PASS][3] -> [DMESG-WARN][4] ([fdo#106387])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-ilk-650/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-ilk-650/igt@i915_module_load@reload.html

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       [PASS][5] -> [SKIP][6] ([fdo#109271] / [fdo#109278]) +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html

  * igt@kms_busy@basic-flip-c:
    - fi-kbl-7500u:       [PASS][7] -> [SKIP][8] ([fdo#109271] / [fdo#109278]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [PASS][9] -> [FAIL][10] ([fdo#103167])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-icl-dsi:         [INCOMPLETE][11] ([fdo#107713] / [fdo#109100]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@gem_exec_reloc@basic-write-gtt-noreloc:
    - fi-icl-u3:          [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt-noreloc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt-noreloc.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [INCOMPLETE][15] ([fdo#107718]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_module_load@reload-no-display:
    - fi-bwr-2160:        [INCOMPLETE][17] ([fdo#111174]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-bwr-2160/igt@i915_module_load@reload-no-display.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-bwr-2160/igt@i915_module_load@reload-no-display.html

  * igt@kms_chamelium@dp-edid-read:
    - {fi-icl-u4}:        [FAIL][19] ([fdo#111045] / [fdo#111046 ]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6586/fi-icl-u4/igt@kms_chamelium@dp-edid-read.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13814/fi-icl-u4/igt@kms_chamelium@dp-edid-read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111046 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111046 
  [fdo#111174]: https://bugs.freedesktop.org/show_bug.cgi?id=111174


Participating hosts (54 -> 45)
------------------------------

  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6586 -> Patchwork_13814

  CI-20190529: 20190529
  CI_DRM_6586: 066993443a56467f54fdcf560e89378f8e93a15b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5116: d2e6dd2f789596da5bd06efc2e9448e3160583b6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13814: 5e027c41eba76662f0e19c40bf7bf6a55e93f2a7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5e027c41eba7 drm/i915/gt: Introduce intel_gt_runtime_suspend/resume
3e17b702a176 drm/i915/uc: Move uC early functions inside the GT ones
03ea9d714bb8 drm/i915/gt: Move gt_cleanup_early out of gem_cleanup_early
f4ad32d03eb2 drm/i915/uc: Move uC WOPCM setup in uc_init_hw
86af9700f302 drm/i915/uc: Don't enable communication twice on resume

== Logs ==

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

Re: [PATCH v2 1/5] drm/i915/uc: Don't enable communication twice on resume

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-07-31 00:07:39)
> When coming out of S3/S4 we sanitize and re-init the HW, which includes
> enabling communication during uc_init_hw. We therefore don't want to do
> that again in uc_resume and can just tell GuC to reload its state.
> 
> v2: split uc_resume and uc_runtime_resume to match the suspend
>     functions and to better differentiate the expected state in the 2
>     scenarios (Chris)
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 35 +++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/gt/uc/intel_uc.h |  1 +
>  drivers/gpu/drm/i915/i915_drv.c       |  4 +--
>  3 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 6eb8bb3fa252..657fdcb70d00 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -233,11 +233,20 @@ static void guc_disable_interrupts(struct intel_guc *guc)
>         guc->interrupts.disable(guc);
>  }
>  
> +#ifdef CONFIG_DRM_I915_DEBUG_GEM
> +static bool guc_communication_enabled(struct intel_guc *guc)
> +{
> +       return guc->send != intel_guc_send_nop;
> +}
> +#endif
> +
>  static int guc_enable_communication(struct intel_guc *guc)
>  {
>         struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>         int ret;
>  
> +       GEM_BUG_ON(guc_communication_enabled(guc));
> +
>         ret = intel_guc_ct_enable(&guc->ct);
>         if (ret)
>                 return ret;
> @@ -550,7 +559,7 @@ void intel_uc_suspend(struct intel_uc *uc)
>                 intel_uc_runtime_suspend(uc);
>  }
>  
> -int intel_uc_resume(struct intel_uc *uc)
> +static int __uc_resume(struct intel_uc *uc, bool enable_communication)
>  {
>         struct intel_guc *guc = &uc->guc;
>         int err;
> @@ -558,7 +567,11 @@ int intel_uc_resume(struct intel_uc *uc)
>         if (!intel_guc_is_running(guc))
>                 return 0;
>  
> -       guc_enable_communication(guc);
> +       /* Make sure we enable communication if and only if it's disabled */
> +       GEM_BUG_ON(enable_communication == guc_communication_enabled(guc));
> +
> +       if (enable_communication)
> +               guc_enable_communication(guc);
>  
>         err = intel_guc_resume(guc);
>         if (err) {
> @@ -568,3 +581,21 @@ int intel_uc_resume(struct intel_uc *uc)
>  
>         return 0;
>  }
> +
> +int intel_uc_resume(struct intel_uc *uc)
> +{
> +       /*
> +        * When coming out of S3/S4 we sanitize and re-init the HW, so
> +        * communication is already re-enabled at this point.
> +        */
> +       return __uc_resume(uc, false);
> +}
> +
> +int intel_uc_runtime_resume(struct intel_uc *uc)
> +{
> +       /*
> +        * During runtime resume we don't sanitize, so we need to re-init
> +        * communication as well.
> +        */
> +       return __uc_resume(uc, true);
> +}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> index fe3362fd7706..25da51e95417 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> @@ -47,6 +47,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc);
>  void intel_uc_suspend(struct intel_uc *uc);
>  void intel_uc_runtime_suspend(struct intel_uc *uc);
>  int intel_uc_resume(struct intel_uc *uc);
> +int intel_uc_runtime_resume(struct intel_uc *uc);
>  
>  static inline bool intel_uc_is_using_guc(struct intel_uc *uc)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f2d3d754af37..761726818a22 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2950,7 +2950,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  
>                 intel_runtime_pm_enable_interrupts(dev_priv);
>  
> -               intel_uc_resume(&dev_priv->gt.uc);
> +               intel_uc_runtime_resume(&dev_priv->gt.uc);
>  
>                 intel_gt_init_swizzling(&dev_priv->gt);
>                 i915_gem_restore_fences(dev_priv);
> @@ -3047,7 +3047,7 @@ static int intel_runtime_resume(struct device *kdev)
>  
>         intel_runtime_pm_enable_interrupts(dev_priv);
>  
> -       intel_uc_resume(&dev_priv->gt.uc);
> +       intel_uc_runtime_resume(&dev_priv->gt.uc);

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

Re: [PATCH v2 2/5] drm/i915/uc: Move uC WOPCM setup in uc_init_hw

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-07-31 00:07:40)
> The register we write are not WOPCM regs but uC ones related to how
> GuC and HuC are going to use the WOPCM, so it makes logical sense
> for them to be programmed as part of uc_init_hw. The WOPCM map on the
> other side is not uC-specific (although that is our main use-case), so
> keep that separate.
> 
> v2: move write_and_verify to uncore, fix log, re-use err_out tag,
>     add intel_wopcm_guc_base, fix log
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 47 ++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h       |  3 +-
>  drivers/gpu/drm/i915/i915_gem.c       |  8 +---
>  drivers/gpu/drm/i915/intel_uncore.h   | 12 +++++
>  drivers/gpu/drm/i915/intel_wopcm.c    | 68 ---------------------------
>  drivers/gpu/drm/i915/intel_wopcm.h    | 18 +++++--
>  6 files changed, 76 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 657fdcb70d00..7794a6a1f932 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -399,6 +399,49 @@ void intel_uc_sanitize(struct intel_uc *uc)
>         __uc_sanitize(uc);
>  }
>  
> +/* Initialize and verify the uC regs related to uC positioning in WOPCM */
> +static int uc_init_wopcm(struct intel_uc *uc)
> +{
> +       struct intel_gt *gt = uc_to_gt(uc);
> +       struct intel_uncore *uncore = gt->uncore;
> +       u32 base = intel_wopcm_guc_base(&gt->i915->wopcm);
> +       u32 size = intel_wopcm_guc_size(&gt->i915->wopcm);
> +       u32 huc_agent = intel_uc_is_using_huc(uc) ? HUC_LOADING_AGENT_GUC : 0;
> +       u32 mask;
> +       int err;
> +
> +       GEM_BUG_ON(!intel_uc_is_using_guc(uc));
> +       GEM_BUG_ON(!(base & GUC_WOPCM_OFFSET_MASK));
> +       GEM_BUG_ON(base & ~GUC_WOPCM_OFFSET_MASK);
> +       GEM_BUG_ON(!(size & GUC_WOPCM_SIZE_MASK));
> +       GEM_BUG_ON(size & ~GUC_WOPCM_SIZE_MASK);

Worth a GEM_BUG_ON(base + size < base) ?

> +
> +       mask = GUC_WOPCM_SIZE_MASK | GUC_WOPCM_SIZE_LOCKED;
> +       err = intel_uncore_write_and_verify(uncore, GUC_WOPCM_SIZE, size, mask,
> +                                           size | GUC_WOPCM_SIZE_LOCKED);
> +       if (err)
> +               goto err_out;
> +
> +       mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID | huc_agent;
> +       err = intel_uncore_write_and_verify(uncore, DMA_GUC_WOPCM_OFFSET,
> +                                           base | huc_agent, mask,
> +                                           base | huc_agent |
> +                                           GUC_WOPCM_OFFSET_VALID);
> +       if (err)
> +               goto err_out;
> +
> +       return 0;
> +
> +err_out:
> +       DRM_ERROR("Failed to init uC WOPCM registers:\n");
> +       DRM_ERROR("DMA_GUC_WOPCM_OFFSET=%#x\n",
> +                 intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
> +       DRM_ERROR("GUC_WOPCM_SIZE=%#x\n",
> +                 intel_uncore_read(uncore, GUC_WOPCM_SIZE));
> +
> +       return err;
> +}

Fair enough,
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

Re: [PATCH v2 3/5] drm/i915/gt: Move gt_cleanup_early out of gem_cleanup_early

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-07-31 00:07:41)
> We don't call the init_early function from within the gem code, so we
> shouldn't do it for the cleanup either.
> 
> v2: while at it, s/gt_cleanup_early/gt_late_release (Chris)

The pattern is intel_gt_driver_late_release. The distinction may become
important later for unbinding.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/5] drm/i915/uc: Don't enable communication twice on resume

[Michal Wajdeczko]

On Wed, 31 Jul 2019 01:07:39 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> When coming out of S3/S4 we sanitize and re-init the HW, which includes
> enabling communication during uc_init_hw. We therefore don't want to do
> that again in uc_resume and can just tell GuC to reload its state.
>
> v2: split uc_resume and uc_runtime_resume to match the suspend
>     functions and to better differentiate the expected state in the 2
>     scenarios (Chris)
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915/uc: Move uC WOPCM setup in uc_init_hw

[Michal Wajdeczko]

On Wed, 31 Jul 2019 01:07:40 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> The register we write are not WOPCM regs but uC ones related to how
> GuC and HuC are going to use the WOPCM, so it makes logical sense
> for them to be programmed as part of uc_init_hw. The WOPCM map on the
> other side is not uC-specific (although that is our main use-case), so
> keep that separate.
>
> v2: move write_and_verify to uncore, fix log, re-use err_out tag,
>     add intel_wopcm_guc_base, fix log
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---

> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -393,6 +393,18 @@ static inline void intel_uncore_rmw_fw(struct  
> intel_uncore *uncore,
>  	intel_uncore_write_fw(uncore, reg, val);
>  }
> +static inline int intel_uncore_write_and_verify(struct intel_uncore  
> *uncore,
> +						i915_reg_t reg, u32 val,
> +						u32 mask, u32 expected_val)
> +{
> +	u32 reg_val;
> +
> +	intel_uncore_write(uncore, reg, val);
> +	reg_val = intel_uncore_read(uncore, reg);
> +
> +	return (reg_val & mask) != expected_val ? -EINVAL : 0;
> +}

nit: I'm not sure that -EINVAL is the best choice (not sure about
-ENODATA or -ENOKEY either) that's why I wanted to use bool ;)

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915/uc: Move uC WOPCM setup in uc_init_hw

[Chris Wilson]

Quoting Michal Wajdeczko (2019-07-31 10:00:57)
> On Wed, 31 Jul 2019 01:07:40 +0200, Daniele Ceraolo Spurio  
> <daniele.ceraolospurio@intel.com> wrote:
> 
> > The register we write are not WOPCM regs but uC ones related to how
> > GuC and HuC are going to use the WOPCM, so it makes logical sense
> > for them to be programmed as part of uc_init_hw. The WOPCM map on the
> > other side is not uC-specific (although that is our main use-case), so
> > keep that separate.
> >
> > v2: move write_and_verify to uncore, fix log, re-use err_out tag,
> >     add intel_wopcm_guc_base, fix log
> >
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> 
> > --- a/drivers/gpu/drm/i915/intel_uncore.h
> > +++ b/drivers/gpu/drm/i915/intel_uncore.h
> > @@ -393,6 +393,18 @@ static inline void intel_uncore_rmw_fw(struct  
> > intel_uncore *uncore,
> >       intel_uncore_write_fw(uncore, reg, val);
> >  }
> > +static inline int intel_uncore_write_and_verify(struct intel_uncore  
> > *uncore,
> > +                                             i915_reg_t reg, u32 val,
> > +                                             u32 mask, u32 expected_val)
> > +{
> > +     u32 reg_val;
> > +
> > +     intel_uncore_write(uncore, reg, val);
> > +     reg_val = intel_uncore_read(uncore, reg);
> > +
> > +     return (reg_val & mask) != expected_val ? -EINVAL : 0;
> > +}
> 
> nit: I'm not sure that -EINVAL is the best choice (not sure about
> -ENODATA or -ENOKEY either) that's why I wanted to use bool ;)

ENXIO? It's a bridge we can cross later.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915/uc: Move uC WOPCM setup in uc_init_hw

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-07-31 00:07:40)
> The register we write are not WOPCM regs but uC ones related to how
> GuC and HuC are going to use the WOPCM, so it makes logical sense
> for them to be programmed as part of uc_init_hw. The WOPCM map on the
> other side is not uC-specific (although that is our main use-case), so
> keep that separate.
> 
> v2: move write_and_verify to uncore, fix log, re-use err_out tag,
>     add intel_wopcm_guc_base, fix log
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Pushed the first 2. Just the usual naming debate as a hold up for the
completion.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx