[alsa-devel] Bug in sound-unstable-2.6 ice1724
Vedran Miletić
rivanvx at gmail.com
Wed Nov 5 17:19:46 CET 2008
Cool. Didn't know it was that easy. Also, I didn't expect it to work
since file is already patched by alsa-driver compatibility for older
kernels, but I was pleasantly surprised:
vedran at kalopsia:~/bin/alsa-driver-unstable/alsa-kernel$ patch -p2 <
/home/vedran/bin/ice1724-takashi.patch (second one you posted)
patching file pci/ice1712/ice1724.c
Hunk #1 succeeded at 243 (offset 2 lines).
Hunk #2 succeeded at 259 (offset 2 lines).
Hunk #3 succeeded at 282 (offset 2 lines).
Hunk #4 succeeded at 319 (offset 2 lines).
Hunk #5 succeeded at 334 (offset 2 lines).
Hunk #6 succeeded at 404 (offset 2 lines).
Hunk #7 succeeded at 2458 (offset 32 lines).
MIDI works, no change here. However, mplayer (on both analog and
digital) fills dmesg with:
[ 1921.595750] ice1724: Too long irq loop, status = 0x20
[ 1921.595775] ice1724: Disabling MPU_TX
[ 1921.630993] ice1724: Too long irq loop, status = 0x20
[ 1921.630993] ice1724: Disabling MPU_TX
[ 1921.662976] ice1724: Too long irq loop, status = 0x20
[ 1921.662976] ice1724: Disabling MPU_TX
[ 1921.694977] ice1724: Too long irq loop, status = 0x20
[ 1921.694977] ice1724: Disabling MPU_TX
[ 1921.726998] ice1724: Too long irq loop, status = 0x20
[ 1921.726998] ice1724: Disabling MPU_TX
[ 1921.759213] ice1724: Too long irq loop, status = 0x20
[ 1921.759213] ice1724: Disabling MPU_TX
[ 1921.791289] ice1724: Too long irq loop, status = 0x20
[ 1921.791289] ice1724: Disabling MPU_TX
[ 1921.823276] ice1724: Too long irq loop, status = 0x20
[ 1921.823276] ice1724: Disabling MPU_TX
[ 1921.855159] ice1724: Too long irq loop, status = 0x20
[ 1921.855159] ice1724: Disabling MPU_TX
This doesn't happen without patch. Audio works normally.
2008/11/5 Takashi Iwai <tiwai at suse.de>:
> At Wed, 5 Nov 2008 16:08:19 +0100,
> =?UTF-8?Q?Vedran_Mileti=C4=87?= wrote:
>>
>> OK. Actually, I was asking what is the command to apply patch to a
>> snapshot, if there is one.
>
> Are you asking the usage of patch program?
> For linux-kernel patch (sound/* path), do like below:
>
> % cd alsa-driver*/alsa-kernel
> % patch -p2 < fix-patch-file-path
>
>
> Takashi
>
>> 2008/11/5 Takashi Iwai <tiwai at suse.de>:
>> > 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ć
>> >
>>
>>
>>
>> --
>> Vedran Miletić
>
--
Vedran Miletić
More information about the Alsa-devel
mailing list