intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
@ 2017-11-03 15:18 Michal Wajdeczko
  2017-11-03 15:18 ` [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion Michal Wajdeczko
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Michal Wajdeczko @ 2017-11-03 15:18 UTC (permalink / raw)
  To: intel-gfx

Transfer of GuC firmware requires few steps that currently
are implemented in two large functions. Split this code into
smaller functions to make these steps small and clear.
Also be prepared for potential DMA xfer step failure.

v2: rename function steps (Sagar)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fw.c | 172 +++++++++++++++++++++---------------
 1 file changed, 103 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index ef67a36..c4f4526 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -97,23 +97,55 @@ int intel_guc_fw_select(struct intel_guc *guc)
 	return 0;
 }
 
-/*
- * Read the GuC status register (GUC_STATUS) and store it in the
- * specified location; then return a boolean indicating whether
- * the value matches either of two values representing completion
- * of the GuC boot process.
- *
- * This is used for polling the GuC status in a wait_for()
- * loop below.
- */
-static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
-				      u32 *status)
+static void guc_prepare_xfer(struct intel_guc *guc)
 {
-	u32 val = I915_READ(GUC_STATUS);
-	u32 uk_val = val & GS_UKERNEL_MASK;
-	*status = val;
-	return (uk_val == GS_UKERNEL_READY ||
-		((val & GS_MIA_CORE_STATE) && uk_val == GS_UKERNEL_LAPIC_DONE));
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	/* Enable MIA caching. GuC clock gating is disabled. */
+	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
+
+	/* WaDisableMinuteIaClockGating:bxt */
+	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
+		I915_WRITE(GUC_SHIM_CONTROL, (I915_READ(GUC_SHIM_CONTROL) &
+					      ~GUC_ENABLE_MIA_CLOCK_GATING));
+	}
+
+	/* WaC6DisallowByGfxPause:bxt */
+	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0))
+		I915_WRITE(GEN6_GFXPAUSE, 0x30FFF);
+
+	if (IS_GEN9_LP(dev_priv))
+		I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
+	else
+		I915_WRITE(GEN9_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
+
+	if (IS_GEN9(dev_priv)) {
+		/* DOP Clock Gating Enable for GuC clocks */
+		I915_WRITE(GEN7_MISCCPCTL, (GEN8_DOP_CLOCK_GATE_GUC_ENABLE |
+					    I915_READ(GEN7_MISCCPCTL)));
+
+		/* allows for 5us (in 10ns units) before GT can go to RC6 */
+		I915_WRITE(GUC_ARAT_C6DIS, 0x1FF);
+	}
+}
+
+/* Copy RSA signature from the fw image to HW for verification */
+static int guc_xfer_rsa(struct intel_guc *guc, struct i915_vma *vma)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct intel_uc_fw *guc_fw = &guc->fw;
+	struct sg_table *sg = vma->pages;
+	u32 rsa[UOS_RSA_SCRATCH_MAX_COUNT];
+	int i;
+
+	if (sg_pcopy_to_buffer(sg->sgl, sg->nents, rsa, sizeof(rsa),
+			       guc_fw->rsa_offset) != sizeof(rsa))
+		return -EINVAL;
+
+	for (i = 0; i < UOS_RSA_SCRATCH_MAX_COUNT; i++)
+		I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
+
+	return 0;
 }
 
 /*
@@ -122,29 +154,17 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
  * Architecturally, the DMA engine is bidirectional, and can potentially even
  * transfer between GTT locations. This functionality is left out of the API
  * for now as there is no need for it.
- *
- * Note that GuC needs the CSS header plus uKernel code to be copied by the
- * DMA engine in one operation, whereas the RSA signature is loaded via MMIO.
  */
-static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
-			      struct i915_vma *vma)
+static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
 {
-	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct intel_uc_fw *guc_fw = &guc->fw;
 	unsigned long offset;
-	struct sg_table *sg = vma->pages;
-	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
-	int i, ret = 0;
-
-	/* where RSA signature starts */
-	offset = guc_fw->rsa_offset;
 
-	/* Copy RSA signature from the fw image to HW for verification */
-	sg_pcopy_to_buffer(sg->sgl, sg->nents, rsa, sizeof(rsa), offset);
-	for (i = 0; i < UOS_RSA_SCRATCH_MAX_COUNT; i++)
-		I915_WRITE(UOS_RSA_SCRATCH(i), rsa[i]);
-
-	/* The header plus uCode will be copied to WOPCM via DMA, excluding any
-	 * other components */
+	/*
+	 * The header plus uCode will be copied to WOPCM via DMA, excluding any
+	 * other components
+	 */
 	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
 
 	/* Set the source address for the new blob */
@@ -162,33 +182,57 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
 	/* Finally start the DMA */
 	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
 
+	return 0;
+}
+
+/*
+ * Read the GuC status register (GUC_STATUS) and store it in the
+ * specified location; then return a boolean indicating whether
+ * the value matches either of two values representing completion
+ * of the GuC boot process.
+ *
+ * This is used for polling the GuC status in a wait_for()
+ * loop below.
+ */
+static inline bool guc_ready(struct intel_guc *guc, u32 *status)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 val = I915_READ(GUC_STATUS);
+	u32 uk_val = val & GS_UKERNEL_MASK;
+
+	*status = val;
+	return (uk_val == GS_UKERNEL_READY) ||
+		((val & GS_MIA_CORE_STATE) && (uk_val == GS_UKERNEL_LAPIC_DONE));
+}
+
+static int guc_wait_ucode(struct intel_guc *guc)
+{
+	u32 status;
+	int ret;
+
 	/*
-	 * Wait for the DMA to complete & the GuC to start up.
+	 * Wait for the GuC to start up.
 	 * NB: Docs recommend not using the interrupt for completion.
 	 * Measurements indicate this should take no more than 20ms, so a
 	 * timeout here indicates that the GuC has failed and is unusable.
 	 * (Higher levels of the driver will attempt to fall back to
 	 * execlist mode if this happens.)
 	 */
-	ret = wait_for(guc_ucode_response(dev_priv, &status), 100);
-
-	DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
-			I915_READ(DMA_CTRL), status);
+	ret = wait_for(guc_ready(guc, &status), 100);
+	DRM_DEBUG_DRIVER("GuC status %#x\n", status);
 
 	if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
 		DRM_ERROR("GuC firmware signature verification failed\n");
 		ret = -ENOEXEC;
 	}
 
-	DRM_DEBUG_DRIVER("returning %d\n", ret);
-
 	return ret;
 }
 
 /*
  * Load the GuC firmware blob into the MinuteIA.
  */
-static int guc_ucode_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
+static int guc_fw_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
 {
 	struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw);
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -198,34 +242,24 @@ static int guc_ucode_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
-	/* Enable MIA caching. GuC clock gating is disabled. */
-	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
-
-	/* WaDisableMinuteIaClockGating:bxt */
-	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
-		I915_WRITE(GUC_SHIM_CONTROL, (I915_READ(GUC_SHIM_CONTROL) &
-					      ~GUC_ENABLE_MIA_CLOCK_GATING));
-	}
-
-	/* WaC6DisallowByGfxPause:bxt */
-	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0))
-		I915_WRITE(GEN6_GFXPAUSE, 0x30FFF);
+	guc_prepare_xfer(guc);
 
-	if (IS_GEN9_LP(dev_priv))
-		I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
-	else
-		I915_WRITE(GEN9_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
-
-	if (IS_GEN9(dev_priv)) {
-		/* DOP Clock Gating Enable for GuC clocks */
-		I915_WRITE(GEN7_MISCCPCTL, (GEN8_DOP_CLOCK_GATE_GUC_ENABLE |
-					    I915_READ(GEN7_MISCCPCTL)));
+	/*
+	 * Note that GuC needs the CSS header plus uKernel code to be copied
+	 * by the DMA engine in one operation, whereas the RSA signature is
+	 * loaded via MMIO.
+	 */
+	ret = guc_xfer_rsa(guc, vma);
+	if (ret)
+		DRM_WARN("GuC firmware signature xfer error %d\n", ret);
 
-		/* allows for 5us (in 10ns units) before GT can go to RC6 */
-		I915_WRITE(GUC_ARAT_C6DIS, 0x1FF);
-	}
+	ret = guc_xfer_ucode(guc, vma);
+	if (ret)
+		DRM_WARN("GuC firmware code xfer error %d\n", ret);
 
-	ret = guc_ucode_xfer_dma(dev_priv, vma);
+	ret = guc_wait_ucode(guc);
+	if (ret)
+		DRM_ERROR("GuC firmware xfer error %d\n", ret);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
@@ -247,5 +281,5 @@ static int guc_ucode_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
  */
 int intel_guc_fw_upload(struct intel_guc *guc)
 {
-	return intel_uc_fw_upload(&guc->fw, guc_ucode_xfer);
+	return intel_uc_fw_upload(&guc->fw, guc_fw_xfer);
 }
-- 
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 v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion
  2017-11-03 15:18 [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Michal Wajdeczko
@ 2017-11-03 15:18 ` Michal Wajdeczko
  2017-11-08 20:36   ` Chris Wilson
  2017-11-03 15:18 ` [PATCH v4 3/5] drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer Michal Wajdeczko
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2017-11-03 15:18 UTC (permalink / raw)
  To: intel-gfx

We silently assumed that DMA transfer will be completed
within assumed timeout and thus we were waiting only at
last step for GuC to become ready. Add intermediate wait
to catch unexpected delays in DMA transfer.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fw.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index c4f4526..74a61fe 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -160,6 +160,8 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct intel_uc_fw *guc_fw = &guc->fw;
 	unsigned long offset;
+	u32 status;
+	int ret;
 
 	/*
 	 * The header plus uCode will be copied to WOPCM via DMA, excluding any
@@ -182,7 +184,12 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
 	/* Finally start the DMA */
 	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
 
-	return 0;
+	/* Wait for DMA to finish */
+	ret = __intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0,
+					   2, 100, &status);
+	DRM_DEBUG_DRIVER("GuC DMA status %#x\n", status);
+
+	return ret;
 }
 
 /*
-- 
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 v4 3/5] drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer
  2017-11-03 15:18 [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Michal Wajdeczko
  2017-11-03 15:18 ` [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion Michal Wajdeczko
@ 2017-11-03 15:18 ` Michal Wajdeczko
  2017-11-03 16:17   ` Sagar Arun Kamble
  2017-11-03 15:18 ` [PATCH v4 4/5] drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL Michal Wajdeczko
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2017-11-03 15:18 UTC (permalink / raw)
  To: intel-gfx

We don't keep the workarounds for pre-production hardware
(see intel_detect_preproduction_hw) thus we can drop some
extra steps during firmware upload needed only for unsupported
platforms.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fw.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 74a61fe..a63b5cf 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -104,16 +104,6 @@ static void guc_prepare_xfer(struct intel_guc *guc)
 	/* Enable MIA caching. GuC clock gating is disabled. */
 	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
 
-	/* WaDisableMinuteIaClockGating:bxt */
-	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
-		I915_WRITE(GUC_SHIM_CONTROL, (I915_READ(GUC_SHIM_CONTROL) &
-					      ~GUC_ENABLE_MIA_CLOCK_GATING));
-	}
-
-	/* WaC6DisallowByGfxPause:bxt */
-	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0))
-		I915_WRITE(GEN6_GFXPAUSE, 0x30FFF);
-
 	if (IS_GEN9_LP(dev_priv))
 		I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
 	else
-- 
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 v4 4/5] drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL
  2017-11-03 15:18 [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Michal Wajdeczko
  2017-11-03 15:18 ` [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion Michal Wajdeczko
  2017-11-03 15:18 ` [PATCH v4 3/5] drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer Michal Wajdeczko
@ 2017-11-03 15:18 ` Michal Wajdeczko
  2017-11-03 16:20   ` Sagar Arun Kamble
  2017-11-03 15:18 ` [PATCH v4 5/5] HAX enable GuC submission for CI Michal Wajdeczko
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2017-11-03 15:18 UTC (permalink / raw)
  To: intel-gfx

We can program GUC_SHIM_CONTROL register with all expected
bits without use of extra macro defined in fwif.h

v2: rebased without pre-prod code
v3: fixed typo

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_reg.h | 7 -------
 drivers/gpu/drm/i915/intel_guc_fw.c | 9 +++++++--
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h
index 35cf991..bc1ae7d 100644
--- a/drivers/gpu/drm/i915/i915_guc_reg.h
+++ b/drivers/gpu/drm/i915/i915_guc_reg.h
@@ -102,13 +102,6 @@
 #define   GUC_ENABLE_MIA_CLOCK_GATING		(1<<15)
 #define   GUC_GEN10_SHIM_WC_ENABLE		(1<<21)
 
-#define GUC_SHIM_CONTROL_VALUE	(GUC_DISABLE_SRAM_INIT_TO_ZEROES	| \
-				 GUC_ENABLE_READ_CACHE_LOGIC		| \
-				 GUC_ENABLE_MIA_CACHING			| \
-				 GUC_ENABLE_READ_CACHE_FOR_SRAM_DATA	| \
-				 GUC_ENABLE_READ_CACHE_FOR_WOPCM_DATA	| \
-				 GUC_ENABLE_MIA_CLOCK_GATING)
-
 #define GUC_SEND_INTERRUPT		_MMIO(0xc4c8)
 #define   GUC_SEND_TRIGGER		  (1<<0)
 
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index a63b5cf..69ba015 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -101,8 +101,13 @@ static void guc_prepare_xfer(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	/* Enable MIA caching. GuC clock gating is disabled. */
-	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
+	/* Must program this register before loading the ucode with DMA */
+	I915_WRITE(GUC_SHIM_CONTROL, GUC_DISABLE_SRAM_INIT_TO_ZEROES |
+				     GUC_ENABLE_READ_CACHE_LOGIC |
+				     GUC_ENABLE_MIA_CACHING |
+				     GUC_ENABLE_READ_CACHE_FOR_SRAM_DATA |
+				     GUC_ENABLE_READ_CACHE_FOR_WOPCM_DATA |
+				     GUC_ENABLE_MIA_CLOCK_GATING);
 
 	if (IS_GEN9_LP(dev_priv))
 		I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
-- 
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 v4 5/5] HAX enable GuC submission for CI
  2017-11-03 15:18 [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Michal Wajdeczko
                   ` (2 preceding siblings ...)
  2017-11-03 15:18 ` [PATCH v4 4/5] drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL Michal Wajdeczko
@ 2017-11-03 15:18 ` Michal Wajdeczko
  2017-11-03 15:52 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Patchwork
  2017-11-03 18:16 ` ✗ Fi.CI.IGT: warning " Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Michal Wajdeczko @ 2017-11-03 15:18 UTC (permalink / raw)
  To: intel-gfx

Also revert ("drm/i915/guc: Assert that we switch between
known ggtt->invalidate functions"

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.h  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0684d5d..a351ddf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3562,17 +3562,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c729226..c38cef0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,8 +44,8 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc_loading, 0) \
-	param(int, enable_guc_submission, 0) \
+	param(int, enable_guc_loading, 1) \
+	param(int, enable_guc_submission, 1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
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

* ✓ Fi.CI.BAT: success for series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
  2017-11-03 15:18 [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Michal Wajdeczko
                   ` (3 preceding siblings ...)
  2017-11-03 15:18 ` [PATCH v4 5/5] HAX enable GuC submission for CI Michal Wajdeczko
@ 2017-11-03 15:52 ` Patchwork
  2017-11-08 21:48   ` Chris Wilson
  2017-11-03 18:16 ` ✗ Fi.CI.IGT: warning " Patchwork
  5 siblings, 1 reply; 13+ messages in thread
From: Patchwork @ 2017-11-03 15:52 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
URL   : https://patchwork.freedesktop.org/series/33135/
State : success

== Summary ==

Series 33135v1 series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
https://patchwork.freedesktop.org/api/1.0/series/33135/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-wb-set-default:
                incomplete -> PASS       (fi-glk-dsi)
Test gem_exec_reloc:
        Subgroup basic-write-gtt-active:
                fail       -> PASS       (fi-gdg-551) fdo#102582
Test gem_sync:
        Subgroup basic-many-each:
                pass       -> FAIL       (fi-kbl-7567u) fdo#103165
Test kms_busy:
        Subgroup basic-flip-b:
                fail       -> PASS       (fi-bwr-2160)
Test drv_module_reload:
        Subgroup basic-no-display:
                fail       -> PASS       (fi-hsw-4770r) fdo#103534

fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#103165 https://bugs.freedesktop.org/show_bug.cgi?id=103165
fdo#103534 https://bugs.freedesktop.org/show_bug.cgi?id=103534

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:445s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:465s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:379s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:556s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:280s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:516s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:520s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:515s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:491s
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:559s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:432s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:262s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:586s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:496s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:434s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:431s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:433s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:467s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:494s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:580s
fi-kbl-7567u     total:289  pass:268  dwarn:0   dfail:0   fail:1   skip:20  time:484s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:587s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:568s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:599s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:655s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:528s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:505s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:466s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:582s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:428s

de359919ae463cdaef6bc6890156df84e19dee2a drm-tip: 2017y-11m-03d-14h-00m-37s UTC integration manifest
3ecbf3a018bf HAX enable GuC submission for CI
dc2ae4c18fdc drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL
df9e25cc0636 drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer
42cba2806413 drm/i915/guc: Wait for ucode DMA transfer completion
c0b68e0fb242 drm/i915/guc: Split GuC firmware xfer function into clear steps

== Logs ==

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

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

* Re: [PATCH v4 3/5] drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer
  2017-11-03 15:18 ` [PATCH v4 3/5] drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer Michal Wajdeczko
@ 2017-11-03 16:17   ` Sagar Arun Kamble
  0 siblings, 0 replies; 13+ messages in thread
From: Sagar Arun Kamble @ 2017-11-03 16:17 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 11/3/2017 8:48 PM, Michal Wajdeczko wrote:
> We don't keep the workarounds for pre-production hardware
> (see intel_detect_preproduction_hw) thus we can drop some
> extra steps during firmware upload needed only for unsupported
> platforms.
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_fw.c | 10 ----------
>   1 file changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
> index 74a61fe..a63b5cf 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -104,16 +104,6 @@ static void guc_prepare_xfer(struct intel_guc *guc)
>   	/* Enable MIA caching. GuC clock gating is disabled. */
>   	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
>   
> -	/* WaDisableMinuteIaClockGating:bxt */
> -	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
> -		I915_WRITE(GUC_SHIM_CONTROL, (I915_READ(GUC_SHIM_CONTROL) &
> -					      ~GUC_ENABLE_MIA_CLOCK_GATING));
> -	}
> -
> -	/* WaC6DisallowByGfxPause:bxt */
> -	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B0))
> -		I915_WRITE(GEN6_GFXPAUSE, 0x30FFF);
> -
>   	if (IS_GEN9_LP(dev_priv))
>   		I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE);
>   	else

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

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

