[PATCH v9 00/11] GuC Scrub vol. 1

[PATCH v9 00/11] GuC Scrub vol. 1

[Arkadiusz Hiler]

Reasoning
=========

General GuC/HuC cleanup simplifying logic and moving chunks around as the area
got pretty rusty.

This is the first part of effort to clean it up.

A lot of logic were extracted from intel_guc_load() to other functions - it did
not only handle the actual loading but had WA implementations and the code
that enabled submission baked into it.

Param sanitization and firmware selection are also extracted and streamlined.


Naming:
=======

I try to adhere to subject_verb_object naming and parameters.

e.g. for intel_guc_init_fw:

* intel_guc is the subject, it determines the first argument taken
* init      is the verb
* fw        is the object

intel_guc_ functions take intel_guc struct pointer
intel_huc_ functions take intel_guc struct pointer

There's no `struct intel_uc`, so this family of functions take `dev_priv`.


New structure looks like this:
==============================

1. sanitize params - module params + selecting firmware happens at this stage
   * this is done mostly in patch 8, and 9
   * some cleanups happen along th way (e.g. pushing check ups, and changeing
     prototypes)
   * patch 10 introduces new modules params to overload used firmware

2. init_fw - requesting and initial parsing of firmware
   * firmware is read and parsed, we also check for versions to match

3. init_hw - firmware is loaded into hardware and hardware is initalized
   * uc_init_hw has now all logic when it comes to retires and resetting fw to
     vanilla state
   * huc_init_hw and guc_init_hw no longer care about each other and
     submission


v2: rebase after HuC merge + feedback
v3: even more renaming that aims to make things more semantic
v4: some naming improvements, some bikeshedding
v5: coding style, some cleanup
    module params for huc and guc firmware path,
    separate fw select step from actual prepare
v6: feedback + pushed a couple of patches with r-b down the stack
v7: reorder, rename, rebase
v8: rebase, onion teardown, naming, added GuC enablement HAX
v9: rebase for rerun

Arkadiusz Hiler (11):
  drm/i915/uc: Drop superfluous externs in intel_uc.h
  drm/i915/huc: Add huc_to_i915
  drm/i915/uc: Rename intel_?uc_{setup,load}() to _init_hw()
  drm/i915/uc: Move intel_uc_fw_fetch() to intel_uc.c
  drm/i915/uc: Introduce intel_uc_init_fw()
  drm/i915/guc: Extract param logic form guc_init_fw()
  drm/i915/guc: Simplify intel_guc_init_hw()
  drm/i915/uc: Simplify firmware path handling
  drm/i915/uc: Separate firmware selection and preparation
  drm/i915/uc: Add params for specifying firmware
  HAX enable GuC for CI

 drivers/gpu/drm/i915/i915_drv.c         |   5 +-
 drivers/gpu/drm/i915/i915_drv.h         |   5 +
 drivers/gpu/drm/i915/i915_gem.c         |   2 +-
 drivers/gpu/drm/i915/i915_params.c      |  14 +-
 drivers/gpu/drm/i915/i915_params.h      |   2 +
 drivers/gpu/drm/i915/intel_guc_loader.c | 379 ++++----------------------------
 drivers/gpu/drm/i915/intel_huc.c        | 109 ++++-----
 drivers/gpu/drm/i915/intel_uc.c         | 284 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h         |  23 +-
 9 files changed, 413 insertions(+), 410 deletions(-)

-- 
2.9.3

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

[PATCH 01/11] drm/i915/uc: Drop superfluous externs in intel_uc.h

[Arkadiusz Hiler]

Externs are implicit and we generally try to avoid them.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uc.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index d74f4d3..bf72342 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -189,12 +189,12 @@ int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 
 /* intel_guc_loader.c */
-extern void intel_guc_init(struct drm_i915_private *dev_priv);
-extern int intel_guc_setup(struct drm_i915_private *dev_priv);
-extern void intel_guc_fini(struct drm_i915_private *dev_priv);
-extern const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
-extern int intel_guc_suspend(struct drm_i915_private *dev_priv);
-extern int intel_guc_resume(struct drm_i915_private *dev_priv);
+void intel_guc_init(struct drm_i915_private *dev_priv);
+int intel_guc_setup(struct drm_i915_private *dev_priv);
+void intel_guc_fini(struct drm_i915_private *dev_priv);
+const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
+int intel_guc_suspend(struct drm_i915_private *dev_priv);
+int intel_guc_resume(struct drm_i915_private *dev_priv);
 void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
 	struct intel_uc_fw *uc_fw);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
-- 
2.9.3

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

[PATCH 02/11] drm/i915/huc: Add huc_to_i915

[Arkadiusz Hiler]

Used to obtain "dev_priv" from huc struct pointer.
We already have similar thing for guc.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48ff648..5156bcc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2541,6 +2541,11 @@ static inline struct drm_i915_private *guc_to_i915(struct intel_guc *guc)
 	return container_of(guc, struct drm_i915_private, guc);
 }
 
+static inline struct drm_i915_private *huc_to_i915(struct intel_huc *huc)
+{
+	return container_of(huc, struct drm_i915_private, huc);
+}
+
 /* Simple iterator over all initialised engines */
 #define for_each_engine(engine__, dev_priv__, id__) \
 	for ((id__) = 0; \
-- 
2.9.3

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

[PATCH 03/11] drm/i915/uc: Rename intel_?uc_{setup, load}() to _init_hw()

[Arkadiusz Hiler]

GuC historically has two "startup" functions called _init() and _setup()

Then HuC came with it's _init() and _load().

This commit renames intel_guc_setup() and intel_huc_load() to
*uc_init_hw() as they called from the i915_gem_init_hw().

The aim is to be consistent in that entry points called during
particular driver init phases (e.g. init_hw) are all suffixed by that
phase. When reading the leaf functions, it should be clear at what stage
during the driver load it is called and therefore what operations are
legal at that point.

Also, since the functions start with intel_guc and intel_huc they take
appropiate structure.

v2: commit message update (Chris Wilson)
v3: change taken parameters to be more "semantic" (M. Wajdeczko)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  2 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 44 ++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_huc.c        | 44 ++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_uc.h         |  4 +--
 4 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 202bb85..5762c9b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4541,7 +4541,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	intel_mocs_init_l3cc_table(dev_priv);
 
 	/* We can't enable contexts until all firmware is loaded */
-	ret = intel_guc_setup(dev_priv);
+	ret = intel_guc_init_hw(&dev_priv->guc);
 	if (ret)
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 759ab34..d89866f 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -364,28 +364,28 @@ static int guc_hw_reset(struct drm_i915_private *dev_priv)
 }
 
 /**
- * intel_guc_setup() - finish preparing the GuC for activity
- * @dev_priv:	i915 device private
+ * intel_guc_init_hw() - finish preparing the GuC for activity
+ * @guc: intel_guc structure
  *
- * Called from gem_init_hw() during driver loading and also after a GPU reset.
+ * 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.
+ * 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_setup(struct drm_i915_private *dev_priv)
+int intel_guc_init_hw(struct intel_guc *guc)
 {
-	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
-	const char *fw_path = guc_fw->path;
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	const char *fw_path = guc->fw.path;
 	int retries, ret, err;
 
 	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));
+		intel_uc_fw_status_repr(guc->fw.fetch_status),
+		intel_uc_fw_status_repr(guc->fw.load_status));
 
 	/* Loading forbidden, or no firmware to load? */
 	if (!i915.enable_guc_loading) {
@@ -403,10 +403,10 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
 	}
 
 	/* Fetch failed, or already fetched but failed to load? */
