| Message ID | 20170602234654.31614-1-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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; }
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(+)