Commit 9270388e authored by Daniel Vetter's avatar Daniel Vetter

drm/i915: fix up ilk drps/ips locking

We change the drps/ips sw/hw state from different callers: Our own irq
handler, the external intel-ips module and from process context. Most
of these callers don't take any lock at all.

Protect everything by making the mchdev_lock irqsave and grabbing it in
all relevant callsites. Note that we have to convert a few sleeps in the
drps enable/disable code to delays, but alas, I'm not volunteering to
restructure the code around a few work items.

For paranoia add a spin_locked assert to ironlake_set_drps, too.

v2: Move one access inside the lock protection. Caught by the
dev_priv->ips mass-rename ...

v3: Resolve rebase conflict.
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 73edd18f
...@@ -296,14 +296,22 @@ static void i915_hotplug_work_func(struct work_struct *work) ...@@ -296,14 +296,22 @@ static void i915_hotplug_work_func(struct work_struct *work)
drm_helper_hpd_irq_event(dev); drm_helper_hpd_irq_event(dev);
} }
/* defined intel_pm.c */
extern spinlock_t mchdev_lock;
static void ironlake_handle_rps_change(struct drm_device *dev) static void ironlake_handle_rps_change(struct drm_device *dev)
{ {
drm_i915_private_t *dev_priv = dev->dev_private; drm_i915_private_t *dev_priv = dev->dev_private;
u32 busy_up, busy_down, max_avg, min_avg; u32 busy_up, busy_down, max_avg, min_avg;
u8 new_delay = dev_priv->cur_delay; u8 new_delay;
unsigned long flags;
spin_lock_irqsave(&mchdev_lock, flags);
I915_WRITE16(MEMINTRSTS, I915_READ(MEMINTRSTS)); I915_WRITE16(MEMINTRSTS, I915_READ(MEMINTRSTS));
new_delay = dev_priv->cur_delay;
I915_WRITE16(MEMINTRSTS, MEMINT_EVAL_CHG); I915_WRITE16(MEMINTRSTS, MEMINT_EVAL_CHG);
busy_up = I915_READ(RCPREVBSYTUPAVG); busy_up = I915_READ(RCPREVBSYTUPAVG);
busy_down = I915_READ(RCPREVBSYTDNAVG); busy_down = I915_READ(RCPREVBSYTDNAVG);
...@@ -326,6 +334,8 @@ static void ironlake_handle_rps_change(struct drm_device *dev) ...@@ -326,6 +334,8 @@ static void ironlake_handle_rps_change(struct drm_device *dev)
if (ironlake_set_drps(dev, new_delay)) if (ironlake_set_drps(dev, new_delay))
dev_priv->cur_delay = new_delay; dev_priv->cur_delay = new_delay;
spin_unlock_irqrestore(&mchdev_lock, flags);
return; return;
} }
......
...@@ -2160,11 +2160,28 @@ intel_alloc_context_page(struct drm_device *dev) ...@@ -2160,11 +2160,28 @@ intel_alloc_context_page(struct drm_device *dev)
return NULL; return NULL;
} }
/**
* 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);
/* Global for IPS driver to get at the current i915 device. Protected by
* mchdev_lock. */
static struct drm_i915_private *i915_mch_dev;
bool ironlake_set_drps(struct drm_device *dev, u8 val) bool ironlake_set_drps(struct drm_device *dev, u8 val)
{ {
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
u16 rgvswctl; u16 rgvswctl;
assert_spin_locked(&mchdev_lock);
rgvswctl = I915_READ16(MEMSWCTL); rgvswctl = I915_READ16(MEMSWCTL);
if (rgvswctl & MEMCTL_CMD_STS) { if (rgvswctl & MEMCTL_CMD_STS) {
DRM_DEBUG("gpu busy, RCS change rejected\n"); DRM_DEBUG("gpu busy, RCS change rejected\n");
...@@ -2188,6 +2205,8 @@ static void ironlake_enable_drps(struct drm_device *dev) ...@@ -2188,6 +2205,8 @@ static void ironlake_enable_drps(struct drm_device *dev)
u32 rgvmodectl = I915_READ(MEMMODECTL); u32 rgvmodectl = I915_READ(MEMMODECTL);
u8 fmax, fmin, fstart, vstart; u8 fmax, fmin, fstart, vstart;
spin_lock_irq(&mchdev_lock);
/* Enable temp reporting */ /* Enable temp reporting */
I915_WRITE16(PMMISC, I915_READ(PMMISC) | MCPPCE_EN); I915_WRITE16(PMMISC, I915_READ(PMMISC) | MCPPCE_EN);
I915_WRITE16(TSC1, I915_READ(TSC1) | TSE); I915_WRITE16(TSC1, I915_READ(TSC1) | TSE);
...@@ -2233,9 +2252,9 @@ static void ironlake_enable_drps(struct drm_device *dev) ...@@ -2233,9 +2252,9 @@ static void ironlake_enable_drps(struct drm_device *dev)
rgvmodectl |= MEMMODE_SWMODE_EN; rgvmodectl |= MEMMODE_SWMODE_EN;
I915_WRITE(MEMMODECTL, rgvmodectl); I915_WRITE(MEMMODECTL, rgvmodectl);
if (wait_for((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10)) if (wait_for_atomic((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10))
DRM_ERROR("stuck trying to change perf mode\n"); DRM_ERROR("stuck trying to change perf mode\n");
msleep(1); mdelay(1);
ironlake_set_drps(dev, fstart); ironlake_set_drps(dev, fstart);
...@@ -2244,12 +2263,18 @@ static void ironlake_enable_drps(struct drm_device *dev) ...@@ -2244,12 +2263,18 @@ static void ironlake_enable_drps(struct drm_device *dev)
dev_priv->last_time1 = jiffies_to_msecs(jiffies); dev_priv->last_time1 = jiffies_to_msecs(jiffies);
dev_priv->last_count2 = I915_READ(0x112f4); dev_priv->last_count2 = I915_READ(0x112f4);
getrawmonotonic(&dev_priv->last_time2); getrawmonotonic(&dev_priv->last_time2);
spin_unlock_irq(&mchdev_lock);
} }
static void ironlake_disable_drps(struct drm_device *dev) static void ironlake_disable_drps(struct drm_device *dev)
{ {
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
u16 rgvswctl = I915_READ16(MEMSWCTL); u16 rgvswctl;
spin_lock_irq(&mchdev_lock);
rgvswctl = I915_READ16(MEMSWCTL);
/* Ack interrupts, disable EFC interrupt */ /* Ack interrupts, disable EFC interrupt */
I915_WRITE(MEMINTREN, I915_READ(MEMINTREN) & ~MEMINT_EVAL_CHG_EN); I915_WRITE(MEMINTREN, I915_READ(MEMINTREN) & ~MEMINT_EVAL_CHG_EN);
...@@ -2260,11 +2285,12 @@ static void ironlake_disable_drps(struct drm_device *dev) ...@@ -2260,11 +2285,12 @@ static void ironlake_disable_drps(struct drm_device *dev)
/* Go back to the starting frequency */ /* Go back to the starting frequency */
ironlake_set_drps(dev, dev_priv->fstart); ironlake_set_drps(dev, dev_priv->fstart);
msleep(1); mdelay(1);
rgvswctl |= MEMCTL_CMD_STS; rgvswctl |= MEMCTL_CMD_STS;
I915_WRITE(MEMSWCTL, rgvswctl); I915_WRITE(MEMSWCTL, rgvswctl);
msleep(1); mdelay(1);
spin_unlock_irq(&mchdev_lock);
} }
/* There's a funny hw issue where the hw returns all 0 when reading from /* There's a funny hw issue where the hw returns all 0 when reading from
...@@ -2713,21 +2739,6 @@ static const struct cparams { ...@@ -2713,21 +2739,6 @@ static const struct cparams {
{ 0, 800, 231, 23784 }, { 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) unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
{ {
u64 total_count, diff, ret; u64 total_count, diff, ret;
...@@ -2978,11 +2989,11 @@ void i915_update_gfx_val(struct drm_i915_private *dev_priv) ...@@ -2978,11 +2989,11 @@ void i915_update_gfx_val(struct drm_i915_private *dev_priv)
if (dev_priv->info->gen != 5) if (dev_priv->info->gen != 5)
return; return;
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
__i915_update_gfx_val(dev_priv); __i915_update_gfx_val(dev_priv);
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
} }
unsigned long i915_gfx_val(struct drm_i915_private *dev_priv) unsigned long i915_gfx_val(struct drm_i915_private *dev_priv)
...@@ -3033,7 +3044,7 @@ unsigned long i915_read_mch_val(void) ...@@ -3033,7 +3044,7 @@ unsigned long i915_read_mch_val(void)
struct drm_i915_private *dev_priv; struct drm_i915_private *dev_priv;
unsigned long chipset_val, graphics_val, ret = 0; unsigned long chipset_val, graphics_val, ret = 0;
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) if (!i915_mch_dev)
goto out_unlock; goto out_unlock;
dev_priv = i915_mch_dev; dev_priv = i915_mch_dev;
...@@ -3044,7 +3055,7 @@ unsigned long i915_read_mch_val(void) ...@@ -3044,7 +3055,7 @@ unsigned long i915_read_mch_val(void)
ret = chipset_val + graphics_val; ret = chipset_val + graphics_val;
out_unlock: out_unlock:
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
return ret; return ret;
} }
...@@ -3060,7 +3071,7 @@ bool i915_gpu_raise(void) ...@@ -3060,7 +3071,7 @@ bool i915_gpu_raise(void)
struct drm_i915_private *dev_priv; struct drm_i915_private *dev_priv;
bool ret = true; bool ret = true;
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) { if (!i915_mch_dev) {
ret = false; ret = false;
goto out_unlock; goto out_unlock;
...@@ -3071,7 +3082,7 @@ bool i915_gpu_raise(void) ...@@ -3071,7 +3082,7 @@ bool i915_gpu_raise(void)
dev_priv->max_delay--; dev_priv->max_delay--;
out_unlock: out_unlock:
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
return ret; return ret;
} }
...@@ -3088,7 +3099,7 @@ bool i915_gpu_lower(void) ...@@ -3088,7 +3099,7 @@ bool i915_gpu_lower(void)
struct drm_i915_private *dev_priv; struct drm_i915_private *dev_priv;
bool ret = true; bool ret = true;
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) { if (!i915_mch_dev) {
ret = false; ret = false;
goto out_unlock; goto out_unlock;
...@@ -3099,7 +3110,7 @@ bool i915_gpu_lower(void) ...@@ -3099,7 +3110,7 @@ bool i915_gpu_lower(void)
dev_priv->max_delay++; dev_priv->max_delay++;
out_unlock: out_unlock:
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
return ret; return ret;
} }
...@@ -3117,7 +3128,7 @@ bool i915_gpu_busy(void) ...@@ -3117,7 +3128,7 @@ bool i915_gpu_busy(void)
bool ret = false; bool ret = false;
int i; int i;
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) if (!i915_mch_dev)
goto out_unlock; goto out_unlock;
dev_priv = i915_mch_dev; dev_priv = i915_mch_dev;
...@@ -3126,7 +3137,7 @@ bool i915_gpu_busy(void) ...@@ -3126,7 +3137,7 @@ bool i915_gpu_busy(void)
ret |= !list_empty(&ring->request_list); ret |= !list_empty(&ring->request_list);
out_unlock: out_unlock:
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
return ret; return ret;
} }
...@@ -3143,7 +3154,7 @@ bool i915_gpu_turbo_disable(void) ...@@ -3143,7 +3154,7 @@ bool i915_gpu_turbo_disable(void)
struct drm_i915_private *dev_priv; struct drm_i915_private *dev_priv;
bool ret = true; bool ret = true;
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) { if (!i915_mch_dev) {
ret = false; ret = false;
goto out_unlock; goto out_unlock;
...@@ -3156,7 +3167,7 @@ bool i915_gpu_turbo_disable(void) ...@@ -3156,7 +3167,7 @@ bool i915_gpu_turbo_disable(void)
ret = false; ret = false;
out_unlock: out_unlock:
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
return ret; return ret;
} }
...@@ -3186,18 +3197,18 @@ void intel_gpu_ips_init(struct drm_i915_private *dev_priv) ...@@ -3186,18 +3197,18 @@ void intel_gpu_ips_init(struct drm_i915_private *dev_priv)
{ {
/* We only register the i915 ips part with intel-ips once everything is /* 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. */ * set up, to avoid intel-ips sneaking in and reading bogus values. */
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
i915_mch_dev = dev_priv; i915_mch_dev = dev_priv;
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
ips_ping_for_i915_load(); ips_ping_for_i915_load();
} }
void intel_gpu_ips_teardown(void) void intel_gpu_ips_teardown(void)
{ {
spin_lock(&mchdev_lock); spin_lock_irq(&mchdev_lock);
i915_mch_dev = NULL; i915_mch_dev = NULL;
spin_unlock(&mchdev_lock); spin_unlock_irq(&mchdev_lock);
} }
static void intel_init_emon(struct drm_device *dev) static void intel_init_emon(struct drm_device *dev)
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment