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

emu/romload.cpp: add flag to reverse bits in each byte of a region #11822

Closed
wants to merge 3 commits into from

Conversation

mgarlanger
Copy link
Contributor

Adds a flag to ROM Region, to allow devices to specify reversing the bits in each byte during loading of the ROM.

In two of the graphic add-ons for the H19/H89 systems, the pixel memory is reverse of the font data(imaginator already merged and SigmaSoft IGC currently in a PR - #11801). This requires reversing one or the other for every byte before the values can be ORed together. By allowing the font ROM to be loaded with the bits reversed, this is no longer needed. This also slightly simplifies the calculation for the pixel drawing of the font in other cases too (no longer need the 7 - b operation to select the bit to draw).

@galibert
Copy link
Member

galibert commented Dec 8, 2023

I have several problems with that PR, sorry:

  • I don't see why it would be a region flag and not a rom load flag
  • it forces the swap to be on bytes instead of taking the actual region data width into account
  • it gives an inaccurate picture of the hardware, since I'm pretty sure the schematics, if any, will show the rom bits in the normal order and just have a left-shifting shifter in there.
  • I doubt there's any performance difference in the end

I'm fascinated that the chargen and the graphics ram have inverted bit order in your system though...

@ajrhacker
Copy link
Contributor

I'm pretty sure the schematics, if any, will show the rom bits in the normal order

Don't think this applies here, but there was a period in IC history where TI and Intel were both major EPROM vendors and numbering bits in the opposite order of each other.

@rb6502
Copy link
Contributor

rb6502 commented Dec 8, 2023

Apple II has different video ROM bit orders (and bit polarities!) between various official models and (especially) between real systems and clones. I templatized the update routine to avoid run-time checks about how to draw things, but some drivers simply reorder the ROM data at machine_start() time to get around this sort of thing. Either way I don't think this is the way to do it.

@cuavas
Copy link
Member

cuavas commented Dec 8, 2023

I’m not keen on this.

A region flag to reversing bit order per byte is not intuitive as regions are not necessarily one byte wide.

A flag on a ROM load command to reverse bit order isn’t going to be intuitive because the ROM load command doesn’t actually know the width of the ROM. The grouping/stride is only used if it’s needed for changing the byte order to match the system’s view of it, and it doesn’t necessarily match the physical data width of the ROM. (Add to that the fact that the system’s view may not match the hardware for convenience or performance reasons.)

Usually we deal with bit scrambling by patching up the data in device_start or a driver init function.

@mgarlanger
Copy link
Contributor Author

I'll go ahead and close this PR and look at adding this to device_start.

@mgarlanger mgarlanger closed this Dec 9, 2023
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.

5 participants