diff options
author | Daniel Vetter <daniel.vetter@ffwll.ch> | 2013-06-27 11:52:15 -0400 |
---|---|---|
committer | Daniel Vetter <daniel.vetter@ffwll.ch> | 2013-07-01 05:14:59 -0400 |
commit | b5ea2d5681522f1b8ef886b5ac039903bf1d39fe (patch) | |
tree | 866e987e68aa8c973f02046eb29ad4abf2dd3639 /drivers/gpu/drm | |
parent | 91d131d21e4916f84e5957cc25bea6dd355dfe77 (diff) |
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 <eich@suse.de>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Diffstat (limited to 'drivers/gpu/drm')
-rw-r--r-- | drivers/gpu/drm/i915/i915_irq.c | 15 |
1 files changed, 10 insertions, 5 deletions
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b29b76fe1123..3d92a7cef154 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, | |||
877 | const u32 *hpd) | 877 | const u32 *hpd) |
878 | { | 878 | { |
879 | drm_i915_private_t *dev_priv = dev->dev_private; | 879 | drm_i915_private_t *dev_priv = dev->dev_private; |
880 | unsigned long irqflags; | ||
881 | int i; | 880 | int i; |
882 | bool storm_detected = false; | 881 | bool storm_detected = false; |
883 | 882 | ||
884 | if (!hotplug_trigger) | 883 | if (!hotplug_trigger) |
885 | return; | 884 | return; |
886 | 885 | ||
887 | spin_lock_irqsave(&dev_priv->irq_lock, irqflags); | 886 | spin_lock(&dev_priv->irq_lock); |
888 | |||
889 | for (i = 1; i < HPD_NUM_PINS; i++) { | 887 | for (i = 1; i < HPD_NUM_PINS; i++) { |
890 | 888 | ||
891 | if (!(hpd[i] & hotplug_trigger) || | 889 | if (!(hpd[i] & hotplug_trigger) || |
@@ -908,10 +906,9 @@ static inline void intel_hpd_irq_handler(struct drm_device *dev, | |||
908 | } | 906 | } |
909 | } | 907 | } |
910 | 908 | ||
911 | spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); | ||
912 | |||
913 | if (storm_detected) | 909 | if (storm_detected) |
914 | dev_priv->display.hpd_irq_setup(dev); | 910 | dev_priv->display.hpd_irq_setup(dev); |
911 | spin_unlock(&dev_priv->irq_lock); | ||
915 | 912 | ||
916 | queue_work(dev_priv->wq, | 913 | queue_work(dev_priv->wq, |
917 | &dev_priv->hotplug_work); | 914 | &dev_priv->hotplug_work); |
@@ -3380,6 +3377,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev) | |||
3380 | struct intel_encoder *intel_encoder; | 3377 | struct intel_encoder *intel_encoder; |
3381 | u32 hotplug_en; | 3378 | u32 hotplug_en; |
3382 | 3379 | ||
3380 | assert_spin_locked(&dev_priv->irq_lock); | ||
3381 | |||
3383 | if (I915_HAS_HOTPLUG(dev)) { | 3382 | if (I915_HAS_HOTPLUG(dev)) { |
3384 | hotplug_en = I915_READ(PORT_HOTPLUG_EN); | 3383 | hotplug_en = I915_READ(PORT_HOTPLUG_EN); |
3385 | hotplug_en &= ~HOTPLUG_INT_EN_MASK; | 3384 | hotplug_en &= ~HOTPLUG_INT_EN_MASK; |
@@ -3663,6 +3662,7 @@ void intel_hpd_init(struct drm_device *dev) | |||
3663 | struct drm_i915_private *dev_priv = dev->dev_private; | 3662 | struct drm_i915_private *dev_priv = dev->dev_private; |
3664 | struct drm_mode_config *mode_config = &dev->mode_config; | 3663 | struct drm_mode_config *mode_config = &dev->mode_config; |
3665 | struct drm_connector *connector; | 3664 | struct drm_connector *connector; |
3665 | unsigned long irqflags; | ||
3666 | int i; | 3666 | int i; |
3667 | 3667 | ||
3668 | for (i = 1; i < HPD_NUM_PINS; i++) { | 3668 | for (i = 1; i < HPD_NUM_PINS; i++) { |
@@ -3675,6 +3675,11 @@ void intel_hpd_init(struct drm_device *dev) | |||
3675 | if (!connector->polled && I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE) | 3675 | if (!connector->polled && I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE) |
3676 | connector->polled = DRM_CONNECTOR_POLL_HPD; | 3676 | connector->polled = DRM_CONNECTOR_POLL_HPD; |
3677 | } | 3677 | } |
3678 | |||
3679 | /* Interrupt setup is already guaranteed to be single-threaded, this is | ||
3680 | * just to make the assert_spin_locked checks happy. */ | ||
3681 | spin_lock_irqsave(&dev_priv->irq_lock, irqflags); | ||
3678 | if (dev_priv->display.hpd_irq_setup) | 3682 | if (dev_priv->display.hpd_irq_setup) |
3679 | dev_priv->display.hpd_irq_setup(dev); | 3683 | dev_priv->display.hpd_irq_setup(dev); |
3684 | spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); | ||
3680 | } | 3685 | } |