Fix RGBLINK scrambling algorithm to work for SRAM#2023
Merged
Conversation
Rangi42
added
bug
Contributor
Author
|
The $ xxd test/link/scramble-specs/out.gb
00000000: 0000 0000 0000 0000 0000 0000 0000 0000 ................
...
00003ff0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00004000: 0404 0404 0404 0404 0404 0404 0404 0404 ................
...
00005ff0: 0404 0404 0404 0404 0404 0404 0404 0404 ................
00006000: 0202 0202 0202 0202 0202 0202 0202 0202 ................
...
00007ff0: 0202 0202 0202 0202 0202 0202 0202 0202 ................
00008000: 0505 0505 0505 0505 0505 0505 0505 0505 ................
...
00009ff0: 0505 0505 0505 0505 0505 0505 0505 0505 ................
0000a000: 0303 0303 0303 0303 0303 0303 0303 0303 ................
...
0000bff0: 0303 0303 0303 0303 0303 0303 0303 0303 ................
0000c000: 0401 0201 0203 0304 0102 0304 0203 0001 ................
0000c010: 0203 ffff ffff ffff ffff ffff ffff ffff ................
0000c020: ffff ffff ffff ffff ffff ffff ffff ffff ................
...
0000cff0: ffff ffff ffff ffff ffff ffff ffff ffff ................
0000d000: 0606 0606 0606 0606 0606 0606 0606 0606 ................
...
0000eff0: 0606 0606 0606 0606 0606 0606 0606 0606 ................
0000f000: 0000 0000 0000 0000 0000 0000 0000 0000 ................
...
0000fff0: 0000 0000 0000 0000 0000 0000 0000 0000 ................
00010000: 0101 0101 0101 0101 0101 0101 0101 0101 ................
...
00011ff0: 0101 0101 0101 0101 0101 0101 0101 0101 ................
00012000: 0000 0000 0000 0000 0000 0000 0000 0000 ................
...
00013ff0: 0000 0000 0000 0000 0000 0000 0000 0000 ................ |
Since SRAM sections start from 0, we had been using a signed `int8_t curScrambleSRAM` counter to allow 0 as a scrambled bank, but this actually just placed all floating SRAM sections in bank 0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since SRAM sections start from 0, we had been using a signed
int8_t curScrambleSRAMcounter to allow 0 as a scrambled bank, but this actually just placed all floating SRAM sections in bank 0.The region spec
sizeis now explicitly documented as a count of banks, not as a maximum bank value. This does not actually change its behavior for ROMX or WRAMX, and is not a breaking change, since scrambling had not placed any guarantees on where sections get placed. (In practice it does predictably place them from the highest to lowest bank, just as the regular placement algorithm is also predictable, and some of our test cases rely on it; but it's always been an implementation detail, and has changed before.)