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

Update PI DMA alignment as per CEN64 #868

Merged
merged 1 commit into from
Jun 14, 2021
Merged

Conversation

loganmc10
Copy link
Member

@loganmc10 loganmc10 commented Jun 11, 2021

Taken from: https://github.com/n64dev/cen64/blob/master/pi/controller.c

Tested using: https://github.com/PeterLemon/N64/tree/master/CPUTest/DMAAlignment-PI-cart

Before:
dma_alignment_(rom_--000

After:
dma_alignment_(rom_--000

There are still 2 tests that fail here that pass on CEN64: n64dev/cen64#200. However, I couldn't get those tests to pass without breaking the graphics in AI Shougi 3, so I left it at the point where AI Shougi 3 would work. AI Shougi 3 graphics are messed up in CEN64, so I assume there is still more work to be done on their implementation.

@Narann
Copy link
Member

Narann commented Jun 12, 2021

Thanks for the hard work. The code is clean, thanks for this!

I can merge this because I suspect no-one will have the required knowledge to validate this and the scope of this flag seems to be limited.

EDIT: I saw some force pushes, tell me when you think the PR is ready for merge. :)

@loganmc10
Copy link
Member Author

Yeah it's ready now, I don't plan on making any further changes

@loganmc10
Copy link
Member Author

Actually hold on for a little bit. @rasky did some hardware tests to figure out how to get the test ROM and AI Shougi 3 working

@Narann
Copy link
Member

Narann commented Jun 12, 2021

Haha! :) No worry!

@loganmc10 loganmc10 force-pushed the pialign branch 8 times, most recently from 46698ac to a83bcd1 Compare June 13, 2021 14:43
@loganmc10
Copy link
Member Author

Ok I'm happy with this now. The test ROM passes at the same level as CEN64, AI Shougi works. I also tested other games I know that have specific PI alignment requirements (like command & conquer and taz express). Everything seems to be working normally.

This work was based on hardware tests done by @rasky:
image

As well as tests previously documented here: https://n64brew.dev/wiki/Peripheral_Interface#Unaligned_DMA_transfer

@Narann
Copy link
Member

Narann commented Jun 14, 2021

Thanks for the hard work!

I don't know how the compatibility situation is but can you just convert the // by /* */ ?

@loganmc10
Copy link
Member Author

ok done

@Narann Narann merged commit 07590ea into mupen64plus:master Jun 14, 2021
@Narann
Copy link
Member

Narann commented Jun 14, 2021

Thanks! :)

@m4xw
Copy link
Contributor

m4xw commented Jun 15, 2021

This caused a regression for 64DD for me.
Reverting pi_controller solves this. No game would boot (stuck on black screen) or it would just enter the IPL.
Didnt check yet which change in particular caused this, just a headsup.

@loganmc10
Copy link
Member Author

No problem, I'll fix this and submit a new pr

@m4xw
Copy link
Contributor

m4xw commented Jun 16, 2021

Thanks, if you read discord, my suspicion rn is the pi->regs[PI_CART_ADDR_REG] touching it in some way that doesnt trigger the

      if ((pi->regs[PI_CART_ADDR_REG] == MM_DD_C2S_BUFFER) ||
           (pi->regs[PI_CART_ADDR_REG] == MM_DD_DS_BUFFER)) {

condition for the if(dd) stuff at end of pi event.
Didnt spend time today investigating further tho.

@m4xw
Copy link
Contributor

m4xw commented Jun 16, 2021

Forgot to add, as far i can tell from the talks with luigi, we can consider the end of pi dma event stuff for DD a hack. So feel free to rework as you see fit.

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

Successfully merging this pull request may close these issues.

None yet

3 participants