On Tue, Apr 25, 2017 at 07:27:32PM -0500, Pierre-Louis Bossart wrote:
On 4/25/17 3:27 PM, ville.syrjala@linux.intel.com wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
vlv_display_irq_postinstall() enables the LPE audio interrupts regardless of whether the LPE audio irq chip has masked/unmasked them. Also the irqchip masking/unmasking doesn't consider the state of the display power well or the device, and hence just leads to dmesg spew when it tries to access the hardware while it's powered down.
If the current way works, then we don't need to do anything in the mask/unmask hooks. If it doesn't work, well, then we'd need to properly track whether the irqchip has masked/unmasked the interrupts when we enable display interrupts. And the mask/unmask hooks would need to check whether display interrupts are even enabled before frobbing with he registers.
So let's just assume the current way works and neuter the mask/unmask hooks. Also clean up vlv_display_irq_postinstall() a bit and stop it from trying to unmask/enable the LPE C interrupt on VLV since it doesn't exist.
No objections, I assumed that we did want to update VLV_IMR and VLV_IIR in the mask/unmask, that was the initial recommendation IIRC
There was also a comment where we removed all tests in vlv_display_irq_postinstall:
if (IS_LPE_AUDIO_ENABLED(dev_priv))if (IS_LPE_AUDIO_IRQ_VALID(dev_priv))I think both of these checks can be removed. We won't unmask the interrupts unless lpe is enabled, so the IIR bits will never be set in that case.
Hmm. Right, so I guess the idea originally was to just enable the LPE interrupts in IER, but leave them masked in IMR until the irqchip unmasks them. But the code wasn't actually doing that because it was appending the bits to 'enable_mask' before it assigned the value to irq_mask. Hence the interrupts were always enabled and unmasked.
Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_irq.c | 15 ++++++-------- drivers/gpu/drm/i915/intel_lpe_audio.c | 36 ---------------------------------- 2 files changed, 6 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fd97fe00cd0d..190f6aa5d15e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2953,7 +2953,6 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv) u32 pipestat_mask; u32 enable_mask; enum pipe pipe;
u32 val;
pipestat_mask = PLANE_FLIP_DONE_INT_STATUS_VLV | PIPE_CRC_DONE_INTERRUPT_STATUS;
@@ -2964,18 +2963,16 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
enable_mask = I915_DISPLAY_PORT_INTERRUPT | I915_DISPLAY_PIPE_A_EVENT_INTERRUPT |
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT;
I915_DISPLAY_PIPE_B_EVENT_INTERRUPT |I915_LPE_PIPE_A_INTERRUPT |I915_LPE_PIPE_B_INTERRUPT;- if (IS_CHERRYVIEW(dev_priv))
enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
enable_mask |= I915_DISPLAY_PIPE_C_EVENT_INTERRUPT |I915_LPE_PIPE_C_INTERRUPT;WARN_ON(dev_priv->irq_mask != ~0);
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_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c index 668f00480d97..292fedf30b00 100644 --- a/drivers/gpu/drm/i915/intel_lpe_audio.c +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c @@ -149,44 +149,10 @@ static void lpe_audio_platdev_destroy(struct drm_i915_private *dev_priv)
static void lpe_audio_irq_unmask(struct irq_data *d) {
- struct drm_i915_private *dev_priv = d->chip_data;
- unsigned long irqflags;
- u32 val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT);- if (IS_CHERRYVIEW(dev_priv))
val |= I915_LPE_PIPE_C_INTERRUPT;- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
- dev_priv->irq_mask &= ~val;
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IMR, dev_priv->irq_mask);
- POSTING_READ(VLV_IMR);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
static void lpe_audio_irq_mask(struct irq_data *d) {
- struct drm_i915_private *dev_priv = d->chip_data;
- unsigned long irqflags;
- u32 val = (I915_LPE_PIPE_A_INTERRUPT |
I915_LPE_PIPE_B_INTERRUPT);- if (IS_CHERRYVIEW(dev_priv))
val |= I915_LPE_PIPE_C_INTERRUPT;- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
- dev_priv->irq_mask |= val;
- I915_WRITE(VLV_IMR, dev_priv->irq_mask);
- I915_WRITE(VLV_IIR, val);
- I915_WRITE(VLV_IIR, val);
- POSTING_READ(VLV_IIR);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
}
static struct irq_chip lpe_audio_irqchip = { @@ -330,8 +296,6 @@ void intel_lpe_audio_teardown(struct drm_i915_private *dev_priv)
desc = irq_to_desc(dev_priv->lpe_audio.irq);
lpe_audio_irq_mask(&desc->irq_data);
lpe_audio_platdev_destroy(dev_priv);
irq_free_desc(dev_priv->lpe_audio.irq);