[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(&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.

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.