public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: use enum pipe consistently in i915_irq.c
@ 2013-10-21 16:04 Daniel Vetter
  2013-10-21 16:04 ` [PATCH 2/2] drm/i915: refactor ilk display interrupt handling Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2013-10-21 16:04 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

Request by Ville in his review of the CRC stuff. This converts
everything but ilk_display_irq_handler since that needs a bit more
than a simple search&replace to look nice.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h |  4 ++--
 drivers/gpu/drm/i915/i915_irq.c | 37 +++++++++++++++++++------------------
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3a0b60f..6993ab9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1883,10 +1883,10 @@ extern void intel_uncore_check_errors(struct drm_device *dev);
 extern void intel_uncore_fini(struct drm_device *dev);
 
 void
-i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask);
+i915_enable_pipestat(drm_i915_private_t *dev_priv, enum pipe pipe, u32 mask);
 
 void
-i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask);
+i915_disable_pipestat(drm_i915_private_t *dev_priv, enum pipe pipe, u32 mask);
 
 /* i915_gem.c */
 int i915_gem_init_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8f7baad..ce94afc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -442,7 +442,7 @@ done:
 
 
 void
-i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
+i915_enable_pipestat(drm_i915_private_t *dev_priv, enum pipe pipe, u32 mask)
 {
 	u32 reg = PIPESTAT(pipe);
 	u32 pipestat = I915_READ(reg) & 0x7fff0000;
@@ -459,7 +459,7 @@ i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
 }
 
 void
-i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask)
+i915_disable_pipestat(drm_i915_private_t *dev_priv, enum pipe pipe, u32 mask)
 {
 	u32 reg = PIPESTAT(pipe);
 	u32 pipestat = I915_READ(reg) & 0x7fff0000;
@@ -487,9 +487,10 @@ static void i915_enable_asle_pipestat(struct drm_device *dev)
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 
-	i915_enable_pipestat(dev_priv, 1, PIPE_LEGACY_BLC_EVENT_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_LEGACY_BLC_EVENT_ENABLE);
 	if (INTEL_INFO(dev)->gen >= 4)
-		i915_enable_pipestat(dev_priv, 0, PIPE_LEGACY_BLC_EVENT_ENABLE);
+		i915_enable_pipestat(dev_priv, PIPE_A,
+				     PIPE_LEGACY_BLC_EVENT_ENABLE);
 
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
@@ -1594,7 +1595,7 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int i;
+	enum pipe i;
 
 	if (de_iir & DE_ERR_INT_IVB)
 		ivb_err_int_handler(dev);
@@ -1605,7 +1606,7 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 	if (de_iir & DE_GSE_IVB)
 		intel_opregion_asle_intr(dev);
 
-	for (i = 0; i < 3; i++) {
+	for_each_pipe(i) {
 		if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
 			drm_handle_vblank(dev, i);
 		if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
@@ -2034,7 +2035,7 @@ static int valleyview_enable_vblank(struct drm_device *dev, int pipe)
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	imr = I915_READ(VLV_IMR);
-	if (pipe == 0)
+	if (pipe == PIPE_A)
 		imr &= ~I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT;
 	else
 		imr &= ~I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
@@ -2086,7 +2087,7 @@ static void valleyview_disable_vblank(struct drm_device *dev, int pipe)
 	i915_disable_pipestat(dev_priv, pipe,
 			      PIPE_START_VBLANK_INTERRUPT_ENABLE);
 	imr = I915_READ(VLV_IMR);
-	if (pipe == 0)
+	if (pipe == PIPE_A)
 		imr |= I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT;
 	else
 		imr |= I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
@@ -2608,9 +2609,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 	/* Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked check happy. */
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	i915_enable_pipestat(dev_priv, 0, pipestat_enable);
-	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
-	i915_enable_pipestat(dev_priv, 1, pipestat_enable);
+	i915_enable_pipestat(dev_priv, PIPE_A, pipestat_enable);
+	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_EVENT_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_B, pipestat_enable);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	I915_WRITE(VLV_IIR, 0xffffffff);
@@ -2725,8 +2726,8 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 	/* Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked check happy. */
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	i915_enable_pipestat(dev_priv, 0, PIPE_CRC_DONE_ENABLE);
-	i915_enable_pipestat(dev_priv, 1, PIPE_CRC_DONE_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_CRC_DONE_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_ENABLE);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	return 0;
@@ -2908,8 +2909,8 @@ static int i915_irq_postinstall(struct drm_device *dev)
 	/* Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked check happy. */
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	i915_enable_pipestat(dev_priv, 0, PIPE_CRC_DONE_ENABLE);
-	i915_enable_pipestat(dev_priv, 1, PIPE_CRC_DONE_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_CRC_DONE_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_ENABLE);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	return 0;
@@ -3124,9 +3125,9 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	/* Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked check happy. */
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE);
-	i915_enable_pipestat(dev_priv, 0, PIPE_CRC_DONE_ENABLE);
-	i915_enable_pipestat(dev_priv, 1, PIPE_CRC_DONE_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_EVENT_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_CRC_DONE_ENABLE);
+	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_ENABLE);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	/*
-- 
1.8.4.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] drm/i915: refactor ilk display interrupt handling
  2013-10-21 16:04 [PATCH 1/2] drm/i915: use enum pipe consistently in i915_irq.c Daniel Vetter
@ 2013-10-21 16:04 ` Daniel Vetter
  2013-10-21 16:48   ` Ville Syrjälä
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2013-10-21 16:04 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

- Use a for_each_loop and add the corresponding #defines.
- Drop the _ILK postfix on the existing DE_PIPE_VBLANK macro for
  consistency with everything else.
- Also use macros (and add the missing one for plane flips) for the
  ivb display interrupt handler.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 50 +++++++++++++++++------------------------
 drivers/gpu/drm/i915/i915_reg.h |  7 ++++--
 2 files changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ce94afc..062a6d6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1535,6 +1535,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe;
 
 	if (de_iir & DE_AUX_CHANNEL_A)
 		dp_aux_irq_handler(dev);
@@ -1542,37 +1543,26 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 	if (de_iir & DE_GSE)
 		intel_opregion_asle_intr(dev);
 
-	if (de_iir & DE_PIPEA_VBLANK)
-		drm_handle_vblank(dev, 0);
-
-	if (de_iir & DE_PIPEB_VBLANK)
-		drm_handle_vblank(dev, 1);
-
 	if (de_iir & DE_POISON)
 		DRM_ERROR("Poison interrupt\n");
 
-	if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
-		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
-			DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
-
-	if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
-		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
-			DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
-
-	if (de_iir & DE_PIPEA_CRC_DONE)
-		i9xx_pipe_crc_irq_handler(dev, PIPE_A);
+	for_each_pipe(pipe) {
+		if (de_iir & DE_PIPE_VBLANK(pipe))
+			drm_handle_vblank(dev, pipe);
 
-	if (de_iir & DE_PIPEB_CRC_DONE)
-		i9xx_pipe_crc_irq_handler(dev, PIPE_B);
+		if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
+			if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
+				DRM_DEBUG_DRIVER("Pipe %c FIFO underrun\n",
+						 pipe_name(pipe));
 
-	if (de_iir & DE_PLANEA_FLIP_DONE) {
-		intel_prepare_page_flip(dev, 0);
-		intel_finish_page_flip_plane(dev, 0);
-	}
+		if (de_iir & DE_PIPE_CRC_DONE(pipe))
+			i9xx_pipe_crc_irq_handler(dev, pipe);
 
-	if (de_iir & DE_PLANEB_FLIP_DONE) {
-		intel_prepare_page_flip(dev, 1);
-		intel_finish_page_flip_plane(dev, 1);
+		/* plane/pipes map 1:1 on ilk+ */
+		if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
+			intel_prepare_page_flip(dev, pipe);
+			intel_finish_page_flip_plane(dev, pipe);
+		}
 	}
 
 	/* check event from PCH */
@@ -1607,9 +1597,11 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 		intel_opregion_asle_intr(dev);
 
 	for_each_pipe(i) {
-		if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
+		if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
 			drm_handle_vblank(dev, i);
-		if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
+
+		/* plane/pipes map 1:1 on ilk+ */
+		if (de_iir & (DE_PLANE_FLIP_DONE_IVB(i))) {
 			intel_prepare_page_flip(dev, i);
 			intel_finish_page_flip_plane(dev, i);
 		}
@@ -2012,7 +2004,7 @@ static int ironlake_enable_vblank(struct drm_device *dev, int pipe)
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
 	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
-						     DE_PIPE_VBLANK_ILK(pipe);
+						     DE_PIPE_VBLANK(pipe);
 
 	if (!i915_pipe_enabled(dev, pipe))
 		return -EINVAL;
@@ -2070,7 +2062,7 @@ static void ironlake_disable_vblank(struct drm_device *dev, int pipe)
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	unsigned long irqflags;
 	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
-						     DE_PIPE_VBLANK_ILK(pipe);
+						     DE_PIPE_VBLANK(pipe);
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
 	ironlake_disable_display_irq(dev_priv, bit);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c97fc94..1af080a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3928,6 +3928,7 @@
 #define DE_SPRITEA_FLIP_DONE    (1 << 28)
 #define DE_PLANEB_FLIP_DONE     (1 << 27)
 #define DE_PLANEA_FLIP_DONE     (1 << 26)
+#define DE_PLANE_FLIP_DONE(plane) (1 << (26 + (plane)))
 #define DE_PCU_EVENT            (1 << 25)
 #define DE_GTT_FAULT            (1 << 24)
 #define DE_POISON               (1 << 23)
@@ -3944,12 +3945,15 @@
 #define DE_PIPEB_CRC_DONE	(1 << 10)
 #define DE_PIPEB_FIFO_UNDERRUN  (1 << 8)
 #define DE_PIPEA_VBLANK         (1 << 7)
+#define DE_PIPE_VBLANK(pipe)    (1 << (7 + 8*(pipe)))
 #define DE_PIPEA_EVEN_FIELD     (1 << 6)
 #define DE_PIPEA_ODD_FIELD      (1 << 5)
 #define DE_PIPEA_LINE_COMPARE   (1 << 4)
 #define DE_PIPEA_VSYNC          (1 << 3)
 #define DE_PIPEA_CRC_DONE	(1 << 2)
+#define DE_PIPE_CRC_DONE(pipe)	(1 << (2 + 8*(pipe)))
 #define DE_PIPEA_FIFO_UNDERRUN  (1 << 0)
+#define DE_PIPE_FIFO_UNDERRUN(pipe)  (1 << (8*(pipe)))
 
 /* More Ivybridge lolz */
 #define DE_ERR_INT_IVB			(1<<30)
@@ -3965,9 +3969,8 @@
 #define DE_PIPEB_VBLANK_IVB		(1<<5)
 #define DE_SPRITEA_FLIP_DONE_IVB	(1<<4)
 #define DE_PLANEA_FLIP_DONE_IVB		(1<<3)
+#define DE_PLANE_FLIP_DONE_IVB(plane)	(1<< (3 + 5*(plane)))
 #define DE_PIPEA_VBLANK_IVB		(1<<0)
-
-#define DE_PIPE_VBLANK_ILK(pipe)	(1 << ((pipe * 8) + 7))
 #define DE_PIPE_VBLANK_IVB(pipe)	(1 << (pipe * 5))
 
 #define VLV_MASTER_IER			0x4400c /* Gunit master IER */
-- 
1.8.4.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] drm/i915: refactor ilk display interrupt handling
  2013-10-21 16:04 ` [PATCH 2/2] drm/i915: refactor ilk display interrupt handling Daniel Vetter
@ 2013-10-21 16:48   ` Ville Syrjälä
  2013-10-30 10:17     ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Ville Syrjälä @ 2013-10-21 16:48 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development

On Mon, Oct 21, 2013 at 06:04:36PM +0200, Daniel Vetter wrote:
> - Use a for_each_loop and add the corresponding #defines.
> - Drop the _ILK postfix on the existing DE_PIPE_VBLANK macro for
>   consistency with everything else.
> - Also use macros (and add the missing one for plane flips) for the
>   ivb display interrupt handler.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 50 +++++++++++++++++------------------------
>  drivers/gpu/drm/i915/i915_reg.h |  7 ++++--
>  2 files changed, 26 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ce94afc..062a6d6 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1535,6 +1535,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
>  static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe;
>  
>  	if (de_iir & DE_AUX_CHANNEL_A)
>  		dp_aux_irq_handler(dev);
> @@ -1542,37 +1543,26 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
>  	if (de_iir & DE_GSE)
>  		intel_opregion_asle_intr(dev);
>  
> -	if (de_iir & DE_PIPEA_VBLANK)
> -		drm_handle_vblank(dev, 0);
> -
> -	if (de_iir & DE_PIPEB_VBLANK)
> -		drm_handle_vblank(dev, 1);
> -
>  	if (de_iir & DE_POISON)
>  		DRM_ERROR("Poison interrupt\n");
>  
> -	if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
> -		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
> -			DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
> -
> -	if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
> -		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
> -			DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
> -
> -	if (de_iir & DE_PIPEA_CRC_DONE)
> -		i9xx_pipe_crc_irq_handler(dev, PIPE_A);
> +	for_each_pipe(pipe) {
> +		if (de_iir & DE_PIPE_VBLANK(pipe))
> +			drm_handle_vblank(dev, pipe);
>  
> -	if (de_iir & DE_PIPEB_CRC_DONE)
> -		i9xx_pipe_crc_irq_handler(dev, PIPE_B);
> +		if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
> +			if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> +				DRM_DEBUG_DRIVER("Pipe %c FIFO underrun\n",
> +						 pipe_name(pipe));
>  
> -	if (de_iir & DE_PLANEA_FLIP_DONE) {
> -		intel_prepare_page_flip(dev, 0);
> -		intel_finish_page_flip_plane(dev, 0);
> -	}
> +		if (de_iir & DE_PIPE_CRC_DONE(pipe))
> +			i9xx_pipe_crc_irq_handler(dev, pipe);
>  
> -	if (de_iir & DE_PLANEB_FLIP_DONE) {
> -		intel_prepare_page_flip(dev, 1);
> -		intel_finish_page_flip_plane(dev, 1);
> +		/* plane/pipes map 1:1 on ilk+ */
> +		if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
> +			intel_prepare_page_flip(dev, pipe);
> +			intel_finish_page_flip_plane(dev, pipe);
> +		}
>  	}
>  
>  	/* check event from PCH */
> @@ -1607,9 +1597,11 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
>  		intel_opregion_asle_intr(dev);
>  
>  	for_each_pipe(i) {
> -		if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
> +		if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
                             ^                     ^

>  			drm_handle_vblank(dev, i);
> -		if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
> +
> +		/* plane/pipes map 1:1 on ilk+ */
> +		if (de_iir & (DE_PLANE_FLIP_DONE_IVB(i))) {
                             ^                         ^

Just a minor complaint about the useless parens.

But otherwise both patches look good, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  			intel_prepare_page_flip(dev, i);
>  			intel_finish_page_flip_plane(dev, i);
>  		}
> @@ -2012,7 +2004,7 @@ static int ironlake_enable_vblank(struct drm_device *dev, int pipe)
>  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
>  	unsigned long irqflags;
>  	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> -						     DE_PIPE_VBLANK_ILK(pipe);
> +						     DE_PIPE_VBLANK(pipe);
>  
>  	if (!i915_pipe_enabled(dev, pipe))
>  		return -EINVAL;
> @@ -2070,7 +2062,7 @@ static void ironlake_disable_vblank(struct drm_device *dev, int pipe)
>  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
>  	unsigned long irqflags;
>  	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> -						     DE_PIPE_VBLANK_ILK(pipe);
> +						     DE_PIPE_VBLANK(pipe);
>  
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>  	ironlake_disable_display_irq(dev_priv, bit);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c97fc94..1af080a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3928,6 +3928,7 @@
>  #define DE_SPRITEA_FLIP_DONE    (1 << 28)
>  #define DE_PLANEB_FLIP_DONE     (1 << 27)
>  #define DE_PLANEA_FLIP_DONE     (1 << 26)
> +#define DE_PLANE_FLIP_DONE(plane) (1 << (26 + (plane)))
>  #define DE_PCU_EVENT            (1 << 25)
>  #define DE_GTT_FAULT            (1 << 24)
>  #define DE_POISON               (1 << 23)
> @@ -3944,12 +3945,15 @@
>  #define DE_PIPEB_CRC_DONE	(1 << 10)
>  #define DE_PIPEB_FIFO_UNDERRUN  (1 << 8)
>  #define DE_PIPEA_VBLANK         (1 << 7)
> +#define DE_PIPE_VBLANK(pipe)    (1 << (7 + 8*(pipe)))
>  #define DE_PIPEA_EVEN_FIELD     (1 << 6)
>  #define DE_PIPEA_ODD_FIELD      (1 << 5)
>  #define DE_PIPEA_LINE_COMPARE   (1 << 4)
>  #define DE_PIPEA_VSYNC          (1 << 3)
>  #define DE_PIPEA_CRC_DONE	(1 << 2)
> +#define DE_PIPE_CRC_DONE(pipe)	(1 << (2 + 8*(pipe)))
>  #define DE_PIPEA_FIFO_UNDERRUN  (1 << 0)
> +#define DE_PIPE_FIFO_UNDERRUN(pipe)  (1 << (8*(pipe)))
>  
>  /* More Ivybridge lolz */
>  #define DE_ERR_INT_IVB			(1<<30)
> @@ -3965,9 +3969,8 @@
>  #define DE_PIPEB_VBLANK_IVB		(1<<5)
>  #define DE_SPRITEA_FLIP_DONE_IVB	(1<<4)
>  #define DE_PLANEA_FLIP_DONE_IVB		(1<<3)
> +#define DE_PLANE_FLIP_DONE_IVB(plane)	(1<< (3 + 5*(plane)))
>  #define DE_PIPEA_VBLANK_IVB		(1<<0)
> -
> -#define DE_PIPE_VBLANK_ILK(pipe)	(1 << ((pipe * 8) + 7))
>  #define DE_PIPE_VBLANK_IVB(pipe)	(1 << (pipe * 5))
>  
>  #define VLV_MASTER_IER			0x4400c /* Gunit master IER */
> -- 
> 1.8.4.rc3

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] drm/i915: refactor ilk display interrupt handling
  2013-10-21 16:48   ` Ville Syrjälä
@ 2013-10-30 10:17     ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2013-10-30 10:17 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Daniel Vetter, Intel Graphics Development

On Mon, Oct 21, 2013 at 07:48:58PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 21, 2013 at 06:04:36PM +0200, Daniel Vetter wrote:
> > - Use a for_each_loop and add the corresponding #defines.
> > - Drop the _ILK postfix on the existing DE_PIPE_VBLANK macro for
> >   consistency with everything else.
> > - Also use macros (and add the missing one for plane flips) for the
> >   ivb display interrupt handler.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 50 +++++++++++++++++------------------------
> >  drivers/gpu/drm/i915/i915_reg.h |  7 ++++--
> >  2 files changed, 26 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index ce94afc..062a6d6 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1535,6 +1535,7 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
> >  static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	enum pipe pipe;
> >  
> >  	if (de_iir & DE_AUX_CHANNEL_A)
> >  		dp_aux_irq_handler(dev);
> > @@ -1542,37 +1543,26 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
> >  	if (de_iir & DE_GSE)
> >  		intel_opregion_asle_intr(dev);
> >  
> > -	if (de_iir & DE_PIPEA_VBLANK)
> > -		drm_handle_vblank(dev, 0);
> > -
> > -	if (de_iir & DE_PIPEB_VBLANK)
> > -		drm_handle_vblank(dev, 1);
> > -
> >  	if (de_iir & DE_POISON)
> >  		DRM_ERROR("Poison interrupt\n");
> >  
> > -	if (de_iir & DE_PIPEA_FIFO_UNDERRUN)
> > -		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_A, false))
> > -			DRM_DEBUG_DRIVER("Pipe A FIFO underrun\n");
> > -
> > -	if (de_iir & DE_PIPEB_FIFO_UNDERRUN)
> > -		if (intel_set_cpu_fifo_underrun_reporting(dev, PIPE_B, false))
> > -			DRM_DEBUG_DRIVER("Pipe B FIFO underrun\n");
> > -
> > -	if (de_iir & DE_PIPEA_CRC_DONE)
> > -		i9xx_pipe_crc_irq_handler(dev, PIPE_A);
> > +	for_each_pipe(pipe) {
> > +		if (de_iir & DE_PIPE_VBLANK(pipe))
> > +			drm_handle_vblank(dev, pipe);
> >  
> > -	if (de_iir & DE_PIPEB_CRC_DONE)
> > -		i9xx_pipe_crc_irq_handler(dev, PIPE_B);
> > +		if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
> > +			if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
> > +				DRM_DEBUG_DRIVER("Pipe %c FIFO underrun\n",
> > +						 pipe_name(pipe));
> >  
> > -	if (de_iir & DE_PLANEA_FLIP_DONE) {
> > -		intel_prepare_page_flip(dev, 0);
> > -		intel_finish_page_flip_plane(dev, 0);
> > -	}
> > +		if (de_iir & DE_PIPE_CRC_DONE(pipe))
> > +			i9xx_pipe_crc_irq_handler(dev, pipe);
> >  
> > -	if (de_iir & DE_PLANEB_FLIP_DONE) {
> > -		intel_prepare_page_flip(dev, 1);
> > -		intel_finish_page_flip_plane(dev, 1);
> > +		/* plane/pipes map 1:1 on ilk+ */
> > +		if (de_iir & DE_PLANE_FLIP_DONE(pipe)) {
> > +			intel_prepare_page_flip(dev, pipe);
> > +			intel_finish_page_flip_plane(dev, pipe);
> > +		}
> >  	}
> >  
> >  	/* check event from PCH */
> > @@ -1607,9 +1597,11 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
> >  		intel_opregion_asle_intr(dev);
> >  
> >  	for_each_pipe(i) {
> > -		if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i)))
> > +		if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
>                              ^                     ^
> 
> >  			drm_handle_vblank(dev, i);
> > -		if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) {
> > +
> > +		/* plane/pipes map 1:1 on ilk+ */
> > +		if (de_iir & (DE_PLANE_FLIP_DONE_IVB(i))) {
>                              ^                         ^
> 
> Just a minor complaint about the useless parens.

Fixed and both merged, thanks for the review.
-Daniel

> 
> But otherwise both patches look good, so:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> >  			intel_prepare_page_flip(dev, i);
> >  			intel_finish_page_flip_plane(dev, i);
> >  		}
> > @@ -2012,7 +2004,7 @@ static int ironlake_enable_vblank(struct drm_device *dev, int pipe)
> >  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> >  	unsigned long irqflags;
> >  	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> > -						     DE_PIPE_VBLANK_ILK(pipe);
> > +						     DE_PIPE_VBLANK(pipe);
> >  
> >  	if (!i915_pipe_enabled(dev, pipe))
> >  		return -EINVAL;
> > @@ -2070,7 +2062,7 @@ static void ironlake_disable_vblank(struct drm_device *dev, int pipe)
> >  	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> >  	unsigned long irqflags;
> >  	uint32_t bit = (INTEL_INFO(dev)->gen >= 7) ? DE_PIPE_VBLANK_IVB(pipe) :
> > -						     DE_PIPE_VBLANK_ILK(pipe);
> > +						     DE_PIPE_VBLANK(pipe);
> >  
> >  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> >  	ironlake_disable_display_irq(dev_priv, bit);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index c97fc94..1af080a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3928,6 +3928,7 @@
> >  #define DE_SPRITEA_FLIP_DONE    (1 << 28)
> >  #define DE_PLANEB_FLIP_DONE     (1 << 27)
> >  #define DE_PLANEA_FLIP_DONE     (1 << 26)
> > +#define DE_PLANE_FLIP_DONE(plane) (1 << (26 + (plane)))
> >  #define DE_PCU_EVENT            (1 << 25)
> >  #define DE_GTT_FAULT            (1 << 24)
> >  #define DE_POISON               (1 << 23)
> > @@ -3944,12 +3945,15 @@
> >  #define DE_PIPEB_CRC_DONE	(1 << 10)
> >  #define DE_PIPEB_FIFO_UNDERRUN  (1 << 8)
> >  #define DE_PIPEA_VBLANK         (1 << 7)
> > +#define DE_PIPE_VBLANK(pipe)    (1 << (7 + 8*(pipe)))
> >  #define DE_PIPEA_EVEN_FIELD     (1 << 6)
> >  #define DE_PIPEA_ODD_FIELD      (1 << 5)
> >  #define DE_PIPEA_LINE_COMPARE   (1 << 4)
> >  #define DE_PIPEA_VSYNC          (1 << 3)
> >  #define DE_PIPEA_CRC_DONE	(1 << 2)
> > +#define DE_PIPE_CRC_DONE(pipe)	(1 << (2 + 8*(pipe)))
> >  #define DE_PIPEA_FIFO_UNDERRUN  (1 << 0)
> > +#define DE_PIPE_FIFO_UNDERRUN(pipe)  (1 << (8*(pipe)))
> >  
> >  /* More Ivybridge lolz */
> >  #define DE_ERR_INT_IVB			(1<<30)
> > @@ -3965,9 +3969,8 @@
> >  #define DE_PIPEB_VBLANK_IVB		(1<<5)
> >  #define DE_SPRITEA_FLIP_DONE_IVB	(1<<4)
> >  #define DE_PLANEA_FLIP_DONE_IVB		(1<<3)
> > +#define DE_PLANE_FLIP_DONE_IVB(plane)	(1<< (3 + 5*(plane)))
> >  #define DE_PIPEA_VBLANK_IVB		(1<<0)
> > -
> > -#define DE_PIPE_VBLANK_ILK(pipe)	(1 << ((pipe * 8) + 7))
> >  #define DE_PIPE_VBLANK_IVB(pipe)	(1 << (pipe * 5))
> >  
> >  #define VLV_MASTER_IER			0x4400c /* Gunit master IER */
> > -- 
> > 1.8.4.rc3
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-10-30 10:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-21 16:04 [PATCH 1/2] drm/i915: use enum pipe consistently in i915_irq.c Daniel Vetter
2013-10-21 16:04 ` [PATCH 2/2] drm/i915: refactor ilk display interrupt handling Daniel Vetter
2013-10-21 16:48   ` Ville Syrjälä
2013-10-30 10:17     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox