diff mbox

[3/4] drm/i915: Fix pre-ILK error interrupt ack

Message ID 20180611200258.27121-3-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>

Adjust the EIR clearing to cope with the edge triggered IIR
on i965/g4x. To guarantee an edge in the ISR master error bit
we temporarily mask everything in EMR. As some of the EIR bits
can't even be directly cleared we also borrow a trick from
i915_clear_error_registers() and permanently mask any bit that
remains high. No real thought given to how we might unmask them
again once the cause for the error has been clered. I suppose
on pre-g4x GPU reset will reinitialize EMR from scratch.

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

Comments

Imre Deak July 3, 2018, 11:58 a.m. UTC | #1 
On Mon, Jun 11, 2018 at 11:02:57PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Adjust the EIR clearing to cope with the edge triggered IIR
> on i965/g4x. To guarantee an edge in the ISR master error bit
> we temporarily mask everything in EMR. As some of the EIR bits
> can't even be directly cleared we also borrow a trick from
> i915_clear_error_registers() and permanently mask any bit that
> remains high. No real thought given to how we might unmask them
> again once the cause for the error has been clered. I suppose
> on pre-g4x GPU reset will reinitialize EMR from scratch.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 105 ++++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/i915_reg.h |   1 -
>  2 files changed, 96 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 59250ecbd0d9..985a137901fb 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3087,7 +3087,7 @@ static void i915_clear_error_registers(struct drm_i915_private *dev_priv)
>  		 */
>  		DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir);
>  		I915_WRITE(EMR, I915_READ(EMR) | eir);
> -		I915_WRITE(IIR, I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
> +		I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
>  	}
>  }
>  
> @@ -4107,6 +4107,81 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
>  	return 0;
>  }
>  
> +static void i8xx_error_irq_ack(struct drm_i915_private *dev_priv,
> +			       u16 *eir, u16 *eir_stuck)
> +{
> +	u16 emr;
> +
> +	*eir = I915_READ16(EIR);
> +
> +	if (*eir)
> +		I915_WRITE16(EIR, *eir);
> +
> +	*eir_stuck = I915_READ16(EIR);

Nit: this could store a new error interrupt after the above read to
eir_stuck instead of eir, but we won't lose the event in any case, so:

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

> +	if (*eir_stuck == 0)
> +		return;
> +
> +	/*
> +	 * Toggle all EMR bits to make sure we get an edge
> +	 * in the ISR master error bit if we don't clear
> +	 * all the EIR bits. Otherwise the edge triggered
> +	 * IIR on i965/g4x wouldn't notice that an interrupt
> +	 * is still pending. Also some EIR bits can't be
> +	 * cleared except by handling the underlying error
> +	 * (or by a GPU reset) so we mask any bit that
> +	 * remains set.
> +	 */
> +	emr = I915_READ16(EMR);
> +	I915_WRITE16(EMR, 0xffff);
> +	I915_WRITE16(EMR, emr | *eir_stuck);
> +}
> +
> +static void i8xx_error_irq_handler(struct drm_i915_private *dev_priv,
> +				   u16 eir, u16 eir_stuck)
> +{
> +	DRM_DEBUG("Master Error: EIR 0x%04x\n", eir);
> +
> +	if (eir_stuck)
> +		DRM_DEBUG_DRIVER("EIR stuck: 0x%04x, masked\n", eir_stuck);
> +}
> +
> +static void i9xx_error_irq_ack(struct drm_i915_private *dev_priv,
> +			       u32 *eir, u32 *eir_stuck)
> +{
> +	u32 emr;
> +
> +	*eir = I915_READ(EIR);
> +
> +	I915_WRITE(EIR, *eir);
> +
> +	*eir_stuck = I915_READ(EIR);
> +	if (*eir_stuck == 0)
> +		return;
> +
> +	/*
> +	 * Toggle all EMR bits to make sure we get an edge
> +	 * in the ISR master error bit if we don't clear
> +	 * all the EIR bits. Otherwise the edge triggered
> +	 * IIR on i965/g4x wouldn't notice that an interrupt
> +	 * is still pending. Also some EIR bits can't be
> +	 * cleared except by handling the underlying error
> +	 * (or by a GPU reset) so we mask any bit that
> +	 * remains set.
> +	 */
> +	emr = I915_READ(EMR);
> +	I915_WRITE(EMR, 0xffffffff);
> +	I915_WRITE(EMR, emr | *eir_stuck);
> +}
> +
> +static void i9xx_error_irq_handler(struct drm_i915_private *dev_priv,
> +				   u32 eir, u32 eir_stuck)
> +{
> +	DRM_DEBUG("Master Error, EIR 0x%08x\n", eir);
> +
> +	if (eir_stuck)
> +		DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masked\n", eir_stuck);
> +}
> +
>  static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>  {
>  	struct drm_device *dev = arg;
> @@ -4121,6 +4196,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>  
>  	do {
>  		u32 pipe_stats[I915_MAX_PIPES] = {};
> +		u16 eir = 0, eir_stuck = 0;
>  		u16 iir;
>  
>  		iir = I915_READ16(IIR);
> @@ -4133,13 +4209,16 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
>  		 * signalled in iir */
>  		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
>  
> +		if (iir & I915_MASTER_ERROR_INTERRUPT)
> +			i8xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
> +
>  		I915_WRITE16(IIR, iir);
>  
>  		if (iir & I915_USER_INTERRUPT)
>  			notify_ring(dev_priv->engine[RCS]);
>  
> -		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
> -			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
> +		if (iir & I915_MASTER_ERROR_INTERRUPT)
> +			i8xx_error_irq_handler(dev_priv, eir, eir_stuck);
>  
>  		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
>  	} while (0);
> @@ -4220,6 +4299,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
>  
>  	do {
>  		u32 pipe_stats[I915_MAX_PIPES] = {};
> +		u32 eir = 0, eir_stuck = 0;
>  		u32 hotplug_status = 0;
>  		u32 iir;
>  
> @@ -4237,13 +4317,16 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
>  		 * signalled in iir */
>  		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
>  
> +		if (iir & I915_MASTER_ERROR_INTERRUPT)
> +			i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
> +
>  		I915_WRITE(IIR, iir);
>  
>  		if (iir & I915_USER_INTERRUPT)
>  			notify_ring(dev_priv->engine[RCS]);
>  
> -		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
> -			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
> +		if (iir & I915_MASTER_ERROR_INTERRUPT)
> +			i9xx_error_irq_handler(dev_priv, eir, eir_stuck);
>  
>  		if (hotplug_status)
>  			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
> @@ -4297,14 +4380,14 @@ static int i965_irq_postinstall(struct drm_device *dev)
>  		  I915_DISPLAY_PORT_INTERRUPT |
>  		  I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
>  		  I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
> -		  I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
> +		  I915_MASTER_ERROR_INTERRUPT);
>  
>  	enable_mask =
>  		I915_ASLE_INTERRUPT |
>  		I915_DISPLAY_PORT_INTERRUPT |
>  		I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
>  		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
> -		I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT |
> +		I915_MASTER_ERROR_INTERRUPT |
>  		I915_USER_INTERRUPT;
>  
>  	if (IS_G4X(dev_priv))
> @@ -4364,6 +4447,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>  
>  	do {
>  		u32 pipe_stats[I915_MAX_PIPES] = {};
> +		u32 eir = 0, eir_stuck = 0;
>  		u32 hotplug_status = 0;
>  		u32 iir;
>  
> @@ -4380,6 +4464,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>  		 * signalled in iir */
>  		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
>  
> +		if (iir & I915_MASTER_ERROR_INTERRUPT)
> +			i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
> +
>  		I915_WRITE(IIR, iir);
>  
>  		if (iir & I915_USER_INTERRUPT)
> @@ -4388,8 +4475,8 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
>  		if (iir & I915_BSD_USER_INTERRUPT)
>  			notify_ring(dev_priv->engine[VCS]);
>  
> -		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
> -			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
> +		if (iir & I915_MASTER_ERROR_INTERRUPT)
> +			i9xx_error_irq_handler(dev_priv, eir, eir_stuck);
>  
>  		if (hotplug_status)
>  			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 987def26ce82..50a47753014b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2805,7 +2805,6 @@ enum i915_power_well_id {
>  #define I915_DISPLAY_PORT_INTERRUPT			(1<<17)
>  #define I915_DISPLAY_PIPE_C_HBLANK_INTERRUPT		(1<<16)
>  #define I915_MASTER_ERROR_INTERRUPT			(1<<15)
> -#define I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
>  #define I915_DISPLAY_PIPE_B_HBLANK_INTERRUPT		(1<<14)
>  #define I915_GMCH_THERMAL_SENSOR_EVENT_INTERRUPT	(1<<14) /* p-state */
>  #define I915_DISPLAY_PIPE_A_HBLANK_INTERRUPT		(1<<13)
> -- 
> 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 59250ecbd0d9..985a137901fb 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3087,7 +3087,7 @@  static void i915_clear_error_registers(struct drm_i915_private *dev_priv)
 		 */
 		DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir);
 		I915_WRITE(EMR, I915_READ(EMR) | eir);
-		I915_WRITE(IIR, I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
+		I915_WRITE(IIR, I915_MASTER_ERROR_INTERRUPT);
 	}
 }
 
@@ -4107,6 +4107,81 @@  static int i8xx_irq_postinstall(struct drm_device *dev)
 	return 0;
 }
 
