On Wed, Apr 27, 2016 at 11:46:22AM -0700, Todd Brandt wrote: > I'd like to propose that we push the i915 suspend_late/resume_early code > into suspend_noirq/resume_noirq in order to reduce the total suspend time > by ~15ms. According to the comments, when i915_pm_suspend_late was first > added to the kernel back in April 2014, it was done so to ensure that it > was called after the snd_hda_intel driver had finished its suspend. > > The comments in i915_drv.c are here: > > /* > * We have a suspedn ordering issue with the snd-hda driver also > * requiring our device to be power up. Due to the lack of a > * parent/child relationship we currently solve this with an late > * suspend hook. > * > * FIXME: This should be solved with a special hdmi sink device or > * similar so that power domains can be employed. > */ > > I believe we could achieve the same ordering by simply pushing it to > suspend/resume_noirq. Thus we can effectively eliminate the suspend_late > and resume_early phases altogether in most simple systems. Does anyone see > a problem with this? > > analyzesuspend outputs for freeze/suspend/hibernate (WITHOUT PATCH): > https://01org.github.io/suspendresume/i915/freeze-160422-155931-ivybridge-dev-late/ > https://01org.github.io/suspendresume/i915/suspend-160422-155735-ivybridge-dev-late/ > https://01org.github.io/suspendresume/i915/hibernate-160422-163915-ivybridge-dev-late/ > > analyzesuspend outputs for freeze/suspend/hibernate (WITH PATCH): > https://01org.github.io/suspendresume/i915/freeze-160422-162811-ivybridge-dev-noirq/ > https://01org.github.io/suspendresume/i915/suspend-160422-162700-ivybridge-dev-noirq/ > https://01org.github.io/suspendresume/i915/hibernate-160422-162952-ivybridge-dev-noirq/ Hmm. Looking at those makes me confused. Why isn't the pci bus .resume_noirq hook (pci_pm_resume_noirq()) waking up our pci device? Instead our wakeup gets delayed until .resume_early for some reason. > > Signed-off-by: Todd Brandt <todd.e.brandt@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 30798cb..759d93c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1628,8 +1628,8 @@ static const struct dev_pm_ops i915_pm_ops = { > * PMSG_RESUME] > */ > .suspend = i915_pm_suspend, > - .suspend_late = i915_pm_suspend_late, > - .resume_early = i915_pm_resume_early, > + .suspend_noirq = i915_pm_suspend_late, > + .resume_noirq = i915_pm_resume_early, > .resume = i915_pm_resume, > > /* > @@ -1648,12 +1648,12 @@ static const struct dev_pm_ops i915_pm_ops = { > * hibernation image [PMSG_RESTORE] > */ > .freeze = i915_pm_suspend, > - .freeze_late = i915_pm_suspend_late, > - .thaw_early = i915_pm_resume_early, > + .freeze_noirq = i915_pm_suspend_late, > + .thaw_noirq = i915_pm_resume_early, > .thaw = i915_pm_resume, > .poweroff = i915_pm_suspend, > - .poweroff_late = i915_pm_poweroff_late, > - .restore_early = i915_pm_resume_early, > + .poweroff_noirq = i915_pm_poweroff_late, > + .restore_noirq = i915_pm_resume_early, > .restore = i915_pm_resume, > > /* S0ix (via runtime suspend) event handlers */ > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx