| Message ID | 20190517162227.6436-5-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | GuC fixes | expand |
Quoting Michal Wajdeczko (2019-05-17 17:22:24) > Knowing that GuC will be reset soon, we may stop all communication > immediately without doing graceful cleanup as it is not needed. > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > --- > drivers/gpu/drm/i915/intel_guc_ct.h | 5 +++++ > drivers/gpu/drm/i915/intel_uc.c | 13 ++++++++++++- > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h > index f5e7f0663304..41ba593a4df7 100644 > --- a/drivers/gpu/drm/i915/intel_guc_ct.h > +++ b/drivers/gpu/drm/i915/intel_guc_ct.h > @@ -96,4 +96,9 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct); > int intel_guc_ct_enable(struct intel_guc_ct *ct); > void intel_guc_ct_disable(struct intel_guc_ct *ct); > > +static inline void intel_guc_ct_stop(struct intel_guc_ct *ct) > +{ > + ct->host_channel.enabled = false; > +} > + > #endif /* _INTEL_GUC_CT_H_ */ > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c > index 9d86cd831ea7..86edfa5ad72e 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -224,6 +224,17 @@ static int guc_enable_communication(struct intel_guc *guc) > return 0; > } > > +static void guc_stop_communication(struct intel_guc *guc) > +{ > + struct drm_i915_private *i915 = guc_to_i915(guc); > + > + if (HAS_GUC_CT(i915)) Does this not fall out of some local knowledge? Though for the moment it is harmless to stop a non-existent intel_guc_ct. > + intel_guc_ct_stop(&guc->ct); Any serialisation required here? Or caveats for callers to observe? > + guc->send = intel_guc_send_nop; > + guc->handler = intel_guc_to_host_event_handler_nop; > +}
On Fri, 17 May 2019 18:30:40 +0200, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Michal Wajdeczko (2019-05-17 17:22:24) >> Knowing that GuC will be reset soon, we may stop all communication >> immediately without doing graceful cleanup as it is not needed. >> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> --- >> drivers/gpu/drm/i915/intel_guc_ct.h | 5 +++++ >> drivers/gpu/drm/i915/intel_uc.c | 13 ++++++++++++- >> 2 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h >> b/drivers/gpu/drm/i915/intel_guc_ct.h >> index f5e7f0663304..41ba593a4df7 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_ct.h >> +++ b/drivers/gpu/drm/i915/intel_guc_ct.h >> @@ -96,4 +96,9 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct); >> int intel_guc_ct_enable(struct intel_guc_ct *ct); >> void intel_guc_ct_disable(struct intel_guc_ct *ct); >> >> +static inline void intel_guc_ct_stop(struct intel_guc_ct *ct) >> +{ >> + ct->host_channel.enabled = false; >> +} >> + >> #endif /* _INTEL_GUC_CT_H_ */ >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index 9d86cd831ea7..86edfa5ad72e 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -224,6 +224,17 @@ static int guc_enable_communication(struct >> intel_guc *guc) >> return 0; >> } >> >> +static void guc_stop_communication(struct intel_guc *guc) >> +{ >> + struct drm_i915_private *i915 = guc_to_i915(guc); >> + >> + if (HAS_GUC_CT(i915)) > > Does this not fall out of some local knowledge? Though for the moment it > is harmless to stop a non-existent intel_guc_ct. We do have CT init/enable/disable/fini functions (all guarded by HAS_GUC_CT) so at the moment we are here, we know that CT was already enabled. > >> + intel_guc_ct_stop(&guc->ct); > > Any serialisation required here? This function might be called from atomic context, so rather not. > Or caveats for callers to observe? As we are resetting the GuC, we simply does not allow any further communication with it, any abusers will see something like this: <4> [322.401614] Unexpected send: action=0x4000 <4> [322.401648] WARNING: CPU: 3 PID: 1357 at drivers/gpu/drm/i915/intel_guc.c:405 intel_guc_send_nop+0xe/0x20 [i915] > >> + guc->send = intel_guc_send_nop; >> + guc->handler = intel_guc_to_host_event_handler_nop; >> +}
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h index f5e7f0663304..41ba593a4df7 100644 --- a/drivers/gpu/drm/i915/intel_guc_ct.h +++ b/drivers/gpu/drm/i915/intel_guc_ct.h @@ -96,4 +96,9 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct); int intel_guc_ct_enable(struct intel_guc_ct *ct); void intel_guc_ct_disable(struct intel_guc_ct *ct); +static inline void intel_guc_ct_stop(struct intel_guc_ct *ct) +{ + ct->host_channel.enabled = false; +} + #endif /* _INTEL_GUC_CT_H_ */ diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 9d86cd831ea7..86edfa5ad72e 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -224,6 +224,17 @@ static int guc_enable_communication(struct intel_guc *guc) return 0; } +static void guc_stop_communication(struct intel_guc *guc) +{ + struct drm_i915_private *i915 = guc_to_i915(guc); + + if (HAS_GUC_CT(i915)) + intel_guc_ct_stop(&guc->ct); + + guc->send = intel_guc_send_nop; + guc->handler = intel_guc_to_host_event_handler_nop; +} + static void guc_disable_communication(struct intel_guc *guc) { struct drm_i915_private *i915 = guc_to_i915(guc); @@ -488,7 +499,7 @@ void intel_uc_reset_prepare(struct drm_i915_private *i915) if (!USES_GUC(i915)) return; - guc_disable_communication(guc); + guc_stop_communication(guc); __uc_sanitize(i915); }
Knowing that GuC will be reset soon, we may stop all communication immediately without doing graceful cleanup as it is not needed. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> --- drivers/gpu/drm/i915/intel_guc_ct.h | 5 +++++ drivers/gpu/drm/i915/intel_uc.c | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-)