OK, this will not be a problem. Can I apply your patch if I don't have a git tree, but just an unpacked snapshot?
2008/11/5 Takashi Iwai tiwai@suse.de:
At Wed, 5 Nov 2008 15:45:43 +0100, =?UTF-8?Q?Vedran_Mileti=C4=87?= wrote:
How can I test if MIDI TX is broken?
Just run aplaymidi to the hardware port. It doesn't matter whether you connected to any device or not. Check the count in /proc/asound/card0/midi* proc file changes, and most importantly, the machine doesn't hang up.
thanks,
Takashi
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-- Vedran Miletić