[alsa-devel] Bug in sound-unstable-2.6 ice1724
Takashi Iwai
tiwai at suse.de
Wed Nov 5 16:05:09 CET 2008
At Wed, 5 Nov 2008 15:54:21 +0100,
=?UTF-8?Q?Vedran_Mileti=C4=87?= wrote:
>
> 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?
The snapshot doesn't include the patch yet since it's untested.
Please apply my patch anyway.
As you see, there are two patches. One is just for fixing PCM thing.
Try this one first. If this works, try the next one, which could .
thanks,
Takashi
> 2008/11/5 Takashi Iwai <tiwai at 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 at 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ć
> >
>
>
>
> --
> Vedran Miletić
More information about the Alsa-devel
mailing list