summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaniel Vetter <daniel.vetter@ffwll.ch>2011-12-14 13:57:03 +0100
committerKeith Packard <keithp@keithp.com>2012-01-19 11:51:31 -0800
commit9f1f46a45a681d357d1ceedecec3671a5ae957f4 (patch)
treefbb1ccb3dc2afded75087e2d187a5160e356849f
parent8109021313c7a3d8947677391ce6ab9cd0bb1d28 (diff)
downloadop-kernel-dev-9f1f46a45a681d357d1ceedecec3671a5ae957f4.zip
op-kernel-dev-9f1f46a45a681d357d1ceedecec3671a5ae957f4.tar.gz
drm/i915: protect force_wake_(get|put) with the gt_lock
The problem this patch solves is that the forcewake accounting necessary for register reads is protected by dev->struct_mutex. But the hangcheck and error_capture code need to access registers without grabbing this mutex because we hold it while waiting for the gpu. So a new lock is required. Because currently the error_state capture is called from the error irq handler and the hangcheck code runs from a timer, it needs to be an irqsafe spinlock (note that the registers used by the irq handler (neglecting the error handling part) only uses registers that don't need the forcewake dance). We could tune this down to a normal spinlock when we rework the error_state capture and hangcheck code to run from a workqueue. But we don't have any read in a fastpath that needs forcewake, so I've decided to not care much about overhead. This prevents tests/gem_hangcheck_forcewake from i-g-t from killing my snb on recent kernels - something must have slightly changed the timings. On previous kernels it only trigger a WARN about the broken locking. v2: Drop the previous patch for the register writes. v3: Improve the commit message per Chris Wilson's suggestions. Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com> Signed-off-by: Keith Packard <keithp@keithp.com>
-rw-r--r--drivers/gpu/drm/i915/i915_debugfs.c8
-rw-r--r--drivers/gpu/drm/i915/i915_dma.c1
-rw-r--r--drivers/gpu/drm/i915/i915_drv.c18
-rw-r--r--drivers/gpu/drm/i915/i915_drv.h10
4 files changed, 26 insertions, 11 deletions
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f8b8ed2..a017b98 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1398,9 +1398,13 @@ static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ unsigned forcewake_count;
- seq_printf(m, "forcewake count = %d\n",
- atomic_read(&dev_priv->forcewake_count));
+ spin_lock_irq(&dev_priv->gt_lock);
+ forcewake_count = dev_priv->forcewake_count;
+ spin_unlock_irq(&dev_priv->gt_lock);
+
+ seq_printf(m, "forcewake count = %u\n", forcewake_count);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5f4d589..ddfe3d9 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2045,6 +2045,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
if (!IS_I945G(dev) && !IS_I945GM(dev))
pci_enable_msi(dev->pdev);
+ spin_lock_init(&dev_priv->gt_lock);
spin_lock_init(&dev_priv->irq_lock);
spin_lock_init(&dev_priv->error_lock);
spin_lock_init(&dev_priv->rps_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 46c36f5..bdf6a1b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -368,11 +368,12 @@ void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv)
*/
void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
{
- WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+ unsigned long irqflags;
- /* Forcewake is atomic in case we get in here without the lock */
- if (atomic_add_return(1, &dev_priv->forcewake_count) == 1)
+ spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+ if (dev_priv->forcewake_count++ == 0)
dev_priv->display.force_wake_get(dev_priv);
+ spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
}
void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
@@ -392,10 +393,12 @@ void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
*/
void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
{
- WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
+ unsigned long irqflags;
- if (atomic_dec_and_test(&dev_priv->forcewake_count))
+ spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+ if (--dev_priv->forcewake_count == 0)
dev_priv->display.force_wake_put(dev_priv);
+ spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
}
void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
@@ -626,6 +629,7 @@ int i915_reset(struct drm_device *dev, u8 flags)
* need to
*/
bool need_display = true;
+ unsigned long irqflags;
int ret;
if (!i915_try_reset)
@@ -644,8 +648,10 @@ int i915_reset(struct drm_device *dev, u8 flags)
case 6:
ret = gen6_do_reset(dev, flags);
/* If reset with a user forcewake, try to restore */
- if (atomic_read(&dev_priv->forcewake_count))
+ spin_lock_irqsave(&dev_priv->gt_lock, irqflags);
+ if (dev_priv->forcewake_count)
dev_priv->display.force_wake_get(dev_priv);
+ spin_unlock_irqrestore(&dev_priv->gt_lock, irqflags);
break;
case 5:
ret = ironlake_do_reset(dev, flags);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 602bc80..9689ca3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -288,7 +288,13 @@ typedef struct drm_i915_private {
int relative_constants_mode;
void __iomem *regs;
- u32 gt_fifo_count;
+ /** gt_fifo_count and the subsequent register write are synchronized
+ * with dev->struct_mutex. */
+ unsigned gt_fifo_count;
+ /** forcewake_count is protected by gt_lock */
+ unsigned forcewake_count;
+ /** gt_lock is also taken in irq contexts. */
+ struct spinlock gt_lock;
struct intel_gmbus {
struct i2c_adapter adapter;
@@ -741,8 +747,6 @@ typedef struct drm_i915_private {
struct drm_property *broadcast_rgb_property;
struct drm_property *force_audio_property;
-
- atomic_t forcewake_count;
} drm_i915_private_t;
enum i915_cache_level {
OpenPOWER on IntegriCloud