[alsa-devel] Bug in sound-unstable-2.6 ice1724

Takashi Iwai tiwai at suse.de
Wed Nov 26 16:51:12 CET 2008


At Wed, 05 Nov 2008 17:57:03 +0100,
I wrote:
> 
> At Wed, 05 Nov 2008 17:40:58 +0100,
> I wrote:
> > 
> > At Wed, 5 Nov 2008 17:36:57 +0100,
> > =?UTF-8?Q?Vedran_Mileti=C4=87?= wrote:
> > > 
> > > Works.
> > > 
> > > vedran at kalopsia:~$ cat /proc/asound/card1/midi0
> > > ICE1724 MIDI
> > > 
> > > Output 0
> > >   Tx bytes     : 20524
> > >   Mode         : native
> > >   Buffer size  : 4096
> > >   Avail        : 4096
> > > Input 0
> > >   Rx bytes     : 0
> > > 
> > > mplayer also doesn't fill dmesg with anything. Seems better than second one.
> > 
> > Hm, OK, then we can put the first patch, then improve if possible later.
> 
> I updated git and snapshot now.  It'll take some time it's sync'ed, though.
> 
> The below is the revised patch to enable/disable the TX irq more
> dynamically.  Give it a try on the top of the new snapshot (where the
> first fix is already applied).

Doh, I forgot to attach the patch.  The below is the one.

Please, if anyone can test it, let me know whether it works.
This would be nice to have in the next kernel.


thanks,

Takashi

===

From 33f96ee2c14b256dfe3a9cfadc6385c72fbe80da Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai at suse.de>
Date: Wed, 12 Nov 2008 16:42:44 +0100
Subject: [PATCH] ALSA: ice1724 - Dynamic MIDI TX irq control

MIDI_TX IRQ seems always pending when any bytes on FIFO is available.
Thus, it's better to enable MPU_TX only when any bytres are really
stored in the substream, and disables immediately when the queue
becomes empty.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/pci/ice1712/ice1724.c |   43 +++++++++++++++++++++++++++----------------
 1 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/sound/pci/ice1712/ice1724.c b/sound/pci/ice1712/ice1724.c
index 0dfa054..009601b 100644
--- a/sound/pci/ice1712/ice1724.c
+++ b/sound/pci/ice1712/ice1724.c
@@ -241,6 +241,8 @@ get_rawmidi_substream(struct snd_ice1712 *ice, unsigned int stream)
 				struct snd_rawmidi_substream, list);
 }
 
+static void enable_midi_irq(struct snd_ice1712 *ice, u8 flag, int enable);
+
 static void vt1724_midi_write(struct snd_ice1712 *ice)
 {
 	struct snd_rawmidi_substream *s;
@@ -254,6 +256,11 @@ static void vt1724_midi_write(struct snd_ice1712 *ice)
 		for (i = 0; i < count; ++i)
 			outb(buffer[i], ICEREG1724(ice, MPU_DATA));
 	}
+	/* mask irq when all bytes have been transmitted.
+	 * enabled again in output_trigger when the new data comes in.
+	 */
+	enable_midi_irq(ice, VT1724_IRQ_MPU_TX,
+			!snd_rawmidi_transmit_empty(s));
 }
 
 static void vt1724_midi_read(struct snd_ice1712 *ice)
@@ -272,31 +279,34 @@ static void vt1724_midi_read(struct snd_ice1712 *ice)
 	}
 }
 
-static void vt1724_enable_midi_irq(struct snd_rawmidi_substream *substream,
-				   u8 flag, int enable)
+/* call with ice->reg_lock */
+static void enable_midi_irq(struct snd_ice1712 *ice, u8 flag, int enable)
 {
-	struct snd_ice1712 *ice = substream->rmidi->private_data;
-	u8 mask;
-
-	spin_lock_irq(&ice->reg_lock);
-	mask = inb(ICEREG1724(ice, IRQMASK));
+	u8 mask = inb(ICEREG1724(ice, IRQMASK));
 	if (enable)
 		mask &= ~flag;
 	else
 		mask |= flag;
 	outb(mask, ICEREG1724(ice, IRQMASK));
+}
+
+static void vt1724_enable_midi_irq(struct snd_rawmidi_substream *substream,
+				   u8 flag, int enable)
+{
+	struct snd_ice1712 *ice = substream->rmidi->private_data;
+
+	spin_lock_irq(&ice->reg_lock);
+	enable_midi_irq(ice, flag, enable);
 	spin_unlock_irq(&ice->reg_lock);
 }
 
 static int vt1724_midi_output_open(struct snd_rawmidi_substream *s)
 {
-	vt1724_enable_midi_irq(s, VT1724_IRQ_MPU_TX, 1);
 	return 0;
 }
 
 static int vt1724_midi_output_close(struct snd_rawmidi_substream *s)
 {
-	vt1724_enable_midi_irq(s, VT1724_IRQ_MPU_TX, 0);
 	return 0;
 }
 
@@ -311,6 +321,7 @@ static void vt1724_midi_output_trigger(struct snd_rawmidi_substream *s, int up)
 		vt1724_midi_write(ice);
 	} else {
 		ice->midi_output = 0;
+		enable_midi_irq(ice, VT1724_IRQ_MPU_TX, 0);
 	}
 	spin_unlock_irqrestore(&ice->reg_lock, flags);
 }
@@ -320,6 +331,7 @@ static void vt1724_midi_output_drain(struct snd_rawmidi_substream *s)
 	struct snd_ice1712 *ice = s->rmidi->private_data;
 	unsigned long timeout;
 
+	vt1724_enable_midi_irq(s, VT1724_IRQ_MPU_TX, 0);
 	/* 32 bytes should be transmitted in less than about 12 ms */
 	timeout = jiffies + msecs_to_jiffies(15);
 	do {
@@ -389,24 +401,24 @@ static irqreturn_t snd_vt1724_interrupt(int irq, void *dev_id)
 		status &= status_mask;
 		if (status == 0)
 			break;
+		spin_lock(&ice->reg_lock);
 		if (++timeout > 10) {
 			status = inb(ICEREG1724(ice, IRQSTAT));
 			printk(KERN_ERR "ice1724: Too long irq loop, "
 			       "status = 0x%x\n", status);
 			if (status & VT1724_IRQ_MPU_TX) {
 				printk(KERN_ERR "ice1724: Disabling MPU_TX\n");
-				outb(inb(ICEREG1724(ice, IRQMASK)) |
-				     VT1724_IRQ_MPU_TX,
-				     ICEREG1724(ice, IRQMASK));
+				enable_midi_irq(ice, VT1724_IRQ_MPU_TX, 0);
 			}
+			spin_unlock(&ice->reg_lock);
 			break;
 		}
 		handled = 1;
 		if (status & VT1724_IRQ_MPU_TX) {
-			spin_lock(&ice->reg_lock);
 			if (ice->midi_output)
 				vt1724_midi_write(ice);
-			spin_unlock(&ice->reg_lock);
+			else
+				enable_midi_irq(ice, VT1724_IRQ_MPU_TX, 0);
 			/* Due to mysterical reasons, MPU_TX is always
 			 * generated (and can't be cleared) when a PCM
 			 * playback is going.  So let's ignore at the
@@ -415,15 +427,14 @@ static irqreturn_t snd_vt1724_interrupt(int irq, void *dev_id)
 			status_mask &= ~VT1724_IRQ_MPU_TX;
 		}
 		if (status & VT1724_IRQ_MPU_RX) {
-			spin_lock(&ice->reg_lock);
 			if (ice->midi_input)
 				vt1724_midi_read(ice);
 			else
 				vt1724_midi_clear_rx(ice);
-			spin_unlock(&ice->reg_lock);
 		}
 		/* ack MPU irq */
 		outb(status, ICEREG1724(ice, IRQSTAT));
+		spin_unlock(&ice->reg_lock);
 		if (status & VT1724_IRQ_MTPCM) {
 			/*
 			 * Multi-track PCM
-- 
1.6.0.4



More information about the Alsa-devel mailing list