Commit 0706f17c authored by Egbert Eich's avatar Egbert Eich Committed by Daniel Vetter
Browse files

drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 



An HPD interrupt may fire while we are in a function that changes
the PORT_HOTPLUG_EN register - especially when an HPD interrupt
storm occurs.
Since the interrupt handler changes the enabled HPD lines when it
detects such a storm the read-modify-write cycles may interfere.
To avoid this, shiled the rmw cycles with IRQ save spinlocks.

Changes since v1:
- Implement a function which takes care of accessing PORT_HOTPLUG_EN.

Signed-off-by: default avatarEgbert Eich <eich@suse.de>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 8204502a

drivers/gpu/drm/i915/i915_drv.h

+3 −0
Original line number Diff line number Diff line
@@ -2742,6 +2742,9 @@ i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, 


void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv);

void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv);

void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,

				   uint32_t mask,

				   uint32_t bits);

void

ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask);

void

drivers/gpu/drm/i915/i915_irq.c

+51 −13
Original line number Diff line number Diff line
@@ -167,6 +167,44 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = { 


static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);



/* For display hotplug interrupt */

static inline void

i915_hotplug_interrupt_update_locked(struct drm_i915_private *dev_priv,

				     uint32_t mask,

				     uint32_t bits)

{

	uint32_t val;



	assert_spin_locked(&dev_priv->irq_lock);

	WARN_ON(bits & ~mask);



	val = I915_READ(PORT_HOTPLUG_EN);

	val &= ~mask;

	val |= bits;

	I915_WRITE(PORT_HOTPLUG_EN, val);

}



/**

 * i915_hotplug_interrupt_update - update hotplug interrupt enable

 * @dev_priv: driver private

 * @mask: bits to update

 * @bits: bits to enable

 * NOTE: the HPD enable bits are modified both inside and outside

 * of an interrupt context. To avoid that read-modify-write cycles

 * interfer, these bits are protected by a spinlock. Since this

 * function is usually not called from a context where the lock is

 * held already, this function acquires the lock itself. A non-locking

 * version is also available.

 */

void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,

				   uint32_t mask,

				   uint32_t bits)

{

	spin_lock_irq(&dev_priv->irq_lock);

	i915_hotplug_interrupt_update_locked(dev_priv, mask, bits);

	spin_unlock_irq(&dev_priv->irq_lock);

}



/**

 * ilk_update_display_irq - update DEIMR

 * @dev_priv: driver private
@@ -3050,7 +3088,7 @@ static void vlv_display_irq_reset(struct drm_i915_private *dev_priv) 
{

	enum pipe pipe;



	I915_WRITE(PORT_HOTPLUG_EN, 0);

	i915_hotplug_interrupt_update(dev_priv, 0xFFFFFFFF, 0);

	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));



	for_each_pipe(dev_priv, pipe)
@@ -3466,7 +3504,7 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) 
{

	dev_priv->irq_mask = ~0;



	I915_WRITE(PORT_HOTPLUG_EN, 0);

	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);

	POSTING_READ(PORT_HOTPLUG_EN);



	I915_WRITE(VLV_IIR, 0xffffffff);
@@ -3840,7 +3878,7 @@ static void i915_irq_preinstall(struct drm_device * dev) 
	int pipe;



	if (I915_HAS_HOTPLUG(dev)) {

		I915_WRITE(PORT_HOTPLUG_EN, 0);

		i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);

		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));

	}
@@ -3874,7 +3912,7 @@ static int i915_irq_postinstall(struct drm_device *dev) 
		I915_USER_INTERRUPT;



	if (I915_HAS_HOTPLUG(dev)) {

		I915_WRITE(PORT_HOTPLUG_EN, 0);

		i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);

		POSTING_READ(PORT_HOTPLUG_EN);



		/* Enable in IER... */
@@ -4036,7 +4074,7 @@ static void i915_irq_uninstall(struct drm_device * dev) 
	int pipe;



	if (I915_HAS_HOTPLUG(dev)) {

		I915_WRITE(PORT_HOTPLUG_EN, 0);

		i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);

		I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));

	}
@@ -4057,7 +4095,7 @@ static void i965_irq_preinstall(struct drm_device * dev) 
	struct drm_i915_private *dev_priv = dev->dev_private;

	int pipe;



	I915_WRITE(PORT_HOTPLUG_EN, 0);

	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);

	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));



	I915_WRITE(HWSTAM, 0xeffe);
@@ -4118,7 +4156,7 @@ static int i965_irq_postinstall(struct drm_device *dev) 
	I915_WRITE(IER, enable_mask);

	POSTING_READ(IER);



	I915_WRITE(PORT_HOTPLUG_EN, 0);

	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);

	POSTING_READ(PORT_HOTPLUG_EN);



	i915_enable_asle_pipestat(dev);
@@ -4133,22 +4171,22 @@ static void i915_hpd_irq_setup(struct drm_device *dev) 


	assert_spin_locked(&dev_priv->irq_lock);



	hotplug_en = I915_READ(PORT_HOTPLUG_EN);

	hotplug_en &= ~HOTPLUG_INT_EN_MASK;

	/* Note HDMI and DP share hotplug bits */

	/* enable bits are the same for all generations */

	hotplug_en |= intel_hpd_enabled_irqs(dev, hpd_mask_i915);

	hotplug_en = intel_hpd_enabled_irqs(dev, hpd_mask_i915);

	/* Programming the CRT detection parameters tends

	   to generate a spurious hotplug event about three

	   seconds later.  So just do it once.

	*/

	if (IS_G4X(dev))

		hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;

	hotplug_en &= ~CRT_HOTPLUG_VOLTAGE_COMPARE_MASK;

	hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;



	/* Ignore TV since it's buggy */

	I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);

	i915_hotplug_interrupt_update_locked(dev_priv,

				      (HOTPLUG_INT_EN_MASK

				       | CRT_HOTPLUG_VOLTAGE_COMPARE_MASK),

				      hotplug_en);

}



static irqreturn_t i965_irq_handler(int irq, void *arg)
@@ -4261,7 +4299,7 @@ static void i965_irq_uninstall(struct drm_device * dev) 
	if (!dev_priv)

		return;



	I915_WRITE(PORT_HOTPLUG_EN, 0);

	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);

	I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));



	I915_WRITE(HWSTAM, 0xffffffff);

drivers/gpu/drm/i915/intel_crt.c

+5 −6
Original line number Diff line number Diff line
@@ -376,7 +376,7 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector) 
{

	struct drm_device *dev = connector->dev;

	struct drm_i915_private *dev_priv = dev->dev_private;

	u32 hotplug_en, orig, stat;

	u32 stat;

	bool ret = false;

	int i, tries = 0;
@@ -395,12 +395,12 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector) 
		tries = 2;

	else

		tries = 1;

	hotplug_en = orig = I915_READ(PORT_HOTPLUG_EN);

	hotplug_en |= CRT_HOTPLUG_FORCE_DETECT;



	for (i = 0; i < tries ; i++) {

		/* turn on the FORCE_DETECT */

		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);

		i915_hotplug_interrupt_update(dev_priv,

					      CRT_HOTPLUG_FORCE_DETECT,

					      CRT_HOTPLUG_FORCE_DETECT);

		/* wait for FORCE_DETECT to go off */

		if (wait_for((I915_READ(PORT_HOTPLUG_EN) &

			      CRT_HOTPLUG_FORCE_DETECT) == 0,
@@ -415,8 +415,7 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector) 
	/* clear the interrupt we just generated, if any */

	I915_WRITE(PORT_HOTPLUG_STAT, CRT_HOTPLUG_INT_STATUS);



	/* and put the bits back */

	I915_WRITE(PORT_HOTPLUG_EN, orig);

	i915_hotplug_interrupt_update(dev_priv, CRT_HOTPLUG_FORCE_DETECT, 0);



	return ret;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE