Resume from suspend broken in 3.15. (bisected)

Resume from suspend broken in 3.15. (bisected)

[Ken Moffat]

Hi Daniel,

 I've only started full testing of 3.15 on one of my machines now
that -rc7 has been released (this one had two issues in the radeon
code, second was fixed in rc7).  Unfortunately, suspend to RAM
(pm-suspend), or rather the wake-up, is broken on this box [ my
other two boxes are fine in rc7 ].

Bisection identified one of your commits -

commit 25f397a429dfa43f22c278d0119a60a343aa568f
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Jul 19 18:57:11 2013 +0200

    drm/crtc-helper: explicit DPMS on after modeset
    
    Atm the crtc helper implementation of set_config has really
    inconsisten semantics: If just an fb update is good enough, dpms state
    will be left as-is, but if we do a full modeset we force everything to
    dpms on.
    
    This change has already been applied to the i915 modeset code in
    
('git show' stops at that point)


 I have attempted to revert this commit from Linus' current tree,
but git considered that there was a conflict, and I did not know how
to address it, so I aborted.  The conflict is :

<<<<<<< HEAD

                                if (connector->dpms != DRM_MODE_DPMS_ON) {
                                        DRM_DEBUG_KMS("connector dpms not on, full mode switch\n");
                                        mode_changed = true;
                                }

=======
>>>>>>> parent of 25f397a429df... drm/crtc-helper: explicit DPMS on
>>>>>>> after modeset
                                break;
                        }
                }


 The processor is an AMD A4 APU.  The symptoms after this commit are
that pm-suspend works (I invoke it from my keyboard's sleep key),
i.e. the power LED on the case goes out and the monitor goes black
and reports no signal.  When I press a key to resume, the power LED
comes on but the screen stays black.  Magic-SysRQ does not work and
I have to use the case switch to reboot.  That results in filesystem
errors which fsck fixes and warns about (probably, just a sign of
unclean shutdown).

 The only thing in the log are a couple of messages from EXT4 at the
end of putting the box to sleep -
May 28 16:19:02 bluesbreaker kernel: [   39.440859] EXT4-fs (sda10): re-mounted. Opts: commit=0
May 28 16:19:02 bluesbreaker kernel: [   39.592318] EXT4-fs (sda13): re-mounted. Opts: commit=0
May 28 16:20:05 bluesbreaker syslogd 1.5.0: restart.

 What can I do to help debug this ?

ĸen
-- 
Nanny Ogg usually went to bed early. After all, she was an old lady.
Sometimes she went to bed as early as 6 a.m.

Re: Resume from suspend broken in 3.15. (bisected)

[Ken Moffat]

On Wed, May 28, 2014 at 06:25:21PM +0100, Ken Moffat wrote:
> Hi Daniel,
> 

 [ correcting details, confirming that reverting this does fix the
problem, adding Cc:s ]

>  I've only started full testing of 3.15 on one of my machines now
> that -rc7 has been released (this one had two issues in the radeon
> code, second was fixed in rc7).  Unfortunately, suspend to RAM
> (pm-suspend), or rather the wake-up, is broken on this box [ my
> other two boxes are fine in rc7 ].
> 
> Bisection identified one of your commits -
> 
> commit 25f397a429dfa43f22c278d0119a60a343aa568f
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Fri Jul 19 18:57:11 2013 +0200
> 
>     drm/crtc-helper: explicit DPMS on after modeset
>     
>     Atm the crtc helper implementation of set_config has really
>     inconsisten semantics: If just an fb update is good enough, dpms state
>     will be left as-is, but if we do a full modeset we force everything to
>     dpms on.
>     
>     This change has already been applied to the i915 modeset code in
>     
> ('git show' stops at that point)

 update : I've no idea what was going on there, nor for the problem
with attempting to revert it.  I've now gone back into git,
extracted the full commit to a file with 'git show', and then used
git apply -R to revert it from 3.15-rc7.  That version wakes up from
suspend to RAM, 3.15-rc7 itself did not.

 Maybe I was still in git log when I thought I was on the command
line.  Anyway, snipping git's view of my failed attempt to revert
it, and adding Dan and Alex who were CC'd on the commit.

> 
>  The processor is an AMD A4 APU.  The symptoms after this commit are
> that pm-suspend works (I invoke it from my keyboard's sleep key),
> i.e. the power LED on the case goes out and the monitor goes black
> and reports no signal.  When I press a key to resume, the power LED
> comes on but the screen stays black.  Magic-SysRQ does not work and
> I have to use the case switch to reboot.  That results in filesystem
> errors which fsck fixes and warns about (probably, just a sign of
> unclean shutdown).
> 
>  The only thing in the log are a couple of messages from EXT4 at the
> end of putting the box to sleep -
> May 28 16:19:02 bluesbreaker kernel: [   39.440859] EXT4-fs (sda10): re-mounted. Opts: commit=0
> May 28 16:19:02 bluesbreaker kernel: [   39.592318] EXT4-fs (sda13): re-mounted. Opts: commit=0
> May 28 16:20:05 bluesbreaker syslogd 1.5.0: restart.
> 
>  What can I do to help debug this ?
> 
ĸen
-- 
Nanny Ogg usually went to bed early. After all, she was an old lady.
Sometimes she went to bed as early as 6 a.m.

Re: Resume from suspend broken in 3.15. (bisected)

[Alex Deucher]

On Wed, May 28, 2014 at 7:49 PM, Ken Moffat <zarniwhoop@ntlworld.com> wrote:
> On Wed, May 28, 2014 at 06:25:21PM +0100, Ken Moffat wrote:
>> Hi Daniel,
>>
>
>  [ correcting details, confirming that reverting this does fix the
> problem, adding Cc:s ]
>
>>  I've only started full testing of 3.15 on one of my machines now
>> that -rc7 has been released (this one had two issues in the radeon
>> code, second was fixed in rc7).  Unfortunately, suspend to RAM
>> (pm-suspend), or rather the wake-up, is broken on this box [ my
>> other two boxes are fine in rc7 ].
>>
>> Bisection identified one of your commits -
>>
>> commit 25f397a429dfa43f22c278d0119a60a343aa568f
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Fri Jul 19 18:57:11 2013 +0200
>>
>>     drm/crtc-helper: explicit DPMS on after modeset
>>
>>     Atm the crtc helper implementation of set_config has really
>>     inconsisten semantics: If just an fb update is good enough, dpms state
>>     will be left as-is, but if we do a full modeset we force everything to
>>     dpms on.
>>
>>     This change has already been applied to the i915 modeset code in
>>
>> ('git show' stops at that point)
>
>  update : I've no idea what was going on there, nor for the problem
> with attempting to revert it.  I've now gone back into git,
> extracted the full commit to a file with 'git show', and then used
> git apply -R to revert it from 3.15-rc7.  That version wakes up from
> suspend to RAM, 3.15-rc7 itself did not.
>
>  Maybe I was still in git log when I thought I was on the command
> line.  Anyway, snipping git's view of my failed attempt to revert
> it, and adding Dan and Alex who were CC'd on the commit.
>

Duplicate of:
https://bugzilla.kernel.org/show_bug.cgi?id=74751
and also reported here:
https://lkml.org/lkml/2014/5/2/388
Unless there is a good reason to keep the commit, I'd say let's just revert it.

Alex

>>
>>  The processor is an AMD A4 APU.  The symptoms after this commit are
>> that pm-suspend works (I invoke it from my keyboard's sleep key),
>> i.e. the power LED on the case goes out and the monitor goes black
>> and reports no signal.  When I press a key to resume, the power LED
>> comes on but the screen stays black.  Magic-SysRQ does not work and
>> I have to use the case switch to reboot.  That results in filesystem
>> errors which fsck fixes and warns about (probably, just a sign of
>> unclean shutdown).
>>
>>  The only thing in the log are a couple of messages from EXT4 at the
>> end of putting the box to sleep -
>> May 28 16:19:02 bluesbreaker kernel: [   39.440859] EXT4-fs (sda10): re-mounted. Opts: commit=0
>> May 28 16:19:02 bluesbreaker kernel: [   39.592318] EXT4-fs (sda13): re-mounted. Opts: commit=0
>> May 28 16:20:05 bluesbreaker syslogd 1.5.0: restart.
>>
>>  What can I do to help debug this ?
>>
> ĸen
> --
> Nanny Ogg usually went to bed early. After all, she was an old lady.
> Sometimes she went to bed as early as 6 a.m.

Re: Resume from suspend broken in 3.15. (bisected)

[Dan Carpenter]

On Wed, May 28, 2014 at 08:26:53PM -0400, Alex Deucher wrote:
> On Wed, May 28, 2014 at 7:49 PM, Ken Moffat <zarniwhoop@ntlworld.com> wrote:
> > On Wed, May 28, 2014 at 06:25:21PM +0100, Ken Moffat wrote:
> >> Hi Daniel,
> >>
> >
> >  [ correcting details, confirming that reverting this does fix the
> > problem, adding Cc:s ]
> >
> >>  I've only started full testing of 3.15 on one of my machines now
> >> that -rc7 has been released (this one had two issues in the radeon
> >> code, second was fixed in rc7).  Unfortunately, suspend to RAM
> >> (pm-suspend), or rather the wake-up, is broken on this box [ my
> >> other two boxes are fine in rc7 ].
> >>
> >> Bisection identified one of your commits -
> >>
> >> commit 25f397a429dfa43f22c278d0119a60a343aa568f
> >> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Date:   Fri Jul 19 18:57:11 2013 +0200
> >>
> >>     drm/crtc-helper: explicit DPMS on after modeset
> >>
> >>     Atm the crtc helper implementation of set_config has really
> >>     inconsisten semantics: If just an fb update is good enough, dpms state
> >>     will be left as-is, but if we do a full modeset we force everything to
> >>     dpms on.
> >>
> >>     This change has already been applied to the i915 modeset code in
> >>
> >> ('git show' stops at that point)
> >
> >  update : I've no idea what was going on there, nor for the problem
> > with attempting to revert it.  I've now gone back into git,
> > extracted the full commit to a file with 'git show', and then used
> > git apply -R to revert it from 3.15-rc7.  That version wakes up from
> > suspend to RAM, 3.15-rc7 itself did not.
> >
> >  Maybe I was still in git log when I thought I was on the command
> > line.  Anyway, snipping git's view of my failed attempt to revert
> > it, and adding Dan and Alex who were CC'd on the commit.
> >
> 
> Duplicate of:
> https://bugzilla.kernel.org/show_bug.cgi?id=74751
> and also reported here:
> https://lkml.org/lkml/2014/5/2/388
> Unless there is a good reason to keep the commit, I'd say let's just revert it.
>

Yes.  Let's revert it.

regards,
dan carpenter

Re: Resume from suspend broken in 3.15. (bisected)

[Alex Deucher]

On Thu, May 29, 2014 at 2:03 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Wed, May 28, 2014 at 08:26:53PM -0400, Alex Deucher wrote:
>> On Wed, May 28, 2014 at 7:49 PM, Ken Moffat <zarniwhoop@ntlworld.com> wrote:
>> > On Wed, May 28, 2014 at 06:25:21PM +0100, Ken Moffat wrote:
>> >> Hi Daniel,
>> >>
>> >
>> >  [ correcting details, confirming that reverting this does fix the
>> > problem, adding Cc:s ]
>> >
>> >>  I've only started full testing of 3.15 on one of my machines now
>> >> that -rc7 has been released (this one had two issues in the radeon
>> >> code, second was fixed in rc7).  Unfortunately, suspend to RAM
>> >> (pm-suspend), or rather the wake-up, is broken on this box [ my
>> >> other two boxes are fine in rc7 ].
>> >>
>> >> Bisection identified one of your commits -
>> >>
>> >> commit 25f397a429dfa43f22c278d0119a60a343aa568f
>> >> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> >> Date:   Fri Jul 19 18:57:11 2013 +0200
>> >>
>> >>     drm/crtc-helper: explicit DPMS on after modeset
>> >>
>> >>     Atm the crtc helper implementation of set_config has really
>> >>     inconsisten semantics: If just an fb update is good enough, dpms state
>> >>     will be left as-is, but if we do a full modeset we force everything to
>> >>     dpms on.
>> >>
>> >>     This change has already been applied to the i915 modeset code in
>> >>
>> >> ('git show' stops at that point)
>> >
>> >  update : I've no idea what was going on there, nor for the problem
>> > with attempting to revert it.  I've now gone back into git,
>> > extracted the full commit to a file with 'git show', and then used
>> > git apply -R to revert it from 3.15-rc7.  That version wakes up from
>> > suspend to RAM, 3.15-rc7 itself did not.
>> >
>> >  Maybe I was still in git log when I thought I was on the command
>> > line.  Anyway, snipping git's view of my failed attempt to revert
>> > it, and adding Dan and Alex who were CC'd on the commit.
>> >
>>
>> Duplicate of:
>> https://bugzilla.kernel.org/show_bug.cgi?id=74751
>> and also reported here:
>> https://lkml.org/lkml/2014/5/2/388
>> Unless there is a good reason to keep the commit, I'd say let's just revert it.
>>
>
> Yes.  Let's revert it.

The actual bad commit is 177cf92de4aa97ec1435987e91696ed8b5023130, but
for some reason git bisect always comes up with
25f397a429dfa43f22c278d0119a60a343aa568f which has been in the tree
for almost a year now.  I don't know why.

Alex


>
> regards,
> dan carpenter
>

Re: Resume from suspend broken in 3.15. (bisected)

[Daniel Vetter]

On Thu, May 29, 2014 at 10:47 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Thu, May 29, 2014 at 2:03 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> On Wed, May 28, 2014 at 08:26:53PM -0400, Alex Deucher wrote:
>>> On Wed, May 28, 2014 at 7:49 PM, Ken Moffat <zarniwhoop@ntlworld.com> wrote:
>>> > On Wed, May 28, 2014 at 06:25:21PM +0100, Ken Moffat wrote:
>>> >> Hi Daniel,
>>> >>
>>> >
>>> >  [ correcting details, confirming that reverting this does fix the
>>> > problem, adding Cc:s ]
>>> >
>>> >>  I've only started full testing of 3.15 on one of my machines now
>>> >> that -rc7 has been released (this one had two issues in the radeon
>>> >> code, second was fixed in rc7).  Unfortunately, suspend to RAM
>>> >> (pm-suspend), or rather the wake-up, is broken on this box [ my
>>> >> other two boxes are fine in rc7 ].
>>> >>
>>> >> Bisection identified one of your commits -
>>> >>
>>> >> commit 25f397a429dfa43f22c278d0119a60a343aa568f
>>> >> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> >> Date:   Fri Jul 19 18:57:11 2013 +0200
>>> >>
>>> >>     drm/crtc-helper: explicit DPMS on after modeset
>>> >>
>>> >>     Atm the crtc helper implementation of set_config has really
>>> >>     inconsisten semantics: If just an fb update is good enough, dpms state
>>> >>     will be left as-is, but if we do a full modeset we force everything to
>>> >>     dpms on.
>>> >>
>>> >>     This change has already been applied to the i915 modeset code in
>>> >>
>>> >> ('git show' stops at that point)
>>> >
>>> >  update : I've no idea what was going on there, nor for the problem
>>> > with attempting to revert it.  I've now gone back into git,
>>> > extracted the full commit to a file with 'git show', and then used
>>> > git apply -R to revert it from 3.15-rc7.  That version wakes up from
>>> > suspend to RAM, 3.15-rc7 itself did not.
>>> >
>>> >  Maybe I was still in git log when I thought I was on the command
>>> > line.  Anyway, snipping git's view of my failed attempt to revert
>>> > it, and adding Dan and Alex who were CC'd on the commit.
>>> >
>>>
>>> Duplicate of:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=74751
>>> and also reported here:
>>> https://lkml.org/lkml/2014/5/2/388
>>> Unless there is a good reason to keep the commit, I'd say let's just revert it.
>>>
>>
>> Yes.  Let's revert it.
>
> The actual bad commit is 177cf92de4aa97ec1435987e91696ed8b5023130, but
> for some reason git bisect always comes up with
> 25f397a429dfa43f22c278d0119a60a343aa568f which has been in the tree
> for almost a year now.  I don't know why.

Quick patch which is worth a shot before we revert 177cf.
-Daniel

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index c2edb2d14030..cf5d299cc623 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1534,11 +1534,6 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 
 	radeon_restore_bios_scratch_regs(rdev);
 
-	if (fbcon) {
-		radeon_fbdev_set_suspend(rdev, 0);
-		console_unlock();
-	}
-
 	/* init dig PHYs, disp eng pll */
 	if (rdev->is_atom_bios) {
 		radeon_atom_encoder_init(rdev);
@@ -1563,6 +1558,12 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 	}
 
 	drm_kms_helper_poll_enable(dev);
+
+	if (fbcon) {
+		radeon_fbdev_set_suspend(rdev, 0);
+		console_unlock();
+	}
+
 	return 0;
 }
 
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: Resume from suspend broken in 3.15. (bisected)

[Ken Moffat]

On Thu, May 29, 2014 at 10:57:20PM +0200, Daniel Vetter wrote:
> On Thu, May 29, 2014 at 10:47 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> > On Thu, May 29, 2014 at 2:03 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >> On Wed, May 28, 2014 at 08:26:53PM -0400, Alex Deucher wrote:
> >>> On Wed, May 28, 2014 at 7:49 PM, Ken Moffat <zarniwhoop@ntlworld.com> wrote:
> >>> > On Wed, May 28, 2014 at 06:25:21PM +0100, Ken Moffat wrote:
> >>> >> Hi Daniel,
> >>> >>
> >>> >
> >>> >  [ correcting details, confirming that reverting this does fix the
> >>> > problem, adding Cc:s ]
> >>> >
> >>> >>  I've only started full testing of 3.15 on one of my machines now
> >>> >> that -rc7 has been released (this one had two issues in the radeon
> >>> >> code, second was fixed in rc7).  Unfortunately, suspend to RAM
> >>> >> (pm-suspend), or rather the wake-up, is broken on this box [ my
> >>> >> other two boxes are fine in rc7 ].
> >>> >>
> >>> >> Bisection identified one of your commits -
> >>> >>
> >>> >> commit 25f397a429dfa43f22c278d0119a60a343aa568f
> >>> >> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>> >> Date:   Fri Jul 19 18:57:11 2013 +0200
> >>> >>
> >>> >>     drm/crtc-helper: explicit DPMS on after modeset
> >>> >>
> >>> >>     Atm the crtc helper implementation of set_config has really
> >>> >>     inconsisten semantics: If just an fb update is good enough, dpms state
> >>> >>     will be left as-is, but if we do a full modeset we force everything to
> >>> >>     dpms on.
> >>> >>
> >>> >>     This change has already been applied to the i915 modeset code in
> >>> >>
> >>> >> ('git show' stops at that point)
> >>> >
> >>> >  update : I've no idea what was going on there, nor for the problem
> >>> > with attempting to revert it.  I've now gone back into git,
> >>> > extracted the full commit to a file with 'git show', and then used
> >>> > git apply -R to revert it from 3.15-rc7.  That version wakes up from
> >>> > suspend to RAM, 3.15-rc7 itself did not.
> >>> >
> >>> >  Maybe I was still in git log when I thought I was on the command
> >>> > line.  Anyway, snipping git's view of my failed attempt to revert
> >>> > it, and adding Dan and Alex who were CC'd on the commit.
> >>> >
> >>>
> >>> Duplicate of:
> >>> https://bugzilla.kernel.org/show_bug.cgi?id=74751
> >>> and also reported here:
> >>> https://lkml.org/lkml/2014/5/2/388
> >>> Unless there is a good reason to keep the commit, I'd say let's just revert it.
> >>>
> >>
> >> Yes.  Let's revert it.
> >
> > The actual bad commit is 177cf92de4aa97ec1435987e91696ed8b5023130, but
> > for some reason git bisect always comes up with
> > 25f397a429dfa43f22c278d0119a60a343aa568f which has been in the tree
> > for almost a year now.  I don't know why.
> 
> Quick patch which is worth a shot before we revert 177cf.
> -Daniel
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index c2edb2d14030..cf5d299cc623 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1534,11 +1534,6 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  
>  	radeon_restore_bios_scratch_regs(rdev);
>  
> -	if (fbcon) {
> -		radeon_fbdev_set_suspend(rdev, 0);
> -		console_unlock();
> -	}
> -
>  	/* init dig PHYs, disp eng pll */
>  	if (rdev->is_atom_bios) {
>  		radeon_atom_encoder_init(rdev);
> @@ -1563,6 +1558,12 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  	}
>  
>  	drm_kms_helper_poll_enable(dev);
> +
> +	if (fbcon) {
> +		radeon_fbdev_set_suspend(rdev, 0);
> +		console_unlock();
> +	}
> +
>  	return 0;
>  }
>  
 Thanks, Daniel.

 That works for me (with -rc7) on my A4 Trinity (Radeon HD7480D).  I
also tested it on my other radeon (RS780L - Radeon 3000) and saw no
problems in suspend-resume.  If useful, you can add my tested-by.

ĸen
-- 
Nanny Ogg usually went to bed early. After all, she was an old lady.
Sometimes she went to bed as early as 6 a.m.

Re: Resume from suspend broken in 3.15. (bisected)

[Pavel Machek]

Hi!

>  I've only started full testing of 3.15 on one of my machines now
> that -rc7 has been released (this one had two issues in the radeon
> code, second was fixed in rc7).  Unfortunately, suspend to RAM
> (pm-suspend), or rather the wake-up, is broken on this box [ my
> other two boxes are fine in rc7 ].
> 
> Bisection identified one of your commits -
> 
> commit 25f397a429dfa43f22c278d0119a60a343aa568f

Thanks a lot for your work. It seems this commit breaks i915, too.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Resume from suspend broken in 3.15. (bisected)

[Daniel Vetter]

On Sat, Jun 7, 2014 at 2:01 PM, Pavel Machek <pavel@ucw.cz> wrote:
>>  I've only started full testing of 3.15 on one of my machines now
>> that -rc7 has been released (this one had two issues in the radeon
>> code, second was fixed in rc7).  Unfortunately, suspend to RAM
>> (pm-suspend), or rather the wake-up, is broken on this box [ my
>> other two boxes are fine in rc7 ].
>>
>> Bisection identified one of your commits -
>>
>> commit 25f397a429dfa43f22c278d0119a60a343aa568f
>
> Thanks a lot for your work. It seems this commit breaks i915, too.

Really, really unlikely since we don't use that code with i915 at all.
Or git bisect has simply gone completely bananas and always brings up
this commit.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch