address decoder (once more)

[Olaf]

Hi,

as a lot of people before, I was gone into the trap of local static
variables of an address decoder.
What is the preferred way here? I would like use constants to write an C
header using the textio library.

Thanks
Olaf

 

[Mike Treseler]

Olaf wrote:

Hi,

as a lot of people before, I was gone into the trap of local static
variables of an address decoder.

See search for "data_ptr_c" int the reference design here:
http://home.comcast.net/~mike_treseler/

I put the bus interface constants in a package
to share with the testbench.
I use a port address and data buses between entities.
I infer io registers locally using variable declarations.

-- Mike Treseler

 

[Olaf]

Hi,

as a lot of people before, I was gone into the trap of local static
variables of an address decoder.

See search for "data_ptr_c" int the reference design here:
http://home.comcast.net/~mike_treseler/

I put the bus interface constants in a package
to share with the testbench.
I use a port address and data buses between entities.
I infer io registers locally using variable declarations.

Thanks for your answer. The Address is only one bit unfortunately. Here
is my address decoder, which looks very complicated and error prune to
me. I wrote some functions, which I can't use as I would like. Maybe
someone can give me hint to write it more compact since there are coming
more addresses which will follow the same schematic.

Thanks
Olaf

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

package decoder_pkg is

subtype address_t is unsigned(7 downto 0);
subtype register_t is std_ulogic_vector(31 downto 0);

constant trigger_control : address_t := x"00";
constant trigger_config : address_t := x"10";

constant trg0_bit_value : address_t := x"20";
constant trg1_bit_value : address_t := x"21";
constant trg2_bit_value : address_t := x"22";
constant trg3_bit_value : address_t := x"23";
constant trg4_bit_value : address_t := x"24";
constant trg5_bit_value : address_t := x"25";
constant trg6_bit_value : address_t := x"26";
constant trg7_bit_value : address_t := x"27";

constant trg0_bit_mask : address_t := x"30";
constant trg1_bit_mask : address_t := x"31";
constant trg2_bit_mask : address_t := x"32";
constant trg3_bit_mask : address_t := x"33";
constant trg4_bit_mask : address_t := x"34";
constant trg5_bit_mask : address_t := x"35";
constant trg6_bit_mask : address_t := x"36";
constant trg7_bit_mask : address_t := x"37";

constant trg0_re_mask : address_t := x"40";
constant trg1_re_mask : address_t := x"41";
constant trg2_re_mask : address_t := x"42";
constant trg3_re_mask : address_t := x"43";
constant trg4_re_mask : address_t := x"44";
constant trg5_re_mask : address_t := x"45";
constant trg6_re_mask : address_t := x"46";
constant trg7_re_mask : address_t := x"47";

constant trg0_fe_mask : address_t := x"50";
constant trg1_fe_mask : address_t := x"51";
constant trg2_fe_mask : address_t := x"52";
constant trg3_fe_mask : address_t := x"53";
constant trg4_fe_mask : address_t := x"54";
constant trg5_fe_mask : address_t := x"55";
constant trg6_fe_mask : address_t := x"56";
constant trg7_fe_mask : address_t := x"57";

constant trg0_edge_mask : address_t := x"60";
constant trg1_edge_mask : address_t := x"61";
constant trg2_edge_mask : address_t := x"62";
constant trg3_edge_mask : address_t := x"63";
constant trg4_edge_mask : address_t := x"64";
constant trg5_edge_mask : address_t := x"65";
constant trg6_edge_mask : address_t := x"66";
constant trg7_edge_mask : address_t := x"67";

end package;

-------------------------------------------------------------------------------
library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.decoder_pkg.all;

entity decoder is

generic (
TRIGGER_DEPTH : integer range 1 to 8 := 2;
RESET_ACTIVE : std_ulogic := '1');

port (
conf_clk : in std_ulogic;
reset : in std_ulogic;
address : in std_ulogic_vector(7 downto 0);
data : in std_ulogic_vector(31 downto 0);
-- decoded address signals
set_bit_value : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_bit_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_re_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_fe_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_edge_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_config : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);

glitch_capture : out std_ulogic;
arm : out std_ulogic);

end entity;



architecture behavioral of decoder is
signal config_reg : std_ulogic_vector(31 downto 0);
signal control_reg : std_ulogic_vector(31 downto 0);
begin
decode: process (conf_clk) is

variable addr : unsigned(address'range);
----------------------------------------
procedure reset_register is
begin
config_reg <= (others => '0');
control_reg <= (others => '0');
-- trigger register
set_bit_value <= (others => '0');
set_bit_mask <= (others => '0');
set_re_mask <= (others => '0');
set_fe_mask <= (others => '0');
set_edge_mask <= (others => '0');
set_config <= (others => '0');
end procedure;
-----------------------------------------
impure function trigger_no return integer is
variable no : unsigned(3 downto 0);
begin
return to_integer(no);
end function;
------------------------------------------
procedure dispatch_bit_value is
begin
set_bit_value(trigger_no) <= '1';
end procedure;
----------------------------------------
procedure dispatch_bit_mask is
begin
set_bit_mask(trigger_no) <= '1';
end procedure;
-----------------------------------------
begin
if rising_edge(conf_clk) then
if (reset = RESET_ACTIVE) then
reset_register;
else
addr := unsigned(address);

case addr is

when trigger_control => control_reg <= data;
when trigger_config => config_reg <= data;

when TRG0_bit_value => set_bit_value(0) <= '1';
when TRG1_bit_value => set_bit_value(1) <= '1';
when TRG2_bit_value => set_bit_value(2) <= '1';
when TRG3_bit_value => set_bit_value(3) <= '1';
when TRG4_bit_value => set_bit_value(4) <= '1';
when TRG5_bit_value => set_bit_value(5) <= '1';
when TRG6_bit_value => set_bit_value(6) <= '1';
when TRG7_bit_value => set_bit_value(7) <= '1';

when TRG0_bit_mask => set_bit_mask(0) <= '1';
when TRG1_bit_mask => set_bit_mask(1) <= '1';
when TRG2_bit_mask => set_bit_mask(2) <= '1';
when TRG3_bit_mask => set_bit_mask(3) <= '1';
when TRG4_bit_mask => set_bit_mask(4) <= '1';
when TRG5_bit_mask => set_bit_mask(5) <= '1';
when TRG6_bit_mask => set_bit_mask(6) <= '1';
when TRG7_bit_mask => set_bit_mask(7) <= '1';

when TRG0_re_mask => set_re_mask(0) <= '1';
when TRG1_re_mask => set_re_mask(1) <= '1';
when TRG2_re_mask => set_re_mask(2) <= '1';
when TRG3_re_mask => set_re_mask(3) <= '1';
when TRG4_re_mask => set_re_mask(4) <= '1';
when TRG5_re_mask => set_re_mask(5) <= '1';
when TRG6_re_mask => set_re_mask(6) <= '1';
when TRG7_re_mask => set_re_mask(7) <= '1';

when TRG0_fe_mask => set_fe_mask(0) <= '1';
when TRG1_fe_mask => set_fe_mask(1) <= '1';
when TRG2_fe_mask => set_fe_mask(2) <= '1';
when TRG3_fe_mask => set_fe_mask(3) <= '1';
when TRG4_fe_mask => set_fe_mask(4) <= '1';
when TRG5_fe_mask => set_fe_mask(5) <= '1';
when TRG6_fe_mask => set_fe_mask(6) <= '1';
when TRG7_fe_mask => set_fe_mask(7) <= '1';

when TRG0_edge_mask => set_edge_mask(0) <= '1';
when TRG1_edge_mask => set_edge_mask(1) <= '1';
when TRG2_edge_mask => set_edge_mask(2) <= '1';
when TRG3_edge_mask => set_edge_mask(3) <= '1';
when TRG4_edge_mask => set_edge_mask(4) <= '1';
when TRG5_edge_mask => set_edge_mask(5) <= '1';
when TRG6_edge_mask => set_edge_mask(6) <= '1';
when TRG7_edge_mask => set_edge_mask(7) <= '1';

when others => null;
end case;

glitch_capture <= config_reg(31);
arm <= control_reg(23);

end if;
end if;
end process;
end architecture;

 

[Mike Treseler]

Olaf wrote:

Thanks for your answer. The Address is only one bit unfortunately. Here
is my address decoder, which looks very complicated and error prune to
me. I wrote some functions, which I can't use as I would like. Maybe
someone can give me hint to write it more compact since there are coming
more addresses which will follow the same schematic.

You code compiles but does not elaborate.

Error (10384): VHDL assignment error at decoder.vhd(143):
index 2 is outside the range (1 downto 0) of object "set_bit_value"

I agree that there must be a simpler way,
but only you know what the requirements are.

You do have to decide if you want to do
a structural or a procedural design.
It is hard to mix the two.

I see address decoding as a boolean
comparison function used when needed,
not a design entity.

Good luck.

-- Mike Treseler

 

[Mike Treseler]

Mike Treseler wrote:

I see address decoding as a boolean
comparison function used when needed,
not a design entity.

Also see the recent thread
"State encoding"
for a better example.

-- Mike Treseler

 

[Olaf]

Mike Treseler schrieb:

Olaf wrote:

Thanks for your answer. The Address is only one bit unfortunately. Here
is my address decoder, which looks very complicated and error prune to
me. I wrote some functions, which I can't use as I would like. Maybe
someone can give me hint to write it more compact since there are coming
more addresses which will follow the same schematic.


You code compiles but does not elaborate.

Error (10384): VHDL assignment error at decoder.vhd(143):
index 2 is outside the range (1 downto 0) of object "set_bit_value"

Yes, I found out these some minutes before too. It's an ugly solution
and not scaleable as it should be as the generics suggest.

I agree that there must be a simpler way,
but only you know what the requirements are.

You do have to decide if you want to do
a structural or a procedural design.
It is hard to mix the two.

I haven't the experience yet for such problem It's a design problem
as well/mainly.

Anyway, it shall be a addressable register set. Some addresses should be
linear/following, since they have only a meaning of a instanced entity
number - the 8 triggers e.g. with each one of set_bit_{value,mask} etc.
Here is the top level of this:

architecture structure of core is
-- address decoder driven signals
signal set_bit_value : std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
signal set_bit_mask : std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
signal set_re_mask : std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
signal set_fe_mask : std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
signal set_edge_mask : std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
signal set_config : std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
signal arm : std_ulogic;

begin

address_decoder: entity work.decoder
generic map (
TRIGGER_DEPTH => TRIGGER_DEPTH,
RESET_ACTIVE => RESET_ACTIVE)
port map (
conf_clk => conf_clk,
reset => reset,
address => address,
data => data,
set_bit_value => set_bit_value,
set_bit_mask => set_bit_mask,
set_re_mask => set_re_mask,
set_fe_mask => set_fe_mask,
set_edge_mask => set_edge_mask,
set_config => set_config,
arm => arm);

trigger: entity work.trigger
generic map (
BIT_WIDTH => BIT_WIDTH,
TRIGGER_DEPTH => TRIGGER_DEPTH,
RESET_ACTIVE => RESET_ACTIVE)
port map (
...
set_bit_value => set_bit_value,
set_bit_mask => set_bit_mask,
set_re_mask => set_re_mask,
set_fe_mask => set_fe_mask,
set_edge_mask => set_edge_mask,
set_config => set_config,
data => data,
arm => arm,
shot => shot);

....

with:

architecture behavioral of trigger is
...
begin
triggers: for n in 0 to TRIGGER_DEPTH-1 generate

trigger_stage_n: entity work.trigger_stage
generic map (
BIT_WIDTH => BIT_WIDTH,
RESET_ACTIVE => RESET_ACTIVE)
port map (
...
set_bit_value => set_bit_value,
set_bit_mask => set_bit_mask,
set_re_mask => set_re_mask,
set_fe_mask => set_fe_mask,
set_edge_mask => set_edge_mask,
set_config => set_config,
data => data,
...
end generate;

Hopefully my intention are more clear now.

Thanks
Olaf

 

[Andy]

Whenever I see long case statements with numeric targets, I think "Is
there a way to do this with an array and/or a loop?" Such solutions
are usually more compact and certainly more maintainable.

Define constants for start and length of bit_value, bit_mask,
re_mask, etc. addresses

Then instead of a case statement, use a series of loops (one for
bit_value, one for bit_mask, etc.)

for i in set_bit_value'range loop
if to_integer(addr) = bit_value_start + i then
set_bit_value(i) <= '1';
end if;
end loop;

The addition is done statically after the loop is unrolled, so no
hardware is consumed.

Synthesis can tell that the address is being compared to mutually
exclusive values, so there is no priority encoding, resulting in the
same circuit as if a case statement had been used.

Andy

 

[Olaf]

Andy schrieb:

Whenever I see long case statements with numeric targets, I think "Is
there a way to do this with an array and/or a loop?" Such solutions
are usually more compact and certainly more maintainable.

Define constants for start and length of bit_value, bit_mask,
re_mask, etc. addresses

Then instead of a case statement, use a series of loops (one for
bit_value, one for bit_mask, etc.)

for i in set_bit_value'range loop
if to_integer(addr) = bit_value_start + i then
set_bit_value(i) <= '1';
end if;
end loop;

The addition is done statically after the loop is unrolled, so no
hardware is consumed.

Synthesis can tell that the address is being compared to mutually
exclusive values, so there is no priority encoding, resulting in the
same circuit as if a case statement had been used.

Thanks Andy, an interesting solution. I wrote:


package decoder_pkg is
subtype address_t is unsigned(7 downto 0);
subtype register_t is std_ulogic_vector(31 downto 0);

constant config_base : address_t := x"00";
constant trigger_control_base : address_t := x"10";
constant bit_value_base : address_t := x"20";
constant bit_mask_base : address_t := x"30";
constant re_mask_base : address_t := x"40";
constant fe_mask_base : address_t := x"50";
constant edge_mask_base : address_t := x"60";
end package;
-------------------------------------------------------------------------------
library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.decoder_pkg.all;

entity decoder is

generic (
TRIGGER_DEPTH : integer range 1 to 8 := 2;
RESET_ACTIVE : std_ulogic := '1');
port (
conf_clk : in std_ulogic;
reset : in std_ulogic;
address : in std_ulogic_vector(7 downto 0);
data : in std_ulogic_vector(31 downto 0);
set_bit_value : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_bit_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_re_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_fe_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_edge_mask : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
set_config : out std_ulogic_vector(TRIGGER_DEPTH-1 downto 0);
...
end entity;

architecture behavioral of decoder is
signal config_reg : register_t;
signal trigger_control_reg : register_t;
begin
decode: process (conf_clk) is
-----------------------------------------------
procedure reset_register is
...
--------------------------------------------
procedure check_address (
constant base_addr : in std_ulogic_vector;
signal action : out std_ulogic_vector)
is
variable port_addr : unsigned(address'range);
variable addr_base : unsigned(base_addr'range);
begin
port_addr := unsigned(address);
addr_base := unsigned(base_addr);
if (port_addr = addr_base) then
action <= data;
end if;
end procedure;
----------------------------------------------
procedure check_address2 (
constant base_addr : in std_ulogic_vector;
signal action_select : out std_ulogic_vector)
is
variable port_addr : unsigned(address'range);
variable addr_base : unsigned(base_addr'range);
begin
port_addr := unsigned(address);
addr_base := unsigned(base_addr);
for i in action_select'range loop
if (to_integer(port_addr) = addr_base + i) then
action_select(i) <= '1';
end if;
end loop;
end procedure;
-------------------------------------------------
begin
if rising_edge(conf_clk) then
if (reset = RESET_ACTIVE) then
reset_register;
else
check_address(config_base, config_reg);
check_address(trigger_control_base, trigger_control_reg);
check_address2(bit_value_base, set_bit_value);
...

which fails to compile unfortunately:

No feasible entries for subprogram "check_address".
No feasible entries for subprogram "check_address".
No feasible entries for subprogram "check_address2"

It's a problem to find the right function signature of curse ...

Any ideas to avoid the use of different names for check_address() and
check_address2()? - overloaded functions would be more appropriate imo.

Thanks
Olaf

 

[Olaf]

check_address(config_base, config_reg);
check_address(trigger_control_base, trigger_control_reg);
check_address2(bit_value_base, set_bit_value);
...

which fails to compile unfortunately:

No feasible entries for subprogram "check_address".
No feasible entries for subprogram "check_address".
No feasible entries for subprogram "check_address2"

It's a problem to find the right function signature of curse ...

yep, function should be:

procedure check_address (
constant base_addr : in address_t;
signal target_reg : out register_t)

with address_t unsigned.

Regards
Olaf