diff mbox

[1/4] CI_ONLY: add enable_guc_loading parameter

Message ID 1461595034-21799-1-git-send-email-david.s.gordon@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Gordon April 25, 2016, 2:37 p.m. UTC
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,
but not yet to use it for submission. A later patch will change
to enable it if appropriate.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c            |  1 -
 drivers/gpu/drm/i915/i915_guc_submission.c |  4 +-
 drivers/gpu/drm/i915/i915_params.c         | 14 ++++-
 drivers/gpu/drm/i915/i915_params.h         |  3 +-
 drivers/gpu/drm/i915/intel_guc_loader.c    | 98 ++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_uncore.c        |  2 +-
 6 files changed, 70 insertions(+), 52 deletions(-)

Comments

Daniel Vetter April 26, 2016, 2:18 p.m. UTC | #1
On Mon, Apr 25, 2016 at 03:37:11PM +0100, Dave Gordon wrote:
> 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,
> but not yet to use it for submission. A later patch will change
> to enable it if appropriate.
> 
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>

intel-gfx-trybot@fd.o is your CI_only patchwork and CI instance:

https://patchwork.freedesktop.org/project/intel-gfx-trybot/series/?ordering=-last_updated

Cheers, Daniel
> ---
>  drivers/gpu/drm/i915/i915_gem.c            |  1 -
>  drivers/gpu/drm/i915/i915_guc_submission.c |  4 +-
>  drivers/gpu/drm/i915/i915_params.c         | 14 ++++-
>  drivers/gpu/drm/i915/i915_params.h         |  3 +-
>  drivers/gpu/drm/i915/intel_guc_loader.c    | 98 ++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_uncore.c        |  2 +-
>  6 files changed, 70 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 261a3ef..1ea4546 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4914,7 +4914,6 @@ int i915_gem_init_engines(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_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index d40c13f..a542664 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -970,7 +970,7 @@ int intel_guc_suspend(struct drm_device *dev)
>  	struct intel_context *ctx;
>  	u32 data[3];
>  
> -	if (!i915.enable_guc_submission)
> +	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
>  		return 0;
>  
>  	ctx = dev_priv->kernel_context;
> @@ -996,7 +996,7 @@ int intel_guc_resume(struct drm_device *dev)
>  	struct intel_context *ctx;
>  	u32 data[3];
>  
> -	if (!i915.enable_guc_submission)
> +	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
>  		return 0;
>  
>  	ctx = dev_priv->kernel_context;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 1779f02..79be9f8 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 = 0,
>  	.guc_log_level = -1,
>  	.enable_dp_mst = true,
>  	.inject_load_failure = 0,
> @@ -197,8 +198,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_loading,
> +		"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, 0=never [default], 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 02bc278..9f1d17b 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;
> @@ -57,7 +59,6 @@ struct i915_params {
>  	bool load_detect_test;
>  	bool reset;
>  	bool disable_display;
> -	bool enable_guc_submission;
>  	bool verbose_state_checks;
>  	bool nuclear_pageflip;
>  	bool enable_dp_mst;
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 876e5da..2ec9cf1 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -387,49 +387,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 retries, 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)
> @@ -483,6 +471,7 @@ int intel_guc_ucode_load(struct drm_device *dev)
>  
>  fail:
>  	DRM_ERROR("GuC firmware load failed, err %d\n", err);
> +
>  	if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_PENDING)
>  		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
>  
> @@ -490,6 +479,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;
>  }
>  
> @@ -631,8 +643,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;
> @@ -641,26 +656,21 @@ void intel_guc_ucode_init(struct drm_device *dev)
>  		guc_fw->guc_fw_major_wanted = 6;
>  		guc_fw->guc_fw_minor_wanted = 1;
>  	} 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);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 4f1dfe6..df698d7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1758,7 +1758,7 @@ int intel_guc_reset(struct drm_i915_private *dev_priv)
>  	int ret;
>  	unsigned long irqflags;
>  
> -	if (!i915.enable_guc_submission)
> +	if (!HAS_GUC_UCODE(dev_priv))
>  		return -EINVAL;
>  
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dave Gordon April 27, 2016, 9:43 a.m. UTC | #2
On 26/04/16 15:18, Daniel Vetter wrote:
> On Mon, Apr 25, 2016 at 03:37:11PM +0100, Dave Gordon wrote:
>> 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,
>> but not yet to use it for submission. A later patch will change
>> to enable it if appropriate.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>
> intel-gfx-trybot@fd.o is your CI_only patchwork and CI instance:
>
> https://patchwork.freedesktop.org/project/intel-gfx-trybot/series/?ordering=-last_updated
>
> Cheers, Daniel

