i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Lubos Kolouch]

Hello,

After upgrading kernel from 3.1.5 to 3.2.0 the notebook cannot wake from
suspend to RAM properly. The problem stays even with 3.3-rc1.

It wakes, but the screen is blank and computer needs to be rebooted.

When woken, computer is working otherwise (can ping it, ssh to it etc.)

I git bisected it to commit
cd9dde44f47501394b9f0715b6a36a92aa74c0d0 is the first bad commit
commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0
Author: Adam Jackson <ajax@redhat.com>
Date:   Fri Oct 14 12:43:49 2011 -0400

    drm/i915/dp: Fix the math in intel_dp_link_required

    The previous code was confused about units, which is pretty reasonable
    given that the units themselves are confusing.

    Signed-off-by: Adam Jackson <ajax@redhat.com>
    Signed-off-by: Keith Packard <keithp@keithp.com>

:040000 040000 3c0a0f38fc6482a401340e03fa3fbd687d11eda4
294673b782b3ecc93540f46f7721d385bd30cfac M    drivers


Reproducible: Always

Steps to Reproduce:
1. suspend to RAM
2. start the computer
Actual Results:  
Screen is blank

Expected Results:  
Screen should be normal as before suspend

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Keith Packard]

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

On Sat, 21 Jan 2012 10:32:14 +0000 (UTC), Lubos Kolouch <lubos.kolouch@gmail.com> wrote:

> I git bisected it to commit
> cd9dde44f47501394b9f0715b6a36a92aa74c0d0 is the first bad commit
> commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0
> Author: Adam Jackson <ajax@redhat.com>
> Date:   Fri Oct 14 12:43:49 2011 -0400

Did you try reverting this single commit on top of 3.3-rc1?

And, can you collect the dmesg output with the kernel parameter drm.debug=0xe
added to the boot line, both messages at boot time *and* messages during
resume would be needed. If it works at boot time, I'm quite surprised it
somehow doesn't work at resume...

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Lubos Kolouch]

On Sat, Jan 21, 2012, 10:15:24, Keith Packard wrote:
> On Sat, 21 Jan 2012 10:32:14 +0000 (UTC), Lubos Kolouch <lubos.kolouch@gmail.com> wrote:
> 
> > I git bisected it to commit
> > cd9dde44f47501394b9f0715b6a36a92aa74c0d0 is the first bad commit
> > commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0
> > Author: Adam Jackson <ajax@redhat.com>
> > Date:   Fri Oct 14 12:43:49 2011 -0400
> 
> Did you try reverting this single commit on top of 3.3-rc1?
> 
> And, can you collect the dmesg output with the kernel parameter drm.debug=0xe
> added to the boot line, both messages at boot time *and* messages during
> resume would be needed. If it works at boot time, I'm quite surprised it
> somehow doesn't work at resume...

I tried now and when I revert this line :

- return (pixel_clock * bpp + 7) / 8;
+ return (pixel_clock * bpp + 9) / 10;

resume works OK for me with 3.3-rc1.

Before change (failed)
----------------------
dmesg after boot - http://paste.pocoo.org/show/538343/
dmesg after failed resume -  http://paste.pocoo.org/show/538344/

After change (OK)
-----------------
dmesg after boot - http://paste.pocoo.org/show/538345/
dmesg after OK resume - http://paste.pocoo.org/show/538346/

Luboš Kolouch

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Lubos Kolouch]

Keith Packard, Sat, 21 Jan 2012 10:15:24 -0800:

> On Sat, 21 Jan 2012 10:32:14 +0000 (UTC), Lubos Kolouch
> <lubos.kolouch@gmail.com> wrote:
> 
>> I git bisected it to commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0 is
>> the first bad commit commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0
>> Author: Adam Jackson <ajax@redhat.com>
>> Date:   Fri Oct 14 12:43:49 2011 -0400
> 
> Did you try reverting this single commit on top of 3.3-rc1?
> 
> And, can you collect the dmesg output with the kernel parameter
> drm.debug=0xe added to the boot line, both messages at boot time *and*
> messages during resume would be needed. If it works at boot time, I'm
> quite surprised it somehow doesn't work at resume...

I tried now and when I revert this line :

- return (pixel_clock * bpp + 7) / 8;
+ return (pixel_clock * bpp + 9) / 10;

resume works OK for me with 3.3-rc1.

Before change (failed)
----------------------
dmesg after boot - http://paste.pocoo.org/show/538343/
dmesg after failed resume -  http://paste.pocoo.org/show/538344/

After change (OK)
-----------------
dmesg after boot - http://paste.pocoo.org/show/538345/
dmesg after OK resume - http://paste.pocoo.org/show/538346/

Luboš Kolouch

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 567 bytes --]

On Sat, 21 Jan 2012 19:44:53 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:

> dmesg after boot - http://paste.pocoo.org/show/538343/
> dmesg after failed resume -  http://paste.pocoo.org/show/538344/

Ok, that's weird -- we're choosing a different display port link speed
on resume than at boot time. I have a theory that intel_dp_mode_fixup is
accidentally using the current bpp value when computing the values
needed for the new mode, and that this value may differ between boot and
resume.

Here's a patch to dump some debug info out to test this theory.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-drm-i915-Output-a-bit-of-debug-info-for-display-port.patch --]
[-- Type: text/x-diff, Size: 1411 bytes --]

>From 2b16db9fccc6927f3e09020144d5148acc025f58 Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Sat, 21 Jan 2012 12:48:38 -0800
Subject: [PATCH] drm/i915: Output a bit of debug info for display port
 bandwidth

Something weird is going on with link bandwidth testing; this may
help sort out what's wrong.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index db3b461..796d534 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -697,10 +697,13 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		mode->clock = intel_dp->panel_fixed_mode->clock;
 	}
 
+	DRM_DEBUG_KMS("using bpp %d to find link_bw and lane_count\n", bpp);
 	for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) {
 		for (clock = 0; clock <= max_clock; clock++) {
 			int link_avail = intel_dp_max_data_rate(intel_dp_link_clock(bws[clock]), lane_count);
 
+			DRM_DEBUG_KMS("lane_count %d clock %d link_avail %d link_required %d\n",
+				      lane_count, clock, link_avail, intel_dp_link_required(intel_dp, mode->clock, bpp));
 			if (intel_dp_link_required(intel_dp, mode->clock, bpp)
 					<= link_avail) {
 				intel_dp->link_bw = bws[clock];
-- 
1.7.8.3


[-- Attachment #3: Type: text/plain, Size: 130 bytes --]


Please try this on the broken kernel, sending along the dmesg output at
boot and resume time again.

-- 
keith.packard@intel.com

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Lubos Kolouch]

On Sat, Jan 21, 2012, 12:53:00, Keith Packard wrote:
> On Sat, 21 Jan 2012 19:44:53 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:
> 
> > dmesg after boot - http://paste.pocoo.org/show/538343/
> > dmesg after failed resume -  http://paste.pocoo.org/show/538344/
> 
> Ok, that's weird -- we're choosing a different display port link speed
> on resume than at boot time. I have a theory that intel_dp_mode_fixup is
> accidentally using the current bpp value when computing the values
> needed for the new mode, and that this value may differ between boot and
> resume.
> 
> Here's a patch to dump some debug info out to test this theory.
> 

Thanks, here you go:
dmesg after boot - http://paste.pocoo.org/show/538625/
dmesg after failed resume - http://paste.pocoo.org/show/538626/

Lubos

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Keith Packard]

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

On Sun, 22 Jan 2012 11:29:12 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:

> Thanks, here you go:
> dmesg after boot - http://paste.pocoo.org/show/538625/
> dmesg after failed resume - http://paste.pocoo.org/show/538626/

Ok, looks like my theory is correct. We're fitting a 24bpp mode to an
18bpp data rate, which doesn't work.

I'm in the middle of bisecting a bluetooth failure in 3.3-rc1, so I
don't have a working kernel at present, but here's a section of
intel_dp.c in the intel_dp_mode_fixup function:

  	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
-	int bpp = mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 0;
+	int bpp = mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };

(this is completely hand-generated, so 'patch' will almost certainly not work)

This will force it to use the correct bpp when computing the bandwidth
used by the new mode. This should fix your problem. If so, I'll have a
larger patch which stops the underlying function from (almost always
incorrectly) using the current bpp when computing bandwidth.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Lubos Kolouch]

On Sun, Jan 22, 2012, 20:00:18, Keith Packard wrote:
> On Sun, 22 Jan 2012 11:29:12 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:
> 
> > Thanks, here you go:
> > dmesg after boot - http://paste.pocoo.org/show/538625/
> > dmesg after failed resume - http://paste.pocoo.org/show/538626/
> 
> Ok, looks like my theory is correct. We're fitting a 24bpp mode to an
> 18bpp data rate, which doesn't work.
> 
> I'm in the middle of bisecting a bluetooth failure in 3.3-rc1, so I
> don't have a working kernel at present, but here's a section of
> intel_dp.c in the intel_dp_mode_fixup function:
> 
>   	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
> -	int bpp = mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 0;
> +	int bpp = mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
>  	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
> 
> (this is completely hand-generated, so 'patch' will almost certainly not work)
> 
> This will force it to use the correct bpp when computing the bandwidth
> used by the new mode. This should fix your problem. If so, I'll have a
> larger patch which stops the underlying function from (almost always
> incorrectly) using the current bpp when computing bandwidth.
> 
> -- 
> keith.packard@intel.com
Thanks, I applied it and it seems to solve the issue!

dmesg after boot - http://paste.pocoo.org/show/539115/
dmesg after OK resume - http://paste.pocoo.org/show/539117/

Lubos

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Keith Packard]

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

On Mon, 23 Jan 2012 07:33:14 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:

> dmesg after boot - http://paste.pocoo.org/show/539115/
> dmesg after OK resume - http://paste.pocoo.org/show/539117/

I've posted a couple of more comprehensive patches that you should have
received; I'd like to know if you can test them to see if they resolve
the issue as well.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Lubos Kolouch]

On Wed, Jan 25, 2012, 23:37:28, Keith Packard wrote:
> On Mon, 23 Jan 2012 07:33:14 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:
> 
> > dmesg after boot - http://paste.pocoo.org/show/539115/
> > dmesg after OK resume - http://paste.pocoo.org/show/539117/
> 
> I've posted a couple of more comprehensive patches that you should have
> received; I'd like to know if you can test them to see if they resolve
> the issue as well.

1) applied the first patch, resume works OK
dmesg after boot : http://paste.pocoo.org/show/541205/
dmesg after resume : http://paste.pocoo.org/show/541206/

2) applied second patch on top, resume works OK
dmesg after boot: http://paste.pocoo.org/show/541477/
dmesg after resume : http://paste.pocoo.org/show/541479/

Lubos

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Keith Packard]

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

On Fri, 27 Jan 2012 11:14:03 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:
> On Wed, Jan 25, 2012, 23:37:28, Keith Packard wrote:
> > On Mon, 23 Jan 2012 07:33:14 +0100, Lubos Kolouch <lubos.kolouch@gmail.com> wrote:
> > 
> > > dmesg after boot - http://paste.pocoo.org/show/539115/
> > > dmesg after OK resume - http://paste.pocoo.org/show/539117/
> > 
> > I've posted a couple of more comprehensive patches that you should have
> > received; I'd like to know if you can test them to see if they resolve
> > the issue as well.
> 
> 1) applied the first patch, resume works OK
> dmesg after boot : http://paste.pocoo.org/show/541205/
> dmesg after resume : http://paste.pocoo.org/show/541206/
> 
> 2) applied second patch on top, resume works OK
> dmesg after boot: http://paste.pocoo.org/show/541477/
> dmesg after resume : http://paste.pocoo.org/show/541479/

Thanks!

I'll append a 'Tested-by: Lubos Kolouch <lubos.kolouch@gmail.com>'

to these patches when they go into the kernel.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Joseph Salisbury]

On 01/27/2012 01:57 PM, Keith Packard wrote:
> On Fri, 27 Jan 2012 11:14:03 +0100, Lubos Kolouch<lubos.kolouch@gmail.com>  wrote:
>> On Wed, Jan 25, 2012, 23:37:28, Keith Packard wrote:
>>> On Mon, 23 Jan 2012 07:33:14 +0100, Lubos Kolouch<lubos.kolouch@gmail.com>  wrote:
>>>
>>>> dmesg after boot - http://paste.pocoo.org/show/539115/
>>>> dmesg after OK resume - http://paste.pocoo.org/show/539117/
>>>
>>> I've posted a couple of more comprehensive patches that you should have
>>> received; I'd like to know if you can test them to see if they resolve
>>> the issue as well.
>>
>> 1) applied the first patch, resume works OK
>> dmesg after boot : http://paste.pocoo.org/show/541205/
>> dmesg after resume : http://paste.pocoo.org/show/541206/
>>
>> 2) applied second patch on top, resume works OK
>> dmesg after boot: http://paste.pocoo.org/show/541477/
>> dmesg after resume : http://paste.pocoo.org/show/541479/
>
> Thanks!
>
> I'll append a 'Tested-by: Lubos Kolouch<lubos.kolouch@gmail.com>'
>
> to these patches when they go into the kernel.
>
Hi Keith,

Do you plan on submitting a request to have these patches go in to the 
3.2 stable kernel as well?

Thanks,

Joe

Re: i915 wakes from suspend to RAM with blank screen after commit cd9dde44f47501394b9f0715b6a36a92aa74c0d0

[Keith Packard]

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

On Tue, 31 Jan 2012 17:39:29 -0500, Joseph Salisbury <joseph.salisbury@canonical.com> wrote:

> Do you plan on submitting a request to have these patches go in to the 
> 3.2 stable kernel as well?

I've marked them as Cc: stable@vger.kernel.org

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]