Re: [Regression, post-2.6.34] Hibernation broken on machines with radeon/KMS and r300

[Alex Deucher <alexdeucher@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3335 bytes --]

On Wed, Jun 16, 2010 at 4:16 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Wednesday, June 16, 2010, Ondrej Zary wrote:
>> On Wednesday 16 June 2010, Rafael J. Wysocki wrote:
>> > On Tuesday, June 15, 2010, Rafael J. Wysocki wrote:
>> > > On Monday, June 14, 2010, Alex Deucher wrote:
>> > > > On Mon, Jun 14, 2010 at 3:03 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > > > > On Monday, June 14, 2010, Alex Deucher wrote:
>> > > > >> On Mon, Jun 14, 2010 at 10:53 AM, Rafael J. Wysocki <rjw@sisk.pl>
>> wrote:
>> > > > >> > Alex, Dave,
>> > > > >> >
>> > > > >> > I'm afraid hibernation is broken on all machines using radeon/KMS
>> > > > >> > with r300 after commit ce8f53709bf440100cb9d31b1303291551cf517f
>> > > > >> > (drm/radeon/kms/pm: rework power management).  At least, I'm able
>> > > > >> > to reproduce the symptom, which is that the machine hangs hard
>> > > > >> > around the point where an image is created (probably during the
>> > > > >> > device thaw phase), on two different boxes with r300 (the output
>> > > > >> > of lspci from one of them is attached for reference, the other one
>> > > > >> > is HP nx6325).
>> > > > >> >
>> > > > >> > Suspend to RAM appears to work fine at least on one of the
>> > > > >> > affected boxes.
>> > > > >> >
>> > > > >> > Unfortunately, the commit above changes a lot of code and it's not
>> > > > >> > too easy to figure out what's wrong with it and I didn't have the
>> > > > >> > time to look more into details of this failure.  However, it looks
>> > > > >> > like you use .suspend() and .resume() callbacks as .freeze() and
>> > > > >> > .thaw() which may not be 100% correct (in fact it looks like the
>> > > > >> > "legacy" PCI suspend/resume is used, which is not recommended any
>> > > > >> > more).
>> > > > >>
>> > > > >> Does it work any better after Dave's last drm pull request?
>> > > > >
>> > > > > Nope.  The symptom is slightly different, though, because now it
>> > > > > hangs after turning off the screen.
>> > > > >
>> > > > >> With the latest changes, pm should not be a factor unless it's
>> > > > >> explicitly enabled via sysfs.
>> > > > >
>> > > > > Well, I guess the first pm patch changed more than just pm, then.
>> > > >
>> > > > Does this patch help?
>> > > > http://lists.freedesktop.org/archives/dri-devel/2010-June/001314.html
>> > >
>> > > No, it doesn't.  I try to hibernate, everything works to the point where
>> > > the screen goes off and the box hangs (solid).  Normally, it would turn
>> > > the screen back on and continue with saving the image.
>> > >
>> > > But, since that happens with the patch above applied, I think it doesn't
>> > > really pass the suspend phase (IOW, it probably hangs somewhere in the
>> > > radeon's suspend routine).
>> >
>> > I've just verified that in fact hibernation works on HP nx6325 with
>> > 2.6.35-rc3, but it takes about 55 sec. to suspend the graphics adapter in
>> > the "freeze" phase.  Surprisingly enough, during suspend to RAM it works
>> > normally (as well as in the "poweroff" phase of hibernation).
>>
>> It takes 2 minutes on RV530:
>> https://bugzilla.redhat.com/show_bug.cgi?id=586522
>
> Well, my second affected box appears to hang somewhere in the radeon's suspend
> routine.

Does the attached patch help?

Alex

[-- Attachment #2: 0001-drm-radeon-kms-only-attempt-to-cancel-delayed-work-i.patch --]
[-- Type: text/x-patch, Size: 1713 bytes --]

From 9d2600fd31c1dda6f41e569fd8aff45c9e5ce54f Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexdeucher@gmail.com>
Date: Wed, 16 Jun 2010 16:41:38 -0400
Subject: [PATCH] drm/radeon/kms: only attempt to cancel delayed work if pm method is PM_METHOD_DYNPM

delayed work is only scheduled if pm method is PM_METHOD_DYNPM.

Signed-off-by: Alex Deucher <alexdeucher@gmail.com>
---
 drivers/gpu/drm/radeon/radeon_pm.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index b9ad366..0751fc8 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -407,11 +407,12 @@ static ssize_t radeon_set_pm_method(struct device *dev,
 		mutex_unlock(&rdev->pm.mutex);
 	} else if (strncmp("profile", buf, strlen("profile")) == 0) {
 		mutex_lock(&rdev->pm.mutex);
-		rdev->pm.pm_method = PM_METHOD_PROFILE;
+		if (rdev->pm.pm_method == PM_METHOD_DYNPM)
+			cancel_delayed_work(&rdev->pm.dynpm_idle_work);
 		/* disable dynpm */
 		rdev->pm.dynpm_state = DYNPM_STATE_DISABLED;
 		rdev->pm.dynpm_planned_action = DYNPM_ACTION_NONE;
-		cancel_delayed_work(&rdev->pm.dynpm_idle_work);
+		rdev->pm.pm_method = PM_METHOD_PROFILE;
 		mutex_unlock(&rdev->pm.mutex);
 	} else {
 		DRM_ERROR("invalid power method!\n");
@@ -428,7 +429,8 @@ static DEVICE_ATTR(power_method, S_IRUGO | S_IWUSR, radeon_get_pm_method, radeon
 void radeon_pm_suspend(struct radeon_device *rdev)
 {
 	mutex_lock(&rdev->pm.mutex);
-	cancel_delayed_work(&rdev->pm.dynpm_idle_work);
+	if (rdev->pm.pm_method == PM_METHOD_DYNPM)
+		cancel_delayed_work(&rdev->pm.dynpm_idle_work);
 	mutex_unlock(&rdev->pm.mutex);
 }
 
-- 
1.7.0.1