[Intel-gfx] [PATCH 7/8] drm/ips: move drps/ips/ilk related variables into dev_priv->ips

Daniel Vetter daniel at ffwll.ch
Tue Aug 28 18:14:01 CEST 2012


On Wed, Aug 08, 2012 at 11:35:39PM +0200, Daniel Vetter wrote:
> Like with the equivalent change for gen6+ rps state, this helps in
> clarifying the code (and in fixing a few places that have fallen through
> the cracks in the locking review).
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

I've just read through the ips code again and got annoyed how things are
splattered all over. Patch applied.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h |   36 ++++++++--------
>  drivers/gpu/drm/i915/i915_irq.c |   20 ++++-----
>  drivers/gpu/drm/i915/intel_pm.c |   86 ++++++++++++++++++---------------------
>  3 files changed, 70 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d6072c0..5225784 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -808,22 +808,26 @@ typedef struct drm_i915_private {
>  		u8 max_delay;
>  	} rps;
>  
> -
> -	u8 cur_delay;
> -	u8 min_delay;
> -	u8 max_delay;
> -	u8 fmax;
> -	u8 fstart;
> -
> -	u64 last_count1;
> -	unsigned long last_time1;
> -	unsigned long chipset_power;
> -	u64 last_count2;
> -	struct timespec last_time2;
> -	unsigned long gfx_power;
> -	int c_m;
> -	int r_t;
> -	u8 corr;
> +	/* ilk-only ips/rps state. Everything in here is protected by the global
> +	 * mchdev_lock in intel_pm.c */
> +	struct {
> +		u8 cur_delay;
> +		u8 min_delay;
> +		u8 max_delay;
> +		u8 fmax;
> +		u8 fstart;
> +
> +		u64 last_count1;
> +		unsigned long last_time1;
> +		unsigned long chipset_power;
> +		u64 last_count2;
> +		struct timespec last_time2;
> +		unsigned long gfx_power;
> +		u8 corr;
> +
> +		int c_m;
> +		int r_t;
> +	} ips;
>  
>  	enum no_fbc_reason no_fbc_reason;
>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d15ea50..135a84d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -310,7 +310,7 @@ static void ironlake_handle_rps_change(struct drm_device *dev)
>  
>  	I915_WRITE16(MEMINTRSTS, I915_READ(MEMINTRSTS));
>  
> -	new_delay = dev_priv->cur_delay;
> +	new_delay = dev_priv->ips.cur_delay;
>  
>  	I915_WRITE16(MEMINTRSTS, MEMINT_EVAL_CHG);
>  	busy_up = I915_READ(RCPREVBSYTUPAVG);
> @@ -320,19 +320,19 @@ static void ironlake_handle_rps_change(struct drm_device *dev)
>  
>  	/* Handle RCS change request from hw */
>  	if (busy_up > max_avg) {
> -		if (dev_priv->cur_delay != dev_priv->max_delay)
> -			new_delay = dev_priv->cur_delay - 1;
> -		if (new_delay < dev_priv->max_delay)
> -			new_delay = dev_priv->max_delay;
> +		if (dev_priv->ips.cur_delay != dev_priv->ips.max_delay)
> +			new_delay = dev_priv->ips.cur_delay - 1;
> +		if (new_delay < dev_priv->ips.max_delay)
> +			new_delay = dev_priv->ips.max_delay;
>  	} else if (busy_down < min_avg) {
> -		if (dev_priv->cur_delay != dev_priv->min_delay)
> -			new_delay = dev_priv->cur_delay + 1;
> -		if (new_delay > dev_priv->min_delay)
> -			new_delay = dev_priv->min_delay;
> +		if (dev_priv->ips.cur_delay != dev_priv->ips.min_delay)
> +			new_delay = dev_priv->ips.cur_delay + 1;
> +		if (new_delay > dev_priv->ips.min_delay)
> +			new_delay = dev_priv->ips.min_delay;
>  	}
>  
>  	if (ironlake_set_drps(dev, new_delay))
> -		dev_priv->cur_delay = new_delay;
> +		dev_priv->ips.cur_delay = new_delay;
>  
>  	spin_unlock_irqrestore(&mchdev_lock, flags);
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5f9f93e..eff0753 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -593,7 +593,7 @@ static void i915_ironlake_get_mem_freq(struct drm_device *dev)
>  		break;
>  	}
>  
> -	dev_priv->r_t = dev_priv->mem_freq;
> +	dev_priv->ips.r_t = dev_priv->mem_freq;
>  
>  	switch (csipll & 0x3ff) {
>  	case 0x00c:
> @@ -625,11 +625,11 @@ static void i915_ironlake_get_mem_freq(struct drm_device *dev)
>  	}
>  
>  	if (dev_priv->fsb_freq == 3200) {
> -		dev_priv->c_m = 0;
> +		dev_priv->ips.c_m = 0;
>  	} else if (dev_priv->fsb_freq > 3200 && dev_priv->fsb_freq <= 4800) {
> -		dev_priv->c_m = 1;
> +		dev_priv->ips.c_m = 1;
>  	} else {
> -		dev_priv->c_m = 2;
> +		dev_priv->ips.c_m = 2;
>  	}
>  }
>  
> @@ -2162,12 +2162,6 @@ err_unref:
>  
>  /**
>   * Lock protecting IPS related data structures
> - *   - i915_mch_dev
> - *   - dev_priv->max_delay
> - *   - dev_priv->min_delay
> - *   - dev_priv->fmax
> - *   - dev_priv->gpu_busy
> - *   - dev_priv->gfx_power
>   */
>  DEFINE_SPINLOCK(mchdev_lock);
>  
> @@ -2230,12 +2224,12 @@ static void ironlake_enable_drps(struct drm_device *dev)
>  	vstart = (I915_READ(PXVFREQ_BASE + (fstart * 4)) & PXVFREQ_PX_MASK) >>
>  		PXVFREQ_PX_SHIFT;
>  
> -	dev_priv->fmax = fmax; /* IPS callback will increase this */
> -	dev_priv->fstart = fstart;
> +	dev_priv->ips.fmax = fmax; /* IPS callback will increase this */
> +	dev_priv->ips.fstart = fstart;
>  
> -	dev_priv->max_delay = fstart;
> -	dev_priv->min_delay = fmin;
> -	dev_priv->cur_delay = fstart;
> +	dev_priv->ips.max_delay = fstart;
> +	dev_priv->ips.min_delay = fmin;
> +	dev_priv->ips.cur_delay = fstart;
>  
>  	DRM_DEBUG_DRIVER("fmax: %d, fmin: %d, fstart: %d\n",
>  			 fmax, fmin, fstart);
> @@ -2258,11 +2252,11 @@ static void ironlake_enable_drps(struct drm_device *dev)
>  
>  	ironlake_set_drps(dev, fstart);
>  
> -	dev_priv->last_count1 = I915_READ(0x112e4) + I915_READ(0x112e8) +
> +	dev_priv->ips.last_count1 = I915_READ(0x112e4) + I915_READ(0x112e8) +
>  		I915_READ(0x112e0);
> -	dev_priv->last_time1 = jiffies_to_msecs(jiffies);
> -	dev_priv->last_count2 = I915_READ(0x112f4);
> -	getrawmonotonic(&dev_priv->last_time2);
> +	dev_priv->ips.last_time1 = jiffies_to_msecs(jiffies);
> +	dev_priv->ips.last_count2 = I915_READ(0x112f4);
> +	getrawmonotonic(&dev_priv->ips.last_time2);
>  
>  	spin_unlock_irq(&mchdev_lock);
>  }
> @@ -2284,7 +2278,7 @@ static void ironlake_disable_drps(struct drm_device *dev)
>  	I915_WRITE(DEIMR, I915_READ(DEIMR) | DE_PCU_EVENT);
>  
>  	/* Go back to the starting frequency */
> -	ironlake_set_drps(dev, dev_priv->fstart);
> +	ironlake_set_drps(dev, dev_priv->ips.fstart);
>  	mdelay(1);
>  	rgvswctl |= MEMCTL_CMD_STS;
>  	I915_WRITE(MEMSWCTL, rgvswctl);
> @@ -2748,7 +2742,7 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>  
>  	assert_spin_locked(&mchdev_lock);
>  
> -	diff1 = now - dev_priv->last_time1;
> +	diff1 = now - dev_priv->ips.last_time1;
>  
>  	/* Prevent division-by-zero if we are asking too fast.
>  	 * Also, we don't get interesting results if we are polling
> @@ -2756,7 +2750,7 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>  	 * in such cases.
>  	 */
>  	if (diff1 <= 10)
> -		return dev_priv->chipset_power;
> +		return dev_priv->ips.chipset_power;
>  
>  	count1 = I915_READ(DMIEC);
>  	count2 = I915_READ(DDREC);
> @@ -2765,16 +2759,16 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>  	total_count = count1 + count2 + count3;
>  
>  	/* FIXME: handle per-counter overflow */
> -	if (total_count < dev_priv->last_count1) {
> -		diff = ~0UL - dev_priv->last_count1;
> +	if (total_count < dev_priv->ips.last_count1) {
> +		diff = ~0UL - dev_priv->ips.last_count1;
>  		diff += total_count;
>  	} else {
> -		diff = total_count - dev_priv->last_count1;
> +		diff = total_count - dev_priv->ips.last_count1;
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(cparams); i++) {
> -		if (cparams[i].i == dev_priv->c_m &&
> -		    cparams[i].t == dev_priv->r_t) {
> +		if (cparams[i].i == dev_priv->ips.c_m &&
> +		    cparams[i].t == dev_priv->ips.r_t) {
>  			m = cparams[i].m;
>  			c = cparams[i].c;
>  			break;
> @@ -2785,10 +2779,10 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
>  	ret = ((m * diff) + c);
>  	ret = div_u64(ret, 10);
>  
> -	dev_priv->last_count1 = total_count;
> -	dev_priv->last_time1 = now;
> +	dev_priv->ips.last_count1 = total_count;
> +	dev_priv->ips.last_time1 = now;
>  
> -	dev_priv->chipset_power = ret;
> +	dev_priv->ips.chipset_power = ret;
>  
>  	return ret;
>  }
> @@ -2959,7 +2953,7 @@ static void __i915_update_gfx_val(struct drm_i915_private *dev_priv)
>  	assert_spin_locked(&mchdev_lock);
>  
>  	getrawmonotonic(&now);
> -	diff1 = timespec_sub(now, dev_priv->last_time2);
> +	diff1 = timespec_sub(now, dev_priv->ips.last_time2);
>  
>  	/* Don't divide by 0 */
>  	diffms = diff1.tv_sec * 1000 + diff1.tv_nsec / 1000000;
> @@ -2968,20 +2962,20 @@ static void __i915_update_gfx_val(struct drm_i915_private *dev_priv)
>  
>  	count = I915_READ(GFXEC);
>  
> -	if (count < dev_priv->last_count2) {
> -		diff = ~0UL - dev_priv->last_count2;
> +	if (count < dev_priv->ips.last_count2) {
> +		diff = ~0UL - dev_priv->ips.last_count2;
>  		diff += count;
>  	} else {
> -		diff = count - dev_priv->last_count2;
> +		diff = count - dev_priv->ips.last_count2;
>  	}
>  
> -	dev_priv->last_count2 = count;
> -	dev_priv->last_time2 = now;
> +	dev_priv->ips.last_count2 = count;
> +	dev_priv->ips.last_time2 = now;
>  
>  	/* More magic constants... */
>  	diff = diff * 1181;
>  	diff = div_u64(diff, diffms * 10);
> -	dev_priv->gfx_power = diff;
> +	dev_priv->ips.gfx_power = diff;
>  }
>  
>  void i915_update_gfx_val(struct drm_i915_private *dev_priv)
> @@ -3023,14 +3017,14 @@ unsigned long i915_gfx_val(struct drm_i915_private *dev_priv)
>  
>  	corr = corr * ((150142 * state1) / 10000 - 78642);
>  	corr /= 100000;
> -	corr2 = (corr * dev_priv->corr);
> +	corr2 = (corr * dev_priv->ips.corr);
>  
>  	state2 = (corr2 * state1) / 10000;
>  	state2 /= 100; /* convert to mW */
>  
>  	__i915_update_gfx_val(dev_priv);
>  
> -	return dev_priv->gfx_power + state2;
> +	return dev_priv->ips.gfx_power + state2;
>  }
>  
>  /**
> @@ -3078,8 +3072,8 @@ bool i915_gpu_raise(void)
>  	}
>  	dev_priv = i915_mch_dev;
>  
> -	if (dev_priv->max_delay > dev_priv->fmax)
> -		dev_priv->max_delay--;
> +	if (dev_priv->ips.max_delay > dev_priv->ips.fmax)
> +		dev_priv->ips.max_delay--;
>  
>  out_unlock:
>  	spin_unlock_irq(&mchdev_lock);
> @@ -3106,8 +3100,8 @@ bool i915_gpu_lower(void)
>  	}
>  	dev_priv = i915_mch_dev;
>  
> -	if (dev_priv->max_delay < dev_priv->min_delay)
> -		dev_priv->max_delay++;
> +	if (dev_priv->ips.max_delay < dev_priv->ips.min_delay)
> +		dev_priv->ips.max_delay++;
>  
>  out_unlock:
>  	spin_unlock_irq(&mchdev_lock);
> @@ -3161,9 +3155,9 @@ bool i915_gpu_turbo_disable(void)
>  	}
>  	dev_priv = i915_mch_dev;
>  
> -	dev_priv->max_delay = dev_priv->fstart;
> +	dev_priv->ips.max_delay = dev_priv->ips.fstart;
>  
> -	if (!ironlake_set_drps(dev_priv->dev, dev_priv->fstart))
> +	if (!ironlake_set_drps(dev_priv->dev, dev_priv->ips.fstart))
>  		ret = false;
>  
>  out_unlock:
> @@ -3276,7 +3270,7 @@ static void intel_init_emon(struct drm_device *dev)
>  
>  	lcfuse = I915_READ(LCFUSE02);
>  
> -	dev_priv->corr = (lcfuse & LCFUSE_HIV_MASK);
> +	dev_priv->ips.corr = (lcfuse & LCFUSE_HIV_MASK);
>  }
>  
>  void intel_disable_gt_powersave(struct drm_device *dev)
> -- 
> 1.7.10.4
> 

-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48



More information about the Intel-gfx mailing list