Re: [Intel-gfx] [RFC PATCH v2 4/8] drm/i915: Add support for enabling/disabling hdmi audio interrupts

[Pierre-Louis Bossart]

+/* Added for HDMI Audio */
+int i915_enable_hdmi_audio_int(struct drm_i915_private *dev_priv)
+{
+   unsigned long irqflags;
+   u32 imr, int_bit;
+   int pipe = -1;
+
+   spin_lock_irqsave(_priv->irq_lock, irqflags);
+
+   imr = I915_READ(VLV_IMR);
+
+   if (IS_CHERRYVIEW(_priv->drm)) {
+   pipe = PIPE_C;
+   int_bit = (pipe ? (I915_LPE_PIPE_B_INTERRUPT >>
+   ((pipe - 1) * 9)) :
+   I915_LPE_PIPE_A_INTERRUPT);


Either parametrize the I915_LPE_PIPE_INTERRUPT macro, or just have eg. a
switch here.


ok



But the bigger issue here is the mess with selecting the right bit. I
assume it should either depend on the pipe or port. I can't figure out
what is going on here.

And actually I don't understand why we even need this function. The
irqchip should take care to unmask all the interrupts when the audio
device does its request_irq.


agree, it's not clear to me either. I'll work with Jerome to figure out 
if/how this can be removed.




@@ -3364,6 +3425,14 @@ static void vlv_display_irq_postinstall(struct 
drm_i915_private *dev_priv)

WARN_ON(dev_priv->irq_mask != ~0);

+   if (IS_LPE_AUDIO_ENABLED(dev_priv)) {
+   u32 val = (I915_LPE_PIPE_A_INTERRUPT |
+   I915_LPE_PIPE_B_INTERRUPT |
+   I915_LPE_PIPE_C_INTERRUPT);


'val' seems like a rather pointless local variable.


+
+   enable_mask |= val;


indeed, will fix.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [RFC PATCH v2 4/8] drm/i915: Add support for enabling/disabling hdmi audio interrupts

[Jerome Anand]

API definitions for enabling/disabling hdmi audio interrupts in
different hdmi pipes are implemented.

Signed-off-by: Jerome Anand 
---
 drivers/gpu/drm/i915/i915_irq.c  | 69 
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d8f515f..1e3663f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2867,6 +2867,67 @@ static void gen8_disable_vblank(struct drm_device *dev, 
unsigned int pipe)
spin_unlock_irqrestore(_priv->irq_lock, irqflags);
 }
 
+/* Added for HDMI Audio */
+int i915_enable_hdmi_audio_int(struct drm_i915_private *dev_priv)
+{
+   unsigned long irqflags;
+   u32 imr, int_bit;
+   int pipe = -1;
+
+   spin_lock_irqsave(_priv->irq_lock, irqflags);
+
+   imr = I915_READ(VLV_IMR);
+
+   if (IS_CHERRYVIEW(_priv->drm)) {
+   pipe = PIPE_C;
+   int_bit = (pipe ? (I915_LPE_PIPE_B_INTERRUPT >>
+   ((pipe - 1) * 9)) :
+   I915_LPE_PIPE_A_INTERRUPT);
+   imr &= ~int_bit;
+   } else {
+   /* Audio is on Stream A but uses HDMI PIPE B */
+   pipe = PIPE_B;
+   imr &= ~I915_LPE_PIPE_B_INTERRUPT;
+   }
+
+   I915_WRITE(VLV_IMR, imr);
+   I915_WRITE(VLV_IER, ~imr);
+   POSTING_READ(VLV_IER);
+   spin_unlock_irqrestore(_priv->irq_lock, irqflags);
+
+   return 0;
+}
+
+/* Added for HDMI Audio */
+int i915_disable_hdmi_audio_int(struct drm_i915_private *dev_priv)
+{
+   unsigned long irqflags;
+   u32 imr, int_bit;
+   int pipe = -1;
+
+   spin_lock_irqsave(_priv->irq_lock, irqflags);
+   imr = I915_READ(VLV_IMR);
+
+   if (IS_CHERRYVIEW(_priv->drm)) {
+   pipe = PIPE_C;
+   int_bit = (pipe ? (I915_LPE_PIPE_B_INTERRUPT >>
+   ((pipe - 1) * 9)) :
+   I915_LPE_PIPE_A_INTERRUPT);
+   imr |= int_bit;
+   } else {
+   pipe = PIPE_B;
+   imr |= I915_LPE_PIPE_B_INTERRUPT;
+   }
+
+   I915_WRITE(VLV_IER, ~imr);
+   I915_WRITE(VLV_IMR, imr);
+   POSTING_READ(VLV_IMR);
+
+   spin_unlock_irqrestore(_priv->irq_lock, irqflags);
+
+   return 0;
+}
+
 static bool
 ipehr_is_semaphore_wait(struct intel_engine_cs *engine, u32 ipehr)
 {
@@ -3364,6 +3425,14 @@ static void vlv_display_irq_postinstall(struct 
drm_i915_private *dev_priv)
 
WARN_ON(dev_priv->irq_mask != ~0);
 
+   if (IS_LPE_AUDIO_ENABLED(dev_priv)) {
+   u32 val = (I915_LPE_PIPE_A_INTERRUPT |
+   I915_LPE_PIPE_B_INTERRUPT |
+   I915_LPE_PIPE_C_INTERRUPT);
+
+   enable_mask |= val;
+   }
+
dev_priv->irq_mask = ~enable_mask;
 
GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 30e3f49..e6504ea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1116,6 +1116,8 @@ void gen6_disable_rps_interrupts(struct drm_i915_private 
*dev_priv);
 u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask);
 void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv);
+int i915_enable_hdmi_audio_int(struct drm_i915_private *dev_priv);
+int i915_disable_hdmi_audio_int(struct drm_i915_private *dev_priv);
 static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
 {
/*
-- 
2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH v2 4/8] drm/i915: Add support for enabling/disabling hdmi audio interrupts

[Ville Syrjälä]

On Sat, Oct 01, 2016 at 05:52:38AM +0530, Jerome Anand wrote:
> API definitions for enabling/disabling hdmi audio interrupts in
> different hdmi pipes are implemented.
> 
> Signed-off-by: Jerome Anand 
> ---
>  drivers/gpu/drm/i915/i915_irq.c  | 69 
> 
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  2 files changed, 71 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d8f515f..1e3663f 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2867,6 +2867,67 @@ static void gen8_disable_vblank(struct drm_device 
> *dev, unsigned int pipe)
>   spin_unlock_irqrestore(_priv->irq_lock, irqflags);
>  }
>  
> +/* Added for HDMI Audio */
> +int i915_enable_hdmi_audio_int(struct drm_i915_private *dev_priv)
> +{
> + unsigned long irqflags;
> + u32 imr, int_bit;
> + int pipe = -1;
> +
> + spin_lock_irqsave(_priv->irq_lock, irqflags);
> +
> + imr = I915_READ(VLV_IMR);
> +
> + if (IS_CHERRYVIEW(_priv->drm)) {
> + pipe = PIPE_C;
> + int_bit = (pipe ? (I915_LPE_PIPE_B_INTERRUPT >>
> + ((pipe - 1) * 9)) :
> + I915_LPE_PIPE_A_INTERRUPT);

Either parametrize the I915_LPE_PIPE_INTERRUPT macro, or just have eg. a
switch here.

But the bigger issue here is the mess with selecting the right bit. I
assume it should either depend on the pipe or port. I can't figure out
what is going on here.

And actually I don't understand why we even need this function. The
irqchip should take care to unmask all the interrupts when the audio
device does its request_irq.

> + imr &= ~int_bit;
> + } else {
> + /* Audio is on Stream A but uses HDMI PIPE B */
> + pipe = PIPE_B;
> + imr &= ~I915_LPE_PIPE_B_INTERRUPT;
> + }
> +
> + I915_WRITE(VLV_IMR, imr);
> + I915_WRITE(VLV_IER, ~imr);
> + POSTING_READ(VLV_IER);
> + spin_unlock_irqrestore(_priv->irq_lock, irqflags);
> +
> + return 0;
> +}
> +
> +/* Added for HDMI Audio */
> +int i915_disable_hdmi_audio_int(struct drm_i915_private *dev_priv)
> +{
> + unsigned long irqflags;
> + u32 imr, int_bit;
> + int pipe = -1;
> +
> + spin_lock_irqsave(_priv->irq_lock, irqflags);
> + imr = I915_READ(VLV_IMR);
> +
> + if (IS_CHERRYVIEW(_priv->drm)) {
> + pipe = PIPE_C;
> + int_bit = (pipe ? (I915_LPE_PIPE_B_INTERRUPT >>
> + ((pipe - 1) * 9)) :
> + I915_LPE_PIPE_A_INTERRUPT);
> + imr |= int_bit;
> + } else {
> + pipe = PIPE_B;
> + imr |= I915_LPE_PIPE_B_INTERRUPT;
> + }
> +
> + I915_WRITE(VLV_IER, ~imr);
> + I915_WRITE(VLV_IMR, imr);
> + POSTING_READ(VLV_IMR);
> +
> + spin_unlock_irqrestore(_priv->irq_lock, irqflags);
> +
> + return 0;
> +}
> +
>  static bool
>  ipehr_is_semaphore_wait(struct intel_engine_cs *engine, u32 ipehr)
>  {
> @@ -3364,6 +3425,14 @@ static void vlv_display_irq_postinstall(struct 
> drm_i915_private *dev_priv)
>  
>   WARN_ON(dev_priv->irq_mask != ~0);
>  
> + if (IS_LPE_AUDIO_ENABLED(dev_priv)) {
> + u32 val = (I915_LPE_PIPE_A_INTERRUPT |
> + I915_LPE_PIPE_B_INTERRUPT |
> + I915_LPE_PIPE_C_INTERRUPT);

'val' seems like a rather pointless local variable.

> +
> + enable_mask |= val;
> + }
> +
>   dev_priv->irq_mask = ~enable_mask;
>  
>   GEN5_IRQ_INIT(VLV_, dev_priv->irq_mask, enable_mask);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 30e3f49..e6504ea 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1116,6 +1116,8 @@ void gen6_disable_rps_interrupts(struct 
> drm_i915_private *dev_priv);
>  u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask);
>  void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv);
> +int i915_enable_hdmi_audio_int(struct drm_i915_private *dev_priv);
> +int i915_disable_hdmi_audio_int(struct drm_i915_private *dev_priv);
>  static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
>  {
>   /*
> -- 
> 2.9.3

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