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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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 <jerome.anand@intel.com>
> ---
>  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(&dev_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(&dev_priv->irq_lock, irqflags);
> +
> +	imr = I915_READ(VLV_IMR);
> +
> +	if (IS_CHERRYVIEW(&dev_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(&dev_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(&dev_priv->irq_lock, irqflags);
> +	imr = I915_READ(VLV_IMR);
> +
> +	if (IS_CHERRYVIEW(&dev_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(&dev_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