* Re: [PATCH v4 4/5] drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL
  2017-11-03 15:18 ` [PATCH v4 4/5] drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL Michal Wajdeczko
@ 2017-11-03 16:20   ` Sagar Arun Kamble
  0 siblings, 0 replies; 13+ messages in thread
From: Sagar Arun Kamble @ 2017-11-03 16:20 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 11/3/2017 8:48 PM, Michal Wajdeczko wrote:
> We can program GUC_SHIM_CONTROL register with all expected
> bits without use of extra macro defined in fwif.h
>
> v2: rebased without pre-prod code
> v3: fixed typo
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_reg.h | 7 -------
>   drivers/gpu/drm/i915/intel_guc_fw.c | 9 +++++++--
>   2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h b/drivers/gpu/drm/i915/i915_guc_reg.h
> index 35cf991..bc1ae7d 100644
> --- a/drivers/gpu/drm/i915/i915_guc_reg.h
> +++ b/drivers/gpu/drm/i915/i915_guc_reg.h
> @@ -102,13 +102,6 @@
>   #define   GUC_ENABLE_MIA_CLOCK_GATING		(1<<15)
>   #define   GUC_GEN10_SHIM_WC_ENABLE		(1<<21)
>   
> -#define GUC_SHIM_CONTROL_VALUE	(GUC_DISABLE_SRAM_INIT_TO_ZEROES	| \
> -				 GUC_ENABLE_READ_CACHE_LOGIC		| \
> -				 GUC_ENABLE_MIA_CACHING			| \
> -				 GUC_ENABLE_READ_CACHE_FOR_SRAM_DATA	| \
> -				 GUC_ENABLE_READ_CACHE_FOR_WOPCM_DATA	| \
> -				 GUC_ENABLE_MIA_CLOCK_GATING)
> -
>   #define GUC_SEND_INTERRUPT		_MMIO(0xc4c8)
>   #define   GUC_SEND_TRIGGER		  (1<<0)
>   
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
> index a63b5cf..69ba015 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -101,8 +101,13 @@ static void guc_prepare_xfer(struct intel_guc *guc)
>   {
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   
> -	/* Enable MIA caching. GuC clock gating is disabled. */
> -	I915_WRITE(GUC_SHIM_CONTROL, GUC_SHIM_CONTROL_VALUE);
> +	/* Must program this register before loading the ucode with DMA */
> +	I915_WRITE(GUC_SHIM_CONTROL, GUC_DISABLE_SRAM_INIT_TO_ZEROES |
> +				     GUC_ENABLE_READ_CACHE_LOGIC |
> +				     GUC_ENABLE_MIA_CACHING |
> +				     GUC_ENABLE_READ_CACHE_FOR_SRAM_DATA |
> +				     GUC_ENABLE_READ_CACHE_FOR_WOPCM_DATA |
> +				     GUC_ENABLE_MIA_CLOCK_GATING);
>   
>   	if (IS_GEN9_LP(dev_priv))
>   		I915_WRITE(GEN9LP_GT_PM_CONFIG, GT_DOORBELL_ENABLE);

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

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

* ✗ Fi.CI.IGT: warning for series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
  2017-11-03 15:18 [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Michal Wajdeczko
                   ` (4 preceding siblings ...)
  2017-11-03 15:52 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Patchwork
@ 2017-11-03 18:16 ` Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2017-11-03 18:16 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
URL   : https://patchwork.freedesktop.org/series/33135/
State : warning

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-A:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup extended-modeset-hang-oldfb-with-reset-render-C:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_plane_lowres:
        Subgroup pipe-B-tiling-none:
                pass       -> SKIP       (shard-hsw)
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-C-planes:
                pass       -> SKIP       (shard-hsw)
        Subgroup plane-panning-bottom-right-suspend-pipe-B-planes:
                pass       -> SKIP       (shard-hsw)
Test kms_chv_cursor_fail:
        Subgroup pipe-B-128x128-bottom-edge:
                pass       -> SKIP       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2539 pass:1428 dwarn:2   dfail:1   fail:7   skip:1101 time:9235s

== Logs ==

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

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

* Re: [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion
  2017-11-03 15:18 ` [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion Michal Wajdeczko
@ 2017-11-08 20:36   ` Chris Wilson
  2017-11-08 21:17     ` Michal Wajdeczko
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2017-11-08 20:36 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx

Quoting Michal Wajdeczko (2017-11-03 15:18:13)
> We silently assumed that DMA transfer will be completed
> within assumed timeout and thus we were waiting only at
> last step for GuC to become ready. Add intermediate wait
> to catch unexpected delays in DMA transfer.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_fw.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
> index c4f4526..74a61fe 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -160,6 +160,8 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
>         struct intel_uc_fw *guc_fw = &guc->fw;
>         unsigned long offset;
> +       u32 status;
> +       int ret;
>  
>         /*
>          * The header plus uCode will be copied to WOPCM via DMA, excluding any
> @@ -182,7 +184,12 @@ static int guc_xfer_ucode(struct intel_guc *guc, struct i915_vma *vma)
>         /* Finally start the DMA */
>         I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
>  
> -       return 0;
> +       /* Wait for DMA to finish */
> +       ret = __intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA, 0,
> +                                          2, 100, &status);

To use _fw, you need to either hold forcewake or be sure you don't need
it, and you also need to be sure that either you have serialisation
around the routine or that it is not subject to the various concurrent
mmio access bugs. I haven't seen anything that suggests they have been
taken into account.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion
  2017-11-08 20:36   ` Chris Wilson
@ 2017-11-08 21:17     ` Michal Wajdeczko
  2017-11-08 21:26       ` Chris Wilson
  0 siblings, 1 reply; 13+ messages in thread
From: Michal Wajdeczko @ 2017-11-08 21:17 UTC (permalink / raw)
  To: intel-gfx, Chris Wilson

On Wed, 08 Nov 2017 21:36:25 +0100, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-11-03 15:18:13)
>> We silently assumed that DMA transfer will be completed
>> within assumed timeout and thus we were waiting only at
>> last step for GuC to become ready. Add intermediate wait
>> to catch unexpected delays in DMA transfer.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_fw.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c  
>> b/drivers/gpu/drm/i915/intel_guc_fw.c
>> index c4f4526..74a61fe 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
>> @@ -160,6 +160,8 @@ static int guc_xfer_ucode(struct intel_guc *guc,  
>> struct i915_vma *vma)
>>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
>>         struct intel_uc_fw *guc_fw = &guc->fw;
>>         unsigned long offset;
>> +       u32 status;
>> +       int ret;
>>
>>         /*
>>          * The header plus uCode will be copied to WOPCM via DMA,  
>> excluding any
>> @@ -182,7 +184,12 @@ static int guc_xfer_ucode(struct intel_guc *guc,  
>> struct i915_vma *vma)
>>         /* Finally start the DMA */
>>         I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
>>
>> -       return 0;
>> +       /* Wait for DMA to finish */
>> +       ret = __intel_wait_for_register_fw(dev_priv, DMA_CTRL,  
>> START_DMA, 0,
>> +                                          2, 100, &status);
>
> To use _fw, you need to either hold forcewake or be sure you don't need
> it, and you also need to be sure that either you have serialisation
> around the routine or that it is not subject to the various concurrent
> mmio access bugs. I haven't seen anything that suggests they have been
> taken into account.

Note that this function is called by guc_fw_xfer() inside

	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
	...
	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);

and for serialization (with similar code in huc_ucode_xfer) we just
rely on code sequence in intel_uc_init_hw ... is this not enough ?

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

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

* Re: [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion
  2017-11-08 21:17     ` Michal Wajdeczko
@ 2017-11-08 21:26       ` Chris Wilson
  0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2017-11-08 21:26 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx

Quoting Michal Wajdeczko (2017-11-08 21:17:35)
> On Wed, 08 Nov 2017 21:36:25 +0100, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2017-11-03 15:18:13)
> >> We silently assumed that DMA transfer will be completed
> >> within assumed timeout and thus we were waiting only at
> >> last step for GuC to become ready. Add intermediate wait
> >> to catch unexpected delays in DMA transfer.
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_guc_fw.c | 9 ++++++++-
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c  
> >> b/drivers/gpu/drm/i915/intel_guc_fw.c
> >> index c4f4526..74a61fe 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> >> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> >> @@ -160,6 +160,8 @@ static int guc_xfer_ucode(struct intel_guc *guc,  
> >> struct i915_vma *vma)
> >>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
> >>         struct intel_uc_fw *guc_fw = &guc->fw;
> >>         unsigned long offset;
> >> +       u32 status;
> >> +       int ret;
> >>
> >>         /*
> >>          * The header plus uCode will be copied to WOPCM via DMA,  
> >> excluding any
> >> @@ -182,7 +184,12 @@ static int guc_xfer_ucode(struct intel_guc *guc,  
> >> struct i915_vma *vma)
> >>         /* Finally start the DMA */
> >>         I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
> >>
> >> -       return 0;
> >> +       /* Wait for DMA to finish */
> >> +       ret = __intel_wait_for_register_fw(dev_priv, DMA_CTRL,  
> >> START_DMA, 0,
> >> +                                          2, 100, &status);
> >
> > To use _fw, you need to either hold forcewake or be sure you don't need
> > it, and you also need to be sure that either you have serialisation
> > around the routine or that it is not subject to the various concurrent
> > mmio access bugs. I haven't seen anything that suggests they have been
> > taken into account.
> 
> Note that this function is called by guc_fw_xfer() inside
> 
>         intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>         ...
>         intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> 
> and for serialization (with similar code in huc_ucode_xfer) we just
> rely on code sequence in intel_uc_init_hw ... is this not enough ?

That's all that is required. It was odd to see the raw _fw version as
the first use of _FW, hence the question. You are using that version
because it's the only supplier of out_status. Less eyebrows will be
raised if we made it available via intel_wait_for_register().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✓ Fi.CI.BAT: success for series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
  2017-11-03 15:52 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Patchwork
@ 2017-11-08 21:48   ` Chris Wilson
  0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2017-11-08 21:48 UTC (permalink / raw)
  To: Patchwork, Michal Wajdeczko; +Cc: intel-gfx

Quoting Patchwork (2017-11-03 15:52:24)
> == Series Details ==
> 
> Series: series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
> URL   : https://patchwork.freedesktop.org/series/33135/
> State : success
> 
> == Summary ==
> 
> Series 33135v1 series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps
> https://patchwork.freedesktop.org/api/1.0/series/33135/revisions/1/mbox/
> 
> Test gem_exec_flush:
>         Subgroup basic-wb-set-default:
>                 incomplete -> PASS       (fi-glk-dsi)
> Test gem_exec_reloc:
>         Subgroup basic-write-gtt-active:
>                 fail       -> PASS       (fi-gdg-551) fdo#102582
> Test gem_sync:
>         Subgroup basic-many-each:
>                 pass       -> FAIL       (fi-kbl-7567u) fdo#103165

Since this isn't '165; are we making progress on this? There's also the
prime-busy/hang death to worry about. But quieter than it has been for a
long time.

Series pushed, thanks for the patches and review.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-11-08 21:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-03 15:18 [PATCH v4 1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Michal Wajdeczko
2017-11-03 15:18 ` [PATCH v4 2/5] drm/i915/guc: Wait for ucode DMA transfer completion Michal Wajdeczko
2017-11-08 20:36   ` Chris Wilson
2017-11-08 21:17     ` Michal Wajdeczko
2017-11-08 21:26       ` Chris Wilson
2017-11-03 15:18 ` [PATCH v4 3/5] drm/i915/guc: Drop legacy workarounds from guc_prepare_xfer Michal Wajdeczko
2017-11-03 16:17   ` Sagar Arun Kamble
2017-11-03 15:18 ` [PATCH v4 4/5] drm/i915/guc: Simplify programming of GUC_SHIM_CONTROL Michal Wajdeczko
2017-11-03 16:20   ` Sagar Arun Kamble
2017-11-03 15:18 ` [PATCH v4 5/5] HAX enable GuC submission for CI Michal Wajdeczko
2017-11-03 15:52 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/5] drm/i915/guc: Split GuC firmware xfer function into clear steps Patchwork
2017-11-08 21:48   ` Chris Wilson
2017-11-03 18:16 ` ✗ Fi.CI.IGT: warning " Patchwork

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