From 7cf8db9acaa9d6cb889719e6d51ed04c5618f0e9 Mon Sep 17 00:00:00 2001 From: Philip Smart Date: Wed, 1 Apr 2026 23:31:52 +0100 Subject: [PATCH] Synchroniser bug fix --- CPLD/v1.2/sfd700.vhd | 240 ++++++++++++++++++++++++++++----------- CPLD/v1.2/sfd700_pkg.vhd | 9 +- 2 files changed, 180 insertions(+), 69 deletions(-) diff --git a/CPLD/v1.2/sfd700.vhd b/CPLD/v1.2/sfd700.vhd index 3425215..3697003 100644 --- a/CPLD/v1.2/sfd700.vhd +++ b/CPLD/v1.2/sfd700.vhd @@ -12,7 +12,7 @@ -- Copyright: (c) 2018-23 Philip Smart -- -- History: July 2023 - v1.0 - Initial write. - +-- Apr 2026 - v1.1 - Synchroniser bug fixes. --------------------------------------------------------------------------------------------------------- -- This source file is free software: you can redistribute it and-or modify -- it under the terms of the GNU General Public License as published @@ -140,6 +140,37 @@ architecture rtl of cpld128 is -- Selected mode of the interface, ie. which machine it will be plugged into. signal IFMODE : integer range 0 to 7 := 0; + -- Synchronised bus select signals (1-stage registered to eliminate glitches from combinational decode). + -- The existing *_LAST variable in each process provides the second synchronisation stage, + -- giving a full 2-stage pipeline for edge detection. + signal SIDE_WR_SYNC : std_logic := '1'; + signal DDEN_WR_SYNC : std_logic := '1'; + signal DRIVE_WR_SYNC : std_logic := '1'; + signal INTEN_SYNC : std_logic := '1'; + signal EXXX_WR_SYNC : std_logic := '1'; + signal FXXX_WR_SYNC : std_logic := '1'; + signal RAMEN_WR_SYNC : std_logic := '1'; + signal ROMINHSET_WR_SYNC : std_logic := '1'; + signal ROMINHCLR_WR_SYNC : std_logic := '1'; + signal ROMDISSET_WR_SYNC : std_logic := '1'; + signal ROMDISCLR_WR_SYNC : std_logic := '1'; + + -- Registered Z80 data bus and RDn for clean capture in write processes. + signal Z80_DATA_SYNC : std_logic_vector(7 downto 0) := (others => '0'); + signal Z80_RDn_SYNC : std_logic := '1'; + + -- Registered memory chip selects — eliminates combinational glitches from address bus + -- skew propagating through MEM_EXXX/FXXX decode into ROM_SELni/RAM_SELni. + -- One CLK_16M cycle (62.5ns) of added latency; the Z80 at 3.54MHz has ~200ns of read + -- access window so this is well within margin for typical FlashROM/RAM (55-70ns). + signal ROM_CSn_REG : std_logic := '1'; + signal RAM_CSn_REG : std_logic := '1'; + + -- Registered ID bus output for page address — prevents brief high-Z glitch on + -- ID during address transitions from corrupting the ROM/RAM upper address. + signal ID_REG : std_logic_vector(7 downto 0) := (others => '0'); + signal ID_OEn : std_logic := '1'; + -- Clocks. signal CLK_8Mi : std_logic := '0'; @@ -162,53 +193,95 @@ begin end if; end process; - -- Process to register configuration mode on reset. - SETMODE: process( Z80_RESETn, MODE ) + -- Synchronise asynchronous Z80 bus-derived select signals into the CLK_16M domain. + -- This eliminates glitches caused by address/control bus skew on the combinational decodes. + -- Each signal is registered once here; the *_LAST variable in each consumer process provides + -- the second stage, giving a full 2-stage synchronisation pipeline for edge detection. + SYNCBUS: process( Z80_RESETn, CLK_16M ) begin if(Z80_RESETn = '0') then - -- Configuration jumpers specify the machine the SFD700 card will be used with. - IFMODE <= to_integer(unsigned(MODE)); + SIDE_WR_SYNC <= '1'; + DDEN_WR_SYNC <= '1'; + DRIVE_WR_SYNC <= '1'; + INTEN_SYNC <= '1'; + EXXX_WR_SYNC <= '1'; + FXXX_WR_SYNC <= '1'; + RAMEN_WR_SYNC <= '1'; + ROMINHSET_WR_SYNC <= '1'; + ROMINHCLR_WR_SYNC <= '1'; + ROMDISSET_WR_SYNC <= '1'; + ROMDISCLR_WR_SYNC <= '1'; + Z80_DATA_SYNC <= (others => '0'); + Z80_RDn_SYNC <= '1'; + + elsif(rising_edge(CLK_16M)) then + SIDE_WR_SYNC <= SIDE_WR_SELni; + DDEN_WR_SYNC <= DDEN_WR_SELni; + DRIVE_WR_SYNC <= DRIVE_WR_SELni; + INTEN_SYNC <= INTEN_SELni; + EXXX_WR_SYNC <= EXXX_WR_SELni; + FXXX_WR_SYNC <= FXXX_WR_SELni; + RAMEN_WR_SYNC <= RAMEN_WR_SELni; + ROMINHSET_WR_SYNC <= ROMINHSET_WR_SELni; + ROMINHCLR_WR_SYNC <= ROMINHCLR_WR_SELni; + ROMDISSET_WR_SYNC <= ROMDISSET_WR_SELni; + ROMDISCLR_WR_SYNC <= ROMDISCLR_WR_SELni; + Z80_DATA_SYNC <= Z80_DATA; + Z80_RDn_SYNC <= Z80_RDn; + end if; + end process; + + -- Process to register configuration mode on reset. + -- Synchronous reset captures MODE jumpers cleanly on CLK_16M; once reset deasserts the + -- register holds its value. Avoids latch inference from the original combinational process. + SETMODE: process( CLK_16M ) + begin + if(rising_edge(CLK_16M)) then + if(Z80_RESETn = '0') then + -- Configuration jumpers specify the machine the SFD700 card will be used with. + IFMODE <= to_integer(unsigned(MODE)); + end if; end if; end process; -- Set Floppy Disk Side. - -- A write to 0xDD bit D0 = low sets Floppy Disk side 1, high sets side 0. - SETSIDE: process( Z80_RESETn, CLK_16M, SIDE_WR_SELni ) + -- A write to 0xDD: D0 = 0 selects side 0, D0 = 1 selects side 1 (active-high on SIDE1 output). + SETSIDE: process( Z80_RESETn, CLK_16M ) variable SIDE_SEL_LASTni : std_logic; begin if(Z80_RESETn = '0') then REG_SIDE <= '0'; - SIDE_SEL_LASTni := '0'; + SIDE_SEL_LASTni := '1'; elsif(rising_edge(CLK_16M)) then - if(SIDE_WR_SELni = '0' and SIDE_SEL_LASTni = '1') then - REG_SIDE <= not Z80_DATA(0); + if(SIDE_WR_SYNC = '0' and SIDE_SEL_LASTni = '1') then + REG_SIDE <= not Z80_DATA_SYNC(0); end if; - SIDE_SEL_LASTni := SIDE_WR_SELni; + SIDE_SEL_LASTni := SIDE_WR_SYNC; end if; end process; -- Set Double Data Rate Enable. -- A write to 0xDE bit D0 = low enables double data rate, high enables single data rate. - SETDDEN: process( Z80_RESETn, CLK_16M, DDEN_WR_SELni ) + SETDDEN: process( Z80_RESETn, CLK_16M ) variable DDEN_SEL_LASTni : std_logic; begin if(Z80_RESETn = '0') then REG_DDEN <= '1'; - DDEN_SEL_LASTni := '0'; + DDEN_SEL_LASTni := '1'; elsif(rising_edge(CLK_16M)) then - if(DDEN_WR_SELni = '0' and DDEN_SEL_LASTni = '1') then - REG_DDEN <= not Z80_DATA(0); + if(DDEN_WR_SYNC = '0' and DDEN_SEL_LASTni = '1') then + REG_DDEN <= not Z80_DATA_SYNC(0); end if; - DDEN_SEL_LASTni := DDEN_WR_SELni; + DDEN_SEL_LASTni := DDEN_WR_SYNC; end if; end process; -- Set Drive Number and Motor -- A write to 0xDC, bits D2:0 sets the active drive. Bit 2 high enables the active drive, low disables all drives. -- Bit D7 enables the motor. - SETDRIVE: process( Z80_RESETn, CLK_16M, DRIVE_WR_SELni ) + SETDRIVE: process( Z80_RESETn, CLK_16M ) variable DRIVE_SEL_LASTni: std_logic; begin if(Z80_RESETn = '0') then @@ -217,16 +290,16 @@ begin REG_DRIVEC <= '0'; REG_DRIVED <= '0'; REG_MOTOR <= '0'; - DRIVE_SEL_LASTni := '0'; + DRIVE_SEL_LASTni := '1'; elsif(rising_edge(CLK_16M)) then - if(DRIVE_WR_SELni = '0' and DRIVE_SEL_LASTni = '1') then + if(DRIVE_WR_SYNC = '0' and DRIVE_SEL_LASTni = '1') then REG_DRIVEA <= '0'; REG_DRIVEB <= '0'; REG_DRIVEC <= '0'; REG_DRIVED <= '0'; - REG_MOTOR <= Z80_DATA(7); - case(to_integer(unsigned(Z80_DATA(2 downto 0)))) is + REG_MOTOR <= Z80_DATA_SYNC(7); + case(to_integer(unsigned(Z80_DATA_SYNC(2 downto 0)))) is when 0 => when 4 => @@ -241,86 +314,95 @@ begin when others => end case; end if; - DRIVE_SEL_LASTni := DRIVE_WR_SELni; + DRIVE_SEL_LASTni := DRIVE_WR_SYNC; end if; end process; -- Set Interrupt Enable -- A write to 0xDF enables WD1773 interrupts, a read from 0xDF disables WD1773 interrupts. - SETINT: process( Z80_RESETn, CLK_16M, INTEN_SELni ) + SETINT: process( Z80_RESETn, CLK_16M ) variable INTEN_SEL_LASTni: std_logic; begin if(Z80_RESETn = '0') then REG_INT <= '0'; - INTEN_SEL_LASTni := '0'; + INTEN_SEL_LASTni := '1'; elsif(rising_edge(CLK_16M)) then - if(INTEN_SELni = '0' and INTEN_SEL_LASTni = '1') then - REG_INT <= Z80_RDn; + if(INTEN_SYNC = '0' and INTEN_SEL_LASTni = '1') then + REG_INT <= Z80_RDn_SYNC; end if; - INTEN_SEL_LASTni := INTEN_SELni; + INTEN_SEL_LASTni := INTEN_SYNC; end if; end process; -- Set FlashROM/RAM EXXX (E300:EFFF) page address. Each bank is 4K but due to the memory mapped I/O, only -- E300:EFFF is useable in RFS. -- A write t0 0x60 copies D6:0 into the EXXX page address register which selects a required 4K page in region E300:EFFF - SETEXXXPAGE: process( Z80_RESETn, CLK_16M, IFMODE, EXXX_WR_SELni ) + SETEXXXPAGE: process( Z80_RESETn, CLK_16M ) variable EXXX_SEL_LASTni : std_logic; begin if(Z80_RESETn = '0') then -- Customised UROM containing RFS start bank. REG_EXXX_PAGE <= "0000010"; - EXXX_SEL_LASTni := '0'; - + EXXX_SEL_LASTni := '1'; + elsif(rising_edge(CLK_16M)) then - if(EXXX_WR_SELni = '0' and EXXX_SEL_LASTni = '1') then - REG_EXXX_PAGE <= Z80_DATA(6 downto 0); + if(EXXX_WR_SYNC = '0' and EXXX_SEL_LASTni = '1') then + REG_EXXX_PAGE <= Z80_DATA_SYNC(6 downto 0); end if; - EXXX_SEL_LASTni := EXXX_WR_SELni; + EXXX_SEL_LASTni := EXXX_WR_SYNC; end if; end process; -- Set FlashROM/RAM FXXX (F000:FFFF) page address. -- A write to 0x61 copies D6:0 into the FXXX page address register which selects a 4K page -- window of ROM/RAM accessible in the FXXX window. - SETFXXXPAGE: process( Z80_RESETn, CLK_16M, IFMODE, FXXX_WR_SELni ) + SETFXXXPAGE: process( Z80_RESETn, CLK_16M ) variable FXXX_SEL_LASTni : std_logic; + variable RESET_DONE : std_logic; begin if(Z80_RESETn = '0') then -- Initial page is set to 0, which is the MZ-80A AFI ROM. The original ROM is 2K so potential to add additional code in the upper block. REG_FXXX_PAGE <= (others => '0'); - - -- MZ-700, starting at the second 4k page, set banking registers to 4K block which stores the MZ-700 AFI ROM. - if(IFMODE = MODE_MZ700) then - REG_FXXX_PAGE(1 downto 0) <= "01"; - end if; + FXXX_SEL_LASTni := '1'; + RESET_DONE := '0'; elsif(rising_edge(CLK_16M)) then + -- On the first clock after reset deasserts, apply mode-specific default. + -- IFMODE is a synchronous register captured during reset, so it is stable here. + if(RESET_DONE = '0') then + RESET_DONE := '1'; + -- MZ-700, starting at the second 4K page, set banking register to the block which stores the MZ-700 AFI ROM. + if(IFMODE = MODE_MZ700) then + REG_FXXX_PAGE(1 downto 0) <= "01"; + end if; + end if; + -- Set 4k F000:FFFF page address. - if(FXXX_WR_SELni = '0' and FXXX_SEL_LASTni = '1') then - REG_FXXX_PAGE <= Z80_DATA(6 downto 0); + if(FXXX_WR_SYNC = '0' and FXXX_SEL_LASTni = '1') then + REG_FXXX_PAGE <= Z80_DATA_SYNC(6 downto 0); end if; -- Edge detection. - FXXX_SEL_LASTni := FXXX_WR_SELni; + FXXX_SEL_LASTni := FXXX_WR_SYNC; end if; end process; -- Enable FlashROM or RAM. -- A write to 0x62 with D0 = low enables FlashROM, D0 = high enables RAM. - SETRAMEN: process( Z80_RESETn, CLK_16M, RAMEN_WR_SELni ) + SETRAMEN: process( Z80_RESETn, CLK_16M ) variable RAMEN_SEL_LASTni: std_logic; begin if(Z80_RESETn = '0') then REG_RAMEN <= '0'; + RAMEN_SEL_LASTni := '1'; elsif(rising_edge(CLK_16M)) then - if(RAMEN_WR_SELni = '0' and RAMEN_SEL_LASTni = '1') then - REG_RAMEN <= Z80_DATA(0); + if(RAMEN_WR_SYNC = '0' and RAMEN_SEL_LASTni = '1') then + REG_RAMEN <= Z80_DATA_SYNC(0); end if; - RAMEN_SEL_LASTni := RAMEN_WR_SELni; + RAMEN_SEL_LASTni := RAMEN_WR_SYNC; end if; end process; @@ -339,7 +421,7 @@ begin -- -- A write to 0xE1 enables RAM in the region 0xD000:FFFF so the onboard FlashROM/RAM should be disabled. A write to 0xE3/0xE4 re-enables -- FlashROM/RAM. A write to 0xE5 inhibits all access to region 0xD000:FFFF so the onboard FlashROM/RAM is disabled, write to 0xE6 re-enables it. - SETHIMEM: process( Z80_RESETn, CLK_16M, ROMINHSET_WR_SELni, ROMINHCLR_WR_SELni, ROMDISSET_WR_SELni, ROMDISCLR_WR_SELni ) + SETHIMEM: process( Z80_RESETn, CLK_16M ) variable ROMINHSET_LAST : std_logic; variable ROMINHCLR_LAST : std_logic; variable ROMDISSET_LAST : std_logic; @@ -348,28 +430,28 @@ begin if(Z80_RESETn = '0') then REG_ROMINH <= '0'; REG_ROMDIS <= '0'; - ROMINHSET_LAST := '0'; - ROMINHCLR_LAST := '0'; - ROMDISSET_LAST := '0'; - ROMDISCLR_LAST := '0'; + ROMINHSET_LAST := '1'; + ROMINHCLR_LAST := '1'; + ROMDISSET_LAST := '1'; + ROMDISCLR_LAST := '1'; elsif(rising_edge(CLK_16M)) then - if(ROMINHSET_WR_SELni = '0' and ROMINHSET_LAST = '1') then + if(ROMINHSET_WR_SYNC = '0' and ROMINHSET_LAST = '1') then REG_ROMINH <= '1'; end if; - if(ROMINHCLR_WR_SELni = '0' and ROMINHCLR_LAST = '1') then + if(ROMINHCLR_WR_SYNC = '0' and ROMINHCLR_LAST = '1') then REG_ROMINH <= '0'; end if; - if(ROMDISSET_WR_SELni = '0' and ROMDISSET_LAST = '1') then + if(ROMDISSET_WR_SYNC = '0' and ROMDISSET_LAST = '1') then REG_ROMDIS <= '1'; end if; - if(ROMDISCLR_WR_SELni = '0' and ROMDISCLR_LAST = '1') then + if(ROMDISCLR_WR_SYNC = '0' and ROMDISCLR_LAST = '1') then REG_ROMDIS <= '0'; end if; - ROMINHSET_LAST := ROMINHSET_WR_SELni; - ROMINHCLR_LAST := ROMINHCLR_WR_SELni; - ROMDISSET_LAST := ROMDISSET_WR_SELni; - ROMDISCLR_LAST := ROMDISCLR_WR_SELni; + ROMINHSET_LAST := ROMINHSET_WR_SYNC; + ROMINHCLR_LAST := ROMINHCLR_WR_SYNC; + ROMDISSET_LAST := ROMDISSET_WR_SYNC; + ROMDISCLR_LAST := ROMDISCLR_WR_SYNC; end if; end process; @@ -421,6 +503,34 @@ begin ROMDISCLR_WR_SELni <= '0' when Z80_IORQn = '0' and Z80_WRn = '0' and (unsigned(Z80_ADDR(7 downto 0)) = X"E3" or unsigned(Z80_ADDR(7 downto 0)) = X"E4") else '1'; + -- Register memory chip selects and ID bus page address on CLK_16M to prevent + -- combinational glitches during Z80 address transitions from reaching the FlashROM/RAM. + -- The FDC data path on ID remains combinational (WD1773 needs immediate response). + MEMCS: process( Z80_RESETn, CLK_16M ) + begin + if(Z80_RESETn = '0') then + ROM_CSn_REG <= '1'; + RAM_CSn_REG <= '1'; + ID_REG <= (others => '0'); + ID_OEn <= '1'; + + elsif(rising_edge(CLK_16M)) then + ROM_CSn_REG <= ROM_SELni; + RAM_CSn_REG <= RAM_SELni; + + -- Register the page address and output enable for non-FDC memory accesses. + if(MEM_EXXX_SELni = '0') then + ID_REG <= REG_EXXX_PAGE & Z80_ADDR(11); + ID_OEn <= '0'; + elsif(MEM_FXXX_SELni = '0') then + ID_REG <= REG_FXXX_PAGE & Z80_ADDR(11); + ID_OEn <= '0'; + else + ID_OEn <= '1'; + end if; + end if; + end process; + -- WD1773 master clock. CLK_FDC <= CLK_8Mi; @@ -437,11 +547,11 @@ begin else (others => 'Z'); -- ID is the inverted data bus, the WD1773 uses inverted data. ID doubles up as the FlashROM/RAM upper address bits when not being used for the WD1773. + -- FDC data path remains combinational (WD1773 CSn is also combinational and needs immediate data). + -- Memory page address uses the registered path to prevent glitches during address transitions. ID <= not Z80_DATA when Z80_WRn = '0' and FDC_SELni = '0' else - REG_EXXX_PAGE & Z80_ADDR(11) when MEM_EXXX_SELni = '0' - else - REG_FXXX_PAGE & Z80_ADDR(11) when MEM_FXXX_SELni = '0' + ID_REG when ID_OEn = '0' else (others => 'Z'); @@ -456,9 +566,9 @@ begin else Z80_ADDR(10); RAM_A10 <= Z80_ADDR(10); - -- FlashROM/RAM Chip Select. - ROM_CSn <= ROM_SELni; - RAM_CSn <= RAM_SELni; + -- FlashROM/RAM Chip Select (registered to prevent combinational glitches). + ROM_CSn <= ROM_CSn_REG; + RAM_CSn <= RAM_CSn_REG; -- Floppy Disk Interface Signals. FDCn <= FDC_SELni; diff --git a/CPLD/v1.2/sfd700_pkg.vhd b/CPLD/v1.2/sfd700_pkg.vhd index 6b7a21e..dcbe44f 100644 --- a/CPLD/v1.2/sfd700_pkg.vhd +++ b/CPLD/v1.2/sfd700_pkg.vhd @@ -12,6 +12,7 @@ -- Copyright: (c) 2018-23 Philip Smart -- -- History: July 2023 - Initial write. +-- Apr 2026 - Synchroniser bug fixes. -- --------------------------------------------------------------------------------------------------------- -- This source file is free software: you can redistribute it and-or modify @@ -112,7 +113,6 @@ package body sfd700_pkg is else return b; end if; - return a; end function IntMax; -- Find the number of bits required to represent an integer. @@ -132,16 +132,17 @@ package body sfd700_pkg is end function; -- Function to calculate the number of whole 'clock' cycles in a given time period, the period being in ns. + -- Returns CEIL(period * clock / 1e9), i.e. the minimum number of clock ticks that span the period. function clockTicks(period : in integer; clock : in integer) return integer is variable ticks : real; variable fracTicks : real; begin ticks := (Real(period) * Real(clock)) / 1000000000.0; - fracTicks := ticks - CEIL(ticks); + fracTicks := ticks - FLOOR(ticks); if fracTicks > 0.0001 then - return Integer(CEIL(ticks + 1.0)); + return Integer(FLOOR(ticks)) + 1; else - return Integer(CEIL(ticks)); + return Integer(FLOOR(ticks)); end if; end function;