On Tue, Sep 01, 2015 at 10:21:35PM +0200, Egbert Eich wrote:
> A HPD interrupt may fire during intel_crt_detect_hotplug() - especially
> when HPD interrupt storms occur.
> Since the interrupt handler changes the enabled interrupt lines when it
> detects a storm this races with intel_crt_detect_hotplug().
> To avoid this, shiled the rmw cycles with IRQ save spinlocks.
>
> Signed-off-by: Egbert Eich <eich@xxxxxxx>
I think this only reduces one source of such races, but fundamentally we
can't avoid them. E.g. if you're _very_ unlucky you might cause a real hpd
right when the strom code frobs around. Plugging the race with this known
interrupt source here doesn't fix the fundamental issue.
Also I think the actual interrupt deliver is fairly asynchronous, both in
the hardware and in the sw handling. E.g. spin_lock_irq does not
synchronize with the interrupt handler on SMP systems, it only guarantees
that it's not running concurrently on the same cpu (which would deadlock).
I think fundamentally this race is unfixable.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_crt.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index af5e43b..685f3de 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -376,9 +376,10 @@ 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 hotplug_en, stat;
> bool ret = false;
> int i, tries = 0;
> + unsigned long irqflags;
>
> if (HAS_PCH_SPLIT(dev))
> return intel_ironlake_crt_detect_hotplug(connector);
> @@ -395,12 +396,14 @@ 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);
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + hotplug_en = I915_READ(PORT_HOTPLUG_EN);
> + I915_WRITE(PORT_HOTPLUG_EN, hotplug_en |
> + CRT_HOTPLUG_FORCE_DETECT);
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> /* wait for FORCE_DETECT to go off */
> if (wait_for((I915_READ(PORT_HOTPLUG_EN) &
> CRT_HOTPLUG_FORCE_DETECT) == 0,
> @@ -416,7 +419,11 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector)
> I915_WRITE(PORT_HOTPLUG_STAT, CRT_HOTPLUG_INT_STATUS);
>
> /* and put the bits back */
> - I915_WRITE(PORT_HOTPLUG_EN, orig);
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + hotplug_en = I915_READ(PORT_HOTPLUG_EN);
> + hotplug_en &= ~CRT_HOTPLUG_FORCE_DETECT;
> + I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> return ret;
> }
> --
> 1.8.4.5
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
