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 {