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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx