drm/i915/guc: Clear enable_guc_loading in case of init failure

Submitted by Michel Thierry on June 2, 2017, 11:46 p.m.

Details

Message ID 20170602234654.31614-1-michel.thierry@intel.com
State New
Headers show
Series "drm/i915/guc: Clear enable_guc_loading in case of init failure" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Michel Thierry June 2, 2017, 11:46 p.m.
And prevent calling i915_ggtt_disable_guc twice (the first when GuC init
failed, and the second time during driver unload / intel_uc_fini_hw),
and hitting the GEM_BUG_ON.

Fixes: 04f7b24eccdf ("drm/i915/guc: Assert that we switch between known
ggtt->invalidate functions")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 5 +++++
 1 file changed, 5 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3524ff07a0f2..c5ef4fa2b404 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -433,6 +433,11 @@  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
 	}
 
+	if (i915.enable_guc_loading) {
+		i915.enable_guc_loading = 0;
+		DRM_NOTE("GuC firmware loading disabled\n");
+	}
+
 	return ret;
 }
 

Comments

Quoting Michel Thierry (2017-06-03 00:46:54)
> And prevent calling i915_ggtt_disable_guc twice (the first when GuC init
> failed, and the second time during driver unload / intel_uc_fini_hw),
> and hitting the GEM_BUG_ON.
> 
> Fixes: 04f7b24eccdf ("drm/i915/guc: Assert that we switch between known
> ggtt->invalidate functions")

Go back to the earlier culprit; that is just the messenger.
-Chris
On Fri, Jun 02, 2017 at 04:46:54PM -0700, Michel Thierry wrote:
> And prevent calling i915_ggtt_disable_guc twice (the first when GuC init
> failed, and the second time during driver unload / intel_uc_fini_hw),
> and hitting the GEM_BUG_ON.
> 
> Fixes: 04f7b24eccdf ("drm/i915/guc: Assert that we switch between known
> ggtt->invalidate functions")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 3524ff07a0f2..c5ef4fa2b404 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -433,6 +433,11 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>  		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
>  	}
>  
> +	if (i915.enable_guc_loading) {

This check is redundant (see top of this function)

Michal

> +		i915.enable_guc_loading = 0;
> +		DRM_NOTE("GuC firmware loading disabled\n");
> +	}
> +
>  	return ret;
>  }
>  
> -- 
> 2.11.0
>
You'll also need to change the order of operations in intel_uc_fini_hw 
to make sure guc_free_load_err_log is called before the 
i915.enable_guc_loading check, because that log exists exactly when GuC 
loading failed.

Thanks,
Daniele

On 02/06/17 16:46, Michel Thierry wrote:
> And prevent calling i915_ggtt_disable_guc twice (the first when GuC init
> failed, and the second time during driver unload / intel_uc_fini_hw),
> and hitting the GEM_BUG_ON.
>
> Fixes: 04f7b24eccdf ("drm/i915/guc: Assert that we switch between known
> ggtt->invalidate functions")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 3524ff07a0f2..c5ef4fa2b404 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -433,6 +433,11 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>  		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
>  	}
>
> +	if (i915.enable_guc_loading) {
> +		i915.enable_guc_loading = 0;
> +		DRM_NOTE("GuC firmware loading disabled\n");
> +	}
> +
>  	return ret;
>  }
>
>
On 6/5/2017 10:32 AM, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/guc: Clear enable_guc_loading in case of init failure (rev2)
> URL   : https://patchwork.freedesktop.org/series/25228/
> State : success
> 
> == Summary ==
> 
> Series 25228v2 drm/i915/guc: Clear enable_guc_loading in case of init failure
> https://patchwork.freedesktop.org/api/1.0/series/25228/revisions/2/mbox/
> 
...
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4878/
> 

Hello,

Michal also reviewed this patch already (thanks Michal), let me know if 
something else is needed.

-Michel
On ti, 2017-06-06 at 12:57 -0700, Michel Thierry wrote:
> On 6/5/2017 10:32 AM, Patchwork wrote:
> > 
> > == Series Details ==
> > 
> > Series: drm/i915/guc: Clear enable_guc_loading in case of init failure (rev2)
> > URL   : https://patchwork.freedesktop.org/series/25228/
> > State : success
> > 
> > == Summary ==
> > 
> > Series 25228v2 drm/i915/guc: Clear enable_guc_loading in case of init failure
> > https://patchwork.freedesktop.org/api/1.0/series/25228/revisions/2/mbox/
> > 
> ...
> > 
> > 
> > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4878/
> > 
> 
> Hello,
> 
> Michal also reviewed this patch already (thanks Michal), let me know if 
> something else is needed.

Applied it. Thanks for the patch and review.

Regards, Joonas