From: Daniel Vetter <daniel@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: Re: [PATCH 7/8] drm/ips: move drps/ips/ilk related variables into dev_priv->ips
Date: Tue, 28 Aug 2012 18:14:01 +0200 [thread overview]
Message-ID: <20120828161401.GG5125@phenom.ffwll.local> (raw)
In-Reply-To: <1344461740-1231-8-git-send-email-daniel.vetter@ffwll.ch>
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@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@ffwll.ch
Mobile: +41 (0)79 365 57 48
next prev parent reply other threads:[~2012-08-28 16:13 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-08 21:35 [PATCH 0/8] rps locking fixes v2 Daniel Vetter
2012-08-08 21:35 ` [PATCH 1/8] drm/i915: properly guard ilk ips state Daniel Vetter
2012-08-09 14:44 ` [PATCH] " Daniel Vetter
2012-08-08 21:35 ` [PATCH 2/8] drm/i915: fixup up debugfs rps state handling Daniel Vetter
2012-08-09 9:32 ` Chris Wilson
2012-08-09 13:07 ` [PATCH 1/2] " Daniel Vetter
2012-08-09 13:07 ` [PATCH 2/2] drm/i915: use mutex_lock_interruptible for debugfs files Daniel Vetter
2012-08-09 20:18 ` Ben Widawsky
2012-08-08 21:35 ` [PATCH 3/8] drm/i915: move all rps state into dev_priv->rps Daniel Vetter
2012-08-08 21:35 ` [PATCH 4/8] drm/i915: kill dev_priv->mchdev_lock Daniel Vetter
2012-08-08 21:35 ` [PATCH 5/8] drm/i915: DE_PCU_EVENT irq is ilk-only Daniel Vetter
2012-08-08 21:35 ` [PATCH 6/8] drm/i915: fix up ilk drps/ips locking Daniel Vetter
2012-08-09 14:46 ` [PATCH] " Daniel Vetter
2012-08-08 21:35 ` [PATCH 7/8] drm/ips: move drps/ips/ilk related variables into dev_priv->ips Daniel Vetter
2012-08-28 16:14 ` Daniel Vetter [this message]
2012-08-08 21:35 ` [PATCH 8/8] drm/i915: enable rc6 on ilk again Daniel Vetter
2012-08-09 20:26 ` Ben Widawsky
2012-08-09 9:43 ` [PATCH 0/8] rps locking fixes v2 Chris Wilson
2012-08-09 11:48 ` Daniel Vetter
2012-08-09 14:58 ` Lespiau, Damien
2012-08-09 20:36 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120828161401.GG5125@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.