* [PATCH 0/6] HSW PM - RC6 clean up and fixes.
@ 2013-02-25 23:13 Rodrigo Vivi
2013-02-25 23:13 ` [PATCH 1/6] drm/i915: creating Haswell rc6 function Rodrigo Vivi
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 23:13 UTC (permalink / raw)
To: intel-gfx
Since the patch I sent last week was big and difficult for reviews I split out
it in these 6 patches, where I create a hsw_enable_rps function to receive the
sequence documented at HSW PM programming guide.
Since it has many differences when compared to SNB enable I decided to split
enable functions.
Unfortunatelly I couldn't finish all tests that I'd like to do today, but I'm
sending patch here anyway to speed up reviews and maybe get suggestions and
fixes. Thanks in advance.
Rodrigo Vivi (6):
drm/i915: creating Haswell rc6 function
drm/i915: HSW PM Frequency bits fix
drm/i915: HSW PM Cleaning - Removing unecessary register/bits set.
drm/i915: HSW PM - removing pcode read/write.
drm/i915: Use all PM deferred events for HSW.
drm/i915: More HSW PM clean up.
drivers/gpu/drm/i915/i915_reg.h | 8 +++
drivers/gpu/drm/i915/intel_pm.c | 116 +++++++++++++++++++++++++++++++++++-----
2 files changed, 111 insertions(+), 13 deletions(-)
--
1.8.1.2
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/6] drm/i915: creating Haswell rc6 function
2013-02-25 23:13 [PATCH 0/6] HSW PM - RC6 clean up and fixes Rodrigo Vivi
@ 2013-02-25 23:13 ` Rodrigo Vivi
2013-02-26 20:30 ` Paulo Zanoni
2013-03-24 22:07 ` Ben Widawsky
2013-02-25 23:13 ` [PATCH 2/6] drm/i915: HSW PM Frequency bits fix Rodrigo Vivi
` (4 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 23:13 UTC (permalink / raw)
To: intel-gfx
Power management, in special RC6 enabling, differs across platforms.
This patch just split out enabling function for HSW.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
drivers/gpu/drm/i915/intel_pm.c | 158 ++++++++++++++++++++++++++++++++++++----
1 file changed, 145 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 61fee7f..e8656fd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2581,15 +2581,10 @@ static void gen6_enable_rps(struct drm_device *dev)
rc6_mode = intel_enable_rc6(dev_priv->dev);
if (rc6_mode & INTEL_RC6_ENABLE)
rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
-
- /* We don't use those on Haswell */
- if (!IS_HASWELL(dev)) {
- if (rc6_mode & INTEL_RC6p_ENABLE)
- rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
-
- if (rc6_mode & INTEL_RC6pp_ENABLE)
- rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
- }
+ if (rc6_mode & INTEL_RC6p_ENABLE)
+ rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
+ if (rc6_mode & INTEL_RC6pp_ENABLE)
+ rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
(rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
@@ -2625,7 +2620,141 @@ static void gen6_enable_rps(struct drm_device *dev)
GEN6_RP_MEDIA_IS_GFX |
GEN6_RP_ENABLE |
GEN6_RP_UP_BUSY_AVG |
- (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT));
+ GEN6_RP_DOWN_IDLE_CONT);
+
+ ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0);
+ if (!ret) {
+ pcu_mbox = 0;
+ ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
+ if (ret && pcu_mbox & (1<<31)) { /* OC supported */
+ 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");
+ }
+
+ gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
+
+ /* requires MSI enabled */
+ I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS);
+ spin_lock_irq(&dev_priv->rps.lock);
+ WARN_ON(dev_priv->rps.pm_iir != 0);
+ I915_WRITE(GEN6_PMIMR, 0);
+ spin_unlock_irq(&dev_priv->rps.lock);
+ /* enable all PM interrupts */
+ I915_WRITE(GEN6_PMINTRMSK, 0);
+
+ rc6vids = 0;
+ ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
+ if (IS_GEN6(dev) && ret) {
+ DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n");
+ } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) {
+ DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n",
+ GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
+ rc6vids &= 0xffff00;
+ rc6vids |= GEN6_ENCODE_RC6_VID(450);
+ ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
+ if (ret)
+ DRM_ERROR("Couldn't fix incorrect rc6 voltage\n");
+ }
+
+ gen6_gt_force_wake_put(dev_priv);
+}
+
+static void hsw_enable_rps(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_ring_buffer *ring;
+ u32 rp_state_cap;
+ u32 gt_perf_status;
+ u32 rc6vids, pcu_mbox, rc6_mask = 0;
+ u32 gtfifodbg;
+ int rc6_mode;
+ int i, ret;
+
+ WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
+
+ /* Here begins a magic sequence of register writes to enable
+ * auto-downclocking.
+ *
+ * Perhaps there might be some value in exposing these to
+ * userspace...
+ */
+ I915_WRITE(GEN6_RC_STATE, 0);
+
+ /* Clear the DBG now so we don't confuse earlier errors */
+ if ((gtfifodbg = I915_READ(GTFIFODBG))) {
+ DRM_ERROR("GT fifo had a previous error %x\n", gtfifodbg);
+ I915_WRITE(GTFIFODBG, gtfifodbg);
+ }
+
+ gen6_gt_force_wake_get(dev_priv);
+
+ rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+ 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.min_delay = (rp_state_cap & 0xff0000) >> 16;
+ dev_priv->rps.cur_delay = 0;
+
+ /* disable the counters and set deterministic thresholds */
+ I915_WRITE(GEN6_RC_CONTROL, 0);
+
+ I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16);
+ I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30);
+ I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30);
+ I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000);
+ I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25);
+
+ for_each_ring(ring, dev_priv, i)
+ I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
+
+ I915_WRITE(GEN6_RC_SLEEP, 0);
+ I915_WRITE(GEN6_RC1e_THRESHOLD, 1000);
+ I915_WRITE(GEN6_RC6_THRESHOLD, 50000);
+ I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
+ I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
+
+ /* Check if we are enabling RC6 */
+ rc6_mode = intel_enable_rc6(dev_priv->dev);
+ if (rc6_mode & INTEL_RC6_ENABLE)
+ rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
+
+ DRM_INFO("Enabling RC6 states: RC6 %s\n",
+ (rc6_mask & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
+
+ I915_WRITE(GEN6_RC_CONTROL,
+ rc6_mask |
+ GEN6_RC_CTL_EI_MODE(1) |
+ GEN6_RC_CTL_HW_ENABLE);
+
+ I915_WRITE(GEN6_RPNSWREQ,
+ GEN6_FREQUENCY(10) |
+ GEN6_OFFSET(0) |
+ GEN6_AGGRESSIVE_TURBO);
+ I915_WRITE(GEN6_RC_VIDEO_FREQ,
+ GEN6_FREQUENCY(12));
+
+ I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000);
+ I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
+ dev_priv->rps.max_delay << 24 |
+ dev_priv->rps.min_delay << 16);
+
+ I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
+ I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000);
+ I915_WRITE(GEN6_RP_UP_EI, 66000);
+ I915_WRITE(GEN6_RP_DOWN_EI, 350000);
+
+ I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
+ I915_WRITE(GEN6_RP_CONTROL,
+ GEN6_RP_MEDIA_TURBO |
+ GEN6_RP_MEDIA_HW_NORMAL_MODE |
+ GEN6_RP_MEDIA_IS_GFX |
+ GEN6_RP_ENABLE |
+ GEN6_RP_UP_BUSY_AVG |
+ GEN7_RP_DOWN_IDLE_AVG);
ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0);
if (!ret) {
@@ -3448,7 +3577,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
}
}
-static void intel_gen6_powersave_work(struct work_struct *work)
+static void intel_powersave_work(struct work_struct *work)
{
struct drm_i915_private *dev_priv =
container_of(work, struct drm_i915_private,
@@ -3456,7 +3585,10 @@ static void intel_gen6_powersave_work(struct work_struct *work)
struct drm_device *dev = dev_priv->dev;
mutex_lock(&dev_priv->rps.hw_lock);
- gen6_enable_rps(dev);
+ if (IS_HASWELL(dev))
+ hsw_enable_rps(dev);
+ else
+ gen6_enable_rps(dev);
gen6_update_ring_freq(dev);
mutex_unlock(&dev_priv->rps.hw_lock);
}
@@ -4459,7 +4591,7 @@ void intel_gt_init(struct drm_device *dev)
dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
}
INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
- intel_gen6_powersave_work);
+ intel_powersave_work);
}
int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/6] drm/i915: HSW PM Frequency bits fix
2013-02-25 23:13 [PATCH 0/6] HSW PM - RC6 clean up and fixes Rodrigo Vivi
2013-02-25 23:13 ` [PATCH 1/6] drm/i915: creating Haswell rc6 function Rodrigo Vivi
@ 2013-02-25 23:13 ` Rodrigo Vivi
2013-02-26 20:57 ` Paulo Zanoni
2013-02-25 23:13 ` [PATCH 3/6] drm/i915: HSW PM Cleaning - Removing unecessary register/bits set Rodrigo Vivi
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 23:13 UTC (permalink / raw)
To: intel-gfx
According to HSW PM programming guide, frequency bits starts at
24 instead of 25
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 527b664..ef47cee 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4184,6 +4184,7 @@
#define GEN6_RPNSWREQ 0xA008
#define GEN6_TURBO_DISABLE (1<<31)
#define GEN6_FREQUENCY(x) ((x)<<25)
+#define HSW_FREQUENCY(x) ((x)<<24)
#define GEN6_OFFSET(x) ((x)<<19)
#define GEN6_AGGRESSIVE_TURBO (0<<15)
#define GEN6_RC_VIDEO_FREQ 0xA00C
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e8656fd..fbe9779 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2731,11 +2731,11 @@ static void hsw_enable_rps(struct drm_device *dev)
GEN6_RC_CTL_HW_ENABLE);
I915_WRITE(GEN6_RPNSWREQ,
- GEN6_FREQUENCY(10) |
+ HSW_FREQUENCY(10) |
GEN6_OFFSET(0) |
GEN6_AGGRESSIVE_TURBO);
I915_WRITE(GEN6_RC_VIDEO_FREQ,
- GEN6_FREQUENCY(12));
+ HSW_FREQUENCY(12));
I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000);
I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/6] drm/i915: HSW PM Cleaning - Removing unecessary register/bits set.
2013-02-25 23:13 [PATCH 0/6] HSW PM - RC6 clean up and fixes Rodrigo Vivi
2013-02-25 23:13 ` [PATCH 1/6] drm/i915: creating Haswell rc6 function Rodrigo Vivi
2013-02-25 23:13 ` [PATCH 2/6] drm/i915: HSW PM Frequency bits fix Rodrigo Vivi
@ 2013-02-25 23:13 ` Rodrigo Vivi
2013-02-26 22:26 ` Paulo Zanoni
2013-02-25 23:13 ` [PATCH 4/6] drm/i915: HSW PM - removing pcode read/write Rodrigo Vivi
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 23:13 UTC (permalink / raw)
To: intel-gfx
According to HSW PM Programming guide it is not needed touch this registers
or setting these values anymore.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
drivers/gpu/drm/i915/intel_pm.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fbe9779..322c562 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2702,9 +2702,7 @@ static void hsw_enable_rps(struct drm_device *dev)
/* disable the counters and set deterministic thresholds */
I915_WRITE(GEN6_RC_CONTROL, 0);
- I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16);
- I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30);
- I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30);
+ I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000);
I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25);
@@ -2712,10 +2710,7 @@ static void hsw_enable_rps(struct drm_device *dev)
I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
I915_WRITE(GEN6_RC_SLEEP, 0);
- I915_WRITE(GEN6_RC1e_THRESHOLD, 1000);
I915_WRITE(GEN6_RC6_THRESHOLD, 50000);
- I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
- I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
/* Check if we are enabling RC6 */
rc6_mode = intel_enable_rc6(dev_priv->dev);
@@ -2731,9 +2726,7 @@ static void hsw_enable_rps(struct drm_device *dev)
GEN6_RC_CTL_HW_ENABLE);
I915_WRITE(GEN6_RPNSWREQ,
- HSW_FREQUENCY(10) |
- GEN6_OFFSET(0) |
- GEN6_AGGRESSIVE_TURBO);
+ HSW_FREQUENCY(10));
I915_WRITE(GEN6_RC_VIDEO_FREQ,
HSW_FREQUENCY(12));
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] drm/i915: HSW PM - removing pcode read/write.
2013-02-25 23:13 [PATCH 0/6] HSW PM - RC6 clean up and fixes Rodrigo Vivi
` (2 preceding siblings ...)
2013-02-25 23:13 ` [PATCH 3/6] drm/i915: HSW PM Cleaning - Removing unecessary register/bits set Rodrigo Vivi
@ 2013-02-25 23:13 ` Rodrigo Vivi
2013-02-26 22:35 ` Paulo Zanoni
2013-02-25 23:13 ` [PATCH 5/6] drm/i915: Use all PM deferred events for HSW Rodrigo Vivi
2013-02-25 23:13 ` [PATCH 6/6] drm/i915: More HSW PM clean up Rodrigo Vivi
5 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 23:13 UTC (permalink / raw)
To: intel-gfx
Yet according to pm spec pcode read/write operations
aren't necessary for HSW.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
drivers/gpu/drm/i915/intel_pm.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 322c562..761be6f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2749,18 +2749,6 @@ static void hsw_enable_rps(struct drm_device *dev)
GEN6_RP_UP_BUSY_AVG |
GEN7_RP_DOWN_IDLE_AVG);
- ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0);
- if (!ret) {
- pcu_mbox = 0;
- ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
- if (ret && pcu_mbox & (1<<31)) { /* OC supported */
- 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");
- }
-
gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
/* requires MSI enabled */
@@ -2772,20 +2760,6 @@ static void hsw_enable_rps(struct drm_device *dev)
/* enable all PM interrupts */
I915_WRITE(GEN6_PMINTRMSK, 0);
- rc6vids = 0;
- ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
- if (IS_GEN6(dev) && ret) {
- DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n");
- } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) {
- DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n",
- GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
- rc6vids &= 0xffff00;
- rc6vids |= GEN6_ENCODE_RC6_VID(450);
- ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
- if (ret)
- DRM_ERROR("Couldn't fix incorrect rc6 voltage\n");
- }
-
gen6_gt_force_wake_put(dev_priv);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/6] drm/i915: Use all PM deferred events for HSW.
2013-02-25 23:13 [PATCH 0/6] HSW PM - RC6 clean up and fixes Rodrigo Vivi
` (3 preceding siblings ...)
2013-02-25 23:13 ` [PATCH 4/6] drm/i915: HSW PM - removing pcode read/write Rodrigo Vivi
@ 2013-02-25 23:13 ` Rodrigo Vivi
2013-02-26 9:38 ` Chris Wilson
2013-02-25 23:13 ` [PATCH 6/6] drm/i915: More HSW PM clean up Rodrigo Vivi
5 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 23:13 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
drivers/gpu/drm/i915/intel_pm.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ef47cee..7977b65 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4258,6 +4258,13 @@
#define GEN6_PM_DEFERRED_EVENTS (GEN6_PM_RP_UP_THRESHOLD | \
GEN6_PM_RP_DOWN_THRESHOLD | \
GEN6_PM_RP_DOWN_TIMEOUT)
+#define GEN7_PM_DEFERRED_EVENTS (GEN6_PM_MBOX_EVENT | \
+ GEN6_PM_THERMAL_EVENT | \
+ GEN6_PM_RP_DOWN_TIMEOUT | \
+ GEN6_PM_RP_UP_THRESHOLD | \
+ GEN6_PM_RP_DOWN_THRESHOLD | \
+ GEN6_PM_RP_UP_EI_EXPIRED | \
+ GEN6_PM_RP_DOWN_EI_EXPIRED)
#define GEN6_GT_GFX_RC6_LOCKED 0x138104
#define GEN6_GT_GFX_RC6 0x138108
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 761be6f..d43e011 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2752,7 +2752,7 @@ static void hsw_enable_rps(struct drm_device *dev)
gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
/* requires MSI enabled */
- I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS);
+ I915_WRITE(GEN6_PMIER, GEN7_PM_DEFERRED_EVENTS);
spin_lock_irq(&dev_priv->rps.lock);
WARN_ON(dev_priv->rps.pm_iir != 0);
I915_WRITE(GEN6_PMIMR, 0);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] drm/i915: More HSW PM clean up.
2013-02-25 23:13 [PATCH 0/6] HSW PM - RC6 clean up and fixes Rodrigo Vivi
` (4 preceding siblings ...)
2013-02-25 23:13 ` [PATCH 5/6] drm/i915: Use all PM deferred events for HSW Rodrigo Vivi
@ 2013-02-25 23:13 ` Rodrigo Vivi
2013-02-27 13:46 ` Paulo Zanoni
5 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2013-02-25 23:13 UTC (permalink / raw)
To: intel-gfx
This is the last cleaning up patch for HSW, letting render standby
programming sequence like the documented one at HSW PM programing guide.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
drivers/gpu/drm/i915/intel_pm.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d43e011..ef51174 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2749,16 +2749,7 @@ static void hsw_enable_rps(struct drm_device *dev)
GEN6_RP_UP_BUSY_AVG |
GEN7_RP_DOWN_IDLE_AVG);
- gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
-
- /* requires MSI enabled */
I915_WRITE(GEN6_PMIER, GEN7_PM_DEFERRED_EVENTS);
- spin_lock_irq(&dev_priv->rps.lock);
- WARN_ON(dev_priv->rps.pm_iir != 0);
- I915_WRITE(GEN6_PMIMR, 0);
- spin_unlock_irq(&dev_priv->rps.lock);
- /* enable all PM interrupts */
- I915_WRITE(GEN6_PMINTRMSK, 0);
gen6_gt_force_wake_put(dev_priv);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] drm/i915: Use all PM deferred events for HSW.
2013-02-25 23:13 ` [PATCH 5/6] drm/i915: Use all PM deferred events for HSW Rodrigo Vivi
@ 2013-02-26 9:38 ` Chris Wilson
0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2013-02-26 9:38 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
This is just silly. You are asking the GPU to generate interrupts which
we don't process (other than to discard).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/6] drm/i915: creating Haswell rc6 function
2013-02-25 23:13 ` [PATCH 1/6] drm/i915: creating Haswell rc6 function Rodrigo Vivi
@ 2013-02-26 20:30 ` Paulo Zanoni
2013-03-24 22:07 ` Ben Widawsky
1 sibling, 0 replies; 18+ messages in thread
From: Paulo Zanoni @ 2013-02-26 20:30 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
Hi
2 comments below:
2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> Power management, in special RC6 enabling, differs across platforms.
> This patch just split out enabling function for HSW.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 158 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 145 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 61fee7f..e8656fd 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2581,15 +2581,10 @@ static void gen6_enable_rps(struct drm_device *dev)
> rc6_mode = intel_enable_rc6(dev_priv->dev);
> if (rc6_mode & INTEL_RC6_ENABLE)
> rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
> -
> - /* We don't use those on Haswell */
> - if (!IS_HASWELL(dev)) {
> - if (rc6_mode & INTEL_RC6p_ENABLE)
> - rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
> -
> - if (rc6_mode & INTEL_RC6pp_ENABLE)
> - rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
> - }
> + if (rc6_mode & INTEL_RC6p_ENABLE)
> + rc6_mask |= GEN6_RC_CTL_RC6p_ENABLE;
Wrong indentation on the line above.
> + if (rc6_mode & INTEL_RC6pp_ENABLE)
> + rc6_mask |= GEN6_RC_CTL_RC6pp_ENABLE;
>
> DRM_INFO("Enabling RC6 states: RC6 %s, RC6p %s, RC6pp %s\n",
> (rc6_mask & GEN6_RC_CTL_RC6_ENABLE) ? "on" : "off",
> @@ -2625,7 +2620,141 @@ static void gen6_enable_rps(struct drm_device *dev)
> GEN6_RP_MEDIA_IS_GFX |
> GEN6_RP_ENABLE |
> GEN6_RP_UP_BUSY_AVG |
> - (IS_HASWELL(dev) ? GEN7_RP_DOWN_IDLE_AVG : GEN6_RP_DOWN_IDLE_CONT));
> + GEN6_RP_DOWN_IDLE_CONT);
> +
> + ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0);
> + if (!ret) {
> + pcu_mbox = 0;
> + ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
> + if (ret && pcu_mbox & (1<<31)) { /* OC supported */
> + 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");
> + }
> +
> + gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
> +
> + /* requires MSI enabled */
> + I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS);
> + spin_lock_irq(&dev_priv->rps.lock);
> + WARN_ON(dev_priv->rps.pm_iir != 0);
> + I915_WRITE(GEN6_PMIMR, 0);
> + spin_unlock_irq(&dev_priv->rps.lock);
> + /* enable all PM interrupts */
> + I915_WRITE(GEN6_PMINTRMSK, 0);
> +
> + rc6vids = 0;
> + ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
> + if (IS_GEN6(dev) && ret) {
> + DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n");
> + } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) {
> + DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n",
> + GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
> + rc6vids &= 0xffff00;
> + rc6vids |= GEN6_ENCODE_RC6_VID(450);
> + ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
> + if (ret)
> + DRM_ERROR("Couldn't fix incorrect rc6 voltage\n");
> + }
> +
> + gen6_gt_force_wake_put(dev_priv);
> +}
> +
> +static void hsw_enable_rps(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_ring_buffer *ring;
> + u32 rp_state_cap;
> + u32 gt_perf_status;
> + u32 rc6vids, pcu_mbox, rc6_mask = 0;
> + u32 gtfifodbg;
> + int rc6_mode;
> + int i, ret;
> +
> + WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
> +
> + /* Here begins a magic sequence of register writes to enable
> + * auto-downclocking.
> + *
> + * Perhaps there might be some value in exposing these to
> + * userspace...
> + */
> + I915_WRITE(GEN6_RC_STATE, 0);
> +
> + /* Clear the DBG now so we don't confuse earlier errors */
> + if ((gtfifodbg = I915_READ(GTFIFODBG))) {
> + DRM_ERROR("GT fifo had a previous error %x\n", gtfifodbg);
> + I915_WRITE(GTFIFODBG, gtfifodbg);
> + }
> +
> + gen6_gt_force_wake_get(dev_priv);
> +
> + rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> + 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.min_delay = (rp_state_cap & 0xff0000) >> 16;
> + dev_priv->rps.cur_delay = 0;
> +
> + /* disable the counters and set deterministic thresholds */
> + I915_WRITE(GEN6_RC_CONTROL, 0);
> +
> + I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16);
> + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30);
> + I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30);
> + I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000);
> + I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25);
> +
> + for_each_ring(ring, dev_priv, i)
> + I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
> +
> + I915_WRITE(GEN6_RC_SLEEP, 0);
> + I915_WRITE(GEN6_RC1e_THRESHOLD, 1000);
> + I915_WRITE(GEN6_RC6_THRESHOLD, 50000);
> + I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
> + I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
> +
> + /* Check if we are enabling RC6 */
> + rc6_mode = intel_enable_rc6(dev_priv->dev);
> + if (rc6_mode & INTEL_RC6_ENABLE)
> + rc6_mask |= GEN6_RC_CTL_RC6_ENABLE;
> +
> + DRM_INFO("Enabling RC6 states: RC6 %s\n",
> + (rc6_mask & GEN6_RC_CTL_RC6pp_ENABLE) ? "on" : "off");
s/GEN6_RC_CTL_RC6pp_ENABLE/GEN6_RC_CTL_RC6_ENABLE/
> +
> + I915_WRITE(GEN6_RC_CONTROL,
> + rc6_mask |
> + GEN6_RC_CTL_EI_MODE(1) |
> + GEN6_RC_CTL_HW_ENABLE);
> +
> + I915_WRITE(GEN6_RPNSWREQ,
> + GEN6_FREQUENCY(10) |
> + GEN6_OFFSET(0) |
> + GEN6_AGGRESSIVE_TURBO);
> + I915_WRITE(GEN6_RC_VIDEO_FREQ,
> + GEN6_FREQUENCY(12));
> +
> + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000);
> + I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> + dev_priv->rps.max_delay << 24 |
> + dev_priv->rps.min_delay << 16);
> +
> + I915_WRITE(GEN6_RP_UP_THRESHOLD, 59400);
> + I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 245000);
> + I915_WRITE(GEN6_RP_UP_EI, 66000);
> + I915_WRITE(GEN6_RP_DOWN_EI, 350000);
> +
> + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
> + I915_WRITE(GEN6_RP_CONTROL,
> + GEN6_RP_MEDIA_TURBO |
> + GEN6_RP_MEDIA_HW_NORMAL_MODE |
> + GEN6_RP_MEDIA_IS_GFX |
> + GEN6_RP_ENABLE |
> + GEN6_RP_UP_BUSY_AVG |
> + GEN7_RP_DOWN_IDLE_AVG);
>
> ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0);
> if (!ret) {
> @@ -3448,7 +3577,7 @@ void intel_disable_gt_powersave(struct drm_device *dev)
> }
> }
>
> -static void intel_gen6_powersave_work(struct work_struct *work)
> +static void intel_powersave_work(struct work_struct *work)
> {
> struct drm_i915_private *dev_priv =
> container_of(work, struct drm_i915_private,
> @@ -3456,7 +3585,10 @@ static void intel_gen6_powersave_work(struct work_struct *work)
> struct drm_device *dev = dev_priv->dev;
>
> mutex_lock(&dev_priv->rps.hw_lock);
> - gen6_enable_rps(dev);
> + if (IS_HASWELL(dev))
> + hsw_enable_rps(dev);
> + else
> + gen6_enable_rps(dev);
> gen6_update_ring_freq(dev);
> mutex_unlock(&dev_priv->rps.hw_lock);
> }
> @@ -4459,7 +4591,7 @@ void intel_gt_init(struct drm_device *dev)
> dev_priv->gt.force_wake_put = __gen6_gt_force_wake_put;
> }
> INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
> - intel_gen6_powersave_work);
> + intel_powersave_work);
> }
>
> int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u8 mbox, u32 *val)
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] drm/i915: HSW PM Frequency bits fix
2013-02-25 23:13 ` [PATCH 2/6] drm/i915: HSW PM Frequency bits fix Rodrigo Vivi
@ 2013-02-26 20:57 ` Paulo Zanoni
2013-03-24 22:16 ` Ben Widawsky
0 siblings, 1 reply; 18+ messages in thread
From: Paulo Zanoni @ 2013-02-26 20:57 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
Hi
2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> According to HSW PM programming guide, frequency bits starts at
> 24 instead of 25
This looks incomplete. Please check all the cases where RPNSWREQ is
used, I think we need to fix them too. Also, according to the PM
programming guide, all the other RPNSWREQ bits are reserved/read-only,
but I still see our code trying to set some of these bits (even if
it's trying to set it to zero), so I guess that on Haswell all the
writes to RPNSWREQ should only contain the HSW_FREQUENCY macro, not
others. And for those other bits, we need to discover where did they
go.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 527b664..ef47cee 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4184,6 +4184,7 @@
> #define GEN6_RPNSWREQ 0xA008
> #define GEN6_TURBO_DISABLE (1<<31)
> #define GEN6_FREQUENCY(x) ((x)<<25)
> +#define HSW_FREQUENCY(x) ((x)<<24)
> #define GEN6_OFFSET(x) ((x)<<19)
> #define GEN6_AGGRESSIVE_TURBO (0<<15)
> #define GEN6_RC_VIDEO_FREQ 0xA00C
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e8656fd..fbe9779 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2731,11 +2731,11 @@ static void hsw_enable_rps(struct drm_device *dev)
> GEN6_RC_CTL_HW_ENABLE);
>
> I915_WRITE(GEN6_RPNSWREQ,
> - GEN6_FREQUENCY(10) |
> + HSW_FREQUENCY(10) |
> GEN6_OFFSET(0) |
> GEN6_AGGRESSIVE_TURBO);
> I915_WRITE(GEN6_RC_VIDEO_FREQ,
> - GEN6_FREQUENCY(12));
> + HSW_FREQUENCY(12));
>
> I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 1000000);
> I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] drm/i915: HSW PM Cleaning - Removing unecessary register/bits set.
2013-02-25 23:13 ` [PATCH 3/6] drm/i915: HSW PM Cleaning - Removing unecessary register/bits set Rodrigo Vivi
@ 2013-02-26 22:26 ` Paulo Zanoni
0 siblings, 0 replies; 18+ messages in thread
From: Paulo Zanoni @ 2013-02-26 22:26 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
Hi
2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> According to HSW PM Programming guide it is not needed touch this registers
> or setting these values anymore.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fbe9779..322c562 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2702,9 +2702,7 @@ static void hsw_enable_rps(struct drm_device *dev)
> /* disable the counters and set deterministic thresholds */
> I915_WRITE(GEN6_RC_CONTROL, 0);
>
> - I915_WRITE(GEN6_RC1_WAKE_RATE_LIMIT, 1000 << 16);
> - I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16 | 30);
> - I915_WRITE(GEN6_RC6pp_WAKE_RATE_LIMIT, 30);
> + I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
> I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000);
> I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25);
>
> @@ -2712,10 +2710,7 @@ static void hsw_enable_rps(struct drm_device *dev)
> I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>
> I915_WRITE(GEN6_RC_SLEEP, 0);
> - I915_WRITE(GEN6_RC1e_THRESHOLD, 1000);
> I915_WRITE(GEN6_RC6_THRESHOLD, 50000);
> - I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
> - I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
>
> /* Check if we are enabling RC6 */
> rc6_mode = intel_enable_rc6(dev_priv->dev);
> @@ -2731,9 +2726,7 @@ static void hsw_enable_rps(struct drm_device *dev)
> GEN6_RC_CTL_HW_ENABLE);
>
> I915_WRITE(GEN6_RPNSWREQ,
> - HSW_FREQUENCY(10) |
> - GEN6_OFFSET(0) |
> - GEN6_AGGRESSIVE_TURBO);
> + HSW_FREQUENCY(10));
> I915_WRITE(GEN6_RC_VIDEO_FREQ,
> HSW_FREQUENCY(12));
>
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] drm/i915: HSW PM - removing pcode read/write.
2013-02-25 23:13 ` [PATCH 4/6] drm/i915: HSW PM - removing pcode read/write Rodrigo Vivi
@ 2013-02-26 22:35 ` Paulo Zanoni
2013-02-26 22:40 ` Paulo Zanoni
0 siblings, 1 reply; 18+ messages in thread
From: Paulo Zanoni @ 2013-02-26 22:35 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
Hi
2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> Yet according to pm spec pcode read/write operations
> aren't necessary for HSW.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 26 --------------------------
> 1 file changed, 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 322c562..761be6f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2749,18 +2749,6 @@ static void hsw_enable_rps(struct drm_device *dev)
> GEN6_RP_UP_BUSY_AVG |
> GEN7_RP_DOWN_IDLE_AVG);
>
> - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0);
> - if (!ret) {
> - pcu_mbox = 0;
> - ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
> - if (ret && pcu_mbox & (1<<31)) { /* OC supported */
> - 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");
> - }
> -
> gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
>
> /* requires MSI enabled */
> @@ -2772,20 +2760,6 @@ static void hsw_enable_rps(struct drm_device *dev)
> /* enable all PM interrupts */
> I915_WRITE(GEN6_PMINTRMSK, 0);
>
> - rc6vids = 0;
> - ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
> - if (IS_GEN6(dev) && ret) {
> - DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n");
> - } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) {
> - DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n",
> - GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
> - rc6vids &= 0xffff00;
> - rc6vids |= GEN6_ENCODE_RC6_VID(450);
> - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
> - if (ret)
> - DRM_ERROR("Couldn't fix incorrect rc6 voltage\n");
> - }
> -
> gen6_gt_force_wake_put(dev_priv);
> }
>
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] drm/i915: HSW PM - removing pcode read/write.
2013-02-26 22:35 ` Paulo Zanoni
@ 2013-02-26 22:40 ` Paulo Zanoni
0 siblings, 0 replies; 18+ messages in thread
From: Paulo Zanoni @ 2013-02-26 22:40 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
Hi
2013/2/26 Paulo Zanoni <przanoni@gmail.com>:
> Hi
>
> 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>> Yet according to pm spec pcode read/write operations
>> aren't necessary for HSW.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Where's the "undo send email" button?
drivers/gpu/drm/i915/intel_pm.c:2674:9: warning: unused variable ‘ret’
[-Wunused-variable]
drivers/gpu/drm/i915/intel_pm.c:2671:15: warning: unused variable
‘pcu_mbox’ [-Wunused-variable]
drivers/gpu/drm/i915/intel_pm.c:2671:6: warning: unused variable
‘rc6vids’ [-Wunused-variable]
>
>> ---
>> drivers/gpu/drm/i915/intel_pm.c | 26 --------------------------
>> 1 file changed, 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 322c562..761be6f 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -2749,18 +2749,6 @@ static void hsw_enable_rps(struct drm_device *dev)
>> GEN6_RP_UP_BUSY_AVG |
>> GEN7_RP_DOWN_IDLE_AVG);
>>
>> - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_MIN_FREQ_TABLE, 0);
>> - if (!ret) {
>> - pcu_mbox = 0;
>> - ret = sandybridge_pcode_read(dev_priv, GEN6_READ_OC_PARAMS, &pcu_mbox);
>> - if (ret && pcu_mbox & (1<<31)) { /* OC supported */
>> - 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");
>> - }
>> -
>> gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
>>
>> /* requires MSI enabled */
>> @@ -2772,20 +2760,6 @@ static void hsw_enable_rps(struct drm_device *dev)
>> /* enable all PM interrupts */
>> I915_WRITE(GEN6_PMINTRMSK, 0);
>>
>> - rc6vids = 0;
>> - ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
>> - if (IS_GEN6(dev) && ret) {
>> - DRM_DEBUG_DRIVER("Couldn't check for BIOS workaround\n");
>> - } else if (IS_GEN6(dev) && (GEN6_DECODE_RC6_VID(rc6vids & 0xff) < 450)) {
>> - DRM_DEBUG_DRIVER("You should update your BIOS. Correcting minimum rc6 voltage (%dmV->%dmV)\n",
>> - GEN6_DECODE_RC6_VID(rc6vids & 0xff), 450);
>> - rc6vids &= 0xffff00;
>> - rc6vids |= GEN6_ENCODE_RC6_VID(450);
>> - ret = sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_RC6VIDS, rc6vids);
>> - if (ret)
>> - DRM_ERROR("Couldn't fix incorrect rc6 voltage\n");
>> - }
>> -
>> gen6_gt_force_wake_put(dev_priv);
>> }
>>
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] drm/i915: More HSW PM clean up.
2013-02-25 23:13 ` [PATCH 6/6] drm/i915: More HSW PM clean up Rodrigo Vivi
@ 2013-02-27 13:46 ` Paulo Zanoni
2013-03-22 21:13 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: Paulo Zanoni @ 2013-02-27 13:46 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
Hi
2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> This is the last cleaning up patch for HSW, letting render standby
> programming sequence like the documented one at HSW PM programing guide.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d43e011..ef51174 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2749,16 +2749,7 @@ static void hsw_enable_rps(struct drm_device *dev)
> GEN6_RP_UP_BUSY_AVG |
> GEN7_RP_DOWN_IDLE_AVG);
>
> - gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
Looks like the line above is not even on the SNB docs, and git blame
shows it's a bug fix. I'll let Chris comment on that. Having a nice
comment explaining why that code is there would be good :)
> -
> - /* requires MSI enabled */
> I915_WRITE(GEN6_PMIER, GEN7_PM_DEFERRED_EVENTS);
> - spin_lock_irq(&dev_priv->rps.lock);
> - WARN_ON(dev_priv->rps.pm_iir != 0);
> - I915_WRITE(GEN6_PMIMR, 0);
> - spin_unlock_irq(&dev_priv->rps.lock);
> - /* enable all PM interrupts */
> - I915_WRITE(GEN6_PMINTRMSK, 0);
This is all part of the interrupt enabling: just changing PMIER is not
enough, you have to change the masks and possibly clear the IIR
register.
>
> gen6_gt_force_wake_put(dev_priv);
> }
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] drm/i915: More HSW PM clean up.
2013-02-27 13:46 ` Paulo Zanoni
@ 2013-03-22 21:13 ` Rodrigo Vivi
2013-03-22 22:12 ` Chris Wilson
0 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2013-03-22 21:13 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 2045 bytes --]
On Wed, Feb 27, 2013 at 10:46 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> Hi
>
> 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> > This is the last cleaning up patch for HSW, letting render standby
> > programming sequence like the documented one at HSW PM programing guide.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 9 ---------
> > 1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> > index d43e011..ef51174 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2749,16 +2749,7 @@ static void hsw_enable_rps(struct drm_device *dev)
> > GEN6_RP_UP_BUSY_AVG |
> > GEN7_RP_DOWN_IDLE_AVG);
> >
> > - gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
>
> Looks like the line above is not even on the SNB docs, and git blame
> shows it's a bug fix. I'll let Chris comment on that. Having a nice
> comment explaining why that code is there would be good :)
>
Any comment Chris? what is this for? is this still needed for HSW?
>
> > -
> > - /* requires MSI enabled */
> > I915_WRITE(GEN6_PMIER, GEN7_PM_DEFERRED_EVENTS);
> > - spin_lock_irq(&dev_priv->rps.lock);
> > - WARN_ON(dev_priv->rps.pm_iir != 0);
> > - I915_WRITE(GEN6_PMIMR, 0);
> > - spin_unlock_irq(&dev_priv->rps.lock);
> > - /* enable all PM interrupts */
> > - I915_WRITE(GEN6_PMINTRMSK, 0);
>
> This is all part of the interrupt enabling: just changing PMIER is not
> enough, you have to change the masks and possibly clear the IIR
> register.
>
Why?
>
> >
> > gen6_gt_force_wake_put(dev_priv);
> > }
> > --
> > 1.8.1.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
[-- Attachment #1.2: Type: text/html, Size: 3439 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] 18+ messages in thread
* Re: [PATCH 6/6] drm/i915: More HSW PM clean up.
2013-03-22 21:13 ` Rodrigo Vivi
@ 2013-03-22 22:12 ` Chris Wilson
0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2013-03-22 22:12 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Fri, Mar 22, 2013 at 06:13:50PM -0300, Rodrigo Vivi wrote:
>
>
>
> On Wed, Feb 27, 2013 at 10:46 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> Hi
>
> 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> > This is the last cleaning up patch for HSW, letting render standby
> > programming sequence like the documented one at HSW PM programing
> guide.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 9 ---------
> > 1 file changed, 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/
> i915/intel_pm.c
> > index d43e011..ef51174 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2749,16 +2749,7 @@ static void hsw_enable_rps(struct drm_device
> *dev)
> > GEN6_RP_UP_BUSY_AVG |
> > GEN7_RP_DOWN_IDLE_AVG);
> >
> > - gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00)
> >> 8);
>
> Looks like the line above is not even on the SNB docs, and git blame
> shows it's a bug fix. I'll let Chris comment on that. Having a nice
> comment explaining why that code is there would be good :)
>
> Any comment Chris? what is this for? is this still needed for HSW?
Yes, it is still needed for our keeping our RPS state tracking in sync
with the register values.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/6] drm/i915: creating Haswell rc6 function
2013-02-25 23:13 ` [PATCH 1/6] drm/i915: creating Haswell rc6 function Rodrigo Vivi
2013-02-26 20:30 ` Paulo Zanoni
@ 2013-03-24 22:07 ` Ben Widawsky
1 sibling, 0 replies; 18+ messages in thread
From: Ben Widawsky @ 2013-03-24 22:07 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Mon, Feb 25, 2013 at 08:13:38PM -0300, Rodrigo Vivi wrote:
> Power management, in special RC6 enabling, differs across platforms.
> This patch just split out enabling function for HSW.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
I'm not at all opposed to splitting out Haswell rps stuff, but can you
confirm that we really need to? I suspect gen6 + HSW are mostly the same
and many of the differences currently exist because our gen6 stuff is
very stale.
[snip]
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] drm/i915: HSW PM Frequency bits fix
2013-02-26 20:57 ` Paulo Zanoni
@ 2013-03-24 22:16 ` Ben Widawsky
0 siblings, 0 replies; 18+ messages in thread
From: Ben Widawsky @ 2013-03-24 22:16 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx
On Tue, Feb 26, 2013 at 05:57:20PM -0300, Paulo Zanoni wrote:
> Hi
>
> 2013/2/25 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> > According to HSW PM programming guide, frequency bits starts at
> > 24 instead of 25
>
> This looks incomplete. Please check all the cases where RPNSWREQ is
> used, I think we need to fix them too. Also, according to the PM
> programming guide, all the other RPNSWREQ bits are reserved/read-only,
> but I still see our code trying to set some of these bits (even if
> it's trying to set it to zero), so I guess that on Haswell all the
> writes to RPNSWREQ should only contain the HSW_FREQUENCY macro, not
> others. And for those other bits, we need to discover where did they
> go.
We really should get this, or some version of this patch committed
sooner rather than later.
If there is disagreement over the entire series, could you please
extract the important part into an individual patch?
[snip]
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-03-24 22:14 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-25 23:13 [PATCH 0/6] HSW PM - RC6 clean up and fixes Rodrigo Vivi
2013-02-25 23:13 ` [PATCH 1/6] drm/i915: creating Haswell rc6 function Rodrigo Vivi
2013-02-26 20:30 ` Paulo Zanoni
2013-03-24 22:07 ` Ben Widawsky
2013-02-25 23:13 ` [PATCH 2/6] drm/i915: HSW PM Frequency bits fix Rodrigo Vivi
2013-02-26 20:57 ` Paulo Zanoni
2013-03-24 22:16 ` Ben Widawsky
2013-02-25 23:13 ` [PATCH 3/6] drm/i915: HSW PM Cleaning - Removing unecessary register/bits set Rodrigo Vivi
2013-02-26 22:26 ` Paulo Zanoni
2013-02-25 23:13 ` [PATCH 4/6] drm/i915: HSW PM - removing pcode read/write Rodrigo Vivi
2013-02-26 22:35 ` Paulo Zanoni
2013-02-26 22:40 ` Paulo Zanoni
2013-02-25 23:13 ` [PATCH 5/6] drm/i915: Use all PM deferred events for HSW Rodrigo Vivi
2013-02-26 9:38 ` Chris Wilson
2013-02-25 23:13 ` [PATCH 6/6] drm/i915: More HSW PM clean up Rodrigo Vivi
2013-02-27 13:46 ` Paulo Zanoni
2013-03-22 21:13 ` Rodrigo Vivi
2013-03-22 22:12 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.