+static void i8xx_error_irq_ack(struct drm_i915_private *dev_priv,
+			       u16 *eir, u16 *eir_stuck)
+{
+	u16 emr;
+
+	*eir = I915_READ16(EIR);
+
+	if (*eir)
+		I915_WRITE16(EIR, *eir);
+
+	*eir_stuck = I915_READ16(EIR);
+	if (*eir_stuck == 0)
+		return;
+
+	/*
+	 * Toggle all EMR bits to make sure we get an edge
+	 * in the ISR master error bit if we don't clear
+	 * all the EIR bits. Otherwise the edge triggered
+	 * IIR on i965/g4x wouldn't notice that an interrupt
+	 * is still pending. Also some EIR bits can't be
+	 * cleared except by handling the underlying error
+	 * (or by a GPU reset) so we mask any bit that
+	 * remains set.
+	 */
+	emr = I915_READ16(EMR);
+	I915_WRITE16(EMR, 0xffff);
+	I915_WRITE16(EMR, emr | *eir_stuck);
+}
+
+static void i8xx_error_irq_handler(struct drm_i915_private *dev_priv,
+				   u16 eir, u16 eir_stuck)
+{
+	DRM_DEBUG("Master Error: EIR 0x%04x\n", eir);
+
+	if (eir_stuck)
+		DRM_DEBUG_DRIVER("EIR stuck: 0x%04x, masked\n", eir_stuck);
+}
+
+static void i9xx_error_irq_ack(struct drm_i915_private *dev_priv,
+			       u32 *eir, u32 *eir_stuck)
+{
+	u32 emr;
+
+	*eir = I915_READ(EIR);
+
+	I915_WRITE(EIR, *eir);
+
+	*eir_stuck = I915_READ(EIR);
+	if (*eir_stuck == 0)
+		return;
+
+	/*
+	 * Toggle all EMR bits to make sure we get an edge
+	 * in the ISR master error bit if we don't clear
+	 * all the EIR bits. Otherwise the edge triggered
+	 * IIR on i965/g4x wouldn't notice that an interrupt
+	 * is still pending. Also some EIR bits can't be
+	 * cleared except by handling the underlying error
+	 * (or by a GPU reset) so we mask any bit that
+	 * remains set.
+	 */
+	emr = I915_READ(EMR);
+	I915_WRITE(EMR, 0xffffffff);
+	I915_WRITE(EMR, emr | *eir_stuck);
+}
+
+static void i9xx_error_irq_handler(struct drm_i915_private *dev_priv,
+				   u32 eir, u32 eir_stuck)
+{
+	DRM_DEBUG("Master Error, EIR 0x%08x\n", eir);
+
+	if (eir_stuck)
+		DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masked\n", eir_stuck);
+}
+
 static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = arg;
