Sorry I can't describe this better but midi ends up being played
too fast/slow here and some notes have the wrong duration when
using eap(4) an mt-32 and scummvm.  The timing of when the notes
start seems to be off as well.

http://jsg.id.au/misc/with_patch.ogg
http://jsg.id.au/misc/without_patch.ogg

On Wed, Mar 07, 2012 at 10:39:25PM +0100, Alexandre Ratchov wrote:
> Certain midi uarts don't have interrupts, so when data is sent, the
> driver spins (at IPL_AUDIO) until the uart becomes ready. This is not
> a big problem because uarts have ~16-32 byte fifos, and programs don't
> send large chunks of data, so uarts always appear ready. Except for
> large messages, which cause spinning.
> 
> This diff is to avoid spinning and to make midi output code simpler.
> On my system, interrupt time during a large transfer drops from 5%
> to ~0%.
> 
> If you use external midi synths connected to umidi, eap, autri, and/or
> mpu, I'd like to know if this diff causes any regression. To test,
> just do your usual stuff involving midi output.
> 
> thanks
> 
> -- Alexandre
> 
> Index: midi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/midi.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 midi.c
> --- midi.c    2 Jul 2011 22:20:07 -0000       1.26
> +++ midi.c    7 Mar 2012 20:50:43 -0000
> @@ -232,51 +232,14 @@ void
>  midi_out_do(struct midi_softc *sc)
>  {
>       struct midi_buffer *mb = &sc->outbuf;
> -     unsigned            i;
> -     int                 error;
>  
> -     /*
> -      * If output interrupts are not supported then we write MIDI_MAXWRITE
> -      * bytes instead of 1, and then we wait sc->wait
> -      */
> -
> -     i = sc->props & MIDI_PROP_OUT_INTR ? 1 : MIDI_MAXWRITE;
> -     while (i != 0) {
> -             if (mb->used == 0)
> +     while (mb->used > 0) {
> +             if (!sc->hw_if->output(sc->hw_hdl, mb->data[mb->start]))
>                       break;
> -             error = sc->hw_if->output(sc->hw_hdl, mb->data[mb->start]);
> -             /*
> -              * 0 means that data is being sent, an interrupt will
> -              * be generated when the interface becomes ready again
> -              *
> -              * EINPROGRESS means that data has been queued, but
> -              * will not be sent immediately and thus will not
> -              * generate interrupt, in this case we can send
> -              * another byte. The flush() method can be called
> -              * to force the transfer.
> -              *
> -              * EAGAIN means that data cannot be queued or sent;
> -              * because the interface isn't ready. An interrupt
> -              * will be generated once the interface is ready again
> -              *
> -              * any other (fatal) error code means that data couldn't
> -              * be sent and was lost, interrupt will not be generated
> -              */
> -             if (error == EINPROGRESS) {
> -                     MIDIBUF_REMOVE(mb, 1);
> -                     if (MIDIBUF_ISEMPTY(mb)) {
> -                             if (sc->hw_if->flush != NULL)
> -                                     sc->hw_if->flush(sc->hw_hdl);
> -                             midi_out_stop(sc);
> -                             return;
> -                     }
> -             } else if (error == 0) {
> -                     MIDIBUF_REMOVE(mb, 1);
> -                     i--;
> -             } else if (error == EAGAIN) {
> -                     break;
> -             } else {
> -                     MIDIBUF_INIT(mb);
> +             MIDIBUF_REMOVE(mb, 1);
> +             if (MIDIBUF_ISEMPTY(mb)) {
> +                     if (sc->hw_if->flush != NULL)
> +                             sc->hw_if->flush(sc->hw_hdl);
>                       midi_out_stop(sc);
>                       return;
>               }
> @@ -286,7 +249,7 @@ midi_out_do(struct midi_softc *sc)
>               if (MIDIBUF_ISEMPTY(mb))
>                       midi_out_stop(sc);
>               else
> -                     timeout_add(&sc->timeo, sc->wait);
> +                     timeout_add(&sc->timeo, 1);
>       }
>  }
>  
> @@ -554,13 +517,11 @@ midiclose(dev_t dev, int fflag, int devt
>       /*
>        * some hw_if->close() reset immediately the midi uart
>        * which flushes the internal buffer of the uart device,
> -      * so we may lose some (important) data. To avoid this, we sleep 2*wait,
> -      * which gives the time to the uart to drain its internal buffers.
> -      *
> -      * Note: we'd better sleep in the corresponding hw_if->close()
> +      * so we may lose some (important) data. To avoid this,
> +      * sleep 20ms (around 64 bytes) to give the time to the
> +      * uart to drain its internal buffers.
>        */
> -
> -     tsleep(&sc->wchan, PWAIT, "mid_cl", 2 * sc->wait);
> +     tsleep(&sc->wchan, PWAIT, "mid_cl", hz * MIDI_MAXWRITE / MIDI_RATE);
>       sc->hw_if->close(sc->hw_hdl);
>       sc->isopen = 0;
>       return 0;
> @@ -582,9 +543,6 @@ midi_attach(struct midi_softc *sc, struc
>       struct midi_info          mi;
>  
>       sc->isdying = 0;
> -     sc->wait = (hz * MIDI_MAXWRITE) /  MIDI_RATE;
> -     if (sc->wait == 0)
> -             sc->wait = 1;
>       sc->hw_if->getinfo(sc->hw_hdl, &mi);
>       sc->props = mi.props;
>       sc->isopen = 0;
> Index: midivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/midivar.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 midivar.h
> --- midivar.h 21 Nov 2005 18:16:38 -0000      1.5
> +++ midivar.h 7 Mar 2012 20:50:44 -0000
> @@ -85,7 +85,6 @@ struct midi_softc {
>       int                 props;              /* midi hw proprieties */
>       int                 rchan;
>       int                 wchan;
> -     unsigned            wait;               /* see midi_out_do */
>       struct selinfo      rsel;
>       struct selinfo      wsel;
>       struct proc        *async;
> Index: isa/mpu401.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/isa/mpu401.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 mpu401.c
> --- isa/mpu401.c      26 Jun 2008 05:42:16 -0000      1.11
> +++ isa/mpu401.c      7 Mar 2012 20:50:45 -0000
> @@ -208,12 +208,12 @@ mpu_output(v, d)
>               mpu_readinput(sc);
>               splx(s);
>       }
> -     if (mpu_waitready(sc)) {
> -             DPRINTF(("mpu_output: not ready\n"));
> -             return EIO;
> -     }
> +     if (MPU_GETSTATUS(sc->iot, sc->ioh) & MPU_OUTPUT_BUSY)
> +             delay(10);
> +     if (MPU_GETSTATUS(sc->iot, sc->ioh) & MPU_OUTPUT_BUSY)
> +             return 0;
>       bus_space_write_1(sc->iot, sc->ioh, MPU_DATA, d);
> -     return 0;
> +     return 1;
>  }
>  
>  void
> Index: isa/sbdsp.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/isa/sbdsp.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 sbdsp.c
> --- isa/sbdsp.c       15 Jul 2010 03:43:11 -0000      1.31
> +++ isa/sbdsp.c       7 Mar 2012 20:50:47 -0000
> @@ -2385,10 +2385,9 @@ sbdsp_midi_output(addr, d)
>       struct sbdsp_softc *sc = addr;
>  
>       if (sc->sc_model < SB_20 && sbdsp_wdsp(sc, SB_MIDI_WRITE))
> -             return EIO;
> -     if (sbdsp_wdsp(sc, d))
> -             return EIO;
> -     return 0;
> +             return 1;
> +     (void)sbdsp_wdsp(sc, d);
> +     return 1;
>  }
>  
>  void
> Index: pci/autri.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/autri.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 autri.c
> --- pci/autri.c       3 Jul 2011 15:47:16 -0000       1.30
> +++ pci/autri.c       7 Mar 2012 20:50:48 -0000
> @@ -1612,16 +1612,12 @@ int
>  autri_midi_output(void *addr, int d)
>  {
>       struct autri_softc *sc = addr;
> -     int x;
>  
> -     for (x = 0; x != MIDI_BUSY_WAIT; x++) {
> -             if ((TREAD1(sc, AUTRI_MPUR1) & AUTRI_MIDIOUT_READY) == 0) {
> -                     TWRITE1(sc, AUTRI_MPUR0, d);
> -                     return (0);
> -             }
> -             delay(MIDI_BUSY_DELAY);
> +     if ((TREAD1(sc, AUTRI_MPUR1) & AUTRI_MIDIOUT_READY) != 0) {
> +             TWRITE1(sc, AUTRI_MPUR0, d);
> +             return 0;
>       }
> -     return (EIO);
> +     return 1;
>  }
>  
>  void
> Index: pci/eap.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/eap.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 eap.c
> --- pci/eap.c 3 Jul 2011 15:47:17 -0000       1.43
> +++ pci/eap.c 7 Mar 2012 20:50:50 -0000
> @@ -1707,16 +1707,11 @@ int
>  eap_midi_output(void *addr, int d)
>  {
>       struct eap_softc *sc = addr;
> -     int x;
>  
> -     for (x = 0; x != MIDI_BUSY_WAIT; x++) {
> -             if (EREAD1(sc, EAP_UART_STATUS) & EAP_US_TXRDY) {
> -                     EWRITE1(sc, EAP_UART_DATA, d);
> -                     return (0);
> -             }
> -             delay(MIDI_BUSY_DELAY);
> -     }
> -     return (EIO);
> +     if (!(EREAD1(sc, EAP_UART_STATUS) & EAP_US_TXRDY))
> +             return 0;
> +     EWRITE1(sc, EAP_UART_DATA, d);
> +     return 1;
>  }
>  
>  void
> Index: pci/eapreg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/eapreg.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 eapreg.h
> --- pci/eapreg.h      26 Jun 2008 05:42:17 -0000      1.3
> +++ pci/eapreg.h      7 Mar 2012 20:50:50 -0000
> @@ -263,9 +263,6 @@
>  #define EAP_RECORD_CLASS     11
>  #define EAP_INPUT_CLASS              12
>  
> -#define MIDI_BUSY_WAIT               100
> -#define MIDI_BUSY_DELAY              100     /* Delay when UART is busy */
> -
>  #define EAP_EV1938_A  0x00
>  #define EAP_ES1371_A  0x02
>  #define EAP_CT5880_C  0x02
> Index: pci/envy.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/envy.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 envy.c
> --- pci/envy.c        27 Apr 2011 07:01:33 -0000      1.50
> +++ pci/envy.c        7 Mar 2012 20:50:52 -0000
> @@ -2390,10 +2390,13 @@ int
>  envy_midi_output(void *self, int data)
>  {
>       struct envy_softc *sc = (struct envy_softc *)self;
> +     int st;
>       
> -     envy_midi_wait(sc);
> +     st = envy_ccs_read(sc, ENVY_CCS_MIDISTAT0);
> +     if (st & ENVY_MIDISTAT_OBUSY(sc))
> +             return 0;
>       envy_ccs_write(sc, ENVY_CCS_MIDIDATA0, data);
> -     return 0;
> +     return 1;
>  }
>  
>  void
> Index: usb/umidi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/umidi.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 umidi.c
> --- usb/umidi.c       3 Jul 2011 15:47:17 -0000       1.32
> +++ usb/umidi.c       7 Mar 2012 20:50:54 -0000
> @@ -313,7 +313,7 @@ umidi_output(void *addr, int d)
>       struct umidi_mididev *mididev = addr;
>  
>       if (!mididev->out_jack || !mididev->opened)
> -             return EIO;
> +             return 1;
>  
>       return out_jack_output(mididev->out_jack, d);
>  }
> @@ -745,9 +745,7 @@ alloc_all_jacks(struct umidi_softc *sc)
>               jack->binded = 0;
>               jack->arg = NULL;
>               jack->u.out.intr = NULL;
> -#ifdef DIAGNOSTIC
> -             jack->wait = 0;
> -#endif
> +             jack->intr = 0;
>               jack->cable_number = i;
>               jack++;
>       }
> @@ -1165,56 +1163,31 @@ out_jack_output(struct umidi_jack *j, in
>       int s;
>  
>       if (sc->sc_dying)
> -             return EIO;
> +             return 1;
>       if (!j->opened)
> -             return ENODEV;
> -             
> +             return 1;
>       s = splusb();
> -     if (ep->used == ep->packetsize) {
> -#ifdef DIAGNOSTIC
> -             if (j->wait == 0) {
> -                     j->wait = 1;
> -#endif
> +     if (ep->busy) {
> +             if (!j->intr) {
>                       SIMPLEQ_INSERT_TAIL(&ep->intrq, j, intrq_entry);
>                       ep->pending++;
> -#ifdef DIAGNOSTIC
> -             } else {
> -                     printf("umidi: (again) %d: already on intrq\n", 
> -                         j->cable_number);
> -             }
> -#endif
> +                     j->intr = 1;
> +             }               
>               splx(s);
> -             return EAGAIN;
> +             return 0;
>       }
> -     
> -     if (!out_build_packet(j->cable_number, &j->packet, d, 
> -         ep->buffer + ep->used)) {
> +     if (!out_build_packet(j->cable_number, &j->packet, d,
> +             ep->buffer + ep->used)) {
>               splx(s);
> -             return EINPROGRESS;
> +             return 1;
>       }
>       ep->used += UMIDI_PACKET_SIZE;
> -     if (ep->used < ep->packetsize) {
> -             splx(s);
> -             return EINPROGRESS;
> -     }
> -#ifdef DIAGNOSTIC
> -     if (j->wait == 0) {
> -             j->wait = 1;            
> -#endif
> -             SIMPLEQ_INSERT_TAIL(&ep->intrq, j, intrq_entry);
> -             ep->pending++;
> -#ifdef DIAGNOSTIC
> -     } else {
> -             printf("umidi: (ok) %d: already on intrq\n", 
> -                 j->cable_number);
> -     }
> -#endif
> -     if (!ep->busy) {
> +     if (ep->used == ep->packetsize) {
>               ep->busy = 1;
>               start_output_transfer(ep);
>       }
>       splx(s);
> -     return 0;
> +     return 1;
>  }
>  
>  static void
> @@ -1278,6 +1251,7 @@ out_intr(usbd_xfer_handle xfer, usbd_pri
>               return;
>  
>       ep->used = 0;
> +     ep->busy = 0;
>       for (pending = ep->pending; pending > 0; pending--) {
>               j = SIMPLEQ_FIRST(&ep->intrq);
>  #ifdef DIAGNOSTIC
> @@ -1288,17 +1262,9 @@ out_intr(usbd_xfer_handle xfer, usbd_pri
>  #endif
>               SIMPLEQ_REMOVE_HEAD(&ep->intrq, intrq_entry);
>               ep->pending--;
> -#ifdef DIAGNOSTIC
> -             j->wait = 0;
> -#endif
> +             j->intr = 0;
>               if (j->opened && j->u.out.intr)
>                       (*j->u.out.intr)(j->arg);
> -     }
> -
> -     if (ep->used == 0) {
> -             ep->busy = 0;
> -     } else {
> -             start_output_transfer(ep);
>       }
>  }
>  
> Index: usb/umidivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/umidivar.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 umidivar.h
> --- usb/umidivar.h    26 Jun 2008 05:42:19 -0000      1.12
> +++ usb/umidivar.h    7 Mar 2012 20:50:54 -0000
> @@ -69,7 +69,7 @@ struct umidi_jack {
>       int                     opened;
>       SIMPLEQ_ENTRY(umidi_jack) intrq_entry;  
>  #ifdef DIAGNOSTIC
> -     unsigned                wait;
> +     unsigned                intr;
>  #endif
>       union {
>               struct {

Reply via email to