Roland PG-1000: Added internal layout and complete driver implementation

[felipesanches]

[felipesanches]

I do have concerns, though, about the declaration of MIDI ports, as I am not extremely familiar with the way MAME does it, so I would appreciate some help here, because it may be incorrect or there may be cleaner ways to describe this circuit:

[felipesanches]

I do have concerns, though, about the declaration of MIDI ports, as I am not extremely familiar with the way MAME does it, so I would appreciate some help here, because it may be incorrect or there may be cleaner ways to describe this circuit:

Connection diagrams from user manual

These are the relevant diagrams for emulating the MIDI ports:

---

---

[m1macrophage]

I do have concerns, though, about the declaration of MIDI ports [...]

It is possible I missed some detail, but your first commit seems to emulate all 4 midi ports correctly, based on the linked schematic and documentation.

However, the MIDI_PORT declarations can be simplified a bit. Try grep -r 'MIDI_PORT.*_slot' src/mame for examples. Not sure if the alternate declaration method would make a functional difference.

[felipesanches]

I do have concerns, though, about the declaration of MIDI ports [...]

It is possible I missed some detail, but your first commit seems to emulate all 4 midi ports correctly, based on the linked schematic and documentation.

However, the MIDI_PORT declarations can be simplified a bit. Try grep -r 'MIDI_PORT.*_slot' src/mame for examples. Not sure if the alternate declaration method would make a functional difference.

It seems to me that using midiin_slot(...) for declaring the "Parameter In" port was problematic as it was (as far as I could grasp) creating a second -midiin command line option (instead of a custom -paramin option).

[felipesanches]

I've done the bulk of the work developing this driver and layout and it is now looking beautiful and working almost perfectly. The only one thing that still does not work are the MIDI ports (which is kind of essential for it to be useful, LOL) and I have no idea what's still missing to make it work.

Not sure if I made some mistake in the MIDI description on the driver, or if the serial port support in the UPD78C10 core is not implemented correctly. I inspected a bit of it and it looks reasonable, though...

I would appreciate if someone could take a look at it and give it the final touch to make it work, in case it is something simple. Otherwise, it would be wiser to simply merge it as is and then someday we'll end up figuring out how to fix it. But I personally feel that I have nothing else to add to it at this moment.

Owners' manual explains that all messages received on its MIDI IN are replicated in its MIDI OUT and any parameter adjustments made on the sliders are also sent over MIDI OUT as sysex messages.

So I tried amidi -p virtual -d to dump to the terminal anything that is written to the virtual midi device and then I loaded the driver with -midiin "my usb-midi controller" -midiout "virtual device" but I get nothing logged at MIDI OUT when I play on the controller and I also get nothing while dragging the sliders.

The method of using amidi -p virtual -d works fine with other synth drivers like the Ensoniq ESQ-1.

[m1macrophage]

I do have concerns, though, about the declaration of MIDI ports [...]

It is possible I missed some detail, but your first commit seems to emulate all 4 midi ports correctly, based on the linked schematic and documentation.
However, the MIDI_PORT declarations can be simplified a bit. Try grep -r 'MIDI_PORT.*_slot' src/mame for examples. Not sure if the alternate declaration method would make a functional difference.

It seems to me that using midiin_slot(...) for declaring the "Parameter In" port was problematic as it was (as far as I could grasp) creating a second -midiin command line option (instead of a custom -paramin option).

I can't speak to the approach that uses separate midiin_slot(...) calls. But if you follow the declaration examples above, you should end up with -midiin1, -midiin2, -midiout1 and -midiout2 (use -listmedia to verify). I haven't checked if it is possible to provide custom names (such as -paramin) for these. MAME devs can speak to that.

Once that's set up, you could run your test, but log midi thru instead of midi out. If that works, then you at least know that the MIDI_PORT setup for midi in and thru, and your testing strategy, are sound.

Edit: Also, consider initializing m_midiin_bit and m_paramin_bit to true, which is MIDI "idle". I've seen drivers ignore the first midi message when initializing to false / 0.

[felipesanches]

Earlier today I figured out that, by setting m_irr to 0x0400 (equivalent to IRR |= INTFST) via the debugger, I finally get valid sysex messages sent to MIDI OUT.

I have been able to connect the emulated PG-1000 to my real D-550 unit using ALSA's aconnect and successfully tweaked parameters of sounds.

So, this means that the handling of interrupts in the upd78c10 core in MAME is a bit broken.

It is unclear to me exactly when should the interrupt requests register be set to trigger the serial interrupt. I have tried to set the flag in the implementation of the MOV SMH, A instruction, but that did not work. I'm here still scratching my head trying to understand what's the correct behavior. All I know is that the only place in the code where the transmit buffer is set is at address 0071, inside the serial int handler.

[felipesanches]

ok, I got it! This fixed the issue!

[felipesanches]

Surely the more general solution would be to create a set_serial_mode(u16 value) method and then use it in all instructions that change the value of SMH (and SML). But I don't know how much do we want to change the CPU core in this PG-1000 PR.

[felipesanches]

For now, I made it write_smh(uint8_t data) to not over-complicate things...

[felipesanches]

I've just fixed MIDI IN. Now every message received on MIDI IN is repeated to MIDI OUT, as the service manual describes as the expected normal behavior.

[felipesanches]

@cuavas, I believe this is now ready for your code-review.

[felipesanches]

I've just git-squashed everything into a single commit and took the opportunity to re-read all my own code. I also cleaned up a few remaining white spaces.

This is ready for review. It is almost perfectly functional. The only remaining issue is some bug related to the MIDI ports of this device. While playing with my piano and a real Roland D-550 I get some data corruption specially when using the sustain pedal. I am actively investigating the bug, and I suspect it can be some CPU bug (perhaps an incorrect or incomplete implementation of one of the upd7810 instructions used in the serial port code?) but it will take some further research & troubleshooting time and it is impossible for me to predict how long will it take until I can find the solution.

I'd like to ask for this to be merged, keeping the MACHINE_NOT_WORKING flag due do this last remaining bug.

If, for any reason, anyone is interested in the original commits, they're still available at https://github.com/felipesanches/mame/tree/roland_pg1000__original_set_of_commits

[felipesanches]

I noticed today that I was getting INTER due to "framing error" (start and stop bits did not match expected values). And one of the reasons for that was because when both midi_in and param_in were disabled, the RXD pin was going low, causing a stream of zeroes into the RX shift register. This pin should instead be set to 5V (due to the pull-up resistor). Taking that into account on my driver, that source of framing errors (that was frequently happening when I used the sustain pedal) is gone:

	m_maincpu->rxd_func().set([this]() {
		return int(
			(m_mdin_bit && m_midi_in_enable) ||
			(m_paramin_bit && m_param_in_enable) ||
			(!m_midi_in_enable && !m_param_in_enable) /* due to pull-up */
		);
	});

But I still have something else causing a similar issue. From time to time I still get a framing error and I still don't know why.

[felipesanches]

Thanks!