intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: implemented dynamic WOPCM partition.
@ 2017-11-04  0:01 Jackie Li
  2017-11-04  0:01 ` [PATCH 2/2] HAX enable guc submission for CI Jackie Li
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Jackie Li @ 2017-11-04  0:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sujaritha Sundaresan

Static WOPCM partitioning would lead to GuC loading failure
if the GuC/HuC firmware size exceeded current static 512KB
partition boundary.

This patch enabled the dynamical calculation of the WOPCM aperture
used by GuC/HuC firmware. GuC WOPCM offset was set to
HuC size + reserved WOPCM size. GuC WOPCM size was set to
total WOPCM size - GuC WOPCM offset - RC6CTX size.

Signed-off-by: Jackie Li <yaodong.li@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: John Spotswood <john.a.spotswood@intel.com>
Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  15 ++++
 drivers/gpu/drm/i915/i915_drv.h         |  13 ++++
 drivers/gpu/drm/i915/i915_gem_context.c |   4 +-
 drivers/gpu/drm/i915/i915_guc_reg.h     |  18 ++++-
 drivers/gpu/drm/i915/intel_guc.c        |  46 ++++++++++--
 drivers/gpu/drm/i915/intel_guc.h        |  18 +----
 drivers/gpu/drm/i915/intel_huc.c        |   3 +-
 drivers/gpu/drm/i915/intel_uc.c         | 128 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_uc_fw.c      |  12 ++-
 9 files changed, 223 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e7e9e06..19509fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -623,6 +623,15 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv)
 	WARN_ON(!list_empty(&dev_priv->contexts.list));
 }
 
+static void i915_wopcm_init(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_info *wopcm = &dev_priv->wopcm;
+
+	wopcm->size = WOPCM_DEFAULT_SIZE;
+
+	DRM_DEBUG_DRIVER("WOPCM size: %dKB\n", wopcm->size >> 10);
+}
+
 static int i915_load_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -670,6 +679,12 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_irq;
 
+	/*
+	 * Get the wopcm memory info.
+	 * NOTE: this need to be called before init FW.
+	 */
+	i915_wopcm_init(dev_priv);
+
 	intel_uc_init_fw(dev_priv);
 
 	ret = i915_gem_init(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72bb5b5..61cd290 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2235,6 +2235,16 @@ struct intel_cdclk_state {
 	u8 voltage_level;
 };
 
+struct intel_wopcm_info {
+	u32 size;
+};
+
+struct intel_wopcm_partition {
+	u32 guc_wopcm_offset;
+	u32 guc_wopcm_size;
+	u32 guc_wopcm_top;
+};
+
 struct drm_i915_private {
 	struct drm_device drm;
 
@@ -2258,6 +2268,9 @@ struct drm_i915_private {
 	struct intel_huc huc;
 	struct intel_guc guc;
 
+	struct intel_wopcm_info wopcm;
+	struct intel_wopcm_partition wopcm_partition;
+
 	struct intel_csr csr;
 
 	struct intel_gmbus gmbus[GMBUS_NUM_PINS];
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 10affb3..7347fd7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -312,12 +312,12 @@ __create_hw_context(struct drm_i915_private *dev_priv,
 	ctx->desc_template =
 		default_desc_template(dev_priv, dev_priv->mm.aliasing_ppgtt);
 
-	/* GuC requires the ring to be placed above GUC_WOPCM_TOP. If GuC is not
+	/* GuC requires the ring to be placed above guc wopcm top. If GuC is not
 	 * present or not in use we still need a small bias as ring wraparound
 	 * at offset 0 sometimes hangs. No idea why.
 	 */
 	if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading)
-		ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
+		ctx->ggtt_offset_bias = intel_guc_wopcm_top(dev_priv);
 	else
 		ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
 
diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h
index 35cf991..d309884 100644
--- a/drivers/gpu/drm/i915/i915_guc_reg.h
+++ b/drivers/gpu/drm/i915/i915_guc_reg.h
@@ -67,17 +67,27 @@
 #define DMA_GUC_WOPCM_OFFSET		_MMIO(0xc340)
 #define   HUC_LOADING_AGENT_VCR		  (0<<1)
 #define   HUC_LOADING_AGENT_GUC		  (1<<1)
-#define   GUC_WOPCM_OFFSET_VALUE	  0x80000	/* 512KB */
 #define GUC_MAX_IDLE_COUNT		_MMIO(0xC3E4)
 
 #define HUC_STATUS2             _MMIO(0xD3B0)
 #define   HUC_FW_VERIFIED       (1<<7)
 
 /* Defines WOPCM space available to GuC firmware */
+/* default WOPCM size 1MB */
+#define WOPCM_DEFAULT_SIZE		(0x1 << 20)
+/* reserved WOPCM size 16KB */
+#define WOPCM_RESERVED_SIZE		(0x4000)
+/* GUC WOPCM Offset need to be 16KB aligned */
+#define WOPCM_OFFSET_ALIGNMENT		(0x4000)
+/* 8KB stack reserved for GuC FW*/
+#define GUC_WOPCM_STACK_RESERVED	(0x2000)
+/* 24KB WOPCM reserved for RC6 CTX on BXT */
+#define BXT_WOPCM_RC6_RESERVED		(0x6000)
+
+#define GEN9_GUC_WOPCM_DELTA		4
+#define GEN9_GUC_WOPCM_OFFSET		(0x24000)
+
 #define GUC_WOPCM_SIZE			_MMIO(0xc050)
-/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
-#define   GUC_WOPCM_TOP			  (0x80 << 12)	/* 512KB */
-#define   BXT_GUC_WOPCM_RC6_RESERVED	  (0x10 << 12)	/* 64KB  */
 
 /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
 #define GUC_GGTT_TOP			0xFEE00000
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 9678630..0efcfb4 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -337,7 +337,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
  * This is a wrapper to create an object for use with the GuC. In order to
  * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
  * both some backing storage and a range inside the Global GTT. We must pin
- * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
+ * it in the GGTT somewhere other than than [0, guc wopcm_top) because that
  * range is reserved inside GuC.
  *
  * Return:	A i915_vma if successful, otherwise an ERR_PTR.
@@ -358,7 +358,8 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 		goto err;
 
 	ret = i915_vma_pin(vma, 0, PAGE_SIZE,
-			   PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+			   PIN_GLOBAL | PIN_OFFSET_BIAS |
+			   intel_guc_wopcm_top(dev_priv));
 	if (ret) {
 		vma = ERR_PTR(ret);
 		goto err;
@@ -373,11 +374,42 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
 {
-	u32 wopcm_size = GUC_WOPCM_TOP;
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
 
-	/* On BXT, the top of WOPCM is reserved for RC6 context */
-	if (IS_GEN9_LP(dev_priv))
-		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+	GEM_BUG_ON(!wp->guc_wopcm_size);
 
-	return wopcm_size;
+	return wp->guc_wopcm_size;
+}
+
+u32 intel_guc_wopcm_top(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+
+	GEM_BUG_ON(!dev_priv->wopcm.size);
+
+	return wp->guc_wopcm_top ? wp->guc_wopcm_top : dev_priv->wopcm.size;
+}
+
+u32 intel_guc_wopcm_offset(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+
+	GEM_BUG_ON(!wp->guc_wopcm_size);
+
+	return wp->guc_wopcm_offset;
+}
+
+/*
+ * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
+ * which is reserved for Boot ROM, SRAM and WOPCM. All gfx objects
+ * used by GuC is pinned with PIN_OFFSET_BIAS along with top of WOPCM.
+ */
+u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+	struct drm_i915_private *dev_priv = vma->vm->i915;
+	u32 offset = i915_ggtt_offset(vma);
+
+	GEM_BUG_ON(offset < intel_guc_wopcm_top(dev_priv));
+	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
+	return offset;
 }
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 607e025..1493de0 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -100,21 +100,6 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 	guc->notify(guc);
 }
 
-/*
- * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
- * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
- * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
- * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
- */
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
-{
-	u32 offset = i915_ggtt_offset(vma);
-
-	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
-	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
-
-	return offset;
-}
 
 void intel_guc_init_early(struct intel_guc *guc);
 void intel_guc_init_send_regs(struct intel_guc *guc);
@@ -127,5 +112,8 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv);
 int intel_guc_resume(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
+u32 intel_guc_wopcm_top(struct drm_i915_private *dev_priv);
+u32 intel_guc_wopcm_offset(struct drm_i915_private *dev_priv);
+u32 guc_ggtt_offset(struct i915_vma *vma);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 98d1725..a985aa5 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -202,7 +202,8 @@ void intel_huc_auth(struct intel_huc *huc)
 		return;
 
 	vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+				PIN_OFFSET_BIAS |
+				intel_guc_wopcm_top(i915));
 	if (IS_ERR(vma)) {
 		DRM_ERROR("failed to pin huc fw object %d\n",
 				(int)PTR_ERR(vma));
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index aec2954..83b2516 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -86,6 +86,125 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 	intel_guc_init_early(&dev_priv->guc);
 }
 
+static u32 rc6_context_size(struct drm_i915_private *dev_priv)
+{
+	/* On BXT, the top of WOPCM is reserved for RC6 context */
+	if (IS_GEN9_LP(dev_priv))
+		return BXT_WOPCM_RC6_RESERVED;
+
+	return 0;
+}
+
+static int do_wopcm_partition(struct drm_i915_private *dev_priv,
+	u32 offset, u32 reserved_size)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+	u32 aligned_offset = ALIGN(offset, WOPCM_OFFSET_ALIGNMENT);
+
+	if (offset >= dev_priv->wopcm.size)
+		return -E2BIG;
+
+	if (reserved_size >= dev_priv->wopcm.size)
+		return -E2BIG;
+
+	if ((aligned_offset + reserved_size) >= dev_priv->wopcm.size)
+		return -E2BIG;
+
+	wp->guc_wopcm_offset = aligned_offset;
+	wp->guc_wopcm_top = dev_priv->wopcm.size - wp->guc_wopcm_offset;
+	wp->guc_wopcm_size = wp->guc_wopcm_top - reserved_size;
+
+	return 0;
+}
+
+static int intel_uc_init_wopcm_partition(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
+	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
+	size_t huc_size, guc_size;
+	u32 offset;
+	u32 reserved;
+	u32 wopcm_base;
+	u32 delta;
+	int err;
+
+	/*Return if WOPCM partition has been initialized*/
+	if (wp->guc_wopcm_size)
+		return 0;
+
+	GEM_BUG_ON(!dev_priv->wopcm.size);
+
+	/*No need to do partition if failed to fetch both GuC and HuC FW*/
+	if (guc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS &&
+		huc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return -EIO;
+
+	huc_size = 0;
+	guc_size = 0;
+	offset = WOPCM_RESERVED_SIZE;
+	reserved = rc6_context_size(dev_priv);
+
+	if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS)
+		huc_size = huc_fw->header_size + huc_fw->ucode_size;
+
+	err = do_wopcm_partition(dev_priv, offset + huc_size, reserved);
+	if (err) {
+		if (!huc_size)
+			goto pt_done;
+
+		/*partition failed with HuC FW, block HuC loading*/
+		huc_size = 0;
+
+		/*partition without loading HuC FW*/
+		err = do_wopcm_partition(dev_priv, offset, reserved);
+		if (err)
+			goto pt_done;
+	}
+
+	/*
+	 * Check hardware restriction on Gen9
+	 * GuC WOPCM size is at least 4 bytes larger than GuC WOPCM base due
+	 * to hardware limitation on Gen9.
+	 */
+	if (IS_GEN9(dev_priv)) {
+		wopcm_base = wp->guc_wopcm_offset + GEN9_GUC_WOPCM_OFFSET;
+		if (unlikely(wopcm_base > wp->guc_wopcm_size))
+			goto pt_done;
+
+		delta = wp->guc_wopcm_size - wopcm_base;
+		if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
+			goto pt_done;
+	}
+
+	if (guc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS) {
+		guc_size = guc_fw->header_size + guc_fw->ucode_size;
+		/*need 8K stack for GuC*/
+		guc_size += GUC_WOPCM_STACK_RESERVED;
+	}
+
+	if (guc_size > wp->guc_wopcm_size)
+		guc_size = 0;
+
+pt_done:
+	if (!huc_size) {
+		DRM_ERROR("HuC firmware is too large to fit in WOPCM\n");
+		intel_uc_fw_fini(huc_fw);
+	}
+
+	if (!guc_size) {
+		DRM_ERROR("GuC firmware is too large to fit in WOPCM\n");
+		intel_uc_fw_fini(guc_fw);
+	}
+
+	DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n",
+		wp->guc_wopcm_offset >> 10,
+		wp->guc_wopcm_size >> 10,
+		wp->guc_wopcm_top >> 10);
+
+	return err;
+}
+
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 {
 	intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
@@ -157,6 +276,11 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (!i915_modparams.enable_guc_loading)
 		return 0;
 
+	/*init WOPCM partition*/
+	ret = intel_uc_init_wopcm_partition(dev_priv);
+	if (ret)
+		goto err_wopcm;
+
 	guc_disable_communication(guc);
 	gen9_reset_guc_interrupts(dev_priv);
 
@@ -176,7 +300,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	/* init WOPCM */
 	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
 	I915_WRITE(DMA_GUC_WOPCM_OFFSET,
-		   GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
+		intel_guc_wopcm_offset(dev_priv) | HUC_LOADING_AGENT_GUC);
 
 	/* WaEnableuKernelHeaderValidFix:skl */
 	/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
@@ -249,7 +373,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 		i915_guc_submission_fini(dev_priv);
 err_guc:
 	i915_ggtt_disable_guc(dev_priv);
-
+err_wopcm:
 	if (i915_modparams.enable_guc_loading > 1 ||
 	    i915_modparams.enable_guc_submission > 1) {
 		DRM_ERROR("GuC init failed. Firmware loading disabled.\n");
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 973888e..aefba13 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -95,9 +95,13 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
 	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
 
-	/* Header and uCode will be loaded to WOPCM */
+	/*
+	 * Header and uCode will be loaded to WOPCM
+	 * Only check the size against the overall available WOPCM here. Will
+	 * continue to check the size during WOPCM partition calculation.
+	 */
 	size = uc_fw->header_size + uc_fw->ucode_size;
-	if (size > intel_guc_wopcm_size(dev_priv)) {
+	if (size > dev_priv->wopcm.size) {
 		DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
 			 intel_uc_fw_type_repr(uc_fw->type));
 		err = -E2BIG;
@@ -207,6 +211,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 		       int (*xfer)(struct intel_uc_fw *uc_fw,
 				   struct i915_vma *vma))
 {
+	struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
 	struct i915_vma *vma;
 	int err;
 
@@ -230,7 +235,8 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 	}
 
 	vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
-				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+				       PIN_OFFSET_BIAS |
+				       intel_guc_wopcm_top(i915));
 	if (IS_ERR(vma)) {
 		err = PTR_ERR(vma);
 		DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH v2 1/2] drm/i915/guc: Fill preempt context once at init time
@ 2018-02-26 16:37 Michał Winiarski
  2018-02-26 16:38 ` [PATCH 2/2] HAX: Enable GuC submission for CI Michał Winiarski
  0 siblings, 1 reply; 13+ messages in thread
From: Michał Winiarski @ 2018-02-26 16:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

Since we're inhibiting context save of preempt context, we're no longer
tracking the position of HEAD/TAIL. With GuC, we're adding a new
breadcrumb for each preemption, which means that the HW will do more and
more breadcrumb writes. Eventually the ring is filled, and we're
submitting the preemption context with HEAD==TAIL==0, which won't result
in breadcrumb write, but will trigger hangcheck instead.
Instead of writing a new preempt breadcrumb for each preemption, let's
just fill the ring once at init time (which also saves a couple of
instructions in the tasklet).

v2: Assert that context save restore is inhibited, don't assert on ring
    alignment. (Chris)

Fixes: 517aaffe0c1b ("drm/i915/execlists: Inhibit context save/restore for the fake preempt context")
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 86 ++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 586dde579903..3805839f567b 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -26,8 +26,13 @@
 #include <trace/events/dma_fence.h>
 
 #include "intel_guc_submission.h"
+#include "intel_lrc_reg.h"
 #include "i915_drv.h"
 
+#define GUC_PREEMPT_FINISHED		0x1
+#define GUC_PREEMPT_BREADCRUMB_DWORDS	0x8
+#define GUC_PREEMPT_BREADCRUMB_BYTES	(sizeof(u32) * GUC_PREEMPT_BREADCRUMB_DWORDS)
+
 /**
  * DOC: GuC-based command submission
  *
@@ -535,8 +540,6 @@ static void flush_ggtt_writes(struct i915_vma *vma)
 		POSTING_READ_FW(GUC_STATUS);
 }
 
-#define GUC_PREEMPT_FINISHED 0x1
-#define GUC_PREEMPT_BREADCRUMB_DWORDS 0x8
 static void inject_preempt_context(struct work_struct *work)
 {
 	struct guc_preempt_work *preempt_work =
@@ -546,37 +549,17 @@ static void inject_preempt_context(struct work_struct *work)
 					     preempt_work[engine->id]);
 	struct intel_guc_client *client = guc->preempt_client;
 	struct guc_stage_desc *stage_desc = __get_stage_desc(client);
-	struct intel_ring *ring = client->owner->engine[engine->id].ring;
 	u32 ctx_desc = lower_32_bits(intel_lr_context_descriptor(client->owner,
 								 engine));
-	u32 *cs = ring->vaddr + ring->tail;
 	u32 data[7];
 
-	if (engine->id == RCS) {
-		cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
-				intel_hws_preempt_done_address(engine));
-	} else {
-		cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
-				intel_hws_preempt_done_address(engine));
-		*cs++ = MI_NOOP;
-		*cs++ = MI_NOOP;
-	}
-	*cs++ = MI_USER_INTERRUPT;
-	*cs++ = MI_NOOP;
-
-	GEM_BUG_ON(!IS_ALIGNED(ring->size,
-			       GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32)));
-	GEM_BUG_ON((void *)cs - (ring->vaddr + ring->tail) !=
-		   GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32));
-
-	ring->tail += GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32);
-	ring->tail &= (ring->size - 1);
-
-	flush_ggtt_writes(ring->vma);
-
+	/*
+	 * The ring should containt commands writing GUC_PREEMPT_FINISHED to
+	 * HWSP at client initialization time.
+	 */
 	spin_lock_irq(&client->wq_lock);
 	guc_wq_item_append(client, engine->guc_id, ctx_desc,
-			   ring->tail / sizeof(u64), 0);
+			   GUC_PREEMPT_BREADCRUMB_BYTES / sizeof(u64), 0);
 	spin_unlock_irq(&client->wq_lock);
 
 	/*
@@ -972,6 +955,53 @@ static void guc_client_free(struct intel_guc_client *client)
 	kfree(client);
 }
 
+static void guc_fill_preempt_context(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct intel_guc_client *client = guc->preempt_client;
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+	u32 *cs;
+
+	for_each_engine(engine, dev_priv, id) {
+		struct intel_context *ce = &client->owner->engine[id];
+
+		GEM_BUG_ON(!ce->pin_count);
+
+		/*
+		 * We rely on this context image *not* being saved after
+		 * preemption. This ensures that the RING_HEAD / RING_TAIL
+		 * remaing pointing at initial values forever.
+		 */
+		GEM_BUG_ON((ce->lrc_reg_state[CTX_CONTEXT_CONTROL + 1] &
+			    _MASKED_BIT_ENABLE(
+				CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
+				CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)) !=
+			   _MASKED_BIT_ENABLE(
+				CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
+				CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT));
+
+		cs = ce->ring->vaddr;
+
+		if (id == RCS) {
+			cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
+					intel_hws_preempt_done_address(engine));
+		} else {
+			cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
+					intel_hws_preempt_done_address(engine));
+			*cs++ = MI_NOOP;
+			*cs++ = MI_NOOP;
+		}
+		*cs++ = MI_USER_INTERRUPT;
+		*cs++ = MI_NOOP;
+
+		GEM_BUG_ON((void *)cs - ce->ring->vaddr !=
+			   GUC_PREEMPT_BREADCRUMB_BYTES);
+
+		flush_ggtt_writes(ce->ring->vma);
+	}
+}
+
 static int guc_clients_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -1002,6 +1032,8 @@ static int guc_clients_create(struct intel_guc *guc)
 			return PTR_ERR(client);
 		}
 		guc->preempt_client = client;
+
+		guc_fill_preempt_context(guc);
 	}
 
 	return 0;
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time
@ 2018-02-26 13:59 Michał Winiarski
  2018-02-26 14:00 ` [PATCH 2/2] HAX: Enable GuC submission for CI Michał Winiarski
  0 siblings, 1 reply; 13+ messages in thread
From: Michał Winiarski @ 2018-02-26 13:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

Since we're inhibiting context save of preempt context, we're no longer
tracking the position of HEAD/TAIL. With GuC, we're adding a new
breadcrumb for each preemption, which means that the HW will do more and
more breadcrumb writes. Eventually the ring is filled, and we're
submitting the preemption context with HEAD==TAIL==0, which won't result
in breadcrumb write, but will trigger hangcheck instead.
Instead of writing a new preempt breadcrumb for each preemption, let's
just fill the ring once at init time (which also saves a couple of
instructions in the tasklet).

Fixes: 517aaffe0c1b ("drm/i915/execlists: Inhibit context save/restore for the fake preempt context")
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 68 +++++++++++++++++------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 586dde579903..89e5b036061d 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -28,6 +28,10 @@
 #include "intel_guc_submission.h"
 #include "i915_drv.h"
 
+#define GUC_PREEMPT_FINISHED		0x1
+#define GUC_PREEMPT_BREADCRUMB_DWORDS	0x8
+#define GUC_PREEMPT_BREADCRUMB_BYTES	(sizeof(u32) * GUC_PREEMPT_BREADCRUMB_DWORDS)
+
 /**
  * DOC: GuC-based command submission
  *
@@ -535,8 +539,6 @@ static void flush_ggtt_writes(struct i915_vma *vma)
 		POSTING_READ_FW(GUC_STATUS);
 }
 
-#define GUC_PREEMPT_FINISHED 0x1
-#define GUC_PREEMPT_BREADCRUMB_DWORDS 0x8
 static void inject_preempt_context(struct work_struct *work)
 {
 	struct guc_preempt_work *preempt_work =
@@ -546,37 +548,13 @@ static void inject_preempt_context(struct work_struct *work)
 					     preempt_work[engine->id]);
 	struct intel_guc_client *client = guc->preempt_client;
 	struct guc_stage_desc *stage_desc = __get_stage_desc(client);
-	struct intel_ring *ring = client->owner->engine[engine->id].ring;
 	u32 ctx_desc = lower_32_bits(intel_lr_context_descriptor(client->owner,
 								 engine));
-	u32 *cs = ring->vaddr + ring->tail;
 	u32 data[7];
 
-	if (engine->id == RCS) {
-		cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
-				intel_hws_preempt_done_address(engine));
-	} else {
-		cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
-				intel_hws_preempt_done_address(engine));
-		*cs++ = MI_NOOP;
-		*cs++ = MI_NOOP;
-	}
-	*cs++ = MI_USER_INTERRUPT;
-	*cs++ = MI_NOOP;
-
-	GEM_BUG_ON(!IS_ALIGNED(ring->size,
-			       GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32)));
-	GEM_BUG_ON((void *)cs - (ring->vaddr + ring->tail) !=
-		   GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32));
-
-	ring->tail += GUC_PREEMPT_BREADCRUMB_DWORDS * sizeof(u32);
-	ring->tail &= (ring->size - 1);
-
-	flush_ggtt_writes(ring->vma);
-
 	spin_lock_irq(&client->wq_lock);
 	guc_wq_item_append(client, engine->guc_id, ctx_desc,
-			   ring->tail / sizeof(u64), 0);
+			   GUC_PREEMPT_BREADCRUMB_BYTES / sizeof(u64), 0);
 	spin_unlock_irq(&client->wq_lock);
 
 	/*
@@ -972,6 +950,40 @@ static void guc_client_free(struct intel_guc_client *client)
 	kfree(client);
 }
 
+static void guc_fill_preempt_context(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct intel_guc_client *client = guc->preempt_client;
+	struct intel_engine_cs *engine;
+	struct intel_ring *ring;
+	enum intel_engine_id id;
+	u32 *cs;
+
+	for_each_engine(engine, dev_priv, id) {
+		ring = client->owner->engine[id].ring;
+		cs = ring->vaddr;
+
+		if (id == RCS) {
+			cs = gen8_emit_ggtt_write_rcs(cs, GUC_PREEMPT_FINISHED,
+					intel_hws_preempt_done_address(engine));
+		} else {
+			cs = gen8_emit_ggtt_write(cs, GUC_PREEMPT_FINISHED,
+					intel_hws_preempt_done_address(engine));
+			*cs++ = MI_NOOP;
+			*cs++ = MI_NOOP;
+		}
+		*cs++ = MI_USER_INTERRUPT;
+		*cs++ = MI_NOOP;
+
+		GEM_BUG_ON(!IS_ALIGNED(ring->size,
+			   GUC_PREEMPT_BREADCRUMB_BYTES));
+		GEM_BUG_ON((void *)cs - ring->vaddr !=
+			   GUC_PREEMPT_BREADCRUMB_BYTES);
+
+		flush_ggtt_writes(ring->vma);
+	}
+}
+
 static int guc_clients_create(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -1002,6 +1014,8 @@ static int guc_clients_create(struct intel_guc *guc)
 			return PTR_ERR(client);
 		}
 		guc->preempt_client = client;
+
+		guc_fill_preempt_context(guc);
 	}
 
 	return 0;
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 1/2] drm/i915/guc: Advance over port[0] if set and not preempting
@ 2017-11-24 13:37 Chris Wilson
  2017-11-24 13:37 ` [PATCH 2/2] HAX Enable GuC Submission for CI Chris Wilson
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2017-11-24 13:37 UTC (permalink / raw)
  To: intel-gfx

Our execlist emulation is intended to only use a maximum of 2 ports per
engine, so as to not overflow the wq. (By knowing the limits, we can
avoid having to handle the wq exhaustion.) However, upon adding
preemption, we lost the skip over the first port if set for the
non-preemption path. Restore it.

Reported-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Fixes: c41937fd994a ("drm/i915/guc: Preemption! With GuC")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index cbf5a96f5806..70e64bdb73dd 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -743,23 +743,26 @@ static void guc_dequeue(struct intel_engine_cs *engine)
 	if (!rb)
 		goto unlock;
 
-	if (HAS_LOGICAL_RING_PREEMPTION(engine->i915) && port_isset(port)) {
-		struct guc_preempt_work *preempt_work =
-			&engine->i915->guc.preempt_work[engine->id];
-
-		if (rb_entry(rb, struct i915_priolist, node)->priority >
-		    max(port_request(port)->priotree.priority, 0)) {
-			execlists_set_active(execlists,
-					     EXECLISTS_ACTIVE_PREEMPT);
-			queue_work(engine->i915->guc.preempt_wq,
-				   &preempt_work->work);
-			goto unlock;
-		} else if (port_isset(last_port)) {
-			goto unlock;
+	if (port_isset(port)) {
+		if (HAS_LOGICAL_RING_PREEMPTION(engine->i915)) {
+			struct guc_preempt_work *preempt_work =
+				&engine->i915->guc.preempt_work[engine->id];
+
+			if (rb_entry(rb, struct i915_priolist, node)->priority >
+			    max(port_request(port)->priotree.priority, 0)) {
+				execlists_set_active(execlists,
+						     EXECLISTS_ACTIVE_PREEMPT);
+				queue_work(engine->i915->guc.preempt_wq,
+					   &preempt_work->work);
+				goto unlock;
+			}
 		}
 
 		port++;
+		if (port_isset(port))
+			goto unlock;
 	}
+	GEM_BUG_ON(port_isset(port));
 
 	do {
 		struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
-- 
2.15.0

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 1/2] drm/i915: implemented dynamic WOPCM partition.
@ 2017-11-04  0:18 Jackie Li
  2017-11-04  0:18 ` [PATCH 2/2] HAX enable guc submission for CI Jackie Li
  0 siblings, 1 reply; 13+ messages in thread
From: Jackie Li @ 2017-11-04  0:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: Sujaritha Sundaresan

Static WOPCM partitioning would lead to GuC loading failure
if the GuC/HuC firmware size exceeded current static 512KB
partition boundary.

This patch enabled the dynamical calculation of the WOPCM aperture
used by GuC/HuC firmware. GuC WOPCM offset was set to
HuC size + reserved WOPCM size. GuC WOPCM size was set to
total WOPCM size - GuC WOPCM offset - RC6CTX size.

Signed-off-by: Jackie Li <yaodong.li@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: John Spotswood <john.a.spotswood@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  15 ++++
 drivers/gpu/drm/i915/i915_drv.h         |  13 ++++
 drivers/gpu/drm/i915/i915_gem_context.c |   4 +-
 drivers/gpu/drm/i915/i915_guc_reg.h     |  18 ++++-
 drivers/gpu/drm/i915/intel_guc.c        |  46 ++++++++++--
 drivers/gpu/drm/i915/intel_guc.h        |  18 +----
 drivers/gpu/drm/i915/intel_huc.c        |   3 +-
 drivers/gpu/drm/i915/intel_uc.c         | 128 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_uc_fw.c      |  12 ++-
 9 files changed, 223 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e7e9e06..19509fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -623,6 +623,15 @@ static void i915_gem_fini(struct drm_i915_private *dev_priv)
 	WARN_ON(!list_empty(&dev_priv->contexts.list));
 }
 
+static void i915_wopcm_init(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_info *wopcm = &dev_priv->wopcm;
+
+	wopcm->size = WOPCM_DEFAULT_SIZE;
+
+	DRM_DEBUG_DRIVER("WOPCM size: %dKB\n", wopcm->size >> 10);
+}
+
 static int i915_load_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -670,6 +679,12 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_irq;
 
+	/*
+	 * Get the wopcm memory info.
+	 * NOTE: this need to be called before init FW.
+	 */
+	i915_wopcm_init(dev_priv);
+
 	intel_uc_init_fw(dev_priv);
 
 	ret = i915_gem_init(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72bb5b5..61cd290 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2235,6 +2235,16 @@ struct intel_cdclk_state {
 	u8 voltage_level;
 };
 
+struct intel_wopcm_info {
+	u32 size;
+};
+
+struct intel_wopcm_partition {
+	u32 guc_wopcm_offset;
+	u32 guc_wopcm_size;
+	u32 guc_wopcm_top;
+};
+
 struct drm_i915_private {
 	struct drm_device drm;
 
@@ -2258,6 +2268,9 @@ struct drm_i915_private {
 	struct intel_huc huc;
 	struct intel_guc guc;
 
+	struct intel_wopcm_info wopcm;
+	struct intel_wopcm_partition wopcm_partition;
+
 	struct intel_csr csr;
 
 	struct intel_gmbus gmbus[GMBUS_NUM_PINS];
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 10affb3..7347fd7 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -312,12 +312,12 @@ __create_hw_context(struct drm_i915_private *dev_priv,
 	ctx->desc_template =
 		default_desc_template(dev_priv, dev_priv->mm.aliasing_ppgtt);
 
-	/* GuC requires the ring to be placed above GUC_WOPCM_TOP. If GuC is not
+	/* GuC requires the ring to be placed above guc wopcm top. If GuC is not
 	 * present or not in use we still need a small bias as ring wraparound
 	 * at offset 0 sometimes hangs. No idea why.
 	 */
 	if (HAS_GUC(dev_priv) && i915_modparams.enable_guc_loading)
-		ctx->ggtt_offset_bias = GUC_WOPCM_TOP;
+		ctx->ggtt_offset_bias = intel_guc_wopcm_top(dev_priv);
 	else
 		ctx->ggtt_offset_bias = I915_GTT_PAGE_SIZE;
 
diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h
index 35cf991..d309884 100644
--- a/drivers/gpu/drm/i915/i915_guc_reg.h
+++ b/drivers/gpu/drm/i915/i915_guc_reg.h
@@ -67,17 +67,27 @@
 #define DMA_GUC_WOPCM_OFFSET		_MMIO(0xc340)
 #define   HUC_LOADING_AGENT_VCR		  (0<<1)
 #define   HUC_LOADING_AGENT_GUC		  (1<<1)
-#define   GUC_WOPCM_OFFSET_VALUE	  0x80000	/* 512KB */
 #define GUC_MAX_IDLE_COUNT		_MMIO(0xC3E4)
 
 #define HUC_STATUS2             _MMIO(0xD3B0)
 #define   HUC_FW_VERIFIED       (1<<7)
 
 /* Defines WOPCM space available to GuC firmware */
+/* default WOPCM size 1MB */
+#define WOPCM_DEFAULT_SIZE		(0x1 << 20)
+/* reserved WOPCM size 16KB */
+#define WOPCM_RESERVED_SIZE		(0x4000)
+/* GUC WOPCM Offset need to be 16KB aligned */
+#define WOPCM_OFFSET_ALIGNMENT		(0x4000)
+/* 8KB stack reserved for GuC FW*/
+#define GUC_WOPCM_STACK_RESERVED	(0x2000)
+/* 24KB WOPCM reserved for RC6 CTX on BXT */
+#define BXT_WOPCM_RC6_RESERVED		(0x6000)
+
+#define GEN9_GUC_WOPCM_DELTA		4
+#define GEN9_GUC_WOPCM_OFFSET		(0x24000)
+
 #define GUC_WOPCM_SIZE			_MMIO(0xc050)
-/* GuC addresses below GUC_WOPCM_TOP don't map through the GTT */
-#define   GUC_WOPCM_TOP			  (0x80 << 12)	/* 512KB */
-#define   BXT_GUC_WOPCM_RC6_RESERVED	  (0x10 << 12)	/* 64KB  */
 
 /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
 #define GUC_GGTT_TOP			0xFEE00000
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 9678630..0efcfb4 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -337,7 +337,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
  * This is a wrapper to create an object for use with the GuC. In order to
  * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
  * both some backing storage and a range inside the Global GTT. We must pin
- * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
+ * it in the GGTT somewhere other than than [0, guc wopcm_top) because that
  * range is reserved inside GuC.
  *
  * Return:	A i915_vma if successful, otherwise an ERR_PTR.
@@ -358,7 +358,8 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 		goto err;
 
 	ret = i915_vma_pin(vma, 0, PAGE_SIZE,
-			   PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+			   PIN_GLOBAL | PIN_OFFSET_BIAS |
+			   intel_guc_wopcm_top(dev_priv));
 	if (ret) {
 		vma = ERR_PTR(ret);
 		goto err;
@@ -373,11 +374,42 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
 {
-	u32 wopcm_size = GUC_WOPCM_TOP;
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
 
-	/* On BXT, the top of WOPCM is reserved for RC6 context */
-	if (IS_GEN9_LP(dev_priv))
-		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+	GEM_BUG_ON(!wp->guc_wopcm_size);
 
-	return wopcm_size;
+	return wp->guc_wopcm_size;
+}
+
+u32 intel_guc_wopcm_top(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+
+	GEM_BUG_ON(!dev_priv->wopcm.size);
+
+	return wp->guc_wopcm_top ? wp->guc_wopcm_top : dev_priv->wopcm.size;
+}
+
+u32 intel_guc_wopcm_offset(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+
+	GEM_BUG_ON(!wp->guc_wopcm_size);
+
+	return wp->guc_wopcm_offset;
+}
+
+/*
+ * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
+ * which is reserved for Boot ROM, SRAM and WOPCM. All gfx objects
+ * used by GuC is pinned with PIN_OFFSET_BIAS along with top of WOPCM.
+ */
+u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+	struct drm_i915_private *dev_priv = vma->vm->i915;
+	u32 offset = i915_ggtt_offset(vma);
+
+	GEM_BUG_ON(offset < intel_guc_wopcm_top(dev_priv));
+	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
+	return offset;
 }
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 607e025..1493de0 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -100,21 +100,6 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 	guc->notify(guc);
 }
 
-/*
- * GuC does not allow any gfx GGTT address that falls into range [0, WOPCM_TOP),
- * which is reserved for Boot ROM, SRAM and WOPCM. Currently this top address is
- * 512K. In order to exclude 0-512K address space from GGTT, all gfx objects
- * used by GuC is pinned with PIN_OFFSET_BIAS along with size of WOPCM.
- */
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
-{
-	u32 offset = i915_ggtt_offset(vma);
-
-	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
-	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
-
-	return offset;
-}
 
 void intel_guc_init_early(struct intel_guc *guc);
 void intel_guc_init_send_regs(struct intel_guc *guc);
@@ -127,5 +112,8 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv);
 int intel_guc_resume(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
+u32 intel_guc_wopcm_top(struct drm_i915_private *dev_priv);
+u32 intel_guc_wopcm_offset(struct drm_i915_private *dev_priv);
+u32 guc_ggtt_offset(struct i915_vma *vma);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 98d1725..a985aa5 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -202,7 +202,8 @@ void intel_huc_auth(struct intel_huc *huc)
 		return;
 
 	vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+				PIN_OFFSET_BIAS |
+				intel_guc_wopcm_top(i915));
 	if (IS_ERR(vma)) {
 		DRM_ERROR("failed to pin huc fw object %d\n",
 				(int)PTR_ERR(vma));
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index aec2954..83b2516 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -86,6 +86,125 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 	intel_guc_init_early(&dev_priv->guc);
 }
 
+static u32 rc6_context_size(struct drm_i915_private *dev_priv)
+{
+	/* On BXT, the top of WOPCM is reserved for RC6 context */
+	if (IS_GEN9_LP(dev_priv))
+		return BXT_WOPCM_RC6_RESERVED;
+
+	return 0;
+}
+
+static int do_wopcm_partition(struct drm_i915_private *dev_priv,
+	u32 offset, u32 reserved_size)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+	u32 aligned_offset = ALIGN(offset, WOPCM_OFFSET_ALIGNMENT);
+
+	if (offset >= dev_priv->wopcm.size)
+		return -E2BIG;
+
+	if (reserved_size >= dev_priv->wopcm.size)
+		return -E2BIG;
+
+	if ((aligned_offset + reserved_size) >= dev_priv->wopcm.size)
+		return -E2BIG;
+
+	wp->guc_wopcm_offset = aligned_offset;
+	wp->guc_wopcm_top = dev_priv->wopcm.size - wp->guc_wopcm_offset;
+	wp->guc_wopcm_size = wp->guc_wopcm_top - reserved_size;
+
+	return 0;
+}
+
+static int intel_uc_init_wopcm_partition(struct drm_i915_private *dev_priv)
+{
+	struct intel_wopcm_partition *wp = &dev_priv->wopcm_partition;
+	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
+	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
+	size_t huc_size, guc_size;
+	u32 offset;
+	u32 reserved;
+	u32 wopcm_base;
+	u32 delta;
+	int err;
+
+	/*Return if WOPCM partition has been initialized*/
+	if (wp->guc_wopcm_size)
+		return 0;
+
+	GEM_BUG_ON(!dev_priv->wopcm.size);
+
+	/*No need to do partition if failed to fetch both GuC and HuC FW*/
+	if (guc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS &&
+		huc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return -EIO;
+
+	huc_size = 0;
+	guc_size = 0;
+	offset = WOPCM_RESERVED_SIZE;
+	reserved = rc6_context_size(dev_priv);
+
+	if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS)
+		huc_size = huc_fw->header_size + huc_fw->ucode_size;
+
+	err = do_wopcm_partition(dev_priv, offset + huc_size, reserved);
+	if (err) {
+		if (!huc_size)
+			goto pt_done;
+
+		/*partition failed with HuC FW, block HuC loading*/
+		huc_size = 0;
+
+		/*partition without loading HuC FW*/
+		err = do_wopcm_partition(dev_priv, offset, reserved);
+		if (err)
+			goto pt_done;
+	}
+
+	/*
+	 * Check hardware restriction on Gen9
+	 * GuC WOPCM size is at least 4 bytes larger than GuC WOPCM base due
+	 * to hardware limitation on Gen9.
+	 */
+	if (IS_GEN9(dev_priv)) {
+		wopcm_base = wp->guc_wopcm_offset + GEN9_GUC_WOPCM_OFFSET;
+		if (unlikely(wopcm_base > wp->guc_wopcm_size))
+			goto pt_done;
+
+		delta = wp->guc_wopcm_size - wopcm_base;
+		if (unlikely(delta < GEN9_GUC_WOPCM_DELTA))
+			goto pt_done;
+	}
+
+	if (guc_fw->fetch_status == INTEL_UC_FIRMWARE_SUCCESS) {
+		guc_size = guc_fw->header_size + guc_fw->ucode_size;
+		/*need 8K stack for GuC*/
+		guc_size += GUC_WOPCM_STACK_RESERVED;
+	}
+
+	if (guc_size > wp->guc_wopcm_size)
+		guc_size = 0;
+
+pt_done:
+	if (!huc_size) {
+		DRM_ERROR("HuC firmware is too large to fit in WOPCM\n");
+		intel_uc_fw_fini(huc_fw);
+	}
+
+	if (!guc_size) {
+		DRM_ERROR("GuC firmware is too large to fit in WOPCM\n");
+		intel_uc_fw_fini(guc_fw);
+	}
+
+	DRM_DEBUG_DRIVER("GuC WOPCM offset %dKB, size %dKB, top %dKB\n",
+		wp->guc_wopcm_offset >> 10,
+		wp->guc_wopcm_size >> 10,
+		wp->guc_wopcm_top >> 10);
+
+	return err;
+}
+
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 {
 	intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw);
@@ -157,6 +276,11 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (!i915_modparams.enable_guc_loading)
 		return 0;
 
+	/*init WOPCM partition*/
+	ret = intel_uc_init_wopcm_partition(dev_priv);
+	if (ret)
+		goto err_wopcm;
+
 	guc_disable_communication(guc);
 	gen9_reset_guc_interrupts(dev_priv);
 
@@ -176,7 +300,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	/* init WOPCM */
 	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
 	I915_WRITE(DMA_GUC_WOPCM_OFFSET,
-		   GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
+		intel_guc_wopcm_offset(dev_priv) | HUC_LOADING_AGENT_GUC);
 
 	/* WaEnableuKernelHeaderValidFix:skl */
 	/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
@@ -249,7 +373,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 		i915_guc_submission_fini(dev_priv);
 err_guc:
 	i915_ggtt_disable_guc(dev_priv);
-
+err_wopcm:
 	if (i915_modparams.enable_guc_loading > 1 ||
 	    i915_modparams.enable_guc_submission > 1) {
 		DRM_ERROR("GuC init failed. Firmware loading disabled.\n");
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 973888e..aefba13 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -95,9 +95,13 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
 	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
 
-	/* Header and uCode will be loaded to WOPCM */
+	/*
+	 * Header and uCode will be loaded to WOPCM
+	 * Only check the size against the overall available WOPCM here. Will
+	 * continue to check the size during WOPCM partition calculation.
+	 */
 	size = uc_fw->header_size + uc_fw->ucode_size;
-	if (size > intel_guc_wopcm_size(dev_priv)) {
+	if (size > dev_priv->wopcm.size) {
 		DRM_WARN("%s: Firmware is too large to fit in WOPCM\n",
 			 intel_uc_fw_type_repr(uc_fw->type));
 		err = -E2BIG;
@@ -207,6 +211,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 		       int (*xfer)(struct intel_uc_fw *uc_fw,
 				   struct i915_vma *vma))
 {
+	struct drm_i915_private *i915 = to_i915(uc_fw->obj->base.dev);
 	struct i915_vma *vma;
 	int err;
 
@@ -230,7 +235,8 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
 	}
 
 	vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
-				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+				       PIN_OFFSET_BIAS |
+				       intel_guc_wopcm_top(i915));
 	if (IS_ERR(vma)) {
 		err = PTR_ERR(vma);
 		DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 1/2] drm/i915/guc: Submit GuC workitems containing coalesced requests
@ 2017-09-11 14:17 Michał Winiarski
  2017-09-11 14:17 ` [PATCH 2/2] HAX Enable GuC Submission for CI Michał Winiarski
  0 siblings, 1 reply; 13+ messages in thread
From: Michał Winiarski @ 2017-09-11 14:17 UTC (permalink / raw)
  To: intel-gfx

To create an upper bound on number of GuC workitems, we need to change
the way that requests are being submitted. Rather than submitting each
request as an individual workitem, we can do coalescing in a similar way
we're handlig execlist submission ports. We also need to stop pretending
that we're doing "lite-restore" in GuC submission (we would create a
workitem each time we hit this condition). This allows us to completely
remove the reservation, replacing it with a compile time check.

v2: Also coalesce when replaying on reset (Daniele)
v3: Consistent wq_resv - per-request (Daniele)
v4: Squash removing wq_resv

References: https://bugs.freedesktop.org/show_bug.cgi?id=101873
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |   2 -
 drivers/gpu/drm/i915/i915_guc_submission.c | 179 ++++++++++-------------------
 drivers/gpu/drm/i915/intel_lrc.c           |  25 +---
 drivers/gpu/drm/i915/intel_uc.h            |   8 --
 4 files changed, 62 insertions(+), 152 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 6338018f655d..f5fd00cfb3b0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2450,8 +2450,6 @@ static void i915_guc_client_info(struct seq_file *m,
 	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
 		client->wq_size, client->wq_offset, client->wq_tail);
 
-	seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space);
-
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = client->submissions[id];
 		tot += submissions;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e9349a2c..77ded6c64aca 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -406,63 +406,6 @@ static void guc_stage_desc_fini(struct intel_guc *guc,
 	memset(desc, 0, sizeof(*desc));
 }
 
-/**
- * i915_guc_wq_reserve() - reserve space in the GuC's workqueue
- * @request:	request associated with the commands
- *
- * Return:	0 if space is available
- *		-EAGAIN if space is not currently available
- *
- * This function must be called (and must return 0) before a request
- * is submitted to the GuC via i915_guc_submit() below. Once a result
- * of 0 has been returned, it must be balanced by a corresponding
- * call to submit().
- *
- * Reservation allows the caller to determine in advance that space
- * will be available for the next submission before committing resources
- * to it, and helps avoid late failures with complicated recovery paths.
- */
-int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
-{
-	const size_t wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *client = request->i915->guc.execbuf_client;
-	struct guc_process_desc *desc = __get_process_desc(client);
-	u32 freespace;
-	int ret;
-
-	spin_lock_irq(&client->wq_lock);
-	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
-	freespace -= client->wq_rsvd;
-	if (likely(freespace >= wqi_size)) {
-		client->wq_rsvd += wqi_size;
-		ret = 0;
-	} else {
-		client->no_wq_space++;
-		ret = -EAGAIN;
-	}
-	spin_unlock_irq(&client->wq_lock);
-
-	return ret;
-}
-
-static void guc_client_update_wq_rsvd(struct i915_guc_client *client, int size)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&client->wq_lock, flags);
-	client->wq_rsvd += size;
-	spin_unlock_irqrestore(&client->wq_lock, flags);
-}
-
-void i915_guc_wq_unreserve(struct drm_i915_gem_request *request)
-{
-	const int wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *client = request->i915->guc.execbuf_client;
-
-	GEM_BUG_ON(READ_ONCE(client->wq_rsvd) < wqi_size);
-	guc_client_update_wq_rsvd(client, -wqi_size);
-}
-
 /* Construct a Work Item and append it to the GuC's Work Queue */
 static void guc_wq_item_append(struct i915_guc_client *client,
 			       struct drm_i915_gem_request *rq)
