Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSU1 Audio Support #328

Merged
merged 1 commit into from
May 13, 2022
Merged

MSU1 Audio Support #328

merged 1 commit into from
May 13, 2022

Conversation

dentnz
Copy link
Contributor

@dentnz dentnz commented May 12, 2022

Initial pull request. Work in progress.

  • MSU1 AUDIO support only so far - some datafile code is present to handle it gracefully, mostly commented out for now
  • EXT bus is used for messaging (audio track selection, sector request, track missing, etc)
  • FXPak-compatible MSU1 hacks only - no support for the manifest file now, maybe ever
  • Main_MiSTer update also required - separate PR
  • Credit to ElectronAsh for his help on overall design, audio summing and volume handling
  • Credit to Qwertymodo for his test rom and assistance
  • Credit to Near for MSU1. RIP.

@birdybro
Copy link
Member

Might want to convert this to a draft, it doesn't currently meet timing requirements.

image

This may improve however if the framework is updated by sorg and you merge that change into your branch later.

@dentnz
Copy link
Contributor Author

dentnz commented May 12, 2022

Might want to convert this to a draft, it doesn't currently meet timing requirements.

image

This may improve however if the framework is updated by sorg and you merge that change into your branch later.

Not sure if it met timing requirements beforehand. This is using latest framework, so I don't think that would help.

@dentnz
Copy link
Contributor Author

dentnz commented May 12, 2022

Might want to convert this to a draft, it doesn't currently meet timing requirements.

I am trying a recompile of the master branch to see if it meets timing constraints. As I said, I didn't think it did the last time I checked. Will report back.

@dentnz
Copy link
Contributor Author

dentnz commented May 13, 2022

Might want to convert this to a draft, it doesn't currently meet timing requirements.

image

This may improve however if the framework is updated by sorg and you merge that change into your branch later.

@birdybro. As I suspected, the core without any MSU1 code (master) also has these timing constraint issues:
image

Granted, the MSU1 changes would increase timing constraint violations fractionally (and they have according to your screenshot versus mine), but the problems were there prior to my work. Thoughts?

@birdybro
Copy link
Member

birdybro commented May 13, 2022

The current master branch from mister-devel does not have the timing improvements that were done by @wickerwaka so merging won't change anything anyways. EDIT: Oh you meant the current master to compare, got it. :)

Nice catch. The framework for the SNES core should probably be updated first anyways. :)

@dentnz
Copy link
Contributor Author

dentnz commented May 13, 2022

I won't step on anyone's toes and leave the framework update to Sorg or Srg320. I also don't have much experience with timing constraint violations and fixing them. That said, I am aware there is a good chance that the MSU1 code could be optimized, and am very open to improvements. Probably some of the internal registers are on the large size. And some of the calculations (division by 4 for example) could be handled bitwise, which may mean less logic on the FPGA. For now though, I am most interested in the overall design and ensuring that it's acceptable. I'll probably work on video/datafile next without that approval. Getting the audio side of the functionality in officially is the most important part as 95 percent of MSU1 content is audio 'enhancement' only anyway.

@sorgelig sorgelig merged commit c793fda into MiSTer-devel:master May 13, 2022
@sorgelig
Copy link
Member

sorgelig commented May 13, 2022

well, since it was hard to check changes i missed one important change which would stop me from merging.
MSU is connected to CPU data input by default without being explicitly selected. Old code used OPENBUS, and it was there not for fun.
Therefore please provide the fix ASAP, so MSU won't be on bus without explicit selection and connect OPENBUS as it was before.

@sorgelig
Copy link
Member

one more important fix should be added on both FPGA and HPS sides: if no MSU files are available for current ROM then MSU should be completely shutdown and don't reply even if its registers are selected, so it won't produce bugs in games not expecting MSU registers on bus.

@sorgelig
Copy link
Member

I've added MSU enable flag based on availability of pcm files for rom. It fully hides MSU. I don't have MSU ROMs for test, so hope i didn't break anything.

@paulb-nl
Copy link
Contributor

It doesn't seem correct. According to here the MSU registers range from $2000-$2007 so MSU_SEL should be MSU_ENABLE and CA = $2000-$2007 otherwise OPENBUS won't work correctly: http://helmet.kafuka.org/msu1.htm

Also is it correct that MSU_SEL is only set at 96Mbit section?

@sorgelig
Copy link
Member

sorgelig commented May 13, 2022

Also is it correct that MSU_SEL is only set at 96Mbit section?

yeah, i had similar question but thought all MSU roms are big to fall into 96mbit range.

My fix was to isolate MSU roms from all others. So i've made sure that MSU is disabled if no CD tracks are found. But how MSU is selected on MSU-enabled ROMs i don't know. It's up to original implementation

@sorgelig
Copy link
Member

generally speaking MSU mapper implementation doesn't follow original concept and do selections inside its own module instead of mapper.

@sorgelig
Copy link
Member

i've added some tweaks and fixes including better way to enable MSU1.
I found Zelda MSU1 - it doesn't work.. Actually core sends track number in BCD format while Main expects it in plain binary format. Even after change it to BCD it doesn't work. Game requests the same track "million times" and then hangs...

@paulb-nl
Copy link
Contributor

First issue is msu_audio_download is not connected to hps_ext in SNES.sv so it stays 0.

hps_ext hps_ext

input msu_audio_download

Second issue is Main doesn't send MSU_AUDIO_TRACKMOUNTED because msu_trackrequest is set to 0 here:
https://github.com/MiSTer-devel/Main_MiSTer/blob/12ea16f024779515cb067998898387692ddd2fa2/support/snes/snes.cpp#L523

Here it checks for msu_trackmounted == 1 && msu_trackrequest == 1:
https://github.com/MiSTer-devel/Main_MiSTer/blob/12ea16f024779515cb067998898387692ddd2fa2/support/snes/snes.cpp#L545

After fixing those I have music in Zelda.

SNES MSU: Track requested
SNES MSU - New track selected: 0x1
SNES MSU - Full MSU track path is: SNES/msu/alttp_msu-1.pcm
Mount SNES/msu/alttp_msu-1.pcm as read-write on 2 slot
SNES MSU - Track mounted
SNES MSU: sending track mounted - 201

Haven't touched anything about BCD format so it doesn't seem to matter.

@paulb-nl
Copy link
Contributor

audio_fifo_dout should be 32 bit now. The right channel stays silent.

wire [15:0] audio_fifo_dout;

@sorgelig
Copy link
Member

Making commit before going to sleep probably wasn't good idea :)
I was testing on display with mono speaker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants