| Message ID | 1507712056-25030-10-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 11 Oct 2017 10:54:04 +0200, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > Prepared generic helpers intel_uc_suspend, intel_uc_resume. These are > called from respective GEM functions. Only exception is intel_uc_resume > that needs to be called w/ or w/o GuC loaded in i915_drm_resume path. > Changes to add WOPCM condition check to load GuC during resume will be > added in later patches. > > v2: Rebase w.r.t removal of GuC code restructuring. > > v3: Calling intel_uc_resume from i915_gem_resume post resuming > i915 gem setup. This is symmetrical with i915_gem_suspend. > Removed error messages from i915 suspend/resume routines as > uC suspend/resume routines will have those. (Michal Wajdeczko) > Declare wedged on uc_suspend failure and uc_resume failure. > (Michał Winiarski) > Keeping the uC suspend/resume function definitions close to other > uC functions. > > v4: Added implementation to intel_uc_resume as GuC resume is > needed to be triggered post reloading the firmware as well. Added > comments about semantics of GuC resume with the firmware reload. > > v5: Updated return from i915_gem_runtime_suspend. Moved the comment > about GuC reload optimization to intel_uc_init_hw. (Michal Wajdeczko) > Updated comments as FIXME. > > v6: Kept error handling for failure from i915_gem_runtime_suspend only. > We don't want GEM/GuC resume failure to impact intel_runtime_resume or > i915_drm_resume. GEM suspend failure along i915_drm_suspend can also > be ignored as we reset GPU post that. Updated comments. (Chris, Joonas) > > v7: Removed intel_uc_resume from i915_drm_resume as it will be done as > part of intel_uc_init_hw in further patches. Removed TODO comments about > handling GuC load skip on resume. This is to be addressed in further > patches. Added error return from intel_uc_suspend as we plan to add > functionality to resume submission in case of suspend failure in further > patches. Removed runtime uC suspend/resume functions as functionality > will be similar in both paths. > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 17 +++++++++++------ > drivers/gpu/drm/i915/intel_uc.c | 16 ++++++++++++++++ > drivers/gpu/drm/i915/intel_uc.h | 2 ++ > 3 files changed, 29 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c > index 38447ae..7d1b7e1 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2056,11 +2056,13 @@ static void > __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj) > int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv) > { > struct drm_i915_gem_object *obj, *on; > - int i; > + int i, ret; > mutex_lock(&dev_priv->drm.struct_mutex); > - intel_guc_suspend(&dev_priv->guc); > + ret = intel_uc_suspend(dev_priv); > + if (ret) > + goto out_unlock; > /* > * Only called during RPM suspend. All users of the userfault_list > @@ -2098,9 +2100,10 @@ int i915_gem_runtime_suspend(struct > drm_i915_private *dev_priv) > reg->dirty = true; > } > +out_unlock: > mutex_unlock(&dev_priv->drm.struct_mutex); > - return 0; > + return ret; > } > void i915_gem_runtime_resume(struct drm_i915_private *dev_priv) > @@ -2110,7 +2113,7 @@ void i915_gem_runtime_resume(struct > drm_i915_private *dev_priv) > i915_gem_init_swizzling(dev_priv); > i915_gem_restore_fences(dev_priv); > - intel_guc_resume(&dev_priv->guc); > + intel_uc_resume(dev_priv); > mutex_unlock(&dev_priv->drm.struct_mutex); > } > @@ -4681,7 +4684,9 @@ int i915_gem_suspend(struct drm_i915_private > *dev_priv) > i915_gem_set_wedged(dev_priv); /* no hope, discard everything */ > mutex_lock(&dev->struct_mutex); > - intel_guc_suspend(&dev_priv->guc); > + ret = intel_uc_suspend(dev_priv); > + if (ret) > + goto err_unlock; > mutex_unlock(&dev->struct_mutex); > /* > @@ -4730,7 +4735,7 @@ void i915_gem_resume(struct drm_i915_private > *dev_priv) > */ > dev_priv->gt.resume(dev_priv); > - intel_guc_resume(&dev_priv->guc); > + intel_uc_resume(dev_priv); > mutex_unlock(&dev->struct_mutex); > } > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index 7305486..b5c132c 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -283,3 +283,19 @@ void intel_uc_fini_hw(struct drm_i915_private > *dev_priv) > if (i915_modparams.enable_guc_loading) > i915_ggtt_disable_guc(dev_priv); > } > + > +int intel_uc_suspend(struct drm_i915_private *dev_priv) > +{ > + int ret; > + > + ret = intel_guc_suspend(&dev_priv->guc); > + if (ret) > + DRM_ERROR("Failed to suspend GuC\n"); > + > + return ret; > +} > + > +void intel_uc_resume(struct drm_i915_private *dev_priv) > +{ > + intel_guc_resume(&dev_priv->guc); > +} Can we add kerneldoc for above new functions? With doc added Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > diff --git a/drivers/gpu/drm/i915/intel_uc.h > b/drivers/gpu/drm/i915/intel_uc.h > index e18d3bb..e20bc72 100644 > --- a/drivers/gpu/drm/i915/intel_uc.h > +++ b/drivers/gpu/drm/i915/intel_uc.h > @@ -34,5 +34,7 @@ > 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_suspend(struct drm_i915_private *dev_priv); > +void intel_uc_resume(struct drm_i915_private *dev_priv); > #endif
On 10/11/2017 9:27 PM, Michal Wajdeczko wrote: > On Wed, 11 Oct 2017 10:54:04 +0200, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> Prepared generic helpers intel_uc_suspend, intel_uc_resume. These are >> called from respective GEM functions. Only exception is intel_uc_resume >> that needs to be called w/ or w/o GuC loaded in i915_drm_resume path. >> Changes to add WOPCM condition check to load GuC during resume will be >> added in later patches. >> >> v2: Rebase w.r.t removal of GuC code restructuring. >> >> v3: Calling intel_uc_resume from i915_gem_resume post resuming >> i915 gem setup. This is symmetrical with i915_gem_suspend. >> Removed error messages from i915 suspend/resume routines as >> uC suspend/resume routines will have those. (Michal Wajdeczko) >> Declare wedged on uc_suspend failure and uc_resume failure. >> (Michał Winiarski) >> Keeping the uC suspend/resume function definitions close to other >> uC functions. >> >> v4: Added implementation to intel_uc_resume as GuC resume is >> needed to be triggered post reloading the firmware as well. Added >> comments about semantics of GuC resume with the firmware reload. >> >> v5: Updated return from i915_gem_runtime_suspend. Moved the comment >> about GuC reload optimization to intel_uc_init_hw. (Michal Wajdeczko) >> Updated comments as FIXME. >> >> v6: Kept error handling for failure from i915_gem_runtime_suspend only. >> We don't want GEM/GuC resume failure to impact intel_runtime_resume or >> i915_drm_resume. GEM suspend failure along i915_drm_suspend can also >> be ignored as we reset GPU post that. Updated comments. (Chris, Joonas) >> >> v7: Removed intel_uc_resume from i915_drm_resume as it will be done as >> part of intel_uc_init_hw in further patches. Removed TODO comments about >> handling GuC load skip on resume. This is to be addressed in further >> patches. Added error return from intel_uc_suspend as we plan to add >> functionality to resume submission in case of suspend failure in further >> patches. Removed runtime uC suspend/resume functions as functionality >> will be similar in both paths. >> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Michał Winiarski <michal.winiarski@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem.c | 17 +++++++++++------ >> drivers/gpu/drm/i915/intel_uc.c | 16 ++++++++++++++++ >> drivers/gpu/drm/i915/intel_uc.h | 2 ++ >> 3 files changed, 29 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c >> b/drivers/gpu/drm/i915/i915_gem.c >> index 38447ae..7d1b7e1 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -2056,11 +2056,13 @@ static void >> __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj) >> int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv) >> { >> struct drm_i915_gem_object *obj, *on; >> - int i; >> + int i, ret; >> mutex_lock(&dev_priv->drm.struct_mutex); >> - intel_guc_suspend(&dev_priv->guc); >> + ret = intel_uc_suspend(dev_priv); >> + if (ret) >> + goto out_unlock; >> /* >> * Only called during RPM suspend. All users of the userfault_list >> @@ -2098,9 +2100,10 @@ int i915_gem_runtime_suspend(struct >> drm_i915_private *dev_priv) >> reg->dirty = true; >> } >> +out_unlock: >> mutex_unlock(&dev_priv->drm.struct_mutex); >> - return 0; >> + return ret; >> } >> void i915_gem_runtime_resume(struct drm_i915_private *dev_priv) >> @@ -2110,7 +2113,7 @@ void i915_gem_runtime_resume(struct >> drm_i915_private *dev_priv) >> i915_gem_init_swizzling(dev_priv); >> i915_gem_restore_fences(dev_priv); >> - intel_guc_resume(&dev_priv->guc); >> + intel_uc_resume(dev_priv); >> mutex_unlock(&dev_priv->drm.struct_mutex); >> } >> @@ -4681,7 +4684,9 @@ int i915_gem_suspend(struct drm_i915_private >> *dev_priv) >> i915_gem_set_wedged(dev_priv); /* no hope, discard >> everything */ >> mutex_lock(&dev->struct_mutex); >> - intel_guc_suspend(&dev_priv->guc); >> + ret = intel_uc_suspend(dev_priv); >> + if (ret) >> + goto err_unlock; >> mutex_unlock(&dev->struct_mutex); >> /* >> @@ -4730,7 +4735,7 @@ void i915_gem_resume(struct drm_i915_private >> *dev_priv) >> */ >> dev_priv->gt.resume(dev_priv); >> - intel_guc_resume(&dev_priv->guc); >> + intel_uc_resume(dev_priv); >> mutex_unlock(&dev->struct_mutex); >> } >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index 7305486..b5c132c 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -283,3 +283,19 @@ void intel_uc_fini_hw(struct drm_i915_private >> *dev_priv) >> if (i915_modparams.enable_guc_loading) >> i915_ggtt_disable_guc(dev_priv); >> } >> + >> +int intel_uc_suspend(struct drm_i915_private *dev_priv) >> +{ >> + int ret; >> + >> + ret = intel_guc_suspend(&dev_priv->guc); >> + if (ret) >> + DRM_ERROR("Failed to suspend GuC\n"); >> + >> + return ret; >> +} >> + >> +void intel_uc_resume(struct drm_i915_private *dev_priv) >> +{ >> + intel_guc_resume(&dev_priv->guc); >> +} > > Can we add kerneldoc for above new functions? > With doc added Since full flow was established in the later patches hence deferred doc addition. Will add partially here. Thanks. > > Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > >> diff --git a/drivers/gpu/drm/i915/intel_uc.h >> b/drivers/gpu/drm/i915/intel_uc.h >> index e18d3bb..e20bc72 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.h >> +++ b/drivers/gpu/drm/i915/intel_uc.h >> @@ -34,5 +34,7 @@ >> 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_suspend(struct drm_i915_private *dev_priv); >> +void intel_uc_resume(struct drm_i915_private *dev_priv); >> #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 38447ae..7d1b7e1 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2056,11 +2056,13 @@ static void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj) int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv) { struct drm_i915_gem_object *obj, *on; - int i; + int i, ret; mutex_lock(&dev_priv->drm.struct_mutex); - intel_guc_suspend(&dev_priv->guc); + ret = intel_uc_suspend(dev_priv); + if (ret) + goto out_unlock; /* * Only called during RPM suspend. All users of the userfault_list @@ -2098,9 +2100,10 @@ int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv) reg->dirty = true; } +out_unlock: mutex_unlock(&dev_priv->drm.struct_mutex); - return 0; + return ret; } void i915_gem_runtime_resume(struct drm_i915_private *dev_priv) @@ -2110,7 +2113,7 @@ void i915_gem_runtime_resume(struct drm_i915_private *dev_priv) i915_gem_init_swizzling(dev_priv); i915_gem_restore_fences(dev_priv); - intel_guc_resume(&dev_priv->guc); + intel_uc_resume(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); } @@ -4681,7 +4684,9 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) i915_gem_set_wedged(dev_priv); /* no hope, discard everything */ mutex_lock(&dev->struct_mutex); - intel_guc_suspend(&dev_priv->guc); + ret = intel_uc_suspend(dev_priv); + if (ret) + goto err_unlock; mutex_unlock(&dev->struct_mutex); /* @@ -4730,7 +4735,7 @@ void i915_gem_resume(struct drm_i915_private *dev_priv) */ dev_priv->gt.resume(dev_priv); - intel_guc_resume(&dev_priv->guc); + intel_uc_resume(dev_priv); mutex_unlock(&dev->struct_mutex); } diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 7305486..b5c132c 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -283,3 +283,19 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv) if (i915_modparams.enable_guc_loading) i915_ggtt_disable_guc(dev_priv); } + +int intel_uc_suspend(struct drm_i915_private *dev_priv) +{ + int ret; + + ret = intel_guc_suspend(&dev_priv->guc); + if (ret) + DRM_ERROR("Failed to suspend GuC\n"); + + return ret; +} + +void intel_uc_resume(struct drm_i915_private *dev_priv) +{ + intel_guc_resume(&dev_priv->guc); +} diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index e18d3bb..e20bc72 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -34,5 +34,7 @@ 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_suspend(struct drm_i915_private *dev_priv); +void intel_uc_resume(struct drm_i915_private *dev_priv); #endif
Prepared generic helpers intel_uc_suspend, intel_uc_resume. These are called from respective GEM functions. Only exception is intel_uc_resume that needs to be called w/ or w/o GuC loaded in i915_drm_resume path. Changes to add WOPCM condition check to load GuC during resume will be added in later patches. v2: Rebase w.r.t removal of GuC code restructuring. v3: Calling intel_uc_resume from i915_gem_resume post resuming i915 gem setup. This is symmetrical with i915_gem_suspend. Removed error messages from i915 suspend/resume routines as uC suspend/resume routines will have those. (Michal Wajdeczko) Declare wedged on uc_suspend failure and uc_resume failure. (Michał Winiarski) Keeping the uC suspend/resume function definitions close to other uC functions. v4: Added implementation to intel_uc_resume as GuC resume is needed to be triggered post reloading the firmware as well. Added comments about semantics of GuC resume with the firmware reload. v5: Updated return from i915_gem_runtime_suspend. Moved the comment about GuC reload optimization to intel_uc_init_hw. (Michal Wajdeczko) Updated comments as FIXME. v6: Kept error handling for failure from i915_gem_runtime_suspend only. We don't want GEM/GuC resume failure to impact intel_runtime_resume or i915_drm_resume. GEM suspend failure along i915_drm_suspend can also be ignored as we reset GPU post that. Updated comments. (Chris, Joonas) v7: Removed intel_uc_resume from i915_drm_resume as it will be done as part of intel_uc_init_hw in further patches. Removed TODO comments about handling GuC load skip on resume. This is to be addressed in further patches. Added error return from intel_uc_suspend as we plan to add functionality to resume submission in case of suspend failure in further patches. Removed runtime uC suspend/resume functions as functionality will be similar in both paths. Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 17 +++++++++++------ drivers/gpu/drm/i915/intel_uc.c | 16 ++++++++++++++++ drivers/gpu/drm/i915/intel_uc.h | 2 ++ 3 files changed, 29 insertions(+), 6 deletions(-)