Adding two new 8008 homebrew computers#13208
Conversation
cuavas
left a comment
There was a problem hiding this comment.
Please rework the code to take advantage of MAME’s framework, e.g. the memory system.
src/mame/mame.lst
Outdated
| @source:homebrew/sbc8008.cpp | ||
| sbc8008 // Jim Loos - 8008-SBC | ||
|
|
||
| @source:homebrew/super8008.cpp | ||
| super8008 // Dr. Scott M. Baker - 8008-Super | ||
|
|
There was a problem hiding this comment.
Please keep this file sorted. “S” does not come after “Z”.
There was a problem hiding this comment.
These should be in the correct order now.
src/mame/layout/super8008.lay
Outdated
| <group ref="b1_leds"> | ||
| <bounds x="0" y="345" width="10" height="47" /> | ||
| </group> | ||
| <group ref="b2_leds"> | ||
| <bounds x="7" y="345" width="10" height="47" /> | ||
| </group> | ||
| <group ref="b3_leds"> | ||
| <bounds x="14" y="345" width="10" height="47" /> | ||
| </group> | ||
| <group ref="b4_leds"> | ||
| <bounds x="21" y="345" width="10" height="47" /> | ||
| </group> | ||
| <group ref="b5_leds"> | ||
| <bounds x="28" y="345" width="10" height="47" /> | ||
| </group> | ||
| <group ref="b6_leds"> | ||
| <bounds x="35" y="345" width="10" height="47" /> | ||
| </group> | ||
| <group ref="b7_leds"> | ||
| <bounds x="42" y="345" width="10" height="47" /> | ||
| </group> | ||
| <group ref="b8_leds"> | ||
| <bounds x="49" y="345" width="10" height="47" /> | ||
| </group> |
There was a problem hiding this comment.
You could use a <repeat> to make this more terse.
You should be able to use a <repeat> to generate the groups and label elements themselves to reduce copy/paste as well.
There was a problem hiding this comment.
Thanks for the tip on repeat. I've made those changes.
src/mame/layout/sbc8008.lay
Outdated
| <element name="led0" ref="orange_led"> | ||
| <bounds x="32" y="2.5" width="5" height="5" /> | ||
| </element> | ||
| <element name="led1" ref="orange_led"> | ||
| <bounds x="37" y="2.5" width="5" height="5" /> | ||
| </element> | ||
| <element name="led2" ref="orange_led"> | ||
| <bounds x="42" y="2.5" width="5" height="5" /> | ||
| </element> | ||
| <element name="led3" ref="orange_led"> | ||
| <bounds x="47" y="2.5" width="5" height="5" /> | ||
| </element> | ||
| <element name="led4" ref="orange_led"> | ||
| <bounds x="52" y="2.5" width="5" height="5" /> | ||
| </element> | ||
| <element name="led5" ref="orange_led"> | ||
| <bounds x="57" y="2.5" width="5" height="5" /> | ||
| </element> | ||
| <element name="led6" ref="orange_led"> | ||
| <bounds x="62" y="2.5" width="5" height="5" /> | ||
| </element> | ||
| <element name="led7" ref="orange_led"> | ||
| <bounds x="67" y="2.5" width="5" height="5" /> | ||
| </element> |
There was a problem hiding this comment.
You can use a <repeat> to place these LEDs.
src/mame/homebrew/super8008.cpp
Outdated
| #include "cpu/i8008/i8008.h" | ||
| #include "machine/clock.h" | ||
| #include "machine/i8251.h" | ||
| #include "machine/ram.h" | ||
| #include "bus/rs232/rs232.h" | ||
| #include "bus/super8008/super8008.h" |
There was a problem hiding this comment.
Please keep these sorted so they’re easier to keep track of as the list grows.
src/mame/homebrew/super8008.cpp
Outdated
| // This function sets up the machine configuration | ||
| void super8008(machine_config &config); | ||
|
|
||
| protected: | ||
| //TODO(jhe): Do we have another way to get the S0, S1 and S2 so we know when the PC has been halted? | ||
|
|
||
| // address maps for program memory and io memory | ||
| void super8008_mem(address_map &map); | ||
| void super8008_io(address_map &map); |
There was a problem hiding this comment.
Please add ATTR_COLD for cold lifecycle functions.
| void super8008_blaster_device::device_start() | ||
| { | ||
| m_leds.resolve(); | ||
| } |
|
|
||
| RAM(config, m_ram).set_default_size("32K"); |
There was a problem hiding this comment.
You shouldn’t be using ram_device for this – use a memory_share_creator or something.
There was a problem hiding this comment.
A memory_share_creator is being used now.
| void super8008_blaster_device::blaster_mem(address_map &map) | ||
| { | ||
| map(0x0000, 0x3fff).rw( | ||
| FUNC(super8008_blaster_device::blaster_memory_read), | ||
| FUNC(super8008_blaster_device::blaster_memory_write)); | ||
| } |
There was a problem hiding this comment.
Please use a memory_view to deal with swapping the RAM in and out rather than the trampoline functions. You can attach a callback to the jumper input so the view entry can be updated when it changes.
| virtual void device_start() override; | ||
| virtual void device_reset() override; | ||
| virtual void device_post_load() override; |
There was a problem hiding this comment.
Please use ATTR_COLD for cold lifecycle members.
| void super8008_bus_device::device_reset() | ||
| { | ||
| } | ||
|
|
||
| void super8008_bus_device::device_post_load() | ||
| { | ||
| } |
There was a problem hiding this comment.
You don’t need to override these if you aren’t doing anything special.
jeng
left a comment
There was a problem hiding this comment.
Thanks for the review. I think I have all of the changes made.
Also, apologies in advance about any spam from the one-off comments. I had forgotten for a minute how github worked.
src/mame/homebrew/super8008.cpp
Outdated
| void super8008_state::super8008_state::ext_stat_w(offs_t offset, uint8_t data) | ||
| { | ||
| switch(offset) | ||
| { | ||
| case 0: | ||
| //logerror("ext_stat_w %04X take data %02X\n", offset, data); | ||
| take_state = data; | ||
| m_bus->ext_take(data); | ||
| break;//take | ||
| case 1: | ||
| //logerror("ext_stat_w %04X int data %02X\n", offset, data); | ||
| m_bus->ext_int(); | ||
| break;//int | ||
| case 2: | ||
| //logerror("ext_stat_w %04X req data %02X\n", offset, data); | ||
| m_bus->ext_req(); | ||
| break;//req | ||
| case 3: | ||
| //logerror("ext_stat_w %04X reset data %02X\n", offset, data); | ||
| m_bus->ext_reset(); | ||
| break;//reset | ||
|
|
||
| default: | ||
| logerror("ext_stat_w INVALID %04X data %02X\n", offset, data);break; | ||
| } | ||
| } |
There was a problem hiding this comment.
These have been moved to the address map.
src/mame/homebrew/super8008.cpp
Outdated
| uint8_t index = offset & MMAP_MASK; | ||
| mmap[index] = data & 0xff; | ||
| //logerror("super-8008 memory mapper write ($%04X) masked ($%04X) data %04X \n", offset, index, mmap[index]); |
There was a problem hiding this comment.
Good point about masking the uint8_t. I've removed that. I wasn't really sure how to use mirror in a way that simulates a 4x4 register.
src/mame/homebrew/super8008.cpp
Outdated
| //and bit 3 is used for issuing an external chip select. | ||
| map(0x0C, 0x0F).w(FUNC(super8008_state::memory_mapper_w)); |
| void super8008_bus_device::device_reset() | ||
| { | ||
| } | ||
|
|
||
| void super8008_bus_device::device_post_load() | ||
| { | ||
| } |
| virtual void device_start() override; | ||
| virtual void device_reset() override; | ||
| virtual void device_post_load() override; |
src/devices/cpu/i8008/i8008.cpp
Outdated
| for (int addrnum = 0; addrnum < 8; addrnum++) | ||
| state_add(I8008_ADDR1 + addrnum, string_format("ADDR%d", addrnum + 1).c_str(), m_ADDR[addrnum].w.l).mask(0xfff); | ||
|
|
||
| state_add(I8008_STOPPED, "STOPPED", m_HALT); |
src/mame/homebrew/sbc8008.cpp
Outdated
| // This function sets up the machine configuration | ||
| void sbc8008(machine_config &config); | ||
|
|
||
| protected: | ||
| // address maps for program memory and io memory | ||
| void sbc8008_mem(address_map &map); | ||
| void sbc8008_io(address_map &map); | ||
|
|
||
| required_device<cpu_device> m_maincpu; | ||
| required_device<ram_device> m_ram; | ||
| required_memory_region m_rom; | ||
| required_memory_bank m_rom_bank; | ||
| required_device<rs232_port_device> m_rs232; | ||
| output_finder<8> m_leds; | ||
| output_finder<> m_run_led; | ||
| output_finder<> m_txd_led; | ||
| output_finder<> m_rxd_led; | ||
|
|
||
| uint8_t bitbang_read(); | ||
| void bitbang_write(uint8_t data); | ||
| uint8_t port_1_read(); | ||
| void port_9_write(uint8_t data); | ||
| void port_10_write(uint8_t data); | ||
|
|
||
| virtual void machine_start() override; | ||
|
|
||
| bool start = true; | ||
|
|
||
| uint8_t memory_read(offs_t offset); | ||
| void memory_write(offs_t offset, uint8_t data); |
src/mame/homebrew/sbc8008.cpp
Outdated
| uint8_t sbc8008_state::memory_read(offs_t offset) | ||
| { | ||
| if (start && 0 <= offset && offset < SBC8008_ROM_SIZE) | ||
| { | ||
| return ((uint8_t*)m_rom_bank->base())[offset]; | ||
| } | ||
| else if (0 <= offset && offset < m_ram->size()) | ||
| { | ||
| return m_ram->pointer()[offset]; | ||
| } | ||
| else | ||
| { | ||
| logerror("%s:ld invalid mode read ($%02X) start %d size %d\n", | ||
| machine().describe_context(), offset, start, m_ram->size()); | ||
| return 0xff; | ||
| } | ||
| } |
src/mame/homebrew/sbc8008.cpp
Outdated
| void sbc8008_state::memory_write(offs_t offset, uint8_t data) | ||
| { | ||
| if (0 <= offset && offset < m_ram->size()) | ||
| { | ||
| m_ram->pointer()[offset] = data; | ||
| } | ||
| } |
There was a problem hiding this comment.
The custom memory_write has been removed
| uint8_t sbc8008_state::port_1_read() | ||
| { | ||
| start = false; | ||
| return (uint8_t)start; //Is this value used in the monitor? | ||
| } |
There was a problem hiding this comment.
port_1_read is using the memory_view now to avoid side effects.
The first driver is for the 8008-SBC written by Jim Loos.
The second one is for the 8008-Super written by Dr. Scott Baker.
I have uploaded the ROMS to https://github.com/jeng/homebrew-8008-mame-roms
A writeup and demos of the drivers are at Mame Master Blaster