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

modules/zstd: Rework ZSTD Decoder #1654

Draft
wants to merge 38 commits into
base: main
Choose a base branch
from
Draft

Conversation

lpawelcz
Copy link
Contributor

@lpawelcz lpawelcz commented Oct 9, 2024

This PR reworks the existing ZSTD Decoder to memory-based approach instead of relying on stream-based architecture.

The PR relies on peripherals (MemReader and MemWriter) for accessing external memory through AXI interface that are introduced in: #1613. It is also based on #1616.

The Decoder is able to decompress ZSTD frames that consist of RAW and RLE blocks. This is tested in a verilog simulation with a cocotb testbench.

The changes in this PR include:

  • Rewriting ZstdDecoder proc to work with memory-based approach
  • Adding verilog-level tests of the ZstdDecoder
    • Introducing python library for generating ZSTD frames with decodecorpus
    • Adding python bindings for ZSTD reference library (zstandard) used for crosschecking decoding results in the simulation
    • Adding cocotb testbench that generates multiple ZSTD frames with RAW and RLE blocks and decodes those with simulated ZstdDecoder
  • Reworking FrameHeaderDecoder, BlockHeaderDecoder, Raw- and RleBlockDecoders to use memory-based approach instead of streaming input data from the input of the decoder
  • Adding CsrConfig and AxiCsrAccessor procs that implement the internal registers of the ZstdDecoder and provide external (AXI) and internal (native) facing interfaces for accessing those
  • Simplifying:
    • RleBlockDecoder proc
    • BlockHeader library
  • Removing components not used in memory-based approach:
    • magic number functions
    • BlockDecoder proc
    • DecDemux proc
  • Added common_codegen_args list to the BUILD file

This PR is currently a WIP. It is opened to showcase the work done and to create a space for discussion on certain topics. The following list enumerates task that are currently being addressed and which are required for this PR to be completed before it is marked as ready for review.

Next steps:

  • Update documentation - describe the new architecture of the decoder and how to interact with the IP from the software (CSRs)
  • Add DSLX tests for the ZstdDecoder proc - create facilities for preparing test data and write DSLX tests
  • Improve error handling - write error codes to the Status CSR
  • Improve verilog tests - reorganize testbench to handle decoding of multiple ZSTD frames in one test case
  • Change the output format from stream-based to memory-based (write decoded frame to memory)
  • Organize structs and new type aliases (cleanup)
  • Remove a workaround (85f1f80) for type check failure during IR generation for the ZstdDecoder proc (TypeCheck failure for a parameterized proc #1655)

CC @proppy

@lpawelcz
Copy link
Contributor Author

Updated the PR with:

  • DSLX tests for the ZstdDecoder
  • Improved error handling - propagating status responses from internal procs to the Status CSR
  • Handling of the Reset CSR
  • Updated README

Also rebased the code onto current main

CC @proppy

@lpawelcz lpawelcz force-pushed the zstd_decoder branch 3 times, most recently from 855c65c to bba8a27 Compare November 18, 2024 15:15
koblonczek and others added 21 commits November 22, 2024 11:06
- XLSStruct for easier handling and serializing/deserializing XLS structs
- XLSChannel that serves as a dummy receiving channel
- XLSMonitor that monitors transactions on an XLS channel
- XLSDriver that can send data on an XLS channel
- LatencyScoreboard that can measure latency between corresponding transactions on input and output buses
- File-backed AXI memory python model

Internal-tag: [#64075]
Signed-off-by: Krzysztof Obłonczek <koblonczek@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authred-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authred-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Michal Czyz <mczyz@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Maciej Torhan <mtorhan@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Maciej Torhan <mtorhan@antmicro.com>
Remove references to buffer structs as those are not used anywhere

Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authored-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authored-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Maciej Torhan <mtorhan@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Krzysztof Oblonczek <koblonczek@antmicro.com>
Signed-off-by: Maciej Torhan <mtorhan@antmicro.com>
Co-authored-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Maciej Torhan <mtorhan@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authored-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authored-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Maciej Torhan <mtorhan@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Co-authored-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
lpawelcz and others added 17 commits November 22, 2024 11:09
This reverts commit 04ad379225b706ddf492d440c673e77348d7a409.
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Internal-tag: [#67096]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Maciej Torhan <mtorhan@antmicro.com>
Signed-off-by: Krzysztof Obłonczek <koblonczek@antmicro.com>
Signed-off-by: Krzysztof Obłonczek <koblonczek@antmicro.com>
Internal-tag: [#66955]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
* Fix byte ordering when receiving a series of non-empty packets
* Adjust MemReader DSLX tests

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
…es function into a separate proc

* Extract the operation of removing not-strobed bytes from input frames
  to a separate proc
* Extract control logic to AxiStreamRemoveEmptyInternal proc
* Optimize strobe calculation

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Fix paramaterization of the proc

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
…ackets

Add AxiStreamRemoveEmpty proc to the processing pipeline. It removes non-strobed
bytes from the input AXI Stream frames and forms full frames (ensures that only
the last input data packet won't be full).

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
* Extract control logic to MemWriterInternal proc
* Create alias for the MemWriter response type

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
SequenceExecutor:
  * Add output channel in the format compliant with MemWriter input data channel type

ZstdDecoder:
  * Add MemWriter proc:
    * Write request formed based on the address of the OutputBuffer CSR and
      FrameContentSize field from the Frame Header
    * Data to write is sent out to the proc by the SequenceExecutor
  * Transition to the FINISH state (and triggers notify channel) only after
    receiving the response from the MemWriter
  * DSLX tests:
    * Receive decoded data sent out on the AXI interface by the MemWriter proc
    * Mock the output memory buffer as a DSLX array
  * Cocotb tests:
    * Move third-party verilog modules (AXI Interconnect) to external directory
    * Replace AXI Interconnect with AXI Crossbar that handles simultaneous AXI Read
      and Write transactions
    * Add reference memory and fill it with expected data for comparison
      against testbench memory at the end of the decoding

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
* Remove Repacketizer proc
* Remove stream-based output channels from
  * SequenceExecutor
  * ZstdDecoder

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
* Decode multiple ZSTD frames in a single cocotb testbench
* Add one cocotb testbench per type of the ZSTD frames:
  * Frames with RAW blocks only
  * Frames with RLE blocks only
  * Frames with Compressed blocks only (disabled)
  * Frames with mixed blocks (disabled)

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
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

Successfully merging this pull request may close these issues.

5 participants