Yes, I tried sending this to trybot several times before this, but it 
wasn't working well, with many tests giving spurious "INCOMPLETE" 
results, especially on the Romanian machines. So this was still the only 
was to get a good test run.

Hopefully trybot will be back in working order soon, but that link shows 
there has not been a single successful submission by anybody since 1st 
April!

.Dave.
Daniel Vetter April 28, 2016, 8:04 a.m. UTC | #3
On Wed, Apr 27, 2016 at 10:43:59AM +0100, Dave Gordon wrote:
> On 26/04/16 15:18, Daniel Vetter wrote:
> >On Mon, Apr 25, 2016 at 03:37:11PM +0100, Dave Gordon wrote:
> >>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,
> >>but not yet to use it for submission. A later patch will change
> >>to enable it if appropriate.
> >>
> >>Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> >
> >intel-gfx-trybot@fd.o is your CI_only patchwork and CI instance:
> >
> >https://patchwork.freedesktop.org/project/intel-gfx-trybot/series/?ordering=-last_updated
> >
> >Cheers, Daniel
> 
> Yes, I tried sending this to trybot several times before this, but it wasn't
> working well, with many tests giving spurious "INCOMPLETE" results,
> especially on the Romanian machines. So this was still the only was to get a
> good test run.
> 
> Hopefully trybot will be back in working order soon, but that link shows
> there has not been a single successful submission by anybody since 1st
> April!

You need to escalate this all over the place. CI is meant as a service for
developers, if it doesn't work that's pretty critical (and imo much more
important than getting the next feature in).
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 261a3ef..1ea4546 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4914,7 +4914,6 @@  int i915_gem_init_engines(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_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index d40c13f..a542664 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -970,7 +970,7 @@  int intel_guc_suspend(struct drm_device *dev)
 	struct intel_context *ctx;
 	u32 data[3];
 
-	if (!i915.enable_guc_submission)
+	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
 		return 0;
 
 	ctx = dev_priv->kernel_context;
@@ -996,7 +996,7 @@  int intel_guc_resume(struct drm_device *dev)
 	struct intel_context *ctx;
 	u32 data[3];
 
-	if (!i915.enable_guc_submission)
+	if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
 		return 0;
 
 	ctx = dev_priv->kernel_context;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 1779f02..79be9f8 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 = 0,
 	.guc_log_level = -1,
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
@@ -197,8 +198,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_loading,
+		"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, 0=never [default], 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 02bc278..9f1d17b 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;
@@ -57,7 +59,6 @@  struct i915_params {
 	bool load_detect_test;
 	bool reset;
 	bool disable_display;
-	bool enable_guc_submission;
 	bool verbose_state_checks;
 	bool nuclear_pageflip;
 	bool enable_dp_mst;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 876e5da..2ec9cf1 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -387,49 +387,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 retries, 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)
@@ -483,6 +471,7 @@  int intel_guc_ucode_load(struct drm_device *dev)
 
 fail:
 	DRM_ERROR("GuC firmware load failed, err %d\n", err);
+
 	if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_PENDING)
 		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
 
@@ -490,6 +479,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;
 }
 
@@ -631,8 +643,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;
@@ -641,26 +656,21 @@  void intel_guc_ucode_init(struct drm_device *dev)
 		guc_fw->guc_fw_major_wanted = 6;
 		guc_fw->guc_fw_minor_wanted = 1;
 	} 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);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 4f1dfe6..df698d7 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1758,7 +1758,7 @@  int intel_guc_reset(struct drm_i915_private *dev_priv)
 	int ret;
 	unsigned long irqflags;
 
-	if (!i915.enable_guc_submission)
+	if (!HAS_GUC_UCODE(dev_priv))
 		return -EINVAL;
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);