Bug 52411 - [snb] intel gpu never go in rc6 after suspend
Summary: [snb] intel gpu never go in rc6 after suspend
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Video(DRI - Intel) (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: intel-gfx-bugs@lists.freedesktop.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-07 09:19 UTC by Alexander Bersenev
Modified: 2013-04-21 15:55 UTC (History)
9 users (show)

See Also:
Kernel Version: 3.7.1
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments
Xorg.0.log (27.49 KB, text/plain)
2013-01-07 09:19 UTC, Alexander Bersenev
Details
intel_reg_checker output when all is good (2.05 KB, text/plain)
2013-01-07 09:29 UTC, Alexander Bersenev
Details
intel_reg_checker output when in permanent "powered on state" (2.02 KB, text/plain)
2013-01-07 09:30 UTC, Alexander Bersenev
Details
patch (1.57 KB, patch)
2013-01-09 20:34 UTC, Alexander Bersenev
Details | Diff
Second version of fix patch (870 bytes, patch)
2013-01-10 12:09 UTC, Alexander Bersenev
Details | Diff
drm/i915: fix FORCEWAKE posting reads (3.66 KB, patch)
2013-01-10 12:26 UTC, Jani Nikula
Details | Diff

Description Alexander Bersenev 2013-01-07 09:19:38 UTC
Created attachment 90581 [details]
Xorg.0.log

In kernels 3.7.0 and 3.7.1 gpu sometimes switches in permanent "always-powered on" state after resume from suspend.

This results in increased power consumpution and a noise from fans.

An output from powertop:
|             GPU     |
|                     |
| Powered On100.0%    |
| RC6         0.0%    |
| RC6p        0.0%    |
| RC6pp       0.0%    |

intel_gpu_top says that there are no programs who use gpu:

render busy:      0%:                 render space: 0/131072
bitstream busy:   0%:                 bitstream space: 0/131072
  blitter busy:   0%:                 blitter space: 0/131072

task  percent busy
                                      vert fetch: 0 (0/sec)
                                      prim fetch: 0 (0/sec)
                                  VS invocations: 0 (0/sec)
                                  GS invocations: 0 (0/sec)
                                        GS prims: 0 (0/sec)
                                  CL invocations: 0 (0/sec)
                                        CL prims: 0 (0/sec)
                                  PS invocations: 0 (0/sec)
                                   PS depth pass: 0 (0/sec)

I haven't seen this on older kernels(3.6.8, 3.6.6, 3.6.1, 3.6.0, 3.5.2, 3.5.0, 3.4.5, 3.4.0, 3.3.0, 3.2.5, 3.2.0, 3.1.0, 3.0.3)

Some data about the system:
# uname -a
Linux BaysGentooNotebook 3.7.1-gentoo #1 SMP Thu Jan 3 22:16:59 YEKT 2013 x86_64 Intel(R) Core(TM) i5-2410M CPU @ 2.30GHz GenuineIntel GNU/Linux
# intel_stepping 
Vendor: 0x8086, Device: 0x0116, Revision: 0x09 (??)
# dmesg | grep RC6
[ 3388.580819] [drm] Enabling RC6 states: RC6 on, RC6p off, RC6pp off
[ 5458.208680] [drm] Enabling RC6 states: RC6 on, RC6p off, RC6pp off
[27481.114489] [drm] Enabling RC6 states: RC6 on, RC6p off, RC6pp off
[29492.097527] [drm] Enabling RC6 states: RC6 on, RC6p off, RC6pp off
[29909.872220] [drm] Enabling RC6 states: RC6 on, RC6p off, RC6pp off

Kernel command line: root=/dev/sda7 rootfstype=ext4 resume=/dev/sda6 rw pcie_aspm=force i915.i915_enable_rc6=1 quiet
Comment 1 Alexander Bersenev 2013-01-07 09:29:30 UTC
Created attachment 90591 [details]
intel_reg_checker output when all is good
Comment 2 Alexander Bersenev 2013-01-07 09:30:08 UTC
Created attachment 90601 [details]
intel_reg_checker output when in permanent "powered on state"
Comment 3 Daniel Vetter 2013-01-07 10:00:56 UTC
Since this seems rather clear-cut, can you attempt to bisect where this regression has been introduced?
Comment 4 Alexander Bersenev 2013-01-08 20:25:49 UTC
612a9aab56a93533e76e3ad91642db7033e03b69 is the first bad commit
Comment 5 Jani Nikula 2013-01-09 09:00:53 UTC
(In reply to comment #4)
> 612a9aab56a93533e76e3ad91642db7033e03b69 is the first bad commit

Hmm, that's a merge commit, not very helpful. :(
Comment 6 Alexander Bersenev 2013-01-09 20:34:49 UTC
Created attachment 90941 [details]
patch

I've attached a patch. It fixes bug.
Comment 7 Jani Nikula 2013-01-10 08:04:45 UTC
(In reply to comment #6)
> Created an attachment (id=90941) [details]
> patch
> 
> I've attached a patch. It fixes bug.

Interesting. That's effectively a revert of:

commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Sat Sep 1 22:59:50 2012 -0700

    drm/i915: Never read FORCEWAKE

CC Ben. Any thoughts?
Comment 8 Alexander Bersenev 2013-01-10 10:48:50 UTC
May be, how follows from commit:

  I915_WRITE_NOTRACE(FORCEWAKE, 1);
- POSTING_READ(ECOBUS); /* something from same cacheline, but !FORCEWAKE */
+ POSTING_READ(FORCEWAKE);

driver should read something from same cacheline after writing to FORCEWAKE register, but in another place:

- /* gen6_gt_check_fifodbg doubles as the POSTING_READ */
+ POSTING_READ(FORCEWAKE); 
  gen6_gt_check_fifodbg(dev_priv);

it is said that gen6_gt_check_fifodbg doing POSING_READ, but gen6_gt_check_fifodbg is reading from GTFIFODBG register and it is not on the same cacheline
Comment 9 Alexander Bersenev 2013-01-10 12:09:38 UTC
Created attachment 90971 [details]
Second version of fix patch

Here is a second version of fix patch
Comment 10 Jani Nikula 2013-01-10 12:26:40 UTC
Created attachment 90981 [details]
drm/i915: fix FORCEWAKE posting reads

Ahah, seems like you beat me to it, but here's my version anyway.
Comment 11 Alexander Bersenev 2013-01-10 19:11:43 UTC
Thank you! 

Hope to see this patch in next kernel versions.

I think the bug can be closed as fixed.
Comment 12 Ben Widawsky 2013-01-11 18:45:55 UTC
This patch has been submitted to our developer's mailing list. While the patch itself is fairly benign, I am worried that there is another problem here which we are missing.

Personally, I fear the extra read is what's actually fixing the problem. Alexander, could you please try replacing the read of ECOBUS with some other register. GEN6_GT_MODE for instance. If that works, please try a second read of GTFIFODBG. If it doesn't work, can you try some other register in the same CL that isn't ECOBUS or FORCEWAKE?

I'd just like to get as much info into the commit message of the fix before we upstream the patch.

Thank you.
Comment 13 Alexander Bersenev 2013-01-11 21:14:50 UTC
It works good with one read operation of ECOBUS or GEN6_GT_MODE register. Also it works with any two read operations, for example with 2xGTFIFODBG. It doesn't work in the right way with one read operation of GTFIFODBG or FORCEWAKE_VLV.
Comment 14 Alexander Bersenev 2013-01-12 07:26:50 UTC
The part about any two operations was wrong. It works 2xGTFIFODBG, but not works with some other registers.
Comment 15 Ben Widawsky 2013-01-12 18:08:33 UTC
(In reply to comment #14)
> The part about any two operations was wrong. It works 2xGTFIFODBG, but not
> works with some other registers.

Which registers do not work (that you've tried)? 
Please only try registers < 0x40000 as to not require forcewake for those.
Comment 16 Alexander Bersenev 2013-01-12 18:30:43 UTC
I tried to insert just after I915_WRITE_NOTRACE(FORCEWAKE, 0) posting readings of:
ECOBUS (GOOD)
GEN6_GT_MODE (GOOD)
ECOBUS + GEN6_GT_MODE (GOOD)
GTFIFODBG + GTFIFODBG (GOOD)
ECOBUS + GTFIFODBG (GOOD)
GEN6_GT_MODE + GTFIFODBG (GOOD)

<NONE> (BAD)
GTFIFODBG (BAD)
FORCEWAKE_VLV (BAD)
<SOME REGISTER > 0x40000> <SOME REGISTER > 0x40000> (BAD)

I not remember what was the value of SOME REGISTER exactly but is definately > 0x40000.

I can check any other readings after I915_WRITE_NOTRACE if it helps.
Comment 17 Alexander Bersenev 2013-01-12 20:22:42 UTC
Checked few more:

GEN6_UCGCTL1 (GOOD)
GEN6_RP_UP_THRESHOLD (GOOD)
_PALETTE_A (GOOD)

udelay(1000) and no read (BAD)
Comment 18 Ben Widawsky 2013-01-14 15:55:49 UTC
FORCEWAKE_VLV makes sense, since this platform is not Valleyview, and won't have that register.

So what have we learned then, GTFIFODBG and registers requiring forcewake are bad? 

I wonder if it's not always valid to read GTFIFODBG.
Comment 19 Alexander Bersenev 2013-01-14 17:00:03 UTC
By the way, GTFIFODBG is 0x120000, this is more than 0x40000.

May be my platform doesn't have GTFIFODBG regsiter as well(or the value of GTFIFODBG is wrong)?
Comment 20 Jani Nikula 2013-01-15 13:29:57 UTC
Alexander, if it's not too much trouble, please try these registers, one at a time:

FENCE_REG_SANDYBRIDGE_0
TILECTL
_HTOTAL_A
PP_CONTROL
PCH_PORT_HOTPLUG
Comment 21 Alexander Bersenev 2013-01-15 15:44:25 UTC
FENCE_REG_SANDYBRIDGE_0(BAD)
TILECTL(BAD)
_HTOTAL_A(BAD)
PP_CONTROL(BAD)
PCH_PORT_HOTPLUG(GOOD)

BAD means that I managed to catch permanent 100% powered on state for 10-15 minutes when there was no applications who uses gpu.

GOOD - means that I tried for quite long time and bug wasn't occured.
Comment 22 Jani Nikula 2013-01-16 12:47:10 UTC
Thanks for the resuts. The registers were all > 0x40000.

(In reply to comment #21)
> FENCE_REG_SANDYBRIDGE_0(BAD)
> TILECTL(BAD)

The above two are GTTMMADR.

> _HTOTAL_A(BAD)

This is a CPU reg.

> PP_CONTROL(BAD)
> PCH_PORT_HOTPLUG(GOOD)

These are both supposed to be PCH regs.

Ben, Daniel, any ideas for explaining this? Or shall I just "think of something" for the commit message...?
Comment 23 Daniel Vetter 2013-01-16 22:26:32 UTC
(In reply to comment #22)
> Ben, Daniel, any ideas for explaining this? Or shall I just "think of
> something" for the commit message...?

I think just summarize the findings here and if you want to be creative, give this new elephant a cute name.
Comment 24 Jani Nikula 2013-01-25 13:54:51 UTC
commit b514407547890686572606c9dfa4b7f832db9958
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Thu Jan 17 10:24:09 2013 +0200

    drm/i915: fix FORCEWAKE posting reads
Comment 25 Toralf Förster 2013-03-19 15:01:23 UTC
Isn't this a dup of bug #48791 ?

Note You need to log in before you can comment on or make changes to this bug.