[PATCH 0/3] drm/i915: Display irq enable/disable OCD

[PATCH 0/3] drm/i915: Display irq enable/disable OCD

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I spotted that we're duplicating the BDW+ pipe IMR frobbing a few
places, so I figured I'd consolidate that. And while doing that I
also cleaned up the ibx/ilk stuff a bit as well.

Ville Syrjälä (3):
  drm/i915: Make ibx_{enable,disable}_display_interrupt() static inlines
  drm/i915: Make ironlake_{enable,disable}_display_irq() static inlines
  drm/i915: Introduce bdw_{update,enable,disable}_pipe_irq()

 drivers/gpu/drm/i915/i915_drv.h            | 46 +++++++++++++++++----
 drivers/gpu/drm/i915/i915_irq.c            | 65 +++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_fifo_underrun.c | 16 +++-----
 3 files changed, 85 insertions(+), 42 deletions(-)

-- 
2.4.10

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

[PATCH 1/3] drm/i915: Make ibx_{enable, disable}_display_interrupt() static inlines

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

No reason why ibx_{enable,disable}_display_interrupt() couldn't be
static inlines instead of cpp macros.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a47e0f4fab56..43087d513637 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2756,10 +2756,17 @@ ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask);
 void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
 				  uint32_t interrupt_mask,
 				  uint32_t enabled_irq_mask);
-#define ibx_enable_display_interrupt(dev_priv, bits) \
-	ibx_display_interrupt_update((dev_priv), (bits), (bits))
-#define ibx_disable_display_interrupt(dev_priv, bits) \
-	ibx_display_interrupt_update((dev_priv), (bits), 0)
+static inline void
+ibx_enable_display_interrupt(struct drm_i915_private *dev_priv, uint32_t bits)
+{
+	ibx_display_interrupt_update(dev_priv, bits, bits);
+}
+static inline void
+ibx_disable_display_interrupt(struct drm_i915_private *dev_priv, uint32_t bits)
+{
+	ibx_display_interrupt_update(dev_priv, bits, 0);
+}
+
 
 /* i915_gem.c */
 int i915_gem_create_ioctl(struct drm_device *dev, void *data,
-- 
2.4.10

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

[PATCH 2/3] drm/i915: Make ironlake_{enable, disable}_display_irq() static inlines

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

ironlake_{enable,disable}_display_irq() each just call
ilk_update_display_irq() so let's make them static inlines.

While at it s/ironlake/ilk/ to make things shorter, and a bit more
consistent with the ibx functions.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            | 17 +++++++++++++----
 drivers/gpu/drm/i915/i915_irq.c            | 24 ++++++------------------
 drivers/gpu/drm/i915/intel_fifo_underrun.c |  8 ++++----
 3 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 43087d513637..b9f86a73c543 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2749,10 +2749,19 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv);
 void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
 				   uint32_t mask,
 				   uint32_t bits);
-void
-ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask);
-void
-ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask);
+void ilk_update_display_irq(struct drm_i915_private *dev_priv,
+			    uint32_t interrupt_mask,
+			    uint32_t enabled_irq_mask);
+static inline void
+ilk_enable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits)
+{
+	ilk_update_display_irq(dev_priv, bits, bits);
+}
+static inline void
+ilk_disable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits)
+{
+	ilk_update_display_irq(dev_priv, bits, 0);
+}
 void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
 				  uint32_t interrupt_mask,
 				  uint32_t enabled_irq_mask);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c8ba94968aaf..5aea557f3776 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -215,9 +215,9 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
  * @interrupt_mask: mask of interrupt bits to update
  * @enabled_irq_mask: mask of interrupt bits to enable
  */
-static void ilk_update_display_irq(struct drm_i915_private *dev_priv,
-				   uint32_t interrupt_mask,
-				   uint32_t enabled_irq_mask)
+void ilk_update_display_irq(struct drm_i915_private *dev_priv,
+			    uint32_t interrupt_mask,
+			    uint32_t enabled_irq_mask)
 {
 	uint32_t new_val;
 
@@ -239,18 +239,6 @@ static void ilk_update_display_irq(struct drm_i915_private *dev_priv,
 	}
 }
 
-void
-ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
-{
-	ilk_update_display_irq(dev_priv, mask, mask);
-}
-
-void
-ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
-{
-	ilk_update_display_irq(dev_priv, mask, 0);
-}
-
 /**
  * ilk_update_gt_irq - update GTIMR
  * @dev_priv: driver private
@@ -2645,7 +2633,7 @@ static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe)
 						     DE_PIPE_VBLANK(pipe);
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_enable_display_irq(dev_priv, bit);
+	ilk_enable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 
 	return 0;
@@ -2700,7 +2688,7 @@ static void ironlake_disable_vblank(struct drm_device *dev, unsigned int pipe)
 						     DE_PIPE_VBLANK(pipe);
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	ironlake_disable_display_irq(dev_priv, bit);
+	ilk_disable_display_irq(dev_priv, bit);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
@@ -3452,7 +3440,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 		 * setup is guaranteed to run in single-threaded context. But we
 		 * need it to make the assert_spin_locked happy. */
 		spin_lock_irq(&dev_priv->irq_lock);
-		ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT);
+		ilk_enable_display_irq(dev_priv, DE_PCU_EVENT);
 		spin_unlock_irq(&dev_priv->irq_lock);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index 7ae182d0594b..48bd079bdb06 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -128,9 +128,9 @@ static void ironlake_set_fifo_underrun_reporting(struct drm_device *dev,
 					  DE_PIPEB_FIFO_UNDERRUN;
 
 	if (enable)
-		ironlake_enable_display_irq(dev_priv, bit);
+		ilk_enable_display_irq(dev_priv, bit);
 	else
-		ironlake_disable_display_irq(dev_priv, bit);
+		ilk_disable_display_irq(dev_priv, bit);
 }
 
 static void ivybridge_check_fifo_underruns(struct intel_crtc *crtc)
@@ -161,9 +161,9 @@ static void ivybridge_set_fifo_underrun_reporting(struct drm_device *dev,
 		if (!ivb_can_enable_err_int(dev))
 			return;
 
-		ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB);
+		ilk_enable_display_irq(dev_priv, DE_ERR_INT_IVB);
 	} else {
-		ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
+		ilk_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
 
 		if (old &&
 		    I915_READ(GEN7_ERR_INT) & ERR_INT_FIFO_UNDERRUN(pipe)) {
-- 
2.4.10

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

[PATCH 3/3] drm/i915: Introduce bdw_{update, enable, disable}_pipe_irq()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pull the BDW+ DE pipe interrupt mask frobbing into a central place,
like we have for other platforms.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            | 14 ++++++++++
 drivers/gpu/drm/i915/i915_irq.c            | 41 +++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_fifo_underrun.c |  8 ++----
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b9f86a73c543..c3330ff5016d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2762,6 +2762,20 @@ ilk_disable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits)
 {
 	ilk_update_display_irq(dev_priv, bits, 0);
 }
+void bdw_update_pipe_irq(struct drm_i915_private *dev_priv,
+			 enum pipe pipe,
+			 uint32_t interrupt_mask,
+			 uint32_t enabled_irq_mask);
+static inline void bdw_enable_pipe_irq(struct drm_i915_private *dev_priv,
+				       enum pipe pipe, uint32_t bits)
+{
+	bdw_update_pipe_irq(dev_priv, pipe, bits, bits);
+}
+static inline void bdw_disable_pipe_irq(struct drm_i915_private *dev_priv,
+					enum pipe pipe, uint32_t bits)
+{
+	bdw_update_pipe_irq(dev_priv, pipe, bits, 0);
+}
 void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
 				  uint32_t interrupt_mask,
 				  uint32_t enabled_irq_mask);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5aea557f3776..92389a9bb301 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -438,6 +438,38 @@ static void bdw_update_port_irq(struct drm_i915_private *dev_priv,
 }
 
 /**
+  * bdw_update_pipe_irq - update DE pipe interrupt
+  * @dev_priv: driver private
+  * @pipe: pipe whose interrupt to update
+  * @interrupt_mask: mask of interrupt bits to update
+  * @enabled_irq_mask: mask of interrupt bits to enable
+  */
+void bdw_update_pipe_irq(struct drm_i915_private *dev_priv,
+			 enum pipe pipe,
+			 uint32_t interrupt_mask,
+			 uint32_t enabled_irq_mask)
+{
+	uint32_t new_val;
+
+	assert_spin_locked(&dev_priv->irq_lock);
+
+	WARN_ON(enabled_irq_mask & ~interrupt_mask);
+
+	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
+		return;
+
+	new_val = dev_priv->de_irq_mask[pipe];
+	new_val &= ~interrupt_mask;
+	new_val |= (~enabled_irq_mask & interrupt_mask);
+
+	if (new_val != dev_priv->de_irq_mask[pipe]) {
+		dev_priv->de_irq_mask[pipe] = new_val;
+		I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
+		POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
+	}
+}
+
+/**
  * ibx_display_interrupt_update - update SDEIMR
  * @dev_priv: driver private
  * @interrupt_mask: mask of interrupt bits to update
@@ -2658,10 +2690,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe)
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_VBLANK;
-	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
-	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
+	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
 	return 0;
 }
 
@@ -2709,9 +2740,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe)
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
-	dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_VBLANK;
-	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
-	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
+	bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index 48bd079bdb06..bda526660e20 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -178,14 +178,10 @@ static void broadwell_set_fifo_underrun_reporting(struct drm_device *dev,
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	assert_spin_locked(&dev_priv->irq_lock);
-
 	if (enable)
-		dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_FIFO_UNDERRUN;
+		bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN);
 	else
-		dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_FIFO_UNDERRUN;
-	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
-	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
+		bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN);
 }
 
 static void ibx_set_fifo_underrun_reporting(struct drm_device *dev,
-- 
2.4.10

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

Re: [PATCH 1/3] drm/i915: Make ibx_{enable, disable}_display_interrupt() static inlines

[Daniel Vetter]

On Mon, Nov 23, 2015 at 06:06:15PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> No reason why ibx_{enable,disable}_display_interrupt() couldn't be
> static inlines instead of cpp macros.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Not really an opinion on this, but we have this style in a bunch of
places. Anyway:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a47e0f4fab56..43087d513637 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2756,10 +2756,17 @@ ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask);
>  void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
>  				  uint32_t interrupt_mask,
>  				  uint32_t enabled_irq_mask);
> -#define ibx_enable_display_interrupt(dev_priv, bits) \
> -	ibx_display_interrupt_update((dev_priv), (bits), (bits))
> -#define ibx_disable_display_interrupt(dev_priv, bits) \
> -	ibx_display_interrupt_update((dev_priv), (bits), 0)
> +static inline void
> +ibx_enable_display_interrupt(struct drm_i915_private *dev_priv, uint32_t bits)
> +{
> +	ibx_display_interrupt_update(dev_priv, bits, bits);
> +}
> +static inline void
> +ibx_disable_display_interrupt(struct drm_i915_private *dev_priv, uint32_t bits)
> +{
> +	ibx_display_interrupt_update(dev_priv, bits, 0);
> +}
> +
>  
>  /* i915_gem.c */
>  int i915_gem_create_ioctl(struct drm_device *dev, void *data,
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Make ironlake_{enable, disable}_display_irq() static inlines

[Daniel Vetter]

On Mon, Nov 23, 2015 at 06:06:16PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> ironlake_{enable,disable}_display_irq() each just call
> ilk_update_display_irq() so let's make them static inlines.
> 
> While at it s/ironlake/ilk/ to make things shorter, and a bit more
> consistent with the ibx functions.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yeah we might want to standarize on the TLA platform acronyms. And
document that somewhere.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_drv.h            | 17 +++++++++++++----
>  drivers/gpu/drm/i915/i915_irq.c            | 24 ++++++------------------
>  drivers/gpu/drm/i915/intel_fifo_underrun.c |  8 ++++----
>  3 files changed, 23 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 43087d513637..b9f86a73c543 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2749,10 +2749,19 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv);
>  void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
>  				   uint32_t mask,
>  				   uint32_t bits);
> -void
> -ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask);
> -void
> -ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask);
> +void ilk_update_display_irq(struct drm_i915_private *dev_priv,
> +			    uint32_t interrupt_mask,
> +			    uint32_t enabled_irq_mask);
> +static inline void
> +ilk_enable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits)
> +{
> +	ilk_update_display_irq(dev_priv, bits, bits);
> +}
> +static inline void
> +ilk_disable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits)
> +{
> +	ilk_update_display_irq(dev_priv, bits, 0);
> +}
>  void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
>  				  uint32_t interrupt_mask,
>  				  uint32_t enabled_irq_mask);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index c8ba94968aaf..5aea557f3776 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -215,9 +215,9 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
>   * @interrupt_mask: mask of interrupt bits to update
>   * @enabled_irq_mask: mask of interrupt bits to enable
>   */
> -static void ilk_update_display_irq(struct drm_i915_private *dev_priv,
> -				   uint32_t interrupt_mask,
> -				   uint32_t enabled_irq_mask)
> +void ilk_update_display_irq(struct drm_i915_private *dev_priv,
> +			    uint32_t interrupt_mask,
> +			    uint32_t enabled_irq_mask)
>  {
>  	uint32_t new_val;
>  
> @@ -239,18 +239,6 @@ static void ilk_update_display_irq(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> -void
> -ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
> -{
> -	ilk_update_display_irq(dev_priv, mask, mask);
> -}
> -
> -void
> -ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
> -{
> -	ilk_update_display_irq(dev_priv, mask, 0);
> -}
> -
>  /**
>   * ilk_update_gt_irq - update GTIMR
>   * @dev_priv: driver private
> @@ -2645,7 +2633,7 @@ static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe)
>  						     DE_PIPE_VBLANK(pipe);
>  
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> -	ironlake_enable_display_irq(dev_priv, bit);
> +	ilk_enable_display_irq(dev_priv, bit);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
>  	return 0;
> @@ -2700,7 +2688,7 @@ static void ironlake_disable_vblank(struct drm_device *dev, unsigned int pipe)
>  						     DE_PIPE_VBLANK(pipe);
>  
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> -	ironlake_disable_display_irq(dev_priv, bit);
> +	ilk_disable_display_irq(dev_priv, bit);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  }
>  
> @@ -3452,7 +3440,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>  		 * setup is guaranteed to run in single-threaded context. But we
>  		 * need it to make the assert_spin_locked happy. */
>  		spin_lock_irq(&dev_priv->irq_lock);
> -		ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT);
> +		ilk_enable_display_irq(dev_priv, DE_PCU_EVENT);
>  		spin_unlock_irq(&dev_priv->irq_lock);
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index 7ae182d0594b..48bd079bdb06 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -128,9 +128,9 @@ static void ironlake_set_fifo_underrun_reporting(struct drm_device *dev,
>  					  DE_PIPEB_FIFO_UNDERRUN;
>  
>  	if (enable)
> -		ironlake_enable_display_irq(dev_priv, bit);
> +		ilk_enable_display_irq(dev_priv, bit);
>  	else
> -		ironlake_disable_display_irq(dev_priv, bit);
> +		ilk_disable_display_irq(dev_priv, bit);
>  }
>  
>  static void ivybridge_check_fifo_underruns(struct intel_crtc *crtc)
> @@ -161,9 +161,9 @@ static void ivybridge_set_fifo_underrun_reporting(struct drm_device *dev,
>  		if (!ivb_can_enable_err_int(dev))
>  			return;
>  
> -		ironlake_enable_display_irq(dev_priv, DE_ERR_INT_IVB);
> +		ilk_enable_display_irq(dev_priv, DE_ERR_INT_IVB);
>  	} else {
> -		ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
> +		ilk_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
>  
>  		if (old &&
>  		    I915_READ(GEN7_ERR_INT) & ERR_INT_FIFO_UNDERRUN(pipe)) {
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Introduce bdw_{update, enable, disable}_pipe_irq()

[Daniel Vetter]

On Mon, Nov 23, 2015 at 06:06:17PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pull the BDW+ DE pipe interrupt mask frobbing into a central place,
> like we have for other platforms.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            | 14 ++++++++++
>  drivers/gpu/drm/i915/i915_irq.c            | 41 +++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_fifo_underrun.c |  8 ++----
>  3 files changed, 51 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b9f86a73c543..c3330ff5016d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2762,6 +2762,20 @@ ilk_disable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits)
>  {
>  	ilk_update_display_irq(dev_priv, bits, 0);
>  }
> +void bdw_update_pipe_irq(struct drm_i915_private *dev_priv,
> +			 enum pipe pipe,
> +			 uint32_t interrupt_mask,
> +			 uint32_t enabled_irq_mask);
> +static inline void bdw_enable_pipe_irq(struct drm_i915_private *dev_priv,
> +				       enum pipe pipe, uint32_t bits)
> +{
> +	bdw_update_pipe_irq(dev_priv, pipe, bits, bits);
> +}
> +static inline void bdw_disable_pipe_irq(struct drm_i915_private *dev_priv,
> +					enum pipe pipe, uint32_t bits)
> +{
> +	bdw_update_pipe_irq(dev_priv, pipe, bits, 0);
> +}
>  void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
>  				  uint32_t interrupt_mask,
>  				  uint32_t enabled_irq_mask);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 5aea557f3776..92389a9bb301 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -438,6 +438,38 @@ static void bdw_update_port_irq(struct drm_i915_private *dev_priv,
>  }
>  
>  /**
> +  * bdw_update_pipe_irq - update DE pipe interrupt
> +  * @dev_priv: driver private
> +  * @pipe: pipe whose interrupt to update
> +  * @interrupt_mask: mask of interrupt bits to update
> +  * @enabled_irq_mask: mask of interrupt bits to enable
> +  */

kerneldoc is aligned like

/**
 * 
 */

With that OCD in the OCD addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +void bdw_update_pipe_irq(struct drm_i915_private *dev_priv,
> +			 enum pipe pipe,
> +			 uint32_t interrupt_mask,
> +			 uint32_t enabled_irq_mask)
> +{
> +	uint32_t new_val;
> +
> +	assert_spin_locked(&dev_priv->irq_lock);
> +
> +	WARN_ON(enabled_irq_mask & ~interrupt_mask);
> +
> +	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> +		return;
> +
> +	new_val = dev_priv->de_irq_mask[pipe];
> +	new_val &= ~interrupt_mask;
> +	new_val |= (~enabled_irq_mask & interrupt_mask);
> +
> +	if (new_val != dev_priv->de_irq_mask[pipe]) {
> +		dev_priv->de_irq_mask[pipe] = new_val;
> +		I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> +		POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> +	}
> +}
> +
> +/**
>   * ibx_display_interrupt_update - update SDEIMR
>   * @dev_priv: driver private
>   * @interrupt_mask: mask of interrupt bits to update
> @@ -2658,10 +2690,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe)
>  	unsigned long irqflags;
>  
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> -	dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_VBLANK;
> -	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> -	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> +	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> +
>  	return 0;
>  }
>  
> @@ -2709,9 +2740,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe)
>  	unsigned long irqflags;
>  
>  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> -	dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_VBLANK;
> -	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> -	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> +	bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index 48bd079bdb06..bda526660e20 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -178,14 +178,10 @@ static void broadwell_set_fifo_underrun_reporting(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	assert_spin_locked(&dev_priv->irq_lock);
> -
>  	if (enable)
> -		dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_FIFO_UNDERRUN;
> +		bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN);
>  	else
> -		dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_FIFO_UNDERRUN;
> -	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> -	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> +		bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN);
>  }
>  
>  static void ibx_set_fifo_underrun_reporting(struct drm_device *dev,
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Introduce bdw_{update, enable, disable}_pipe_irq()

[Ville Syrjälä]

On Tue, Nov 24, 2015 at 06:52:09PM +0100, Daniel Vetter wrote:
> On Mon, Nov 23, 2015 at 06:06:17PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Pull the BDW+ DE pipe interrupt mask frobbing into a central place,
> > like we have for other platforms.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h            | 14 ++++++++++
> >  drivers/gpu/drm/i915/i915_irq.c            | 41 +++++++++++++++++++++++++-----
> >  drivers/gpu/drm/i915/intel_fifo_underrun.c |  8 ++----
> >  3 files changed, 51 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index b9f86a73c543..c3330ff5016d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2762,6 +2762,20 @@ ilk_disable_display_irq(struct drm_i915_private *dev_priv, uint32_t bits)
> >  {
> >  	ilk_update_display_irq(dev_priv, bits, 0);
> >  }
> > +void bdw_update_pipe_irq(struct drm_i915_private *dev_priv,
> > +			 enum pipe pipe,
> > +			 uint32_t interrupt_mask,
> > +			 uint32_t enabled_irq_mask);
> > +static inline void bdw_enable_pipe_irq(struct drm_i915_private *dev_priv,
> > +				       enum pipe pipe, uint32_t bits)
> > +{
> > +	bdw_update_pipe_irq(dev_priv, pipe, bits, bits);
> > +}
> > +static inline void bdw_disable_pipe_irq(struct drm_i915_private *dev_priv,
> > +					enum pipe pipe, uint32_t bits)
> > +{
> > +	bdw_update_pipe_irq(dev_priv, pipe, bits, 0);
> > +}
> >  void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
> >  				  uint32_t interrupt_mask,
> >  				  uint32_t enabled_irq_mask);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 5aea557f3776..92389a9bb301 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -438,6 +438,38 @@ static void bdw_update_port_irq(struct drm_i915_private *dev_priv,
> >  }
> >  
> >  /**
> > +  * bdw_update_pipe_irq - update DE pipe interrupt
> > +  * @dev_priv: driver private
> > +  * @pipe: pipe whose interrupt to update
> > +  * @interrupt_mask: mask of interrupt bits to update
> > +  * @enabled_irq_mask: mask of interrupt bits to enable
> > +  */
> 
> kerneldoc is aligned like
> 
> /**
>  * 
>  */

Hmm. Copy pasted from bdw_update_port_irq() so I guess I get to fix that
one as well. I'll send a separate patch for it.

> 
> With that OCD in the OCD addressed:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > +void bdw_update_pipe_irq(struct drm_i915_private *dev_priv,
> > +			 enum pipe pipe,
> > +			 uint32_t interrupt_mask,
> > +			 uint32_t enabled_irq_mask)
> > +{
> > +	uint32_t new_val;
> > +
> > +	assert_spin_locked(&dev_priv->irq_lock);
> > +
> > +	WARN_ON(enabled_irq_mask & ~interrupt_mask);
> > +
> > +	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> > +		return;
> > +
> > +	new_val = dev_priv->de_irq_mask[pipe];
> > +	new_val &= ~interrupt_mask;
> > +	new_val |= (~enabled_irq_mask & interrupt_mask);
> > +
> > +	if (new_val != dev_priv->de_irq_mask[pipe]) {
> > +		dev_priv->de_irq_mask[pipe] = new_val;
> > +		I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> > +		POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> > +	}
> > +}
> > +
> > +/**
> >   * ibx_display_interrupt_update - update SDEIMR
> >   * @dev_priv: driver private
> >   * @interrupt_mask: mask of interrupt bits to update
> > @@ -2658,10 +2690,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe)
> >  	unsigned long irqflags;
> >  
> >  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> > -	dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_VBLANK;
> > -	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> > -	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> > +	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
> >  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -2709,9 +2740,7 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe)
> >  	unsigned long irqflags;
> >  
> >  	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> > -	dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_VBLANK;
> > -	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> > -	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> > +	bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
> >  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > index 48bd079bdb06..bda526660e20 100644
> > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> > @@ -178,14 +178,10 @@ static void broadwell_set_fifo_underrun_reporting(struct drm_device *dev,
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > -	assert_spin_locked(&dev_priv->irq_lock);
> > -
> >  	if (enable)
> > -		dev_priv->de_irq_mask[pipe] &= ~GEN8_PIPE_FIFO_UNDERRUN;
> > +		bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN);
> >  	else
> > -		dev_priv->de_irq_mask[pipe] |= GEN8_PIPE_FIFO_UNDERRUN;
> > -	I915_WRITE(GEN8_DE_PIPE_IMR(pipe), dev_priv->de_irq_mask[pipe]);
> > -	POSTING_READ(GEN8_DE_PIPE_IMR(pipe));
> > +		bdw_disable_pipe_irq(dev_priv, pipe, GEN8_PIPE_FIFO_UNDERRUN);
> >  }
> >  
> >  static void ibx_set_fifo_underrun_reporting(struct drm_device *dev,
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] drm/i915: Display irq enable/disable OCD

[Ville Syrjälä]

On Mon, Nov 23, 2015 at 06:06:14PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I spotted that we're duplicating the BDW+ pipe IMR frobbing a few
> places, so I figured I'd consolidate that. And while doing that I
> also cleaned up the ibx/ilk stuff a bit as well.
> 
> Ville Syrjälä (3):
>   drm/i915: Make ibx_{enable,disable}_display_interrupt() static inlines
>   drm/i915: Make ironlake_{enable,disable}_display_irq() static inlines
>   drm/i915: Introduce bdw_{update,enable,disable}_pipe_irq()

Series pushed to dinq. Thanks for the reviews.

> 
>  drivers/gpu/drm/i915/i915_drv.h            | 46 +++++++++++++++++----
>  drivers/gpu/drm/i915/i915_irq.c            | 65 +++++++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_fifo_underrun.c | 16 +++-----
>  3 files changed, 85 insertions(+), 42 deletions(-)
> 
> -- 
> 2.4.10

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx