[alsa-devel] [PATCH 5/8] ALSA: x86: Avoid register accesses during disconnection

[Takashi Iwai]

On Wed, 15 Feb 2017 22:29:33 +0100,
Takashi Iwai wrote:
> 
> It seems that accessing registers during disconnection often leads to
> the GPU pipe error.  The original driver had a similar check in the
> past, but it was lost through refactoring.  Now put a connection check
> in the register access functions.

One thing that came to my mind later is about an interrupt that is
triggered after the disconnection.  Although this shouldn't happen,
it's still better to clear the IRQ if it really happens.

Below is a revised patch to cover it.
topic/intel-lpe-audio branch was updated accordingly, too.


Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH v2] ALSA: x86: Avoid register accesses during disconnection

It seems that accessing registers during disconnection often leads to
the GPU pipe error.  The original driver had a similar check in the
past, but it was lost through refactoring.  Now put a connection check
in the register access functions.

One exception is the irq handler: it still needs to access the raw
register even while disconnected, because it has to read and write to
ACK the irq mask.  Although the irq shouldn't be raised while
disconnected (the stream should have been disabled), let's make it
safer for now.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/x86/intel_hdmi_audio.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index 71f01204a590..8d365718c692 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -190,14 +190,28 @@ static void had_substream_put(struct snd_intelhad *intelhaddata)
 }
 
 /* Register access functions */
+static u32 had_read_register_raw(struct snd_intelhad *ctx, u32 reg)
+{
+	return ioread32(ctx->mmio_start + ctx->had_config_offset + reg);
+}
+
+static void had_write_register_raw(struct snd_intelhad *ctx, u32 reg, u32 val)
+{
+	iowrite32(val, ctx->mmio_start + ctx->had_config_offset + reg);
+}
+
 static void had_read_register(struct snd_intelhad *ctx, u32 reg, u32 *val)
 {
-	*val = ioread32(ctx->mmio_start + ctx->had_config_offset + reg);
+	if (!ctx->connected)
+		*val = 0;
+	else
+		*val = had_read_register_raw(ctx, reg);
 }
 
 static void had_write_register(struct snd_intelhad *ctx, u32 reg, u32 val)
 {
-	iowrite32(val, ctx->mmio_start + ctx->had_config_offset + reg);
+	if (ctx->connected)
+		had_write_register_raw(ctx, reg, val);
 }
 
 /*
@@ -229,6 +243,8 @@ static void had_ack_irqs(struct snd_intelhad *ctx)
 {
 	u32 status_reg;
 
+	if (!ctx->connected)
+		return;
 	had_read_register(ctx, AUD_HDMI_STATUS, &status_reg);
 	status_reg |= HDMI_AUDIO_BUFFER_DONE | HDMI_AUDIO_UNDERRUN;
 	had_write_register(ctx, AUD_HDMI_STATUS, status_reg);
@@ -998,7 +1014,7 @@ static void wait_clear_underrun_bit(struct snd_intelhad *intelhaddata)
  */
 static void had_do_reset(struct snd_intelhad *intelhaddata)
 {
-	if (!intelhaddata->need_reset)
+	if (!intelhaddata->need_reset || !intelhaddata->connected)
 		return;
 
 	/* Reset buffer pointers */
@@ -1526,18 +1542,20 @@ static const struct snd_kcontrol_new had_controls[] = {
 static irqreturn_t display_pipe_interrupt_handler(int irq, void *dev_id)
 {
 	struct snd_intelhad *ctx = dev_id;
-	u32 audio_stat, audio_reg;
+	u32 audio_stat;
 
-	audio_reg = AUD_HDMI_STATUS;
-	had_read_register(ctx, audio_reg, &audio_stat);
+	/* use raw register access to ack IRQs even while disconnected */
+	audio_stat = had_read_register_raw(ctx, AUD_HDMI_STATUS);
 
 	if (audio_stat & HDMI_AUDIO_UNDERRUN) {
-		had_write_register(ctx, audio_reg, HDMI_AUDIO_UNDERRUN);
+		had_write_register_raw(ctx, AUD_HDMI_STATUS,
+				       HDMI_AUDIO_UNDERRUN);
 		had_process_buffer_underrun(ctx);
 	}
 
 	if (audio_stat & HDMI_AUDIO_BUFFER_DONE) {
-		had_write_register(ctx, audio_reg, HDMI_AUDIO_BUFFER_DONE);
+		had_write_register_raw(ctx, AUD_HDMI_STATUS,
+				       HDMI_AUDIO_BUFFER_DONE);
 		had_process_buffer_done(ctx);
 	}
 
-- 
2.11.1