Re: Bug in sound-unstable-2.6 ice1724

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Cool. Didn't know it was that easy. Also, I didn't expect it to worksince file is already patched by alsa-driver compatibility for olderkernels, but I was pleasantly surprised:vedran@kalopsia:~/bin/alsa-driver-unstable/alsa-kernel$ patch -p2 </home/vedran/bin/ice1724-takashi.patch (second one you posted)patching file pci/ice1712/ice1724.cHunk #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 anddigital) 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@xxxxxxx>:> 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@xxxxxxx>:>> > 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@xxxxxxx>:>> >> > 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@xxxxxxx>:>> >> >> > 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ć_______________________________________________Alsa-devel mailing listAlsa-devel@xxxxxxxxxxxxxxxxxxxx://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux