public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq."
@ 2014-03-27  8:24 Chris Wilson
  2014-03-27  8:24 ` [PATCH 2/3] drm/i915: Refactor gen6_set_rps Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Chris Wilson @ 2014-03-27  8:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Deepak S, Daniel Vetter

This reverts commit 2754436913b94626a5414d82f0996489628c513d.

Conflicts:
	drivers/gpu/drm/i915/i915_irq.c

The partial application of interrupt masking without regard to other
pathways for adjusting the RPS frequency results in completely disabling
the PM interrupts. This leads to excessive power consumption as the GPU
is kept at max clocks (until the failsafe mechanism fires of explicitly
downclocking the GPU when all requests are idle). Or equally as bad for
the UX, the GPU is kept at minimum clocks and prevented from upclocking
in response to a requirement for more power.

Testcase: pm_rps/blocking
Cc: Deepak S <deepak.s@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 ---
 drivers/gpu/drm/i915/i915_irq.c | 38 --------------------------------------
 drivers/gpu/drm/i915/intel_pm.c |  8 --------
 3 files changed, 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf214a4b8fdc..0eae9cd8347e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -846,9 +846,6 @@ struct intel_gen6_power_mgmt {
 	u8 rp1_freq;		/* "less than" RP0 power/freqency */
 	u8 rp0_freq;		/* Non-overclocked max frequency. */
 
-	bool rp_up_masked;
-	bool rp_down_masked;
-
 	int last_adj;
 	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6e37580de4bc..8df8876f557c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1098,43 +1098,6 @@ static void notify_ring(struct drm_device *dev,
 	i915_queue_hangcheck(dev);
 }
 
-void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
-			     u32 pm_iir, int new_delay)
-{
-	if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
-		if (new_delay >= dev_priv->rps.max_freq_softlimit) {
-			/* Mask UP THRESHOLD Interrupts */
-			I915_WRITE(GEN6_PMINTRMSK,
-				   I915_READ(GEN6_PMINTRMSK) |
-				   GEN6_PM_RP_UP_THRESHOLD);
-			dev_priv->rps.rp_up_masked = true;
-		}
-		if (dev_priv->rps.rp_down_masked) {
-			/* UnMask DOWN THRESHOLD Interrupts */
-			I915_WRITE(GEN6_PMINTRMSK,
-				   I915_READ(GEN6_PMINTRMSK) &
-				   ~GEN6_PM_RP_DOWN_THRESHOLD);
-			dev_priv->rps.rp_down_masked = false;
-		}
-	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
-		if (new_delay <= dev_priv->rps.min_freq_softlimit) {
-			/* Mask DOWN THRESHOLD Interrupts */
-			I915_WRITE(GEN6_PMINTRMSK,
-				   I915_READ(GEN6_PMINTRMSK) |
-				   GEN6_PM_RP_DOWN_THRESHOLD);
-			dev_priv->rps.rp_down_masked = true;
-		}
-
-		if (dev_priv->rps.rp_up_masked) {
-			/* UnMask UP THRESHOLD Interrupts */
-			I915_WRITE(GEN6_PMINTRMSK,
-				   I915_READ(GEN6_PMINTRMSK) &
-				   ~GEN6_PM_RP_UP_THRESHOLD);
-			dev_priv->rps.rp_up_masked = false;
-		}
-	}
-}
-
 static void gen6_pm_rps_work(struct work_struct *work)
 {
 	drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
@@ -1194,7 +1157,6 @@ static void gen6_pm_rps_work(struct work_struct *work)
 			    dev_priv->rps.min_freq_softlimit,
 			    dev_priv->rps.max_freq_softlimit);
 
-	gen6_set_pm_mask(dev_priv, pm_iir, new_delay);
 	dev_priv->rps.last_adj = new_delay - dev_priv->rps.cur_freq;
 
 	if (IS_VALLEYVIEW(dev_priv->dev))
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 22134558c452..edf1b29d9856 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3095,11 +3095,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
 		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
 				~VLV_GFX_CLK_FORCE_ON_BIT);
-
-	/* Unmask Up interrupts */
-	dev_priv->rps.rp_up_masked = true;
-	gen6_set_pm_mask(dev_priv, GEN6_PM_RP_DOWN_THRESHOLD,
-						dev_priv->rps.min_freq_softlimit);
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
@@ -3694,9 +3689,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
 
-	dev_priv->rps.rp_up_masked = false;
-	dev_priv->rps.rp_down_masked = false;
-
 	gen6_enable_rps_interrupts(dev);
 
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
-- 
1.9.1

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

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

* [PATCH 2/3] drm/i915: Refactor gen6_set_rps
  2014-03-27  8:24 [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq." Chris Wilson
@ 2014-03-27  8:24 ` Chris Wilson
  2014-03-30  6:45   ` Deepak S
  2014-03-27  8:24 ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Chris Wilson
  2014-03-30  6:48 ` [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq." Deepak S
  2 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2014-03-27  8:24 UTC (permalink / raw)
  To: intel-gfx

What used to be a short-circuit now needs to adjust interrupt masking in
response to user requests for changing the min/max allowed frequencies.
This is currently done by a special case and early return, but the next
patch adds another common action to take, so refactor the code to reduce
duplication.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index edf1b29d9856..3ad590924062 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3017,36 +3017,30 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	WARN_ON(val > dev_priv->rps.max_freq_softlimit);
 	WARN_ON(val < dev_priv->rps.min_freq_softlimit);
 
-	if (val == dev_priv->rps.cur_freq) {
-		/* min/max delay may still have been modified so be sure to
-		 * write the limits value */
-		I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
-			   gen6_rps_limits(dev_priv, val));
+	/* min/max delay may still have been modified so be sure to
+	 * write the limits value.
+	 */
+	if (val != dev_priv->rps.cur_freq) {
+		gen6_set_rps_thresholds(dev_priv, val);
 
-		return;
+		if (IS_HASWELL(dev))
+			I915_WRITE(GEN6_RPNSWREQ,
+				   HSW_FREQUENCY(val));
+		else
+			I915_WRITE(GEN6_RPNSWREQ,
+				   GEN6_FREQUENCY(val) |
+				   GEN6_OFFSET(0) |
+				   GEN6_AGGRESSIVE_TURBO);
 	}
 
-	gen6_set_rps_thresholds(dev_priv, val);
-
-	if (IS_HASWELL(dev))
-		I915_WRITE(GEN6_RPNSWREQ,
-			   HSW_FREQUENCY(val));
-	else
-		I915_WRITE(GEN6_RPNSWREQ,
-			   GEN6_FREQUENCY(val) |
-			   GEN6_OFFSET(0) |
-			   GEN6_AGGRESSIVE_TURBO);
-
 	/* Make sure we continue to get interrupts
 	 * until we hit the minimum or maximum frequencies.
 	 */
-	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
-		   gen6_rps_limits(dev_priv, val));
+	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
 
 	POSTING_READ(GEN6_RPNSWREQ);
 
 	dev_priv->rps.cur_freq = val;
-
 	trace_intel_gpu_freq_change(val * 50);
 }
 
-- 
1.9.1

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

* [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits
  2014-03-27  8:24 [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq." Chris Wilson
  2014-03-27  8:24 ` [PATCH 2/3] drm/i915: Refactor gen6_set_rps Chris Wilson
@ 2014-03-27  8:24 ` Chris Wilson
  2014-03-27  8:34   ` Chris Wilson
                     ` (2 more replies)
  2014-03-30  6:48 ` [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq." Deepak S
  2 siblings, 3 replies; 13+ messages in thread
From: Chris Wilson @ 2014-03-27  8:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Deepak S

The speculation is that we can conserve more power by masking off the
interrupts at source (PMINTRMSK) rather than filtering them by the
up/down thresholds (RPINTLIM).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Deepak S <deepak.s@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3ad590924062..0a76e9baeca2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3006,6 +3006,25 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 	dev_priv->rps.last_adj = 0;
 }
 
+static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
+{
+	u32 mask;
+
+	mask = GEN6_PM_RP_DOWN_TIMEOUT;
+	if (val > dev_priv->rps.min_freq_softlimit)
+		mask |= GEN6_PM_RP_DOWN_THRESHOLD;
+	if (val < dev_priv->rps.max_freq_softlimit)
+		mask |= GEN6_PM_RP_UP_THRESHOLD;
+
+	/* IVB and SNB hard hangs on looping batchbuffer
+	 * if GEN6_PM_UP_EI_EXPIRED is masked.
+	 */
+	if (INTEL_INFO(dev_priv->dev)->gen <= 7 && !IS_HASWELL(dev_priv->dev))
+		mask |= GEN6_PM_RP_UP_EI_EXPIRED;
+
+	return ~mask;
+}
+
 /* gen6_set_rps is called to update the frequency request, but should also be
  * called when the range (min_delay and max_delay) is modified so that we can
  * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
@@ -3037,6 +3056,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	 * until we hit the minimum or maximum frequencies.
 	 */
 	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
+	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	POSTING_READ(GEN6_RPNSWREQ);
 
@@ -3220,24 +3240,12 @@ int intel_enable_rc6(const struct drm_device *dev)
 static void gen6_enable_rps_interrupts(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 enabled_intrs;
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	WARN_ON(dev_priv->rps.pm_iir);
 	snb_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
 	I915_WRITE(GEN6_PMIIR, dev_priv->pm_rps_events);
 	spin_unlock_irq(&dev_priv->irq_lock);
-
-	/* only unmask PM interrupts we need. Mask all others. */
-	enabled_intrs = dev_priv->pm_rps_events;
-
-	/* IVB and SNB hard hangs on looping batchbuffer
-	 * if GEN6_PM_UP_EI_EXPIRED is masked.
-	 */
-	if (INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev))
-		enabled_intrs |= GEN6_PM_RP_UP_EI_EXPIRED;
-
-	I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
 }
 
 static void gen8_enable_rps(struct drm_device *dev)
-- 
1.9.1

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

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

* Re: [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits
  2014-03-27  8:24 ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Chris Wilson
@ 2014-03-27  8:34   ` Chris Wilson
  2014-03-27  8:35   ` [PATCH] drm/i915: Mask PM interrupt generation when at up/down limits for VLV Chris Wilson
  2014-03-27 14:51   ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Deepak S
  2 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2014-03-27  8:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Deepak S

On Thu, Mar 27, 2014 at 08:24:21AM +0000, Chris Wilson wrote:
> The speculation is that we can conserve more power by masking off the
> interrupts at source (PMINTRMSK) rather than filtering them by the
> up/down thresholds (RPINTLIM).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Deepak S <deepak.s@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3ad590924062..0a76e9baeca2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3006,6 +3006,25 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>  	dev_priv->rps.last_adj = 0;
>  }
>  
> +static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
> +{
> +	u32 mask;
> +
> +	mask = GEN6_PM_RP_DOWN_TIMEOUT;
> +	if (val > dev_priv->rps.min_freq_softlimit)
> +		mask |= GEN6_PM_RP_DOWN_THRESHOLD;

Actually we only need DOWN_TIMEOUT when above min freq as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* [PATCH] drm/i915: Mask PM interrupt generation when at up/down limits for VLV
  2014-03-27  8:24 ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Chris Wilson
  2014-03-27  8:34   ` Chris Wilson
@ 2014-03-27  8:35   ` Chris Wilson
  2014-03-27  8:46     ` Chris Wilson
  2014-03-27 14:51   ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Deepak S
  2 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2014-03-27  8:35 UTC (permalink / raw)
  To: intel-gfx; +Cc: Deepak S

The speculation is that we can conserve more power by masking off the
interrupts at source (PMINTRMSK) rather than filtering them by the
up/down thresholds (RPINTLIM).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Deepak S <deepak.s@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_pm.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0a76e9baeca2..d41cce93772b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3109,6 +3109,9 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
 		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
 				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	I915_WRITE(GEN6_PMINTRMSK,
+		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
@@ -3154,13 +3157,12 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 			 dev_priv->rps.cur_freq,
 			 vlv_gpu_freq(dev_priv, val), val);
 
-	if (val == dev_priv->rps.cur_freq)
-		return;
+	if (val != dev_priv->rps.cur_freq)
+		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 
-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+	I915_WRITE(GEN6_PMINTRMSK, val);
 
 	dev_priv->rps.cur_freq = val;
-
 	trace_intel_gpu_freq_change(vlv_gpu_freq(dev_priv, val));
 }
 
-- 
1.9.1

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

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

* Re: [PATCH] drm/i915: Mask PM interrupt generation when at up/down limits for VLV
  2014-03-27  8:35   ` [PATCH] drm/i915: Mask PM interrupt generation when at up/down limits for VLV Chris Wilson
@ 2014-03-27  8:46     ` Chris Wilson
  0 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2014-03-27  8:46 UTC (permalink / raw)
  To: intel-gfx; +Cc: Deepak S

On Thu, Mar 27, 2014 at 08:35:11AM +0000, Chris Wilson wrote:
> The speculation is that we can conserve more power by masking off the
> interrupts at source (PMINTRMSK) rather than filtering them by the
> up/down thresholds (RPINTLIM).

Drat, cut'n'paste comment doesn't completely apply to vlv.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits
  2014-03-27  8:24 ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Chris Wilson
  2014-03-27  8:34   ` Chris Wilson
  2014-03-27  8:35   ` [PATCH] drm/i915: Mask PM interrupt generation when at up/down limits for VLV Chris Wilson
@ 2014-03-27 14:51   ` Deepak S
  2014-03-27 15:26     ` Chris Wilson
  2014-03-28  8:03     ` [PATCH] drm/i915: Mask PM/RPS interrupt generation based on activity Chris Wilson
  2 siblings, 2 replies; 13+ messages in thread
From: Deepak S @ 2014-03-27 14:51 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Deepak S


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


On Thursday 27 March 2014 01:54 PM, Chris Wilson wrote:
> The speculation is that we can conserve more power by masking off the
> interrupts at source (PMINTRMSK) rather than filtering them by the
> up/down thresholds (RPINTLIM).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Deepak S <deepak.s@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++------------
>   1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3ad590924062..0a76e9baeca2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3006,6 +3006,25 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>   	dev_priv->rps.last_adj = 0;
>   }
>   
> +static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
> +{
> +	u32 mask;
> +
> +	mask = GEN6_PM_RP_DOWN_TIMEOUT;
> +	if (val > dev_priv->rps.min_freq_softlimit)
> +		mask |= GEN6_PM_RP_DOWN_THRESHOLD;
> +	if (val < dev_priv->rps.max_freq_softlimit)
> +		mask |= GEN6_PM_RP_UP_THRESHOLD;
> +
> +	/* IVB and SNB hard hangs on looping batchbuffer
> +	 * if GEN6_PM_UP_EI_EXPIRED is masked.
> +	 */
> +	if (INTEL_INFO(dev_priv->dev)->gen <= 7 && !IS_HASWELL(dev_priv->dev))
> +		mask |= GEN6_PM_RP_UP_EI_EXPIRED;
> +
> +	return ~mask;
> +}
> +
>   /* gen6_set_rps is called to update the frequency request, but should also be
>    * called when the range (min_delay and max_delay) is modified so that we can
>    * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
> @@ -3037,6 +3056,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
>   	 * until we hit the minimum or maximum frequencies.
>   	 */
>   	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
> +	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
>   
>   	POSTING_READ(GEN6_RPNSWREQ);
>   
> @@ -3220,24 +3240,12 @@ int intel_enable_rc6(const struct drm_device *dev)
>   static void gen6_enable_rps_interrupts(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 enabled_intrs;
>   
>   	spin_lock_irq(&dev_priv->irq_lock);
>   	WARN_ON(dev_priv->rps.pm_iir);
>   	snb_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
>   	I915_WRITE(GEN6_PMIIR, dev_priv->pm_rps_events);
>   	spin_unlock_irq(&dev_priv->irq_lock);
> -
> -	/* only unmask PM interrupts we need. Mask all others. */
> -	enabled_intrs = dev_priv->pm_rps_events;
> -
> -	/* IVB and SNB hard hangs on looping batchbuffer
> -	 * if GEN6_PM_UP_EI_EXPIRED is masked.
> -	 */
> -	if (INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev))
> -		enabled_intrs |= GEN6_PM_RP_UP_EI_EXPIRED;
> -
> -	I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
>   }
>   
>   static void gen8_enable_rps(struct drm_device *dev)
On VLV,  gen6_enable_rps_interrupts  is used to enable turbo 
interrutpts. I think we need to extend gen6_rps_pm_maskto valleyview also?

[-- Attachment #1.2: Type: text/html, Size: 3703 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] 13+ messages in thread

* Re: [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits
  2014-03-27 14:51   ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Deepak S
@ 2014-03-27 15:26     ` Chris Wilson
  2014-03-28  8:03     ` [PATCH] drm/i915: Mask PM/RPS interrupt generation based on activity Chris Wilson
  1 sibling, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2014-03-27 15:26 UTC (permalink / raw)
  To: Deepak S; +Cc: Deepak S, intel-gfx

On Thu, Mar 27, 2014 at 08:21:29PM +0530, Deepak S wrote:
>    On VLV,  gen6_enable_rps_interrupts  is used to enable turbo interrutpts.
>    I think we need to extend gen6_rps_pm_mask  to valleyview also?

Check patch 4/3.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* [PATCH] drm/i915: Mask PM/RPS interrupt generation based on activity
  2014-03-27 14:51   ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Deepak S
  2014-03-27 15:26     ` Chris Wilson
@ 2014-03-28  8:03     ` Chris Wilson
  2014-03-30  6:38       ` Deepak S
  1 sibling, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2014-03-28  8:03 UTC (permalink / raw)
  To: intel-gfx

The speculation is that we can conserve more power by masking off
the interrupts at source (PMINTRMSK) rather than filtering them by the
up/down thresholds (RPINTLIM). We can select which events we know will
be active based on the current frequency versus our imposed range, i.e.
if at minimum, we know we will not want to generate any more
down-interrupts and vice versa.

v2: We only need the TIMEOUT when above min frequency.
v3: Tweak VLV at the same time

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Deepak S <deepak.s@linux.intel.com>
---
I see your point that I cannot remove PMINTRMSK from enable_interrupts
without tweaking VLV at the same time. I did consider making this 4
patches (factor out gen6_rps_pm_mask, tweak gen6+, tweak, vlv, remove it
from enable_interrupts) but decided that was just being silly and
squashed the two patches together instead.
---
 drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 154aa07d51a7..35a7b5b65883 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3006,6 +3006,24 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 	dev_priv->rps.last_adj = 0;
 }
 
+static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
+{
+	u32 mask = 0;
+
+	if (val > dev_priv->rps.min_freq_softlimit)
+		mask |= GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT;
+	if (val < dev_priv->rps.max_freq_softlimit)
+		mask |= GEN6_PM_RP_UP_THRESHOLD;
+
+	/* IVB and SNB hard hangs on looping batchbuffer
+	 * if GEN6_PM_UP_EI_EXPIRED is masked.
+	 */
+	if (INTEL_INFO(dev_priv->dev)->gen <= 7 && !IS_HASWELL(dev_priv->dev))
+		mask |= GEN6_PM_RP_UP_EI_EXPIRED;
+
+	return ~mask;
+}
+
 /* gen6_set_rps is called to update the frequency request, but should also be
  * called when the range (min_delay and max_delay) is modified so that we can
  * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
@@ -3037,6 +3055,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
 	 * until we hit the minimum or maximum frequencies.
 	 */
 	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
+	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	POSTING_READ(GEN6_RPNSWREQ);
 
@@ -3089,6 +3108,9 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
 	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
 		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
 				~VLV_GFX_CLK_FORCE_ON_BIT);
+
+	I915_WRITE(GEN6_PMINTRMSK,
+		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
 }
 
 void gen6_rps_idle(struct drm_i915_private *dev_priv)
@@ -3134,13 +3156,12 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
 			 dev_priv->rps.cur_freq,
 			 vlv_gpu_freq(dev_priv, val), val);
 
-	if (val == dev_priv->rps.cur_freq)
-		return;
+	if (val != dev_priv->rps.cur_freq)
+		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
 
-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+	I915_WRITE(GEN6_PMINTRMSK, val);
 
 	dev_priv->rps.cur_freq = val;
-
 	trace_intel_gpu_freq_change(vlv_gpu_freq(dev_priv, val));
 }
 
@@ -3220,24 +3241,12 @@ int intel_enable_rc6(const struct drm_device *dev)
 static void gen6_enable_rps_interrupts(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 enabled_intrs;
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	WARN_ON(dev_priv->rps.pm_iir);
 	snb_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
 	I915_WRITE(GEN6_PMIIR, dev_priv->pm_rps_events);
 	spin_unlock_irq(&dev_priv->irq_lock);
-
-	/* only unmask PM interrupts we need. Mask all others. */
-	enabled_intrs = dev_priv->pm_rps_events;
-
-	/* IVB and SNB hard hangs on looping batchbuffer
-	 * if GEN6_PM_UP_EI_EXPIRED is masked.
-	 */
-	if (INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev))
-		enabled_intrs |= GEN6_PM_RP_UP_EI_EXPIRED;
-
-	I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
 }
 
 static void gen8_enable_rps(struct drm_device *dev)
