Re: [PATCH] i915 suspend/resume_noirq instead of suspend_late/resume_early

[Todd Brandt <todd.e.brandt@linux.intel.com>]

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.
> 
> 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.
> 

Here's the actual commit that introduced it:

git checkout 76c4b250080fff6e4befaa3619942422fd0ea380
git diff HEAD^ drivers/gpu/drm/i915/i915_drv.c

That shows the initial addition of the _late code.


> 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-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/
> > 
> > Signed-off-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> > ---
> >  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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx