How can I test if MIDI TX is broken?
2008/11/3 Takashi Iwai tiwai@suse.de:
At Sun, 02 Nov 2008 19:21:37 +0100, I wrote:
At Sun, 2 Nov 2008 15:21:05 +0200, Karsten Wiese wrote:
Am Sonntag, 2. November 2008 schrieb Takashi Iwai:
nAt Sat, 1 Nov 2008 19:08:59 +0200, Karsten Wiese wrote:
Hi Takashi,
Am Sonntag, 26. Oktober 2008 schrieb Takashi Iwai:
At Sun, 26 Oct 2008 17:31:02 +0100, =?UTF-8?Q?Vedran_Mileti=C4=87?= wrote: > > Yeap, that one does it. No need to revert it fully, though. I just > uncommented the unsigned char mask line and removed that #if 0 and > #endif. Now it works perfectly. Thanks! Can you fix it in your tree?
Well, before doing that, I'd like to understand the problem more.
If the problem is about the initial IRQ mask value, setting 0 should work as well. Could you change the code just to do like below, and check whether it works? outb(0, ICEREG1724(ice, IRQMASK));
Makes a pc hang sometimes here, while reverting the entire patch is stable. no midi tested though, only audio.
Hmm, that's a bad news. Basically my change is just to disable MPU_TX irq when unresolved irqs occur. The detection was already there, but it was enabled only when CONFIG_SND_DEBUG.
So, the question is, whether does the hang-up happens even with the old code when you enable CONFIG_SND_DEBUG?
Also, a bit more detail would be appreciated -- what kind of hang-up, whether you get the error message, or so.
the hang happened on an always on production machine. I don't have a second card to test with currently, nor do i like to test on that machine again ;-)
May be its just that the mask has to be applied the other way round: I'd guess a 1 means "masked" or "not active".
Damn, you must be right! Could you give the patch below?
... and looking through the code again, I think (hopefully) the patch below will fix the MIDI TX problem, too.
Could someone give it a try? It includes the previous patch.
thanks,
Takashi
diff --git a/sound/pci/ice1712/ice1724.c b/sound/pci/ice1712/ice1724.c index 40725df..fd7b440 100644 --- a/sound/pci/ice1712/ice1724.c +++ b/sound/pci/ice1712/ice1724.c @@ -241,6 +241,9 @@ get_rawmidi_substream(struct snd_ice1712 *ice, unsigned int stream) struct snd_rawmidi_substream, list); }
+static void enable_midi_irq_unlocked(struct snd_ice1712 *ice,
u8 flag, int enable);static void vt1724_midi_write(struct snd_ice1712 *ice) { struct snd_rawmidi_substream *s; @@ -254,6 +257,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.*/if (snd_rawmidi_transmit_empty(s))enable_midi_irq_unlocked(ice, VT1724_IRQ_MPU_TX, 0);}
static void vt1724_midi_read(struct snd_ice1712 *ice) @@ -272,31 +280,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)+static void enable_midi_irq_unlocked(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_unlocked(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;}
@@ -306,6 +317,7 @@ static void vt1724_midi_output_trigger(struct snd_rawmidi_substream *s, int up) unsigned long flags;
spin_lock_irqsave(&ice->reg_lock, flags);
enable_midi_irq_unlocked(ice, VT1724_IRQ_MPU_TX, up); if (up) { ice->midi_output = 1; vt1724_midi_write(ice);@@ -320,6 +332,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,41 +402,41 @@ 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_unlocked(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);+#if 0 /* should have been fixed */ /* 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 * next loop. */ status_mask &= ~VT1724_IRQ_MPU_TX; +#endif } 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@@ -2413,8 +2426,8 @@ static int __devinit snd_vt1724_create(struct snd_card *card, return -EIO; }
/* clear interrupts -- otherwise you'll get irq problems later */outb(0, ICEREG1724(ice, IRQMASK));
/* MPU_RX and TX irq masks are cleared later dynamically */outb(VT1724_IRQ_MPU_RX | VT1724_IRQ_MPU_TX , ICEREG1724(ice, IRQMASK)); /* don't handle FIFO overrun/underruns (just yet), * since they cause machine lockups