axi_xbar: Change default master port setting from input to parameter

[andreaskurth]

Currently, the default master port feature of axi_xbar is set by the input signals en_default_mst_port_i and default_mst_port_i instead of by parameters.

The advantage of using input signals is that the default master port can be activated/deactivated and changed at run time. However, this may only happen when there is no unserved Ax beat. There are assertions that check that, but this restriction is not enforced in the synthesized logic. (Although it would be possible to enforce it in the synthesized logic at the expense of a few extra registers). Additionally, it is questionable whether changing the default port at run time has a real use case and therefore is a relevant feature.

The advantage of using parameters would be that the activation of a default master port allows to omit the instantiation of the internal error slave, which also reduces the number of master ports on each of the internal demultiplexers by one. Additionally, for a crossbar with a single master port, this would allow to entirely omit the demultiplexer at each slave port. This is especially relevant in networks with high ID widths, where demultiplexers are very expensive.

[andreaskurth]

@WRoenninger: What do you think?

[WRoenninger]

Yes, as you mentioned, the intend was to have the default ports runtime configurable. However I have not seen this feature being used dynamically yet and see the overhead that arises when trying to use it.

I am fine with changing it to a parameter. However this should probably be done after #157 as there is the whole config struct replacement going on.

Also this ties in with an issue I see with the addr_decode from common_cells. The recommended struct definition there is:

  typedef struct packed {
    int unsigned idx;
    addr_t       start_addr;
    addr_t       end_addr;
  } rule_t;

The main issue that can happen with this struct definition is that some tools do not like the int unsigned inside of a packed structure. For the default mst_port functionality it should not be an issue, as the decoder requires the cf_math_pkg::idx_width(NumIdx) typed index for its default mapping anyway, because the decoder does a typecast form addr_map_i.idx to this idx_t when it is checking the address map for a rule. I am wondering if it would be a good idea to recommend defining the addr_map_t.idx field to be of type idx_t or a set a fixed width logic in the example rules found in axi_pkg? So e.g. the rule xbar_rule_32_t would then read:

  typedef struct packed {
    logic [31:0] idx;
    logic [31:0] start_addr;
    logic [31:0] end_addr;
  } xbar_rule_32_t;

This struct definition change should not have any influence on the behavior of addr_decode and help with tool compatibility.

[andreaskurth]

However this should probably be done after #157 as there is the whole config struct replacement going on.

Okay, I agree.

I am wondering if it would be a good idea to recommend defining the addr_map_t.idx field to be of type idx_t or a set a fixed width logic in the example rules found in axi_pkg?

Yes, makes sense to me.