[PATCH v3 00/14] Firmware code reorg

[PATCH v3 00/14] Firmware code reorg

[Michal Wajdeczko]

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>

v1: was sent with wrong prefix "P v4"
v2: apply review comments from Chris/Daniele
v3: restore DRM_WARN, add firmware url

Michal Wajdeczko (14):
  drm/i915: Move intel_guc_wopcm_size to intel_guc.c
  drm/i915/guc: Move GuC boot param initialization out of xfer
  drm/i915/guc: Small fixups post code move
  drm/i915/guc: Move doc near related definitions
  drm/i915/guc: Rename intel_guc_loader.c to intel_guc_fw.c
  drm/i915/guc: Reorder functions in intel_guc_fw.c
  drm/i915/guc: Move firmware size check out of generic code
  drm/i915/guc: Pick better place for Guc final status message
  drm/i915/uc: Improve debug messages in firmware fetch
  drm/i915/uc: Add message with firmware url
  drm/i915/uc: Unify firmware loading
  drm/i915/guc: Update Guc messages on load failure
  drm/i915/huc: Move fw select function
  HAX enable GuC submission for CI

 drivers/gpu/drm/i915/Makefile           |   2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c     |   8 +-
 drivers/gpu/drm/i915/i915_params.h      |   4 +-
 drivers/gpu/drm/i915/intel_guc.c        | 104 ++++++++
 drivers/gpu/drm/i915/intel_guc.h        |  16 +-
 drivers/gpu/drm/i915/intel_guc_fw.c     | 256 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_fw.h     |  33 +++
 drivers/gpu/drm/i915/intel_guc_fwif.h   |   4 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 417 --------------------------------
 drivers/gpu/drm/i915/intel_huc.c        | 125 ++++------
 drivers/gpu/drm/i915/intel_uc.c         |  20 +-
 drivers/gpu/drm/i915/intel_uc_fw.c      | 164 ++++++++++---
 drivers/gpu/drm/i915/intel_uc_fw.h      |  11 +
 13 files changed, 606 insertions(+), 558 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_fw.c
 create mode 100644 drivers/gpu/drm/i915/intel_guc_fw.h
 delete mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c

-- 
2.7.4

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

[PATCH v3 01/14] drm/i915: Move intel_guc_wopcm_size to intel_guc.c

[Michal Wajdeczko]

Function intel_guc_wopcm_size didn't fit well in the old place.
With this move upcoming cleanup will be simpler.

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

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 9e18c4f..be1c9a7 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -263,3 +263,14 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 	i915_gem_object_put(obj);
 	return vma;
 }
+
+u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
+{
+	u32 wopcm_size = GUC_WOPCM_TOP;
+
+	/* On BXT, the top of WOPCM is reserved for RC6 context */
+	if (IS_GEN9_LP(dev_priv))
+		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+
+	return wopcm_size;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c7a800a..30b70f5 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -250,17 +250,6 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
-{
-	u32 wopcm_size = GUC_WOPCM_TOP;
-
-	/* On BXT, the top of WOPCM is reserved for RC6 context */
-	if (IS_GEN9_LP(dev_priv))
-		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
-
-	return wopcm_size;
-}
-
 /*
  * Load the GuC firmware blob into the MinuteIA.
  */
-- 
2.7.4

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

[PATCH v3 02/14] drm/i915/guc: Move GuC boot param initialization out of xfer

[Michal Wajdeczko]

We want to keep ucode xfer functions separate from other
initialization. Once separated, add explicit forcewake.

v2: use BLITTER domain only and add comment (Daniele)

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #1
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c        | 92 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h        |  1 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 85 ------------------------------
 drivers/gpu/drm/i915/intel_uc.c         |  1 +
 4 files changed, 94 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index be1c9a7..719144a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -67,6 +67,98 @@ void intel_guc_init_early(struct intel_guc *guc)
 	guc->notify = gen8_guc_raise_irq;
 }
 
+static u32 get_gttype(struct drm_i915_private *dev_priv)
+{
+	/* XXX: GT type based on PCI device ID? field seems unused by fw */
+	return 0;
+}
+
+static u32 get_core_family(struct drm_i915_private *dev_priv)
+{
+	u32 gen = INTEL_GEN(dev_priv);
+
+	switch (gen) {
+	case 9:
+		return GUC_CORE_FAMILY_GEN9;
+
+	default:
+		MISSING_CASE(gen);
+		return GUC_CORE_FAMILY_UNKNOWN;
+	}
+}
+
+/*
+ * Initialise the GuC parameter block before starting the firmware
+ * transfer. These parameters are read by the firmware on startup
+ * and cannot be changed thereafter.
+ */
+void intel_guc_init_params(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 params[GUC_CTL_MAX_DWORDS];
+	int i;
+
+	memset(&params, 0, sizeof(params));
+
+	params[GUC_CTL_DEVICE_INFO] |=
+		(get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
+		(get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
+
+	/*
+	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
+	 * second. This ARAR is calculated by:
+	 * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
+	 */
+	params[GUC_CTL_ARAT_HIGH] = 0;
+	params[GUC_CTL_ARAT_LOW] = 100000000;
+
+	params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
+
+	params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
+			GUC_CTL_VCS2_ENABLED;
+
+	params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
+
+	if (i915_modparams.guc_log_level >= 0) {
+		params[GUC_CTL_DEBUG] =
+			i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
+	} else
+		params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
+
+	/* If GuC submission is enabled, set up additional parameters here */
+	if (i915_modparams.enable_guc_submission) {
+		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
+		u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
+		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
+
+		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
+		params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
+
+		pgs >>= PAGE_SHIFT;
+		params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
+			(ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
+
+		params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
+
+		/* Unmask this bit to enable the GuC's internal scheduler */
+		params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
+	}
+
+	/*
+	 * All SOFT_SCRATCH registers are in FORCEWAKE_BLITTER domain and
+	 * they are power context saved so it's ok to release forcewake
+	 * when we are done here and take it again at xfer time.
+	 */
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
+
+	I915_WRITE(SOFT_SCRATCH(0), 0);
+
+	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
+		I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
+
+	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
+}
+
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
 {
 	WARN(1, "Unexpected send: action=%#x\n", *action);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index aa9a7b5..8b44165 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -95,6 +95,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 
 void intel_guc_init_early(struct intel_guc *guc);
 void intel_guc_init_send_regs(struct intel_guc *guc);
+void intel_guc_init_params(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 30b70f5..d9089bc 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -79,89 +79,6 @@ MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
 #define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR)
 
 
-static u32 get_gttype(struct drm_i915_private *dev_priv)
-{
-	/* XXX: GT type based on PCI device ID? field seems unused by fw */
-	return 0;
-}
-
-static u32 get_core_family(struct drm_i915_private *dev_priv)
-{
-	u32 gen = INTEL_GEN(dev_priv);
-
-	switch (gen) {
-	case 9:
-		return GUC_CORE_FAMILY_GEN9;
-
-	default:
-		MISSING_CASE(gen);
-		return GUC_CORE_FAMILY_UNKNOWN;
-	}
-}
-
-/*
- * Initialise the GuC parameter block before starting the firmware
- * transfer. These parameters are read by the firmware on startup
- * and cannot be changed thereafter.
- */
-static void guc_params_init(struct drm_i915_private *dev_priv)
-{
-	struct intel_guc *guc = &dev_priv->guc;
-	u32 params[GUC_CTL_MAX_DWORDS];
-	int i;
-
-	memset(&params, 0, sizeof(params));
-
-	params[GUC_CTL_DEVICE_INFO] |=
-		(get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
-		(get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
-
-	/*
-	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
-	 * second. This ARAR is calculated by:
-	 * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
-	 */
-	params[GUC_CTL_ARAT_HIGH] = 0;
-	params[GUC_CTL_ARAT_LOW] = 100000000;
-
-	params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
-
-	params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
-			GUC_CTL_VCS2_ENABLED;
-
-	params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
-
-	if (i915_modparams.guc_log_level >= 0) {
-		params[GUC_CTL_DEBUG] =
-			i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
-	} else
-		params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
-
-	/* If GuC submission is enabled, set up additional parameters here */
-	if (i915_modparams.enable_guc_submission) {
-		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
-		u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
-		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
-
-		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
-		params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
-
-		pgs >>= PAGE_SHIFT;
-		params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
-			(ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
-
-		params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
-
-		/* Unmask this bit to enable the GuC's internal scheduler */
-		params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
-	}
-
-	I915_WRITE(SOFT_SCRATCH(0), 0);
-
-	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
-		I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
-}
-
 /*
  * Read the GuC status register (GUC_STATUS) and store it in the
  * specified location; then return a boolean indicating whether
@@ -301,8 +218,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 		I915_WRITE(GUC_ARAT_C6DIS, 0x1FF);
 	}
 
-	guc_params_init(dev_priv);
-
 	ret = guc_ucode_xfer_dma(dev_priv, vma);
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 7b938e8..53fdd9a 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -195,6 +195,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 			goto err_submission;
 
 		intel_huc_init_hw(&dev_priv->huc);
+		intel_guc_init_params(guc);
 		ret = intel_guc_init_hw(&dev_priv->guc);
 		if (ret == 0 || ret != -EAGAIN)
 			break;
-- 
2.7.4

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

[PATCH v3 03/14] drm/i915/guc: Small fixups post code move

[Michal Wajdeczko]

Existing code needs some style fixes. To avoid polluting
pure move patch, do it now as separate step.

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>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc.c      | 11 ++++++-----
 drivers/gpu/drm/i915/intel_guc_fwif.h |  4 ++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 719144a..10037c0 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -67,7 +67,7 @@ void intel_guc_init_early(struct intel_guc *guc)
 	guc->notify = gen8_guc_raise_irq;
 }
 
-static u32 get_gttype(struct drm_i915_private *dev_priv)
+static u32 get_gt_type(struct drm_i915_private *dev_priv)
 {
 	/* XXX: GT type based on PCI device ID? field seems unused by fw */
 	return 0;
@@ -98,11 +98,11 @@ void intel_guc_init_params(struct intel_guc *guc)
 	u32 params[GUC_CTL_MAX_DWORDS];
 	int i;
 
-	memset(&params, 0, sizeof(params));
+	memset(params, 0, sizeof(params));
 
 	params[GUC_CTL_DEVICE_INFO] |=
-		(get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
-		(get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
+		(get_gt_type(dev_priv) << GUC_CTL_GT_TYPE_SHIFT) |
+		(get_core_family(dev_priv) << GUC_CTL_CORE_FAMILY_SHIFT);
 
 	/*
 	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
@@ -122,8 +122,9 @@ void intel_guc_init_params(struct intel_guc *guc)
 	if (i915_modparams.guc_log_level >= 0) {
 		params[GUC_CTL_DEBUG] =
 			i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
-	} else
+	} else {
 		params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
+	}
 
 	/* If GuC submission is enabled, set up additional parameters here */
 	if (i915_modparams.enable_guc_submission) {
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 1c0a2a3..80c5074 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -82,8 +82,8 @@
 #define GUC_CTL_ARAT_LOW		2
 
 #define GUC_CTL_DEVICE_INFO		3
-#define   GUC_CTL_GTTYPE_SHIFT		0
-#define   GUC_CTL_COREFAMILY_SHIFT	7
+#define   GUC_CTL_GT_TYPE_SHIFT		0
+#define   GUC_CTL_CORE_FAMILY_SHIFT	7
 
 #define GUC_CTL_LOG_PARAMS		4
 #define   GUC_LOG_VALID			(1 << 0)
-- 
2.7.4

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

[PATCH v3 04/14] drm/i915/guc: Move doc near related definitions

[Michal Wajdeczko]

After Guc code reorg some documentation was left in wrong
place. Move it closer to corresponding definitions.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h        | 11 +++++++++++
 drivers/gpu/drm/i915/intel_guc_loader.c | 23 -----------------------
 drivers/gpu/drm/i915/intel_uc_fw.h      |  5 +++++
 3 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 8b44165..c7088a7 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -33,6 +33,11 @@
 #include "i915_guc_reg.h"
 #include "i915_vma.h"
 
+/*
+ * Top level structure of guc. It handles firmware loading and manages client
+ * pool and doorbells. intel_guc owns a i915_guc_client to replace the legacy
+ * ExecList submission.
+ */
 struct intel_guc {
 	struct intel_uc_fw fw;
 	struct intel_guc_log log;
@@ -83,6 +88,12 @@ 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);
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index d9089bc..8508b94 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -29,29 +29,6 @@
 #include "i915_drv.h"
 #include "intel_uc.h"
 
-/**
- * DOC: GuC-specific firmware loader
- *
- * intel_guc:
- * Top level structure of guc. It handles firmware loading and manages client
- * pool and doorbells. intel_guc owns a i915_guc_client to replace the legacy
- * ExecList submission.
- *
- * Firmware versioning:
- * The firmware build process will generate a version header file with major and
- * minor version defined. The versions are built into CSS header of firmware.
- * i915 kernel driver set the minimal firmware version required per platform.
- * The firmware installation package will install (symbolic link) proper version
- * of firmware.
- *
- * GuC address space:
- * 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.
- *
- */
-
 #define SKL_FW_MAJOR 6
 #define SKL_FW_MINOR 1
 
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index c3e9af4..5c01849 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -50,6 +50,11 @@ struct intel_uc_fw {
 	enum intel_uc_fw_status fetch_status;
 	enum intel_uc_fw_status load_status;
 
+	/*
+	 * The firmware build process will generate a version header file with major and
+	 * minor version defined. The versions are built into CSS header of firmware.
+	 * i915 kernel driver set the minimal firmware version required per platform.
+	 */
 	u16 major_ver_wanted;
 	u16 minor_ver_wanted;
 	u16 major_ver_found;
-- 
2.7.4

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

[PATCH v3 05/14] drm/i915/guc: Rename intel_guc_loader.c to intel_guc_fw.c

[Michal Wajdeczko]

Remaining functions in intel_guc_loader.c were focused around
GuC firmware. Rename them to match object-verb pattern and
rename file itself.

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Makefile           |   2 +-
 drivers/gpu/drm/i915/intel_guc.h        |   4 +-
 drivers/gpu/drm/i915/intel_guc_fw.c     | 298 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_fw.h     |  33 ++++
 drivers/gpu/drm/i915/intel_guc_loader.c | 298 --------------------------------
 drivers/gpu/drm/i915/intel_uc.c         |   4 +-
 6 files changed, 335 insertions(+), 304 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_fw.c
 create mode 100644 drivers/gpu/drm/i915/intel_guc_fw.h
 delete mode 100644 drivers/gpu/drm/i915/intel_guc_loader.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 66d23b6..6c3b048 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -64,7 +64,7 @@ i915-y += intel_uc.o \
 	  intel_guc.o \
 	  intel_guc_ct.o \
 	  intel_guc_log.o \
-	  intel_guc_loader.o \
+	  intel_guc_fw.o \
 	  intel_huc.o \
 	  i915_guc_submission.o
 
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index c7088a7..f643e04 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -26,6 +26,7 @@
 #define _INTEL_GUC_H_
 
 #include "intel_uncore.h"
+#include "intel_guc_fw.h"
 #include "intel_guc_fwif.h"
 #include "intel_guc_ct.h"
 #include "intel_guc_log.h"
@@ -114,9 +115,6 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 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);
-
-int intel_guc_select_fw(struct intel_guc *guc);
-int intel_guc_init_hw(struct intel_guc *guc);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
new file mode 100644
index 0000000..61d6369
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -0,0 +1,298 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Vinit Azad <vinit.azad@intel.com>
+ *    Ben Widawsky <ben@bwidawsk.net>
+ *    Dave Gordon <david.s.gordon@intel.com>
+ *    Alex Dai <yu.dai@intel.com>
+ */
+
+#include "intel_guc_fw.h"
+#include "i915_drv.h"
+
+#define SKL_FW_MAJOR 6
+#define SKL_FW_MINOR 1
+
+#define BXT_FW_MAJOR 8
+#define BXT_FW_MINOR 7
+
+#define KBL_FW_MAJOR 9
+#define KBL_FW_MINOR 14
+
+#define GLK_FW_MAJOR 10
+#define GLK_FW_MINOR 56
+
+#define GUC_FW_PATH(platform, major, minor) \
+       "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin"
+
+#define I915_SKL_GUC_UCODE GUC_FW_PATH(skl, SKL_FW_MAJOR, SKL_FW_MINOR)
+MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
+
+#define I915_BXT_GUC_UCODE GUC_FW_PATH(bxt, BXT_FW_MAJOR, BXT_FW_MINOR)
+MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
+
+#define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, KBL_FW_MINOR)
+MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
+
+#define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR)
+
+/*
+ * 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)
+{
+	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));
+}
+
+/*
+ * Transfer the firmware image to RAM for execution by the microcontroller.
+ *
+ * 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)
+{
+	struct intel_uc_fw *guc_fw = &dev_priv->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 */
+	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
+
+	/* Set the source address for the new blob */
+	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
+	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
+	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
+
+	/*
+	 * Set the DMA destination. Current uCode expects the code to be
+	 * loaded at 8k; locations below this are used for the stack.
+	 */
+	I915_WRITE(DMA_ADDR_1_LOW, 0x2000);
+	I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
+
+	/* Finally start the DMA */
+	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
+
+	/*
+	 * Wait for the DMA to complete & 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);
+
+	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 drm_i915_private *dev_priv)
+{
+	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
+	struct i915_vma *vma;
+	int ret;
+
+	ret = i915_gem_object_set_to_gtt_domain(guc_fw->obj, false);
+	if (ret) {
+		DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
+		return ret;
+	}
+
+	vma = i915_gem_object_ggtt_pin(guc_fw->obj, NULL, 0, 0,
+				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+	if (IS_ERR(vma)) {
+		DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
+		return PTR_ERR(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);
+
+	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);
+	}
+
+	ret = guc_ucode_xfer_dma(dev_priv, vma);
+
+	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+
+	/*
+	 * We keep the object pages for reuse during resume. But we can unpin it
+	 * now that DMA has completed, so it doesn't continue to take up space.
+	 */
+	i915_vma_unpin(vma);
+
+	return ret;
+}
+
+/**
+ * intel_guc_fw_upload() - finish preparing the GuC for activity
+ * @guc: intel_guc structure
+ *
+ * Called during driver loading and also after a GPU reset.
+ *
+ * The main action required here it to load the GuC uCode into the device.
+ * The firmware image should have already been fetched into memory by the
+ * earlier call to intel_guc_init(), so here we need only check that
+ * worked, and then transfer the image to the h/w.
+ *
+ * Return:	non-zero code on error
+ */
+int intel_guc_fw_upload(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	const char *fw_path = guc->fw.path;
+	int ret;
+
+	DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
+		fw_path,
+		intel_uc_fw_status_repr(guc->fw.fetch_status),
+		intel_uc_fw_status_repr(guc->fw.load_status));
+
+	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return -EIO;
+
+	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
+
+	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
+		intel_uc_fw_status_repr(guc->fw.fetch_status),
+		intel_uc_fw_status_repr(guc->fw.load_status));
+
+	ret = guc_ucode_xfer(dev_priv);
+
+	if (ret)
+		return -EAGAIN;
+
+	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
+
+	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
+		 i915_modparams.enable_guc_submission ? "submission enabled" :
+							"loaded",
+		 guc->fw.path,
+		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
+
+	return 0;
+}
+
+/**
+ * intel_guc_fw_select() - selects GuC firmware for loading
+ * @guc:	intel_guc struct
+ *
+ * Return: zero when we know firmware, non-zero in other case
+ */
+int intel_guc_fw_select(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	intel_uc_fw_init(&guc->fw, INTEL_UC_FW_TYPE_GUC);
+
+	if (i915_modparams.guc_firmware_path) {
+		guc->fw.path = i915_modparams.guc_firmware_path;
+		guc->fw.major_ver_wanted = 0;
+		guc->fw.minor_ver_wanted = 0;
+	} else if (IS_SKYLAKE(dev_priv)) {
+		guc->fw.path = I915_SKL_GUC_UCODE;
+		guc->fw.major_ver_wanted = SKL_FW_MAJOR;
+		guc->fw.minor_ver_wanted = SKL_FW_MINOR;
+	} else if (IS_BROXTON(dev_priv)) {
+		guc->fw.path = I915_BXT_GUC_UCODE;
+		guc->fw.major_ver_wanted = BXT_FW_MAJOR;
+		guc->fw.minor_ver_wanted = BXT_FW_MINOR;
+	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
+		guc->fw.path = I915_KBL_GUC_UCODE;
+		guc->fw.major_ver_wanted = KBL_FW_MAJOR;
+		guc->fw.minor_ver_wanted = KBL_FW_MINOR;
+	} else if (IS_GEMINILAKE(dev_priv)) {
+		guc->fw.path = I915_GLK_GUC_UCODE;
+		guc->fw.major_ver_wanted = GLK_FW_MAJOR;
+		guc->fw.minor_ver_wanted = GLK_FW_MINOR;
+	} else {
+		DRM_ERROR("No GuC firmware known for platform with GuC!\n");
+		return -ENOENT;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.h b/drivers/gpu/drm/i915/intel_guc_fw.h
new file mode 100644
index 0000000..023f5ba
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_fw.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _INTEL_GUC_FW_H_
+#define _INTEL_GUC_FW_H_
+
+struct intel_guc;
+
+int intel_guc_fw_select(struct intel_guc *guc);
+int intel_guc_fw_upload(struct intel_guc *guc);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
deleted file mode 100644
index 8508b94..0000000
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ /dev/null
@@ -1,298 +0,0 @@
-/*
- * Copyright © 2014 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- *    Vinit Azad <vinit.azad@intel.com>
- *    Ben Widawsky <ben@bwidawsk.net>
- *    Dave Gordon <david.s.gordon@intel.com>
- *    Alex Dai <yu.dai@intel.com>
- */
-#include "i915_drv.h"
-#include "intel_uc.h"
-
-#define SKL_FW_MAJOR 6
-#define SKL_FW_MINOR 1
-
-#define BXT_FW_MAJOR 8
-#define BXT_FW_MINOR 7
-
-#define KBL_FW_MAJOR 9
-#define KBL_FW_MINOR 14
-
-#define GLK_FW_MAJOR 10
-#define GLK_FW_MINOR 56
-
-#define GUC_FW_PATH(platform, major, minor) \
-       "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin"
-
-#define I915_SKL_GUC_UCODE GUC_FW_PATH(skl, SKL_FW_MAJOR, SKL_FW_MINOR)
-MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
-
-#define I915_BXT_GUC_UCODE GUC_FW_PATH(bxt, BXT_FW_MAJOR, BXT_FW_MINOR)
-MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
-
-#define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, KBL_FW_MINOR)
-MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
-
-#define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR)
-
-
-/*
- * 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)
-{
-	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));
-}
-
-/*
- * Transfer the firmware image to RAM for execution by the microcontroller.
- *
- * 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)
-{
-	struct intel_uc_fw *guc_fw = &dev_priv->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 */
-	I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size);
-
-	/* Set the source address for the new blob */
-	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
-	I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
-	I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
-
-	/*
-	 * Set the DMA destination. Current uCode expects the code to be
-	 * loaded at 8k; locations below this are used for the stack.
-	 */
-	I915_WRITE(DMA_ADDR_1_LOW, 0x2000);
-	I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
-
-	/* Finally start the DMA */
-	I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(UOS_MOVE | START_DMA));
-
-	/*
-	 * Wait for the DMA to complete & 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);
-
-	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 drm_i915_private *dev_priv)
-{
-	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
-	struct i915_vma *vma;
-	int ret;
-
-	ret = i915_gem_object_set_to_gtt_domain(guc_fw->obj, false);
-	if (ret) {
-		DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
-		return ret;
-	}
-
-	vma = i915_gem_object_ggtt_pin(guc_fw->obj, NULL, 0, 0,
-				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-	if (IS_ERR(vma)) {
-		DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
-		return PTR_ERR(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);
-
-	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);
-	}
-
-	ret = guc_ucode_xfer_dma(dev_priv, vma);
-
-	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
-
-	/*
-	 * We keep the object pages for reuse during resume. But we can unpin it
-	 * now that DMA has completed, so it doesn't continue to take up space.
-	 */
-	i915_vma_unpin(vma);
-
-	return ret;
-}
-
-/**
- * intel_guc_init_hw() - finish preparing the GuC for activity
- * @guc: intel_guc structure
- *
- * Called during driver loading and also after a GPU reset.
- *
- * The main action required here it to load the GuC uCode into the device.
- * The firmware image should have already been fetched into memory by the
- * earlier call to intel_guc_init(), so here we need only check that
- * worked, and then transfer the image to the h/w.
- *
- * Return:	non-zero code on error
- */
-int intel_guc_init_hw(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	const char *fw_path = guc->fw.path;
-	int ret;
-
-	DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
-		fw_path,
-		intel_uc_fw_status_repr(guc->fw.fetch_status),
-		intel_uc_fw_status_repr(guc->fw.load_status));
-
-	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return -EIO;
-
-	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
-
-	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
-		intel_uc_fw_status_repr(guc->fw.fetch_status),
-		intel_uc_fw_status_repr(guc->fw.load_status));
-
-	ret = guc_ucode_xfer(dev_priv);
-
-	if (ret)
-		return -EAGAIN;
-
-	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
-
-	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
-		 i915_modparams.enable_guc_submission ? "submission enabled" :
-							"loaded",
-		 guc->fw.path,
-		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
-
-	return 0;
-}
-
-/**
- * intel_guc_select_fw() - selects GuC firmware for loading
- * @guc:	intel_guc struct
- *
- * Return: zero when we know firmware, non-zero in other case
- */
-int intel_guc_select_fw(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-
-	intel_uc_fw_init(&guc->fw, INTEL_UC_FW_TYPE_GUC);
-
-	if (i915_modparams.guc_firmware_path) {
-		guc->fw.path = i915_modparams.guc_firmware_path;
-		guc->fw.major_ver_wanted = 0;
-		guc->fw.minor_ver_wanted = 0;
-	} else if (IS_SKYLAKE(dev_priv)) {
-		guc->fw.path = I915_SKL_GUC_UCODE;
-		guc->fw.major_ver_wanted = SKL_FW_MAJOR;
-		guc->fw.minor_ver_wanted = SKL_FW_MINOR;
-	} else if (IS_BROXTON(dev_priv)) {
-		guc->fw.path = I915_BXT_GUC_UCODE;
-		guc->fw.major_ver_wanted = BXT_FW_MAJOR;
-		guc->fw.minor_ver_wanted = BXT_FW_MINOR;
-	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
-		guc->fw.path = I915_KBL_GUC_UCODE;
-		guc->fw.major_ver_wanted = KBL_FW_MAJOR;
-		guc->fw.minor_ver_wanted = KBL_FW_MINOR;
-	} else if (IS_GEMINILAKE(dev_priv)) {
-		guc->fw.path = I915_GLK_GUC_UCODE;
-		guc->fw.major_ver_wanted = GLK_FW_MAJOR;
-		guc->fw.minor_ver_wanted = GLK_FW_MINOR;
-	} else {
-		DRM_ERROR("No GuC firmware known for platform with GuC!\n");
-		return -ENOENT;
-	}
-
-	return 0;
-}
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 53fdd9a..048f5c4 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -68,7 +68,7 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 		if (HAS_HUC_UCODE(dev_priv))
 			intel_huc_select_fw(&dev_priv->huc);
 
-		if (intel_guc_select_fw(&dev_priv->guc))
+		if (intel_guc_fw_select(&dev_priv->guc))
 			i915_modparams.enable_guc_loading = 0;
 	}
 
@@ -196,7 +196,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 		intel_huc_init_hw(&dev_priv->huc);
 		intel_guc_init_params(guc);
-		ret = intel_guc_init_hw(&dev_priv->guc);
+		ret = intel_guc_fw_upload(guc);
 		if (ret == 0 || ret != -EAGAIN)
 			break;
 
-- 
2.7.4

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

[PATCH v3 06/14] drm/i915/guc: Reorder functions in intel_guc_fw.c

[Michal Wajdeczko]

Functions should be defined in their use order.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc_fw.c | 81 +++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 61d6369..71dacc3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -56,6 +56,47 @@ MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
 
 #define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR)
 
+/**
+ * intel_guc_fw_select() - selects GuC firmware for uploading
+ *
+ * @guc:	intel_guc struct
+ *
+ * Return: zero when we know firmware, non-zero in other case
+ */
+int intel_guc_fw_select(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	intel_uc_fw_init(&guc->fw, INTEL_UC_FW_TYPE_GUC);
+
+	if (i915_modparams.guc_firmware_path) {
+		guc->fw.path = i915_modparams.guc_firmware_path;
+		guc->fw.major_ver_wanted = 0;
+		guc->fw.minor_ver_wanted = 0;
+	} else if (IS_SKYLAKE(dev_priv)) {
+		guc->fw.path = I915_SKL_GUC_UCODE;
+		guc->fw.major_ver_wanted = SKL_FW_MAJOR;
+		guc->fw.minor_ver_wanted = SKL_FW_MINOR;
+	} else if (IS_BROXTON(dev_priv)) {
+		guc->fw.path = I915_BXT_GUC_UCODE;
+		guc->fw.major_ver_wanted = BXT_FW_MAJOR;
+		guc->fw.minor_ver_wanted = BXT_FW_MINOR;
+	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
+		guc->fw.path = I915_KBL_GUC_UCODE;
+		guc->fw.major_ver_wanted = KBL_FW_MAJOR;
+		guc->fw.minor_ver_wanted = KBL_FW_MINOR;
+	} else if (IS_GEMINILAKE(dev_priv)) {
+		guc->fw.path = I915_GLK_GUC_UCODE;
+		guc->fw.major_ver_wanted = GLK_FW_MAJOR;
+		guc->fw.minor_ver_wanted = GLK_FW_MINOR;
+	} else {
+		DRM_ERROR("No GuC firmware known for platform with GuC!\n");
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
 /*
  * Read the GuC status register (GUC_STATUS) and store it in the
  * specified location; then return a boolean indicating whether
@@ -256,43 +297,3 @@ int intel_guc_fw_upload(struct intel_guc *guc)
 
 	return 0;
 }
-
-/**
- * intel_guc_fw_select() - selects GuC firmware for loading
- * @guc:	intel_guc struct
- *
- * Return: zero when we know firmware, non-zero in other case
- */
-int intel_guc_fw_select(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-
-	intel_uc_fw_init(&guc->fw, INTEL_UC_FW_TYPE_GUC);
-
-	if (i915_modparams.guc_firmware_path) {
-		guc->fw.path = i915_modparams.guc_firmware_path;
-		guc->fw.major_ver_wanted = 0;
-		guc->fw.minor_ver_wanted = 0;
-	} else if (IS_SKYLAKE(dev_priv)) {
-		guc->fw.path = I915_SKL_GUC_UCODE;
-		guc->fw.major_ver_wanted = SKL_FW_MAJOR;
-		guc->fw.minor_ver_wanted = SKL_FW_MINOR;
-	} else if (IS_BROXTON(dev_priv)) {
-		guc->fw.path = I915_BXT_GUC_UCODE;
-		guc->fw.major_ver_wanted = BXT_FW_MAJOR;
-		guc->fw.minor_ver_wanted = BXT_FW_MINOR;
-	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
-		guc->fw.path = I915_KBL_GUC_UCODE;
-		guc->fw.major_ver_wanted = KBL_FW_MAJOR;
-		guc->fw.minor_ver_wanted = KBL_FW_MINOR;
-	} else if (IS_GEMINILAKE(dev_priv)) {
-		guc->fw.path = I915_GLK_GUC_UCODE;
-		guc->fw.major_ver_wanted = GLK_FW_MAJOR;
-		guc->fw.minor_ver_wanted = GLK_FW_MINOR;
-	} else {
-		DRM_ERROR("No GuC firmware known for platform with GuC!\n");
-		return -ENOENT;
-	}
-
-	return 0;
-}
-- 
2.7.4

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

[PATCH v3 07/14] drm/i915/guc: Move firmware size check out of generic code

[Michal Wajdeczko]

Checking GuC firmware size can be done in GuC specific code
right before DMA copy as it is unlikely to fail anyway.

v2: rebased

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1
---
 drivers/gpu/drm/i915/intel_guc_fw.c | 7 ++++++-
 drivers/gpu/drm/i915/intel_uc_fw.c  | 8 --------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 71dacc3..020ce26 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -133,6 +133,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
 	unsigned long offset;
 	struct sg_table *sg = vma->pages;
 	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
+	u32 size = guc_fw->header_size + guc_fw->ucode_size;
 	int i, ret = 0;
 
 	/* where RSA signature starts */
@@ -145,7 +146,11 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
 
 	/* 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);
+	if (size > intel_guc_wopcm_size(dev_priv)) {
+		DRM_ERROR("Firmware is too large to fit in WOPCM\n");
+		return -EFBIG;
+	}
+	I915_WRITE(DMA_COPY_SIZE, size);
 
 	/* Set the source address for the new blob */
 	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 766b1cb..482115b 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -108,14 +108,6 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	 */
 	switch (uc_fw->type) {
 	case INTEL_UC_FW_TYPE_GUC:
-		/* Header and uCode will be loaded to WOPCM. Size of the two. */
-		size = uc_fw->header_size + uc_fw->ucode_size;
-
-		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
-		if (size > intel_guc_wopcm_size(dev_priv)) {
-			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
-			goto fail;
-		}
 		uc_fw->major_ver_found = css->guc.sw_version >> 16;
 		uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
 		break;
-- 
2.7.4

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

[PATCH v3 08/14] drm/i915/guc: Pick better place for Guc final status message

[Michal Wajdeczko]

Guc status message printed right after firmware upload may be too
optimistic, as we may fail on subsequent steps. Move that message
to the end of intel_uc_init_hw where we know the status for sure.

v2: use dev_info (Chris)

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

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 020ce26..51be318 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -294,11 +294,5 @@ int intel_guc_fw_upload(struct intel_guc *guc)
 
 	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
 
-	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
-		 i915_modparams.enable_guc_submission ? "submission enabled" :
-							"loaded",
-		 guc->fw.path,
-		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
-
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 048f5c4..9d84fdd 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -222,6 +222,12 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 			goto err_interrupts;
 	}
 
+	dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version %u.%u])\n",
+		 i915_modparams.enable_guc_submission ? "submission enabled" :
+							"loaded",
+		 guc->fw.path,
+		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
+
 	return 0;
 
 	/*
-- 
2.7.4

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

[PATCH v3 09/14] drm/i915/uc: Improve debug messages in firmware fetch

[Michal Wajdeczko]

Time to remove less important info and make messages clear
and consistent.

v2: change some message levels (Chris)
v3: restore DRM_WARN (Chris)

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

diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 482115b..560fe39 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -45,26 +45,33 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	size_t size;
 	int err;
 
+	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
+
 	if (!uc_fw->path)
 		return;
 
 	uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
-
-	DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
+	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
 			 intel_uc_fw_status_repr(uc_fw->fetch_status));
 
 	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
-	if (err)
-		goto fail;
-	if (!fw)
+	if (err) {
+		DRM_DEBUG_DRIVER("%s fw request_firmware err=%d\n",
+				 intel_uc_fw_type_repr(uc_fw->type), err);
 		goto fail;
+	}
 
-	DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n",
-			 uc_fw->path, fw);
+	DRM_DEBUG_DRIVER("%s fw size %zu ptr %p\n",
+			 intel_uc_fw_type_repr(uc_fw->type), fw->size, fw);
 
 	/* Check the size of the blob before examining buffer contents */
 	if (fw->size < sizeof(struct uc_css_header)) {
-		DRM_NOTE("Firmware header is missing\n");
+		DRM_WARN("%s: Unexpected firmware size (%zu, min %zu)\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 fw->size, sizeof(struct uc_css_header));
+		err = -ENODATA;
 		goto fail;
 	}
 
@@ -77,7 +84,9 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 			     sizeof(u32);
 
 	if (uc_fw->header_size != sizeof(struct uc_css_header)) {
-		DRM_NOTE("CSS header definition mismatch\n");
+		DRM_WARN("%s: Mismatched firmware header definition\n",
+			 intel_uc_fw_type_repr(uc_fw->type));
+		err = -ENOEXEC;
 		goto fail;
 	}
 
@@ -87,7 +96,9 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 
 	/* now RSA */
 	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
-		DRM_NOTE("RSA key size is bad\n");
+		DRM_WARN("%s: Mismatched firmware RSA key size (%u)\n",
+			 intel_uc_fw_type_repr(uc_fw->type), css->key_size_dw);
+		err = -ENOEXEC;
 		goto fail;
 	}
 	uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
@@ -96,7 +107,9 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	/* At least, it should have header, uCode and RSA. Size of all three. */
 	size = uc_fw->header_size + uc_fw->ucode_size + uc_fw->rsa_size;
 	if (fw->size < size) {
-		DRM_NOTE("Missing firmware components\n");
+		DRM_WARN("%s: Truncated firmware (%zu, expected %zu)\n",
+			 intel_uc_fw_type_repr(uc_fw->type), fw->size, size);
+		err = -ENOEXEC;
 		goto fail;
 	}
 
@@ -118,17 +131,21 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 		break;
 
 	default:
-		DRM_ERROR("Unknown firmware type %d\n", uc_fw->type);
-		err = -ENOEXEC;
-		goto fail;
+		MISSING_CASE(uc_fw->type);
+		break;
 	}
 
+	DRM_DEBUG_DRIVER("%s fw version %u.%u (wanted %u.%u)\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
+			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
+
 	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
-		DRM_NOTE("Skipping %s firmware version check\n",
+		DRM_NOTE("%s: Skipping firmware version check\n",
 			 intel_uc_fw_type_repr(uc_fw->type));
 	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
 		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		DRM_NOTE("%s firmware version %d.%d, required %d.%d\n",
+		DRM_NOTE("%s: Wrong firmware version (%u.%u, required %u.%u)\n",
 			 intel_uc_fw_type_repr(uc_fw->type),
 			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
 			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
@@ -136,34 +153,34 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 		goto fail;
 	}
 
-	DRM_DEBUG_DRIVER("firmware version %d.%d OK (minimum %d.%d)\n",
-			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-
 	obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
 	if (IS_ERR(obj)) {
 		err = PTR_ERR(obj);
+		DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
+				 intel_uc_fw_type_repr(uc_fw->type), err);
 		goto fail;
 	}
 
 	uc_fw->obj = obj;
 	uc_fw->size = fw->size;
-
-	DRM_DEBUG_DRIVER("uC fw fetch status SUCCESS, obj %p\n",
-			 uc_fw->obj);
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
+	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 intel_uc_fw_status_repr(uc_fw->fetch_status));
 
 	release_firmware(fw);
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
 	return;
 
 fail:
-	DRM_WARN("Failed to fetch valid uC firmware from %s (error %d)\n",
-		 uc_fw->path, err);
-	DRM_DEBUG_DRIVER("uC fw fetch status FAIL; err %d, fw %p, obj %p\n",
-			 err, fw, uc_fw->obj);
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
+	DRM_DEBUG_DRIVER("%s fw fetch %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 intel_uc_fw_status_repr(uc_fw->fetch_status));
+
+	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
+		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
 
 	release_firmware(fw);		/* OK even if fw is NULL */
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
 }
 
 /**
-- 
2.7.4

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

[PATCH v3 10/14] drm/i915/uc: Add message with firmware url

[Michal Wajdeczko]

In case of firmware fetch failure we should help user
find latest firmware.

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>
---
 drivers/gpu/drm/i915/intel_uc_fw.c | 2 ++
 drivers/gpu/drm/i915/intel_uc_fw.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index 560fe39..e246dbd 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -179,6 +179,8 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 
 	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
 		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
+	DRM_INFO("%s: Firmware can be downloaded from %s\n",
+		 intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL);
 
 	release_firmware(fw);		/* OK even if fw is NULL */
 }
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index 5c01849..1d580fa 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -27,6 +27,8 @@
 
 struct drm_i915_private;
 
+#define INTEL_UC_FIRMWARE_URL "https://01.org/linuxgraphics/downloads/firmware"
+
 enum intel_uc_fw_status {
 	INTEL_UC_FIRMWARE_FAIL = -1,
 	INTEL_UC_FIRMWARE_NONE = 0,
-- 
2.7.4

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

[PATCH v3 11/14] drm/i915/uc: Unify firmware loading

[Michal Wajdeczko]

Firmware loading for GuC and Huc are similar. Move common code
into generic function for maximum reuse.

v2: change message levels (Chris)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc_fw.c | 52 +++---------------------
 drivers/gpu/drm/i915/intel_huc.c    | 53 +++----------------------
 drivers/gpu/drm/i915/intel_uc_fw.c  | 79 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc_fw.h  |  4 ++
 4 files changed, 93 insertions(+), 95 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
index 51be318..4629ff3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/intel_guc_fw.c
@@ -193,24 +193,13 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
 /*
  * Load the GuC firmware blob into the MinuteIA.
  */
-static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
+static int guc_ucode_xfer(struct intel_uc_fw *guc_fw, struct i915_vma *vma)
 {
-	struct intel_uc_fw *guc_fw = &dev_priv->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);
 	int ret;
 
-	ret = i915_gem_object_set_to_gtt_domain(guc_fw->obj, false);
-	if (ret) {
-		DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
-		return ret;
-	}
-
-	vma = i915_gem_object_ggtt_pin(guc_fw->obj, NULL, 0, 0,
-				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-	if (IS_ERR(vma)) {
-		DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
-		return PTR_ERR(vma);
-	}
+	GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC);
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
@@ -245,12 +234,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
-	/*
-	 * We keep the object pages for reuse during resume. But we can unpin it
-	 * now that DMA has completed, so it doesn't continue to take up space.
-	 */
-	i915_vma_unpin(vma);
-
 	return ret;
 }
 
@@ -269,30 +252,5 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
  */
 int intel_guc_fw_upload(struct intel_guc *guc)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	const char *fw_path = guc->fw.path;
-	int ret;
-
-	DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
-		fw_path,
-		intel_uc_fw_status_repr(guc->fw.fetch_status),
-		intel_uc_fw_status_repr(guc->fw.load_status));
-
-	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return -EIO;
-
-	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
-
-	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
-		intel_uc_fw_status_repr(guc->fw.fetch_status),
-		intel_uc_fw_status_repr(guc->fw.load_status));
-
-	ret = guc_ucode_xfer(dev_priv);
-
-	if (ret)
-		return -EAGAIN;
-
-	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
-
-	return 0;
+	return intel_uc_fw_upload(&guc->fw, guc_ucode_xfer);
 }
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 4b4cf56..8be3bfe 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -85,26 +85,15 @@ MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
  *
  * Return: 0 on success, non-zero on failure
  */
-static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
+static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
 {
-	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
-	struct i915_vma *vma;
+	struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 	unsigned long offset = 0;
 	u32 size;
 	int ret;
 
-	ret = i915_gem_object_set_to_gtt_domain(huc_fw->obj, false);
-	if (ret) {
-		DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
-		return ret;
-	}
-
-	vma = i915_gem_object_ggtt_pin(huc_fw->obj, NULL, 0, 0,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-	if (IS_ERR(vma)) {
-		DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
-		return PTR_ERR(vma);
-	}
+	GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
@@ -135,12 +124,6 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
 
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
-	/*
-	 * We keep the object pages for reuse during resume. But we can unpin it
-	 * now that DMA has completed, so it doesn't continue to take up space.
-	 */
-	i915_vma_unpin(vma);
-
 	return ret;
 }
 
@@ -194,33 +177,7 @@ void intel_huc_select_fw(struct intel_huc *huc)
  */
 void intel_huc_init_hw(struct intel_huc *huc)
 {
-	struct drm_i915_private *dev_priv = huc_to_i915(huc);
-	int err;
-
-	DRM_DEBUG_DRIVER("%s fw status: fetch %s, load %s\n",
-		huc->fw.path,
-		intel_uc_fw_status_repr(huc->fw.fetch_status),
-		intel_uc_fw_status_repr(huc->fw.load_status));
-
-	if (huc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return;
-
-	huc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
-
-	err = huc_ucode_xfer(dev_priv);
-
-	huc->fw.load_status = err ?
-		INTEL_UC_FIRMWARE_FAIL : INTEL_UC_FIRMWARE_SUCCESS;
-
-	DRM_DEBUG_DRIVER("%s fw status: fetch %s, load %s\n",
-		huc->fw.path,
-		intel_uc_fw_status_repr(huc->fw.fetch_status),
-		intel_uc_fw_status_repr(huc->fw.load_status));
-
-	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
-		DRM_ERROR("Failed to complete HuC uCode load with ret %d\n", err);
-
-	return;
+	intel_uc_fw_upload(&huc->fw, huc_ucode_xfer);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
index e246dbd..7559b82 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -186,6 +186,85 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 }
 
 /**
+ * intel_uc_fw_upload - load uC firmware using custom loader
+ *
+ * @uc_fw: uC firmware
+ * @loader: custom uC firmware loader function
+ *
+ * Loads uC firmware using custom loader and updates internal flags.
+ */
+int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
+		       int (*xfer)(struct intel_uc_fw *uc_fw,
+				   struct i915_vma *vma))
+{
+	struct i915_vma *vma;
+	int err;
+
+	DRM_DEBUG_DRIVER("%s fw load %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path);
+
+	if (uc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return -EIO;
+
+	uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+	DRM_DEBUG_DRIVER("%s fw load %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 intel_uc_fw_status_repr(uc_fw->load_status));
+
+	/* Pin object with firmware */
+	err = i915_gem_object_set_to_gtt_domain(uc_fw->obj, false);
+	if (err) {
+		DRM_DEBUG_DRIVER("%s fw set-domain err=%d\n",
+				 intel_uc_fw_type_repr(uc_fw->type), err);
+		goto fail;
+	}
+
+	vma = i915_gem_object_ggtt_pin(uc_fw->obj, NULL, 0, 0,
+				       PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+	if (IS_ERR(vma)) {
+		err = PTR_ERR(vma);
+		DRM_DEBUG_DRIVER("%s fw ggtt-pin err=%d\n",
+				 intel_uc_fw_type_repr(uc_fw->type), err);
+		goto fail;
+	}
+
+	/* Call custom loader */
+	err = xfer(uc_fw, vma);
+
+	/*
+	 * We keep the object pages for reuse during resume. But we can unpin it
+	 * now that DMA has completed, so it doesn't continue to take up space.
+	 */
+	i915_vma_unpin(vma);
+
+	if (err)
+		goto fail;
+
+	uc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
+	DRM_DEBUG_DRIVER("%s fw load %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 intel_uc_fw_status_repr(uc_fw->load_status));
+
+	DRM_INFO("%s: Loaded firmware %s (version %u.%u)\n",
+		 intel_uc_fw_type_repr(uc_fw->type),
+		 uc_fw->path,
+		 uc_fw->major_ver_found, uc_fw->minor_ver_found);
+
+	return 0;
+
+fail:
+	uc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
+	DRM_DEBUG_DRIVER("%s fw load %s\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 intel_uc_fw_status_repr(uc_fw->load_status));
+
+	DRM_WARN("%s: Failed to load firmware %s (error %d)\n",
+		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
+
+	return err;
+}
+
+/**
  * intel_uc_fw_fini - cleanup uC firmware
  *
  * @uc_fw: uC firmware
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index 1d580fa..78ce556 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -26,6 +26,7 @@
 #define _INTEL_UC_FW_H_
 
 struct drm_i915_private;
+struct i915_vma;
 
 #define INTEL_UC_FIRMWARE_URL "https://01.org/linuxgraphics/downloads/firmware"
 
@@ -109,6 +110,9 @@ void intel_uc_fw_init(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type)
 
 void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 		       struct intel_uc_fw *uc_fw);
+int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
+		       int (*xfer)(struct intel_uc_fw *uc_fw,
+				   struct i915_vma *vma));
 void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
 
 #endif
-- 
2.7.4

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

[PATCH v3 12/14] drm/i915/guc: Update Guc messages on load failure

[Michal Wajdeczko]

In case of GuC firmware loading failure we were reporting
DRM_ERROR also for case when GuC loading was not strictly
required.

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: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_uc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9d84fdd..25bd162 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -250,12 +250,14 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 err_guc:
 	i915_ggtt_disable_guc(dev_priv);
 
-	DRM_ERROR("GuC init failed\n");
 	if (i915_modparams.enable_guc_loading > 1 ||
-	    i915_modparams.enable_guc_submission > 1)
+	    i915_modparams.enable_guc_submission > 1) {
+		DRM_ERROR("GuC init failed. Firmware loading disabled.\n");
 		ret = -EIO;
-	else
+	} else {
+		DRM_NOTE("GuC init failed. Firmware loading disabled.\n");
 		ret = 0;
+	}
 
 	if (i915_modparams.enable_guc_submission) {
 		i915_modparams.enable_guc_submission = 0;
@@ -263,7 +265,6 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	}
 
 	i915_modparams.enable_guc_loading = 0;
-	DRM_NOTE("GuC firmware loading disabled\n");
 
 	return ret;
 }
-- 
2.7.4

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

[PATCH v3 13/14] drm/i915/huc: Move fw select function

[Michal Wajdeczko]

No functional change. Just place the function closer to related
definitions.

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

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 8be3bfe..c8a48cb 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -78,6 +78,42 @@ MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
 	GLK_HUC_FW_MINOR, GLK_BLD_NUM)
 
 /**
+ * intel_huc_select_fw() - selects HuC firmware for loading
+ * @huc:	intel_huc struct
+ */
+void intel_huc_select_fw(struct intel_huc *huc)
+{
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
+
+	intel_uc_fw_init(&huc->fw, INTEL_UC_FW_TYPE_HUC);
+
+	if (i915_modparams.huc_firmware_path) {
+		huc->fw.path = i915_modparams.huc_firmware_path;
+		huc->fw.major_ver_wanted = 0;
+		huc->fw.minor_ver_wanted = 0;
+	} else if (IS_SKYLAKE(dev_priv)) {
+		huc->fw.path = I915_SKL_HUC_UCODE;
+		huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR;
+		huc->fw.minor_ver_wanted = SKL_HUC_FW_MINOR;
+	} else if (IS_BROXTON(dev_priv)) {
+		huc->fw.path = I915_BXT_HUC_UCODE;
+		huc->fw.major_ver_wanted = BXT_HUC_FW_MAJOR;
+		huc->fw.minor_ver_wanted = BXT_HUC_FW_MINOR;
+	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
+		huc->fw.path = I915_KBL_HUC_UCODE;
+		huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR;
+		huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR;
+	} else if (IS_GEMINILAKE(dev_priv)) {
+		huc->fw.path = I915_GLK_HUC_UCODE;
+		huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
+		huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
+	} else {
+		DRM_ERROR("No HuC firmware known for platform with HuC!\n");
+		return;
+	}
+}
+
+/**
  * huc_ucode_xfer() - DMA's the firmware
  * @dev_priv: the drm_i915_private device
  *
@@ -128,42 +164,6 @@ static int huc_ucode_xfer(struct intel_uc_fw *huc_fw, struct i915_vma *vma)
 }
 
 /**
- * intel_huc_select_fw() - selects HuC firmware for loading
- * @huc:	intel_huc struct
- */
-void intel_huc_select_fw(struct intel_huc *huc)
-{
-	struct drm_i915_private *dev_priv = huc_to_i915(huc);
-
-	intel_uc_fw_init(&huc->fw, INTEL_UC_FW_TYPE_HUC);
-
-	if (i915_modparams.huc_firmware_path) {
-		huc->fw.path = i915_modparams.huc_firmware_path;
-		huc->fw.major_ver_wanted = 0;
-		huc->fw.minor_ver_wanted = 0;
-	} else if (IS_SKYLAKE(dev_priv)) {
-		huc->fw.path = I915_SKL_HUC_UCODE;
-		huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR;
-		huc->fw.minor_ver_wanted = SKL_HUC_FW_MINOR;
-	} else if (IS_BROXTON(dev_priv)) {
-		huc->fw.path = I915_BXT_HUC_UCODE;
-		huc->fw.major_ver_wanted = BXT_HUC_FW_MAJOR;
-		huc->fw.minor_ver_wanted = BXT_HUC_FW_MINOR;
-	} else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
-		huc->fw.path = I915_KBL_HUC_UCODE;
-		huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR;
-		huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR;
-	} else if (IS_GEMINILAKE(dev_priv)) {
-		huc->fw.path = I915_GLK_HUC_UCODE;
-		huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
-		huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
-	} else {
-		DRM_ERROR("No HuC firmware known for platform with HuC!\n");
-		return;
-	}
-}
-
-/**
  * intel_huc_init_hw() - load HuC uCode to device
  * @huc: intel_huc structure
  *
-- 
2.7.4

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

[PATCH v3 14/14] HAX enable GuC submission for CI

[Michal Wajdeczko]

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 daba55a..2a38ad9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3568,17 +3568,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..97c0611 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, 2) \
+	param(int, enable_guc_submission, 2) \
 	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

Re: [PATCH v3 10/14] drm/i915/uc: Add message with firmware url

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-12 23:54:43)
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
> index 5c01849..1d580fa 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
> @@ -27,6 +27,8 @@
>  
>  struct drm_i915_private;
>  
> +#define INTEL_UC_FIRMWARE_URL "https://01.org/linuxgraphics/downloads/firmware"

Pull it out from intel_csr.c, say intel_drv.h?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Firmware code reorg (rev2)

[Patchwork]

== Series Details ==

Series: Firmware code reorg (rev2)
URL   : https://patchwork.freedesktop.org/series/31846/
State : failure

== Summary ==

Series 31846v2 Firmware code reorg
https://patchwork.freedesktop.org/api/1.0/series/31846/revisions/2/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> SKIP       (fi-glk-1)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> SKIP       (fi-glk-1)
Test gem_busy:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-hang-default:
                pass       -> SKIP       (fi-glk-1)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-threads:
                pass       -> SKIP       (fi-glk-1)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-1)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-1)
Test gem_ctx_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-files:
                pass       -> SKIP       (fi-glk-1)
Test gem_ctx_exec:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-1)
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-default-heavy:
                pass       -> SKIP       (fi-glk-1)
Test gem_exec_basic:
        Subgroup basic-blt:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-bsd:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-render:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-vebox:
                pass       -> SKIP       (fi-glk-1)
        Subgroup gtt-blt:
                pass       -> SKIP       (fi-glk-1)
        Subgroup gtt-bsd:
                pass       -> SKIP       (fi-glk-1)
        Subgroup gtt-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup gtt-render:
                pass       -> SKIP       (fi-glk-1)
        Subgroup gtt-vebox:
                pass       -> SKIP       (fi-glk-1)
        Subgroup readonly-blt:
                pass       -> SKIP       (fi-glk-1)
        Subgroup readonly-bsd:
                pass       -> SKIP       (fi-glk-1)
        Subgroup readonly-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup readonly-render:
                pass       -> SKIP       (fi-glk-1)
        Subgroup readonly-vebox:
                pass       -> SKIP       (fi-glk-1)
Test gem_exec_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-1)
Test gem_exec_fence:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-wait-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-await-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup await-hang-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup nb-await-default:
                pass       -> SKIP       (fi-glk-1)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-batch-kernel-default-wb:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-uc-pro-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-uc-prw-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-uc-ro-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-uc-rw-default:
                pass       -> SKIP       (fi-glk-1)
        Subgroup basic-uc-set-default:
WARNING: Long output truncated

490bd134d63554815d2ea6ff56e6d32fd50ff472 drm-tip: 2017y-10m-12d-20h-08m-10s UTC integration manifest
9a8805cbd2d8 HAX enable GuC submission for CI
4a5b2aabfbb6 drm/i915/huc: Move fw select function
5e60965598b1 drm/i915/guc: Update Guc messages on load failure
69ec9d58087a drm/i915/uc: Unify firmware loading
da7268f052c1 drm/i915/uc: Add message with firmware url
28711f5a832a drm/i915/uc: Improve debug messages in firmware fetch
4c227b6e4851 drm/i915/guc: Pick better place for Guc final status message
0a3fc4e5fda3 drm/i915/guc: Move firmware size check out of generic code
2f6dd5acae83 drm/i915/guc: Reorder functions in intel_guc_fw.c
cfa5498effbb drm/i915/guc: Rename intel_guc_loader.c to intel_guc_fw.c
b101e15485a9 drm/i915/guc: Move doc near related definitions
f166e4ac781e drm/i915/guc: Small fixups post code move
75a6d77f7a78 drm/i915/guc: Move GuC boot param initialization out of xfer
b0a9e7e131c3 drm/i915: Move intel_guc_wopcm_size to intel_guc.c

== Logs ==

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

Re: [PATCH v3 02/14] drm/i915/guc: Move GuC boot param initialization out of xfer

[Sagar Arun Kamble]

On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
> We want to keep ucode xfer functions separate from other
> initialization. Once separated, add explicit forcewake.
>
> v2: use BLITTER domain only and add comment (Daniele)
>
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #1
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc.c        | 92 +++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_guc.h        |  1 +
>   drivers/gpu/drm/i915/intel_guc_loader.c | 85 ------------------------------
>   drivers/gpu/drm/i915/intel_uc.c         |  1 +
>   4 files changed, 94 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index be1c9a7..719144a 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -67,6 +67,98 @@ void intel_guc_init_early(struct intel_guc *guc)
>   	guc->notify = gen8_guc_raise_irq;
>   }
>   
> +static u32 get_gttype(struct drm_i915_private *dev_priv)
I think renaming this as get_gt_type as suggested would be good.
> +{
> +	/* XXX: GT type based on PCI device ID? field seems unused by fw */
> +	return 0;
> +}
> +
> +static u32 get_core_family(struct drm_i915_private *dev_priv)
> +{
> +	u32 gen = INTEL_GEN(dev_priv);
> +
> +	switch (gen) {
> +	case 9:
> +		return GUC_CORE_FAMILY_GEN9;
> +
> +	default:
> +		MISSING_CASE(gen);
> +		return GUC_CORE_FAMILY_UNKNOWN;
> +	}
> +}
> +
> +/*
> + * Initialise the GuC parameter block before starting the firmware
> + * transfer. These parameters are read by the firmware on startup
> + * and cannot be changed thereafter.
> + */
> +void intel_guc_init_params(struct intel_guc *guc)
> +{
> +	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +	u32 params[GUC_CTL_MAX_DWORDS];
> +	int i;
> +
> +	memset(&params, 0, sizeof(params));
> +
> +	params[GUC_CTL_DEVICE_INFO] |=
> +		(get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
> +		(get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
> +
> +	/*
> +	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
> +	 * second. This ARAR is calculated by:
> +	 * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
> +	 */
> +	params[GUC_CTL_ARAT_HIGH] = 0;
> +	params[GUC_CTL_ARAT_LOW] = 100000000;
> +
> +	params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
> +
> +	params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
> +			GUC_CTL_VCS2_ENABLED;
> +
> +	params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
> +
> +	if (i915_modparams.guc_log_level >= 0) {
> +		params[GUC_CTL_DEBUG] =
> +			i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
> +	} else
> +		params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
Unbalanced {} here. Doing this checkpatch fix now would be good.
> +
> +	/* If GuC submission is enabled, set up additional parameters here */
> +	if (i915_modparams.enable_guc_submission) {
> +		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
> +		u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
> +		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
> +
> +		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
> +		params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
> +
> +		pgs >>= PAGE_SHIFT;
> +		params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
> +			(ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
> +
> +		params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
> +
> +		/* Unmask this bit to enable the GuC's internal scheduler */
> +		params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
> +	}
> +
> +	/*
> +	 * All SOFT_SCRATCH registers are in FORCEWAKE_BLITTER domain and
> +	 * they are power context saved so it's ok to release forcewake
> +	 * when we are done here and take it again at xfer time.
> +	 */
> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
> +
> +	I915_WRITE(SOFT_SCRATCH(0), 0);
> +
> +	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> +		I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
May be for later (since retry is only for Gen9) but will it be good idea 
to keep these params in intel_guc struct and just doing register writes 
during guc_init_hw.
That way possibly we can move the params initialization to 
guc_submission_init (although that is more than submission currently)
> +
> +	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
> +}
> +
>   int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
>   {
>   	WARN(1, "Unexpected send: action=%#x\n", *action);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index aa9a7b5..8b44165 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -95,6 +95,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
>   
>   void intel_guc_init_early(struct intel_guc *guc);
>   void intel_guc_init_send_regs(struct intel_guc *guc);
> +void intel_guc_init_params(struct intel_guc *guc);
>   int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
>   int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
>   int intel_guc_sample_forcewake(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 30b70f5..d9089bc 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -79,89 +79,6 @@ MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
>   #define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR)
>   
>   
> -static u32 get_gttype(struct drm_i915_private *dev_priv)
> -{
> -	/* XXX: GT type based on PCI device ID? field seems unused by fw */
> -	return 0;
> -}
> -
> -static u32 get_core_family(struct drm_i915_private *dev_priv)
> -{
> -	u32 gen = INTEL_GEN(dev_priv);
> -
> -	switch (gen) {
> -	case 9:
> -		return GUC_CORE_FAMILY_GEN9;
> -
> -	default:
> -		MISSING_CASE(gen);
> -		return GUC_CORE_FAMILY_UNKNOWN;
> -	}
> -}
> -
> -/*
> - * Initialise the GuC parameter block before starting the firmware
> - * transfer. These parameters are read by the firmware on startup
> - * and cannot be changed thereafter.
> - */
> -static void guc_params_init(struct drm_i915_private *dev_priv)
> -{
> -	struct intel_guc *guc = &dev_priv->guc;
> -	u32 params[GUC_CTL_MAX_DWORDS];
> -	int i;
> -
> -	memset(&params, 0, sizeof(params));
> -
> -	params[GUC_CTL_DEVICE_INFO] |=
> -		(get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
> -		(get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
> -
> -	/*
> -	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
> -	 * second. This ARAR is calculated by:
> -	 * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
> -	 */
> -	params[GUC_CTL_ARAT_HIGH] = 0;
> -	params[GUC_CTL_ARAT_LOW] = 100000000;
> -
> -	params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
> -
> -	params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
> -			GUC_CTL_VCS2_ENABLED;
> -
> -	params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
> -
> -	if (i915_modparams.guc_log_level >= 0) {
> -		params[GUC_CTL_DEBUG] =
> -			i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
> -	} else
> -		params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
> -
> -	/* If GuC submission is enabled, set up additional parameters here */
> -	if (i915_modparams.enable_guc_submission) {
> -		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
> -		u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
> -		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
> -
> -		params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
> -		params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
> -
> -		pgs >>= PAGE_SHIFT;
> -		params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
> -			(ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
> -
> -		params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
> -
> -		/* Unmask this bit to enable the GuC's internal scheduler */
> -		params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
> -	}
> -
> -	I915_WRITE(SOFT_SCRATCH(0), 0);
> -
> -	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> -		I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> -}
> -
>   /*
>    * Read the GuC status register (GUC_STATUS) and store it in the
>    * specified location; then return a boolean indicating whether
> @@ -301,8 +218,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
>   		I915_WRITE(GUC_ARAT_C6DIS, 0x1FF);
>   	}
>   
> -	guc_params_init(dev_priv);
> -
>   	ret = guc_ucode_xfer_dma(dev_priv, vma);
>   
>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 7b938e8..53fdd9a 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -195,6 +195,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   			goto err_submission;
>   
>   		intel_huc_init_hw(&dev_priv->huc);
> +		intel_guc_init_params(guc);
>   		ret = intel_guc_init_hw(&dev_priv->guc);
>   		if (ret == 0 || ret != -EAGAIN)
>   			break;

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

Re: [PATCH v3 02/14] drm/i915/guc: Move GuC boot param initialization out of xfer

[Sagar Arun Kamble]

Ignore checkpatch related review. Saw the fixes in 03/14


On 10/13/2017 10:43 AM, Sagar Arun Kamble wrote:
>
>
> On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
>> We want to keep ucode xfer functions separate from other
>> initialization. Once separated, add explicit forcewake.
>>
>> v2: use BLITTER domain only and add comment (Daniele)
>>
>> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #1
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc.c        | 92 
>> +++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_guc.h        |  1 +
>>   drivers/gpu/drm/i915/intel_guc_loader.c | 85 
>> ------------------------------
>>   drivers/gpu/drm/i915/intel_uc.c         |  1 +
>>   4 files changed, 94 insertions(+), 85 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index be1c9a7..719144a 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -67,6 +67,98 @@ void intel_guc_init_early(struct intel_guc *guc)
>>       guc->notify = gen8_guc_raise_irq;
>>   }
>>   +static u32 get_gttype(struct drm_i915_private *dev_priv)
> I think renaming this as get_gt_type as suggested would be good.
>> +{
>> +    /* XXX: GT type based on PCI device ID? field seems unused by fw */
>> +    return 0;
>> +}
>> +
>> +static u32 get_core_family(struct drm_i915_private *dev_priv)
>> +{
>> +    u32 gen = INTEL_GEN(dev_priv);
>> +
>> +    switch (gen) {
>> +    case 9:
>> +        return GUC_CORE_FAMILY_GEN9;
>> +
>> +    default:
>> +        MISSING_CASE(gen);
>> +        return GUC_CORE_FAMILY_UNKNOWN;
>> +    }
>> +}
>> +
>> +/*
>> + * Initialise the GuC parameter block before starting the firmware
>> + * transfer. These parameters are read by the firmware on startup
>> + * and cannot be changed thereafter.
>> + */
>> +void intel_guc_init_params(struct intel_guc *guc)
>> +{
>> +    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +    u32 params[GUC_CTL_MAX_DWORDS];
>> +    int i;
>> +
>> +    memset(&params, 0, sizeof(params));
>> +
>> +    params[GUC_CTL_DEVICE_INFO] |=
>> +        (get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
>> +        (get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
>> +
>> +    /*
>> +     * GuC ARAT increment is 10 ns. GuC default scheduler quantum is 
>> one
>> +     * second. This ARAR is calculated by:
>> +     * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
>> +     */
>> +    params[GUC_CTL_ARAT_HIGH] = 0;
>> +    params[GUC_CTL_ARAT_LOW] = 100000000;
>> +
>> +    params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
>> +
>> +    params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
>> +            GUC_CTL_VCS2_ENABLED;
>> +
>> +    params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
>> +
>> +    if (i915_modparams.guc_log_level >= 0) {
>> +        params[GUC_CTL_DEBUG] =
>> +            i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
>> +    } else
>> +        params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
> Unbalanced {} here. Doing this checkpatch fix now would be good.
>> +
>> +    /* If GuC submission is enabled, set up additional parameters 
>> here */
>> +    if (i915_modparams.enable_guc_submission) {
>> +        u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
>> +        u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
>> +        u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>> +
>> +        params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
>> +        params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
>> +
>> +        pgs >>= PAGE_SHIFT;
>> +        params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
>> +            (ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
>> +
>> +        params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
>> +
>> +        /* Unmask this bit to enable the GuC's internal scheduler */
>> +        params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
>> +    }
>> +
>> +    /*
>> +     * All SOFT_SCRATCH registers are in FORCEWAKE_BLITTER domain and
>> +     * they are power context saved so it's ok to release forcewake
>> +     * when we are done here and take it again at xfer time.
>> +     */
>> +    intel_uncore_forcewake_get(dev_priv, FORCEWAKE_BLITTER);
>> +
>> +    I915_WRITE(SOFT_SCRATCH(0), 0);
>> +
>> +    for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
>> +        I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> May be for later (since retry is only for Gen9) but will it be good 
> idea to keep these params in intel_guc struct and just doing register 
> writes during guc_init_hw.
> That way possibly we can move the params initialization to 
> guc_submission_init (although that is more than submission currently)
>> +
>> +    intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
>> +}
>> +
>>   int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, 
>> u32 len)
>>   {
>>       WARN(1, "Unexpected send: action=%#x\n", *action);
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index aa9a7b5..8b44165 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -95,6 +95,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma 
>> *vma)
>>     void intel_guc_init_early(struct intel_guc *guc);
>>   void intel_guc_init_send_regs(struct intel_guc *guc);
>> +void intel_guc_init_params(struct intel_guc *guc);
>>   int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, 
>> u32 len);
>>   int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, 
>> u32 len);
>>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
>> b/drivers/gpu/drm/i915/intel_guc_loader.c
>> index 30b70f5..d9089bc 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> @@ -79,89 +79,6 @@ MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
>>   #define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, 
>> GLK_FW_MINOR)
>>     -static u32 get_gttype(struct drm_i915_private *dev_priv)
>> -{
>> -    /* XXX: GT type based on PCI device ID? field seems unused by fw */
>> -    return 0;
>> -}
>> -
>> -static u32 get_core_family(struct drm_i915_private *dev_priv)
>> -{
>> -    u32 gen = INTEL_GEN(dev_priv);
>> -
>> -    switch (gen) {
>> -    case 9:
>> -        return GUC_CORE_FAMILY_GEN9;
>> -
>> -    default:
>> -        MISSING_CASE(gen);
>> -        return GUC_CORE_FAMILY_UNKNOWN;
>> -    }
>> -}
>> -
>> -/*
>> - * Initialise the GuC parameter block before starting the firmware
>> - * transfer. These parameters are read by the firmware on startup
>> - * and cannot be changed thereafter.
>> - */
>> -static void guc_params_init(struct drm_i915_private *dev_priv)
>> -{
>> -    struct intel_guc *guc = &dev_priv->guc;
>> -    u32 params[GUC_CTL_MAX_DWORDS];
>> -    int i;
>> -
>> -    memset(&params, 0, sizeof(params));
>> -
>> -    params[GUC_CTL_DEVICE_INFO] |=
>> -        (get_gttype(dev_priv) << GUC_CTL_GTTYPE_SHIFT) |
>> -        (get_core_family(dev_priv) << GUC_CTL_COREFAMILY_SHIFT);
>> -
>> -    /*
>> -     * GuC ARAT increment is 10 ns. GuC default scheduler quantum is 
>> one
>> -     * second. This ARAR is calculated by:
>> -     * Scheduler-Quantum-in-ns / ARAT-increment-in-ns = 1000000000 / 10
>> -     */
>> -    params[GUC_CTL_ARAT_HIGH] = 0;
>> -    params[GUC_CTL_ARAT_LOW] = 100000000;
>> -
>> -    params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
>> -
>> -    params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
>> -            GUC_CTL_VCS2_ENABLED;
>> -
>> -    params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
>> -
>> -    if (i915_modparams.guc_log_level >= 0) {
>> -        params[GUC_CTL_DEBUG] =
>> -            i915_modparams.guc_log_level << GUC_LOG_VERBOSITY_SHIFT;
>> -    } else
>> -        params[GUC_CTL_DEBUG] = GUC_LOG_DISABLED;
>> -
>> -    /* If GuC submission is enabled, set up additional parameters 
>> here */
>> -    if (i915_modparams.enable_guc_submission) {
>> -        u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
>> -        u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
>> -        u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>> -
>> -        params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
>> -        params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
>> -
>> -        pgs >>= PAGE_SHIFT;
>> -        params[GUC_CTL_CTXINFO] = (pgs << GUC_CTL_BASE_ADDR_SHIFT) |
>> -            (ctx_in_16 << GUC_CTL_CTXNUM_IN16_SHIFT);
>> -
>> -        params[GUC_CTL_FEATURE] |= GUC_CTL_KERNEL_SUBMISSIONS;
>> -
>> -        /* Unmask this bit to enable the GuC's internal scheduler */
>> -        params[GUC_CTL_FEATURE] &= ~GUC_CTL_DISABLE_SCHEDULER;
>> -    }
>> -
>> -    I915_WRITE(SOFT_SCRATCH(0), 0);
>> -
>> -    for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
>> -        I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
>> -}
>> -
>>   /*
>>    * Read the GuC status register (GUC_STATUS) and store it in the
>>    * specified location; then return a boolean indicating whether
>> @@ -301,8 +218,6 @@ static int guc_ucode_xfer(struct drm_i915_private 
>> *dev_priv)
>>           I915_WRITE(GUC_ARAT_C6DIS, 0x1FF);
>>       }
>>   -    guc_params_init(dev_priv);
>> -
>>       ret = guc_ucode_xfer_dma(dev_priv, vma);
>>         intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 7b938e8..53fdd9a 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -195,6 +195,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>               goto err_submission;
>>             intel_huc_init_hw(&dev_priv->huc);
>> +        intel_guc_init_params(guc);
>>           ret = intel_guc_init_hw(&dev_priv->guc);
>>           if (ret == 0 || ret != -EAGAIN)
>>               break;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v3 04/14] drm/i915/guc: Move doc near related definitions

[Sagar Arun Kamble]

On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
> After Guc code reorg some documentation was left in wrong
> place. Move it closer to corresponding definitions.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 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>
nitpick: Can we use "GuC" or other preferred name consistently in all 
comments/commit message/subject except for tag
for all GuC related changes.
> ---
>   drivers/gpu/drm/i915/intel_guc.h        | 11 +++++++++++
>   drivers/gpu/drm/i915/intel_guc_loader.c | 23 -----------------------
>   drivers/gpu/drm/i915/intel_uc_fw.h      |  5 +++++
>   3 files changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 8b44165..c7088a7 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -33,6 +33,11 @@
>   #include "i915_guc_reg.h"
>   #include "i915_vma.h"
>   
> +/*
> + * Top level structure of guc. It handles firmware loading and manages client
> + * pool and doorbells. intel_guc owns a i915_guc_client to replace the legacy
> + * ExecList submission.
> + */
>   struct intel_guc {
>   	struct intel_uc_fw fw;
>   	struct intel_guc_log log;
> @@ -83,6 +88,12 @@ 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);
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index d9089bc..8508b94 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -29,29 +29,6 @@
>   #include "i915_drv.h"
>   #include "intel_uc.h"
>   
> -/**
> - * DOC: GuC-specific firmware loader
> - *
> - * intel_guc:
> - * Top level structure of guc. It handles firmware loading and manages client
> - * pool and doorbells. intel_guc owns a i915_guc_client to replace the legacy
> - * ExecList submission.
> - *
> - * Firmware versioning:
> - * The firmware build process will generate a version header file with major and
> - * minor version defined. The versions are built into CSS header of firmware.
> - * i915 kernel driver set the minimal firmware version required per platform.
> - * The firmware installation package will install (symbolic link) proper version
> - * of firmware.
> - *
> - * GuC address space:
> - * 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.
> - *
> - */
> -
>   #define SKL_FW_MAJOR 6
>   #define SKL_FW_MINOR 1
>   
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
> index c3e9af4..5c01849 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
> @@ -50,6 +50,11 @@ struct intel_uc_fw {
>   	enum intel_uc_fw_status fetch_status;
>   	enum intel_uc_fw_status load_status;
>   
> +	/*
> +	 * The firmware build process will generate a version header file with major and
> +	 * minor version defined. The versions are built into CSS header of firmware.
> +	 * i915 kernel driver set the minimal firmware version required per platform.
> +	 */
>   	u16 major_ver_wanted;
>   	u16 minor_ver_wanted;
>   	u16 major_ver_found;

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

Re: [PATCH v3 10/14] drm/i915/uc: Add message with firmware url

[Michal Wajdeczko]

On Fri, 13 Oct 2017 01:20:05 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-10-12 23:54:43)
>> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h  
>> b/drivers/gpu/drm/i915/intel_uc_fw.h
>> index 5c01849..1d580fa 100644
>> --- a/drivers/gpu/drm/i915/intel_uc_fw.h
>> +++ b/drivers/gpu/drm/i915/intel_uc_fw.h
>> @@ -27,6 +27,8 @@
>>
>>  struct drm_i915_private;
>>
>> +#define INTEL_UC_FIRMWARE_URL  
>> "https://01.org/linuxgraphics/downloads/firmware"
>
> Pull it out from intel_csr.c, say intel_drv.h?

Well, the secret plan was to update intel_csr.c in separate patch.
Then we could fix message there to be more clear and user friendly,
as today URL is in place where one will expect firmware filename:

    "Failed to load DMC firmware"
    " [" FIRMWARE_URL "],"
    " disabling runtime power management.\n");

And for location of the URL macro, I still prefer intel_uc_fw.h
over intel_drv.h, as the latter one is little overloaded, while
former is still accessible to csr.c and there is a chance to
even reuse more uc_fw functions in csr.c in the future.

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

Re: [PATCH v3 07/14] drm/i915/guc: Move firmware size check out of generic code

[Sagar Arun Kamble]

On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
> Checking GuC firmware size can be done in GuC specific code
> right before DMA copy as it is unlikely to fail anyway.
>
> v2: rebased
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1
> ---
>   drivers/gpu/drm/i915/intel_guc_fw.c | 7 ++++++-
>   drivers/gpu/drm/i915/intel_uc_fw.c  | 8 --------
>   2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
> index 71dacc3..020ce26 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -133,6 +133,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
>   	unsigned long offset;
>   	struct sg_table *sg = vma->pages;
>   	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
> +	u32 size = guc_fw->header_size + guc_fw->ucode_size;
>   	int i, ret = 0;
>   
>   	/* where RSA signature starts */
> @@ -145,7 +146,11 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
>   
>   	/* 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);
> +	if (size > intel_guc_wopcm_size(dev_priv)) {
This size check better be done before RSA copy. With that:
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> +		DRM_ERROR("Firmware is too large to fit in WOPCM\n");
> +		return -EFBIG;
> +	}
> +	I915_WRITE(DMA_COPY_SIZE, size);
>   
>   	/* Set the source address for the new blob */
>   	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 766b1cb..482115b 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -108,14 +108,6 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
>   	 */
>   	switch (uc_fw->type) {
>   	case INTEL_UC_FW_TYPE_GUC:
> -		/* Header and uCode will be loaded to WOPCM. Size of the two. */
> -		size = uc_fw->header_size + uc_fw->ucode_size;
> -
> -		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
> -		if (size > intel_guc_wopcm_size(dev_priv)) {
> -			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
> -			goto fail;
> -		}
>   		uc_fw->major_ver_found = css->guc.sw_version >> 16;
>   		uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
>   		break;

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

Re: [PATCH v3 08/14] drm/i915/guc: Pick better place for Guc final status message

[Sagar Arun Kamble]

On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
> Guc status message printed right after firmware upload may be too
> optimistic, as we may fail on subsequent steps. Move that message
> to the end of intel_uc_init_hw where we know the status for sure.
>
> v2: use dev_info (Chris)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/intel_guc_fw.c | 6 ------
>   drivers/gpu/drm/i915/intel_uc.c     | 6 ++++++
>   2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c
> index 020ce26..51be318 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -294,11 +294,5 @@ int intel_guc_fw_upload(struct intel_guc *guc)
>   
>   	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
>   
> -	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
> -		 i915_modparams.enable_guc_submission ? "submission enabled" :
> -							"loaded",
> -		 guc->fw.path,
> -		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
> -
It would be good to say "GuC loaded (firmware _path_ [version x.x])" here.
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 048f5c4..9d84fdd 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -222,6 +222,12 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   			goto err_interrupts;
>   	}
>   
> +	dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version %u.%u])\n",
> +		 i915_modparams.enable_guc_submission ? "submission enabled" :
> +							"loaded",
> +		 guc->fw.path,
> +		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
> +
And move this print inside i915_guc_submission_enable (And limit to 
"submission enabled"/"submission disabled" as we plan to enable/disable
submission during resume from sleep/reset.
>   	return 0;
>   
>   	/*

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

Re: [PATCH v3 07/14] drm/i915/guc: Move firmware size check out of generic code

[Michal Wajdeczko]

On Fri, 13 Oct 2017 08:49:22 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

>
>
> On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
>> Checking GuC firmware size can be done in GuC specific code
>> right before DMA copy as it is unlikely to fail anyway.
>>
>> v2: rebased
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1
>> ---
>>   drivers/gpu/drm/i915/intel_guc_fw.c | 7 ++++++-
>>   drivers/gpu/drm/i915/intel_uc_fw.c  | 8 --------
>>   2 files changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c  
>> b/drivers/gpu/drm/i915/intel_guc_fw.c
>> index 71dacc3..020ce26 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
>> @@ -133,6 +133,7 @@ static int guc_ucode_xfer_dma(struct  
>> drm_i915_private *dev_priv,
>>   	unsigned long offset;
>>   	struct sg_table *sg = vma->pages;
>>   	u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
>> +	u32 size = guc_fw->header_size + guc_fw->ucode_size;
>>   	int i, ret = 0;
>>     	/* where RSA signature starts */
>> @@ -145,7 +146,11 @@ static int guc_ucode_xfer_dma(struct  
>> drm_i915_private *dev_priv,
>>     	/* 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);
>> +	if (size > intel_guc_wopcm_size(dev_priv)) {
> This size check better be done before RSA copy. With that:
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Hmm, I was planning to separate handling of RSA signature out of
this function and focus here only on DMA xfer, so that move will
be void. And I still think that size check is better to be done
closer to the place where that size will be used in HW commands.

Michal


>> +		DRM_ERROR("Firmware is too large to fit in WOPCM\n");
>> +		return -EFBIG;
>> +	}
>> +	I915_WRITE(DMA_COPY_SIZE, size);
>>     	/* Set the source address for the new blob */
>>   	offset = guc_ggtt_offset(vma) + guc_fw->header_offset;
>> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c  
>> b/drivers/gpu/drm/i915/intel_uc_fw.c
>> index 766b1cb..482115b 100644
>> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
>> @@ -108,14 +108,6 @@ void intel_uc_fw_fetch(struct drm_i915_private  
>> *dev_priv,
>>   	 */
>>   	switch (uc_fw->type) {
>>   	case INTEL_UC_FW_TYPE_GUC:
>> -		/* Header and uCode will be loaded to WOPCM. Size of the two. */
>> -		size = uc_fw->header_size + uc_fw->ucode_size;
>> -
>> -		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
>> -		if (size > intel_guc_wopcm_size(dev_priv)) {
>> -			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
>> -			goto fail;
>> -		}
>>   		uc_fw->major_ver_found = css->guc.sw_version >> 16;
>>   		uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
>>   		break;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 08/14] drm/i915/guc: Pick better place for Guc final status message

[Michal Wajdeczko]

On Fri, 13 Oct 2017 08:54:14 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

>
>
> On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
>> Guc status message printed right after firmware upload may be too
>> optimistic, as we may fail on subsequent steps. Move that message
>> to the end of intel_uc_init_hw where we know the status for sure.
>>
>> v2: use dev_info (Chris)
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/intel_guc_fw.c | 6 ------
>>   drivers/gpu/drm/i915/intel_uc.c     | 6 ++++++
>>   2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c  
>> b/drivers/gpu/drm/i915/intel_guc_fw.c
>> index 020ce26..51be318 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
>> @@ -294,11 +294,5 @@ int intel_guc_fw_upload(struct intel_guc *guc)
>>     	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
>>   -	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
>> -		 i915_modparams.enable_guc_submission ? "submission enabled" :
>> -							"loaded",
>> -		 guc->fw.path,
>> -		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
>> -
> It would be good to say "GuC loaded (firmware _path_ [version x.x])"  
> here.

Note that this message will return in 11/14 as part of intel_uc_fw_upload:

	DRM_INFO("%s: Loaded firmware %s (version %u.%u)\n",
		 intel_uc_fw_type_repr(uc_fw->type),
		 uc_fw->path,
		 uc_fw->major_ver_found, uc_fw->minor_ver_found);

Michal

>>   	return 0;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 048f5c4..9d84fdd 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -222,6 +222,12 @@ int intel_uc_init_hw(struct drm_i915_private  
>> *dev_priv)
>>   			goto err_interrupts;
>>   	}
>>   +	dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version  
>> %u.%u])\n",
>> +		 i915_modparams.enable_guc_submission ? "submission enabled" :
>> +							"loaded",
>> +		 guc->fw.path,
>> +		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
>> +
> And move this print inside i915_guc_submission_enable (And limit to  
> "submission enabled"/"submission disabled" as we plan to enable/disable
> submission during resume from sleep/reset.
>>   	return 0;
>>     	/*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 08/14] drm/i915/guc: Pick better place for Guc final status message

[Michal Wajdeczko]

On Fri, 13 Oct 2017 08:54:14 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

>
>
> On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
>> Guc status message printed right after firmware upload may be too
>> optimistic, as we may fail on subsequent steps. Move that message
>> to the end of intel_uc_init_hw where we know the status for sure.
>>
>> v2: use dev_info (Chris)
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/intel_guc_fw.c | 6 ------
>>   drivers/gpu/drm/i915/intel_uc.c     | 6 ++++++
>>   2 files changed, 6 insertions(+), 6 deletions(-)
>>

...

>> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 048f5c4..9d84fdd 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -222,6 +222,12 @@ int intel_uc_init_hw(struct drm_i915_private  
>> *dev_priv)
>>   			goto err_interrupts;
>>   	}
>>   +	dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version  
>> %u.%u])\n",
>> +		 i915_modparams.enable_guc_submission ? "submission enabled" :
>> +							"loaded",
>> +		 guc->fw.path,
>> +		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
>> +
> And move this print inside i915_guc_submission_enable (And limit to  
> "submission enabled"/"submission disabled" as we plan to enable/disable
> submission during resume from sleep/reset.

So maybe this is better to be done together with related changes?

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

Re: [PATCH v3 08/14] drm/i915/guc: Pick better place for Guc final status message

[Sagar Arun Kamble]

On 10/13/2017 3:44 PM, Michal Wajdeczko wrote:
> On Fri, 13 Oct 2017 08:54:14 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>>
>>
>> On 10/13/2017 4:24 AM, Michal Wajdeczko wrote:
>>> Guc status message printed right after firmware upload may be too
>>> optimistic, as we may fail on subsequent steps. Move that message
>>> to the end of intel_uc_init_hw where we know the status for sure.
>>>
>>> v2: use dev_info (Chris)
>>>
>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>   drivers/gpu/drm/i915/intel_guc_fw.c | 6 ------
>>>   drivers/gpu/drm/i915/intel_uc.c     | 6 ++++++
>>>   2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>
> ...
>
>>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>>> b/drivers/gpu/drm/i915/intel_uc.c
>>> index 048f5c4..9d84fdd 100644
>>> --- a/drivers/gpu/drm/i915/intel_uc.c
>>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>>> @@ -222,6 +222,12 @@ int intel_uc_init_hw(struct drm_i915_private 
>>> *dev_priv)
>>>               goto err_interrupts;
>>>       }
>>>   +    dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version 
>>> %u.%u])\n",
>>> +         i915_modparams.enable_guc_submission ? "submission enabled" :
>>> +                            "loaded",
>>> +         guc->fw.path,
>>> +         guc->fw.major_ver_found, guc->fw.minor_ver_found);
>>> +
>> And move this print inside i915_guc_submission_enable (And limit to 
>> "submission enabled"/"submission disabled" as we plan to enable/disable
>> submission during resume from sleep/reset.
>
> So maybe this is better to be done together with related changes?
Ok. Sure.
>
> Michal

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