@@ -4121,6 +4196,7 @@  static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 
 	do {
 		u32 pipe_stats[I915_MAX_PIPES] = {};
+		u16 eir = 0, eir_stuck = 0;
 		u16 iir;
 
 		iir = I915_READ16(IIR);
@@ -4133,13 +4209,16 @@  static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 		 * signalled in iir */
 		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
+		if (iir & I915_MASTER_ERROR_INTERRUPT)
+			i8xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
+
 		I915_WRITE16(IIR, iir);
 
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[RCS]);
 
-		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
-			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
+		if (iir & I915_MASTER_ERROR_INTERRUPT)
+			i8xx_error_irq_handler(dev_priv, eir, eir_stuck);
 
 		i8xx_pipestat_irq_handler(dev_priv, iir, pipe_stats);
 	} while (0);
@@ -4220,6 +4299,7 @@  static irqreturn_t i915_irq_handler(int irq, void *arg)
 
 	do {
 		u32 pipe_stats[I915_MAX_PIPES] = {};
+		u32 eir = 0, eir_stuck = 0;
 		u32 hotplug_status = 0;
 		u32 iir;
 
@@ -4237,13 +4317,16 @@  static irqreturn_t i915_irq_handler(int irq, void *arg)
 		 * signalled in iir */
 		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
+		if (iir & I915_MASTER_ERROR_INTERRUPT)
+			i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
+
 		I915_WRITE(IIR, iir);
 
 		if (iir & I915_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[RCS]);
 
-		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
-			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
+		if (iir & I915_MASTER_ERROR_INTERRUPT)
+			i9xx_error_irq_handler(dev_priv, eir, eir_stuck);
 
 		if (hotplug_status)
 			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
@@ -4297,14 +4380,14 @@  static int i965_irq_postinstall(struct drm_device *dev)
 		  I915_DISPLAY_PORT_INTERRUPT |
 		  I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
 		  I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
-		  I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
+		  I915_MASTER_ERROR_INTERRUPT);
 
 	enable_mask =
 		I915_ASLE_INTERRUPT |
 		I915_DISPLAY_PORT_INTERRUPT |
 		I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
 		I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |
-		I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT |
+		I915_MASTER_ERROR_INTERRUPT |
 		I915_USER_INTERRUPT;
 
 	if (IS_G4X(dev_priv))
@@ -4364,6 +4447,7 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 
 	do {
 		u32 pipe_stats[I915_MAX_PIPES] = {};
+		u32 eir = 0, eir_stuck = 0;
 		u32 hotplug_status = 0;
 		u32 iir;
 
@@ -4380,6 +4464,9 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 		 * signalled in iir */
 		i9xx_pipestat_irq_ack(dev_priv, iir, pipe_stats);
 
+		if (iir & I915_MASTER_ERROR_INTERRUPT)
+			i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck);
+
 		I915_WRITE(IIR, iir);
 
 		if (iir & I915_USER_INTERRUPT)
@@ -4388,8 +4475,8 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 		if (iir & I915_BSD_USER_INTERRUPT)
 			notify_ring(dev_priv->engine[VCS]);
 
-		if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT)
-			DRM_DEBUG("Command parser error, iir 0x%08x\n", iir);
+		if (iir & I915_MASTER_ERROR_INTERRUPT)
+			i9xx_error_irq_handler(dev_priv, eir, eir_stuck);
 
 		if (hotplug_status)
 			i9xx_hpd_irq_handler(dev_priv, hotplug_status);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 987def26ce82..50a47753014b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2805,7 +2805,6 @@  enum i915_power_well_id {
 #define I915_DISPLAY_PORT_INTERRUPT			(1<<17)
 #define I915_DISPLAY_PIPE_C_HBLANK_INTERRUPT		(1<<16)
 #define I915_MASTER_ERROR_INTERRUPT			(1<<15)
-#define I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
 #define I915_DISPLAY_PIPE_B_HBLANK_INTERRUPT		(1<<14)
 #define I915_GMCH_THERMAL_SENSOR_EVENT_INTERRUPT	(1<<14) /* p-state */
 #define I915_DISPLAY_PIPE_A_HBLANK_INTERRUPT		(1<<13)