| Message ID | 1461173277-16090-2-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 20 Apr 2016 20:27:54 +0300 Imre Deak <imre.deak@intel.com> wrote: > Initially we thought that the platform specific suspend/resume sequences > can be shared between the runtime and system suspend/resume handlers. > This turned out to be not true, we have quite a few differences on most > of the platforms. This was realized already earlier by Paulo who > uninlined the platform specific resume_prepare handlers. We have the > same problem with the corresponding suspend_complete handlers, there are > platform differences that make it unfeasible to share the code between > the runtime and system suspend paths. Also now we call functions that > need to be paired like hsw_enable_pc8()/hsw_disable_pc8() from different > levels of the call stack, which is confusing. Fix this by uninlining the > suspend_complete handlers too. > > This is also needed by the next patch that removes a redundant > uninit/init call during system suspend/resume on BXT. > > No functional change. Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com> > > CC: Paulo Zanoni <przanoni@gmail.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 83 ++++++++++++++--------------------------- > 1 file changed, 29 insertions(+), 54 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 9a016f1..ea9b3fe 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -567,10 +567,9 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv) > drm_modeset_unlock_all(dev); > } > > -static int intel_suspend_complete(struct drm_i915_private *dev_priv); > static int vlv_resume_prepare(struct drm_i915_private *dev_priv, > bool rpm_resume); > -static int bxt_resume_prepare(struct drm_i915_private *dev_priv); > +static int vlv_suspend_complete(struct drm_i915_private *dev_priv); > > static bool suspend_to_idle(struct drm_i915_private *dev_priv) > { > @@ -668,7 +667,14 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) > if (!fw_csr) > intel_power_domains_suspend(dev_priv); > > - ret = intel_suspend_complete(dev_priv); > + ret = 0; > + if (IS_BROXTON(dev_priv)) { > + bxt_display_core_uninit(dev_priv); > + bxt_enable_dc9(dev_priv); > + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > + hsw_enable_pc8(dev_priv); > + else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > + ret = vlv_suspend_complete(dev_priv); > > if (ret) { > DRM_ERROR("Suspend complete failed: %d\n", ret); > @@ -862,9 +868,10 @@ static int i915_drm_resume_early(struct drm_device *dev) > > intel_uncore_early_sanitize(dev, true); > > - if (IS_BROXTON(dev)) > - ret = bxt_resume_prepare(dev_priv); > - else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > + if (IS_BROXTON(dev)) { > + bxt_disable_dc9(dev_priv); > + bxt_display_core_init(dev_priv, true); > + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > hsw_disable_pc8(dev_priv); > > intel_uncore_sanitize(dev); > @@ -1102,29 +1109,6 @@ static int i915_pm_resume(struct device *dev) > return i915_drm_resume(drm_dev); > } > > -static int hsw_suspend_complete(struct drm_i915_private *dev_priv) > -{ > - hsw_enable_pc8(dev_priv); > - > - return 0; > -} > - > -static int bxt_suspend_complete(struct drm_i915_private *dev_priv) > -{ > - bxt_display_core_uninit(dev_priv); > - bxt_enable_dc9(dev_priv); > - > - return 0; > -} > - > -static int bxt_resume_prepare(struct drm_i915_private *dev_priv) > -{ > - bxt_disable_dc9(dev_priv); > - bxt_display_core_init(dev_priv, true); > - > - return 0; > -} > - > /* > * Save all Gunit registers that may be lost after a D3 and a subsequent > * S0i[R123] transition. The list of registers needing a save/restore is > @@ -1530,7 +1514,16 @@ static int intel_runtime_suspend(struct device *device) > intel_suspend_gt_powersave(dev); > intel_runtime_pm_disable_interrupts(dev_priv); > > - ret = intel_suspend_complete(dev_priv); > + ret = 0; > + if (IS_BROXTON(dev_priv)) { > + bxt_display_core_uninit(dev_priv); > + bxt_enable_dc9(dev_priv); > + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > + hsw_enable_pc8(dev_priv); > + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > + ret = vlv_suspend_complete(dev_priv); > + } > + > if (ret) { > DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret); > intel_runtime_pm_enable_interrupts(dev_priv); > @@ -1604,12 +1597,14 @@ static int intel_runtime_resume(struct device *device) > if (IS_GEN6(dev_priv)) > intel_init_pch_refclk(dev); > > - if (IS_BROXTON(dev)) > - ret = bxt_resume_prepare(dev_priv); > - else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > + if (IS_BROXTON(dev)) { > + bxt_disable_dc9(dev_priv); > + bxt_display_core_init(dev_priv, true); > + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > hsw_disable_pc8(dev_priv); > - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > ret = vlv_resume_prepare(dev_priv, true); > + } > > /* > * No point of rolling back things in case of an error, as the best > @@ -1640,26 +1635,6 @@ static int intel_runtime_resume(struct device *device) > return ret; > } > > -/* > - * This function implements common functionality of runtime and system > - * suspend sequence. > - */ > -static int intel_suspend_complete(struct drm_i915_private *dev_priv) > -{ > - int ret; > - > - if (IS_BROXTON(dev_priv)) > - ret = bxt_suspend_complete(dev_priv); > - else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > - ret = hsw_suspend_complete(dev_priv); > - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > - ret = vlv_suspend_complete(dev_priv); > - else > - ret = 0; > - > - return ret; > -} > - > static const struct dev_pm_ops i915_pm_ops = { > /* > * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 9a016f1..ea9b3fe 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -567,10 +567,9 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv) drm_modeset_unlock_all(dev); } -static int intel_suspend_complete(struct drm_i915_private *dev_priv); static int vlv_resume_prepare(struct drm_i915_private *dev_priv, bool rpm_resume); -static int bxt_resume_prepare(struct drm_i915_private *dev_priv); +static int vlv_suspend_complete(struct drm_i915_private *dev_priv); static bool suspend_to_idle(struct drm_i915_private *dev_priv) { @@ -668,7 +667,14 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) if (!fw_csr) intel_power_domains_suspend(dev_priv); - ret = intel_suspend_complete(dev_priv); + ret = 0; + if (IS_BROXTON(dev_priv)) { + bxt_display_core_uninit(dev_priv); + bxt_enable_dc9(dev_priv); + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) + hsw_enable_pc8(dev_priv); + else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) + ret = vlv_suspend_complete(dev_priv); if (ret) { DRM_ERROR("Suspend complete failed: %d\n", ret); @@ -862,9 +868,10 @@ static int i915_drm_resume_early(struct drm_device *dev) intel_uncore_early_sanitize(dev, true); - if (IS_BROXTON(dev)) - ret = bxt_resume_prepare(dev_priv); - else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) + if (IS_BROXTON(dev)) { + bxt_disable_dc9(dev_priv); + bxt_display_core_init(dev_priv, true); + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) hsw_disable_pc8(dev_priv); intel_uncore_sanitize(dev); @@ -1102,29 +1109,6 @@ static int i915_pm_resume(struct device *dev) return i915_drm_resume(drm_dev); } -static int hsw_suspend_complete(struct drm_i915_private *dev_priv) -{ - hsw_enable_pc8(dev_priv); - - return 0; -} - -static int bxt_suspend_complete(struct drm_i915_private *dev_priv) -{ - bxt_display_core_uninit(dev_priv); - bxt_enable_dc9(dev_priv); - - return 0; -} - -static int bxt_resume_prepare(struct drm_i915_private *dev_priv) -{ - bxt_disable_dc9(dev_priv); - bxt_display_core_init(dev_priv, true); - - return 0; -} - /* * Save all Gunit registers that may be lost after a D3 and a subsequent * S0i[R123] transition. The list of registers needing a save/restore is @@ -1530,7 +1514,16 @@ static int intel_runtime_suspend(struct device *device) intel_suspend_gt_powersave(dev); intel_runtime_pm_disable_interrupts(dev_priv); - ret = intel_suspend_complete(dev_priv); + ret = 0; + if (IS_BROXTON(dev_priv)) { + bxt_display_core_uninit(dev_priv); + bxt_enable_dc9(dev_priv); + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { + hsw_enable_pc8(dev_priv); + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { + ret = vlv_suspend_complete(dev_priv); + } + if (ret) { DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret); intel_runtime_pm_enable_interrupts(dev_priv); @@ -1604,12 +1597,14 @@ static int intel_runtime_resume(struct device *device) if (IS_GEN6(dev_priv)) intel_init_pch_refclk(dev); - if (IS_BROXTON(dev)) - ret = bxt_resume_prepare(dev_priv); - else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) + if (IS_BROXTON(dev)) { + bxt_disable_dc9(dev_priv); + bxt_display_core_init(dev_priv, true); + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { hsw_disable_pc8(dev_priv); - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { ret = vlv_resume_prepare(dev_priv, true); + } /* * No point of rolling back things in case of an error, as the best @@ -1640,26 +1635,6 @@ static int intel_runtime_resume(struct device *device) return ret; } -/* - * This function implements common functionality of runtime and system - * suspend sequence. - */ -static int intel_suspend_complete(struct drm_i915_private *dev_priv) -{ - int ret; - - if (IS_BROXTON(dev_priv)) - ret = bxt_suspend_complete(dev_priv); - else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) - ret = hsw_suspend_complete(dev_priv); - else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) - ret = vlv_suspend_complete(dev_priv); - else - ret = 0; - - return ret; -} - static const struct dev_pm_ops i915_pm_ops = { /* * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
Initially we thought that the platform specific suspend/resume sequences can be shared between the runtime and system suspend/resume handlers. This turned out to be not true, we have quite a few differences on most of the platforms. This was realized already earlier by Paulo who uninlined the platform specific resume_prepare handlers. We have the same problem with the corresponding suspend_complete handlers, there are platform differences that make it unfeasible to share the code between the runtime and system suspend paths. Also now we call functions that need to be paired like hsw_enable_pc8()/hsw_disable_pc8() from different levels of the call stack, which is confusing. Fix this by uninlining the suspend_complete handlers too. This is also needed by the next patch that removes a redundant uninit/init call during system suspend/resume on BXT. No functional change. CC: Paulo Zanoni <przanoni@gmail.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 83 ++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 54 deletions(-)