Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-24 Thread Daniel Vetter 
On Sun, Mar 24, 2013 at 12:56 AM, Ben Widawsky b...@bwidawsk.net wrote:
 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?

I think not having a module option is ok, I don't expect that the
enthusiast boards required to adjust the limits are that common. So it
shouldn't affect more than just a few users, and those it does affect
should know what their doing. Module options after all don't prevent
them from setting unstable rc6 options, either ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-23 Thread Daniel Vetter 
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 b...@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 jbar...@virtuousgeek.org
 Signed-off-by: Ben Widawsky b...@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 ;-)

Cheers, Daniel

 ---
  drivers/gpu/drm/i915/intel_pm.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index c30e89a..86729b1 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
   if (!ret) {
   pcu_mbox = 0;
   ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
 pcu_mbox);
 - if (ret  pcu_mbox  (131)) { /* OC supported */
 + if (!ret  (pcu_mbox  (131))) { /* OC supported */
 + DRM_DEBUG_DRIVER(overclocking supported, adjusting 
 frequency max from %dMHz to %dMHz\n,
 +  ((dev_priv-rps.max_delay  0xff) * 
 50),
 +  ((pcu_mbox  0xff) * 50));
   dev_priv-rps.max_delay = pcu_mbox  0xff;
 - DRM_DEBUG_DRIVER(overclocking supported, adjusting 
 frequency max to %dMHz\n, pcu_mbox * 50);
   }
   } else {
   DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
 -- 
 1.8.2
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-23 Thread Daniel Vetter 
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 b...@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 jbar...@virtuousgeek.org
  Signed-off-by: Ben Widawsky b...@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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-23 Thread Ben Widawsky 
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 b...@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 jbar...@virtuousgeek.org
   Signed-off-by: Ben Widawsky b...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-20 Thread Chris Wilson 
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 b...@bwidawsk.net
 Date:   Wed Sep 26 10:34:00 2012 -0700
 
 drm/i915: Extract PCU communication
 
 I added some parentheses for clarity,

(Clarity)! ((They) ((were) (not)) (even) (connected) (to) ((the) (mistake))).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-20 Thread Jesse Barnes 
On Tue, 19 Mar 2013 20:19:56 -0700
Ben Widawsky b...@bwidawsk.net 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 b...@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 jbar...@virtuousgeek.org
 Signed-off-by: Ben Widawsky b...@bwidawsk.net
 ---
  drivers/gpu/drm/i915/intel_pm.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index c30e89a..86729b1 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
   if (!ret) {
   pcu_mbox = 0;
   ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
 pcu_mbox);
 - if (ret  pcu_mbox  (131)) { /* OC supported */
 + if (!ret  (pcu_mbox  (131))) { /* OC supported */
 + DRM_DEBUG_DRIVER(overclocking supported, adjusting 
 frequency max from %dMHz to %dMHz\n,
 +  ((dev_priv-rps.max_delay  0xff) * 
 50),
 +  ((pcu_mbox  0xff) * 50));
   dev_priv-rps.max_delay = pcu_mbox  0xff;
 - DRM_DEBUG_DRIVER(overclocking supported, adjusting 
 frequency max to %dMHz\n, pcu_mbox * 50);
   }
   } else {
   DRM_DEBUG_DRIVER(Failed to set the min frequency\n);

Yeah looks ok.  I don't think I've seen a system where this flag gets
set, but according to the docs this is the right thing to do.

Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-20 Thread Ben Widawsky 
On Wed, Mar 20, 2013 at 09:21:47AM -0700, Jesse Barnes wrote:
 On Tue, 19 Mar 2013 20:19:56 -0700
 Ben Widawsky b...@bwidawsk.net 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 b...@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 jbar...@virtuousgeek.org
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
  ---
   drivers/gpu/drm/i915/intel_pm.c | 6 --
   1 file changed, 4 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index c30e89a..86729b1 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
  if (!ret) {
  pcu_mbox = 0;
  ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
  pcu_mbox);
  -   if (ret  pcu_mbox  (131)) { /* OC supported */
  +   if (!ret  (pcu_mbox  (131))) { /* OC supported */
  +   DRM_DEBUG_DRIVER(overclocking supported, adjusting 
  frequency max from %dMHz to %dMHz\n,
  +((dev_priv-rps.max_delay  0xff) * 
  50),
  +((pcu_mbox  0xff) * 50));
  dev_priv-rps.max_delay = pcu_mbox  0xff;
  -   DRM_DEBUG_DRIVER(overclocking supported, adjusting 
  frequency max to %dMHz\n, pcu_mbox * 50);
  }
  } else {
  DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
 
 Yeah looks ok.  I don't think I've seen a system where this flag gets
 set, but according to the docs this is the right thing to do.
 
 Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
 
 -- 
 Jesse Barnes, Intel Open Source Technology Center

So from what I gather, we shouldn't bother with -fixes, or stable,
correct?

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-20 Thread Jesse Barnes 
On Wed, 20 Mar 2013 09:33:28 -0700
Ben Widawsky b...@bwidawsk.net wrote:

 On Wed, Mar 20, 2013 at 09:21:47AM -0700, Jesse Barnes wrote:
  On Tue, 19 Mar 2013 20:19:56 -0700
  Ben Widawsky b...@bwidawsk.net 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 b...@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 jbar...@virtuousgeek.org
   Signed-off-by: Ben Widawsky b...@bwidawsk.net
   ---
drivers/gpu/drm/i915/intel_pm.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/intel_pm.c 
   b/drivers/gpu/drm/i915/intel_pm.c
   index c30e89a..86729b1 100644
   --- a/drivers/gpu/drm/i915/intel_pm.c
   +++ b/drivers/gpu/drm/i915/intel_pm.c
   @@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
 if (!ret) {
 pcu_mbox = 0;
 ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
   pcu_mbox);
   - if (ret  pcu_mbox  (131)) { /* OC supported */
   + if (!ret  (pcu_mbox  (131))) { /* OC supported */
   + DRM_DEBUG_DRIVER(overclocking supported, adjusting 
   frequency max from %dMHz to %dMHz\n,
   +  ((dev_priv-rps.max_delay  0xff) * 
   50),
   +  ((pcu_mbox  0xff) * 50));
 dev_priv-rps.max_delay = pcu_mbox  0xff;
   - DRM_DEBUG_DRIVER(overclocking supported, adjusting 
   frequency max to %dMHz\n, pcu_mbox * 50);
 }
 } else {
 DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
  
  Yeah looks ok.  I don't think I've seen a system where this flag gets
  set, but according to the docs this is the right thing to do.
  
  Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
  
  -- 
  Jesse Barnes, Intel Open Source Technology Center
 
 So from what I gather, we shouldn't bother with -fixes, or stable,
 correct?

Correct, unless someone complains about their gfx suddenly being a bit
slower, it's not worth the risk.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-20 Thread Daniel Vetter 
On Wed, Mar 20, 2013 at 09:57:53AM -0700, Jesse Barnes wrote:
 On Wed, 20 Mar 2013 09:33:28 -0700
 Ben Widawsky b...@bwidawsk.net wrote:
 
  On Wed, Mar 20, 2013 at 09:21:47AM -0700, Jesse Barnes wrote:
   On Tue, 19 Mar 2013 20:19:56 -0700
   Ben Widawsky b...@bwidawsk.net 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 b...@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 jbar...@virtuousgeek.org
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/intel_pm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c 
b/drivers/gpu/drm/i915/intel_pm.c
index c30e89a..86729b1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device 
*dev)
if (!ret) {
pcu_mbox = 0;
ret = sandybridge_pcode_read(dev_priv, 
GEN6_READ_OC_PARAMS, pcu_mbox);
-   if (ret  pcu_mbox  (131)) { /* OC supported */
+   if (!ret  (pcu_mbox  (131))) { /* OC supported */
+   DRM_DEBUG_DRIVER(overclocking supported, 
adjusting frequency max from %dMHz to %dMHz\n,
+((dev_priv-rps.max_delay  
0xff) * 50),
+((pcu_mbox  0xff) * 50));
dev_priv-rps.max_delay = pcu_mbox  0xff;
-   DRM_DEBUG_DRIVER(overclocking supported, 
adjusting frequency max to %dMHz\n, pcu_mbox * 50);
}
} else {
DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
   
   Yeah looks ok.  I don't think I've seen a system where this flag gets
   set, but according to the docs this is the right thing to do.
   
   Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
   
   -- 
   Jesse Barnes, Intel Open Source Technology Center
  
  So from what I gather, we shouldn't bother with -fixes, or stable,
  correct?
 
 Correct, unless someone complains about their gfx suddenly being a bit
 slower, it's not worth the risk.

Concurred and so merged to dinq, thanks for the patch. I've also applied
Chris' bikeshed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Correct sandybrige overclocking

2013-03-19 Thread Ben Widawsky 
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 b...@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 jbar...@virtuousgeek.org
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/intel_pm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c30e89a..86729b1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2631,9 +2631,11 @@ static void gen6_enable_rps(struct drm_device *dev)
if (!ret) {
pcu_mbox = 0;
ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, 
pcu_mbox);
-   if (ret  pcu_mbox  (131)) { /* OC supported */
+   if (!ret  (pcu_mbox  (131))) { /* OC supported */
+   DRM_DEBUG_DRIVER(overclocking supported, adjusting 
frequency max from %dMHz to %dMHz\n,
+((dev_priv-rps.max_delay  0xff) * 
50),
+((pcu_mbox  0xff) * 50));
dev_priv-rps.max_delay = pcu_mbox  0xff;
-   DRM_DEBUG_DRIVER(overclocking supported, adjusting 
frequency max to %dMHz\n, pcu_mbox * 50);
}
} else {
DRM_DEBUG_DRIVER(Failed to set the min frequency\n);
-- 
1.8.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx