Commit 02d71956 authored by Daniel Vetter's avatar Daniel Vetter
Browse files

drm/i915: properly guard ilk ips state 



The update_gfx_val function called from mark_busy wasn't taking the
mchdev_lock, as it should have. Also sprinkle a few spinlock asserts
over the code to document things better.

Things are still rather confusing, especially since a few variables
in dev_priv are used by both the gen6+ rps code and the ilk ips code.
But protected by totally different locks. Follow-on patches will clean
that up.

v2: Don't add a deadlock ... hence split up update_gfx_val into a
wrapper that grabs the lock and an internal __ variant for callsites
within intel_pm.c that already have taken the lock.

v3: Mark the internal helper as static, noticed by Ben Widawsky.

v4: Damien Lespiau had questions about the safety of the ips setup
sequence, explain in a comment why it works.

Reviewed-by: default avatarDamien Lespiau <damien.lespiau@intel.com>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-Off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 745ca3be

drivers/gpu/drm/i915/intel_pm.c

+36 −16
Original line number Diff line number Diff line
@@ -2710,6 +2710,21 @@ static const struct cparams { 
	{ 0, 800, 231, 23784 },

};



/**

 * 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

 */

static DEFINE_SPINLOCK(mchdev_lock);



/* Global for IPS driver to get at the current i915 device. Protected by

 * mchdev_lock. */

static struct drm_i915_private *i915_mch_dev;



unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)

{

	u64 total_count, diff, ret;
@@ -2717,6 +2732,8 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv) 
	unsigned long now = jiffies_to_msecs(jiffies), diff1;

	int i;



	assert_spin_locked(&mchdev_lock);



	diff1 = now - dev_priv->last_time1;



	/* Prevent division-by-zero if we are asking too fast.
@@ -2918,15 +2935,14 @@ static u16 pvid_to_extvid(struct drm_i915_private *dev_priv, u8 pxvid) 
		return v_table[pxvid].vd;

}



void i915_update_gfx_val(struct drm_i915_private *dev_priv)

static void __i915_update_gfx_val(struct drm_i915_private *dev_priv)

{

	struct timespec now, diff1;

	u64 diff;

	unsigned long diffms;

	u32 count;



	if (dev_priv->info->gen != 5)

		return;

	assert_spin_locked(&mchdev_lock);



	getrawmonotonic(&now);

	diff1 = timespec_sub(now, dev_priv->last_time2);
@@ -2954,11 +2970,25 @@ void i915_update_gfx_val(struct drm_i915_private *dev_priv) 
	dev_priv->gfx_power = diff;

}



void i915_update_gfx_val(struct drm_i915_private *dev_priv)

{

	if (dev_priv->info->gen != 5)

		return;



	spin_lock(&mchdev_lock);



	__i915_update_gfx_val(dev_priv);



	spin_unlock(&mchdev_lock);

}



unsigned long i915_gfx_val(struct drm_i915_private *dev_priv)

{

	unsigned long t, corr, state1, corr2, state2;

	u32 pxvid, ext_v;



	assert_spin_locked(&mchdev_lock);



	pxvid = I915_READ(PXVFREQ_BASE + (dev_priv->cur_delay * 4));

	pxvid = (pxvid >> 24) & 0x7f;

	ext_v = pvid_to_extvid(dev_priv, pxvid);
@@ -2984,23 +3014,11 @@ unsigned long i915_gfx_val(struct drm_i915_private *dev_priv) 
	state2 = (corr2 * state1) / 10000;

	state2 /= 100; /* convert to mW */



	i915_update_gfx_val(dev_priv);

	__i915_update_gfx_val(dev_priv);



	return dev_priv->gfx_power + state2;

}



/* Global for IPS driver to get at the current i915 device */

static struct drm_i915_private *i915_mch_dev;

/*

 * Lock protecting IPS related data structures

 *   - i915_mch_dev

 *   - dev_priv->max_delay

 *   - dev_priv->min_delay

 *   - dev_priv->fmax

 *   - dev_priv->gpu_busy

 */

static DEFINE_SPINLOCK(mchdev_lock);



/**

 * i915_read_mch_val - return value for IPS use

 *
@@ -3163,6 +3181,8 @@ ips_ping_for_i915_load(void) 


void intel_gpu_ips_init(struct drm_i915_private *dev_priv)

{

	/* We only register the i915 ips part with intel-ips once everything is

	 * set up, to avoid intel-ips sneaking in and reading bogus values. */

	spin_lock(&mchdev_lock);

	i915_mch_dev = dev_priv;

	dev_priv->mchdev_lock = &mchdev_lock;

CRA Git | Maintained and supported by SUSTech CRA and CCSE