[alsa-devel] Bug in sound-unstable-2.6 ice1724
Takashi Iwai
tiwai at suse.de
Mon Nov 3 08:14:54 CET 2008
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
More information about the Alsa-devel
mailing list