aaronsgiles / ymfm

BSD-licensed Yamaha FM sound cores (OPM, OPN, OPL, and others)
BSD 3-Clause "New" or "Revised" License
259 stars 40 forks source link

ADPCM: fixed address checking on read/write #38

Closed hyano closed 2 years ago

hyano commented 2 years ago

@aaronsgiles Thanks for merging https://github.com/aaronsgiles/ymfm/pull/35 .

We have changed the definition of at_end() and at_limit() in https://github.com/aaronsgiles/ymfm/pull/33#issuecomment-1114387840 .

So, we need to modify the address checking not only on clock(), but also on read()/write(). Please check this patch.

P.S. I have resumed verification of real YM2608's ADPCM. https://github.com/hyano/opna-analyze/blob/main/doc/OPNA.md If I can find the internal behavior I will try to implement and send you another PR.

aaronsgiles commented 2 years ago

Wow, thanks for that great analysis! I'm currently reshuffling the code to align better with what you've discovered so far. Once I get it working, I'll update the code so that we can adjust it from there.

hyano commented 2 years ago

Thank you for your comment and seeing my memo even in Japanese. This PR is just temporary fix for external behavior.

I'm happy that you will update the code with my analysis. Please reject this PR whenever you like.

The charts and memos are summarized from signal and register value (with some guessing). If you feel anything suspicious or unclear, please let me know. I'll revisit logs or add tests.

aaronsgiles commented 2 years ago

Do you have examples of systems/software that actually read or write data through the YM2608? (Or the YM2610?) I'd like to verify the reading and writing with live software if I can find some.

aaronsgiles commented 2 years ago

Ok, I have made my first attempt. Please see the branch adpcm-test.

Using a test program I seem to be matching your readings.

I am unsure of a few things, such as:

You can try this code in your environment to see if it behaves the way you expect.

aaronsgiles commented 2 years ago

I just pushed an update to the branch that tries to fix some of the flag behaviors. Are these correct assumptions? (I'm guessing from your diagrams):

hyano commented 2 years ago

I tried 0565762bd630c05bb597c23e305d05854d11682d. But it does not seem to work as expected with my environment. Write and read(verify) data doesn't work. I'll check.

I extracted the register access sequence for testing with real chip. https://github.com/hyano/opna-analyze/blob/main/test/metal_force_98.py It contains the result by real chip. You can check it. (bit.6 of status 1 is unknown and not deterministic.)

  • what does R10:$80 mean in your diagrams? (did you mean R00:$80?)

R10:$80 means write 0x80 to FLAG CONTROL REGISTER(0x10) to clear status (make all status flag to "0").

  • state of BRDY flag

I cannot check in detail because of my environment (slow to read status quickly). So far I saw is:

You can refer another analysis memo by KAJA. http://www5.airnet.ne.jp/kajapon/brdy_eos.txt

  • behavior of writes to sample RAM (are there dummy writes like dummy reads?)

No dummy write like behavior. MDEN is asserted just after write register 0x08.

I have logs and will create chart later.

  • behavior relative to delta-N (does ADPCM state only change on overflow or is it more complex?)

I can check and will try.

aaronsgiles commented 2 years ago

Ok thank you for the logs. I have made some updates.

It seems EOS and PLAYING flags are sticky until reset by the chip. There are 3 chips that use the ADPCM-B engine: YM2608, YM2610, and Y8950. I have changed the behavior so that the chips need to explicitly reset these flags.

See commit e2cd8df0ae4b04dab8a6987881f9f2e8726b22df for a new update that matches your test behavior exactly, except for undocumented status flag 0x40, which I do not understand.

hyano commented 2 years ago
  • behavior relative to delta-N (does ADPCM state only change on overflow or is it more complex?)

I can check and will try.

I misunderstood your point as PCM outpu overflow. I'll try if I can understand what kind of testing is required. Please tell me if you have.

However, I have created PCM overflow test and captured signals. I noticed that somthing like 'clamp' happens during delta-N period. YM2608 may have a separate ADPCM clamp mechanism before accumlating to mixed output.

Reference:

Is it OK to continue this kind of topic on this PR...?

aaronsgiles commented 2 years ago

Discussion on the PR is fine, or we can move to an issue instead if you prefer.

hyano commented 2 years ago

See commit e2cd8df for a new update that matches your test behavior exactly, except for undocumented status flag 0x40, which I do not understand.

It looks working fine. Thanks. The external DRAM is detected as expected. And ADPCM data transfer was also performed successfully.

aaronsgiles commented 2 years ago

New commit on this branch makes all the tests in https://github.com/hyano/opna-analyze/blob/main/log/mem_rw_mden/mem_rw.txt match except for the first two dummy reads in "READ ADDRESS 0fff- CHANGE STOP" -- I do not understand why those would read $08.

hyano commented 2 years ago

except for the first two dummy reads in "READ ADDRESS 0fff- CHANGE STOP" -- I do not understand why those would read $08.

I checked where dummy read data came from. https://github.com/hyano/opna-analyze/blob/main/test/mem_rw10.py https://github.com/hyano/opna-analyze/blob/main/log/mem_rw10_mden/mem_rw10.txt If the READ sequence is aborted, the data remains in m_buffer and would be seen in the next DUMMY READ.

Below are the results of the experiment with some parameters.

I also experimented with checking DUMMY READ after writing too much data with REPEAT=1. (It is illegal usage and for analysis.)

After writing at STOP ADDRESS, it appears to switch to reading even if CPU is writing to REG $08. Just after writing data at STOP ADDRESS, data on START ADDRESS seems to be loaded to buffer. The last written data is seen on the top of m_buffer in the next DUMMY READ. And the 2nd data of DUMMY READ seems to be the data in m_buffer which was read from memory.

aaronsgiles commented 2 years ago

Ok, if you grab the latest adpcm-test branch, it should produce matching results for all the cases you mention above. ade388dda51cd7d3e8142ea2cdb3dd48426d044f

I also checked in a C++ version of your rw13/14/15 tests that I used to test it out.

aaronsgiles commented 2 years ago

At this point, I have created a new PR for all this work: https://github.com/aaronsgiles/ymfm/pull/40

Let's move any further discussion over there and close out this PR.