-- 
1.9.1

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

* Re: [PATCH] drm/i915: Mask PM/RPS interrupt generation based on activity
  2014-03-28  8:03     ` [PATCH] drm/i915: Mask PM/RPS interrupt generation based on activity Chris Wilson
@ 2014-03-30  6:38       ` Deepak S
  2014-03-31  8:30         ` Daniel Vetter
  0 siblings, 1 reply; 13+ messages in thread
From: Deepak S @ 2014-03-30  6:38 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On Friday 28 March 2014 01:33 PM, Chris Wilson wrote:
> The speculation is that we can conserve more power by masking off
> the interrupts at source (PMINTRMSK) rather than filtering them by the
> up/down thresholds (RPINTLIM). We can select which events we know will
> be active based on the current frequency versus our imposed range, i.e.
> if at minimum, we know we will not want to generate any more
> down-interrupts and vice versa.
>
> v2: We only need the TIMEOUT when above min frequency.
> v3: Tweak VLV at the same time
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Deepak S <deepak.s@linux.intel.com>
> ---
> I see your point that I cannot remove PMINTRMSK from enable_interrupts
> without tweaking VLV at the same time. I did consider making this 4
> patches (factor out gen6_rps_pm_mask, tweak gen6+, tweak, vlv, remove it
> from enable_interrupts) but decided that was just being silly and
> squashed the two patches together instead.
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++++++++++++++++----------------
>   1 file changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 154aa07d51a7..35a7b5b65883 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3006,6 +3006,24 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>   	dev_priv->rps.last_adj = 0;
>   }
>   
> +static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
> +{
> +	u32 mask = 0;
> +
> +	if (val > dev_priv->rps.min_freq_softlimit)
> +		mask |= GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT;
> +	if (val < dev_priv->rps.max_freq_softlimit)
> +		mask |= GEN6_PM_RP_UP_THRESHOLD;
> +
> +	/* IVB and SNB hard hangs on looping batchbuffer
> +	 * if GEN6_PM_UP_EI_EXPIRED is masked.
> +	 */
> +	if (INTEL_INFO(dev_priv->dev)->gen <= 7 && !IS_HASWELL(dev_priv->dev))
> +		mask |= GEN6_PM_RP_UP_EI_EXPIRED;
> +
> +	return ~mask;
> +}
> +
>   /* gen6_set_rps is called to update the frequency request, but should also be
>    * called when the range (min_delay and max_delay) is modified so that we can
>    * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
> @@ -3037,6 +3055,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
>   	 * until we hit the minimum or maximum frequencies.
>   	 */
>   	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
> +	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
>   
>   	POSTING_READ(GEN6_RPNSWREQ);
>   
> @@ -3089,6 +3108,9 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>   	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
>   		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
>   				~VLV_GFX_CLK_FORCE_ON_BIT);
> +
> +	I915_WRITE(GEN6_PMINTRMSK,
> +		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
>   }
>   
>   void gen6_rps_idle(struct drm_i915_private *dev_priv)
> @@ -3134,13 +3156,12 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
>   			 dev_priv->rps.cur_freq,
>   			 vlv_gpu_freq(dev_priv, val), val);
>   
> -	if (val == dev_priv->rps.cur_freq)
> -		return;
> +	if (val != dev_priv->rps.cur_freq)
> +		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
>   
> -	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> +	I915_WRITE(GEN6_PMINTRMSK, val);
>   
>   	dev_priv->rps.cur_freq = val;
> -
>   	trace_intel_gpu_freq_change(vlv_gpu_freq(dev_priv, val));
>   }
>   
> @@ -3220,24 +3241,12 @@ int intel_enable_rc6(const struct drm_device *dev)
>   static void gen6_enable_rps_interrupts(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	u32 enabled_intrs;
>   
>   	spin_lock_irq(&dev_priv->irq_lock);
>   	WARN_ON(dev_priv->rps.pm_iir);
>   	snb_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
>   	I915_WRITE(GEN6_PMIIR, dev_priv->pm_rps_events);
>   	spin_unlock_irq(&dev_priv->irq_lock);
> -
> -	/* only unmask PM interrupts we need. Mask all others. */
> -	enabled_intrs = dev_priv->pm_rps_events;
> -
> -	/* IVB and SNB hard hangs on looping batchbuffer
> -	 * if GEN6_PM_UP_EI_EXPIRED is masked.
> -	 */
> -	if (INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev))
> -		enabled_intrs |= GEN6_PM_RP_UP_EI_EXPIRED;
> -
> -	I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
>   }
>   
>   static void gen8_enable_rps(struct drm_device *dev)

Reviewed-by:Deepak S <deepak.s@linux.intel.com>

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

* Re: [PATCH 2/3] drm/i915: Refactor gen6_set_rps
  2014-03-27  8:24 ` [PATCH 2/3] drm/i915: Refactor gen6_set_rps Chris Wilson
