intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Better overclock support
@ 2013-04-03  2:45 Ben Widawsky
  2013-04-03  8:27 ` Daniel Vetter
  2013-04-09 13:42 ` [PATCH] drm/i915: Better overclock support Mika Kuoppala
  0 siblings, 2 replies; 7+ messages in thread
From: Ben Widawsky @ 2013-04-03  2:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Most importantly this will allow users to set overclock frequencies in
sysfs. Previously the max was limited by the RP0 max as opposed to the
overclock max. This is useful if one wants to either limit the max
overclock frequency, or set the minimum frequency to be in the overclock
range. It also fixes an issue where if one sets the max frequency to be
below the overclock max, they wouldn't be able to set back the proper
overclock max.

In addition I've added a couple of other bits:
Show the overclock freq. as max in sysfs
Print the overclock max in debugfs.
Print a warning if the user sets the min frequency to be in the
overclock range.

In this patch I've decided to store the hw_max when we read it from the
pcode at init. The reason I do this is the pcode reads can fail, and are
slow.

Reported-by: freezer?
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  3 +++
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_sysfs.c   | 12 ++++++++----
 drivers/gpu/drm/i915/intel_pm.c     |  3 ++-
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7df8351..f081bb3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1006,6 +1006,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 		max_freq = rp_state_cap & 0xff;
 		seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
 			   max_freq * GT_FREQUENCY_MULTIPLIER);
+
+		seq_printf(m, "Max overclocked frequency: %dMHz\n",
+			   dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER);
 	} else {
 		seq_printf(m, "no P-state info available\n");
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1657d873..9b53b39c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -648,6 +648,7 @@ struct intel_gen6_power_mgmt {
 	u8 cur_delay;
 	u8 min_delay;
 	u8 max_delay;
+	u8 hw_max;
 
 	struct delayed_work delayed_resume_work;
 
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index a3a3e22..5faf1a7 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -226,7 +226,7 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
 	int ret;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
-	ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
+	ret = dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER;
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
@@ -251,7 +251,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
-	hw_max = (rp_state_cap & 0xff);
+	hw_max = dev_priv->rps.hw_max;
 	hw_min = ((rp_state_cap & 0xff0000) >> 16);
 
 	if (val < hw_min || val > hw_max || val < dev_priv->rps.min_delay) {
@@ -290,7 +290,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
 	struct drm_device *dev = minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 val, rp_state_cap, hw_max, hw_min;
+	u32 val, rp_state_cap, hw_max, hw_min, non_oc_max;
 	ssize_t ret;
 
 	ret = kstrtou32(buf, 0, &val);
@@ -302,7 +302,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
-	hw_max = (rp_state_cap & 0xff);
+	hw_max = dev_priv->rps.hw_max;
+	non_oc_max = (rp_state_cap & 0xff);
 	hw_min = ((rp_state_cap & 0xff0000) >> 16);
 
 	if (val < hw_min || val > hw_max || val > dev_priv->rps.max_delay) {
@@ -310,6 +311,9 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 		return -EINVAL;
 	}
 
+	if (val > non_oc_max)
+		DRM_DEBUG("User selected overclocked frequency for min\n");
+
 	if (dev_priv->rps.cur_delay < val)
 		gen6_set_rps(dev_priv->dev, val);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ce3db2c..2edb743 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2555,7 +2555,7 @@ static void gen6_enable_rps(struct drm_device *dev)
 	gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
 
 	/* In units of 100MHz */
-	dev_priv->rps.max_delay = rp_state_cap & 0xff;
+	dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff;
 	dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16;
 	dev_priv->rps.cur_delay = 0;
 
@@ -2635,6 +2635,7 @@ static void gen6_enable_rps(struct drm_device *dev)
 			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.hw_max = pcu_mbox & 0xff;
 			dev_priv->rps.max_delay = pcu_mbox & 0xff;
 		}
 	} else {
-- 
1.8.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Better overclock support
  2013-04-03  2:45 [PATCH] drm/i915: Better overclock support Ben Widawsky
@ 2013-04-03  8:27 ` Daniel Vetter
  2013-04-05 21:29   ` [PATCH 1/2] [v2] " Ben Widawsky
  2013-04-09 13:42 ` [PATCH] drm/i915: Better overclock support Mika Kuoppala
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2013-04-03  8:27 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 6426 bytes --]

On Wed, Apr 3, 2013 at 4:45 AM, Ben Widawsky <ben@bwidawsk.net> wrote:

> Most importantly this will allow users to set overclock frequencies in
> sysfs. Previously the max was limited by the RP0 max as opposed to the
> overclock max. This is useful if one wants to either limit the max
> overclock frequency, or set the minimum frequency to be in the overclock
> range. It also fixes an issue where if one sets the max frequency to be
> below the overclock max, they wouldn't be able to set back the proper
> overclock max.
>
> In addition I've added a couple of other bits:
> Show the overclock freq. as max in sysfs
> Print the overclock max in debugfs.
> Print a warning if the user sets the min frequency to be in the
> overclock range.
>
> In this patch I've decided to store the hw_max when we read it from the
> pcode at init. The reason I do this is the pcode reads can fail, and are
> slow.
>
> Reported-by: freezer?
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>

I think we should go one step further and set the default max to rp0 and
not the overclocked max. That way users can set the BIOS to the highest it
allows without risking that their machines will hang right away.

Then we could switch the debug printk to max (we don't really care about
the min being too high) and mark all such bugs as wontfix ;-)
-Daniel


> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/i915_sysfs.c   | 12 ++++++++----
>  drivers/gpu/drm/i915/intel_pm.c     |  3 ++-
>  4 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7df8351..f081bb3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1006,6 +1006,9 @@ static int i915_cur_delayinfo(struct seq_file *m,
> void *unused)
>                 max_freq = rp_state_cap & 0xff;
>                 seq_printf(m, "Max non-overclocked (RP0) frequency:
> %dMHz\n",
>                            max_freq * GT_FREQUENCY_MULTIPLIER);
> +
> +               seq_printf(m, "Max overclocked frequency: %dMHz\n",
> +                          dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER);
>         } else {
>                 seq_printf(m, "no P-state info available\n");
>         }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 1657d873..9b53b39c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -648,6 +648,7 @@ struct intel_gen6_power_mgmt {
>         u8 cur_delay;
>         u8 min_delay;
>         u8 max_delay;
> +       u8 hw_max;
>
>         struct delayed_work delayed_resume_work;
>
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c
> b/drivers/gpu/drm/i915/i915_sysfs.c
> index a3a3e22..5faf1a7 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -226,7 +226,7 @@ static ssize_t gt_max_freq_mhz_show(struct device
> *kdev, struct device_attribute
>         int ret;
>
>         mutex_lock(&dev_priv->rps.hw_lock);
> -       ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
> +       ret = dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER;
>         mutex_unlock(&dev_priv->rps.hw_lock);
>
>         return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> @@ -251,7 +251,7 @@ static ssize_t gt_max_freq_mhz_store(struct device
> *kdev,
>         mutex_lock(&dev_priv->rps.hw_lock);
>
>         rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> -       hw_max = (rp_state_cap & 0xff);
> +       hw_max = dev_priv->rps.hw_max;
>         hw_min = ((rp_state_cap & 0xff0000) >> 16);
>
>         if (val < hw_min || val > hw_max || val < dev_priv->rps.min_delay)
> {
> @@ -290,7 +290,7 @@ static ssize_t gt_min_freq_mhz_store(struct device
> *kdev,
>         struct drm_minor *minor = container_of(kdev, struct drm_minor,
> kdev);
>         struct drm_device *dev = minor->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       u32 val, rp_state_cap, hw_max, hw_min;
> +       u32 val, rp_state_cap, hw_max, hw_min, non_oc_max;
>         ssize_t ret;
>
>         ret = kstrtou32(buf, 0, &val);
> @@ -302,7 +302,8 @@ static ssize_t gt_min_freq_mhz_store(struct device
> *kdev,
>         mutex_lock(&dev_priv->rps.hw_lock);
>
>         rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> -       hw_max = (rp_state_cap & 0xff);
> +       hw_max = dev_priv->rps.hw_max;
> +       non_oc_max = (rp_state_cap & 0xff);
>         hw_min = ((rp_state_cap & 0xff0000) >> 16);
>
>         if (val < hw_min || val > hw_max || val > dev_priv->rps.max_delay)
> {
> @@ -310,6 +311,9 @@ static ssize_t gt_min_freq_mhz_store(struct device
> *kdev,
>                 return -EINVAL;
>         }
>
> +       if (val > non_oc_max)
> +               DRM_DEBUG("User selected overclocked frequency for min\n");
> +
>         if (dev_priv->rps.cur_delay < val)
>                 gen6_set_rps(dev_priv->dev, val);
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index ce3db2c..2edb743 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2555,7 +2555,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>         gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
>
>         /* In units of 100MHz */
> -       dev_priv->rps.max_delay = rp_state_cap & 0xff;
> +       dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap &
> 0xff;
>         dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16;
>         dev_priv->rps.cur_delay = 0;
>
> @@ -2635,6 +2635,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>                         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.hw_max = pcu_mbox & 0xff;
>                         dev_priv->rps.max_delay = pcu_mbox & 0xff;
>                 }
>         } else {
> --
> 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

[-- Attachment #1.2: Type: text/html, Size: 7851 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] [v2] drm/i915: Better overclock support
  2013-04-03  8:27 ` Daniel Vetter
@ 2013-04-05 21:29   ` Ben Widawsky
  2013-04-05 21:29     ` [PATCH 2/2] drm/i915: Don't default to overclock max Ben Widawsky
  0 siblings, 1 reply; 7+ messages in thread
From: Ben Widawsky @ 2013-04-05 21:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Most importantly this will allow users to set overclock frequencies in
sysfs. Previously the max was limited by the RP0 max as opposed to the
overclock max. This is useful if one wants to either limit the max
overclock frequency, or set the minimum frequency to be in the overclock
range. It also fixes an issue where if one sets the max frequency to be
below the overclock max, they wouldn't be able to set back the proper
overclock max.

In addition I've added a couple of other bits:
Show the overclock freq. as max in sysfs
Print the overclock max in debugfs.
Print a warning if the user sets the min frequency to be in the
overclock range.

In this patch I've decided to store the hw_max when we read it from the
pcode at init. The reason I do this is the pcode reads can fail, and are
slow.

v2: Report when user requested overclocked max (Daniel)
Remove when user sets min to overclock range (Daniel)

Reported-by: freezer?
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  3 +++
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/i915_sysfs.c   | 13 +++++++++----
 drivers/gpu/drm/i915/intel_pm.c     |  3 ++-
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7df8351..f081bb3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1006,6 +1006,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
 		max_freq = rp_state_cap & 0xff;
 		seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
 			   max_freq * GT_FREQUENCY_MULTIPLIER);
+
+		seq_printf(m, "Max overclocked frequency: %dMHz\n",
+			   dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER);
 	} else {
 		seq_printf(m, "no P-state info available\n");
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1657d873..9b53b39c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -648,6 +648,7 @@ struct intel_gen6_power_mgmt {
 	u8 cur_delay;
 	u8 min_delay;
 	u8 max_delay;
+	u8 hw_max;
 
 	struct delayed_work delayed_resume_work;
 
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index a3a3e22..fa4b4e8 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -226,7 +226,7 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
 	int ret;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
-	ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
+	ret = dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER;
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
@@ -239,7 +239,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
 	struct drm_device *dev = minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 val, rp_state_cap, hw_max, hw_min;
+	u32 val, rp_state_cap, hw_max, hw_min, non_oc_max;
 	ssize_t ret;
 
 	ret = kstrtou32(buf, 0, &val);
@@ -251,7 +251,8 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
-	hw_max = (rp_state_cap & 0xff);
+	hw_max = dev_priv->rps.hw_max;
+	non_oc_max = (rp_state_cap & 0xff);
 	hw_min = ((rp_state_cap & 0xff0000) >> 16);
 
 	if (val < hw_min || val > hw_max || val < dev_priv->rps.min_delay) {
@@ -259,6 +260,10 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
 		return -EINVAL;
 	}
 
+	if (val > non_oc_max)
+		DRM_DEBUG("User requested overclocking to %d\n",
+			  val * GT_FREQUENCY_MULTIPLIER);
+
 	if (dev_priv->rps.cur_delay > val)
 		gen6_set_rps(dev_priv->dev, val);
 
@@ -302,7 +307,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 	mutex_lock(&dev_priv->rps.hw_lock);
 
 	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
-	hw_max = (rp_state_cap & 0xff);
+	hw_max = dev_priv->rps.hw_max;
 	hw_min = ((rp_state_cap & 0xff0000) >> 16);
 
 	if (val < hw_min || val > hw_max || val > dev_priv->rps.max_delay) {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ce3db2c..2edb743 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2555,7 +2555,7 @@ static void gen6_enable_rps(struct drm_device *dev)
 	gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
 
 	/* In units of 100MHz */
-	dev_priv->rps.max_delay = rp_state_cap & 0xff;
+	dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff;
 	dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16;
 	dev_priv->rps.cur_delay = 0;
 
@@ -2635,6 +2635,7 @@ static void gen6_enable_rps(struct drm_device *dev)
 			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.hw_max = pcu_mbox & 0xff;
 			dev_priv->rps.max_delay = pcu_mbox & 0xff;
 		}
 	} else {
-- 
1.8.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drm/i915: Don't default to overclock max
  2013-04-05 21:29   ` [PATCH 1/2] [v2] " Ben Widawsky
@ 2013-04-05 21:29     ` Ben Widawsky
  2013-04-09 13:44       ` Mika Kuoppala
  0 siblings, 1 reply; 7+ messages in thread
From: Ben Widawsky @ 2013-04-05 21:29 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Requested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_pm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2edb743..c81921b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2632,11 +2632,10 @@ static void gen6_enable_rps(struct drm_device *dev)
 		pcu_mbox = 0;
 		ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
 		if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */
-			DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n",
+			DRM_DEBUG_DRIVER("Overclocking supported. Max: %dMHz, Overclock max: %dMHz\n",
 					 (dev_priv->rps.max_delay & 0xff) * 50,
 					 (pcu_mbox & 0xff) * 50);
 			dev_priv->rps.hw_max = pcu_mbox & 0xff;
-			dev_priv->rps.max_delay = pcu_mbox & 0xff;
 		}
 	} else {
 		DRM_DEBUG_DRIVER("Failed to set the min frequency\n");
-- 
1.8.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Better overclock support
  2013-04-03  2:45 [PATCH] drm/i915: Better overclock support Ben Widawsky
  2013-04-03  8:27 ` Daniel Vetter
@ 2013-04-09 13:42 ` Mika Kuoppala
  1 sibling, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2013-04-09 13:42 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Ben Widawsky <ben@bwidawsk.net> writes:

> Most importantly this will allow users to set overclock frequencies in
> sysfs. Previously the max was limited by the RP0 max as opposed to the
> overclock max. This is useful if one wants to either limit the max
> overclock frequency, or set the minimum frequency to be in the overclock
> range. It also fixes an issue where if one sets the max frequency to be
> below the overclock max, they wouldn't be able to set back the proper
> overclock max.
>
> In addition I've added a couple of other bits:
> Show the overclock freq. as max in sysfs
> Print the overclock max in debugfs.
> Print a warning if the user sets the min frequency to be in the
> overclock range.
>
> In this patch I've decided to store the hw_max when we read it from the
> pcode at init. The reason I do this is the pcode reads can fail, and are
> slow.
>
> Reported-by: freezer?
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h     |  1 +
>  drivers/gpu/drm/i915/i915_sysfs.c   | 12 ++++++++----
>  drivers/gpu/drm/i915/intel_pm.c     |  3 ++-
>  4 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7df8351..f081bb3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1006,6 +1006,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
>  		max_freq = rp_state_cap & 0xff;
>  		seq_printf(m, "Max non-overclocked (RP0) frequency: %dMHz\n",
>  			   max_freq * GT_FREQUENCY_MULTIPLIER);
> +
> +		seq_printf(m, "Max overclocked frequency: %dMHz\n",
> +			   dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER);
>  	} else {
>  		seq_printf(m, "no P-state info available\n");
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1657d873..9b53b39c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -648,6 +648,7 @@ struct intel_gen6_power_mgmt {
>  	u8 cur_delay;
>  	u8 min_delay;
>  	u8 max_delay;
> +	u8 hw_max;
>  
>  	struct delayed_work delayed_resume_work;
>  
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index a3a3e22..5faf1a7 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -226,7 +226,7 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
>  	int ret;
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
> -	ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
> +	ret = dev_priv->rps.hw_max * GT_FREQUENCY_MULTIPLIER;
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
>  	return snprintf(buf, PAGE_SIZE, "%d\n", ret);
> @@ -251,7 +251,7 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev,
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> -	hw_max = (rp_state_cap & 0xff);
> +	hw_max = dev_priv->rps.hw_max;
>  	hw_min = ((rp_state_cap & 0xff0000) >> 16);
>  
>  	if (val < hw_min || val > hw_max || val < dev_priv->rps.min_delay) {
> @@ -290,7 +290,7 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  	struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev);
>  	struct drm_device *dev = minor->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 val, rp_state_cap, hw_max, hw_min;
> +	u32 val, rp_state_cap, hw_max, hw_min, non_oc_max;
>  	ssize_t ret;
>  
>  	ret = kstrtou32(buf, 0, &val);
> @@ -302,7 +302,8 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
>  	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> -	hw_max = (rp_state_cap & 0xff);
> +	hw_max = dev_priv->rps.hw_max;
> +	non_oc_max = (rp_state_cap & 0xff);
>  	hw_min = ((rp_state_cap & 0xff0000) >> 16);
>  
>  	if (val < hw_min || val > hw_max || val > dev_priv->rps.max_delay) {
> @@ -310,6 +311,9 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>  		return -EINVAL;
>  	}
>  
> +	if (val > non_oc_max)
> +		DRM_DEBUG("User selected overclocked frequency for min\n");
> +
>  	if (dev_priv->rps.cur_delay < val)
>  		gen6_set_rps(dev_priv->dev, val);
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ce3db2c..2edb743 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2555,7 +2555,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>  	gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
>  
>  	/* In units of 100MHz */

Not a problem with this patch but the above comment should be
fixed as the freq is in units of 50Mhz right?

> -	dev_priv->rps.max_delay = rp_state_cap & 0xff;
> +	dev_priv->rps.hw_max = dev_priv->rps.max_delay = rp_state_cap & 0xff;
>  	dev_priv->rps.min_delay = (rp_state_cap & 0xff0000) >> 16;
>  	dev_priv->rps.cur_delay = 0;
>  
> @@ -2635,6 +2635,7 @@ static void gen6_enable_rps(struct drm_device *dev)
>  			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.hw_max = pcu_mbox & 0xff;
>  			dev_priv->rps.max_delay = pcu_mbox & 0xff;
>  		}
>  	} else {
> -- 
> 1.8.2

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/i915: Don't default to overclock max
  2013-04-05 21:29     ` [PATCH 2/2] drm/i915: Don't default to overclock max Ben Widawsky
@ 2013-04-09 13:44       ` Mika Kuoppala
  2013-04-09 16:58         ` Daniel Vetter
  0 siblings, 1 reply; 7+ messages in thread
From: Mika Kuoppala @ 2013-04-09 13:44 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

Ben Widawsky <ben@bwidawsk.net> writes:

> Requested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2edb743..c81921b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2632,11 +2632,10 @@ static void gen6_enable_rps(struct drm_device *dev)
>  		pcu_mbox = 0;
>  		ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
>  		if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */
> -			DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n",
> +			DRM_DEBUG_DRIVER("Overclocking supported. Max: %dMHz, Overclock max: %dMHz\n",
>  					 (dev_priv->rps.max_delay & 0xff) * 50,
>  					 (pcu_mbox & 0xff) * 50);
Minor nitpick:                                              
Use GT_FREQUENCY_MULTIPLIER instead of 50

>  			dev_priv->rps.hw_max = pcu_mbox & 0xff;
> -			dev_priv->rps.max_delay = pcu_mbox & 0xff;
>  		}
>  	} else {
>  		DRM_DEBUG_DRIVER("Failed to set the min frequency\n");
> -- 
> 1.8.2

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drm/i915: Don't default to overclock max
  2013-04-09 13:44       ` Mika Kuoppala
@ 2013-04-09 16:58         ` Daniel Vetter
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2013-04-09 16:58 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: Ben Widawsky, intel-gfx

On Tue, Apr 09, 2013 at 04:44:28PM +0300, Mika Kuoppala wrote:
> Ben Widawsky <ben@bwidawsk.net> writes:
> 
> > Requested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 2edb743..c81921b 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2632,11 +2632,10 @@ static void gen6_enable_rps(struct drm_device *dev)
> >  		pcu_mbox = 0;
> >  		ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
> >  		if (!ret && (pcu_mbox & (1<<31))) { /* OC supported */
> > -			DRM_DEBUG_DRIVER("overclocking supported, adjusting frequency max from %dMHz to %dMHz\n",
> > +			DRM_DEBUG_DRIVER("Overclocking supported. Max: %dMHz, Overclock max: %dMHz\n",
> >  					 (dev_priv->rps.max_delay & 0xff) * 50,
> >  					 (pcu_mbox & 0xff) * 50);
> Minor nitpick:                                              
> Use GT_FREQUENCY_MULTIPLIER instead of 50

I've ignored that one for now ...

> 
> >  			dev_priv->rps.hw_max = pcu_mbox & 0xff;
> > -			dev_priv->rps.max_delay = pcu_mbox & 0xff;
> >  		}
> >  	} else {
> >  		DRM_DEBUG_DRIVER("Failed to set the min frequency\n");
> > -- 
> > 1.8.2
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Mika clarified that he indeed reviewed v2 of patch 1 but replied to the
wrong one. Both merged, thanks for the patches.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-04-09 16:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-03  2:45 [PATCH] drm/i915: Better overclock support Ben Widawsky
2013-04-03  8:27 ` Daniel Vetter
2013-04-05 21:29   ` [PATCH 1/2] [v2] " Ben Widawsky
2013-04-05 21:29     ` [PATCH 2/2] drm/i915: Don't default to overclock max Ben Widawsky
2013-04-09 13:44       ` Mika Kuoppala
2013-04-09 16:58         ` Daniel Vetter
2013-04-09 13:42 ` [PATCH] drm/i915: Better overclock support Mika Kuoppala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).