diff mbox

[2/4] drm/i915: Fix hotplug irq ack on i965/g4x

Message ID 20180611200258.27121-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä June 11, 2018, 8:02 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just like with PIPESTAT, the edge triggered IIR on i965/g4x
also causes problems for hotplug interrupts. To make sure
we don't get the IIR port interrupt bit stuck low with the
ISR bit high we must force an edge in ISR. We do that by
clearing PORT_HOTPLUG_EN temporaryly when we ack
PORT_HOTPLUG_STAT.

Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

Comments

Chris Wilson June 11, 2018, 8:58 p.m. UTC | #1
Quoting Ville Syrjala (2018-06-11 21:02:56)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just like with PIPESTAT, the edge triggered IIR on i965/g4x
> also causes problems for hotplug interrupts. To make sure
> we don't get the IIR port interrupt bit stuck low with the
> ISR bit high we must force an edge in ISR. We do that by
> clearing PORT_HOTPLUG_EN temporaryly when we ack
> PORT_HOTPLUG_STAT.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Having accepted the first,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Ville Syrjälä June 14, 2018, 4:27 p.m. UTC | #2
On Mon, Jun 11, 2018 at 11:02:56PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just like with PIPESTAT, the edge triggered IIR on i965/g4x
> also causes problems for hotplug interrupts. To make sure
> we don't get the IIR port interrupt bit stuck low with the
> ISR bit high we must force an edge in ISR. We do that by
> clearing PORT_HOTPLUG_EN temporaryly when we ack
> PORT_HOTPLUG_STAT.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 364e1c85315e..59250ecbd0d9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1998,10 +1998,35 @@ static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
>  
>  static u32 i9xx_hpd_irq_ack(struct drm_i915_private *dev_priv)
>  {
> -	u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
> +	u32 hotplug_status, hotplug_en;
>  
> -	if (hotplug_status)
> -		I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> +	hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
> +
> +	if (IS_G4X(dev_priv) ||
> +	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +		hotplug_status &= HOTPLUG_INT_STATUS_G4X |
> +			DP_AUX_CHANNEL_MASK_INT_STATUS_G4X;
> +	else
> +		hotplug_status &= HOTPLUG_INT_STATUS_I915;
> +
> +	if (hotplug_status == 0)
> +		return 0;
> +
> +	spin_lock(&dev_priv->irq_lock);
> +
> +	/*
> +	 * Toggle all PORT_HOTPLUG_EN bits to make sure we
> +	 * get an edge in the ISR port interrupt bit if we
> +	 * don't clear all the enabled status bits. Otherwise
> +	 * the edge triggered IIR on i965/g4x wouldn't notice
> +	 * that an interrupt is still pending.
> +	 */
> +	hotplug_en = I915_READ(PORT_HOTPLUG_EN);
> +	I915_WRITE(PORT_HOTPLUG_EN, 0);
> +	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> +	I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);

Actually this seems to be a bad idea. At least one elk here likes
to signal a long pulse on disconnected ports every time I toggle
the enable bit in PORT_HOTPLUG_EN. The spec even warns that something
like this could happen. Not sure why it specifically happens for
ports where nothing is connected. Feels like it should be the other
way around.

So looks like I'll need to come up with some other way to guarantee
the ISR edge. Can't immediately think of any way apart from clearing
PORT_HOTPLUG_STAT in a loop :(

> +
> +	spin_unlock(&dev_priv->irq_lock);
>  
>  	return hotplug_status;
>  }
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 364e1c85315e..59250ecbd0d9 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1998,10 +1998,35 @@  static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
 
 static u32 i9xx_hpd_irq_ack(struct drm_i915_private *dev_priv)
 {
-	u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+	u32 hotplug_status, hotplug_en;
 
-	if (hotplug_status)
-		I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
+	hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+
+	if (IS_G4X(dev_priv) ||
+	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		hotplug_status &= HOTPLUG_INT_STATUS_G4X |
+			DP_AUX_CHANNEL_MASK_INT_STATUS_G4X;
+	else
+		hotplug_status &= HOTPLUG_INT_STATUS_I915;
+
+	if (hotplug_status == 0)
+		return 0;
+
+	spin_lock(&dev_priv->irq_lock);
+
+	/*
+	 * Toggle all PORT_HOTPLUG_EN bits to make sure we
+	 * get an edge in the ISR port interrupt bit if we
+	 * don't clear all the enabled status bits. Otherwise
+	 * the edge triggered IIR on i965/g4x wouldn't notice
+	 * that an interrupt is still pending.
+	 */
+	hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+	I915_WRITE(PORT_HOTPLUG_EN, 0);
+	I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
+	I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
+
+	spin_unlock(&dev_priv->irq_lock);
 
 	return hotplug_status;
 }