Re: [PATCH] drm/i915: Correct sandybrige overclocking

[Ben Widawsky <ben@bwidawsk.net>]

On Sat, Mar 23, 2013 at 08:39:30PM +0100, Daniel Vetter wrote:
> On Sat, Mar 23, 2013 at 08:32:53PM +0100, Daniel Vetter wrote:
> > On Tue, Mar 19, 2013 at 08:19:56PM -0700, Ben Widawsky wrote:
> > > Change the gen6+ max delay if the pcode read was successful (not the
> > > inverse).
> > > 
> > > The previous code was all sorts of wrong and has existed since I broke
> > > it:
> > > commit 42c0526c930523425ff6edc95b7235ce7ab9308d
> > > Author: Ben Widawsky <ben@bwidawsk.net>
> > > Date:   Wed Sep 26 10:34:00 2012 -0700
> > > 
> > >     drm/i915: Extract PCU communication
> > > 
> > > I added some parentheses for clarity, and I also corrected the debug
> > > message message to use the mask (wrong before I came along) and added a
> > > print to show the value we're changing from.
> > > 
> > > Looking over the code, I'm not actually sure what we're trying to do. I
> > > introduced the bug simply by extracting the function not implementing
> > > anything new. We already set max_delay based on the capabilities
> > > register (which is what we use elsewhere to determine min and max).
> > > This would potentially increase it, I suppose? Jesse, I can't find the
> > > document which explains the definitions of the pcode commands, maybe you
> > > have it around.
> > > 
> > > Based on Jesse's response, this could potentially be for -fixes, or
> > > stable, or maybe lead to us dropping it entirely. As the current code is
> > > is, things won't completely break because of the aforementioned
> > > capabilities register, and in my experimentation, enabling this has no
> > > effect, it goes from 1100->1100.
> > > 
> > > I found this while reviewing Jesse's VLV patches.
> > > 
> > > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > Random guy on irc just confirmed that this actually fixes gpu overclocking
> > on his machine: The overclock limit jumped from 1.1GHz to 1.9GHz (which is
> > what he selected in his bios). Unfortunately he jumped irc before I could
> > grab his tested-by, so won't (yet) move the patch to -fixes with a cc:
> > stable.
> > 
> > So: (Other) testers highly welcome ;-)
> 
> Apparently massive overclocking like that makes your system die
> prematurely in a crash. So 3.10 it is, imo.
> -Daniel

It's like all overclocking I guess, we simply give the user an
opportunity to shoot themselves in the foot. Glad to hear it is actually
doing something though.

I agree that 3.10 sounds right; I'm thinking we'll want a module
parameter or something as well, to prevent spurious bug reports. What do
you think?

-- 
Ben Widawsky, Intel Open Source Technology Center