@ 2014-03-30  6:45   ` Deepak S
  0 siblings, 0 replies; 13+ messages in thread
From: Deepak S @ 2014-03-30  6:45 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On Thursday 27 March 2014 01:54 PM, Chris Wilson wrote:
> What used to be a short-circuit now needs to adjust interrupt masking in
> response to user requests for changing the min/max allowed frequencies.
> This is currently done by a special case and early return, but the next
> patch adds another common action to take, so refactor the code to reduce
> duplication.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++--------------------
>   1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index edf1b29d9856..3ad590924062 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3017,36 +3017,30 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
>   	WARN_ON(val > dev_priv->rps.max_freq_softlimit);
>   	WARN_ON(val < dev_priv->rps.min_freq_softlimit);
>   
> -	if (val == dev_priv->rps.cur_freq) {
> -		/* min/max delay may still have been modified so be sure to
> -		 * write the limits value */
> -		I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> -			   gen6_rps_limits(dev_priv, val));
> +	/* min/max delay may still have been modified so be sure to
> +	 * write the limits value.
> +	 */
> +	if (val != dev_priv->rps.cur_freq) {
> +		gen6_set_rps_thresholds(dev_priv, val);
>   
> -		return;
> +		if (IS_HASWELL(dev))
> +			I915_WRITE(GEN6_RPNSWREQ,
> +				   HSW_FREQUENCY(val));
> +		else
> +			I915_WRITE(GEN6_RPNSWREQ,
> +				   GEN6_FREQUENCY(val) |
> +				   GEN6_OFFSET(0) |
> +				   GEN6_AGGRESSIVE_TURBO);
>   	}
>   
> -	gen6_set_rps_thresholds(dev_priv, val);
> -
> -	if (IS_HASWELL(dev))
> -		I915_WRITE(GEN6_RPNSWREQ,
> -			   HSW_FREQUENCY(val));
> -	else
> -		I915_WRITE(GEN6_RPNSWREQ,
> -			   GEN6_FREQUENCY(val) |
> -			   GEN6_OFFSET(0) |
> -			   GEN6_AGGRESSIVE_TURBO);
> -
>   	/* Make sure we continue to get interrupts
>   	 * until we hit the minimum or maximum frequencies.
>   	 */
> -	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS,
> -		   gen6_rps_limits(dev_priv, val));
> +	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
>   
>   	POSTING_READ(GEN6_RPNSWREQ);
>   
>   	dev_priv->rps.cur_freq = val;
> -
>   	trace_intel_gpu_freq_change(val * 50);
>   }
>   

Reviewed-by:Deepak S <deepak.s@linux.intel.com>

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

* Re: [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq."
  2014-03-27  8:24 [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq." Chris Wilson
  2014-03-27  8:24 ` [PATCH 2/3] drm/i915: Refactor gen6_set_rps Chris Wilson
  2014-03-27  8:24 ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Chris Wilson
