Our interrupt handler (in hardird 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.
Cc: Egbert Eich <eich at suse.de>
Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
drivers/gpu/drm/i915/i915_irq.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 95999bc..6637575 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -881,15 +881,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) ||
@@ -912,10 +910,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);
@@ -2851,7 +2848,7 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
I915_WRITE(PIPESTAT(1), 0xffff);
POSTING_READ(VLV_IER);
- /* Interrup setup is already guaranteed to be single-threaded, this is
+ /* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
i915_enable_pipestat(dev_priv, 0, pipestat_enable);
@@ -3395,6 +3392,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;
@@ -3678,6 +3677,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++) {
@@ -3690,6 +3690,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;
}
+
+ /* Interrup 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);
}
--
1.8.1.4
