From: Egbert Eich Date: Wed, 23 Sep 2015 14:15:27 +0000 (+0200) Subject: drm/i915: Avoid race of intel_crt_detect_hotplug() with HPD interrupt, v2 X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=0706f17c307b056ff6f1848320ba82d76945a6ff;p=linux-beck.git 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: Egbert Eich Signed-off-by: Daniel Vetter --- diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4e8178d34a34..ba4596e9e7eb 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -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 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index ca6d806a195a..6ca6c16fe500 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -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); diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index af5e43bef4a4..6ce38e3edf21 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -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; }