@@ -475,7 +418,7 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	struct guc_wq_item *wqi;
 	u32 freespace, tail, wq_off;
 
-	/* Free space is guaranteed, see i915_guc_wq_reserve() above */
+	/* Free space is guaranteed */
 	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
 	GEM_BUG_ON(freespace < wqi_size);
 
@@ -491,14 +434,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	 * workqueue buffer dw by dw.
 	 */
 	BUILD_BUG_ON(wqi_size != 16);
-	GEM_BUG_ON(client->wq_rsvd < wqi_size);
 
 	/* postincrement WQ tail for next time */
 	wq_off = client->wq_tail;
 	GEM_BUG_ON(wq_off & (wqi_size - 1));
 	client->wq_tail += wqi_size;
 	client->wq_tail &= client->wq_size - 1;
-	client->wq_rsvd -= wqi_size;
 
 	/* WQ starts from the page after doorbell / process_desc */
 	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
@@ -580,48 +521,44 @@ static int guc_ring_doorbell(struct i915_guc_client *client)
 }
 
 /**
- * __i915_guc_submit() - Submit commands through GuC
+ * i915_guc_submit() - Submit commands through GuC
  * @rq:		request associated with the commands
  *
- * The caller must have already called i915_guc_wq_reserve() above with
- * a result of 0 (success), guaranteeing that there is space in the work
- * queue for the new request, so enqueuing the item cannot fail.
- *
- * Bad Things Will Happen if the caller violates this protocol e.g. calls
- * submit() when _reserve() says there's no space, or calls _submit()
- * a different number of times from (successful) calls to _reserve().
- *
  * The only error here arises if the doorbell hardware isn't functioning
  * as expected, which really shouln't happen.
  */
-static void __i915_guc_submit(struct drm_i915_gem_request *rq)
+static void i915_guc_submit(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = rq->i915;
-	struct intel_engine_cs *engine = rq->engine;
-	unsigned int engine_id = engine->id;
-	struct intel_guc *guc = &rq->i915->guc;
+	struct drm_i915_private *dev_priv = engine->i915;
+	struct intel_guc *guc = &dev_priv->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
+	struct execlist_port *port = engine->execlist_port;
+	unsigned int engine_id = engine->id;
+	unsigned int n;
 	unsigned long flags;
 	int b_ret;
 
-	/* WA to flush out the pending GMADR writes to ring buffer. */
-	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
-		POSTING_READ_FW(GUC_STATUS);
+	for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) {
+		struct drm_i915_gem_request *rq;
+		unsigned int count;
 
-	spin_lock_irqsave(&client->wq_lock, flags);
+		rq = port_unpack(&port[n], &count);
+		if (rq && count == 0) {
+			port_set(&port[n], port_pack(rq, ++count));
 
-	guc_wq_item_append(client, rq);
-	b_ret = guc_ring_doorbell(client);
+			if (i915_vma_is_map_and_fenceable(rq->ring->vma))
+				POSTING_READ_FW(GUC_STATUS);
 
-	client->submissions[engine_id] += 1;
+			spin_lock_irqsave(&client->wq_lock, flags);
 
-	spin_unlock_irqrestore(&client->wq_lock, flags);
-}
+			guc_wq_item_append(client, rq);
+			b_ret = guc_ring_doorbell(client);
 
-static void i915_guc_submit(struct drm_i915_gem_request *rq)
-{
-	__i915_gem_request_submit(rq);
-	__i915_guc_submit(rq);
+			client->submissions[engine_id] += 1;
+
+			spin_unlock_irqrestore(&client->wq_lock, flags);
+		}
+	}
 }
 
 static void nested_enable_signaling(struct drm_i915_gem_request *rq)
@@ -655,16 +592,19 @@ static void port_assign(struct execlist_port *port,
 	if (port_isset(port))
 		i915_gem_request_put(port_request(port));
 
-	port_set(port, i915_gem_request_get(rq));
+	port_set(port, port_pack(i915_gem_request_get(rq), port_count(port)));
 	nested_enable_signaling(rq);
 }
 
-static bool i915_guc_dequeue(struct intel_engine_cs *engine)
+static void i915_guc_dequeue(struct intel_engine_cs *engine)
 {
 	struct execlist_port *port = engine->execlist_port;
-	struct drm_i915_gem_request *last = port_request(port);
-	struct rb_node *rb;
+	struct drm_i915_gem_request *last = NULL;
 	bool submit = false;
+	struct rb_node *rb;
+
+	if (port_isset(port))
+		port++;
 
 	spin_lock_irq(&engine->timeline->lock);
 	rb = engine->execlist_first;
@@ -689,7 +629,7 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 			INIT_LIST_HEAD(&rq->priotree.link);
 			rq->priotree.priority = INT_MAX;
 
-			i915_guc_submit(rq);
+			__i915_gem_request_submit(rq);
 			trace_i915_gem_request_in(rq, port_index(port, engine));
 			last = rq;
 			submit = true;
@@ -703,11 +643,11 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 	}
 done:
 	engine->execlist_first = rb;
-	if (submit)
+	if (submit) {
 		port_assign(port, last);
+		i915_guc_submit(engine);
+	}
 	spin_unlock_irq(&engine->timeline->lock);
-
-	return submit;
 }
 
 static void i915_guc_irq_handler(unsigned long data)
@@ -715,24 +655,20 @@ static void i915_guc_irq_handler(unsigned long data)
 	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
 	struct execlist_port *port = engine->execlist_port;
 	struct drm_i915_gem_request *rq;
-	bool submit;
 
-	do {
-		rq = port_request(&port[0]);
-		while (rq && i915_gem_request_completed(rq)) {
-			trace_i915_gem_request_out(rq);
-			i915_gem_request_put(rq);
+	rq = port_request(&port[0]);
+	while (rq && i915_gem_request_completed(rq)) {
+		trace_i915_gem_request_out(rq);
+		i915_gem_request_put(rq);
 
-			port[0] = port[1];
-			memset(&port[1], 0, sizeof(port[1]));
+		port[0] = port[1];
+		memset(&port[1], 0, sizeof(port[1]));
 
-			rq = port_request(&port[0]);
-		}
+		rq = port_request(&port[0]);
+	}
 
-		submit = false;
-		if (!port_count(&port[1]))
-			submit = i915_guc_dequeue(engine);
-	} while (submit);
+	if (!port_isset(&port[1]))
+		i915_guc_dequeue(engine);
 }
 
 /*
@@ -1221,6 +1157,19 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 	enum intel_engine_id id;
 	int err;
 
+	/*
+	 * We're using GuC work items for submitting work through GuC. Since
+	 * we're coalescing multiple requests from a single context into a
+	 * single work item prior to assigning it to execlist_port, we can
+	 * never have more work items than the total number of ports (for all
+	 * engines). The GuC firmware is controlling the HEAD of work queue,
+	 * and it is guaranteed that it will remove the work item from the
+	 * queue before our request is completed.
+	 */
+	BUILD_BUG_ON(ARRAY_SIZE(engine->execlist_port) *
+		     sizeof(struct guc_wq_item) *
+		     I915_NUM_ENGINES > GUC_WQ_SIZE);
+
 	if (!client) {
 		client = guc_client_alloc(dev_priv,
 					  INTEL_INFO(dev_priv)->ring_mask,
@@ -1248,9 +1197,6 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 	guc_interrupts_capture(dev_priv);
 
 	for_each_engine(engine, dev_priv, id) {
-		const int wqi_size = sizeof(struct guc_wq_item);
-		struct drm_i915_gem_request *rq;
-
 		/* The tasklet was initialised by execlists, and may be in
 		 * a state of flux (across a reset) and so we just want to
 		 * take over the callback without changing any other state
@@ -1258,14 +1204,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		 */
 		engine->irq_tasklet.func = i915_guc_irq_handler;
 		clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
-
-		/* Replay the current set of previously submitted requests */
-		spin_lock_irq(&engine->timeline->lock);
-		list_for_each_entry(rq, &engine->timeline->requests, link) {
-			guc_client_update_wq_rsvd(client, wqi_size);
-			__i915_guc_submit(rq);
-		}
-		spin_unlock_irq(&engine->timeline->lock);
+		i915_guc_submit(engine);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d89e1b8e1cc5..5837b33f9705 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -914,27 +914,14 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 	 */
 	request->reserved_space += EXECLISTS_REQUEST_SIZE;
 
-	if (i915.enable_guc_submission) {
-		/*
-		 * Check that the GuC has space for the request before
-		 * going any further, as the i915_add_request() call
-		 * later on mustn't fail ...
-		 */
-		ret = i915_guc_wq_reserve(request);
-		if (ret)
-			goto err;
-	}
-
 	cs = intel_ring_begin(request, 0);
-	if (IS_ERR(cs)) {
-		ret = PTR_ERR(cs);
-		goto err_unreserve;
-	}
+	if (IS_ERR(cs))
+		return PTR_ERR(cs);
 
 	if (!ce->initialised) {
 		ret = engine->init_context(request);
 		if (ret)
-			goto err_unreserve;
+			return ret;
 
 		ce->initialised = true;
 	}
@@ -948,12 +935,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 
 	request->reserved_space -= EXECLISTS_REQUEST_SIZE;
 	return 0;
-
-err_unreserve:
-	if (i915.enable_guc_submission)
-		i915_guc_wq_unreserve(request);
-err:
-	return ret;
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..cfbc28e2396b 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -55,10 +55,6 @@ struct drm_i915_gem_request;
  *
  * We also keep a few statistics on failures. Ideally, these should all
  * be zero!
- *   no_wq_space: times that the submission pre-check found no space was
- *                available in the work queue (note, the queue is shared,
- *                not per-engine). It is OK for this to be nonzero, but
- *                it should not be huge!
  *   b_fail: failed to ring the doorbell. This should never happen, unless
  *           somehow the hardware misbehaves, or maybe if the GuC firmware
  *           crashes? We probably need to reset the GPU to recover.
@@ -83,8 +79,6 @@ struct i915_guc_client {
 	uint32_t wq_offset;
 	uint32_t wq_size;
 	uint32_t wq_tail;
-	uint32_t wq_rsvd;
-	uint32_t no_wq_space;
 
 	/* Per-engine counts of GuC submissions */
 	uint64_t submissions[I915_NUM_ENGINES];
@@ -250,8 +244,6 @@ u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 /* i915_guc_submission.c */
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
-int i915_guc_wq_reserve(struct drm_i915_gem_request *rq);
-void i915_guc_wq_unreserve(struct drm_i915_gem_request *request);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 1/2] drm/i915/scheduler: emulate a scheduler for guc
@ 2017-03-16 12:56 Chris Wilson
  2017-03-16 12:56 ` [PATCH 2/2] HAX enable guc submission for CI Chris Wilson
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2017-03-16 12:56 UTC (permalink / raw)
  To: intel-gfx

This emulates execlists on top of the GuC in order to defer submission of
requests to the hardware. This deferral allows time for high priority
requests to gazump their way to the head of the queue, however it nerfs
the GuC by converting it back into a simple execlist (where the CPU has
to wake up after every request to feed new commands into the GuC).

v2: Drop hack status - though iirc there is still a lockdep inversion
between fence and engine->timeline->lock (which is impossible as the
nesting only occurs on different fences - hopefully just requires some
judicious lockdep annotation)
v3: Apply lockdep nesting to enabling signaling on the request, using
the pattern we already have in __i915_gem_request_submit();
v4: Replaying requests after a hang also now needs the timeline
spinlock, to disable the interrupts at least
v5: Hold wq lock for completeness, and emit a tracepoint for enabling signal
v6: Reorder interrupt checking for a happier gcc.
v7: Only signal the tasklet after a user-interrupt if using guc scheduling
v8: Restore lost update of rq through the i915_guc_irq_handler (Tvrtko)
v9: Avoid re-initialising the engine->irq_tasklet from inside a reset
v10: Hook up the execlists-style tracepoints
v11: Clear the execlists irq_posted bit after taking over the interrupt/tasklet

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---

Do we still want this? Do we want this now?
-Chris

---
 drivers/gpu/drm/i915/i915_guc_submission.c | 110 ++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_irq.c            |  13 +++-
 drivers/gpu/drm/i915/intel_lrc.c           |   5 +-
 3 files changed, 112 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 5ec2cbda2ba9..46acdfb4a03a 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -25,6 +25,8 @@
 #include "i915_drv.h"
 #include "intel_uc.h"
 
+#include <trace/events/dma_fence.h>
+
 /**
  * DOC: GuC-based command submission
  *
@@ -522,8 +524,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
 		POSTING_READ_FW(GUC_STATUS);
 
-	trace_i915_gem_request_in(rq, 0);
-
 	spin_lock_irqsave(&client->wq_lock, flags);
 
 	guc_wq_item_append(client, rq);
@@ -542,10 +542,99 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 
 static void i915_guc_submit(struct drm_i915_gem_request *rq)
 {
-	i915_gem_request_submit(rq);
+	__i915_gem_request_submit(rq);
 	__i915_guc_submit(rq);
 }
 
+static void nested_enable_signaling(struct drm_i915_gem_request *rq)
+{
+	/* If we use dma_fence_enable_sw_signaling() directly, lockdep
+	 * detects an ordering issue between the fence lockclass and the
+	 * global_timeline. This circular dependency can only occur via 2
+	 * different fences (but same fence lockclass), so we use the nesting
+	 * annotation here to prevent the warn, equivalent to the nesting
+	 * inside i915_gem_request_submit() for when we also enable the
+	 * signaler.
+	 */
+
+	if (test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+			     &rq->fence.flags))
+		return;
+
+	GEM_BUG_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &rq->fence.flags));
+	trace_dma_fence_enable_signal(&rq->fence);
+
+	spin_lock_nested(&rq->lock, SINGLE_DEPTH_NESTING);
+	intel_engine_enable_signaling(rq);
+	spin_unlock(&rq->lock);
+}
+
+static bool i915_guc_dequeue(struct intel_engine_cs *engine)
+{
+	struct execlist_port *port = engine->execlist_port;
+	struct drm_i915_gem_request *last = port[0].request;
+	unsigned long flags;
+	struct rb_node *rb;
+	bool submit = false;
+
+	spin_lock_irqsave(&engine->timeline->lock, flags);
+	rb = engine->execlist_first;
+	while (rb) {
+		struct drm_i915_gem_request *rq =
+			rb_entry(rb, typeof(*rq), priotree.node);
+
+		if (last && rq->ctx != last->ctx) {
+			if (port != engine->execlist_port)
+				break;
+
+			i915_gem_request_assign(&port->request, last);
+			nested_enable_signaling(last);
+			port++;
+		}
+
+		rb = rb_next(rb);
+		rb_erase(&rq->priotree.node, &engine->execlist_queue);
+		RB_CLEAR_NODE(&rq->priotree.node);
+		rq->priotree.priority = INT_MAX;
+
+		trace_i915_gem_request_in(rq, port - engine->execlist_port);
+		i915_guc_submit(rq);
+		last = rq;
+		submit = true;
+	}
+	if (submit) {
+		i915_gem_request_assign(&port->request, last);
+		nested_enable_signaling(last);
+		engine->execlist_first = rb;
+	}
+	spin_unlock_irqrestore(&engine->timeline->lock, flags);
+
+	return submit;
+}
+
+static void i915_guc_irq_handler(unsigned long data)
+{
+	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
+	struct execlist_port *port = engine->execlist_port;
+	struct drm_i915_gem_request *rq;
+	bool submit;
+
+	do {
+		rq = port[0].request;
+		while (rq && i915_gem_request_completed(rq)) {
+			trace_i915_gem_request_out(rq);
+			i915_gem_request_put(rq);
+			port[0].request = port[1].request;
+			port[1].request = NULL;
+			rq = port[0].request;
+		}
+
+		submit = false;
+		if (!port[1].request)
+			submit = i915_guc_dequeue(engine);
+	} while (submit);
+}
+
 /*
  * Everything below here is concerned with setup & teardown, and is
  * therefore not part of the somewhat time-critical batch-submission
@@ -987,18 +1076,21 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 	guc_init_doorbell_hw(guc);
 
 	/* Take over from manual control of ELSP (execlists) */
-	for_each_engine(engine, dev_priv, id) {
-		engine->submit_request = i915_guc_submit;
-		engine->schedule = NULL;
-	}
-
 	guc_interrupts_capture(dev_priv);
 
-	/* Replay the current set of previously submitted requests */
 	for_each_engine(engine, dev_priv, id) {
 		const int wqi_size = sizeof(struct guc_wq_item);
 		struct drm_i915_gem_request *rq;
 
+		/* The tasklet was initialised by execlists, and may be in
+		 * a state of flux (across a reset) and so we just want to
+		 * take over the callback without changing any other state
+		 * in the tasklet.
+		 */
+		engine->irq_tasklet.func = i915_guc_irq_handler;
+		clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
+
+		/* Replay the current set of previously submitted requests */
 		spin_lock_irq(&engine->timeline->lock);
 		list_for_each_entry(rq, &engine->timeline->requests, link) {
 			guc_client_update_wq_rsvd(client, wqi_size);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ea92af229133..6e716399ea44 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1355,13 +1355,20 @@ static void snb_gt_irq_handler(struct drm_i915_private *dev_priv,
 static __always_inline void
 gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir, int test_shift)
 {
-	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift))
-		notify_ring(engine);
+	bool tasklet = false;
 
 	if (iir & (GT_CONTEXT_SWITCH_INTERRUPT << test_shift)) {
 		set_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
-		tasklet_hi_schedule(&engine->irq_tasklet);
+		tasklet = true;
 	}
+
+	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift)) {
+		notify_ring(engine);
+		tasklet |= i915.enable_guc_submission;
+	}
+
+	if (tasklet)
+		tasklet_hi_schedule(&engine->irq_tasklet);
 }
 
 static irqreturn_t gen8_gt_irq_ack(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 89f38e7def9f..e664dbf6bd11 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1159,7 +1159,7 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
 
 	/* After a GPU reset, we may have requests to replay */
 	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
-	if (!execlists_elsp_idle(engine)) {
+	if (!i915.enable_guc_submission && !execlists_elsp_idle(engine)) {
 		DRM_DEBUG_DRIVER("Restarting %s from requests [0x%x, 0x%x]\n",
 				 engine->name,
 				 port_seqno(&engine->execlist_port[0]),
@@ -1244,9 +1244,6 @@ static void reset_common_ring(struct intel_engine_cs *engine,
 	request->ring->last_retired_head = -1;
 	intel_ring_update_space(request->ring);
 
-	if (i915.enable_guc_submission)
-		return;
-
 	/* Catch up with any missed context-switch interrupts */
 	if (request->ctx != port[0].request->ctx) {
 		i915_gem_request_put(port[0].request);
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-02-26 16:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-04  0:01 [PATCH 1/2] drm/i915: implemented dynamic WOPCM partition Jackie Li
2017-11-04  0:01 ` [PATCH 2/2] HAX enable guc submission for CI Jackie Li
2017-11-04  0:16 ` [PATCH 1/2] drm/i915: implemented dynamic WOPCM partition Yaodong Li
2017-11-04  0:20 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
2017-11-06 13:24 ` [PATCH 1/2] " Ville Syrjälä
2017-11-06 18:20   ` Yaodong Li
2017-11-08 12:33 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2018-02-26 16:37 [PATCH v2 1/2] drm/i915/guc: Fill preempt context once at init time Michał Winiarski
2018-02-26 16:38 ` [PATCH 2/2] HAX: Enable GuC submission for CI Michał Winiarski
2018-02-26 13:59 [PATCH 1/2] drm/i915/guc: Fill preempt context once at init time Michał Winiarski
2018-02-26 14:00 ` [PATCH 2/2] HAX: Enable GuC submission for CI Michał Winiarski
2017-11-24 13:37 [PATCH 1/2] drm/i915/guc: Advance over port[0] if set and not preempting Chris Wilson
2017-11-24 13:37 ` [PATCH 2/2] HAX Enable GuC Submission for CI Chris Wilson
2017-11-04  0:18 [PATCH 1/2] drm/i915: implemented dynamic WOPCM partition Jackie Li
2017-11-04  0:18 ` [PATCH 2/2] HAX enable guc submission for CI Jackie Li
2017-09-11 14:17 [PATCH 1/2] drm/i915/guc: Submit GuC workitems containing coalesced requests Michał Winiarski
2017-09-11 14:17 ` [PATCH 2/2] HAX Enable GuC Submission for CI Michał Winiarski
2017-03-16 12:56 [PATCH 1/2] drm/i915/scheduler: emulate a scheduler for guc Chris Wilson
2017-03-16 12:56 ` [PATCH 2/2] HAX enable guc submission for CI Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).