| Message ID | 1506055758-25442-2-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 22 Sep 2017 06:49:12 +0200, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > Prepared generic helpers intel_uc_suspend, intel_uc_resume, > intel_uc_runtime_suspend, intel_uc_runtime_resume. Added > error handling to all the calls for suspend/resume. > > v2: Rebase w.r.t removal of GuC code restructuring. > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 23 ++++++++++++++++++++--- > drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- > drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++ > drivers/gpu/drm/i915/intel_uc.h | 4 ++++ > 4 files changed, 50 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > index 5c111ea..8635f40 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device *dev) > } > mutex_unlock(&dev->struct_mutex); > - intel_guc_resume(dev_priv); > + /* > + * NB: Full gem reinitialization is being done above, hence > + * intel_uc_resume will be of no use. Currently intel_uc_resume > + * is nop. If full reinitialization is removed, will need to put > + * functionality to resume from sleep in intel_uc_resume. > + */ > + ret = intel_uc_resume(dev_priv); > + if (ret) > + DRM_ERROR("failed to resume uc\n"); > intel_modeset_init_hw(dev); > @@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device > *kdev) > */ > i915_gem_runtime_suspend(dev_priv); > - intel_guc_suspend(dev_priv); > + ret = intel_uc_runtime_suspend(dev_priv); > + if (ret) { > + DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", ret); > + enable_rpm_wakeref_asserts(dev_priv); > + return ret; > + } > intel_runtime_pm_disable_interrupts(dev_priv); > @@ -2578,7 +2591,11 @@ static int intel_runtime_resume(struct device > *kdev) > if (intel_uncore_unclaimed_mmio(dev_priv)) > DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n"); > - intel_guc_resume(dev_priv); > + ret = intel_uc_runtime_resume(dev_priv); > + if (ret) { > + DRM_ERROR("uc runtime resume failed (%d)\n", ret); > + return ret; > + } > if (IS_GEN9_LP(dev_priv)) { > bxt_disable_dc9(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c > index c4bf348..dd56d45 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4539,7 +4539,11 @@ int i915_gem_suspend(struct drm_i915_private > *dev_priv) > i915_gem_contexts_lost(dev_priv); > mutex_unlock(&dev->struct_mutex); > - intel_guc_suspend(dev_priv); > + ret = intel_uc_suspend(dev_priv); Is it ok that we suspend uc in 'gem_suspend' but resume in 'drm_resume' ? Michal > + if (ret) { > + DRM_ERROR("uc suspend failed (%d)\n", ret); > + goto out; > + } > cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); > cancel_delayed_work_sync(&dev_priv->gt.retire_work); > @@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private > *dev_priv) > err_unlock: > mutex_unlock(&dev->struct_mutex); > +out: > intel_runtime_pm_put(dev_priv); > return ret; > } > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index 0178ba4..8e4d8b0 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc > *guc) > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > + > +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv) > +{ > + return intel_guc_suspend(dev_priv); > +} > + > +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv) > +{ > + return intel_guc_resume(dev_priv); > +} > + > +int intel_uc_suspend(struct drm_i915_private *dev_priv) > +{ > + return intel_guc_suspend(dev_priv); > +} > + > +int intel_uc_resume(struct drm_i915_private *dev_priv) > +{ > + return 0; > +} > diff --git a/drivers/gpu/drm/i915/intel_uc.h > b/drivers/gpu/drm/i915/intel_uc.h > index 7703c9a..3d33a51 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -208,6 +208,10 @@ struct intel_huc { > void intel_uc_fini_fw(struct drm_i915_private *dev_priv); > int intel_uc_init_hw(struct drm_i915_private *dev_priv); > void intel_uc_fini_hw(struct drm_i915_private *dev_priv); > +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv); > +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv); > +int intel_uc_suspend(struct drm_i915_private *dev_priv); > +int intel_uc_resume(struct drm_i915_private *dev_priv); > int intel_guc_sample_forcewake(struct intel_guc *guc); > int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 > len); > int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 > len);
On 9/22/2017 12:41 PM, Michal Wajdeczko wrote: > On Fri, 22 Sep 2017 06:49:12 +0200, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> Prepared generic helpers intel_uc_suspend, intel_uc_resume, >> intel_uc_runtime_suspend, intel_uc_runtime_resume. Added >> error handling to all the calls for suspend/resume. >> >> v2: Rebase w.r.t removal of GuC code restructuring. >> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Michał Winiarski <michal.winiarski@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 23 ++++++++++++++++++++--- >> drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- >> drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_uc.h | 4 ++++ >> 4 files changed, 50 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c >> b/drivers/gpu/drm/i915/i915_drv.c >> index 5c111ea..8635f40 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device >> *dev) >> } >> mutex_unlock(&dev->struct_mutex); >> - intel_guc_resume(dev_priv); >> + /* >> + * NB: Full gem reinitialization is being done above, hence >> + * intel_uc_resume will be of no use. Currently intel_uc_resume >> + * is nop. If full reinitialization is removed, will need to put >> + * functionality to resume from sleep in intel_uc_resume. >> + */ >> + ret = intel_uc_resume(dev_priv); >> + if (ret) >> + DRM_ERROR("failed to resume uc\n"); >> intel_modeset_init_hw(dev); >> @@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device >> *kdev) >> */ >> i915_gem_runtime_suspend(dev_priv); >> - intel_guc_suspend(dev_priv); >> + ret = intel_uc_runtime_suspend(dev_priv); >> + if (ret) { >> + DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", >> ret); >> + enable_rpm_wakeref_asserts(dev_priv); >> + return ret; >> + } >> intel_runtime_pm_disable_interrupts(dev_priv); >> @@ -2578,7 +2591,11 @@ static int intel_runtime_resume(struct device >> *kdev) >> if (intel_uncore_unclaimed_mmio(dev_priv)) >> DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n"); >> - intel_guc_resume(dev_priv); >> + ret = intel_uc_runtime_resume(dev_priv); >> + if (ret) { >> + DRM_ERROR("uc runtime resume failed (%d)\n", ret); >> + return ret; >> + } >> if (IS_GEN9_LP(dev_priv)) { >> bxt_disable_dc9(dev_priv); >> diff --git a/drivers/gpu/drm/i915/i915_gem.c >> b/drivers/gpu/drm/i915/i915_gem.c >> index c4bf348..dd56d45 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -4539,7 +4539,11 @@ int i915_gem_suspend(struct drm_i915_private >> *dev_priv) >> i915_gem_contexts_lost(dev_priv); >> mutex_unlock(&dev->struct_mutex); >> - intel_guc_suspend(dev_priv); >> + ret = intel_uc_suspend(dev_priv); > > Is it ok that we suspend uc in 'gem_suspend' but resume in 'drm_resume' ? > > Michal Currently it is fine as we are doing full reinit. But in future, it is better to have it in i915_gem_resume. Will move it there. Thanks. > >> + if (ret) { >> + DRM_ERROR("uc suspend failed (%d)\n", ret); >> + goto out; >> + } >> cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); >> cancel_delayed_work_sync(&dev_priv->gt.retire_work); >> @@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private >> *dev_priv) >> err_unlock: >> mutex_unlock(&dev->struct_mutex); >> +out: >> intel_runtime_pm_put(dev_priv); >> return ret; >> } >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index 0178ba4..8e4d8b0 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc >> *guc) >> return intel_guc_send(guc, action, ARRAY_SIZE(action)); >> } >> + >> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv) >> +{ >> + return intel_guc_suspend(dev_priv); >> +} >> + >> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv) >> +{ >> + return intel_guc_resume(dev_priv); >> +} >> + >> +int intel_uc_suspend(struct drm_i915_private *dev_priv) >> +{ >> + return intel_guc_suspend(dev_priv); >> +} >> + >> +int intel_uc_resume(struct drm_i915_private *dev_priv) >> +{ >> + return 0; >> +} >> diff --git a/drivers/gpu/drm/i915/intel_uc.h >> b/drivers/gpu/drm/i915/intel_uc.h >> index 7703c9a..3d33a51 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.h >> +++ b/drivers/gpu/drm/i915/intel_uc.h >> @@ -208,6 +208,10 @@ struct intel_huc { >> void intel_uc_fini_fw(struct drm_i915_private *dev_priv); >> int intel_uc_init_hw(struct drm_i915_private *dev_priv); >> void intel_uc_fini_hw(struct drm_i915_private *dev_priv); >> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv); >> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv); >> +int intel_uc_suspend(struct drm_i915_private *dev_priv); >> +int intel_uc_resume(struct drm_i915_private *dev_priv); >> int intel_guc_sample_forcewake(struct intel_guc *guc); >> int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 >> len); >> int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, >> u32 len);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 5c111ea..8635f40 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device *dev) } mutex_unlock(&dev->struct_mutex); - intel_guc_resume(dev_priv); + /* + * NB: Full gem reinitialization is being done above, hence + * intel_uc_resume will be of no use. Currently intel_uc_resume + * is nop. If full reinitialization is removed, will need to put + * functionality to resume from sleep in intel_uc_resume. + */ + ret = intel_uc_resume(dev_priv); + if (ret) + DRM_ERROR("failed to resume uc\n"); intel_modeset_init_hw(dev); @@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device *kdev) */ i915_gem_runtime_suspend(dev_priv); - intel_guc_suspend(dev_priv); + ret = intel_uc_runtime_suspend(dev_priv); + if (ret) { + DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", ret); + enable_rpm_wakeref_asserts(dev_priv); + return ret; + } intel_runtime_pm_disable_interrupts(dev_priv); @@ -2578,7 +2591,11 @@ static int intel_runtime_resume(struct device *kdev) if (intel_uncore_unclaimed_mmio(dev_priv)) DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n"); - intel_guc_resume(dev_priv); + ret = intel_uc_runtime_resume(dev_priv); + if (ret) { + DRM_ERROR("uc runtime resume failed (%d)\n", ret); + return ret; + } if (IS_GEN9_LP(dev_priv)) { bxt_disable_dc9(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index c4bf348..dd56d45 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4539,7 +4539,11 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) i915_gem_contexts_lost(dev_priv); mutex_unlock(&dev->struct_mutex); - intel_guc_suspend(dev_priv); + ret = intel_uc_suspend(dev_priv); + if (ret) { + DRM_ERROR("uc suspend failed (%d)\n", ret); + goto out; + } cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work); cancel_delayed_work_sync(&dev_priv->gt.retire_work); @@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) err_unlock: mutex_unlock(&dev->struct_mutex); +out: intel_runtime_pm_put(dev_priv); return ret; } diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 0178ba4..8e4d8b0 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc *guc) return intel_guc_send(guc, action, ARRAY_SIZE(action)); } + +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv) +{ + return intel_guc_suspend(dev_priv); +} + +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv) +{ + return intel_guc_resume(dev_priv); +} + +int intel_uc_suspend(struct drm_i915_private *dev_priv) +{ + return intel_guc_suspend(dev_priv); +} + +int intel_uc_resume(struct drm_i915_private *dev_priv) +{ + return 0; +} diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 7703c9a..3d33a51 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -208,6 +208,10 @@ struct intel_huc { void intel_uc_fini_fw(struct drm_i915_private *dev_priv); int intel_uc_init_hw(struct drm_i915_private *dev_priv); void intel_uc_fini_hw(struct drm_i915_private *dev_priv); +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv); +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv); +int intel_uc_suspend(struct drm_i915_private *dev_priv); +int intel_uc_resume(struct drm_i915_private *dev_priv); int intel_guc_sample_forcewake(struct intel_guc *guc); int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
Prepared generic helpers intel_uc_suspend, intel_uc_resume, intel_uc_runtime_suspend, intel_uc_runtime_resume. Added error handling to all the calls for suspend/resume. v2: Rebase w.r.t removal of GuC code restructuring. Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 23 ++++++++++++++++++++--- drivers/gpu/drm/i915/i915_gem.c | 7 ++++++- drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++ drivers/gpu/drm/i915/intel_uc.h | 4 ++++ 4 files changed, 50 insertions(+), 4 deletions(-)