From e5aa0e9dc97f0092fe75bfa63728ac6980efc2a3 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Fri, 12 Jun 2020 21:46:37 +1000 Subject: [PATCH 1/3] uart: Remove combinational loops on ack and stall signal They hurt timing forcing signals to come from the master and back again in one cycle. Stall isn't sampled by the master unless there is an active cycle so masking it with cyc is pointless. Masking acks is somewhat pointless too as we don't handle early dropping of cyc in any of our slaves properly anyways. Signed-off-by: Benjamin Herrenschmidt --- fpga/pp_soc_uart.vhd | 22 +++++++++------------- soc.vhdl | 2 +- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/fpga/pp_soc_uart.vhd b/fpga/pp_soc_uart.vhd index 8c4c93c..1ed3bb9 100644 --- a/fpga/pp_soc_uart.vhd +++ b/fpga/pp_soc_uart.vhd @@ -107,8 +107,6 @@ architecture behaviour of pp_soc_uart is type wb_state_type is (IDLE, WRITE_ACK, READ_ACK); signal wb_state : wb_state_type; - signal wb_ack : std_logic; --! Wishbone acknowledge signal - signal rxd2 : std_logic := '1'; signal rxd3 : std_logic := '1'; signal txd2 : std_ulogic := '1'; @@ -319,13 +317,11 @@ begin ---------- Wishbone Interface ---------- - wb_ack_out <= wb_ack and wb_cyc_in and wb_stb_in; - wishbone: process(clk) begin if rising_edge(clk) then if reset = '1' then - wb_ack <= '0'; + wb_ack_out <= '0'; wb_state <= IDLE; send_buffer_push <= '0'; recv_buffer_pop <= '0'; @@ -348,7 +344,7 @@ begin end if; -- Invalid writes are acked and ignored. - wb_ack <= '1'; + wb_ack_out <= '1'; wb_state <= WRITE_ACK; else -- Read from register if wb_adr_in = x"008" then @@ -356,18 +352,18 @@ begin elsif wb_adr_in = x"010" then wb_dat_out <= x"0" & send_buffer_full & recv_buffer_full & send_buffer_empty & recv_buffer_empty; - wb_ack <= '1'; + wb_ack_out <= '1'; elsif wb_adr_in = x"018" then wb_dat_out <= sample_clk_divisor; - wb_ack <= '1'; + wb_ack_out <= '1'; elsif wb_adr_in = x"020" then wb_dat_out <= (0 => irq_recv_enable, 1 => irq_tx_ready_enable, others => '0'); - wb_ack <= '1'; + wb_ack_out <= '1'; else wb_dat_out <= (others => '0'); - wb_ack <= '1'; + wb_ack_out <= '1'; end if; wb_state <= READ_ACK; end if; @@ -376,7 +372,7 @@ begin send_buffer_push <= '0'; if wb_stb_in = '0' then - wb_ack <= '0'; + wb_ack_out <= '0'; wb_state <= IDLE; end if; when READ_ACK => @@ -384,11 +380,11 @@ begin recv_buffer_pop <= '0'; else wb_dat_out <= recv_buffer_output; - wb_ack <= '1'; + wb_ack_out <= '1'; end if; if wb_stb_in = '0' then - wb_ack <= '0'; + wb_ack_out <= '0'; wb_state <= IDLE; end if; end case; diff --git a/soc.vhdl b/soc.vhdl index 047be96..23d3885 100644 --- a/soc.vhdl +++ b/soc.vhdl @@ -548,7 +548,7 @@ begin wb_ack_out => wb_uart0_out.ack ); wb_uart0_out.dat <= x"000000" & uart_dat8; - wb_uart0_out.stall <= '0' when wb_uart0_in.cyc = '0' else not wb_uart0_out.ack; + wb_uart0_out.stall <= not wb_uart0_out.ack; spiflash_gen: if HAS_SPI_FLASH generate spiflash: entity work.spi_flash_ctrl From 6c3a8bf4171173c87d694d6721464c8e2f6423a6 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Fri, 12 Jun 2020 21:47:06 +1000 Subject: [PATCH 2/3] bram: Remove combinational loop on stall It hurts timing and is pointless Signed-off-by: Benjamin Herrenschmidt --- soc.vhdl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soc.vhdl b/soc.vhdl index 23d3885..7c8e825 100644 --- a/soc.vhdl +++ b/soc.vhdl @@ -609,7 +609,7 @@ begin no_bram: if MEMORY_SIZE = 0 generate wb_bram_out.ack <= wb_bram_in.cyc and wb_bram_in.stb; wb_bram_out.dat <= x"FFFFFFFFFFFFFFFF"; - wb_bram_out.stall <= wb_bram_in.cyc and not wb_bram_out.ack; + wb_bram_out.stall <= not wb_bram_out.ack; end generate; -- DMI(debug bus) <-> JTAG bridge From 176ae5c306ac61269d2489f39f65712d569b0cb7 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Fri, 12 Jun 2020 21:48:01 +1000 Subject: [PATCH 3/3] syscon: Remove combinational loop on ack and stall Those hurt timings. Instead latch the wishbone response for one cycle Signed-off-by: Benjamin Herrenschmidt --- syscon.vhdl | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/syscon.vhdl b/syscon.vhdl index e319f02..96053b5 100644 --- a/syscon.vhdl +++ b/syscon.vhdl @@ -91,6 +91,9 @@ architecture behaviour of syscon is signal info_has_spif : std_ulogic; signal info_clk : std_ulogic_vector(39 downto 0); signal info_fl_off : std_ulogic_vector(31 downto 0); + + -- Wishbone response latch + signal wb_rsp : wb_io_slave_out; begin -- Generated output signals @@ -98,10 +101,6 @@ begin soc_reset <= reg_ctrl(SYS_REG_CTRL_SOC_RESET); core_reset <= reg_ctrl(SYS_REG_CTRL_CORE_RESET); - -- All register accesses are single cycle - wishbone_out.ack <= wishbone_in.cyc and wishbone_in.stb; - wishbone_out.stall <= '0'; - -- Info register is hard wired info_has_uart <= '1' when HAS_UART else '0'; info_has_dram <= '1' when HAS_DRAM else '0'; @@ -128,7 +127,8 @@ begin reg_ctrl_out <= (63 downto SYS_REG_CTRL_BITS => '0', SYS_REG_CTRL_BITS-1 downto 0 => reg_ctrl); - -- Register read mux + -- Wishbone response + wb_rsp.ack <= wishbone_in.cyc and wishbone_in.stb; with wishbone_in.adr(SYS_REG_BITS+2 downto 3) select reg_out <= SIG_VALUE when SYS_REG_SIG, reg_info when SYS_REG_INFO, @@ -139,8 +139,18 @@ begin reg_ctrl_out when SYS_REG_CTRL, reg_spiinfo when SYS_REG_SPIFLASHINFO, (others => '0') when others; - wishbone_out.dat <= reg_out(63 downto 32) when wishbone_in.adr(2) = '1' else - reg_out(31 downto 0); + wb_rsp.dat <= reg_out(63 downto 32) when wishbone_in.adr(2) = '1' else + reg_out(31 downto 0); + wb_rsp.stall <= '0'; + + -- Wishbone response latch + regs_read: process(clk) + begin + if rising_edge(clk) then + -- Send response from latch + wishbone_out <= wb_rsp; + end if; + end process; -- Register writes regs_write: process(clk)