At Tue, 22 Apr 2008 22:23:55 +0200, Pavel Hofman wrote: > > Hi, > > I am trying to fix the ancient MIDI problem with ice1724 cards. I > applied Takashi's patch from > http://mailman.alsa-project.org/pipermail/alsa-devel/2007-April/000641.html > and made some minor changes (MPU401_INFO_INPUT | MPU401_INFO_OUTPUT to > mpu401_uart info_flags, added a few debugs for testing). Thanks for tracking this bug! > I can test only MIDI input using my MIDI keyboard, I have no MIDI output > device available. The input works fine now, the output most probably too > since the method snd_vt1724_mpu401_write gets called when transmitting > some data with amidi. > > But here is the problem I do not understand: > > When no MIDI input/output is used, the interrupt handler > snd_vt1724_interrupt gets called with interrupt status 0x10 which > correctly refers to the audio data interrupt (VT1724_IRQ_MTPCM). > > However, when MIDI input or output is used (opened) even for a short > time after the module is loaded, any subsequent audio playback generates > interrupt status of 0x30 which refers to VT1724_IRQ_MTPCM AND > VT1724_IRQ_MPU_TX. I have no idea why VT1724_IRQ_MPU_TX gets generated. > > The first run of the snd_vt1724_interrupt loop tries to handle the MPU > TX status, calling uart_interrupt_tx in mpu401_uart.c: > > if (test_bit(MPU401_MODE_BIT_OUTPUT, &mpu->mode) && > test_bit(MPU401_MODE_BIT_OUTPUT_TRIGGER, &mpu->mode)) { > spin_lock_irqsave(&mpu->output_lock, flags); > snd_mpu401_uart_output_write(mpu); > spin_unlock_irqrestore(&mpu->output_lock, flags); > } > > However, since the MPU output is not open at this time (no MIDI is being > transmitted), the call to snd_mpu401_uart_output_write is skipped. Even > though I think the interrupt status should get cleared by > > outb(status, ICEREG1724(ice, IRQSTAT)) > > the loop repeats with the MPU TX interrupt status enabled until the > timeout check breaks the while loop, returning to the interrupt handler > the next moment again. > > Apr 19 22:20:47 nahore kernel: [ 2402.938245] uart interrupt: status 0x30 > Apr 19 22:20:47 nahore kernel: [ 2402.938260] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938267] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938274] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938281] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938288] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938295] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938302] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938309] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938316] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938323] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.938326] ice1724: Too long irq > loop, status = 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961247] uart interrupt: status 0x30 > Apr 19 22:20:47 nahore kernel: [ 2402.961278] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961285] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961292] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961299] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961306] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961313] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961320] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961327] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961334] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961341] uart interrupt: status 0x20 > Apr 19 22:20:47 nahore kernel: [ 2402.961344] ice1724: Too long irq > loop, status = 0x20 > > After the playback stops, the interrupts are gone too. It seems as if > the playback interrupt initiates the MPU TX interrupt. > > If we could avoid generating the MPU TX interrupt during regular > playback, I believe the major problem would be resolved. > > Even if I mask the interrupts (CCS01) and do not explicitly unmask them > (according to proc ice1724 the CCS01 register stays at 0xa0), the > interrupt gets generated. OK, then the simplest way would be to just ignore the TX bit at the second or later check. How about the patch below? thanks, Takashi --- diff -r f8fbce7ba459 drivers/mpu401/mpu401_uart.c --- a/drivers/mpu401/mpu401_uart.c Tue Apr 22 18:39:49 2008 +0200 +++ b/drivers/mpu401/mpu401_uart.c Wed Apr 23 12:04:54 2008 +0200 @@ -49,12 +49,10 @@ */ -#define snd_mpu401_input_avail(mpu) (!(mpu->read(mpu, MPU401C(mpu)) & 0x80)) -#define snd_mpu401_output_ready(mpu) (!(mpu->read(mpu, MPU401C(mpu)) & 0x40)) - -#define MPU401_RESET 0xff -#define MPU401_ENTER_UART 0x3f -#define MPU401_ACK 0xfe +#define snd_mpu401_input_avail(mpu) \ + (!(mpu->read(mpu, MPU401C(mpu)) & MPU401_RX_EMPTY)) +#define snd_mpu401_output_ready(mpu) \ + (!(mpu->read(mpu, MPU401C(mpu)) & MPU401_TX_FULL)) /* Build in lowlevel io */ static void mpu401_write_port(struct snd_mpu401 *mpu, unsigned char data, diff -r f8fbce7ba459 include/mpu401.h --- a/include/mpu401.h Tue Apr 22 18:39:49 2008 +0200 +++ b/include/mpu401.h Wed Apr 23 12:04:54 2008 +0200 @@ -103,6 +103,21 @@ #define MPU401D(mpu) (mpu)->port /* + * control register bits + */ +/* read MPU401C() */ +#define MPU401_RX_EMPTY 0x80 +#define MPU401_TX_FULL 0x40 + +/* write MPU401C() */ +#define MPU401_RESET 0xff +#define MPU401_ENTER_UART 0x3f + +/* read MPU401D() */ +#define MPU401_ACK 0xfe + + +/* */ diff -r f8fbce7ba459 pci/ice1712/ice1724.c --- a/pci/ice1712/ice1724.c Tue Apr 22 18:39:49 2008 +0200 +++ b/pci/ice1712/ice1724.c Wed Apr 23 12:04:54 2008 +0200 @@ -223,6 +223,32 @@ } /* + * MPU401 accessor + */ +static unsigned char snd_vt1724_mpu401_read(struct snd_mpu401 *mpu, + unsigned long addr) +{ + /* fix status bits to the standard position */ + /* only RX_EMPTY and TX_FULL are checked */ + if (addr == MPU401C(mpu)) + return (inb(addr) & 0x0c) << 4; + else + return inb(addr); +} + +static void snd_vt1724_mpu401_write(struct snd_mpu401 *mpu, + unsigned char data, unsigned long addr) +{ + if (addr == MPU401C(mpu)) { + if (data == MPU401_ENTER_UART) + outb(0x01, addr); + /* what else? */ + } else + outb(data, addr); +} + + +/* * Interrupt handler */ @@ -230,24 +256,53 @@ { struct snd_ice1712 *ice = dev_id; unsigned char status; + unsigned char status_mask = + VT1724_IRQ_MPU_RX | VT1724_IRQ_MPU_TX | VT1724_IRQ_MTPCM; int handled = 0; +#ifdef CONFIG_SND_DEBUG + int timeout = 0; +#endif while (1) { status = inb(ICEREG1724(ice, IRQSTAT)); + status &= status_mask; if (status == 0) break; - +#ifdef CONFIG_SND_DEBUG + if (++timeout > 10) { + printk(KERN_ERR + "ice1724: Too long irq loop, status = 0x%x\n", + status); + break; + } +#endif handled = 1; - /* these should probably be separated at some point, - * but as we don't currently have MPU support on the board - * I will leave it - */ - if ((status & VT1724_IRQ_MPU_RX)||(status & VT1724_IRQ_MPU_TX)) { + if (status & VT1724_IRQ_MPU_TX) { if (ice->rmidi[0]) - snd_mpu401_uart_interrupt(irq, ice->rmidi[0]->private_data); - outb(status & (VT1724_IRQ_MPU_RX|VT1724_IRQ_MPU_TX), ICEREG1724(ice, IRQSTAT)); - status &= ~(VT1724_IRQ_MPU_RX|VT1724_IRQ_MPU_TX); + snd_mpu401_uart_interrupt_tx(irq, + ice->rmidi[0]->private_data); + else /* disable TX to be sure */ + outb(inb(ICEREG1724(ice, IRQMASK)) | + VT1724_IRQ_MPU_TX, + ICEREG1724(ice, IRQMASK)); + /* 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; } + if (status & VT1724_IRQ_MPU_RX) { + if (ice->rmidi[0]) + snd_mpu401_uart_interrupt(irq, + ice->rmidi[0]->private_data); + else /* disable RX to be sure */ + outb(inb(ICEREG1724(ice, IRQMASK)) | + VT1724_IRQ_MPU_RX, + ICEREG1724(ice, IRQMASK)); + } + /* ack MPU irq */ + outb(status, ICEREG1724(ice, IRQSTAT)); if (status & VT1724_IRQ_MTPCM) { /* * Multi-track PCM @@ -2236,10 +2291,7 @@ } /* unmask used interrupts */ - if (! (ice->eeprom.data[ICE_EEP2_SYSCONF] & VT1724_CFG_MPU401)) - mask = VT1724_IRQ_MPU_RX | VT1724_IRQ_MPU_TX; - else - mask = 0; + mask = VT1724_IRQ_MPU_RX | VT1724_IRQ_MPU_TX; outb(mask, ICEREG1724(ice, IRQMASK)); /* don't handle FIFO overrun/underruns (just yet), * since they cause machine lockups @@ -2373,14 +2425,29 @@ if (! c->no_mpu401) { if (ice->eeprom.data[ICE_EEP2_SYSCONF] & VT1724_CFG_MPU401) { + struct snd_mpu401 *mpu; if ((err = snd_mpu401_uart_new(card, 0, MPU401_HW_ICE1712, ICEREG1724(ice, MPU_CTRL), - MPU401_INFO_INTEGRATED, + (MPU401_INFO_INTEGRATED | + MPU401_INFO_TX_IRQ), ice->irq, 0, &ice->rmidi[0])) < 0) { snd_card_free(card); return err; } + mpu = ice->rmidi[0]->private_data; + mpu->read = snd_vt1724_mpu401_read; + mpu->write = snd_vt1724_mpu401_write; + /* unmask MPU RX/TX irqs */ + outb(inb(ICEREG1724(ice, IRQMASK)) & + ~(VT1724_IRQ_MPU_RX | VT1724_IRQ_MPU_TX), + ICEREG1724(ice, IRQMASK)); +#if 0 /* for testing */ + /* set watermarks */ + outb(VT1724_MPU_RX_FIFO | 0x1, + ICEREG1724(ice, MPU_FIFO_WM)); + outb(0x1, ICEREG1724(ice, MPU_FIFO_WM)); +#endif } } diff -r f8fbce7ba459 pci/ice1712/prodigy192.c --- a/pci/ice1712/prodigy192.c Tue Apr 22 18:39:49 2008 +0200 +++ b/pci/ice1712/prodigy192.c Wed Apr 23 12:04:54 2008 +0200 @@ -812,10 +812,6 @@ .build_controls = prodigy192_add_controls, .eeprom_size = sizeof(prodigy71_eeprom), .eeprom_data = prodigy71_eeprom, - /* the current MPU401 code loops infinitely - * when opening midi device - */ - .no_mpu401 = 1, }, { } /* terminator */ }; _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
