Tek4404 work_in_progress. #12574

Elektraglide · 2024-07-14T07:53:09Z

This is a WIP getting Tek4404 to run.
It now passes all the h/w tests apart from mouse. ( RAM, MMU, Serial, Timers, Sound, Video, Storage Devices)
A boot manages to load the OS from "system.boot", starts up and tries to write some swap data. After writing 512 bytes, it never receives a STATUS ACK and just spins forever running the idle job and checking whether the write has completed.

It implements a derived ncr5385 that drives an LED for disk activity

I do not have the knowledge around scsi and new_scsi_bus to understand what the emulated devices are not doing that Tek4404 expects, so this is where I have got to. Very close but no cigar.

rb6502 · 2024-07-14T16:23:51Z

Can you fix the conflicts with the already-merged ncr5385 changes?

Elektraglide · 2024-07-14T16:34:49Z

DATA IN finishes with a STATUS REQ shown below

[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x029 phase DATA IN REQ
[:scsi:7:led_ncr5385] aux_status_r 0x88 (':maincpu' (747ADC))
[:scsi:7:led_ncr5385] xfi_in: data 0x75
[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x049 phase DATA IN ACK
[:scsi:7:led_ncr5385] xfi_in: 0 remaining
[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x00b phase STATUS
[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x02b phase STATUS REQ
[:scsi:7:led_ncr5385] xfi_in: transfer complete
[:scsi:7:led_ncr5385] update_int 1
[:scsi:7:led_ncr5385] update_int 1  pc((no context))
[:scsi:7:led_ncr5385] aux_status_r 0x1a (':maincpu' (747E12))			<=  AUX_STATUS_CD | AUX_STATUS_IO | AUX_STATUS_PAUSED??
[:scsi:7:led_ncr5385] int_status_r 0x02 (':maincpu' (747E18))
[:scsi:7:led_ncr5385] update_int 0
[:scsi:7:led_ncr5385] update_int 0  pc(':maincpu' (747E18))
[:scsi:7:led_ncr5385] cmd_w 0x54 (':maincpu' (747964))
[:scsi:7:led_ncr5385] transfer info (pio, single byte)
[:scsi:7:led_ncr5385] aux_status_r 0x9a (':maincpu' (747CD0))
[:scsi:7:led_ncr5385] xfi_in: data 0x00
[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x04b phase STATUS ACK

DATA OUT finishes with a STATUS REQ shown below and never gets a STATUS ACK

[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x028 phase DATA OUT REQ
[:scsi:7:led_ncr5385] aux_status_r 0x00 (':maincpu' (01258C))
[:scsi:7:led_ncr5385] dat_w 0x00 (':maincpu' (01259C))
[:scsi:7:led_ncr5385] xfi_out: data 0x00
[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x048 phase DATA OUT ACK
[:scsi:7:led_ncr5385] xfi_out: 0 remaining
[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x00b phase STATUS
[:scsi:7:led_ncr5385] scsi_ctrl_changed 0x02b phase STATUS REQ
[:scsi:7:led_ncr5385] xfi_out: transfer complete
[:scsi:7:led_ncr5385] update_int 1
[:scsi:7:led_ncr5385] update_int 1  pc((no context))
[:scsi:7:led_ncr5385] aux_status_r 0x1a (':maincpu' (012844))			<= why is the same as for a READ?
[:scsi:7:led_ncr5385] int_status_r 0x02 (':maincpu' (01284E))
[:scsi:7:led_ncr5385] update_int 0
[:scsi:7:led_ncr5385] update_int 0  pc(':maincpu' (01284E))
[:scsi:7:led_ncr5385] aux_status_r 0x1a (':maincpu' (012866))

pmackinlay

There are a few stray blank lines, some possible accidental whitespace changes, and some instances of inconsistent formatting that should be cleaned up prior to merging, but since this is a WIP asking for assistance, I haven't tried to nitpick those here.

pmackinlay · 2024-07-15T09:40:34Z

src/mame/tektronix/tek440x.cpp

+		m_screen(*this, "screen"),
+		m_acia(*this, "acia"),
+		m_printer(*this, "printer"),
+		m_prom(*this, "maincpu"),			// FIXME why is the bootrom called 'maincpu'?


The region can be called anything, it just needs to match the name given in the ROM_REGION macro, and the parameter to the .region() function in the memory map.

pmackinlay · 2024-07-15T09:44:01Z

src/mame/tektronix/tek440x.cpp

+		m_led_1(*this, "led_1"),
+		m_led_2(*this, "led_2"),
+		m_led_4(*this, "led_4"),
+		m_led_8(*this, "led_8"),


It's better to combine these into a single output_finder<4>, and use the m_led(*this, "led_%u", 0U) syntax here, and m_led[N] = ...; elsewhere.

pmackinlay · 2024-07-15T09:45:33Z

src/mame/tektronix/tek440x.cpp

+	m_led_8.resolve();
+
+	m_led_disk.resolve();
+	m_maincpu->space(AS_PROGRAM).install_write_tap(0x7be002, 0x7be003, "led_tap", [this](offs_t offset, u16 &data, u16 mem_mask) { m_led_disk = !BIT(data, 3);});


Since the tap is 16 bits wide and the ncr5385 is hooked up to the high byte on the bus, you should probably be testing bit 11 instead of bit 3 here.

pmackinlay · 2024-07-15T09:48:50Z

src/mame/tektronix/tek440x.cpp

+	}
+}
+
+u8 tek440x_state::rtc_r(offs_t offset)


If it's really an mc146818, then it's already in the machine config should be added to the memory map and these stubs won't be required.

pmackinlay · 2024-07-15T10:00:37Z

src/devices/machine/ncr5385.cpp

@@ -245,6 +246,9 @@ u8 ncr5385_device::aux_status_r()

 	if (!m_int_status)
 	{
+		// AB mask out any bits
+		data &= ~(AUX_STATUS_MSG | AUX_STATUS_CD | AUX_STATUS_IO);


I don't think this should be necessary; resetting and latching of these bits should be properly controlled by changes to the interrupt status. Did you find a bug in the logic?

pmackinlay · 2024-07-15T10:06:58Z

src/devices/machine/ncr5385.cpp

 			// clear data and ACK
 			scsi_bus->data_w(scsi_refid, 0);
 			scsi_bus->ctrl_w(scsi_refid, 0, S_ACK);
+#else
+			// clear ACK except after last byte of message output phase


I don't think this is correct; this logic only applies for the message input phase, to allow the initiator the chance to raise ATN to reject a message from the target if necessary. The same thing does not apply during message output.

cuavas · 2024-10-08T13:53:24Z

src/mame/tektronix/tek440x.cpp

-
+				* 8255 Centronics printer interface
+


The indentation is way off here, and the blank line has trailing spaces.

cuavas · 2024-10-08T13:54:27Z

src/mame/tektronix/tek440x.cpp

 #include "machine/ncr5385.h"
 #include "machine/ns32081.h"
+#include "machine/i8255.h"
 #include "machine/nscsi_bus.h"


Please keep the #include statements for headers from src/devices sorted.

cuavas · 2024-10-08T13:55:12Z

src/mame/tektronix/tek440x.cpp

+// mapcntl bits
+#define MAP_VM_ENABLE 4
+#define MAP_SYS_WR_ENABLE 5
+// mapcntrl result bits
+#define MAP_BLOCK_ACCESS 6
+#define MAP_CPU_WR 7
+
+#define OFF8_TO_OFF16(A)	((A)>>1)
+#define OFF16_TO_OFF8(A)	((A)<<1)
+
+#define MAXRAM 0x200000	// +1MB
+//#define MAXRAM 0x400000	// +3MB (which was never a Thing)


You can use cosntexpr variables and functions for these, there’s no need to use preprocessor macros.

cuavas · 2024-10-08T13:56:06Z

src/mame/tektronix/tek440x.cpp

+class m68010_tekmmu_device : public m68010_device
+{
+	using m68010_device::m68010_device;


You’ll get a validity error if you do this because the device type for the instance won’t match what you instantiated it with.

cuavas · 2024-10-08T13:57:31Z

src/mame/tektronix/tek440x.cpp

+	bool memory_translate(int spacenum, int intention, offs_t &address, address_space *&target_space) override
+	{
+
+	target_space = &space(spacenum);
+
+		//if (m_emmu_enabled)
+		if (spacenum == AS_PROGRAM)
+		if ((get_fc() & 4) == 0)			// only in User mode
+		if (BIT(*m_map_control, MAP_VM_ENABLE))
+		{
+			if (intention == TR_WRITE)
+			if (BIT(m_map[address >> 12], 14) == 0)	// read only
+			{
+				LOG("memory_translate: write fail\n");
+				return false;
+			}
+
+			// dont try and translate a null page
+			if (BIT(m_map[address >> 12], 11, 3) != 0)
+			{
+				LOG("memory_translate: map %08x => paddr(%08x)\n",(address), (BIT(address, 0, 12) | (BIT(m_map[address >> 12], 0, 11) << 12) ) );
+
+				address = BIT(address, 0, 12) | (BIT(m_map[address >> 12], 0, 11) << 12);
+			}
+		}
+
+		return true;
+	}


Indentation is off, and this style is confusing and error-prone. Just you know you can use && in the conditions for if statements.

cuavas · 2024-10-08T13:58:07Z

src/mame/tektronix/tek440x.cpp

+		if (!buserror_occurred)
+		if (*m_map_control & (1 << MAP_BLOCK_ACCESS))
+		if ((get_fc() & 4) == 0)				// only in User mode
+		if (BIT(*m_map_control, MAP_VM_ENABLE))
+		{


Same here – please don’t use this style.

cuavas · 2024-10-08T14:08:31Z

src/mame/tektronix/tek440x.cpp

+u16 PTEread(offs_t address)
+{
+	// selftest does a read and expects it to fail iff !MAP_SYS_WR_ENABLE; its not WR enable, its enable..
+	if (!BIT(*m_map_control, MAP_SYS_WR_ENABLE))
+	{
+			LOG("PTEread: bus error: PID(%d) %08x fc(%d) pc(%08x)\n", BIT(m_map[(address >> 12) & 0x7ff], 11, 3), (address), get_fc(), pc());
+			buserror_occurred = true;
+			set_buserror_details(address, 1, get_fc(), true);
+			return 0;
+	}
+
+	return m_map[(address>>12) & 0x7ff];
+}
+
+void PTEwrite(offs_t address, u16 data)
+{
+	if (((address>>12) & 0x7ff) < 20)
+		LOG("PTEwrite: %08x  <= %04x paddr(%08x) PID(%d) dirty(%d) write_enable(%d)\n",
+		(address>>12) & 0x7ff, data,
+		(BIT(data, 0, 12)<<12), BIT(data, 11, 3), data & 0x8000 ? 1 : 0, data & 0x4000 ? 1 : 0);
+
+	if (BIT(*m_map_control, MAP_SYS_WR_ENABLE))
+	{
+		m_map[(address>>12) & 0x7ff] = data;
+	}
+}


Indentation is off here:

These functions are at the left margin while the other inline member function bodies are indented.

You should use one level of indentation per nested scope level.

Please use braces for any multi-line if/else body even if it’s technically only a single statement.

Please indent statement continuations by two tabs.

cuavas · 2024-10-08T14:12:20Z

src/devices/machine/am9513.cpp

I already explained on one of your previous pull requests why these logging changes are undesirable.

rb6502 · 2024-11-26T14:46:27Z

src/mame/tektronix/tek440x.cpp

@@ -105,7 +105,7 @@ class m68010_tekmmu_device : public m68010_device

 		//if (m_emmu_enabled)
 		if (spacenum == AS_PROGRAM)
-		if ((get_fc() & 4) == 0)			// only in User mode
+		if (!(m_s_flag & 4))			// only in User mode


This can't be right. External hardware like the MMU can't see the 68k's status register. Bit 2 of the FC lines determines user vs. supervisor, like you had it before.

rb6502 · 2024-11-26T15:34:45Z

Is this in a state where we could look at getting it applied as a checkpoint?

Because it's very difficult to review a PR with this much churn I went and looked directly at your source branch, and I have a few notes. I think Vas touched on some of these already but I want to reiterate them.

Use LOGMASKED() so that logging can be controlled by category instead of being all or nothing.
Always use braces with if statements. memory_translate() and similar functions are very hard to track when you're reading them because of the lack of braces.
Because the Tektronix MMU is external to the 680x0 it should not be a superclass of the 68K core. I realize I set a poor example here with the Mac II and LC external MMUs so I won't hold that against merging this right now, but it's something to keep in mind for later. sun3.cpp is a better example of what an external 68K MMU should look like.

And addressing some comments I see in the code:

The boot prom is called "maincpu" because a ROM region with the same tag name as a CPU will inherit that CPU's endianness and bus width properties.
.mirror() is an inverse AND mask on the address. "1" bits in the .mirror() field are ignored when looking up the address.

- some comments in trying to understand how to attach a target scsi device

…sent in m_videocntl

… option

- MAP_SYS_WR_ENABLE eppears to really mean read/write enable - page table needs to be accessible without MAP_VM_ENABLE

- need to find a way of faking a connection so we can get debug output

…lftest mamedev#20 before failing

…tests

- mouse position (gray codes?) seems borked

…lftest now passes but not sure this is right solution - Timer selftest fails; need a way of resetting interrupt when writing to 0x1xx

- hook writes to m_timer 0x1xx and clear IRQ1

- helped by Lord Nightmare; Interval Timer now passes

- added 8255 mapped to 0x7b2000 to handle Centronics interface; not working

…ing something else

…ide the main CRT box.

- testing CMD_INT check is correct

- fixes runaway key repeating on input

…formance

- video panning not right

…tride

- not sure about FCMP / DCMP return values.

- added FGetStat and FSetStat to set ns32081 status register

- comment out log of every memory_r and memory_w

…e many things work - what we actually want is a way of knowing its a prefetch memory_r()

…egular instr fetch.

- unsure if this is a Mac keyboard mapping issue or just wrong

pmackinlay reviewed Jul 15, 2024

View reviewed changes

cuavas reviewed Oct 8, 2024

View reviewed changes

rb6502 reviewed Nov 26, 2024

View reviewed changes

Elektraglide force-pushed the tek4404_working branch from 74ed34d to ad8a862 Compare December 29, 2024 10:49

Elektraglide force-pushed the tek4404_working branch from 520f03c to 598a1c9 Compare January 17, 2025 08:49

Adam added 22 commits January 19, 2025 19:59

- corrected to use MONOCHROME_INVERTED

8528654

- some comments in trying to understand how to attach a target scsi device

- added logging of LEDs on write to 0x740000

ef182a0

- fix for memory sizing tests

5d37447

- removed nomem_r; just need to test memory bounds wrt *word* offset

fc52615

- Display control selftest now passes; no idea what bit 4,5 & 6 repre…

df797db

…sent in m_videocntl

- map_w tests WRenable before writing

8d08cc7

- added a MAXRAM define to me able select +1MB ram option or +3MB ram…

2060c50

… option

- if FC2 do not do address translation; supervisor is pa

9cd9585

- MAP_SYS_WR_ENABLE eppears to really mean read/write enable - page table needs to be accessible without MAP_VM_ENABLE

- added a (required_device) acia device

0081ee7

- need to find a way of faking a connection so we can get debug output

- wip getting MMU map_control bits right; we get all the way to VM se…

c847168

…lftest mamedev#20 before failing

- pmackinlay helped decode SysWrEn logic, now passes Virtual Mem self…

30dbf25

…tests

- fixed mouse button mappings for left,middle,right

b286c73

- mouse position (gray codes?) seems borked

- seems to be relying on timer IRQ

b81924b

- added timer IRQ.. I think...

1cc5e70

- enable duart loopback mode iff m_diag & 0x80. Keyboard Interface se…

2968c1c

…lftest now passes but not sure this is right solution - Timer selftest fails; need a way of resetting interrupt when writing to 0x1xx

- input merger of cnt1 and cnt2 to assert IRQ1

d7025af

- hook writes to m_timer 0x1xx and clear IRQ1

- removed unused experiment

9551b51

- cleaning up more experiments

7d4902b

- helped by Lord Nightmare; Interval Timer now passes

- videoaddr is 16-bit reads/writes

aeebd63

- added 8255 mapped to 0x7b2000 to handle Centronics interface; not working

- made irq1 latch state a member var (m_u244latch)

9e4419d

- added atari ST graycode mouse processing but I think 4404 is expect…

99c07d9

…ing something else

- added pretty LEDs at the bottom of the display that are located ins…

8faaa90

…ide the main CRT box.

Adam added 29 commits January 19, 2025 19:59

- fixed wrong log message for cascading TCs

67dd0dd

- fixing formatting

76c249d

- testing CMD_INT check is correct

- added m68010_tekmmu_device that implements memory_translate()

7c02b48

- restoring SCSI_BUS_SKEW that was changed..

d8590c9

- bad merge

baaf836

- wip but with fixes from @shattered, now boots and gets to login prompt

935d59d

- swapped back Capslock and Ctrl to allow Ctrl key to work in Tek4404

5c3731f

- fixes for gating VBenable as suggested by @shattered

a7fcf71

- fixes runaway key repeating on input

- removed masking as its already in master for some reason..

7eefa12

- dont wait for 250 timeout (@shattered fix); hugely increases HD per…

b05f100

…formance

- less spammy logging

9834d64

- mouse input kinda works (thanks Phil)

d667594

- video panning not right

- fixed fine scrolling

e162cc9

- LOG_MMU for map_rw

13ce483

- video address panning almost works; something wrong with vertical s…

9f43ba2

…tride

- nvram stored as 4-bit nybbles (used for MAC address)

01f052e

- page 2.1-52 shows WrMapEn coming directly from latch

bc5abae

- wip of NS32081 interface

5aa8d59

- copy m_latched_map_control => m_map_control on every user access

6e8d18b

- ns32081 interface seems to work

74f45a6

- not sure about FCMP / DCMP return values.

less spammy

6d31c99

- fpu_latch... returns enum of result type

5d4712e

- added FGetStat and FSetStat to set ns32081 status register

- disable video panning for now; something not right with address calc

59a0453

- switched to using device/machine/ns32081 for fpu support

ee3acf6

- easier to read logging of MMU state changes

e5e8a25

- comment out log of every memory_r and memory_w

- TEMP disable cancelling bus error iff during prefetch; seems to mak…

f74a066

…e many things work - what we actually want is a way of knowing its a prefetch memory_r()

- when doing prefetch, we need to save/restore DA just as we do for r…

b97c9c8

…egular instr fetch.

- tweaks to keyboard mapping to make them work

61c9640

- unsure if this is a Mac keyboard mapping issue or just wrong

- removed MACHINE_NOT_WORKING Woot!

e5ae753

Elektraglide force-pushed the tek4404_working branch from bd7521b to e5ae753 Compare January 19, 2025 20:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tek4404 work_in_progress. #12574

Tek4404 work_in_progress. #12574

Elektraglide commented Jul 14, 2024 •

edited

Loading

rb6502 commented Jul 14, 2024

Elektraglide commented Jul 14, 2024

pmackinlay left a comment

pmackinlay Jul 15, 2024

pmackinlay Jul 15, 2024

pmackinlay Jul 15, 2024

pmackinlay Jul 15, 2024

pmackinlay Jul 15, 2024

pmackinlay Jul 15, 2024

cuavas Oct 8, 2024

cuavas Oct 8, 2024

cuavas Oct 8, 2024

cuavas Oct 8, 2024

cuavas Oct 8, 2024

cuavas Oct 8, 2024

cuavas Oct 8, 2024

cuavas Oct 8, 2024

rb6502 Nov 26, 2024

rb6502 commented Nov 26, 2024

Tek4404 work_in_progress. #12574

Are you sure you want to change the base?

Tek4404 work_in_progress. #12574

Conversation

Elektraglide commented Jul 14, 2024 • edited Loading

rb6502 commented Jul 14, 2024

Elektraglide commented Jul 14, 2024

pmackinlay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rb6502 commented Nov 26, 2024

Elektraglide commented Jul 14, 2024 •

edited

Loading