Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b5ea2d56 authored by Daniel Vetter's avatar Daniel Vetter
Browse files

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: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 91d131d2

drivers/gpu/drm/i915/i915_irq.c

+10 −5
Original line number Diff line number Diff line
@@ -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);

}