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

Migrate cv1k and nexus3d drivers to use nandflash.h instead of serflash.h #11708

Merged
merged 8 commits into from Nov 12, 2023

Conversation

buffis
Copy link
Contributor

@buffis buffis commented Nov 5, 2023

serflash.h looks real old and sortof messy, and is pretty much implementing the basic functionality of K9F1G08U0 (it has that manufacturer/device ID hardcoded).

It's only used by cv1k and nexus3d.cpp (which I'll also migrate in a followup, removing serflash).

nandflash.h seems newer, has support for SAMSUNG_K9F1G08U0B and looks less jank

Migrating to it is pretty straightforward and seems to work.

Only difference is the write to 0x10C00003 which controls the CE pin of U2 (through the U13 CPLD). serflash seems to have a setter for this (flash_enab_w), but I don't really see how it's needed. As far as I can understand, its only used to signal that U2 should be the one to access the shared data-bus, which is a hardware limitation not relevant for emulation.

I assume that's why there's no handling of CE pin in nandflash.h/cpp

Tried a few different games, and seems to work fine

serflash.h looks real old and sortof messy, and is pretty much implementing the basic functionality of K9F1G08U0 (it has that manufacturer/device ID hardcoded).

It's only used by cv1k and nexus3d.cpp (which I'll fix in a followup).

nandflash.h seems newer, has support for SAMSUNG_K9F1G08U0B and looks less jank

Migrating to it is pretty straightforward and seems to work.

Only difference is the write to 0x10C00003 which controls the CE pin of U2 (through the U13 CPLD). serflash seems to have a setter for this (flash_enab_w), but I don't really see how it's needed. As far as I can understand, its only used to signal that U2 should be the one to access the shared data-bus, which is a hardware limitation not relevant for emulation.

I assume that's why there's no handling of CE pin in nandflash.h/cpp

Tried a few different games, and seems to work fine
The actual nand is K9F1G08U0M, not K9F1G08U0B

The only difference is result of READ ID, see:
http://site.buffis.com/cv1k/datasheets/K9F1G08U0M_U2.pdf
section "Read ID" on page 29

In practice this probably wont matter, since only first two bytes are used for verification, but this is more correct
Nexus 3D skeleton driver currently uses some hacky incomplete methods in serialflash.

This board uses SAMSUNG K9F2G08U0M NAND, so lets just add that to nandflash as well. Values for Read ID + page/block sizes are from datasheet.

Since this is a skeleton driver, only verification I did was making sure it compiles.
@buffis buffis changed the title Migrate cv1k driver to use nandflash.h instead of serflash.h Migrate cv1k and nexus3d drivers to use nandflash.h instead of serflash.h Nov 6, 2023
@angelosa
Copy link
Member

angelosa commented Nov 6, 2023

For nexus3d.cpp it's mildly interesting if you can check by patching in debugger the two CMPS listed in init and see if it doesn't diverge afterwards, but I honestly don't mind either way.

@angelosa
Copy link
Member

angelosa commented Nov 7, 2023

I checked thru the CI build, it looks like it's not loading the initial bootstrap properly with this.

@buffis
Copy link
Contributor Author

buffis commented Nov 7, 2023

Thanks for spotting.
I've been messing around with the debugger, and I think this is a bug in nandflash.cpp

nexus3d does the following read sequence after a while:

00 -> 9c000010 # READ CYCLE1
00 -> 9c000018 # A COL0
00 -> 9c000018 # A COL1
01 -> 9c000018 # A ROW1
00 -> 9c000018 # A ROW2
00 -> 9c000018 # A ROW3
30 -> 9c000010 # READ CYCLE2
70 -> 9c000010 # READ STATUS
e0 <- 9c000000 # STATUS = OK 
00 -> 9c000010 # READ CYCLE1
XX <- 9c000000 # DATA READ ....
XX <- 9c000000 # DATA READ ....
XX <- 9c000000 # DATA READ .... 
...

The read status, followed by READCYCLE1 before reading looks kindof weird, but datasheet states under READ STATUS

"The command register remains in Status Read mode until further commands are issued to it. Therefore, if the status register is read during a random read cycle, the read command(00h) should be given before starting read cycles."

which I guess explains this.

nandflash.cpp will however reset the address when this triggers, which seems like a bug. This means that for the serflash function, row=1 will be used. For nandflash, row=0 will be used.

I'll look into fixing it.

@buffis
Copy link
Contributor Author

buffis commented Nov 7, 2023

TL;DR for above is that this should not be executed in command_w (data=0x00)

m_page_addr = 0 

m_page_addr should only be updated on the second command cycle of a read command in nandflash. Should be easy to fix, but wont have time today probably

…EAD/PROGRAM cycle

Address should not be updated until the second cycle. There's not a lot of great examples on this in datasheets, but as an example, the K9F1G08Q0M datasheet mentions

"The command register remains in Status Read mode until further commands are issued to it. Therefore, if the status register is read during a random read cycle, the read command(00h) should be given before starting read cycles."

nexus3d does this.

Example:

```
00 -> 9c000010 # READ CYCLE1
00 -> 9c000018 # A COL0
00 -> 9c000018 # A COL1
01 -> 9c000018 # A ROW1
00 -> 9c000018 # A ROW2
00 -> 9c000018 # A ROW3
30 -> 9c000010 # READ CYCLE2
70 -> 9c000010 # READ STATUS
e0 <- 9c000000 # STATUS = OK
00 -> 9c000010 # READ CYCLE1
XX <- 9c000000 # DATA READ ....
XX <- 9c000000 # DATA READ ....
XX <- 9c000000 # DATA READ ....
...

```
@buffis
Copy link
Contributor Author

buffis commented Nov 7, 2023

Actually, turns out it was pretty easy. Haven't verified other nandflash users yet, which I should, but now for nexus32 with rom "acheart", you can trigger the bootstrapping by doing the following in mame debugger

> bp 0x1234
> run
> R0 = 1
> run

There will be some blinking text on the top of screen afterwards then, which seems cool

@angelosa , please take a look

@angelosa
Copy link
Member

angelosa commented Nov 7, 2023

Yeah, iirc after a while it will start dialoguing with the VRender3d FIFO with that debug trick in, never managed to make a sense over that one for the lack of any footage in the first place. :)

@buffis
Copy link
Contributor Author

buffis commented Nov 7, 2023

Actually, the rabbit hole goes deeper I guess.

Some of the NAND types don't use a second read cycle (k9f5608u0d at least does not), which means that this will not work for that, so will have to separate out 1 and 2 cycle read IC's a bit.

I'll do that tomorrow or something. Don't merge this until then

@angelosa angelosa marked this pull request as draft November 7, 2023 21:42
@angelosa
Copy link
Member

angelosa commented Nov 7, 2023

Please mark as non-draft once ready.

@mamehaze
Copy link
Contributor

mamehaze commented Nov 8, 2023

I wonder what check it's failing to need that debugger hack in the first place

@galibert
Copy link
Member

galibert commented Nov 8, 2023

The call of 8b2cc at 8b834 returns ff in r0, while the code wants c4. The c4 comes from that part of the flash:
000e9d80: 0000 0000 8000 c4ff ffea 4372 617a 7920 ..........Crazy
000e9d90: 4172 6361 6465 2042 6e42 2028 5645 4741 Arcade BnB (VEGA
000e9da0: 3329 0000 0000 e813 9fe5 5000 81e5 0ef0 3)........P.....

The code at 8b2cc is rather complicated, I don't have time to analyze it right now :-)

OG.

@mamehaze
Copy link
Contributor

mamehaze commented Nov 8, 2023

Arcade BnB is... "bballoon BnB Arcade" on different hardware, so is this some kind of recycled code?

… first READ/PROGRAM cycle"

This reverts commit 4af5f68.
Comment on lines 195 to 196
#include "machine/rtc9701.h"
#include "machine/serflash.h"
#include "machine/nandflash.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remember to get these back in alphabetical order.

This seems like the cleanest way to support the case of Read Status during Read, as seen in acheart.

Basically from datasheet:
"The command register remains in Status Read mode until further commands are issued to it. Therefore, if the status register is read during a random read cycle, the read command(00h) should be given before starting read cycles."

Seems cleanest to do this for Program as well, since otherwise there would be a lot of code duplication, and this works just as well.

Example from acheart:

```
00 -> 9c000010 # READ CYCLE1
00 -> 9c000018 # A COL0
00 -> 9c000018 # A COL1
01 -> 9c000018 # A ROW1
00 -> 9c000018 # A ROW2
00 -> 9c000018 # A ROW3
30 -> 9c000010 # READ CYCLE2
70 -> 9c000010 # READ STATUS
e0 <- 9c000000 # STATUS = OK
00 -> 9c000010 # READ CYCLE1
XX <- 9c000000 # DATA READ ....
XX <- 9c000000 # DATA READ ....
XX <- 9c000000 # DATA READ ....
```
@buffis
Copy link
Contributor Author

buffis commented Nov 8, 2023

There, I think this is the cleanest way to support the Read Status during Read as described further up.

I verified that acheart, futari15, espgal2, palmz22, mrdrilrg and mini2440 works as before, to have good coverage of different NAND chips.

@buffis buffis marked this pull request as ready for review November 8, 2023 20:32
@cuavas cuavas merged commit 74b3292 into mamedev:master Nov 12, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants