[alsa-devel] MIDI on ice1724 - preliminary findings and questions
Takashi Iwai
tiwai at suse.de
Wed Apr 23 12:05:42 CEST 2008
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 */
};
More information about the Alsa-devel
mailing list