@ 2014-03-30  6:48 ` Deepak S
  2 siblings, 0 replies; 13+ messages in thread
From: Deepak S @ 2014-03-30  6:48 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Deepak S, Daniel Vetter


On Thursday 27 March 2014 01:54 PM, Chris Wilson wrote:
> This reverts commit 2754436913b94626a5414d82f0996489628c513d.
>
> Conflicts:
> 	drivers/gpu/drm/i915/i915_irq.c
>
> The partial application of interrupt masking without regard to other
> pathways for adjusting the RPS frequency results in completely disabling
> the PM interrupts. This leads to excessive power consumption as the GPU
> is kept at max clocks (until the failsafe mechanism fires of explicitly
> downclocking the GPU when all requests are idle). Or equally as bad for
> the UX, the GPU is kept at minimum clocks and prevented from upclocking
> in response to a requirement for more power.
>
> Testcase: pm_rps/blocking
> Cc: Deepak S <deepak.s@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h |  3 ---
>   drivers/gpu/drm/i915/i915_irq.c | 38 --------------------------------------
>   drivers/gpu/drm/i915/intel_pm.c |  8 --------
>   3 files changed, 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cf214a4b8fdc..0eae9cd8347e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -846,9 +846,6 @@ struct intel_gen6_power_mgmt {
>   	u8 rp1_freq;		/* "less than" RP0 power/freqency */
>   	u8 rp0_freq;		/* Non-overclocked max frequency. */
>   
> -	bool rp_up_masked;
> -	bool rp_down_masked;
> -
>   	int last_adj;
>   	enum { LOW_POWER, BETWEEN, HIGH_POWER } power;
>   
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 6e37580de4bc..8df8876f557c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1098,43 +1098,6 @@ static void notify_ring(struct drm_device *dev,
>   	i915_queue_hangcheck(dev);
>   }
>   
> -void gen6_set_pm_mask(struct drm_i915_private *dev_priv,
> -			     u32 pm_iir, int new_delay)
> -{
> -	if (pm_iir & GEN6_PM_RP_UP_THRESHOLD) {
> -		if (new_delay >= dev_priv->rps.max_freq_softlimit) {
> -			/* Mask UP THRESHOLD Interrupts */
> -			I915_WRITE(GEN6_PMINTRMSK,
> -				   I915_READ(GEN6_PMINTRMSK) |
> -				   GEN6_PM_RP_UP_THRESHOLD);
> -			dev_priv->rps.rp_up_masked = true;
> -		}
> -		if (dev_priv->rps.rp_down_masked) {
> -			/* UnMask DOWN THRESHOLD Interrupts */
> -			I915_WRITE(GEN6_PMINTRMSK,
> -				   I915_READ(GEN6_PMINTRMSK) &
> -				   ~GEN6_PM_RP_DOWN_THRESHOLD);
> -			dev_priv->rps.rp_down_masked = false;
> -		}
> -	} else if (pm_iir & GEN6_PM_RP_DOWN_THRESHOLD) {
> -		if (new_delay <= dev_priv->rps.min_freq_softlimit) {
> -			/* Mask DOWN THRESHOLD Interrupts */
> -			I915_WRITE(GEN6_PMINTRMSK,
> -				   I915_READ(GEN6_PMINTRMSK) |
> -				   GEN6_PM_RP_DOWN_THRESHOLD);
> -			dev_priv->rps.rp_down_masked = true;
> -		}
> -
> -		if (dev_priv->rps.rp_up_masked) {
> -			/* UnMask UP THRESHOLD Interrupts */
> -			I915_WRITE(GEN6_PMINTRMSK,
> -				   I915_READ(GEN6_PMINTRMSK) &
> -				   ~GEN6_PM_RP_UP_THRESHOLD);
> -			dev_priv->rps.rp_up_masked = false;
> -		}
> -	}
> -}
> -
>   static void gen6_pm_rps_work(struct work_struct *work)
>   {
>   	drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t,
> @@ -1194,7 +1157,6 @@ static void gen6_pm_rps_work(struct work_struct *work)
>   			    dev_priv->rps.min_freq_softlimit,
>   			    dev_priv->rps.max_freq_softlimit);
>   
> -	gen6_set_pm_mask(dev_priv, pm_iir, new_delay);
>   	dev_priv->rps.last_adj = new_delay - dev_priv->rps.cur_freq;
>   
>   	if (IS_VALLEYVIEW(dev_priv->dev))
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 22134558c452..edf1b29d9856 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3095,11 +3095,6 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
>   	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
>   		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
>   				~VLV_GFX_CLK_FORCE_ON_BIT);
> -
> -	/* Unmask Up interrupts */
> -	dev_priv->rps.rp_up_masked = true;
> -	gen6_set_pm_mask(dev_priv, GEN6_PM_RP_DOWN_THRESHOLD,
> -						dev_priv->rps.min_freq_softlimit);
>   }
>   
>   void gen6_rps_idle(struct drm_i915_private *dev_priv)
> @@ -3694,9 +3689,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
>   
>   	valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
>   
> -	dev_priv->rps.rp_up_masked = false;
> -	dev_priv->rps.rp_down_masked = false;
> -
>   	gen6_enable_rps_interrupts(dev);
>   
>   	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);

Reviewed-by:Deepak S <deepak.s@linux.intel.com>

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

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

* Re: [PATCH] drm/i915: Mask PM/RPS interrupt generation based on activity
  2014-03-30  6:38       ` Deepak S
@ 2014-03-31  8:30         ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2014-03-31  8:30 UTC (permalink / raw)
  To: Deepak S; +Cc: intel-gfx

