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

[Konstantin Khlebnikov]

On Tue, Jul 16, 2013 at 10:31 AM, Daniel Vetter <daniel at ffwll.ch> wrote:

> On Sun, Jul 14, 2013 at 09:56:45PM +0400, Konstantin Khlebnikov wrote:
> > Daniel Vetter wrote:
> > >On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov
> > ><khlebnikov at openvz.org>  wrote:
> > >>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 was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0
> > >>("drm/i915: load boot context at driver init time"). Without
> documentation
> > >>it's not clear what is happening here, probably this breaks internal
> state of
> > >>hardware ring buffers and confuses RPS engine. Fortunately keeping
> forcewake
> > >>during whole initialization sequence in gen6_init_clock_gating() fixes
> this bug.
> > >>
> > >>References: https://bugs.freedesktop.org/show_bug.cgi?id=54089
> > >>Signed-off-by: Konstantin Khlebnikov<khlebnikov at openvz.org>
> > >
> > >We already hold an forcewake reference while setting up the rps stuff,
> > >should we maybe hold the forcewake for the entire duration, i.e. grab
> > >it here in clock_gating and release it only in gen6/vlv_enable_rps?
> > >Can you please test that version, too?
> >
> > This will be racy because rps stuff is done in separate work which might
> be canceled
> > if intel_disable_gt_powersave() happens before its completion.
>
> Can be fixed with a flush_delayed_work. And since that has the same
> requirements wrt locking to prevent deadlocks as cancel_work_sync it would
> be a drop-in replacement. Can I volunteer you to look into testing that
> out a bit? Otherwise I could volunteer someone from our team.
>
> In any case I think we should apply this trick to all platforms where
> we've added the MBCTL write (i.e. snb, ivb, hsw & vlv) since rps/rc6 works
> _very_ similar on all of those.
>

I've tested that patch and it really works for me. If you want change
something for other hardware or
extend range where forcewake is held prease do it in a separate patch.
This will be good for bisecting new bugs in the future.


>
> Thanks, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130716/85ae4ac4/attachment.html>