On Thu, Jan 19, 2023 at 11:10:21AM -0800, Ceraolo Spurio, Daniele wrote: > > > On 1/12/2023 5:18 PM, Alan Previn wrote: > > During suspend flow, i915 currently achors' on the pm_suspend_prepare > > callback as the location where we quiesce the entire GPU and perform > > all necessary cleanup in order to go into suspend. PXP is also called > > during this time to perform the arbitration session teardown (with > > the assurance no additional GEM IOCTLs will come after that could > > restart the session). > > > > However, if other devices or drivers fail their suspend_prepare, the > > system will not go into suspend and i915 will be expected to resume > > operation. In this case, we need to re-initialize the PXP hardware > > and this really should be done within the pm_resume_complete callback > > which is the correct opposing function in the resume sequence to > > match pm_suspend_prepare of the suspend sequence. > > > > Because this callback is the last thing at the end of resuming > > we expect little to no impact to the rest of the i915 resume sequence > > with this change. > > > > Signed-off-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/gt/intel_gt_pm.h | 1 + > > drivers/gpu/drm/i915/i915_driver.c | 20 ++++++++++++++++++-- > > drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 2 +- > > drivers/gpu/drm/i915/pxp/intel_pxp_pm.h | 6 +++--- > > 4 files changed, 23 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h > > index 6c9a46452364..fd1a23621222 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h > > @@ -77,6 +77,7 @@ void intel_gt_pm_fini(struct intel_gt *gt); > > void intel_gt_suspend_prepare(struct intel_gt *gt); > > void intel_gt_suspend_late(struct intel_gt *gt); > > + > > Stray newline. With this removed: > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> Acked-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Daniele > > > int intel_gt_resume(struct intel_gt *gt); > > void intel_gt_runtime_suspend(struct intel_gt *gt); > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > > index c1e427ba57ae..4c68a3f26e96 100644 > > --- a/drivers/gpu/drm/i915/i915_driver.c > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > @@ -1170,6 +1170,13 @@ static bool suspend_to_idle(struct drm_i915_private *dev_priv) > > return false; > > } > > +static void i915_drm_complete(struct drm_device *dev) > > +{ > > + struct drm_i915_private *i915 = to_i915(dev); > > + > > + intel_pxp_resume_complete(i915->pxp); > > +} > > + > > static int i915_drm_prepare(struct drm_device *dev) > > { > > struct drm_i915_private *i915 = to_i915(dev); > > @@ -1370,8 +1377,6 @@ static int i915_drm_resume(struct drm_device *dev) > > i915_gem_resume(dev_priv); > > - intel_pxp_resume(dev_priv->pxp); > > - > > intel_modeset_init_hw(dev_priv); > > intel_init_clock_gating(dev_priv); > > intel_hpd_init(dev_priv); > > @@ -1563,6 +1568,16 @@ static int i915_pm_resume(struct device *kdev) > > return i915_drm_resume(&i915->drm); > > } > > +static void i915_pm_complete(struct device *kdev) > > +{ > > + struct drm_i915_private *i915 = kdev_to_i915(kdev); > > + > > + if (i915->drm.switch_power_state == DRM_SWITCH_POWER_OFF) > > + return; > > + > > + i915_drm_complete(&i915->drm); > > +} > > + > > /* freeze: before creating the hibernation_image */ > > static int i915_pm_freeze(struct device *kdev) > > { > > @@ -1783,6 +1798,7 @@ const struct dev_pm_ops i915_pm_ops = { > > .suspend_late = i915_pm_suspend_late, > > .resume_early = i915_pm_resume_early, > > .resume = i915_pm_resume, > > + .complete = i915_pm_complete, > > /* > > * S4 event handlers > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c > > index e427464aa131..4f836b317424 100644 > > --- a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c > > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c > > @@ -34,7 +34,7 @@ void intel_pxp_suspend(struct intel_pxp *pxp) > > } > > } > > -void intel_pxp_resume(struct intel_pxp *pxp) > > +void intel_pxp_resume_complete(struct intel_pxp *pxp) > > { > > if (!intel_pxp_is_enabled(pxp)) > > return; > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h > > index 586be769104f..06b46f535b42 100644 > > --- a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h > > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.h > > @@ -11,7 +11,7 @@ struct intel_pxp; > > #ifdef CONFIG_DRM_I915_PXP > > void intel_pxp_suspend_prepare(struct intel_pxp *pxp); > > void intel_pxp_suspend(struct intel_pxp *pxp); > > -void intel_pxp_resume(struct intel_pxp *pxp); > > +void intel_pxp_resume_complete(struct intel_pxp *pxp); > > void intel_pxp_runtime_suspend(struct intel_pxp *pxp); > > #else > > static inline void intel_pxp_suspend_prepare(struct intel_pxp *pxp) > > @@ -22,7 +22,7 @@ static inline void intel_pxp_suspend(struct intel_pxp *pxp) > > { > > } > > -static inline void intel_pxp_resume(struct intel_pxp *pxp) > > +static inline void intel_pxp_resume_complete(struct intel_pxp *pxp) > > { > > } > > @@ -32,6 +32,6 @@ static inline void intel_pxp_runtime_suspend(struct intel_pxp *pxp) > > #endif > > static inline void intel_pxp_runtime_resume(struct intel_pxp *pxp) > > { > > - intel_pxp_resume(pxp); > > + intel_pxp_resume_complete(pxp); > > } > > #endif /* __INTEL_PXP_PM_H__ */ >