drm/i915: fix long-standing SNB regression in power consumption after resume v2

[emaste]

Summary

Obtained from: Linux 7dcd2677ea912573d9ed4bcd629b0023b2d11505
https://github.com/torvalds/linux/commit/7dcd2677ea912573d9ed4bcd629b0023b2d11505

This patch fixes regression in power consumtion of sandy bridge gpu, which
exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that
it's extremely busy. After that it never reaches rc6 state.

Bug exists since kernel v3.6:

commit b4ae3f2
Author: Jesse Barnes <[e-mail]>
Date: Thu Jun 14 11:04:48 2012 -0700

drm/i915: load boot context at driver init time

For some reason RC6 is already enabled at the beginning of resuming process.
Following initliaztion breaks some internal state and confuses RPS engine.
This patch disables RC6 at the beginnig of resume and initialization.

I've rearranged initialization sequence, because intel_disable_gt_powersave()
needs initialized force_wake_get/put and some locks from the dev_priv.

Note: The culprit in the initialization sequence seems to be the write
to MBCTL added in the above mentioned commit. The first version of
this patch just held a forcewake reference across the clock gating
init functions, which seems to have been enought to gather quite a few
positive test reports. But since that smelled a bit like ad-hoc
duct-tape v2 now just disables rps/rc6 across the entire hw setup.

References: https://bugs.freedesktop.org/show_bug.cgi?id=54089
References: https://bugzilla.kernel.org/show_bug.cgi?id=58971
References: https://patchwork.kernel.org/patch/2827634/ (patch v1)
Signed-off-by: Konstantin Khlebnikov <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
Cc: Chris Wilson <[e-mail]>
Cc: Jesse Barnes <[e-mail]>
[danvet: Add note about v1 vs. v2 of this patch and use standard
layout for the commit citation. Also add the tested-bys from v1 and a
cc: stable.]
Cc: [e-mail] (Note: tiny conflict due to the addition of
the backlight lock in 3.11)
Tested-by: Alexander Kaltsas <[e-mail]> (v1)
Tested-by: rocko <[e-mail]> (v1)
Tested-by: JohnMB <[e-mail]> (v1)
Signed-off-by: Daniel Vetter <[e-mail]>

With this patch I see reasonable power consumption on my SB laptop (Thinkpad X220) after a suspend-resume cycle -- about 15W in X with firefox running. Power consumption after reboot and loading i915kms.ko, but not starting X is not reasonable -- still about 30W.

[cem]

With this patch I see reasonable power consumption on my SB laptop (Thinkpad X220) after a suspend-resume cycle -- about 15W in X with firefox running. Power consumption after reboot and loading i915kms.ko, but not starting X is not reasonable -- still about 30W.

I don't see a lot of point in taking this patch without fixing the normal boot case too. It does seem promising, though.

[emaste]

I don't see a lot of point in taking this patch without fixing the normal boot case too. It does seem promising, though.

Even if it only solves power consumption after S+R I'd still want to commit it, both for the specific improvement and to have a direct import of the linux commit.

If we do come up with other changes for the straight boot case I'd want to commit them separately anyway.

More testing is still needed, with and without this patch, before it can be committed though.

[emaste]

drm/i915: fix up gt init sequence fallout

The regression fix for gen6+ rps fallout

commit 7dcd2677ea912573d9ed4bcd629b0023b2d11505
Author: Konstantin Khlebnikov <[e-mail]>
Date: Wed Jul 17 10:22:58 2013 +0400

drm/i915: fix long-standing SNB regression in power consumption after resume

unintentionally also changed the init sequence ordering between
gt_init and gt_reset - we need to reset BIOS damage like leftover
forcewake references before we run our own code. Otherwise we can get
nasty dmesg noise like

[drm:__gen6_gt_force_wake_mt_get] *ERROR* Timed out waiting for forcewake old ack to clear.

again. Since _reset suggests that we first need to have stuff
initialized (which isn't the case here) call it sanitze instead.

While at it also block out the rps disable introduced by the above
commit on ilk: We don't have any knowledge of ilk rps being broken in
similar ways. And the disable functions uses the default hw state
which is only read out when we're enabling rps. So essentially we've
been writing random grabage into that register.

Reported-by: Chris Wilson <[e-mail]>
Cc: Chris Wilson <[e-mail]>
Cc: Konstantin Khlebnikov <[e-mail]>
Cc: Jesse Barnes <[e-mail]>
Cc: [e-mail]
Tested-by: Chris Wilson <[e-mail]>
Reviewed-by: Chris Wilson <[e-mail]>
Signed-off-by: Daniel Vetter <[e-mail]>

[cem]

If these two patches fix the issue, please commit.

[dumbbell]

@emaste: So both patches together fix the power consumption regression you saw (after kldload and after suspend/resume), is that right?

Are they two patches from Linux, or one from Linux and one from you?

[emaste]

Oops, I missed the hash for the second one -- it's commit d11a96d0a4927e8948a6e92a581779f961d9745b. I found it while looking through other changes in drivers/gpu/drm/i915 and it referenced the first one.

I'll do more testing today, but my x220 + this patch after suspend/resume has normal power consumption. It's more important I think that we make sure it doesn't introduce regressions on newer hardware though.

[dumbbell]

Should this review be closed, now that D5665 was committed?

[emaste]

Not necessary after D5665