LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: linux-kernel@vger.kernel.org
Cc: "Daniel Vetter" <daniel.vetter@intel.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	"Bjørn Mork" <bjorn@mork.no>,
	stable@vger.kernel.org, "Imre Deak" <imre.deak@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Subject: [PATCH] drm/i915: fix failure to power off after hibernate
Date: Tue, 24 Feb 2015 15:58:24 +0100	[thread overview]
Message-ID: <1424789904-26699-1-git-send-email-bjorn@mork.no> (raw)
In-Reply-To: <87bnkjcqjt.fsf@nemi.mork.no>

This fixes a bug where hibernation completes, but the system
fails to power off after the image has been saved.

Bisection lead to commit da2bc1b9db33 ("drm/i915: add poweroff_late
handler") which added a .poweroff_late hook pointing to the same
function as the .freeze_late hook, without any justification or
explanation why this would be correct, except "for consistency".

The assumption that this "makes the power off handling identical to
the S3 suspend and S4 freeze handling" is completely bogus. As clearly
documented in Documentation/power/devices.txt, the poweroff* hooks
are part of the hibernation API along with the freeze* hooks.  The
phases involved in hibernation are:

   prepare, freeze, freeze_late, freeze_noirq, thaw_noirq, thaw_early,
   thaw, complete, prepare, poweroff, poweroff_late, poweroff_noirq

With the above sequence in mind, it should be fairly obvious that you
cannot save registers and disable the device in both the freeze_late
and poweroff_late phases.  Or as Documentation/power/devices.txt
explains it:

 The poweroff, poweroff_late and poweroff_noirq callbacks should do essentially
 the same things as the suspend, suspend_late and suspend_noirq callbacks,
 respectively.  The only notable difference is that they need not store the
 device register values, because the registers should already have been stored
 during the freeze, freeze_late or freeze_noirq phases.

The "consistency" between suspend and hibernate was already in
place, with freeze_late pointing to the same function as suspend_late.
There is no need for any poweroff_late hook in this driver.

This reverts commit da2bc1b9db33.

Fixes: da2bc1b9db33 ("drm/i915: add poweroff_late handler")
Cc: <stable@vger.kernel.org>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/gpu/drm/i915/i915_drv.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8039cec71fc2..9cd695710f93 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1520,7 +1520,6 @@ static const struct dev_pm_ops i915_pm_ops = {
 	.thaw_early = i915_pm_resume_early,
 	.thaw = i915_pm_resume,
 	.poweroff = i915_pm_suspend,
-	.poweroff_late = i915_pm_suspend_late,
 	.restore_early = i915_pm_resume_early,
 	.restore = i915_pm_resume,
 
-- 
1.7.10.4


  reply	other threads:[~2015-02-24 14:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-24 14:49 [BISECTED REGRESSION v3.18->v3.19-rc1] drm/i915: failure to poweroff after hibernation Bjørn Mork
2015-02-24 14:58 ` Bjørn Mork [this message]
2015-02-24 16:12   ` [PATCH] drm/i915: fix failure to power off after hibernate Imre Deak
2015-02-24 19:00     ` Bjørn Mork
2015-02-25 18:33       ` Imre Deak
2015-02-26  9:34         ` Bjørn Mork
2015-02-26 18:50           ` Imre Deak
2015-02-26 19:20             ` Bjørn Mork
2015-02-26 20:01             ` [Intel-gfx] " Daniel Vetter
2015-02-27 12:15               ` David Weinehall
2015-02-27 18:23                 ` Imre Deak
2015-03-02  8:55                   ` David Weinehall
2015-02-26 20:05             ` Ville Syrjälä
2015-02-26 20:29               ` Bjørn Mork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1424789904-26699-1-git-send-email-bjorn@mork.no \
    --to=bjorn@mork.no \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=ville.syrjala@linux.intel.com \
    --subject='Re: [PATCH] drm/i915: fix failure to power off after hibernate' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).