Re: Bug in sound-unstable-2.6 ice1724

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

 



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ć_______________________________________________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