From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 7/9] drm/i915: fix up ilk drps/ips locking
Date: Tue, 24 Jul 2012 23:33:48 +0200 [thread overview]
Message-ID: <1343165630-21604-8-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1343165630-21604-1-git-send-email-daniel.vetter@ffwll.ch>
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 ...
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_irq.c | 12 +++++-
drivers/gpu/drm/i915/intel_pm.c | 83 ++++++++++++++++++++++-----------------
2 files changed, 58 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f46fa6c..75c631d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -296,14 +296,22 @@ static void i915_hotplug_work_func(struct work_struct *work)
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)
{
drm_i915_private_t *dev_priv = dev->dev_private;
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));
+ new_delay = dev_priv->cur_delay;
+
I915_WRITE16(MEMINTRSTS, MEMINT_EVAL_CHG);
busy_up = I915_READ(RCPREVBSYTUPAVG);
busy_down = I915_READ(RCPREVBSYTDNAVG);
@@ -326,6 +334,8 @@ static void ironlake_handle_rps_change(struct drm_device *dev)
if (ironlake_set_drps(dev, new_delay))
dev_priv->cur_delay = new_delay;
+ spin_unlock_irqrestore(&mchdev_lock, flags);
+
return;
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b5a08b0..e582b50 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2160,11 +2160,28 @@ err_unref:
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)
{
struct drm_i915_private *dev_priv = dev->dev_private;
u16 rgvswctl;
+ assert_spin_locked(&mchdev_lock);
+
rgvswctl = I915_READ16(MEMSWCTL);
if (rgvswctl & MEMCTL_CMD_STS) {
DRM_DEBUG("gpu busy, RCS change rejected\n");
@@ -2188,6 +2205,8 @@ static void ironlake_enable_drps(struct drm_device *dev)
u32 rgvmodectl = I915_READ(MEMMODECTL);
u8 fmax, fmin, fstart, vstart;
+ spin_lock_irq(&mchdev_lock);
+
/* Enable temp reporting */
I915_WRITE16(PMMISC, I915_READ(PMMISC) | MCPPCE_EN);
I915_WRITE16(TSC1, I915_READ(TSC1) | TSE);
@@ -2233,9 +2252,9 @@ static void ironlake_enable_drps(struct drm_device *dev)
rgvmodectl |= MEMMODE_SWMODE_EN;
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");
- msleep(1);
+ mdelay(1);
ironlake_set_drps(dev, fstart);
@@ -2244,12 +2263,18 @@ static void ironlake_enable_drps(struct drm_device *dev)
dev_priv->last_time1 = jiffies_to_msecs(jiffies);
dev_priv->last_count2 = I915_READ(0x112f4);
getrawmonotonic(&dev_priv->last_time2);
+
+ spin_unlock_irq(&mchdev_lock);
}
static void ironlake_disable_drps(struct drm_device *dev)
{
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 */
I915_WRITE(MEMINTREN, I915_READ(MEMINTREN) & ~MEMINT_EVAL_CHG_EN);
@@ -2260,11 +2285,12 @@ static void ironlake_disable_drps(struct drm_device *dev)
/* Go back to the starting frequency */
ironlake_set_drps(dev, dev_priv->fstart);
- msleep(1);
+ mdelay(1);
rgvswctl |= MEMCTL_CMD_STS;
I915_WRITE(MEMSWCTL, rgvswctl);
- msleep(1);
+ mdelay(1);
+ spin_unlock_irq(&mchdev_lock);
}
void gen6_set_rps(struct drm_device *dev, u8 val)
@@ -2695,21 +2721,6 @@ 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;
@@ -2960,11 +2971,11 @@ void i915_update_gfx_val(struct drm_i915_private *dev_priv)
if (dev_priv->info->gen != 5)
return;
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
__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)
@@ -3015,7 +3026,7 @@ unsigned long i915_read_mch_val(void)
struct drm_i915_private *dev_priv;
unsigned long chipset_val, graphics_val, ret = 0;
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev)
goto out_unlock;
dev_priv = i915_mch_dev;
@@ -3026,7 +3037,7 @@ unsigned long i915_read_mch_val(void)
ret = chipset_val + graphics_val;
out_unlock:
- spin_unlock(&mchdev_lock);
+ spin_unlock_irq(&mchdev_lock);
return ret;
}
@@ -3042,7 +3053,7 @@ bool i915_gpu_raise(void)
struct drm_i915_private *dev_priv;
bool ret = true;
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) {
ret = false;
goto out_unlock;
@@ -3053,7 +3064,7 @@ bool i915_gpu_raise(void)
dev_priv->max_delay--;
out_unlock:
- spin_unlock(&mchdev_lock);
+ spin_unlock_irq(&mchdev_lock);
return ret;
}
@@ -3070,7 +3081,7 @@ bool i915_gpu_lower(void)
struct drm_i915_private *dev_priv;
bool ret = true;
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) {
ret = false;
goto out_unlock;
@@ -3081,7 +3092,7 @@ bool i915_gpu_lower(void)
dev_priv->max_delay++;
out_unlock:
- spin_unlock(&mchdev_lock);
+ spin_unlock_irq(&mchdev_lock);
return ret;
}
@@ -3099,7 +3110,7 @@ bool i915_gpu_busy(void)
bool ret = false;
int i;
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev)
goto out_unlock;
dev_priv = i915_mch_dev;
@@ -3108,7 +3119,7 @@ bool i915_gpu_busy(void)
ret |= !list_empty(&ring->request_list);
out_unlock:
- spin_unlock(&mchdev_lock);
+ spin_unlock_irq(&mchdev_lock);
return ret;
}
@@ -3125,7 +3136,7 @@ bool i915_gpu_turbo_disable(void)
struct drm_i915_private *dev_priv;
bool ret = true;
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
if (!i915_mch_dev) {
ret = false;
goto out_unlock;
@@ -3138,7 +3149,7 @@ bool i915_gpu_turbo_disable(void)
ret = false;
out_unlock:
- spin_unlock(&mchdev_lock);
+ spin_unlock_irq(&mchdev_lock);
return ret;
}
@@ -3166,18 +3177,18 @@ ips_ping_for_i915_load(void)
void intel_gpu_ips_init(struct drm_i915_private *dev_priv)
{
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
i915_mch_dev = dev_priv;
- spin_unlock(&mchdev_lock);
+ spin_unlock_irq(&mchdev_lock);
ips_ping_for_i915_load();
}
void intel_gpu_ips_teardown(void)
{
- spin_lock(&mchdev_lock);
+ spin_lock_irq(&mchdev_lock);
i915_mch_dev = NULL;
- spin_unlock(&mchdev_lock);
+ spin_unlock_irq(&mchdev_lock);
}
static void intel_init_emon(struct drm_device *dev)
{
--
1.7.10.4
next prev parent reply other threads:[~2012-07-24 21:17 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-24 21:33 [PATCH 0/9] rps locking fixes Daniel Vetter
2012-07-24 21:33 ` [PATCH 1/9] drm/i915: ensure rps state is properly lock-protected Daniel Vetter
2012-07-24 21:33 ` [PATCH 2/9] drm/i915: properly guard ilk ips state Daniel Vetter
2012-07-25 18:09 ` Ben Widawsky
2012-07-25 18:57 ` [PATCH] " Daniel Vetter
2012-07-24 21:33 ` [PATCH 3/9] drm/i915: fixup up debugfs rps state handling Daniel Vetter
2012-07-25 18:15 ` Ben Widawsky
2012-07-24 21:33 ` [PATCH 4/9] drm/i915: move all rps state into dev_priv->rps Daniel Vetter
2012-07-25 19:25 ` Ben Widawsky
2012-07-25 21:26 ` Daniel Vetter
2012-07-24 21:33 ` [PATCH 5/9] drm/i915: kill dev_priv->mchdev_lock Daniel Vetter
2012-07-24 21:33 ` [PATCH 6/9] drm/i915: DE_PCU_EVENT irq is ilk-only Daniel Vetter
2012-07-25 21:09 ` Ben Widawsky
2012-07-24 21:33 ` Daniel Vetter [this message]
2012-07-24 21:33 ` [PATCH 8/9] drm/ips: move drps/ips/ilk related variables into dev_priv->ips Daniel Vetter
2012-07-25 21:25 ` Ben Widawsky
2012-07-25 21:32 ` Daniel Vetter
2012-07-25 23:52 ` Ben Widawsky
2012-07-24 21:33 ` [PATCH 9/9] drm/i915: enable rc6 on ilk again Daniel Vetter
2012-07-25 21:26 ` Ben Widawsky
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=1343165630-21604-8-git-send-email-daniel.vetter@ffwll.ch \
--to=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).