Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of BlackBoxes - support for HLS? #61

Open
wzab opened this issue Mar 17, 2021 · 9 comments
Open

Better handling of BlackBoxes - support for HLS? #61

wzab opened this issue Mar 17, 2021 · 9 comments

Comments

@wzab
Copy link
Owner

wzab commented Mar 17, 2021

It is likely, that AGWB will be used with HLS-generated IP cores. There may be also other sources of blocks with predefined internal addresses. AGWB supports blackboxes, that can be defined as certain areas in the address space. For IPbus and AMAP-XML backends, the user may define the internal address map of the blackbox.
What may be needed, however, is a possibility to create the AGWB internal representation of the blackbox from the user-provided IPbus or AMAP XML. It would allow generating SW interfaces for all backends - C, Forth, Python...

@wzab
Copy link
Owner Author

wzab commented Mar 28, 2021

I have compiled a simple HLS project.
fir.cpp.gz

#include "fir.h"
static din_t delays[N];

void reset(void)
{
  int i;
  for(i=0;i<N;i++) delays[N]=0;
}

void wz_fir (din_t din, dout_t * dout, coeff_t gain, coeff_t coeffs[N], coeff_t offset)
{
 #pragma HLS INTERFACE axis port=din
#pragma HLS INTERFACE s_axilite port=coeffs
#pragma HLS INTERFACE s_axilite port=gain
#pragma HLS INTERFACE s_axilite port=offset
 #pragma HLS INTERFACE axis port=dout
  int i;
  for(i=N-1;i>0;i--) delays[i]=delays[i-1];
  delays[0] = din;
  accum_t accum = 0;  
  for(i=0;i<N;i++) accum += delays[i]*coeffs[i];
  *dout = (dout_t) accum * gain + offset;
}

The Vivado HLS produces the C "driver" files that contain information about asigned addresses:
There is a C header file
xwz_fir_hw.h.gz
which is hard to analyze, and the JSON file
solution0_data.json.gz
It contains a nice description of the generated AXI Lite interface

   "s_axi_AXILiteS": {
      "type": "axi4lite",
      "is_adaptor": "true",
      "mode": "slave",
      "port_prefix": "s_axi_AXILiteS",
      "param_prefix": "C_S_AXI_AXILITES",
      "addr_bits": "7",
      "registers": [
        {
          "offset": "0x10",
          "name": "gain_V",
          "access": "W",
          "reset_value": "0x0",
          "description": "Data signal of gain_V",
          "fields": [
            {
              "offset": "0",
              "width": "18",
              "name": "gain_V",
              "access": "W",
              "reset_value": "0",
              "description": "Bit 17 to 0 Data signal of gain_V"
            },
            {
              "offset": "18",
              "width": "14",
              "name": "RESERVED",
              "access": "R",
              "reset_value": "0",
              "description": "Reserved.  0s on read."
            }
          ]
        },
        {
          "offset": "0x40",
          "name": "offset_V",
          "access": "W",
          "reset_value": "0x0",
          "description": "Data signal of offset_V",
          "fields": [
            {
              "offset": "0",
              "width": "18",
              "name": "offset_V",
              "access": "W",
              "reset_value": "0",
              "description": "Bit 17 to 0 Data signal of offset_V"
            },
            {
              "offset": "18",
              "width": "14",
              "name": "RESERVED",
              "access": "R",
              "reset_value": "0",
              "description": "Reserved.  0s on read."
            }
          ]
        }
      ],
      "memories": {"coeffs_V": {
          "offset": "32",
          "range": "32"
        }},
      "ctype": {
        "AWVALID": {"Type": "bool"},
        "AWREADY": {"Type": "bool"},
        "WVALID": {"Type": "bool"},
        "WREADY": {"Type": "bool"},
        "BVALID": {"Type": "bool"},
        "BREADY": {"Type": "bool"},
        "BRESP": {
          "Type": "integer unsigned",
          "Width": "2"
        },
        "ARVALID": {"Type": "bool"},
        "ARREADY": {"Type": "bool"},
        "RVALID": {"Type": "bool"},
        "RREADY": {"Type": "bool"},
        "RRESP": {
          "Type": "integer unsigned",
          "Width": "2"
        },
        "AWADDR": {
          "Type": "integer unsigned",
          "Width": "7"
        },
        "WDATA": {
          "Type": "real fixed signed 14",
          "Width": "18"
        },
        "WSTRB": {
          "Type": "integer unsigned",
          "Width": "4"
        },
        "ARADDR": {
          "Type": "integer unsigned",
          "Width": "7"
        },
        "RDATA": {
          "Type": "real fixed signed 14",
          "Width": "18"
        }
      },
      "data_width": "32",
      "port_width": {
        "ARADDR": "7",
        "AWADDR": "7",
        "RDATA": "32",
        "WDATA": "32",
        "WSTRB": "4"
      }
    }
  },

Probably the JSON file may be used to extract the information needed to connect the IP core generated by HLS to AGWB.

@wzab
Copy link
Owner Author

wzab commented Mar 30, 2021

Handling of HLS-generated cores is tightly associated with supporting blackboxes with complex address maps.
Up to now, AGWB supports only providing the map of addresses for the blackbox that can be transparently passed to the IPbus XML or to AMAP XML.
Other backends can not use that information.
A better approach would be building for such blocks a similar description as for normal blocks, but with one difference: the addresses must be fixed.

The internal addresses are assigned in the analyze method.
It should be easy to disable it for certain block, requiring that all internal subblocks, registers (or vectors) must have fixed sizes and base addresses.

Then the "analyze" method should be called for children.
It can be done by adding a "fixed" attribute to the block. Each block that has this attribute must also have "addr_size" and "addr_base" attributes. The same applies to its registers. All subblocks of such block must be also "fixed".
The "analyze" method of "block" class should check the "fixed" attribute, and if it is set, it just copies the addresses and sizes from the corresponding attributes instead of allocating them (of course it should be verified, that the addresses fit into the declared (with "addr_size") address space.
Then the "analyze" method should be called for children.
The internal representation built for such "fixed" blocks by "analyze" will be the same as for normal blocks.

(Another possibility could be subclassing of "block" and creating a "fixed_block" class with modified "analyze" method? However it seems that it wouldn't be more legible and maintainable.)

The backends should treat such blocks as usual (all internal subblocks and registers have their addresses set. Doesn't matter that they were predefined, not allocated).
The only exception is the VHDL backend. It stops at generation of the bus for such block (just as it does for blackboxes now).

The next step would be of course a generator that takes the HLS JSON and converts it into the AGWB XML describing the fixed blocks. The important fact is that it can be separated from AGWB and won't increase its complexity.

@wzab
Copy link
Owner Author

wzab commented Mar 30, 2021

Of course the base addresses should be specified only for registers in "fixed blocks" and for its subblocks.
The size must be specified for each fixed block (so also for its subblocks).
The standard registers (ID, VER, TEST_DEV) should not be generated for fixed blocks.

@wzab
Copy link
Owner Author

wzab commented Mar 31, 2021

It seems that instead of "fixed" the new attribute should be called "external".

@wzab
Copy link
Owner Author

wzab commented Apr 2, 2021

The first version that seems to handle the "external" blocks properly is available in the repository:
a19d104
It has very relaxed requirements for the "externals" - the registers may be defined in any order, the size of the block does not have to be equal to 2^N.

The address maps and the Python service routines seem to be generated correctly.

@wzab
Copy link
Owner Author

wzab commented Apr 3, 2021

Regarding connecting the AXI-Lite slave to the AGWB-controlled Wishbone, we should check the xaxi4lite_wb_bridge.vhd bridge from OHWR general-cores.

@wzab
Copy link
Owner Author

wzab commented Apr 3, 2021

I have implemented a new way of handling the "external blocks with fixed internal structure". To enable better implementation of Relax NG-based syntax validation, the new approach does not use the "external" attributes. Instead of that the "external" blocks must use:

  • xblock instead of block (and it must have "size" attribute describing the size of the fixed block)
  • xsreg instead or sreg and xcreg instead of creg (and they must have "addr" atribute, that defines their address in the containing xblock)
  • xfield instead of field (and it must have the "lsb" attribute defining its position in the underlying register)
  • xsubblock instead of subblock (and it must have the "addr" attribute defining its address in the containing xblock)

Generally the implemenatation is working, but certain errors are found in the RelaxNG syntax validation:

Investigating them I have found that even in the master branch the NG validation sometimes works incorrectly:
#62

@wzab
Copy link
Owner Author

wzab commented Apr 3, 2021

I have solved the problem by switching to DTD-based validation in 62267d6 .
The DTD may be generated from RNG with trang. I have added the rng2dtd.sh script for that:
https://github.com/wzab/agwb/blob/62267d6cf30554e62aa4fec40bf49067892bab96/src/rng2dtd.sh

@wzab
Copy link
Owner Author

wzab commented Apr 3, 2021

The problem with RNG validation has been reported to the Debian (in which I've detected the problem): https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=986343

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant