On Wed, 2016-04-27 at 22:33 +0200, Lukas Wunner wrote: > Hi, > > 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. > > Ordering issues like this one should be solved with > device_pm_wait_for_dev(), > not by shuffling code around among the callbacks. We considered using device_pm_wait_for_dev() but decided not to, since it may dead lock in case of suspend/resume: https://lists.freedesktop.org/archives/intel-gfx/2014-December/057113.html > In any case it would be good if you would name the sha1 of the commit > that added i915_pm_suspend_late to spare readers the git blame / git > log. > > Thanks, > > Lukas > > > > > 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-ivy > > bridge-dev-late/ > > https://01org.github.io/suspendresume/i915/suspend-160422-155735-iv > > ybridge-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-ivy > > bridge-dev-noirq/ > > https://01org.github.io/suspendresume/i915/suspend-160422-162700-iv > > ybridge-dev-noirq/ > > https://01org.github.io/suspendresume/i915/hibernate-160422-162952- > > ivybridge-dev-noirq/ > > > > 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 */ _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
