Re: [PATCH 1/3] Revert "drm/i915: Disable/Enable PM Intrrupts based on the current freq."

[Deepak S <deepak.s@linux.intel.com>]

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