[PATCH] drm/i915/guc: Support GuC SKL v6.1

[PATCH] drm/i915/guc: Support GuC SKL v6.1

[yu.dai]

From: Alex Dai <yu.dai@intel.com>

This version of GuC firmware fixes the engine reset issue where golden
context LRC address is treated as page index by mistake. It also fixes
the problem that scheduler stops submiting to one engine when the other
engine work queue is full.

Signed-off-by: Alex Dai <yu.dai@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index e0093a9..e329a8a 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -59,7 +59,7 @@
  *
  */
 
-#define I915_SKL_GUC_UCODE "i915/skl_guc_ver4.bin"
+#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6.bin"
 MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
 
 /* User-friendly representation of an enum */
@@ -611,8 +611,8 @@ void intel_guc_ucode_init(struct drm_device *dev)
 		fw_path = NULL;
 	} else if (IS_SKYLAKE(dev)) {
 		fw_path = I915_SKL_GUC_UCODE;
-		guc_fw->guc_fw_major_wanted = 4;
-		guc_fw->guc_fw_minor_wanted = 3;
+		guc_fw->guc_fw_major_wanted = 6;
+		guc_fw->guc_fw_minor_wanted = 1;
 	} else {
 		fw_path = "";	/* unknown device */
 	}
-- 
2.5.0

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

Re: [PATCH] drm/i915/guc: Support GuC SKL v6.1

[Dave Gordon]

On 24/02/16 16:49, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@intel.com>
>
> This version of GuC firmware fixes the engine reset issue where golden
> context LRC address is treated as page index by mistake. It also fixes
> the problem that scheduler stops submiting to one engine when the other
> engine work queue is full.
>
> Signed-off-by: Alex Dai <yu.dai@intel.com>

Hi Alex,

I heard a rumour that GuC 6.1 required a change to the driver interface?
If you can confirm that this is not so, then I'm happy with this, so:

Reviewed-by: Dave Gordon <david.s.gordon@intel.com>

Also: GuC 6.1 has no known outstanding bugs to be fixed, so this would 
be a really good time to enable GuC submission by default on machines 
with this firmware installed. So I'm also going to repost the enabling 
patch (which you already R-B'ed) as a reply to this, so they should be 
taken together :)

.Dave.

> ---
>   drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index e0093a9..e329a8a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -59,7 +59,7 @@
>    *
>    */
>
> -#define I915_SKL_GUC_UCODE "i915/skl_guc_ver4.bin"
> +#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6.bin"
>   MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
>
>   /* User-friendly representation of an enum */
> @@ -611,8 +611,8 @@ void intel_guc_ucode_init(struct drm_device *dev)
>   		fw_path = NULL;
>   	} else if (IS_SKYLAKE(dev)) {
>   		fw_path = I915_SKL_GUC_UCODE;
> -		guc_fw->guc_fw_major_wanted = 4;
> -		guc_fw->guc_fw_minor_wanted = 3;
> +		guc_fw->guc_fw_major_wanted = 6;
> +		guc_fw->guc_fw_minor_wanted = 1;
>   	} else {
>   		fw_path = "";	/* unknown device */
>   	}


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

Re: [PATCH] drm/i915/guc: Support GuC SKL v6.1

[Yu Dai]

On 02/25/2016 06:59 AM, Dave Gordon wrote:
> On 24/02/16 16:49, yu.dai@intel.com wrote:
> > From: Alex Dai <yu.dai@intel.com>
> >
> > This version of GuC firmware fixes the engine reset issue where golden
> > context LRC address is treated as page index by mistake. It also fixes
> > the problem that scheduler stops submiting to one engine when the other
> > engine work queue is full.
> >
> > Signed-off-by: Alex Dai <yu.dai@intel.com>
>
> Hi Alex,
>
> I heard a rumour that GuC 6.1 required a change to the driver interface?

Yes, but interface changes are for SLPC and has no impact on submission. 
We will differ the changes to be part of future SLPC patches.

Thanks,
Alex
> If you can confirm that this is not so, then I'm happy with this, so:
>
> Reviewed-by: Dave Gordon <david.s.gordon@intel.com>
>
> Also: GuC 6.1 has no known outstanding bugs to be fixed, so this would
> be a really good time to enable GuC submission by default on machines
> with this firmware installed. So I'm also going to repost the enabling
> patch (which you already R-B'ed) as a reply to this, so they should be
> taken together :)
>
> .Dave.
>
> > ---
> >   drivers/gpu/drm/i915/intel_guc_loader.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> > index e0093a9..e329a8a 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > @@ -59,7 +59,7 @@
> >    *
> >    */
> >
> > -#define I915_SKL_GUC_UCODE "i915/skl_guc_ver4.bin"
> > +#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6.bin"
> >   MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
> >
> >   /* User-friendly representation of an enum */
> > @@ -611,8 +611,8 @@ void intel_guc_ucode_init(struct drm_device *dev)
> >   		fw_path = NULL;
> >   	} else if (IS_SKYLAKE(dev)) {
> >   		fw_path = I915_SKL_GUC_UCODE;
> > -		guc_fw->guc_fw_major_wanted = 4;
> > -		guc_fw->guc_fw_minor_wanted = 3;
> > +		guc_fw->guc_fw_major_wanted = 6;
> > +		guc_fw->guc_fw_minor_wanted = 1;
> >   	} else {
> >   		fw_path = "";	/* unknown device */
> >   	}
>
>

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

[PATCH] drm/i915: add enable_guc_loading parameter

[Dave Gordon]

Split the function of "enable_guc_submission" into two separate options.
The new one "enable_guc_loading" controls only the *fetching and loading*
of the GuC firmware image. The existing one is redefined to control only
the *use* of the GuC for batch submission once the firmware is loaded.

In addition, the degree of control has been refined from a simple bool
to an integer key, allowing several options:
 -1  (default)    whatever the platform default is
  0  DISABLE      don't load/use the GuC
  1  BEST EFFORT  try to load/use the GuC, fallback if not available
  2  REQUIRE      must load/use the GuC, else leave the GPU wedged

The new platform default (as coded here) will be to attempt to load
the GuC iff the device has a GuC that requires firmware, to attempt to
use it iff the device has a GuC that supports the submission protocol
(with or without firmware), and to fall back to execlist mode if any
required firmware cannot be found or fails to load.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Alex Dai <yu.dai@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  1 -
 drivers/gpu/drm/i915/i915_params.c      | 14 ++++-
 drivers/gpu/drm/i915/i915_params.h      |  3 +-
 drivers/gpu/drm/i915/intel_guc_loader.c | 99 ++++++++++++++++++---------------
 4 files changed, 68 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f68f346..3d9397d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4891,7 +4891,6 @@ int i915_gem_init_rings(struct drm_device *dev)
 		ret = intel_guc_ucode_load(dev);
 		if (ret) {
 			DRM_ERROR("Failed to initialize GuC, error %d\n", ret);
-			ret = -EIO;
 			goto out;
 		}
 	}
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 278c9c4..a57e16b 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -54,7 +54,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_submission = false,
+	.enable_guc_loading = -1,
+	.enable_guc_submission = -1,
 	.guc_log_level = -1,
 };
 
@@ -195,8 +196,15 @@ struct i915_params i915 __read_mostly = {
 		 "(0=use value from vbt [default], 1=low power swing(200mV),"
 		 "2=default swing(400mV))");
 
-module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400);
-MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)");
+module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
+MODULE_PARM_DESC(enable_guc_submission,
+		"Enable GuC firmware loading "
+		"(-1=auto [default], 0=never, 1=if available, 2=required)");
+
+module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, int, 0400);
+MODULE_PARM_DESC(enable_guc_submission,
+		"Enable GuC submission "
+		"(-1=auto [default], 0=never, 1=if available, 2=required)");
 
 module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index bd5026b..8eafbce 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -45,6 +45,8 @@ struct i915_params {
 	int enable_ips;
 	int invert_brightness;
 	int enable_cmd_parser;
+	int enable_guc_loading;
+	int enable_guc_submission;
 	int guc_log_level;
 	int use_mmio_flip;
 	int mmio_debug;
@@ -56,7 +58,6 @@ struct i915_params {
 	bool load_detect_test;
 	bool reset;
 	bool disable_display;
-	bool enable_guc_submission;
 	bool verbose_state_checks;
 	bool nuclear_pageflip;
 };
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 82a3c03..e0093a9 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -369,49 +369,37 @@ int intel_guc_ucode_load(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
+	const char *fw_path = guc_fw->guc_fw_path;
 	int err = 0;
 
-	if (!i915.enable_guc_submission)
-		return 0;
-
-	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
+	DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
+		fw_path,
 		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
 		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
-	direct_interrupts_to_host(dev_priv);
+	/* Loading forbidden, or no firmware to load? */
+	if (!i915.enable_guc_loading)
+		goto fail;
+	if (fw_path == NULL)
+		goto fail;
+	if (*fw_path == '\0') {
+		DRM_ERROR("No GuC firmware known for this platform\n");
+		goto fail;
+	}
 
-	if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_NONE)
-		return 0;
+	/* Fetch failed, or already fetched but failed to load? */
+	if (guc_fw->guc_fw_fetch_status != GUC_FIRMWARE_SUCCESS)
+		goto fail;
+	if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL)
+		goto fail;
 
-	if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_SUCCESS &&
-	    guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL)
-		return -ENOEXEC;
+	direct_interrupts_to_host(dev_priv);
 
 	guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
 
-	DRM_DEBUG_DRIVER("GuC fw fetch status %s\n",
-		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status));
-
-	switch (guc_fw->guc_fw_fetch_status) {
-	case GUC_FIRMWARE_FAIL:
-		/* something went wrong :( */
-		err = -EIO;
-		goto fail;
-
-	case GUC_FIRMWARE_NONE:
-	case GUC_FIRMWARE_PENDING:
-	default:
-		/* "can't happen" */
-		WARN_ONCE(1, "GuC fw %s invalid guc_fw_fetch_status %s [%d]\n",
-			guc_fw->guc_fw_path,
-			intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
-			guc_fw->guc_fw_fetch_status);
-		err = -ENXIO;
-		goto fail;
-
-	case GUC_FIRMWARE_SUCCESS:
-		break;
-	}
+	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
+		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
+		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
 
 	err = i915_guc_submission_init(dev);
 	if (err)
@@ -440,6 +428,8 @@ int intel_guc_ucode_load(struct drm_device *dev)
 	return 0;
 
 fail:
+	DRM_DEBUG_DRIVER("GuC firmware load fail, err %d\n", err);
+
 	if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_PENDING)
 		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
 
@@ -447,6 +437,29 @@ int intel_guc_ucode_load(struct drm_device *dev)
 	i915_guc_submission_disable(dev);
 	i915_guc_submission_fini(dev);
 
+	/*
+	 * 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) {
+		err = -EIO;
+	} else if (HAS_GUC_SCHED(dev) && !HAS_GUC_UCODE(dev)) {
+		return 0;
+	} else if (i915.enable_guc_submission > 1) {
+		err = -EIO;
+	} else {
+		err = 0;
+	}
+
+	i915.enable_guc_submission = 0;
+
+	DRM_DEBUG_DRIVER("falling back to execlist mode, err %d\n", err);
+
 	return err;
 }
 
@@ -588,8 +601,11 @@ void intel_guc_ucode_init(struct drm_device *dev)
 	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
 	const char *fw_path;
 
-	if (!HAS_GUC_SCHED(dev))
-		i915.enable_guc_submission = false;
+	/* A negative value means "use platform default" */
+	if (i915.enable_guc_loading < 0)
+		i915.enable_guc_loading = HAS_GUC_UCODE(dev);
+	if (i915.enable_guc_submission < 0)
+		i915.enable_guc_submission = HAS_GUC_SCHED(dev);
 
 	if (!HAS_GUC_UCODE(dev)) {
 		fw_path = NULL;
@@ -598,26 +614,21 @@ void intel_guc_ucode_init(struct drm_device *dev)
 		guc_fw->guc_fw_major_wanted = 4;
 		guc_fw->guc_fw_minor_wanted = 3;
 	} else {
-		i915.enable_guc_submission = false;
 		fw_path = "";	/* unknown device */
 	}
 
-	if (!i915.enable_guc_submission)
-		return;
-
 	guc_fw->guc_dev = dev;
 	guc_fw->guc_fw_path = fw_path;
 	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
 	guc_fw->guc_fw_load_status = GUC_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') {
-		DRM_ERROR("No GuC firmware known for this platform\n");
-		guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_FAIL;
+	if (*fw_path == '\0')
 		return;
-	}
 
 	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_PENDING;
 	DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path);
-- 
1.9.1

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

✗ Fi.CI.BAT: warning for drm/i915: add enable_guc_loading parameter (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: add enable_guc_loading parameter (rev2)
URL   : https://patchwork.freedesktop.org/series/3045/
State : warning

== Summary ==

Series 3045v2 drm/i915: add enable_guc_loading parameter
http://patchwork.freedesktop.org/api/1.0/series/3045/revisions/2/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Test kms_force_connector_basic:
        Subgroup force-load-detect:
                fail       -> DMESG-FAIL (snb-x220t)
                dmesg-fail -> FAIL       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> SKIP       (hsw-gt2)
                pass       -> DMESG-WARN (ivb-t430s)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup basic-rte:
                fail       -> PASS       (bdw-ultra)

bdw-ultra        total:168  pass:154  dwarn:0   dfail:0   fail:0   skip:14 
bsw-nuc-2        total:168  pass:137  dwarn:0   dfail:0   fail:1   skip:30 
byt-nuc          total:168  pass:143  dwarn:0   dfail:0   fail:0   skip:25 
hsw-brixbox      total:168  pass:154  dwarn:0   dfail:0   fail:0   skip:14 
hsw-gt2          total:168  pass:156  dwarn:0   dfail:1   fail:0   skip:11 
ilk-hp8440p      total:168  pass:117  dwarn:1   dfail:0   fail:1   skip:49 
ivb-t430s        total:168  pass:152  dwarn:1   dfail:0   fail:1   skip:14 
skl-i7k-2        total:168  pass:151  dwarn:1   dfail:0   fail:0   skip:16 
snb-dellxps      total:168  pass:145  dwarn:0   dfail:0   fail:1   skip:22 
snb-x220t        total:168  pass:145  dwarn:0   dfail:1   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1478/

38f0cd5314ecb02d15d019631e388d3642275600 drm-intel-nightly: 2016y-02m-25d-15h-33m-53s UTC integration manifest
7cd8e5dc284499930311e7b005f8d5b42607ab73 drm/i915: add enable_guc_loading parameter

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