improve CartEnhanced to allow swapped RAM read/write ports

refactor CartCV
add more CV test ROMs

src/emucore/CartCV.cxx

@@ -21,164 +21,42 @@
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 CartridgeCV::CartridgeCV(const ByteBuffer& image, size_t size,
                          const string& md5, const Settings& settings)
-  : Cartridge(settings, md5),
-    mySize(size)
+  : CartridgeEnhanced(image, size, md5, settings)
 {
-  if(mySize == myImage.size())
-  {
-    // Copy the ROM data into my buffer
-    std::copy_n(image.get(), myImage.size(), myImage.begin());
-  }
-  else if(mySize == 4_KB)
+  myBankShift = BANK_SHIFT;
+  myRamSize = RAM_SIZE;
+  myRamWpHigh = RAM_HIGH_WP;
+
+
+  if(mySize == 4_KB)
   {
     // The game has something saved in the RAM
     // Useful for MagiCard program listings
 
-    // Copy the ROM data into my buffer
-    std::copy_n(image.get() + myImage.size(), myImage.size(), myImage.begin());
+    // Allocate array for the ROM image
+    mySize = 2_KB;
+    myImage = make_unique<uInt8[]>(mySize);
 
+    // Copy the ROM image into my buffer
+    std::copy_n(image.get() + mySize, mySize, myImage.get());
+
+
+    myInitialRAM = make_unique<uInt8[]>(1_KB);
     // Copy the RAM image into a buffer for use in reset()
-    std::copy_n(image.get(), myInitialRAM.size(), myInitialRAM.begin());
+    std::copy_n(image.get(), 1_KB, myInitialRAM.get());
   }
-  createRomAccessArrays(myImage.size() + myRAM.size());
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void CartridgeCV::reset()
 {
-  if(mySize == 4_KB)
+  if(myInitialRAM != nullptr)
   {
     // Copy the RAM image into my buffer
-    myRAM = myInitialRAM;
+    std::copy_n(myInitialRAM.get(), 1_KB, myRAM.get());
   }
   else
-    initializeRAM(myRAM.data(), myRAM.size());
+    initializeRAM(myRAM.get(), myRamSize);
 
   myBankChanged = true;
 }
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void CartridgeCV::install(System& system)
-{
-  mySystem = &system;
-
-  System::PageAccess access(this, System::PageAccessType::READ);
-
-  // Map ROM image into the system
-  for(uInt16 addr = 0x1800; addr < 0x2000; addr += System::PAGE_SIZE)
-  {
-    access.directPeekBase = &myImage[addr & 0x07FF];
-    access.romAccessBase = &myRomAccessBase[addr & 0x07FF];
-    access.romPeekCounter = &myRomAccessCounter[addr & 0x07FF];
-    access.romPokeCounter = &myRomAccessCounter[(addr & 0x07FF) + myAccessSize];
-    mySystem->setPageAccess(addr, access);
-  }
-
-  // Set the page accessing method for the RAM writing pages
-  // Map access to this class, since we need to inspect all accesses to
-  // check if RWP happens
-  access.directPeekBase = nullptr;
-  access.romAccessBase = nullptr;
-  access.type = System::PageAccessType::WRITE;
-  for(uInt16 addr = 0x1400; addr < 0x1800; addr += System::PAGE_SIZE)
-    mySystem->setPageAccess(addr, access);
-
-  // Set the page accessing method for the RAM reading pages
-  access.directPokeBase = nullptr;
-  access.type = System::PageAccessType::READ;
-  for(uInt16 addr = 0x1000; addr < 0x1400; addr += System::PAGE_SIZE)
-  {
-    access.directPeekBase = &myRAM[addr & 0x03FF];
-    access.romAccessBase = &myRomAccessBase[2048 + (addr & 0x03FF)];
-    access.romPeekCounter = &myRomAccessCounter[2048 + (addr & 0x03FF)];
-    access.romPokeCounter = &myRomAccessCounter[2048 + (addr & 0x03FF) + myAccessSize];
-    mySystem->setPageAccess(addr, access);
-  }
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-uInt8 CartridgeCV::peek(uInt16 address)
-{
-  // The only way we can get to this method is if we attempt to read from
-  // the write port (0xF400 - 0xF7FF, 1024 bytes), in which case an
-  // unwanted write is potentially triggered
-  return peekRAM(myRAM[address & 0x03FF], address);
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool CartridgeCV::poke(uInt16 address, uInt8 value)
-{
-
-  if(address & 0x0400)
-  {
-    pokeRAM(myRAM[address & 0x03FF], address, value);
-    return true;
-  }
-  else
-  {
-    // Writing to the read port should be ignored, but trigger a break if option enabled
-    uInt8 dummy;
-
-    pokeRAM(dummy, address, value);
-    myRamWriteAccess = address;
-    return false;
-  }
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool CartridgeCV::patch(uInt16 address, uInt8 value)
-{
-  address &= 0x0FFF;
-
-  if(address < 0x0800)
-  {
-    // Normally, a write to the read port won't do anything
-    // However, the patch command is special in that ignores such
-    // cart restrictions
-    // The following will work for both reads and writes
-    myRAM[address & 0x03FF] = value;
-  }
-  else
-    myImage[address & 0x07FF] = value;
-
-  return myBankChanged = true;
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-const uInt8* CartridgeCV::getImage(size_t& size) const
-{
-  size = myImage.size();
-  return myImage.data();
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool CartridgeCV::save(Serializer& out) const
-{
-  try
-  {
-    out.putByteArray(myRAM.data(), myRAM.size());
-  }
-  catch(...)
-  {
-    cerr << "ERROR: CartridgeCV::save" << endl;
-    return false;
-  }
-
-  return true;
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool CartridgeCV::load(Serializer& in)
-{
-  try
-  {
-    in.getByteArray(myRAM.data(), myRAM.size());
-  }
-  catch(...)
-  {
-    cerr << "ERROR: CartridgeCV::load" << endl;
-    return false;
-  }
-
-  return true;
-}

src/emucore/CartCV.hxx

@@ -21,7 +21,7 @@
 class System;
 
 #include "bspf.hxx"
-#include "Cart.hxx"
+#include "CartEnhanced.hxx"
 #ifdef DEBUGGER_SUPPORT
   #include "CartCVWidget.hxx"
 #endif
@@ -33,9 +33,9 @@ class System;
   $F400-$F7FF write to RAM
   $F800-$FFFF ROM
 
-  @author  Eckhard Stolberg
+  @author  Eckhard Stolberg, Thomas Jentzsch
 */
-class CartridgeCV : public Cartridge
+class CartridgeCV : public CartridgeEnhanced
 {
   friend class CartridgeCVWidget;
 
@@ -58,47 +58,6 @@ class CartridgeCV : public Cartridge
     */
     void reset() override;
 
-    /**
-      Install cartridge in the specified system.  Invoked by the system
-      when the cartridge is attached to it.
-
-      @param system The system the device should install itself in
-    */
-    void install(System& system) override;
-
-    /**
-      Patch the cartridge ROM.
-
-      @param address  The ROM address to patch
-      @param value    The value to place into the address
-      @return    Success or failure of the patch operation
-    */
-    bool patch(uInt16 address, uInt8 value) override;
-
-    /**
-      Access the internal ROM image for this cartridge.
-
-      @param size  Set to the size of the internal ROM image data
-      @return  A pointer to the internal ROM image data
-    */
-    const uInt8* getImage(size_t& size) const override;
-
-    /**
-      Save the current state of this cart to the given Serializer.
-
-      @param out  The Serializer object to use
-      @return  False on any errors, else true
-    */
-    bool save(Serializer& out) const override;
-
-    /**
-      Load the current state of this cart from the given Serializer.
-
-      @param in  The Serializer object to use
-      @return  False on any errors, else true
-    */
-    bool load(Serializer& in) override;
-
     /**
       Get a descriptor for the device name (used in error checking).
 
@@ -118,35 +77,22 @@ class CartridgeCV : public Cartridge
     }
   #endif
 
-  public:
-    /**
-      Get the byte at the specified address
+  private:
+    bool checkSwitchBank(uInt16, uInt8 = 0) override { return false; };
 
-      @return The byte at the specified address
-    */
-    uInt8 peek(uInt16 address) override;
-
-    /**
-      Change the byte at the specified address to the given value
-
-      @param address The address where the value should be stored
-      @param value The value to be stored at the address
-      @return  True if the poke changed the device address space, else false
-    */
-    bool poke(uInt16 address, uInt8 value) override;
+  protected:
+    // Initial RAM data from the cart (doesn't always exist)
+    ByteBuffer myInitialRAM{nullptr};
 
   private:
-    // The 2k ROM image for the cartridge
-    std::array<uInt8, 2_KB> myImage;
+    // Calculated as: log(ROM bank segment size) / log(2)
+    static constexpr uInt16 BANK_SHIFT = 11;  // 2K
 
-    // Initial size of the cart data
-    size_t mySize{0};
+    // RAM size
+    static constexpr uInt16 RAM_SIZE = 0x400; // 1K
 
-    // The 1024 bytes of RAM
-    std::array<uInt8, 1_KB> myRAM;
-
-    // Initial RAM data from the cart (doesn't always exist)
-    std::array<uInt8, 1_KB> myInitialRAM;
+    // Write port for extra RAM is at high address
+    static constexpr bool RAM_HIGH_WP = true;
 
   private:
     // Following constructors and assignment operators not supported

src/emucore/CartEnhanced.cxx

@@ -42,6 +42,8 @@ void CartridgeEnhanced::install(System& system)
   myBankMask = myBankSize - 1;            // e.g. = 0x0FFF
   myBankSegs = 1 << (12 - myBankShift);   // e.g. = 1
   myRamMask = myRamSize - 1;              // e.g. = 0xFFFF (doesn't matter for RAM size 0)
+  myWriteOffset = myRamWpHigh ? myRamSize : 0;
+  myReadOffset  = myRamWpHigh ? 0 : myRamSize;
 
   // Allocate array for the current bank segments slices
   myCurrentSegOffset = make_unique<uInt32[]>(myBankSegs);
@@ -58,24 +60,24 @@ void CartridgeEnhanced::install(System& system)
   // Map access to this class, since we need to inspect all accesses to
   // check if RWP happens
   access.type = System::PageAccessType::WRITE;
-  for(uInt16 addr = 0x1000; addr < 0x1000 + myRamSize; addr += System::PAGE_SIZE)
+  for(uInt16 addr = 0x1000 + myWriteOffset; addr < 0x1000 + myWriteOffset + myRamSize; addr += System::PAGE_SIZE)
   {
     uInt16 offset = addr & myRamMask;
-    access.romAccessBase = &myRomAccessBase[offset];
-    access.romPeekCounter = &myRomAccessCounter[offset];
-    access.romPokeCounter = &myRomAccessCounter[offset + myAccessSize];
+    access.romAccessBase = &myRomAccessBase[myWriteOffset + offset];
+    access.romPeekCounter = &myRomAccessCounter[myWriteOffset + offset];
+    access.romPokeCounter = &myRomAccessCounter[myWriteOffset + offset + myAccessSize];
     mySystem->setPageAccess(addr, access);
   }
 
   // Set the page accessing method for the RAM reading pages
   access.type = System::PageAccessType::READ;
-  for(uInt16 addr = 0x1000 + myRamSize; addr < 0x1000 + myRamSize * 2; addr += System::PAGE_SIZE)
+  for(uInt16 addr = 0x1000 + myReadOffset; addr < 0x1000 + myReadOffset + myRamSize; addr += System::PAGE_SIZE)
   {
     uInt16 offset = addr & myRamMask;
     access.directPeekBase = &myRAM[offset];
-    access.romAccessBase = &myRomAccessBase[myRamSize + offset];
-    access.romPeekCounter = &myRomAccessCounter[myRamSize + offset];
-    access.romPokeCounter = &myRomAccessCounter[myRamSize + offset + myAccessSize];
+    access.romAccessBase = &myRomAccessBase[myReadOffset + offset];
+    access.romPeekCounter = &myRomAccessCounter[myReadOffset + offset];
+    access.romPokeCounter = &myRomAccessCounter[myReadOffset + offset + myAccessSize];
     mySystem->setPageAccess(addr, access);
   }
 
@@ -106,7 +108,9 @@ uInt8 CartridgeEnhanced::peek(uInt16 address)
     checkSwitchBank(address & 0x0FFF);
   address &= myBankMask;
 
-  if(address < myRamSize)  // Write port is at 0xF000 - 0xF07F (128 bytes)
+  // Write port is e.g. at 0xF000 - 0xF07F (128 bytes)
+  if(address < myReadOffset + myRamSize && address >= myReadOffset)
+    // This is a read accees to a write port!
     return peekRAM(myRAM[address], peekAddress);
   else
     return myImage[myCurrentSegOffset[(peekAddress & 0xFFF) >> myBankShift] + address];
@@ -115,15 +119,18 @@ uInt8 CartridgeEnhanced::peek(uInt16 address)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool CartridgeEnhanced::poke(uInt16 address, uInt8 value)
 {
+  uInt16 pokeAddress = address;
+
   // Switch banks if necessary
   if (checkSwitchBank(address & 0x0FFF, value))
     return false;
+  address &= myBankMask;
 
   if(myRamSize)
   {
-    if(!(address & myRamSize))
+    if(bool(address & myRamSize) == myRamWpHigh)
     {
-      pokeRAM(myRAM[address & myRamMask], address, value);
+      pokeRAM(myRAM[address & myRamMask], pokeAddress, value);
       return true;
     }
     else
@@ -131,8 +138,8 @@ bool CartridgeEnhanced::poke(uInt16 address, uInt8 value)
       // Writing to the read port should be ignored, but trigger a break if option enabled
       uInt8 dummy;
 
-      pokeRAM(dummy, address, value);
-      myRamWriteAccess = address;
+      pokeRAM(dummy, pokeAddress, value);
+      myRamWriteAccess = pokeAddress;
     }
   }
   return false;

src/emucore/CartEnhanced.hxx

@@ -160,6 +160,21 @@ class CartridgeEnhanced : public Cartridge
     // The mask for the extra RAM
     uInt16 myRamMask{0};                        // RAM_SIZE - 1, but doesn't matter when RAM_SIZE is 0
 
+    // The offset into ROM space for writing to RAM
+    // - xxSC  = 0x0000
+    // - FA(2) = 0x0000
+    // - CV    = 0x0400
+    uInt16 myWriteOffset{0};
+
+    // The offset into ROM space for reading from RAM
+    // - xxSC  = 0x0080
+    // - FA(2) = 0x0100
+    // - CV    = 0x0000
+    uInt16 myReadOffset{0};
+
+    // Flag, true if write port is at high and read port is at low address
+    bool myRamWpHigh{RAM_HIGH_WP};
+
     // Pointer to a dynamically allocated ROM image of the cartridge
     ByteBuffer myImage{nullptr};
 
@@ -182,6 +197,9 @@ class CartridgeEnhanced : public Cartridge
     // The size of extra RAM in ROM address space
     static constexpr uInt16 RAM_SIZE = 0;     // default = none
 
+    // Write port for extra RAM is at low address by default
+    static constexpr bool RAM_HIGH_WP = false;
+
   protected:
     /**
       Check hotspots and switch bank if triggered.