Updated FE scheme to that described by the original FE patent application.

- This emulation is much simpler, and takes the scheme from being
esoteric to being a simple hotspot-based scheme
- The original patent application for the FE scheme describes in perfect
detail what is happening, and although the previous code worked, it
was complex and actually hid what was really happening.

Now that the scheme is hotspot-based, the debugger can be extended to
work better with it.  That part comes next.

src/emucore/CartFE.cxx

@@ -22,26 +22,21 @@
 CartridgeFE::CartridgeFE(const BytePtr& image, uInt32 size,
                          const Settings& settings)
   : Cartridge(settings),
-    myLastAddress1(0),
+    myBankOffset(0),
-    myLastAddress2(0),
+    myLastAccessWasFE(false)
-    myLastAddressChanged(false)
 {
   // Copy the ROM image into my buffer
   memcpy(myImage, image.get(), std::min(8192u, size));
-
-  // We use System::PageAccess.codeAccessBase, but don't allow its use
-  // through a pointer, since the address space of FE carts can change
-  // at the instruction level, and PageAccess is normally defined at an
-  // interval of 64 bytes
-  //
-  // Instead, access will be through the getAccessFlags and setAccessFlags
-  // methods below
   createCodeAccessBase(8192);
+
+  myStartBank = 0;  // For now, we assume this; more research is needed
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void CartridgeFE::reset()
 {
+  myBankOffset = myStartBank << 12;
+  myLastAccessWasFE = false;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -49,8 +44,14 @@ void CartridgeFE::install(System& system)
 {
   mySystem = &system;
 
-  // Map all of the accesses to call peek and poke
+  // The hotspot $01FE is in a mirror of zero-page RAM
-  System::PageAccess access(this, System::PA_READ);
+  // We need to claim access to it here, and deal with it in peek/poke below
+  System::PageAccess access(this, System::PA_READWRITE);
+  for(uInt32 i = 0x180; i < 0x200; i += (1 << System::PAGE_SHIFT))
+    mySystem->setPageAccess(i >> System::PAGE_SHIFT, access);
+
+  // Map all of the cart accesses to call peek and poke
+  access.type = System::PA_READ;
   for(uInt32 i = 0x1000; i < 0x2000; i += (1 << System::PAGE_SHIFT))
     mySystem->setPageAccess(i >> System::PAGE_SHIFT, access);
 }
@@ -58,41 +59,55 @@ void CartridgeFE::install(System& system)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 uInt8 CartridgeFE::peek(uInt16 address)
 {
-  // The bank is determined by A13 of the processor
+  uInt8 value = 0;
-  // We keep track of the two most recent accesses to determine which bank
-  // we're in, and when the values actually changed
-  myLastAddress2 = myLastAddress1;
-  myLastAddress1 = address;
-  myLastAddressChanged = true;
 
-  return myImage[(address & 0x0FFF) + (((address & 0x2000) == 0) ? 4096 : 0)];
+  if(address < 0x200)
+    value = mySystem->m6532().peek(address);
+  else
+    value = myImage[myBankOffset + (address & 0x0FFF)];
+
+  // Check if we hit hotspot
+  checkBankSwitch(address, value);
+
+  return value;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool CartridgeFE::poke(uInt16, uInt8)
+bool CartridgeFE::poke(uInt16 address, uInt8 value)
 {
+  if(address < 0x200)
+    mySystem->m6532().poke(address, value);
+
+  // Check if we hit hotspot
+  checkBankSwitch(address, value);
+
   return false;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-uInt8 CartridgeFE::getAccessFlags(uInt16 address) const
+void CartridgeFE::checkBankSwitch(uInt16 address, uInt8 value)
 {
-  return myCodeAccessBase[(address & 0x0FFF) +
+  if(bankLocked())
-            (((address & 0x2000) == 0) ? 4096 : 0)];
+    return;
+
+  // Did we detect $01FE on the last address bus access?
+  // If so, we bankswitch according to the upper 3 bits of the data bus
+  // NOTE: see the header file for the significance of 'value & 0x20'
+  if(myLastAccessWasFE)
+  {
+    myBankOffset = ((value & 0x20) ? 0 : 1) << 12;
+    myBankChanged = true;
   }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  // On the next cycle, we use the (then) current data bus value to decode
-void CartridgeFE::setAccessFlags(uInt16 address, uInt8 flags)
+  // the bank to use
-{
+  myLastAccessWasFE = address == 0x01FE;
-  myCodeAccessBase[(address & 0x0FFF) +
-      (((address & 0x2000) == 0) ? 4096 : 0)] |= flags;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 uInt16 CartridgeFE::getBank() const
 {
-  // The current bank depends on the last address accessed
+  return myBankOffset >> 12;
-  return ((myLastAddress1 & 0x2000) == 0) ? 1 : 0;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -101,27 +116,10 @@ uInt16 CartridgeFE::bankCount() const
   return 2;
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool CartridgeFE::bankChanged()
-{
-  if(myLastAddressChanged)
-  {
-    // A bankswitch occurs when the addresses transition from state to another
-    myBankChanged = ((myLastAddress1 & 0x2000) == 0) !=
-                    ((myLastAddress2 & 0x2000) == 0);
-    myLastAddressChanged = false;
-  }
-  else
-    myBankChanged = false;
-
-  // In any event, let the base class know about it
-  return Cartridge::bankChanged();
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool CartridgeFE::patch(uInt16 address, uInt8 value)
 {
-  myImage[(address & 0x0FFF) + (((address & 0x2000) == 0) ? 4096 : 0)] = value;
+  myImage[myBankOffset + (address & 0x0FFF)] = value;
   return myBankChanged = true;
 }
 
@@ -138,8 +136,8 @@ bool CartridgeFE::save(Serializer& out) const
   try
   {
     out.putString(name());
-    out.putShort(myLastAddress1);
+    out.putShort(myBankOffset);
-    out.putShort(myLastAddress2);
+    out.putBool(myLastAccessWasFE);
   }
   catch(...)
   {
@@ -158,8 +156,8 @@ bool CartridgeFE::load(Serializer& in)
     if(in.getString() != name())
       return false;
 
-    myLastAddress1 = in.getShort();
+    myBankOffset = in.getShort();
-    myLastAddress2 = in.getShort();
+    myLastAccessWasFE = in.getBool();
   }
   catch(...)
   {

src/emucore/CartFE.hxx

@@ -29,23 +29,50 @@ class System;
 /**
   Bankswitching method used by Activision's Robot Tank and Decathlon.
 
-  Kevin Horton describes FE as follows:
+  This scheme was originally designed to have up to 8 4K banks, and is
+  triggered by monitoring the address bus for address $01FE.  All released
+  carts only had two banks, and this implementation assumes that (ie, ROM
+  is always 8K, and there are two 4K banks).
 
-    Used only on two carts (Robot Tank and Decathlon).  These
+  The following is paraphrased from the original patent by David Crane,
-    carts are very weird.  It does not use accesses to the stack
+  European Patent Application # 84300730.3, dated 06.02.84:
-    like was previously thought.  Instead, if you watch the called
-    addresses very carefully, you can see that they are either Dxxx
-    or Fxxx.  This determines the bank to use.  Just monitor A13 of
-    the processor and use it to determine your bank! :-)  Of course
-    the 6507 in the 2600 does not have an A13, so the cart must have
-    an extra bit in the ROM matrix to tell when to switch banks.
-    There is *no* way to determine which bank you want to be in from
-    monitoring the bus.
 
-  This cart reports having 2 banks, even though this cannot be
+  ---------------------------------------------------------------------------
-  determined on a real system.
+  The twelve line address bus is connected to a plurality of 4K by eight bit
+  memories.
 
-  @author  Bradford W. Mott
+  The eight line data bus is connected to each of the banks of memory, also.
+  An address comparator is connected to the bus for detecting the presence of
+  the 01FE address.  Actually, the comparator will detect only the lowest 12
+  bits of 1FE, because of the twelve bit limitation of the address bus.  Upon
+  detection of the 01FE address, a one cycle delay is activated which then
+  actuates latch connected to the data bus.  The three most significant bits
+  on the data bus are latched and provide the address bits A13, A14, and A15
+  which are then applied to a 3 to 8 de-multiplexer.  The 3 bits A13-A15
+  define a code for selecting one of the eight banks of memory which is used
+  to enable one of the banks of memory by applying a control signal to the
+  enable, EN, terminal thereof.  Accordingly, memory bank selection is
+  accomplished from address codes on the data bus following a particular
+  program instruction, such as a jump to subroutine.
+  ---------------------------------------------------------------------------
+
+  Note that in the general scheme, we use D7, D6 and D5 for the bank number
+  (3 bits, so 8 possible banks).  However, the scheme as used historically
+  by Activision only uses two banks.  Furthermore, the two banks it uses
+  are actually indicated by binary 110 and 111, and translated as follows:
+
+    binary 110 -> decimal 6 -> Upper 4K ROM (bank 1) @ $D000
+    binary 111 -> decimal 7 -> Lower 4K ROM (bank 0) @ $F000
+
+  Since the actual bank numbers (0 and 1) do not map directly to their
+  respective bitstrings (7 and 6), we simply test for D6 being 0 or 1.
+  This is the significance of the test '(value & 0x20) ? 0 : 1' in the code.
+
+  NOTE: Consult the patent application for more specific information, in
+        particular *why* the address $01FE will be placed on the address
+        bus after both the JSR and RTS opcodes.
+
+  @author  Stephen Anthony; with ideas from Christian Speckner and TomSon
 */
 class CartridgeFE : public Cartridge
 {
@@ -86,14 +113,6 @@ class CartridgeFE : public Cartridge
     */
     uInt16 bankCount() const override;
 
-    /**
-      Answer whether the bank has changed since the last time this
-      method was called.
-
-      @return  Whether the bank was changed
-    */
-    bool bankChanged() override;
-
     /**
       Patch the cartridge ROM.
 
@@ -155,7 +174,7 @@ class CartridgeFE : public Cartridge
     uInt8 peek(uInt16 address) override;
 
     /**
-      Change the byte at the specified address to the given value
+      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
@@ -165,23 +184,20 @@ class CartridgeFE : public Cartridge
 
   private:
     /**
-      Query/change the given address type to use the given disassembly flags
+      Perform bankswitch when necessary, by monitoring for $01FE
-
+      on the address bus and getting the bank number from the data bus.
-      @param address The address to modify
-      @param flags A bitfield of DisasmType directives for the given address
     */
-    uInt8 getAccessFlags(uInt16 address) const override;
+    void checkBankSwitch(uInt16 address, uInt8 value);
-    void setAccessFlags(uInt16 address, uInt8 flags) override;
 
   private:
     // The 8K ROM image of the cartridge
     uInt8 myImage[8192];
 
-    // Previous two addresses accessed by peek()
+    // Indicates the offset into the ROM image (aligns to current bank)
-    uInt16 myLastAddress1, myLastAddress2;
+    uInt16 myBankOffset;
 
-    // Last two addresses have been modified by peek()
+    // Whether previous address by peek/poke equals $01FE (hotspot)
-    bool myLastAddressChanged;
+    bool myLastAccessWasFE;
 
   private:
     // Following constructors and assignment operators not supported

src/emucore/M6502.cxx

@@ -49,7 +49,6 @@ M6502::M6502(const Settings& settings)
     mySettings(settings),
     A(0), X(0), Y(0), SP(0), IR(0), PC(0),
     N(false), V(false), B(false), D(false), I(false), notZ(false), C(false),
-    myLastAccessWasRead(true),
     myNumberOfDistinctAccesses(0),
     myLastAddress(0),
     myLastPeekAddress(0),
@@ -94,9 +93,6 @@ void M6502::reset()
   PS(BSPF::containsIgnoreCase(cpurandom, "P") ?
           mySystem->randGenerator().next() : 0x20);
 
-  // Reset access flag
-  myLastAccessWasRead = true;
-
   // Load PC from the reset vector
   PC = uInt16(mySystem->peek(0xfffc)) | (uInt16(mySystem->peek(0xfffd)) << 8);
 
@@ -133,7 +129,6 @@ inline uInt8 M6502::peek(uInt16 address, uInt8 flags)
 #endif  // DEBUGGER_SUPPORT
 
   uInt8 result = mySystem->peek(address, flags);
-  myLastAccessWasRead = true;
   myLastPeekAddress = address;
   return result;
 }
@@ -161,7 +156,6 @@ inline void M6502::poke(uInt16 address, uInt8 value)
 #endif  // DEBUGGER_SUPPORT
 
   mySystem->poke(address, value);
-  myLastAccessWasRead = false;
   myLastPokeAddress = address;
 }
 

src/emucore/M6502.hxx

@@ -136,13 +136,6 @@ class M6502 : public Serializable
     */
     uInt16 getPC() const { return PC; }
 
-    /**
-      Answer true iff the last memory access was a read.
-
-      @return true iff last access was a read
-    */
-    bool lastAccessWasRead() const { return myLastAccessWasRead; }
-
     /**
       Return the last address that was part of a read/peek.  Note that
       reads which are part of a write are not considered here, unless
@@ -327,9 +320,6 @@ class M6502 : public Serializable
     bool notZ;  // Z flag complement for processor status register
     bool C;     // C flag for processor status register
 
-    /// Indicates if the last memory access was a read or not
-    bool myLastAccessWasRead;
-
     /// Indicates the numer of distinct memory accesses
     uInt32 myNumberOfDistinctAccesses;