-	if (guc_fw->fetch_status != INTEL_UC_FIRMWARE_SUCCESS) {
+	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) {
 		err = -EIO;
 		goto fail;
-	} else if (guc_fw->load_status == INTEL_UC_FIRMWARE_FAIL) {
+	} else if (guc->fw.load_status == INTEL_UC_FIRMWARE_FAIL) {
 		err = -ENOEXEC;
 		goto fail;
 	}
@@ -416,11 +416,11 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
 	/* We need to notify the guc whenever we change the GGTT */
 	i915_ggtt_enable_guc(dev_priv);
 
-	guc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+	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));
+		intel_uc_fw_status_repr(guc->fw.fetch_status),
+		intel_uc_fw_status_repr(guc->fw.load_status));
 
 	err = i915_guc_submission_init(dev_priv);
 	if (err)
@@ -441,7 +441,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
 		if (err)
 			goto fail;
 
-		intel_huc_load(dev_priv);
+		intel_huc_init_hw(&dev_priv->huc);
 		err = guc_ucode_xfer(dev_priv);
 		if (!err)
 			break;
@@ -453,7 +453,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
 			 "retry %d more time(s)\n", err, retries);
 	}
 
-	guc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
+	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
 
 	intel_guc_auth_huc(dev_priv);
 
@@ -468,14 +468,14 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
 
 	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
 		 i915.enable_guc_submission ? "submission enabled" : "loaded",
-		 guc_fw->path,
-		 guc_fw->major_ver_found, guc_fw->minor_ver_found);
+		 guc->fw.path,
+		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
 
 	return 0;
 
 fail:
-	if (guc_fw->load_status == INTEL_UC_FIRMWARE_PENDING)
-		guc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
+	if (guc->fw.load_status == INTEL_UC_FIRMWARE_PENDING)
+		guc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
 
 	i915_guc_submission_disable(dev_priv);
 	i915_guc_submission_fini(dev_priv);
@@ -662,7 +662,7 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
  * Called early during driver load, but after GEM is initialised.
  *
  * The firmware will be transferred to the GuC's memory later,
- * when intel_guc_setup() is called.
+ * when intel_guc_init_hw() is called.
  */
 void intel_guc_init(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index e660109..e42021d 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -150,7 +150,7 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
  * is not capable or driver yet support it. And there will be no error message
  * for INTEL_UC_FIRMWARE_NONE cases.
  *
- * The DMA-copying to HW is done later when intel_huc_load() is called.
+ * The DMA-copying to HW is done later when intel_huc_init_hw() is called.
  */
 void intel_huc_init(struct drm_i915_private *dev_priv)
 {
@@ -193,8 +193,8 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
 }
 
 /**
- * intel_huc_load() - load HuC uCode to device
- * @dev_priv: the drm_i915_private device
+ * intel_huc_init_hw() - load HuC uCode to device
+ * @huc: intel_huc structure
  *
  * Called from guc_setup() during driver loading and also after a GPU reset.
  * Be note that HuC loading must be done before GuC loading.
@@ -205,26 +205,26 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
  *
  * Return:	non-zero code on error
  */
-int intel_huc_load(struct drm_i915_private *dev_priv)
+int intel_huc_init_hw(struct intel_huc *huc)
 {
-	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 	int err;
 
-	if (huc_fw->fetch_status == INTEL_UC_FIRMWARE_NONE)
+	if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_NONE)
 		return 0;
 
 	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));
+		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 &&
-	    huc_fw->load_status == INTEL_UC_FIRMWARE_FAIL)
+	if (huc->fw.fetch_status == INTEL_UC_FIRMWARE_SUCCESS &&
+	    huc->fw.load_status == INTEL_UC_FIRMWARE_FAIL)
 		return -ENOEXEC;
 
-	huc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+	huc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
 
-	switch (huc_fw->fetch_status) {
+	switch (huc->fw.fetch_status) {
 	case INTEL_UC_FIRMWARE_FAIL:
 		/* something went wrong :( */
 		err = -EIO;
@@ -235,9 +235,9 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
 	default:
 		/* "can't happen" */
 		WARN_ONCE(1, "HuC fw %s invalid fetch_status %s [%d]\n",
-			huc_fw->path,
-			intel_uc_fw_status_repr(huc_fw->fetch_status),
-			huc_fw->fetch_status);
+			huc->fw.path,
+			intel_uc_fw_status_repr(huc->fw.fetch_status),
+			huc->fw.fetch_status);
 		err = -ENXIO;
 		goto fail;
 
@@ -249,18 +249,18 @@ int intel_huc_load(struct drm_i915_private *dev_priv)
 	if (err)
 		goto fail;
 
-	huc_fw->load_status = INTEL_UC_FIRMWARE_SUCCESS;
+	huc->fw.load_status = 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));
+		huc->fw.path,
+		intel_uc_fw_status_repr(huc->fw.fetch_status),
+		intel_uc_fw_status_repr(huc->fw.load_status));
 
 	return 0;
 
 fail:
-	if (huc_fw->load_status == INTEL_UC_FIRMWARE_PENDING)
-		huc_fw->load_status = INTEL_UC_FIRMWARE_FAIL;
+	if (huc->fw.load_status == INTEL_UC_FIRMWARE_PENDING)
+		huc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
 
 	DRM_ERROR("Failed to complete HuC uCode load with ret %d\n", err);
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index bf72342..7b90414 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -190,7 +190,7 @@ int intel_guc_sample_forcewake(struct intel_guc *guc);
 
 /* intel_guc_loader.c */
 void intel_guc_init(struct drm_i915_private *dev_priv);
-int intel_guc_setup(struct drm_i915_private *dev_priv);
+int intel_guc_init_hw(struct intel_guc *guc);
 void intel_guc_fini(struct drm_i915_private *dev_priv);
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
 int intel_guc_suspend(struct drm_i915_private *dev_priv);
@@ -225,7 +225,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 /* intel_huc.c */
 void intel_huc_init(struct drm_i915_private *dev_priv);
 void intel_huc_fini(struct drm_i915_private  *dev_priv);
-int intel_huc_load(struct drm_i915_private *dev_priv);
+int intel_huc_init_hw(struct intel_huc *huc);
 void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
 
 #endif
-- 
2.9.3

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

[PATCH 04/11] drm/i915/uc: Move intel_uc_fw_fetch() to intel_uc.c

[Arkadiusz Hiler]

The file fits better.

Additionally rename it to intel_uc_prepare_fw(), as the function does
more than simple fetch.

`obj` cleanup in the function is also fixed (i.e. removed). In the fail
scenario it was always 'put' but there's no possible flow that
initializes the obj properly and then goes to the fail label.

v2: remove second declaration, reorder (M. Wajdeczko)
v3: non-trivial rebase
v4: remove obj cleanup in the fail scenario (C. Wilson)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 137 +-------------------------------
 drivers/gpu/drm/i915/intel_huc.c        |   2 +-
 drivers/gpu/drm/i915/intel_uc.c         | 131 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h         |   4 +-
 4 files changed, 135 insertions(+), 139 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index d89866f..383311b 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -26,7 +26,6 @@
  *    Dave Gordon <david.s.gordon@intel.com>
  *    Alex Dai <yu.dai@intel.com>
  */
-#include <linux/firmware.h>
 #include "i915_drv.h"
 #include "intel_uc.h"
 
@@ -520,140 +519,6 @@ int intel_guc_init_hw(struct intel_guc *guc)
 	return ret;
 }
 
