On Wed, Aug 25, 2021 at 11:22:30AM -0400, Rodrigo Vivi wrote: > No functional changes. Just revamping the functions with > s/dev_priv/i915 > and consolidating along with other runtime_pm functions. > > v2: avoid the extra redirection (Imre) > > Cc: Imre Deak <[email protected]> > Cc: Tilak Tangudu <[email protected]> > Signed-off-by: Rodrigo Vivi <[email protected]>
Looks ok: Reviewed-by: Imre Deak <[email protected]> One nit below. > --- > drivers/gpu/drm/i915/i915_drv.c | 145 +----------------------- > drivers/gpu/drm/i915/intel_runtime_pm.c | 145 ++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_runtime_pm.h | 2 + > 3 files changed, 149 insertions(+), 143 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 59fb4c710c8c..a40b5d806321 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -64,7 +64,6 @@ > #include "gem/i915_gem_mman.h" > #include "gem/i915_gem_pm.h" > #include "gt/intel_gt.h" > -#include "gt/intel_gt_pm.h" > #include "gt/intel_rc6.h" > > #include "i915_debugfs.h" > @@ -1517,146 +1516,6 @@ static int i915_pm_restore(struct device *kdev) > return i915_pm_resume(kdev); > } > > -static int intel_runtime_suspend(struct device *kdev) > -{ > - struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > - struct intel_runtime_pm *rpm = &dev_priv->runtime_pm; > - int ret; > - > - if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv))) > - return -ENODEV; > - > - drm_dbg_kms(&dev_priv->drm, "Suspending device\n"); > - > - disable_rpm_wakeref_asserts(rpm); > - > - /* > - * We are safe here against re-faults, since the fault handler takes > - * an RPM reference. > - */ > - i915_gem_runtime_suspend(dev_priv); > - > - intel_gt_runtime_suspend(&dev_priv->gt); > - > - intel_runtime_pm_disable_interrupts(dev_priv); > - > - intel_uncore_suspend(&dev_priv->uncore); > - > - intel_display_power_suspend(dev_priv); > - > - ret = vlv_suspend_complete(dev_priv); > - if (ret) { > - drm_err(&dev_priv->drm, > - "Runtime suspend failed, disabling it (%d)\n", ret); > - intel_uncore_runtime_resume(&dev_priv->uncore); > - > - intel_runtime_pm_enable_interrupts(dev_priv); > - > - intel_gt_runtime_resume(&dev_priv->gt); > - > - enable_rpm_wakeref_asserts(rpm); > - > - return ret; > - } > - > - enable_rpm_wakeref_asserts(rpm); > - intel_runtime_pm_driver_release(rpm); > - > - if (intel_uncore_arm_unclaimed_mmio_detection(&dev_priv->uncore)) > - drm_err(&dev_priv->drm, > - "Unclaimed access detected prior to suspending\n"); > - > - rpm->suspended = true; > - > - /* > - * FIXME: We really should find a document that references the arguments > - * used below! > - */ > - if (IS_BROADWELL(dev_priv)) { > - /* > - * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop > - * being detected, and the call we do at intel_runtime_resume() > - * won't be able to restore them. Since PCI_D3hot matches the > - * actual specification and appears to be working, use it. > - */ > - intel_opregion_notify_adapter(dev_priv, PCI_D3hot); > - } else { > - /* > - * current versions of firmware which depend on this opregion > - * notification have repurposed the D1 definition to mean > - * "runtime suspended" vs. what you would normally expect (D3) > - * to distinguish it from notifications that might be sent via > - * the suspend path. > - */ > - intel_opregion_notify_adapter(dev_priv, PCI_D1); > - } > - > - assert_forcewakes_inactive(&dev_priv->uncore); > - > - if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) > - intel_hpd_poll_enable(dev_priv); > - > - drm_dbg_kms(&dev_priv->drm, "Device suspended\n"); > - return 0; > -} > - > -static int intel_runtime_resume(struct device *kdev) > -{ > - struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > - struct intel_runtime_pm *rpm = &dev_priv->runtime_pm; > - int ret; > - > - if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv))) > - return -ENODEV; > - > - drm_dbg_kms(&dev_priv->drm, "Resuming device\n"); > - > - drm_WARN_ON_ONCE(&dev_priv->drm, atomic_read(&rpm->wakeref_count)); > - disable_rpm_wakeref_asserts(rpm); > - > - intel_opregion_notify_adapter(dev_priv, PCI_D0); > - rpm->suspended = false; > - if (intel_uncore_unclaimed_mmio(&dev_priv->uncore)) > - drm_dbg(&dev_priv->drm, > - "Unclaimed access during suspend, bios?\n"); > - > - intel_display_power_resume(dev_priv); > - > - ret = vlv_resume_prepare(dev_priv, true); > - > - intel_uncore_runtime_resume(&dev_priv->uncore); > - > - intel_runtime_pm_enable_interrupts(dev_priv); > - > - /* > - * No point of rolling back things in case of an error, as the best > - * we can do is to hope that things will still work (and disable RPM). > - */ > - intel_gt_runtime_resume(&dev_priv->gt); > - > - /* > - * On VLV/CHV display interrupts are part of the display > - * power well, so hpd is reinitialized from there. For > - * everyone else do it here. > - */ > - if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) { > - intel_hpd_init(dev_priv); > - intel_hpd_poll_disable(dev_priv); > - } > - > - intel_enable_ipc(dev_priv); > - > - enable_rpm_wakeref_asserts(rpm); > - > - if (ret) > - drm_err(&dev_priv->drm, > - "Runtime resume failed, disabling it (%d)\n", ret); > - else > - drm_dbg_kms(&dev_priv->drm, "Device resumed\n"); > - > - return ret; > -} > - > const struct dev_pm_ops i915_pm_ops = { > /* > * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND, > @@ -1693,8 +1552,8 @@ const struct dev_pm_ops i915_pm_ops = { > .restore = i915_pm_restore, > > /* S0ix (via runtime suspend) event handlers */ > - .runtime_suspend = intel_runtime_suspend, > - .runtime_resume = intel_runtime_resume, > + .runtime_suspend = intel_runtime_pm_suspend, > + .runtime_resume = intel_runtime_pm_resume, > }; > > static const struct file_operations i915_driver_fops = { > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index eaf7688f517d..f28b5bab61b4 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -32,6 +32,11 @@ > > #include "i915_drv.h" > #include "i915_trace.h" > +#include "gt/intel_gt.h" > +#include "gt/intel_gt_pm.h" > +#include "intel_pm.h" > +#include "vlv_suspend.h" > +#include "display/intel_hotplug.h" I suppose these should be in the following order: display/*.h gt/*.h i915_*.h ... (taken from intel_display.c) Could add docs for the exported funcs at one point. > > /** > * DOC: runtime pm > @@ -652,3 +657,143 @@ void intel_runtime_pm_init_early(struct > intel_runtime_pm *rpm) > > init_intel_runtime_pm_wakeref(rpm); > } > + > +int intel_runtime_pm_suspend(struct device *kdev) > +{ > + struct drm_i915_private *i915 = kdev_to_i915(kdev); > + struct intel_runtime_pm *rpm = &i915->runtime_pm; > + int ret; > + > + if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915))) > + return -ENODEV; > + > + drm_dbg_kms(&i915->drm, "Suspending device\n"); > + > + disable_rpm_wakeref_asserts(rpm); > + > + /* > + * We are safe here against re-faults, since the fault handler takes > + * an RPM reference. > + */ > + i915_gem_runtime_suspend(i915); > + > + intel_gt_runtime_suspend(&i915->gt); > + > + intel_runtime_pm_disable_interrupts(i915); > + > + intel_uncore_suspend(&i915->uncore); > + > + intel_display_power_suspend(i915); > + > + ret = vlv_suspend_complete(i915); > + if (ret) { > + drm_err(&i915->drm, > + "Runtime suspend failed, disabling it (%d)\n", ret); > + intel_uncore_runtime_resume(&i915->uncore); > + > + intel_runtime_pm_enable_interrupts(i915); > + > + intel_gt_runtime_resume(&i915->gt); > + > + enable_rpm_wakeref_asserts(rpm); > + > + return ret; > + } > + > + enable_rpm_wakeref_asserts(rpm); > + intel_runtime_pm_driver_release(rpm); > + > + if (intel_uncore_arm_unclaimed_mmio_detection(&i915->uncore)) > + drm_err(&i915->drm, > + "Unclaimed access detected prior to suspending\n"); > + > + rpm->suspended = true; > + > + /* > + * FIXME: We really should find a document that references the arguments > + * used below! > + */ > + if (IS_BROADWELL(i915)) { > + /* > + * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop > + * being detected, and the call we do at intel_runtime_resume() > + * won't be able to restore them. Since PCI_D3hot matches the > + * actual specification and appears to be working, use it. > + */ > + intel_opregion_notify_adapter(i915, PCI_D3hot); > + } else { > + /* > + * current versions of firmware which depend on this opregion > + * notification have repurposed the D1 definition to mean > + * "runtime suspended" vs. what you would normally expect (D3) > + * to distinguish it from notifications that might be sent via > + * the suspend path. > + */ > + intel_opregion_notify_adapter(i915, PCI_D1); > + } > + > + assert_forcewakes_inactive(&i915->uncore); > + > + if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915)) > + intel_hpd_poll_enable(i915); > + > + drm_dbg_kms(&i915->drm, "Device suspended\n"); > + return 0; > +} > + > +int intel_runtime_pm_resume(struct device *kdev) > +{ > + struct drm_i915_private *i915 = kdev_to_i915(kdev); > + struct intel_runtime_pm *rpm = &i915->runtime_pm; > + int ret; > + > + if (drm_WARN_ON_ONCE(&i915->drm, !HAS_RUNTIME_PM(i915))) > + return -ENODEV; > + > + drm_dbg_kms(&i915->drm, "Resuming device\n"); > + > + drm_WARN_ON_ONCE(&i915->drm, atomic_read(&rpm->wakeref_count)); > + disable_rpm_wakeref_asserts(rpm); > + > + intel_opregion_notify_adapter(i915, PCI_D0); > + rpm->suspended = false; > + if (intel_uncore_unclaimed_mmio(&i915->uncore)) > + drm_dbg(&i915->drm, > + "Unclaimed access during suspend, bios?\n"); > + > + intel_display_power_resume(i915); > + > + ret = vlv_resume_prepare(i915, true); > + > + intel_uncore_runtime_resume(&i915->uncore); > + > + intel_runtime_pm_enable_interrupts(i915); > + > + /* > + * No point of rolling back things in case of an error, as the best > + * we can do is to hope that things will still work (and disable RPM). > + */ > + intel_gt_runtime_resume(&i915->gt); > + > + /* > + * On VLV/CHV display interrupts are part of the display > + * power well, so hpd is reinitialized from there. For > + * everyone else do it here. > + */ > + if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915)) { > + intel_hpd_init(i915); > + intel_hpd_poll_disable(i915); > + } > + > + intel_enable_ipc(i915); > + > + enable_rpm_wakeref_asserts(rpm); > + > + if (ret) > + drm_err(&i915->drm, > + "Runtime resume failed, disabling it (%d)\n", ret); > + else > + drm_dbg_kms(&i915->drm, "Device resumed\n"); > + > + return ret; > +} > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.h > b/drivers/gpu/drm/i915/intel_runtime_pm.h > index 47a85fab4130..88ca531165f7 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.h > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h > @@ -172,6 +172,8 @@ void intel_runtime_pm_init_early(struct intel_runtime_pm > *rpm); > void intel_runtime_pm_enable(struct intel_runtime_pm *rpm); > void intel_runtime_pm_disable(struct intel_runtime_pm *rpm); > void intel_runtime_pm_driver_release(struct intel_runtime_pm *rpm); > +int intel_runtime_pm_suspend(struct device *kdev); > +int intel_runtime_pm_resume(struct device *kdev); > > intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm); > intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm *rpm); > -- > 2.31.1 >
