How can I test if MIDI TX is broken?
2008/11/3 Takashi Iwai <tiwai@xxxxxxx>:> 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ć_______________________________________________Alsa-devel mailing listAlsa-devel@xxxxxxxxxxxxxxxxxxxx://mailman.alsa-project.org/mailman/listinfo/alsa-devel