-void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
-			 struct intel_uc_fw *uc_fw)
-{
-	struct pci_dev *pdev = dev_priv->drm.pdev;
-	struct drm_i915_gem_object *obj;
-	const struct firmware *fw = NULL;
-	struct uc_css_header *css;
-	size_t size;
-	int err;
-
-	DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
-		intel_uc_fw_status_repr(uc_fw->fetch_status));
-
-	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
-	if (err)
-		goto fail;
-	if (!fw)
-		goto fail;
-
-	DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n",
-		uc_fw->path, 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");
-		goto fail;
-	}
-
-	css = (struct uc_css_header *)fw->data;
-
-	/* Firmware bits always start from header */
-	uc_fw->header_offset = 0;
-	uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
-		css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
-
-	if (uc_fw->header_size != sizeof(struct uc_css_header)) {
-		DRM_NOTE("CSS header definition mismatch\n");
-		goto fail;
-	}
-
-	/* then, uCode */
-	uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
-	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
-
-	/* now RSA */
-	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
-		DRM_NOTE("RSA key size is bad\n");
-		goto fail;
-	}
-	uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
-	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
-
-	/* 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");
-		goto fail;
-	}
-
-	/*
-	 * The GuC firmware image has the version number embedded at a well-known
-	 * offset within the firmware blob; note that major / minor version are
-	 * TWO bytes each (i.e. u16), although all pointers and offsets are defined
-	 * in terms of bytes (u8).
-	 */
-	switch (uc_fw->fw) {
-	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;
-
-	case INTEL_UC_FW_TYPE_HUC:
-		uc_fw->major_ver_found = css->huc.sw_version >> 16;
-		uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF;
-		break;
-
-	default:
-		DRM_ERROR("Unknown firmware type %d\n", uc_fw->fw);
-		err = -ENOEXEC;
-		goto fail;
-	}
-
-	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
-	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		DRM_NOTE("uC firmware version %d.%d, required %d.%d\n",
-			uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-		err = -ENOEXEC;
-		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);
-
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-	if (IS_ERR_OR_NULL(obj)) {
-		err = obj ? PTR_ERR(obj) : -ENOMEM;
-		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);
-
-	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);
-
-	obj = fetch_and_zero(&uc_fw->obj);
-	if (obj)
-		i915_gem_object_put(obj);
-
-	release_firmware(fw);		/* OK even if fw is NULL */
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
-}
 
 /**
  * intel_guc_init() - define parameters and fetch firmware
@@ -712,7 +577,7 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
 
 	guc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
 	DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path);
-	intel_uc_fw_fetch(dev_priv, guc_fw);
+	intel_uc_prepare_fw(dev_priv, guc_fw);
 	/* status must now be FAIL or SUCCESS */
 }
 
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index e42021d..36326ca 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -189,7 +189,7 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
 
 	DRM_DEBUG_DRIVER("HuC firmware pending, path %s\n", fw_path);
 
-	intel_uc_fw_fetch(dev_priv, huc_fw);
+	intel_uc_prepare_fw(dev_priv, huc_fw);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c46bc85..2ea8a2c 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -24,6 +24,7 @@
 
 #include "i915_drv.h"
 #include "intel_uc.h"
+#include <linux/firmware.h>
 
 void intel_uc_init_early(struct drm_i915_private *dev_priv)
 {
@@ -114,3 +115,133 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
+void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
+			 struct intel_uc_fw *uc_fw)
+{
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+	struct drm_i915_gem_object *obj;
+	const struct firmware *fw = NULL;
+	struct uc_css_header *css;
+	size_t size;
+	int err;
+
+	DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
+		intel_uc_fw_status_repr(uc_fw->fetch_status));
+
+	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
+	if (err)
+		goto fail;
+	if (!fw)
+		goto fail;
+
+	DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n",
+		uc_fw->path, 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");
+		goto fail;
+	}
+
+	css = (struct uc_css_header *)fw->data;
+
+	/* Firmware bits always start from header */
+	uc_fw->header_offset = 0;
+	uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
+		css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
+
+	if (uc_fw->header_size != sizeof(struct uc_css_header)) {
+		DRM_NOTE("CSS header definition mismatch\n");
+		goto fail;
+	}
+
+	/* then, uCode */
+	uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
+	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
+
+	/* now RSA */
+	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
+		DRM_NOTE("RSA key size is bad\n");
+		goto fail;
+	}
+	uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
+	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
+
+	/* 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");
+		goto fail;
+	}
+
+	/*
+	 * The GuC firmware image has the version number embedded at a
+	 * well-known offset within the firmware blob; note that major / minor
+	 * version are TWO bytes each (i.e. u16), although all pointers and
+	 * offsets are defined in terms of bytes (u8).
+	 */
+	switch (uc_fw->fw) {
+	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;
+
+	case INTEL_UC_FW_TYPE_HUC:
+		uc_fw->major_ver_found = css->huc.sw_version >> 16;
+		uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF;
+		break;
+
+	default:
+		DRM_ERROR("Unknown firmware type %d\n", uc_fw->fw);
+		err = -ENOEXEC;
+		goto fail;
+	}
+
+	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
+	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
+		DRM_NOTE("uC firmware version %d.%d, required %d.%d\n",
+			uc_fw->major_ver_found, uc_fw->minor_ver_found,
+			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
+		err = -ENOEXEC;
+		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);
+
+	mutex_lock(&dev_priv->drm.struct_mutex);
+	obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+	if (IS_ERR_OR_NULL(obj)) {
+		err = obj ? PTR_ERR(obj) : -ENOMEM;
+		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);
+
+	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);
+
+	release_firmware(fw);		/* OK even if fw is NULL */
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 7b90414..9509432 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -185,6 +185,8 @@ struct intel_huc {
 
 /* intel_uc.c */
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
+void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
+			 struct intel_uc_fw *uc_fw);
 int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 
@@ -195,8 +197,6 @@ void intel_guc_fini(struct drm_i915_private *dev_priv);
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
 int intel_guc_suspend(struct drm_i915_private *dev_priv);
 int intel_guc_resume(struct drm_i915_private *dev_priv);
-void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
-	struct intel_uc_fw *uc_fw);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
 /* i915_guc_submission.c */
-- 
2.9.3

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

[PATCH 05/11] drm/i915/uc: Introduce intel_uc_init_fw()

[Arkadiusz Hiler]

Instead of calling intel_guc_init() and intel_huc_init() one by one this
patch introduces intel_uc_init_fw() function that calls them both.

Called functions are renamed accordingly.

Trying to have subject_verb_object ordering and more descriptive names,
the intel_huc_init() and intel_guc_init() functions are renamed.

For guc_init():
 * `intel_guc` is the subject, so those functions now take intel_guc
   structure, instead of the dev_priv
 * init is the verb
 * fw is the object which better describes the function's role

huc_init() change follows the same reasoning.

v2: settle on intel_uc_fetch_fw name (M. Wajdeczko)
v3: yet another rename - intel_uc_init_fw (J. Lahtinen)
v4: non-trivial rebase

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  3 +--
 drivers/gpu/drm/i915/intel_guc_loader.c | 30 ++++++++++++-------------
 drivers/gpu/drm/i915/intel_huc.c        | 39 +++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_uc.c         |  6 +++++
 drivers/gpu/drm/i915/intel_uc.h         |  5 +++--
 5 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5810604..0fbdcb0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -605,8 +605,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		goto cleanup_irq;
 
-	intel_huc_init(dev_priv);
-	intel_guc_init(dev_priv);
+	intel_uc_init_fw(dev_priv);
 
 	ret = i915_gem_init(dev_priv);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 383311b..3a37d14 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -521,17 +521,17 @@ int intel_guc_init_hw(struct intel_guc *guc)
 
 
 /**
- * intel_guc_init() - define parameters and fetch firmware
- * @dev_priv:	i915 device private
+ * intel_guc_init_fw() - select and prepare firmware for loading
+ * @guc:	intel_guc struct
  *
  * Called early during driver load, but after GEM is initialised.
  *
  * The firmware will be transferred to the GuC's memory later,
  * when intel_guc_init_hw() is called.
  */
