From de7218cdf09ebcef1c93fdbd0c2aee51d8618233 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 20 Jan 2016 21:55:38 -0500 Subject: [PATCH] Added a lot of commenting to the CPU6502 definition, simplifying its construction. Added missing nullability modifier to CSElectron. Fixed bad-habit Objective-C style naming on the Electron's Interrupt enum. --- Machines/Atari2600/Atari2600.cpp | 1 - Machines/Electron/Electron.cpp | 21 ++- Machines/Electron/Electron.hpp | 17 ++- .../Mac/Clock Signal/Wrappers/CSElectron.h | 2 +- Processors/6502/CPU6502.hpp | 132 ++++++++++++++---- 5 files changed, 124 insertions(+), 49 deletions(-) diff --git a/Machines/Atari2600/Atari2600.cpp b/Machines/Atari2600/Atari2600.cpp index 5d9a5784b..543e7a406 100644 --- a/Machines/Atari2600/Atari2600.cpp +++ b/Machines/Atari2600/Atari2600.cpp @@ -26,7 +26,6 @@ Machine::Machine() : { _crt = new Outputs::CRT(228, 262, 1, 2); memset(_collisions, 0xff, sizeof(_collisions)); - setup6502(); set_reset_line(true); } diff --git a/Machines/Electron/Electron.cpp b/Machines/Electron/Electron.cpp index 1e8c60b33..c26609cfa 100644 --- a/Machines/Electron/Electron.cpp +++ b/Machines/Electron/Electron.cpp @@ -34,7 +34,6 @@ Machine::Machine() : memset(_roms[c], 0xff, 16384); _speaker.set_input_rate(125000); - setup6502(); } Machine::~Machine() @@ -104,7 +103,7 @@ unsigned int Machine::perform_bus_operation(CPU6502::BusOperation operation, uin if(isReadOperation(operation)) { *value = (uint8_t)(_tape.dataRegister >> 2); - _interruptStatus &= ~InterruptTransmitDataEmpty; + _interruptStatus &= ~Interrupt::TransmitDataEmpty; evaluate_interrupts(); } else @@ -118,9 +117,9 @@ unsigned int Machine::perform_bus_operation(CPU6502::BusOperation operation, uin const uint8_t interruptDisable = (*value)&0xf0; if( interruptDisable ) { - if( interruptDisable&0x10 ) _interruptStatus &= ~InterruptDisplayEnd; - if( interruptDisable&0x20 ) _interruptStatus &= ~InterruptRealTimeClock; - if( interruptDisable&0x40 ) _interruptStatus &= ~InterruptHighToneDetect; + if( interruptDisable&0x10 ) _interruptStatus &= ~Interrupt::DisplayEnd; + if( interruptDisable&0x20 ) _interruptStatus &= ~Interrupt::RealTimeClock; + if( interruptDisable&0x40 ) _interruptStatus &= ~Interrupt::HighToneDetect; evaluate_interrupts(); // TODO: NMI (?) } @@ -265,12 +264,12 @@ unsigned int Machine::perform_bus_operation(CPU6502::BusOperation operation, uin case 128*128: update_audio(); - signal_interrupt(InterruptRealTimeClock); + signal_interrupt(Interrupt::RealTimeClock); break; case 284*128: update_audio(); - signal_interrupt(InterruptDisplayEnd); + signal_interrupt(Interrupt::DisplayEnd); break; case cycles_per_frame: @@ -347,21 +346,21 @@ inline void Machine::push_tape_bit(uint16_t bit) if(_tape.bits_since_start == 7) { - _interruptStatus &= ~InterruptTransmitDataEmpty; + _interruptStatus &= ~Interrupt::TransmitDataEmpty; } } else { if((_tape.dataRegister&0x3) == 0x1) { - _interruptStatus |= InterruptTransmitDataEmpty; + _interruptStatus |= Interrupt::TransmitDataEmpty; _tape.bits_since_start = 9; } if(_tape.dataRegister == 0x3ff) - _interruptStatus |= InterruptHighToneDetect; + _interruptStatus |= Interrupt::HighToneDetect; else - _interruptStatus &= ~InterruptHighToneDetect; + _interruptStatus &= ~Interrupt::HighToneDetect; } // printf("."); diff --git a/Machines/Electron/Electron.hpp b/Machines/Electron/Electron.hpp index 7913dca92..55b9cc375 100644 --- a/Machines/Electron/Electron.hpp +++ b/Machines/Electron/Electron.hpp @@ -14,7 +14,6 @@ #include "../../Outputs/Speaker.hpp" #include "../../Storage/Tape/Tape.hpp" #include -#include "Atari2600Inputs.h" namespace Electron { @@ -32,11 +31,11 @@ enum ROMSlot: uint8_t { }; enum Interrupt: uint8_t { - InterruptDisplayEnd = 0x04, - InterruptRealTimeClock = 0x08, - InterruptTransmitDataEmpty = 0x10, - InterruptReceiveDataFull = 0x20, - InterruptHighToneDetect = 0x40 + DisplayEnd = 0x04, + RealTimeClock = 0x08, + TransmitDataEmpty = 0x10, + ReceiveDataFull = 0x20, + HighToneDetect = 0x40 }; enum Key: uint16_t { @@ -58,6 +57,12 @@ enum Key: uint16_t { KeyBreak = 0xffff }; +/*! + @abstract Represents an Acorn Electron. + + @discussion An instance of Electron::Machine represents the current state of an + Acorn Electron. +*/ class Machine: public CPU6502::Processor { public: diff --git a/OSBindings/Mac/Clock Signal/Wrappers/CSElectron.h b/OSBindings/Mac/Clock Signal/Wrappers/CSElectron.h index 474c6f726..ca21d949d 100644 --- a/OSBindings/Mac/Clock Signal/Wrappers/CSElectron.h +++ b/OSBindings/Mac/Clock Signal/Wrappers/CSElectron.h @@ -14,7 +14,7 @@ - (void)setOSROM:(nonnull NSData *)rom; - (void)setBASICROM:(nonnull NSData *)rom; - (void)setROM:(nonnull NSData *)rom slot:(int)slot; -- (BOOL)openUEFAtURL:(NSURL *)URL; +- (BOOL)openUEFAtURL:(nonnull NSURL *)URL; - (void)setKey:(uint16_t)key isPressed:(BOOL)isPressed; diff --git a/Processors/6502/CPU6502.hpp b/Processors/6502/CPU6502.hpp index b263063c3..8bce31e26 100644 --- a/Processors/6502/CPU6502.hpp +++ b/Processors/6502/CPU6502.hpp @@ -25,7 +25,6 @@ enum Register { S }; - enum Flag { Sign = 0x80, Overflow = 0x40, @@ -41,10 +40,24 @@ enum BusOperation { Read, ReadOpcode, Write, Ready, None }; +/*! + Evaluates to `true` if the operation is a read; `false` if it is a write. +*/ #define isReadOperation(v) (v == CPU6502::BusOperation::Read || v == CPU6502::BusOperation::ReadOpcode) +/*! + An opcode that is guaranteed to cause the CPU to jam. +*/ extern const uint8_t JamOpcode; +/*! + @abstact An abstract base class for emulation of a 6502 processor. + + @discussion Subclasses should implement @c perform_bus_operation(BusOperation operation, uint16_t address, uint8_t *value) in + order to provde the bus on which the 6502 operates. Additional functionality can be provided by the host machine by providing + a jam handler and inserting jam opcodes where appropriate; that will cause call outs when the program counter reaches those + addresses. @c return_from_subroutine can be used to exit from a jammed state. +*/ template class Processor { public: @@ -384,18 +397,6 @@ template class Processor { bool _nmi_line_is_enabled; bool _ready_is_active; - public: - Processor() : - _scheduleProgramsReadPointer(0), - _scheduleProgramsWritePointer(0), - _is_jammed(false), - _jam_handler(nullptr), - _cycles_left_to_run(0), - _ready_line_is_enabled(false), - _ready_is_active(false), - _scheduledPrograms{nullptr, nullptr, nullptr, nullptr} - {} - const MicroOp *get_reset_program() { static const MicroOp reset[] = { CycleFetchOperand, @@ -423,6 +424,41 @@ template class Processor { return reset; } + public: + Processor() : + _scheduleProgramsReadPointer(0), + _scheduleProgramsWritePointer(0), + _is_jammed(false), + _jam_handler(nullptr), + _cycles_left_to_run(0), + _ready_line_is_enabled(false), + _ready_is_active(false), + _scheduledPrograms{nullptr, nullptr, nullptr, nullptr}, + _interruptFlag(Flag::Interrupt), + _s(0), + _nextBusOperation(BusOperation::None) + + { + // only the interrupt flag is defined upon reset but get_flags isn't going to + // mask the other flags so we need to do that, at least + _carryFlag &= Flag::Carry; + _decimalFlag &= Flag::Decimal; + _overflowFlag &= Flag::Overflow; + + // TODO: is this accurate? It feels more likely that a CPU would need to wait + // on an explicit reset command, since the relative startup times of different + // components from power on would be a bit unpredictable. + schedule_program(get_reset_program()); + } + + /*! + Runs the 6502 for a supplied number of cycles. + + @discussion Subclasses must implement @c perform_bus_operation(BusOperation operation, uint16_t address, uint8_t *value) . + The 6502 will call that method for all bus accesses. The 6502 is guaranteed to perform one bus operation call per cycle. + + @param number_of_cycles The number of cycles to run the 6502 for. + */ void run_for_cycles(int number_of_cycles) { static const MicroOp doBranch[] = { @@ -932,6 +968,14 @@ template class Processor { } } + /*! + Gets the value of a register. + + @see set_value_of_register + + @param r The register to set. + @returns The value of the register. 8-bit registers will be returned as unsigned. + */ uint16_t get_value_of_register(Register r) { switch (r) { @@ -947,6 +991,14 @@ template class Processor { } } + /*! + Sets the value of a register. + + @see get_value_of_register + + @param r The register to set. + @param value The value to set. If the register is only 8 bit, the value will be truncated. + */ void set_value_of_register(Register r, uint16_t value) { switch (r) { @@ -961,23 +1013,10 @@ template class Processor { } } - void setup6502() - { - // only the interrupt flag is defined upon reset but get_flags isn't going to - // mask the other flags so we need to do that, at least - _interruptFlag = Flag::Interrupt; - _carryFlag &= Flag::Carry; - _decimalFlag &= Flag::Decimal; - _overflowFlag &= Flag::Overflow; - _s = 0; - _nextBusOperation = BusOperation::None; - - // TODO: is this accurate? It feels more likely that a CPU would need to wait - // on an explicit reset command, since the relative startup times of different - // components from power on would be a bit unpredictable. - schedule_program(get_reset_program()); - } - + /*! + Interrupts current execution flow to perform an RTS and, if the 6502 is currently jammed, + to unjam it. + */ void return_from_subroutine() { _s++; @@ -990,6 +1029,11 @@ template class Processor { } } + /*! + Sets the current level of the RDY line. + + @param active @c true if the line is logically active; @c false otherwise. + */ void set_ready_line(bool active) { if(active) @@ -1001,26 +1045,54 @@ template class Processor { } } + /*! + Sets the current level of the RST line. + + @param active @c true if the line is logically active; @c false otherwise. + */ void set_reset_line(bool active) { _reset_line_is_enabled = active; } + /*! + Sets the current level of the IRQ line. + + @param active @c true if the line is logically active; @c false otherwise. + */ void set_irq_line(bool active) { _irq_line_is_enabled = active; } + /*! + Sets the current level of the NMI line. + + @param active `true` if the line is logically active; `false` otherwise. + */ void set_nmi_line(bool active) { + // TODO: NMI is edge triggered, not level, and in any case _nmi_line_is_enabled + // is not honoured elsewhere. So NMI is yet to be implemented. _nmi_line_is_enabled = active; } + /*! + Queries whether the 6502 is now 'jammed'; i.e. has entered an invalid state + such that it will not of itself perform any more meaningful processing. + + @returns @c true if the 6502 is jammed; @c false otherwise. + */ bool is_jammed() { return _is_jammed; } + /*! + Installs a jam handler. Jam handlers are notified if a running 6502 jams. + + @param handler The class instance that will be this 6502's jam handler from now on. + */ void set_jam_handler(JamHandler *handler) { _jam_handler = handler;