On Sun, Mar 30, 2014 at 12:08:35PM +0530, Deepak S wrote:
> 
> On Friday 28 March 2014 01:33 PM, Chris Wilson wrote:
> >The speculation is that we can conserve more power by masking off
> >the interrupts at source (PMINTRMSK) rather than filtering them by the
> >up/down thresholds (RPINTLIM). We can select which events we know will
> >be active based on the current frequency versus our imposed range, i.e.
> >if at minimum, we know we will not want to generate any more
> >down-interrupts and vice versa.
> >
> >v2: We only need the TIMEOUT when above min frequency.
> >v3: Tweak VLV at the same time
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Deepak S <deepak.s@linux.intel.com>
> >---
> >I see your point that I cannot remove PMINTRMSK from enable_interrupts
> >without tweaking VLV at the same time. I did consider making this 4
> >patches (factor out gen6_rps_pm_mask, tweak gen6+, tweak, vlv, remove it
> >from enable_interrupts) but decided that was just being silly and
> >squashed the two patches together instead.
> >---
> >  drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++++++++++++++++----------------
> >  1 file changed, 25 insertions(+), 16 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index 154aa07d51a7..35a7b5b65883 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -3006,6 +3006,24 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
> >  	dev_priv->rps.last_adj = 0;
> >  }
> >+static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val)
> >+{
> >+	u32 mask = 0;
> >+
> >+	if (val > dev_priv->rps.min_freq_softlimit)
> >+		mask |= GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT;
> >+	if (val < dev_priv->rps.max_freq_softlimit)
> >+		mask |= GEN6_PM_RP_UP_THRESHOLD;
> >+
> >+	/* IVB and SNB hard hangs on looping batchbuffer
> >+	 * if GEN6_PM_UP_EI_EXPIRED is masked.
> >+	 */
> >+	if (INTEL_INFO(dev_priv->dev)->gen <= 7 && !IS_HASWELL(dev_priv->dev))
> >+		mask |= GEN6_PM_RP_UP_EI_EXPIRED;
> >+
> >+	return ~mask;
> >+}
> >+
> >  /* gen6_set_rps is called to update the frequency request, but should also be
> >   * called when the range (min_delay and max_delay) is modified so that we can
> >   * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
> >@@ -3037,6 +3055,7 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
> >  	 * until we hit the minimum or maximum frequencies.
> >  	 */
> >  	I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, gen6_rps_limits(dev_priv, val));
> >+	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
> >  	POSTING_READ(GEN6_RPNSWREQ);
> >@@ -3089,6 +3108,9 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv)
> >  	I915_WRITE(VLV_GTLC_SURVIVABILITY_REG,
> >  		I915_READ(VLV_GTLC_SURVIVABILITY_REG) &
> >  				~VLV_GFX_CLK_FORCE_ON_BIT);
> >+
> >+	I915_WRITE(GEN6_PMINTRMSK,
> >+		   gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
> >  }
> >  void gen6_rps_idle(struct drm_i915_private *dev_priv)
> >@@ -3134,13 +3156,12 @@ void valleyview_set_rps(struct drm_device *dev, u8 val)
> >  			 dev_priv->rps.cur_freq,
> >  			 vlv_gpu_freq(dev_priv, val), val);
> >-	if (val == dev_priv->rps.cur_freq)
> >-		return;
> >+	if (val != dev_priv->rps.cur_freq)
> >+		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> >-	vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
> >+	I915_WRITE(GEN6_PMINTRMSK, val);
> >  	dev_priv->rps.cur_freq = val;
> >-
> >  	trace_intel_gpu_freq_change(vlv_gpu_freq(dev_priv, val));
> >  }
> >@@ -3220,24 +3241,12 @@ int intel_enable_rc6(const struct drm_device *dev)
> >  static void gen6_enable_rps_interrupts(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >-	u32 enabled_intrs;
> >  	spin_lock_irq(&dev_priv->irq_lock);
> >  	WARN_ON(dev_priv->rps.pm_iir);
> >  	snb_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
> >  	I915_WRITE(GEN6_PMIIR, dev_priv->pm_rps_events);
> >  	spin_unlock_irq(&dev_priv->irq_lock);
> >-
> >-	/* only unmask PM interrupts we need. Mask all others. */
> >-	enabled_intrs = dev_priv->pm_rps_events;
> >-
> >-	/* IVB and SNB hard hangs on looping batchbuffer
> >-	 * if GEN6_PM_UP_EI_EXPIRED is masked.
> >-	 */
> >-	if (INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev))
> >-		enabled_intrs |= GEN6_PM_RP_UP_EI_EXPIRED;
> >-
> >-	I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
> >  }
> >  static void gen8_enable_rps(struct drm_device *dev)
> 
> Reviewed-by:Deepak S <deepak.s@linux.intel.com>

All three patches merged, thanks. btw has this changed anything on the
negative effect of the boost logic you've seen on byt (iirc in video
workloads)?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

end of thread, other threads:[~2014-03-31  8:30 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-27  8:24 [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq." Chris Wilson
2014-03-27  8:24 ` [PATCH 2/3] drm/i915: Refactor gen6_set_rps Chris Wilson
2014-03-30  6:45   ` Deepak S
2014-03-27  8:24 ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Chris Wilson
2014-03-27  8:34   ` Chris Wilson
2014-03-27  8:35   ` [PATCH] drm/i915: Mask PM interrupt generation when at up/down limits for VLV Chris Wilson
2014-03-27  8:46     ` Chris Wilson
2014-03-27 14:51   ` [PATCH 3/3] drm/i915: Mask PM interrupt generation when at up/down limits Deepak S
2014-03-27 15:26     ` Chris Wilson
2014-03-28  8:03     ` [PATCH] drm/i915: Mask PM/RPS interrupt generation based on activity Chris Wilson
2014-03-30  6:38       ` Deepak S
2014-03-31  8:30         ` Daniel Vetter
2014-03-30  6:48 ` [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq." Deepak S

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox