| Message ID | 20190730230743.19542-1-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/5] drm/i915/uc: Don't enable communication twice on resume | expand |
Quoting Daniele Ceraolo Spurio (2019-07-31 00:07:39) > When coming out of S3/S4 we sanitize and re-init the HW, which includes > enabling communication during uc_init_hw. We therefore don't want to do > that again in uc_resume and can just tell GuC to reload its state. > > v2: split uc_resume and uc_runtime_resume to match the suspend > functions and to better differentiate the expected state in the 2 > scenarios (Chris) > > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 35 +++++++++++++++++++++++++-- > drivers/gpu/drm/i915/gt/uc/intel_uc.h | 1 + > drivers/gpu/drm/i915/i915_drv.c | 4 +-- > 3 files changed, 36 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > index 6eb8bb3fa252..657fdcb70d00 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > @@ -233,11 +233,20 @@ static void guc_disable_interrupts(struct intel_guc *guc) > guc->interrupts.disable(guc); > } > > +#ifdef CONFIG_DRM_I915_DEBUG_GEM > +static bool guc_communication_enabled(struct intel_guc *guc) > +{ > + return guc->send != intel_guc_send_nop; > +} > +#endif > + > static int guc_enable_communication(struct intel_guc *guc) > { > struct drm_i915_private *i915 = guc_to_gt(guc)->i915; > int ret; > > + GEM_BUG_ON(guc_communication_enabled(guc)); > + > ret = intel_guc_ct_enable(&guc->ct); > if (ret) > return ret; > @@ -550,7 +559,7 @@ void intel_uc_suspend(struct intel_uc *uc) > intel_uc_runtime_suspend(uc); > } > > -int intel_uc_resume(struct intel_uc *uc) > +static int __uc_resume(struct intel_uc *uc, bool enable_communication) > { > struct intel_guc *guc = &uc->guc; > int err; > @@ -558,7 +567,11 @@ int intel_uc_resume(struct intel_uc *uc) > if (!intel_guc_is_running(guc)) > return 0; > > - guc_enable_communication(guc); > + /* Make sure we enable communication if and only if it's disabled */ > + GEM_BUG_ON(enable_communication == guc_communication_enabled(guc)); > + > + if (enable_communication) > + guc_enable_communication(guc); > > err = intel_guc_resume(guc); > if (err) { > @@ -568,3 +581,21 @@ int intel_uc_resume(struct intel_uc *uc) > > return 0; > } > + > +int intel_uc_resume(struct intel_uc *uc) > +{ > + /* > + * When coming out of S3/S4 we sanitize and re-init the HW, so > + * communication is already re-enabled at this point. > + */ > + return __uc_resume(uc, false); > +} > + > +int intel_uc_runtime_resume(struct intel_uc *uc) > +{ > + /* > + * During runtime resume we don't sanitize, so we need to re-init > + * communication as well. > + */ > + return __uc_resume(uc, true); > +} > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h > index fe3362fd7706..25da51e95417 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h > @@ -47,6 +47,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc); > void intel_uc_suspend(struct intel_uc *uc); > void intel_uc_runtime_suspend(struct intel_uc *uc); > int intel_uc_resume(struct intel_uc *uc); > +int intel_uc_runtime_resume(struct intel_uc *uc); > > static inline bool intel_uc_is_using_guc(struct intel_uc *uc) > { > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index f2d3d754af37..761726818a22 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -2950,7 +2950,7 @@ static int intel_runtime_suspend(struct device *kdev) > > intel_runtime_pm_enable_interrupts(dev_priv); > > - intel_uc_resume(&dev_priv->gt.uc); > + intel_uc_runtime_resume(&dev_priv->gt.uc); > > intel_gt_init_swizzling(&dev_priv->gt); > i915_gem_restore_fences(dev_priv); > @@ -3047,7 +3047,7 @@ static int intel_runtime_resume(struct device *kdev) > > intel_runtime_pm_enable_interrupts(dev_priv); > > - intel_uc_resume(&dev_priv->gt.uc); > + intel_uc_runtime_resume(&dev_priv->gt.uc); Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Wed, 31 Jul 2019 01:07:39 +0200, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: > When coming out of S3/S4 we sanitize and re-init the HW, which includes > enabling communication during uc_init_hw. We therefore don't want to do > that again in uc_resume and can just tell GuC to reload its state. > > v2: split uc_resume and uc_runtime_resume to match the suspend > functions and to better differentiate the expected state in the 2 > scenarios (Chris) > > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c index 6eb8bb3fa252..657fdcb70d00 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c @@ -233,11 +233,20 @@ static void guc_disable_interrupts(struct intel_guc *guc) guc->interrupts.disable(guc); } +#ifdef CONFIG_DRM_I915_DEBUG_GEM +static bool guc_communication_enabled(struct intel_guc *guc) +{ + return guc->send != intel_guc_send_nop; +} +#endif + static int guc_enable_communication(struct intel_guc *guc) { struct drm_i915_private *i915 = guc_to_gt(guc)->i915; int ret; + GEM_BUG_ON(guc_communication_enabled(guc)); + ret = intel_guc_ct_enable(&guc->ct); if (ret) return ret; @@ -550,7 +559,7 @@ void intel_uc_suspend(struct intel_uc *uc) intel_uc_runtime_suspend(uc); } -int intel_uc_resume(struct intel_uc *uc) +static int __uc_resume(struct intel_uc *uc, bool enable_communication) { struct intel_guc *guc = &uc->guc; int err; @@ -558,7 +567,11 @@ int intel_uc_resume(struct intel_uc *uc) if (!intel_guc_is_running(guc)) return 0; - guc_enable_communication(guc); + /* Make sure we enable communication if and only if it's disabled */ + GEM_BUG_ON(enable_communication == guc_communication_enabled(guc)); + + if (enable_communication) + guc_enable_communication(guc); err = intel_guc_resume(guc); if (err) { @@ -568,3 +581,21 @@ int intel_uc_resume(struct intel_uc *uc) return 0; } + +int intel_uc_resume(struct intel_uc *uc) +{ + /* + * When coming out of S3/S4 we sanitize and re-init the HW, so + * communication is already re-enabled at this point. + */ + return __uc_resume(uc, false); +} + +int intel_uc_runtime_resume(struct intel_uc *uc) +{ + /* + * During runtime resume we don't sanitize, so we need to re-init + * communication as well. + */ + return __uc_resume(uc, true); +} diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h index fe3362fd7706..25da51e95417 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h @@ -47,6 +47,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc); void intel_uc_suspend(struct intel_uc *uc); void intel_uc_runtime_suspend(struct intel_uc *uc); int intel_uc_resume(struct intel_uc *uc); +int intel_uc_runtime_resume(struct intel_uc *uc); static inline bool intel_uc_is_using_guc(struct intel_uc *uc) { diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f2d3d754af37..761726818a22 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2950,7 +2950,7 @@ static int intel_runtime_suspend(struct device *kdev) intel_runtime_pm_enable_interrupts(dev_priv); - intel_uc_resume(&dev_priv->gt.uc); + intel_uc_runtime_resume(&dev_priv->gt.uc); intel_gt_init_swizzling(&dev_priv->gt); i915_gem_restore_fences(dev_priv); @@ -3047,7 +3047,7 @@ static int intel_runtime_resume(struct device *kdev) intel_runtime_pm_enable_interrupts(dev_priv); - intel_uc_resume(&dev_priv->gt.uc); + intel_uc_runtime_resume(&dev_priv->gt.uc); /* * No point of rolling back things in case of an error, as the best
When coming out of S3/S4 we sanitize and re-init the HW, which includes enabling communication during uc_init_hw. We therefore don't want to do that again in uc_resume and can just tell GuC to reload its state. v2: split uc_resume and uc_runtime_resume to match the suspend functions and to better differentiate the expected state in the 2 scenarios (Chris) Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/uc/intel_uc.c | 35 +++++++++++++++++++++++++-- drivers/gpu/drm/i915/gt/uc/intel_uc.h | 1 + drivers/gpu/drm/i915/i915_drv.c | 4 +-- 3 files changed, 36 insertions(+), 4 deletions(-)