diff mbox

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

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

Commit Message

Ville Syrjälä June 14, 2018, 5:56 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. Unfortunately
we can't borrow the PIPESTAT trick and toggle the enable
bits in PORT_HOTPLUG_EN as that act itself generates hotplug
interrupts. Instead we just have to loop until we've cleared
PORT_HOTPLUG_STAT, or we just give up and WARN.

v2: Don't frob with PORT_HOTPLUG_EN

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

Comments

Imre Deak July 3, 2018, 9:56 a.m. UTC | #1
On Thu, Jun 14, 2018 at 08:56:25PM +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. Unfortunately
> we can't borrow the PIPESTAT trick and toggle the enable
> bits in PORT_HOTPLUG_EN as that act itself generates hotplug
> interrupts. Instead we just have to loop until we've cleared
> PORT_HOTPLUG_STAT, or we just give up and WARN.
> 
> v2: Don't frob with PORT_HOTPLUG_EN
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, not so great that we have to loop, but looks like there's no better
way to go about it. Good that the problem was root caused:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d5aee0b74f4b..55a4d4f3fbb2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1998,10 +1998,38 @@ 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 = 0, hotplug_status_mask;
> +	int i;
> +
> +	if (IS_G4X(dev_priv) ||
> +	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +		hotplug_status_mask = HOTPLUG_INT_STATUS_G4X |
> +			DP_AUX_CHANNEL_MASK_INT_STATUS_G4X;
> +	else
> +		hotplug_status_mask = HOTPLUG_INT_STATUS_I915;
>  
> -	if (hotplug_status)
> +	/*
> +	 * We absolutely have to clear all the pending interrupt
> +	 * bits in PORT_HOTPLUG_STAT. Otherwise the ISR port
> +	 * interrupt bit won't have an edge, and the i965/g4x
> +	 * edge triggered IIR will not notice that an interrupt
> +	 * is still pending. We can't use PORT_HOTPLUG_EN to
> +	 * guarantee the edge as the act of toggling the enable
> +	 * bits can itself generate a new hotplug interrupt :(
> +	 */
> +	for (i = 0; i < 10; i++) {
> +		u32 tmp = I915_READ(PORT_HOTPLUG_STAT) & hotplug_status_mask;
> +
> +		if (tmp == 0)
> +			return hotplug_status;
> +
> +		hotplug_status |= tmp;
>  		I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> +	}
> +
> +	WARN_ONCE(1,
> +		  "PORT_HOTPLUG_STAT did not clear (0x%08x)\n",
> +		  I915_READ(PORT_HOTPLUG_STAT));
>  
>  	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 d5aee0b74f4b..55a4d4f3fbb2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1998,10 +1998,38 @@  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 = 0, hotplug_status_mask;
+	int i;
+
+	if (IS_G4X(dev_priv) ||
+	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		hotplug_status_mask = HOTPLUG_INT_STATUS_G4X |
+			DP_AUX_CHANNEL_MASK_INT_STATUS_G4X;
+	else
+		hotplug_status_mask = HOTPLUG_INT_STATUS_I915;
 
-	if (hotplug_status)
+	/*
+	 * We absolutely have to clear all the pending interrupt
+	 * bits in PORT_HOTPLUG_STAT. Otherwise the ISR port
+	 * interrupt bit won't have an edge, and the i965/g4x
+	 * edge triggered IIR will not notice that an interrupt
+	 * is still pending. We can't use PORT_HOTPLUG_EN to
+	 * guarantee the edge as the act of toggling the enable
+	 * bits can itself generate a new hotplug interrupt :(
+	 */
+	for (i = 0; i < 10; i++) {
+		u32 tmp = I915_READ(PORT_HOTPLUG_STAT) & hotplug_status_mask;
+
+		if (tmp == 0)
+			return hotplug_status;
+
+		hotplug_status |= tmp;
 		I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
+	}
+
+	WARN_ONCE(1,
+		  "PORT_HOTPLUG_STAT did not clear (0x%08x)\n",
+		  I915_READ(PORT_HOTPLUG_STAT));
 
 	return hotplug_status;
 }