sound/midi: Clarify MPU-401 init #1905

n-p-soft · 2025-11-29T11:31:04Z

First, we try to better init MPU-401 chip so that it is put in UART mode using the recommended way: <wait_for_ack><enable_uart_mode>. This must be done before the interrupt handler is active, so that the ACK is the only data received between the two commands. Actually this setup may occur after interrupts are enabled for the card.

Second, there is a mechanism to periodically call the interrupt handler, probably to address very old or spurious cards. We remove this because all cards in the tree using MPU-401 (CSMedia chips, Crystal chips, and Soundblasters) have proper interrupt handling for MIDI events.

Rather old hardware is concerned; but PCI soundcards may work fine using a PCIe-to-PCI bridge, and some have pretty decent chipsets.

25/12/09: testing against some hardware

snd_emu10kx (Soundblaster): seems to work, other tests to come. PR to enable MIDI code.
snd_emu10k1 (previous Soundblaster driver): MIDI not working, probably due to a bad setup of the card. As emu10kx is working for (most of? all?) these cards, is it useful to keep this code ?
snd_cmi (C-Media): I only have a cheap Chinese board. MIDI-IN is working (hardware problem for MIDI-OUT on my board). Patch needed to enable MIDI code (few lines, PR to come).
snd_csa (Crystal): I'm trying to get a cheap card to test this driver. Patch probably needed.

NOTE: this depends on pull requests freebsd#1905 and freebsd#1892. MIDI code in emu10kx driver was disabled due to possible locking problems in MIDI core, which should have been corrected by freebsd#1905. Here, we also fixed that the card interrupts were enabled too early and disabled too late. This was tested under FreeBSD 15.0 using a SoundBlaster Audigy (chipset CA0100, I will also try one Audigy2). Signed-off-by: Nicolas Provost <dev@nicolas-provost.fr>

NOTE: this depends on pull requests freebsd#1902 and freebsd#1905. MIDI code in emu10kx driver was disabled due to possible locking problems in MIDI core, which should have been corrected by freebsd#1902. Here, we also fixed that the card interrupts were enabled too early and disabled too late. This was tested under FreeBSD 15.0 using a SoundBlaster Audigy (chipset CA0100, I will also try one Audigy2). Signed-off-by: Nicolas Provost <dev@nicolas-provost.fr>

christosmarg

Mostly good, just some notes.

christosmarg · 2025-12-02T10:23:11Z

sys/dev/sound/midi/mpu401.c

+	for (i = 0; i < MPU_TRYDATA; i++) {
+		if (TXRDY(m))
+			return (1);
+		else if (RXRDY(m))


As I mentioned in the old PR, it'd be good to have a comment here explaining the reason for discarding input. It is not obvious to someone looking at this for the first time.

I'll rework all these comments soon.

christosmarg · 2025-12-02T10:24:38Z

sys/dev/sound/midi/mpu401.c

+	return (0);
+}
+
+/* Some cards only support UART mode but others will start in


The comment should start on the next line. Same applies to other multi-line comments.

christosmarg · 2025-12-02T10:25:43Z

sys/dev/sound/midi/mpu401.c

+}
+
+/* Some cards only support UART mode but others will start in
+ * "Intelligent" mode, so we must try to switch to UART mode.


Here we say that "we must try to switch to UART mode" but I do not think it is entirely clear why we must do this. I'm not saying it's not correct to do this, but the comment should also explain the "why" in my opinion.

As far as I know, the intelligent mode may change on-the-fly the MIDI messages in some way, whereas in UART mode, the raw bytes are simply transmitted, then safely processed by userspace tools. When a card supports the intelligent mode, it is enabled by default.

christosmarg · 2025-12-02T10:26:34Z

sys/dev/sound/midi/mpu401.c

-		i++;
-		/* DELAY(100); */
-		s = STATUS(m);
+		else


} else {

Same below.

christosmarg · 2025-12-02T10:27:45Z

sys/dev/sound/midi/mpu401.c

-	if ((m->flags & M_TXEN) && (m->si)) {
-		callout_reset(&m->timer, 1, mpu401_timeout, m);
+	/* Read and buffer all input bytes, then send data.
+	 * Note that pending input may inhibits data sending.


inhibits -> inhibit

christosmarg · 2025-12-02T10:28:17Z

sys/dev/sound/midi/mpu401.c

-		callout_reset(&m->timer, 1, mpu401_timeout, m);
+	/* Read and buffer all input bytes, then send data.
+	 * Note that pending input may inhibits data sending.
+	 * Spurious cards may also be sticky, so limit the count of tries


Should explain what "sticky" means. Also "number of tries", not "count of tries".

I removed this because there is no spurious (for example: bad interrupts) card in the FreeBSD tree, to my knowledge.

christosmarg · 2025-12-02T10:29:22Z

sys/dev/sound/midi/mpu401.c

+	 * (probably MPU cards have no more than 16 bytes as internal buffer).
+	 */
+	for (i = 0; RXRDY(m) && i < MPU_INTR_BUF; i++) {
+		/* XXX we should check midi_in has room in its buffer, else


Is there a reason this patch does not do what the comment here says?

Because the midi_in function (midi.c) should be modified to report when its buffer is full. Ideally we should ensure midi_in has room to store the byte before reading it from the MPU. MIDI commands are multi-byte (often 3 bytes) so each byte matters.

As I noticed you're working on midi.c, are you sure the data queues (midiq.h) are safe ?

To be more precise, to get the room available in the input queue, we need a lock on the midi device, which is taken in the function midi_in.
For example we could modify midi_in to return the number of bytes available in the input queue if it is passed a NULL buffer.

NOTE (25/12/10): there is a panic in midi_read/midi_write when opening in blocking mode, I think it is the msleep call.

NOTE: this depends on pull request freebsd#1905. MIDI code in cmi driver (C-Media sound cards) was disabled due to possible locking problems in MIDI core, which should have been corrected by freebsd#1902 (or another pending PR). This was tested under FreeBSD 15.0 against a CM8738 generic card. Signed-off-by: Nicolas Provost <dev@nicolas-provost.fr>

NOTE: this depends on pull requests freebsd#1902 (or similar pending PR) and freebsd#1905. MIDI code in emu10kx driver was disabled due to possible locking problems in MIDI core, which should have been corrected by freebsd#1902. Here, we also fixed that the card interrupts were enabled too early and disabled too late. This was tested under FreeBSD 15.0 using a SoundBlaster Audigy (chipset CA0100, I will also try one Audigy2). Signed-off-by: Nicolas Provost <dev@nicolas-provost.fr>

NOTE: this depends on pull request freebsd#1905. MIDI code in cmi driver (C-Media sound cards) was disabled due to possible locking problems in MIDI core, which should have been corrected by freebsd#1902 (or another pending PR). This was tested under FreeBSD 15.0 against a CM8738 generic card. Signed-off-by: Nicolas Provost <dev@nicolas-provost.fr>

First, we try to better init MPU-401 chip so that it is put in UART mode using the recommended way: <reset><wait_for_ack><enable_uart_mode>. This must be done before the interrupt handler is active, so that the ACK is the only data received between the two commands. Actually this setup may occur after interrupts are enabled for the card. Second, there is a mechanism to periodically call the interrupt handler, probably to address very old or spurious cards. We remove this because all cards in the tree using MPU-401 (CSMedia chips, Crystal chips, and Soundblasters) have proper interrupt handling for MIDI events. Rather old hardware is concerned; but PCI soundcards may work fine using a PCIe-to-PCI bridge, and some have pretty decent chipsets. Signed-off-by: Nicolas Provost <dev@nicolas-provost.fr>

Only midi.c and mpu401.c files are modified. - changes to MPU-401 published in freebsd#1905 - remove MPU_CALLBACKP from the MPU interface (same as MPU_CALLBACK) - changes to midi.c published in freebsd#1902 (locks) = midi.c: fix a kernel panic when opening a MIDI device in blocking mode - midi.c: remove useless 'busy' field in MIDI device data - midi.c: silently ignore errors when closing device (ex: process crash) - midi.c: correct how to unload MIDI device when unloading a driver - midi.c: clarify when rchan/wchan are set to 1 or 0

sys/dev/sound/midi/mpu401.c

github-actions · 2025-12-11T14:58:43Z

Thank you for taking the time to contribute to FreeBSD!
There is an issue that needs to be fixed:

Missing Signed-off-by lines (0f7ab48)

Please review CONTRIBUTING.md, then update and push your branch again.

n-p-soft · 2025-12-11T15:24:04Z

Made a test using the following:

15.0 release source
this PR (2 commits)
sound/pci: Enable MIDI code in emu10kx. #1904
D54129.id167715.diff (midi.c)

MIDI reads and writes are well processed, in blocking and non-blocking mode, using my Soundblaster Audigy2 (emu10kx).

n-p-soft mentioned this pull request Nov 29, 2025

sound/pci: Enable MIDI code in emu10kx. #1904

Open

christosmarg self-requested a review December 2, 2025 10:16

christosmarg reviewed Dec 2, 2025

View reviewed changes

christosmarg mentioned this pull request Dec 8, 2025

sound/midi: Use only one lock to protect the data of a MIDI device. #1902

Closed

n-p-soft force-pushed the mpu-init branch 4 times, most recently from 3e283cd to 21b2ca2 Compare December 9, 2025 10:03

n-p-soft mentioned this pull request Dec 9, 2025

sound/pci: Enable MIDI code in cmi #1921

Open

christosmarg reviewed Dec 11, 2025

View reviewed changes

sys/dev/sound/midi/mpu401.c Show resolved Hide resolved

n-p-soft force-pushed the mpu-init branch from a137a13 to 0f7ab48 Compare December 11, 2025 14:59

Move mpu401_setup call from mpu401_init into mpu401_minit.

0f7ab48

christosmarg approved these changes Dec 12, 2025

View reviewed changes

sound/midi: Clarify MPU-401 init #1905

Are you sure you want to change the base?

sound/midi: Clarify MPU-401 init #1905

Uh oh!

Conversation

n-p-soft commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christosmarg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

n-p-soft Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

n-p-soft commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

n-p-soft commented Nov 29, 2025 •

edited

Loading

n-p-soft Dec 9, 2025 •

edited

Loading

github-actions bot commented Dec 11, 2025 •

edited

Loading

n-p-soft commented Dec 11, 2025 •

edited

Loading