-void intel_guc_init(struct drm_i915_private *dev_priv)
+void intel_guc_init_fw(struct intel_guc *guc)
 {
-	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	const char *fw_path;
 
 	if (!HAS_GUC(dev_priv)) {
@@ -549,23 +549,23 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
 		fw_path = NULL;
 	} else if (IS_SKYLAKE(dev_priv)) {
 		fw_path = I915_SKL_GUC_UCODE;
-		guc_fw->major_ver_wanted = SKL_FW_MAJOR;
-		guc_fw->minor_ver_wanted = SKL_FW_MINOR;
+		guc->fw.major_ver_wanted = SKL_FW_MAJOR;
+		guc->fw.minor_ver_wanted = SKL_FW_MINOR;
 	} else if (IS_BROXTON(dev_priv)) {
 		fw_path = I915_BXT_GUC_UCODE;
-		guc_fw->major_ver_wanted = BXT_FW_MAJOR;
-		guc_fw->minor_ver_wanted = BXT_FW_MINOR;
+		guc->fw.major_ver_wanted = BXT_FW_MAJOR;
+		guc->fw.minor_ver_wanted = BXT_FW_MINOR;
 	} else if (IS_KABYLAKE(dev_priv)) {
 		fw_path = I915_KBL_GUC_UCODE;
-		guc_fw->major_ver_wanted = KBL_FW_MAJOR;
-		guc_fw->minor_ver_wanted = KBL_FW_MINOR;
+		guc->fw.major_ver_wanted = KBL_FW_MAJOR;
+		guc->fw.minor_ver_wanted = KBL_FW_MINOR;
 	} else {
 		fw_path = "";	/* unknown device */
 	}
 
-	guc_fw->path = fw_path;
-	guc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
-	guc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
+	guc->fw.path = fw_path;
+	guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE;
+	guc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
 
 	/* Early (and silent) return if GuC loading is disabled */
 	if (!i915.enable_guc_loading)
@@ -575,9 +575,9 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
 	if (*fw_path == '\0')
 		return;
 
-	guc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
+	guc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING;
 	DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path);
-	intel_uc_prepare_fw(dev_priv, guc_fw);
+	intel_uc_prepare_fw(dev_priv, &guc->fw);
 	/* status must now be FAIL or SUCCESS */
 }
 
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 36326ca..fda473d 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -141,8 +141,8 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
 }
 
 /**
- * intel_huc_init() - initiate HuC firmware loading request
- * @dev_priv: the drm_i915_private device
+ * intel_huc_init_fw() - select and prepare firmware for loading
+ * @huc:	intel_huc struct
  *
  * Called early during driver load, but after GEM is initialised. The loading
  * will continue only when driver explicitly specify firmware name and version.
@@ -152,44 +152,45 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
  *
  * The DMA-copying to HW is done later when intel_huc_init_hw() is called.
  */
-void intel_huc_init(struct drm_i915_private *dev_priv)
+void intel_huc_init_fw(struct intel_huc *huc)
 {
-	struct intel_huc *huc = &dev_priv->huc;
-	struct intel_uc_fw *huc_fw = &huc->fw;
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 	const char *fw_path = NULL;
 
-	huc_fw->path = NULL;
-	huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
-	huc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
-	huc_fw->fw = INTEL_UC_FW_TYPE_HUC;
+	huc->fw.path = NULL;
+	huc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE;
+	huc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
+	huc->fw.fw = INTEL_UC_FW_TYPE_HUC;
 
 	if (!HAS_HUC_UCODE(dev_priv))
 		return;
 
 	if (IS_SKYLAKE(dev_priv)) {
 		fw_path = I915_SKL_HUC_UCODE;
-		huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
-		huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
+		huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR;
+		huc->fw.minor_ver_wanted = SKL_HUC_FW_MINOR;
 	} else if (IS_BROXTON(dev_priv)) {
 		fw_path = I915_BXT_HUC_UCODE;
-		huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
-		huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
+		huc->fw.major_ver_wanted = BXT_HUC_FW_MAJOR;
+		huc->fw.minor_ver_wanted = BXT_HUC_FW_MINOR;
 	} else if (IS_KABYLAKE(dev_priv)) {
 		fw_path = I915_KBL_HUC_UCODE;
-		huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
-		huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
+		huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR;
+		huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR;
 	}
 
-	huc_fw->path = fw_path;
+	huc->fw.path = fw_path;
 
-	if (huc_fw->path == NULL)
+	if (fw_path == NULL)
 		return;
 
-	huc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
+	huc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING;
 
 	DRM_DEBUG_DRIVER("HuC firmware pending, path %s\n", fw_path);
 
-	intel_uc_prepare_fw(dev_priv, huc_fw);
+	WARN(huc->fw.path == NULL, "HuC present but no fw path\n");
+
+	intel_uc_prepare_fw(dev_priv, &huc->fw);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2ea8a2c..e5155de 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -31,6 +31,12 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->guc.send_mutex);
 }
 
+void intel_uc_init_fw(struct drm_i915_private *dev_priv)
+{
+	intel_huc_init_fw(&dev_priv->huc);
+	intel_guc_init_fw(&dev_priv->guc);
+}
+
 /*
  * Read GuC command/status register (SOFT_SCRATCH_0)
  * Return true if it contains a response rather than a command
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 9509432..26d5c6a 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -185,13 +185,14 @@ struct intel_huc {
 
 /* intel_uc.c */
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
+void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
 			 struct intel_uc_fw *uc_fw);
 int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 
 /* intel_guc_loader.c */
-void intel_guc_init(struct drm_i915_private *dev_priv);
+void intel_guc_init_fw(struct intel_guc *guc);
 int intel_guc_init_hw(struct intel_guc *guc);
 void intel_guc_fini(struct drm_i915_private *dev_priv);
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
@@ -223,7 +224,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 }
 
 /* intel_huc.c */
-void intel_huc_init(struct drm_i915_private *dev_priv);
+void intel_huc_init_fw(struct intel_huc *huc);
 void intel_huc_fini(struct drm_i915_private  *dev_priv);
 int intel_huc_init_hw(struct intel_huc *huc);
 void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
-- 
2.9.3

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

[PATCH 06/11] drm/i915/guc: Extract param logic form guc_init_fw()

[Arkadiusz Hiler]

Let intel_guc_init_fw() focus on determining and fetching the correct
firmware.

This patch introduces intel_uc_sanitize_options() that is called from
intel_sanitize_options().

Then, if we have GuC, we can call intel_guc_init_fw() conditionally
and we do not have to do the internal checks.

v2: fix comment, notify when nuking GuC explicitly enabled (M. Wajdeczko)
v3: fix comment again, change the nuke message (M. Wajdeczko)
v4: update title to reflect new function name + rebase
v5: text && remove 2 uneccessary checks (M. Wajdeczko)

Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |  2 ++
 drivers/gpu/drm/i915/intel_guc_loader.c | 18 +-----------------
 drivers/gpu/drm/i915/intel_huc.c        |  3 ---
 drivers/gpu/drm/i915/intel_uc.c         | 28 +++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_uc.h         |  1 +
 5 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0fbdcb0..19ec819 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -988,6 +988,8 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 
 	i915.semaphores = intel_sanitize_semaphores(dev_priv, i915.semaphores);
 	DRM_DEBUG_DRIVER("use GPU semaphores? %s\n", yesno(i915.semaphores));
+
+	intel_uc_sanitize_options(dev_priv);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 3a37d14..e732266 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -534,20 +534,7 @@ void intel_guc_init_fw(struct intel_guc *guc)
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	const char *fw_path;
 
-	if (!HAS_GUC(dev_priv)) {
-		i915.enable_guc_loading = 0;
-		i915.enable_guc_submission = 0;
-	} else {
-		/* A negative value means "use platform default" */
-		if (i915.enable_guc_loading < 0)
-			i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
-		if (i915.enable_guc_submission < 0)
-			i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
-	}
-
-	if (!HAS_GUC_UCODE(dev_priv)) {
-		fw_path = NULL;
-	} else if (IS_SKYLAKE(dev_priv)) {
+	if (IS_SKYLAKE(dev_priv)) {
 		fw_path = I915_SKL_GUC_UCODE;
 		guc->fw.major_ver_wanted = SKL_FW_MAJOR;
 		guc->fw.minor_ver_wanted = SKL_FW_MINOR;
@@ -567,9 +554,6 @@ void intel_guc_init_fw(struct intel_guc *guc)
 	guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE;
 	guc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
 
-	/* Early (and silent) return if GuC loading is disabled */
-	if (!i915.enable_guc_loading)
-		return;
 	if (fw_path == NULL)
 		return;
 	if (*fw_path == '\0')
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index fda473d..168aab1 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -162,9 +162,6 @@ void intel_huc_init_fw(struct intel_huc *huc)
 	huc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
 	huc->fw.fw = INTEL_UC_FW_TYPE_HUC;
 
-	if (!HAS_HUC_UCODE(dev_priv))
-		return;
-
 	if (IS_SKYLAKE(dev_priv)) {
 		fw_path = I915_SKL_HUC_UCODE;
 		huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index e5155de..f0a69d4 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -26,6 +26,27 @@
 #include "intel_uc.h"
 #include <linux/firmware.h>
 
+void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
+{
+	if (!HAS_GUC(dev_priv)) {
+		if (i915.enable_guc_loading > 0)
+			DRM_INFO("Ignoring GuC options, no hardware");
+
+		i915.enable_guc_loading = 0;
+		i915.enable_guc_submission = 0;
+	} else {
+		/* A negative value means "use platform default" */
+		if (i915.enable_guc_loading < 0)
+			i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
+		if (i915.enable_guc_submission < 0)
+			i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
+
+		/* Can't enable guc submission without guc loaded */
+		if (!i915.enable_guc_loading)
+			i915.enable_guc_submission = 0;
+	}
+}
+
 void intel_uc_init_early(struct drm_i915_private *dev_priv)
 {
 	mutex_init(&dev_priv->guc.send_mutex);
@@ -33,7 +54,12 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 {
-	intel_huc_init_fw(&dev_priv->huc);
+	if (!i915.enable_guc_loading)
+		return;
+
+	if (HAS_HUC_UCODE(dev_priv))
+		intel_huc_init_fw(&dev_priv->huc);
+
 	intel_guc_init_fw(&dev_priv->guc);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 26d5c6a..c5179ef 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -184,6 +184,7 @@ struct intel_huc {
 };
 
 /* intel_uc.c */
+void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
-- 
2.9.3

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

[PATCH 07/11] drm/i915/guc: Simplify intel_guc_init_hw()

[Arkadiusz Hiler]

Current version of intel_guc_init_hw() does a lot:
 - cares about submission
 - loads huc
 - implement WA

This change offloads some of the logic to intel_uc_init_hw(), which now
cares about the above.

v2: rename guc_hw_reset and fix typo in define name (M. Wajdeczko)
v3: rename once again
v4: remove spurious comments and add some style (J. Lahtinen)
v5: flow changes, got rid of dead checks (M. Wajdeczko)
v6: rebase
v7: rebase & onion teardown (J. Lahtinen)

Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |   2 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 138 +++-----------------------------
 drivers/gpu/drm/i915/intel_uc.c         | 111 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h         |   1 +
 4 files changed, 122 insertions(+), 130 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5762c9b..d87983b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4541,7 +4541,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	intel_mocs_init_l3cc_table(dev_priv);
 
 	/* We can't enable contexts until all firmware is loaded */
-	ret = intel_guc_init_hw(&dev_priv->guc);
+	ret = intel_uc_init_hw(dev_priv);
 	if (ret)
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index e732266..0a29c1b 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -344,24 +344,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-static int guc_hw_reset(struct drm_i915_private *dev_priv)
-{
-	int ret;
-	u32 guc_status;
-
-	ret = intel_guc_reset(dev_priv);
-	if (ret) {
-		DRM_ERROR("GuC reset failed, ret = %d\n", ret);
-		return ret;
-	}
-
-	guc_status = I915_READ(GUC_STATUS);
-	WARN(!(guc_status & GS_MIA_IN_RESET),
-	     "GuC status: 0x%x, MIA core expected to be in reset\n", guc_status);
-
-	return ret;
-}
-
 /**
  * intel_guc_init_hw() - finish preparing the GuC for activity
  * @guc: intel_guc structure
@@ -379,41 +361,22 @@ 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 retries, ret, err;
+	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));
 
-	/* Loading forbidden, or no firmware to load? */
-	if (!i915.enable_guc_loading) {
-		err = 0;
-		goto fail;
-	} else if (fw_path == NULL) {
-		/* Device is known to have no uCode (e.g. no GuC) */
-		err = -ENXIO;
-		goto fail;
+	if (!fw_path) {
+		return -ENXIO;
 	} else if (*fw_path == '\0') {
-		/* Device has a GuC but we don't know what f/w to load? */
 		WARN(1, "No GuC firmware known for this platform!\n");
-		err = -ENODEV;
-		goto fail;
+		return -ENODEV;
 	}
 
-	/* Fetch failed, or already fetched but failed to load? */
-	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS) {
-		err = -EIO;
-		goto fail;
-	} else if (guc->fw.load_status == INTEL_UC_FIRMWARE_FAIL) {
-		err = -ENOEXEC;
-		goto fail;
-	}
-
-	gen9_reset_guc_interrupts(dev_priv);
-
-	/* We need to notify the guc whenever we change the GGTT */
-	i915_ggtt_enable_guc(dev_priv);
+	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return -EIO;
 
 	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
 
@@ -421,102 +384,19 @@ int intel_guc_init_hw(struct intel_guc *guc)
 		intel_uc_fw_status_repr(guc->fw.fetch_status),
 		intel_uc_fw_status_repr(guc->fw.load_status));
 
-	err = i915_guc_submission_init(dev_priv);
-	if (err)
-		goto fail;
+	ret = guc_ucode_xfer(dev_priv);
 
-	/*
-	 * WaEnableuKernelHeaderValidFix:skl,bxt
-	 * For BXT, this is only upto B0 but below WA is required for later
-	 * steppings also so this is extended as well.
-	 */
-	/* WaEnableGuCBootHashCheckNotSet:skl,bxt */
-	for (retries = 3; ; ) {
-		/*
-		 * Always reset the GuC just before (re)loading, so
-		 * that the state and timing are fairly predictable
-		 */
-		err = guc_hw_reset(dev_priv);
-		if (err)
-			goto fail;
-
-		intel_huc_init_hw(&dev_priv->huc);
-		err = guc_ucode_xfer(dev_priv);
-		if (!err)
-			break;
-
-		if (--retries == 0)
-			goto fail;
-
-		DRM_INFO("GuC fw load failed: %d; will reset and "
-			 "retry %d more time(s)\n", err, retries);
-	}
+	if (ret)
+		return -EAGAIN;
 
 	guc->fw.load_status = INTEL_UC_FIRMWARE_SUCCESS;
 
-	intel_guc_auth_huc(dev_priv);
-
-	if (i915.enable_guc_submission) {
-		if (i915.guc_log_level >= 0)
-			gen9_enable_guc_interrupts(dev_priv);
-
-		err = i915_guc_submission_enable(dev_priv);
-		if (err)
-			goto fail;
-	}
-
 	DRM_INFO("GuC %s (firmware %s [version %u.%u])\n",
 		 i915.enable_guc_submission ? "submission enabled" : "loaded",
 		 guc->fw.path,
 		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
 
 	return 0;
-
-fail:
-	if (guc->fw.load_status == INTEL_UC_FIRMWARE_PENDING)
-		guc->fw.load_status = INTEL_UC_FIRMWARE_FAIL;
-
-	i915_guc_submission_disable(dev_priv);
-	i915_guc_submission_fini(dev_priv);
-	i915_ggtt_disable_guc(dev_priv);
-
-	/*
-	 * We've failed to load the firmware :(
-	 *
-	 * Decide whether to disable GuC submission and fall back to
-	 * execlist mode, and whether to hide the error by returning
-	 * zero or to return -EIO, which the caller will treat as a
-	 * nonfatal error (i.e. it doesn't prevent driver load, but
-	 * marks the GPU as wedged until reset).
-	 */
-	if (i915.enable_guc_loading > 1) {
-		ret = -EIO;
-	} else if (i915.enable_guc_submission > 1) {
-		ret = -EIO;
-	} else {
-		ret = 0;
-	}
-
-	if (err == 0 && !HAS_GUC_UCODE(dev_priv))
-		;	/* Don't mention the GuC! */
-	else if (err == 0)
-		DRM_INFO("GuC firmware load skipped\n");
-	else if (ret != -EIO)
-		DRM_NOTE("GuC firmware load failed: %d\n", err);
-	else
-		DRM_WARN("GuC firmware load failed: %d\n", err);
-
-	if (i915.enable_guc_submission) {
-		if (fw_path == NULL)
-			DRM_INFO("GuC submission without firmware not supported\n");
-		if (ret == 0)
-			DRM_NOTE("Falling back from GuC submission to execlist mode\n");
-		else
-			DRM_ERROR("GuC init failed: %d\n", ret);
-	}
-	i915.enable_guc_submission = 0;
-
-	return ret;
 }
 
 
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f0a69d4..69f21cc 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -26,6 +26,27 @@
 #include "intel_uc.h"
 #include <linux/firmware.h>
 
+/* Reset GuC providing us with fresh state for both GuC and HuC.
+ */
+static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
+{
+	int ret;
+	u32 guc_status;
+
+	ret = intel_guc_reset(dev_priv);
+	if (ret) {
+		DRM_ERROR("GuC reset failed, ret = %d\n", ret);
+		return ret;
+	}
+
+	guc_status = I915_READ(GUC_STATUS);
+	WARN(!(guc_status & GS_MIA_IN_RESET),
+	     "GuC status: 0x%x, MIA core expected to be in reset\n",
+	     guc_status);
+
+	return ret;
+}
+
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 {
 	if (!HAS_GUC(dev_priv)) {
@@ -63,6 +84,96 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 	intel_guc_init_fw(&dev_priv->guc);
 }
 
+int intel_uc_init_hw(struct drm_i915_private *dev_priv)
+{
+	int ret, attempts;
+
+	/* GuC not enabled, nothing to do */
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	gen9_reset_guc_interrupts(dev_priv);
+
+	/* We need to notify the guc whenever we change the GGTT */
+	i915_ggtt_enable_guc(dev_priv);
+
+	if (i915.enable_guc_submission) {
+		ret = i915_guc_submission_init(dev_priv);
+		if (ret)
+			goto err;
+	}
+
+	/* WaEnableuKernelHeaderValidFix:skl */
+	/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
+	if (IS_GEN9(dev_priv))
+		attempts = 3;
+	else
+		attempts = 1;
+
+	while (attempts--) {
+		/*
+		 * Always reset the GuC just before (re)loading, so
+		 * that the state and timing are fairly predictable
+		 */
+		ret = __intel_uc_reset_hw(dev_priv);
+		if (ret)
+			goto err_submission;
+
+		intel_huc_init_hw(&dev_priv->huc);
+		ret = intel_guc_init_hw(&dev_priv->guc);
+		if (ret == 0 || ret != -EAGAIN)
+			break;
+
+		DRM_DEBUG_DRIVER("GuC fw load failed: %d; will reset and "
+				 "retry %d more time(s)\n", ret, attempts);
+	}
+
+	/* Did we succeded or run out of retries? */
+	if (ret)
+		goto err_submission;
+
+	intel_guc_auth_huc(dev_priv);
+	if (i915.enable_guc_submission) {
+		if (i915.guc_log_level >= 0)
+			gen9_enable_guc_interrupts(dev_priv);
+
+		ret = i915_guc_submission_enable(dev_priv);
+		if (ret)
+			goto err_submission;
+	}
+
+	return 0;
+
+	/*
+	 * We've failed to load the firmware :(
+	 *
+	 * Decide whether to disable GuC submission and fall back to
+	 * execlist mode, and whether to hide the error by returning
+	 * zero or to return -EIO, which the caller will treat as a
+	 * nonfatal error (i.e. it doesn't prevent driver load, but
+	 * marks the GPU as wedged until reset).
+	 */
+err_submission:
+	if (i915.enable_guc_submission)
+		i915_guc_submission_fini(dev_priv);
+
+err:
+	i915_ggtt_disable_guc(dev_priv);
+
+	DRM_ERROR("GuC init failed\n");
+	if (i915.enable_guc_loading > 1 || i915.enable_guc_submission > 1)
+		ret = -EIO;
+	else
+		ret = 0;
+
+	if (i915.enable_guc_submission) {
+		i915.enable_guc_submission = 0;
+		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
+	}
+
+	return ret;
+}
+
 /*
  * Read GuC command/status register (SOFT_SCRATCH_0)
  * Return true if it contains a response rather than a command
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index c5179ef..794e6ea 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -187,6 +187,7 @@ struct intel_huc {
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
+int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
 			 struct intel_uc_fw *uc_fw);
 int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
-- 
2.9.3

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

[PATCH 08/11] drm/i915/uc: Simplify firmware path handling

[Arkadiusz Hiler]

Currently fw->path values can represent one of three possible states:

 1) NULL - device without the uC
 2) '\0' - device with the uC but have no firmware
 3) else - device with the uC and we have firmware

Second case is used only to WARN at a later stage.

We can WARN right away and merge cases 1 and 2.

Code can be even further simplified and common (HuC/GuC logic) happening
right before the fetch can be offloaded to the common function.

v2: fewer temporary variables, more straightforward flow (M. Wajdeczko)
v3: DRM_ERROR instead of WARN (M. Wajdeczko)
v4: coding standard (J. Lahtinen)
v5: non-trivial rebase
v6: remove path check, we are checking fetch status (M. Wajdeczko)

Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 36 ++++++++++-----------------------
 drivers/gpu/drm/i915/intel_huc.c        | 21 ++++++-------------
 drivers/gpu/drm/i915/intel_uc.c         |  4 +++-
 3 files changed, 20 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 0a29c1b..d731f68 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -368,13 +368,6 @@ int intel_guc_init_hw(struct intel_guc *guc)
 		intel_uc_fw_status_repr(guc->fw.fetch_status),
 		intel_uc_fw_status_repr(guc->fw.load_status));
 
-	if (!fw_path) {
-		return -ENXIO;
-	} else if (*fw_path == '\0') {
-		WARN(1, "No GuC firmware known for this platform!\n");
-		return -ENODEV;
-	}
-
 	if (guc->fw.fetch_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return -EIO;
 
@@ -399,7 +392,6 @@ int intel_guc_init_hw(struct intel_guc *guc)
 	return 0;
 }
 
-
 /**
  * intel_guc_init_fw() - select and prepare firmware for loading
  * @guc:	intel_guc struct
@@ -412,37 +404,31 @@ int intel_guc_init_hw(struct intel_guc *guc)
 void intel_guc_init_fw(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	const char *fw_path;
+
+	guc->fw.path = NULL;
+	guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE;
+	guc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
+	guc->fw.fw = INTEL_UC_FW_TYPE_GUC;
 
 	if (IS_SKYLAKE(dev_priv)) {
-		fw_path = I915_SKL_GUC_UCODE;
+		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)) {
-		fw_path = I915_BXT_GUC_UCODE;
+		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)) {
-		fw_path = I915_KBL_GUC_UCODE;
+		guc->fw.path = I915_KBL_GUC_UCODE;
 		guc->fw.major_ver_wanted = KBL_FW_MAJOR;
 		guc->fw.minor_ver_wanted = KBL_FW_MINOR;
 	} else {
-		fw_path = "";	/* unknown device */
+		DRM_ERROR("No GuC firmware known for platform with GuC!\n");
+		i915.enable_guc_loading = 0;
+		return;
 	}
 
-	guc->fw.path = fw_path;
-	guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE;
-	guc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
-
-	if (fw_path == NULL)
-		return;
-	if (*fw_path == '\0')
-		return;
-
-	guc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING;
-	DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path);
 	intel_uc_prepare_fw(dev_priv, &guc->fw);
-	/* status must now be FAIL or SUCCESS */
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 168aab1..5fadd55 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -155,7 +155,6 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
 void intel_huc_init_fw(struct intel_huc *huc)
 {
 	struct drm_i915_private *dev_priv = huc_to_i915(huc);
-	const char *fw_path = NULL;
 
 	huc->fw.path = NULL;
 	huc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE;
@@ -163,29 +162,21 @@ void intel_huc_init_fw(struct intel_huc *huc)
 	huc->fw.fw = INTEL_UC_FW_TYPE_HUC;
 
 	if (IS_SKYLAKE(dev_priv)) {
-		fw_path = I915_SKL_HUC_UCODE;
+		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)) {
-		fw_path = I915_BXT_HUC_UCODE;
+		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)) {
-		fw_path = I915_KBL_HUC_UCODE;
+		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;
-	}
-
-	huc->fw.path = fw_path;
-
-	if (fw_path == NULL)
+	} else {
+		DRM_ERROR("No HuC firmware known for platform with HuC!\n");
 		return;
-
-	huc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING;
-
-	DRM_DEBUG_DRIVER("HuC firmware pending, path %s\n", fw_path);
-
-	WARN(huc->fw.path == NULL, "HuC present but no fw path\n");
+	}
 
 	intel_uc_prepare_fw(dev_priv, &huc->fw);
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 69f21cc..4b85a0f 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -268,8 +268,10 @@ void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
 	size_t size;
 	int err;
 
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
+
 	DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
-		intel_uc_fw_status_repr(uc_fw->fetch_status));
+			 intel_uc_fw_status_repr(uc_fw->fetch_status));
 
 	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
 	if (err)
-- 
2.9.3

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

[PATCH 09/11] drm/i915/uc: Separate firmware selection and preparation

[Arkadiusz Hiler]

intel_{h,g}uc_init_fw selects correct firmware and then triggers it's
preparation (fetch + initial parsing).

This change separates out select steps, so those can be called by
the sanitize_options().

Then, during the init_fw(), we prepare the firmware if the firmware was
selected.

Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 14 +++++---------
 drivers/gpu/drm/i915/intel_huc.c        | 14 ++------------
 drivers/gpu/drm/i915/intel_uc.c         | 18 ++++++++++++------
 drivers/gpu/drm/i915/intel_uc.h         |  4 ++--
 4 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index d731f68..f8c9e31 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -393,15 +393,12 @@ int intel_guc_init_hw(struct intel_guc *guc)
 }
 
 /**
- * intel_guc_init_fw() - select and prepare firmware for loading
+ * intel_guc_select_fw() - selects GuC firmware for loading
  * @guc:	intel_guc struct
  *
- * Called early during driver load, but after GEM is initialised.
- *
- * The firmware will be transferred to the GuC's memory later,
- * when intel_guc_init_hw() is called.
+ * Return: zero when we know firmware, non-zero in other case
  */
-void intel_guc_init_fw(struct intel_guc *guc)
+int intel_guc_select_fw(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
@@ -424,11 +421,10 @@ void intel_guc_init_fw(struct intel_guc *guc)
 		guc->fw.minor_ver_wanted = KBL_FW_MINOR;
 	} else {
 		DRM_ERROR("No GuC firmware known for platform with GuC!\n");
-		i915.enable_guc_loading = 0;
-		return;
+		return -ENOENT;
 	}
 
-	intel_uc_prepare_fw(dev_priv, &guc->fw);
+	return 0;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 5fadd55..ea67abc 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -141,18 +141,10 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv)
 }
 
 /**
- * intel_huc_init_fw() - select and prepare firmware for loading
+ * intel_huc_select_fw() - selects HuC firmware for loading
  * @huc:	intel_huc struct
- *
- * Called early during driver load, but after GEM is initialised. The loading
- * will continue only when driver explicitly specify firmware name and version.
- * All other cases are considered as INTEL_UC_FIRMWARE_NONE either because HW
- * is not capable or driver yet support it. And there will be no error message
- * for INTEL_UC_FIRMWARE_NONE cases.
- *
- * The DMA-copying to HW is done later when intel_huc_init_hw() is called.
  */
-void intel_huc_init_fw(struct intel_huc *huc)
+void intel_huc_select_fw(struct intel_huc *huc)
 {
 	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 
@@ -177,8 +169,6 @@ void intel_huc_init_fw(struct intel_huc *huc)
 		DRM_ERROR("No HuC firmware known for platform with HuC!\n");
 		return;
 	}
-
-	intel_uc_prepare_fw(dev_priv, &huc->fw);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 4b85a0f..eaa2b75 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -66,6 +66,14 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 		if (!i915.enable_guc_loading)
 			i915.enable_guc_submission = 0;
 	}
+
+	if (i915.enable_guc_loading) {
+		if (HAS_HUC_UCODE(dev_priv))
+			intel_huc_select_fw(&dev_priv->huc);
+
+		if (intel_guc_select_fw(&dev_priv->guc))
+			i915.enable_guc_loading = 0;
+	}
 }
 
 void intel_uc_init_early(struct drm_i915_private *dev_priv)
@@ -75,13 +83,11 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 {
-	if (!i915.enable_guc_loading)
-		return;
+	if (dev_priv->huc.fw.path)
+		intel_uc_prepare_fw(dev_priv, &dev_priv->huc.fw);
 
-	if (HAS_HUC_UCODE(dev_priv))
-		intel_huc_init_fw(&dev_priv->huc);
-
-	intel_guc_init_fw(&dev_priv->guc);
+	if (dev_priv->guc.fw.path)
+		intel_uc_prepare_fw(dev_priv, &dev_priv->guc.fw);
 }
 
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 794e6ea..170dd70 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -194,7 +194,7 @@ int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 
 /* intel_guc_loader.c */
-void intel_guc_init_fw(struct intel_guc *guc);
+int intel_guc_select_fw(struct intel_guc *guc);
 int intel_guc_init_hw(struct intel_guc *guc);
 void intel_guc_fini(struct drm_i915_private *dev_priv);
 const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
@@ -226,7 +226,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 }
 
 /* intel_huc.c */
-void intel_huc_init_fw(struct intel_huc *huc);
+void intel_huc_select_fw(struct intel_huc *huc);
 void intel_huc_fini(struct drm_i915_private  *dev_priv);
 int intel_huc_init_hw(struct intel_huc *huc);
 void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
-- 
2.9.3

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

[PATCH 10/11] drm/i915/uc: Add params for specifying firmware

[Arkadiusz Hiler]

`guc_firmware_path` and `huc_firmware_path` module parameters are added.

Using the parameter disables version checks and loads desired firmware
instead of the default one.

v2: make params unsafe && notice about disabled fw check (J. Lahtinen)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_params.c      | 10 ++++++++++
 drivers/gpu/drm/i915/i915_params.h      |  2 ++
 drivers/gpu/drm/i915/intel_guc_loader.c |  6 +++++-
 drivers/gpu/drm/i915/intel_huc.c        |  6 +++++-
 drivers/gpu/drm/i915/intel_uc.c         |  6 ++++--
 5 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 2e9645e..b6a7e36 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -59,6 +59,8 @@ struct i915_params i915 __read_mostly = {
 	.enable_guc_loading = 0,
 	.enable_guc_submission = 0,
 	.guc_log_level = -1,
+	.guc_firmware_path = NULL,
+	.huc_firmware_path = NULL,
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
 	.enable_dpcd_backlight = false,
@@ -230,6 +232,14 @@ module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
 	"GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
 
+module_param_named_unsafe(guc_firmware_path, i915.guc_firmware_path, charp, 0400);
+MODULE_PARM_DESC(guc_firmware_path,
+	"GuC firmware path to use instead of the default one");
+
+module_param_named_unsafe(huc_firmware_path, i915.huc_firmware_path, charp, 0400);
+MODULE_PARM_DESC(huc_firmware_path,
+	"HuC firmware path to use instead of the default one");
+
 module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600);
 MODULE_PARM_DESC(enable_dp_mst,
 	"Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 55d47ee..34148cc 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -46,6 +46,8 @@
 	func(int, enable_guc_loading); \
 	func(int, enable_guc_submission); \
 	func(int, guc_log_level); \
+	func(char *, guc_firmware_path); \
+	func(char *, huc_firmware_path); \
 	func(int, use_mmio_flip); \
 	func(int, mmio_debug); \
 	func(int, edp_vswing); \
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index f8c9e31..d1d183b 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -407,7 +407,11 @@ int intel_guc_select_fw(struct intel_guc *guc)
 	guc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
 	guc->fw.fw = INTEL_UC_FW_TYPE_GUC;
 
-	if (IS_SKYLAKE(dev_priv)) {
+	if (i915.guc_firmware_path) {
+		guc->fw.path = i915.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;
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index ea67abc..ab4ee08 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -153,7 +153,11 @@ void intel_huc_select_fw(struct intel_huc *huc)
 	huc->fw.load_status = INTEL_UC_FIRMWARE_NONE;
 	huc->fw.fw = INTEL_UC_FW_TYPE_HUC;
 
-	if (IS_SKYLAKE(dev_priv)) {
+	if (i915.huc_firmware_path) {
+		huc->fw.path = i915.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;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index eaa2b75..c31f05a 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -356,8 +356,10 @@ void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
 		goto fail;
 	}
 
-	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
-	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
+	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
+		DRM_NOTE("Skipping uC firmware version check\n");
+	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
+		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
 		DRM_NOTE("uC firmware version %d.%d, required %d.%d\n",
 			uc_fw->major_ver_found, uc_fw->minor_ver_found,
 			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-- 
2.9.3

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

[PATCH 11/11] HAX enable GuC for CI

[Arkadiusz Hiler]

---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b6a7e36..9dcc8a0 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 1,
+	.enable_guc_submission = 1,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.9.3

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

Re: [PATCH 10/11] drm/i915/uc: Add params for specifying firmware

[Michal Wajdeczko]

On Tue, Mar 14, 2017 at 03:28:14PM +0100, Arkadiusz Hiler wrote:
> `guc_firmware_path` and `huc_firmware_path` module parameters are added.
> 
> Using the parameter disables version checks and loads desired firmware
> instead of the default one.
> 
> v2: make params unsafe && notice about disabled fw check (J. Lahtinen)
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---

snip

> @@ -356,8 +356,10 @@ void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
>  		goto fail;
>  	}
>  
> -	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> -	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> +	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
> +		DRM_NOTE("Skipping uC firmware version check\n");

Do we really need this explicit message?
I would rather promote DRM_DEBUG_DRIVER that prints loaded fw version to
DRM_NOTE to always trace what fw version driver will use (which is even more
important as user may specify any version that may cause driver go crazy)

-Michal

> +	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> +		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
>  		DRM_NOTE("uC firmware version %d.%d, required %d.%d\n",
>  			uc_fw->major_ver_found, uc_fw->minor_ver_found,
>  			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
> -- 
> 2.9.3
> 
> _______________________________________________
> 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

✓ Fi.CI.BAT: success for GuC Scrub vol. 1 (rev13)

[Patchwork]

== Series Details ==

Series: GuC Scrub vol. 1 (rev13)
URL   : https://patchwork.freedesktop.org/series/16856/
State : success

== Summary ==

Series 16856v13 GuC Scrub vol. 1
https://patchwork.freedesktop.org/api/1.0/series/16856/revisions/13/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-bxt-t5700) fdo#100125
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2520m)

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 458s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 574s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 511s
fi-bxt-t5700     total:278  pass:257  dwarn:1   dfail:0   fail:0   skip:20  time: 531s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 499s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 496s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 438s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 432s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 439s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 518s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 498s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 474s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 455s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 589s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 466s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 514s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 412s

c641417b70c6b78efca29ae732d7cbf5716ac6d5 drm-tip: 2017y-03m-14d-16h-04m-56s UTC integration manifest
66490ee HAX enable GuC for CI
0a29d29 drm/i915/uc: Add params for specifying firmware
bf44b79 drm/i915/uc: Separate firmware selection and preparation
292e5c9 drm/i915/uc: Simplify firmware path handling
38dd2e4 drm/i915/guc: Simplify intel_guc_init_hw()
a1f4e44 drm/i915/guc: Extract param logic form guc_init_fw()
01caa58 drm/i915/uc: Introduce intel_uc_init_fw()
9866936 drm/i915/uc: Move intel_uc_fw_fetch() to intel_uc.c
8e684e7 drm/i915/uc: Rename intel_?uc_{setup, load}() to _init_hw()
8dd20fa drm/i915/huc: Add huc_to_i915
02e3f95 drm/i915/uc: Drop superfluous externs in intel_uc.h

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4165/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/11] drm/i915/uc: Add params for specifying firmware

[Arkadiusz Hiler]

On Tue, Mar 14, 2017 at 04:17:17PM +0100, Michal Wajdeczko wrote:
> On Tue, Mar 14, 2017 at 03:28:14PM +0100, Arkadiusz Hiler wrote:
> > `guc_firmware_path` and `huc_firmware_path` module parameters are added.
> > 
> > Using the parameter disables version checks and loads desired firmware
> > instead of the default one.
> > 
> > v2: make params unsafe && notice about disabled fw check (J. Lahtinen)
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> 
> snip
> 
> > @@ -356,8 +356,10 @@ void intel_uc_prepare_fw(struct drm_i915_private *dev_priv,
> >  		goto fail;
> >  	}
> >  
> > -	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> > -	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> > +	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
> > +		DRM_NOTE("Skipping uC firmware version check\n");
> 
> Do we really need this explicit message?
> I would rather promote DRM_DEBUG_DRIVER that prints loaded fw version to
> DRM_NOTE to always trace what fw version driver will use (which is even more
> important as user may specify any version that may cause driver go crazy)

It's logged anyway, right below this if statement :-)

I think it's useful that we are explicit about frimware version
overriding in logs. Additional benefit is that the logical conditions
are split and more readable.


> -Michal
> 
> > +	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> > +		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> >  		DRM_NOTE("uC firmware version %d.%d, required %d.%d\n",
> >  			uc_fw->major_ver_found, uc_fw->minor_ver_found,
> >  			uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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