| Message ID | 20170512150300.69300-1-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 12, 2017 at 03:02:58PM +0000, Michal Wajdeczko wrote: > In earlier patch 789a625 we were enabling send function only > after successful init. For completeness, we should make sure > that we disable it on fini. > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_uc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index 07c5658..940a3c9 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -412,8 +412,11 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) > > if (i915.enable_guc_submission) { > i915_guc_submission_disable(dev_priv); > + guc_disable_communication(&dev_priv->guc); > gen9_disable_guc_interrupts(dev_priv); > i915_guc_submission_fini(dev_priv); > + } else { > + guc_disable_communication(&dev_priv->guc); > } Hmm, is the order that sensitive? Do we initialise it in a different order depending on guc submission? Seems dubious. -Chris
On Fri, May 12, 2017 at 04:17:00PM +0100, Chris Wilson wrote: > On Fri, May 12, 2017 at 03:02:58PM +0000, Michal Wajdeczko wrote: > > In earlier patch 789a625 we were enabling send function only > > after successful init. For completeness, we should make sure > > that we disable it on fini. > > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/intel_uc.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > > index 07c5658..940a3c9 100644 > > --- a/drivers/gpu/drm/i915/intel_uc.c > > +++ b/drivers/gpu/drm/i915/intel_uc.c > > @@ -412,8 +412,11 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) > > > > if (i915.enable_guc_submission) { > > i915_guc_submission_disable(dev_priv); > > + guc_disable_communication(&dev_priv->guc); > > gen9_disable_guc_interrupts(dev_priv); > > i915_guc_submission_fini(dev_priv); > > + } else { > > + guc_disable_communication(&dev_priv->guc); > > } > > Hmm, is the order that sensitive? Do we initialise it in a different > order depending on guc submission? Seems dubious. Order is the same, but some steps are omitted. With submission enabled: 1) i915_ggtt_enable_guc(dev_priv); 2) i915_guc_submission_init(dev_priv); 3) intel_guc_init_hw(&dev_priv->guc); 4) guc_enable_communication(guc); 5) gen9_enable_guc_interrupts(dev_priv); 6) i915_guc_submission_enable(dev_priv); without: 1) i915_ggtt_enable_guc(dev_priv); 3) intel_guc_init_hw(&dev_priv->guc); 4) guc_enable_communication(guc); Alternatively, we can split "if (i915.enable_guc_submission)" into two parts, and put guc_disable_communication() in the middle. -Michal
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 07c5658..940a3c9 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -412,8 +412,11 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) if (i915.enable_guc_submission) { i915_guc_submission_disable(dev_priv); + guc_disable_communication(&dev_priv->guc); gen9_disable_guc_interrupts(dev_priv); i915_guc_submission_fini(dev_priv); + } else { + guc_disable_communication(&dev_priv->guc); } i915_ggtt_disable_guc(dev_priv); }
In earlier patch 789a625 we were enabling send function only after successful init. For completeness, we should make sure that we disable it on fini. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_uc.c | 3 +++ 1 file changed, 3 insertions(+)