From: Daniel Vetter Date: Thu, 27 Jun 2013 15:52:15 +0000 (+0200) Subject: drm/i915: fix hpd interrupt register locking X-Git-Tag: v3.11-rc2~17^2~2^2~14 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b5ea2d5681522f1b8ef886b5ac039903bf1d39fe;p=platform%2Fkernel%2Flinux-stable.git drm/i915: fix hpd interrupt register locking Our interrupt handler (in hardirq context) could race with the timer (in softirq context), hence we need to hold the spinlock around the call to ->hdp_irq_setup in intel_hpd_irq_handler, too. But as an optimization (and more so to clarify things) we don't need to do the irqsave/restore dance in the hardirq context. Note also that on ilk+ the race isn't just against the hotplug reenable timer, but also against the fifo underrun reporting. That one also modifies the SDEIMR register (again protected by the same dev_priv->irq_lock). To lock things down again sprinkle a assert_spin_locked. But exclude the functions touching SDEIMR for now, I want to extract them all into a new helper function (like we do already for pipestate, display interrupts and all the various gt interrupts). v2: Add the missing 't' Egbert spotted in a comment. v3: Actually fix the right misspelled comment (Paulo). Cc: Egbert Eich Reviewed-by: Paulo Zanoni Signed-off-by: Daniel Vetter --- diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b29b76f..3d92a7c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -877,15 +877,13 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, const u32 *hpd) { drm_i915_private_t *dev_priv = dev->dev_private; - unsigned long irqflags; int i; bool storm_detected = false; if (!hotplug_trigger) return; - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); - + spin_lock(&dev_priv->irq_lock); for (i = 1; i < HPD_NUM_PINS; i++) { if (!(hpd[i] & hotplug_trigger) || @@ -908,10 +906,9 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, } } - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); - if (storm_detected) dev_priv->display.hpd_irq_setup(dev); + spin_unlock(&dev_priv->irq_lock); queue_work(dev_priv->wq, &dev_priv->hotplug_work); @@ -3380,6 +3377,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev) struct intel_encoder *intel_encoder; u32 hotplug_en; + assert_spin_locked(&dev_priv->irq_lock); + if (I915_HAS_HOTPLUG(dev)) { hotplug_en = I915_READ(PORT_HOTPLUG_EN); hotplug_en &= ~HOTPLUG_INT_EN_MASK; @@ -3663,6 +3662,7 @@ void intel_hpd_init(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct drm_mode_config *mode_config = &dev->mode_config; struct drm_connector *connector; + unsigned long irqflags; int i; for (i = 1; i < HPD_NUM_PINS; i++) { @@ -3675,6 +3675,11 @@ void intel_hpd_init(struct drm_device *dev) if (!connector->polled && I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE) connector->polled = DRM_CONNECTOR_POLL_HPD; } + + /* Interrupt setup is already guaranteed to be single-threaded, this is + * just to make the assert_spin_locked checks happy. */ + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); if (dev_priv->display.hpd_irq_setup) dev_priv->display.hpd_irq_setup(dev); + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); }