i8244: change pos hold strobe again, add sound interrupt

src/devices/video/i8244.cpp

@@ -140,14 +140,12 @@ void i8244_device::device_start()
 
 	save_item(NAME(m_x_beam_pos));
 	save_item(NAME(m_y_beam_pos));
-	save_item(NAME(m_y_latch));
 	save_item(NAME(m_control_status));
 	save_item(NAME(m_collision_status));
-	save_item(NAME(m_pos_hold));
-	save_item(NAME(m_y_hold));
 	save_item(NAME(m_sh_written));
 	save_item(NAME(m_sh_pending));
 	save_item(NAME(m_sh_prescaler));
+	save_item(NAME(m_sh_count));
 	save_item(NAME(m_sh_output));
 	save_item(NAME(m_sh_duty));
 }
@@ -309,7 +307,7 @@ uint8_t i8244_device::read(offs_t offset)
 			data |= (h >= 225 && h < m_bgate_start && get_y_beam() <= m_vblank_start) ? 1 : 0;
 
 			// position strobe status
-			data |= m_pos_hold ? 2 : 0;
+			data |= m_vdc.s.control & 0x02;
 
 			m_irq_func(CLEAR_LINE);
 			m_control_status &= ~0xcc;
@@ -323,34 +321,12 @@ uint8_t i8244_device::read(offs_t offset)
 			break;
 
 		case 0xa4:
-			if (m_y_hold)
-			{
-				data = m_y_latch;
-				m_y_hold = false;
-			}
-			else if (m_pos_hold)
-				data = m_y_beam_pos;
-			else
-				data = get_y_beam();
-
+			data = (m_vdc.s.control & 0x02) ? get_y_beam() : m_y_beam_pos;
 			break;
 
 		case 0xa5:
-		{
-			if (m_pos_hold)
-			{
-				data = m_x_beam_pos;
-				m_pos_hold = false;
-			}
-			else
-				data = get_x_beam();
-
-			// Y is latched when reading X
-			m_y_hold = true;
-			m_y_latch = get_y_beam();
-
+			data = (m_vdc.s.control & 0x02) ? get_x_beam() : m_x_beam_pos;
 			break;
-		}
 
 		default:
 			data = m_vdc.reg[offset];
@@ -387,15 +363,12 @@ void i8244_device::write(offs_t offset, uint8_t data)
 	switch (offset)
 	{
 		case 0xa0:
-			if ((m_vdc.s.control & 0x02) && !(data & 0x02) && !m_pos_hold)
+			if ((m_vdc.s.control & 0x02) && !(data & 0x02))
 			{
 				// toggling strobe bit, tuck away values
 				m_x_beam_pos = get_x_beam();
 				m_y_beam_pos = get_y_beam();
-				m_pos_hold = true;
 			}
-			// note: manual talks about a hblank interrupt on d0, and a sound interrupt on d2,
-			// but tests done on 8244 reveal no such features
 			break;
 
 		case 0xa7: case 0xa8: case 0xa9:
@@ -800,19 +773,35 @@ void i8244_device::sound_update()
 		int feedback = m_sh_output;
 		signal >>= 1;
 
-		/* Noise tap is on bits 0 and 5 and fed back to bit 15 */
+		// noise tap is on bits 0 and 5 and fed back to bit 15
 		if (m_vdc.s.sound & 0x10)
 		{
 			feedback ^= signal >> 4 & 1; // pre-shift bit 5
 			signal = (signal & ~0x8000) | (feedback << 15);
 		}
 
-		/* Loop sound */
+		// loop sound
 		signal |= feedback << 23;
 
 		m_vdc.s.shift3 = signal & 0xFF;
 		m_vdc.s.shift2 = ( signal >> 8 ) & 0xFF;
 		m_vdc.s.shift1 = ( signal >> 16 ) & 0xFF;
+
+		// sound interrupt
+		if (++m_sh_count == 24)
+		{
+			m_sh_count = 0;
+			if (m_vdc.s.control & 0x04)
+			{
+				m_control_status |= 0x04;
+				m_irq_func(ASSERT_LINE);
+			}
+		}
 	}
-	m_sh_written = false;
+	else if (m_sh_written)
+	{
+		m_sh_count = 0;
+		m_sh_written = false;
+	}
+
 }

src/devices/video/i8244.h

@@ -140,15 +140,13 @@ protected:
 
 	uint8_t m_x_beam_pos = 0;
 	uint8_t m_y_beam_pos = 0;
-	uint8_t m_y_latch = 0;
 	uint8_t m_control_status = 0;
 	uint8_t m_collision_status = 0;
-	bool m_pos_hold = false;
-	bool m_y_hold = false;
 
 	bool m_sh_written = false;
 	bool m_sh_pending = false;
-	u8 m_sh_prescaler = 0 ;
+	u8 m_sh_prescaler = 0;
+	u8 m_sh_count = 0;
 	int m_sh_output = 0;
 	u8 m_sh_duty = 0;
 };

src/mame/drivers/odyssey2.cpp

@@ -54,14 +54,20 @@ XTAL notes (differs per model):
 TODO:
 - backgamm doesn't draw all the chars/sprites, it does multiple screen updates
   and writes to the ptr/color registers, but does not increment the Y regs
-- 824x screen resolution is not strictly defined, height(243) is correct, but
+- screen resolution is not strictly defined, height(243) is correct, but
   horizontal overscan differs depending on monitor/tv? see syracuse for overscan
 - 824x on the real console, overlapping characters on eachother will cause
   glitches (it is used to an advantage in some as-of-yet undumped homebrews)
 - 8244(NTSC) is not supposed to show characters near the upper border, but
   hiding them will cause bugs in some Euro games
-- 8245(PAL) video timing is not 100% accurate, though vtotal and htotal should
-  be correct
+- according to tests, 8244 does not have a sound interrupt, but the Philips
+  service test cartridge for 8245 tests for it and fails if it did not get an irq
+- likewise, 8244 does not have a horizontal interrupt, but does 8245 have it?
+- tests done on 8244 suggests that Y(0xa4) is latched when reading X, but
+  that is inconsistent with the Philips service test cartridge: It reads X, Y, X,
+  then waits for 1 scanline, and reads Y again. It expects Y to change. Latching Y
+  will also cause video glitches to look different on some games when compared
+  to the real console, for example powerlrd.
 - ppp(the tetris game) does not work properly on PAL, is this homebrew NTSC-only,
   or is PAL detection going wrong? It does look like PAL/NTSC detection is working,
   see internal RAM $3D d7. So maybe it is due to inaccurate PAL video timing.