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

SPI Engine: SDO data prefetch #1501

Merged
merged 4 commits into from
Jan 29, 2025
Merged

SPI Engine: SDO data prefetch #1501

merged 4 commits into from
Jan 29, 2025

Conversation

LBFFilho
Copy link
Contributor

@LBFFilho LBFFilho commented Oct 31, 2024

PR Description

Allows spi_engine_execution get sdo data independently of the instruction command. This reduces the trigger to transfer latency, and opens the door to further pipelining this path or reducing latency even more.

(this PR should be merged after #1499)

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the code style guidelines
  • I have performed a self-review of changes
  • I have compiled all hdl projects and libraries affected by this PR
  • I have tested in hardware affected projects, at least on relevant boards
  • I have commented my code, at least hard-to-understand parts
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe files, Copyright etc)
  • I have not introduced new Warnings/Critical Warnings on compilation
  • I have added new hdl testbenches or updated existing ones

@LBFFilho LBFFilho requested a review from sarpadi October 31, 2024 17:05
@LBFFilho LBFFilho force-pushed the spi_sdo_prefetch branch 5 times, most recently from 188d0a7 to 1840af0 Compare November 6, 2024 19:16
@LBFFilho LBFFilho force-pushed the spi_sdo_prefetch branch 2 times, most recently from e6937d4 to 4f982aa Compare November 14, 2024 12:50
@LBFFilho LBFFilho marked this pull request as ready for review November 14, 2024 13:04
@LBFFilho
Copy link
Contributor Author

changed it so the SDO data source is a parameter (synthesis-time), rebased on main

@LBFFilho
Copy link
Contributor Author

LBFFilho commented Dec 20, 2024

Tested on AD4052 with cora, same behavior as in main

@sarpadi
Copy link
Contributor

sarpadi commented Jan 17, 2025

ad4052_ardz.de10nano jenkins build fails with this error
ERROR: Quartus version mismatch; expected 22.1std.0, got 23.1std.1.

Copy link
Contributor

@gastmaier gastmaier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the formatting.
After sergiu review, make sure to squash the fixup commits.

@gastmaier
Copy link
Contributor

#1566 also bumps ad4052 required version.

@sarpadi
Copy link
Contributor

sarpadi commented Jan 29, 2025

"Formatting" commit needs to be split and integrated in existing commits.

LBFFilho and others added 4 commits January 29, 2025 09:29
SDO data can now be clocked in independently from the offload trigger.
This allows lower latencies for executing transfers, since the data can be
obtained from the DMA before the trigger. It also better separates the command
path from the data path.

SDO data source behavior is altered: previously, with SDO_STREAMING=1 the SPI
Engine would prioritize the SDO memory and switch to AXI Streaming when empty.
This unused automatic switching is now removed: if the SPI Engine is built with
SDO_STREAMING=1 it will only get SDO data from AXI Streaming, and with
SDO_STREAMING=0 it will only use the SDO memory.

Signed-off-by: Laez Barbosa <laez.barbosa@analog.com>
Signed-off-by: Laez Barbosa <laez.barbosa@analog.com>
Signed-off-by: Laez Barbosa <laez.barbosa@analog.com>
Signed-off-by: sarpadi <sergiu.arpadi@analog.com>
@LBFFilho LBFFilho merged commit 09c9182 into main Jan 29, 2025
3 of 5 checks passed
@LBFFilho LBFFilho deleted the spi_sdo_prefetch branch January 29, 2025 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants