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

Adding support for Pounder DDS QSPI profile stream #186

Merged
merged 24 commits into from
Dec 7, 2020
Merged

Adding support for Pounder DDS QSPI profile stream #186

merged 24 commits into from
Dec 7, 2020

Conversation

ryan-summers
Copy link
Member

@ryan-summers ryan-summers commented Dec 2, 2020

This is an initial draft PR of the proposed updates to the high-speed QSPI data stream to Pounder.

Current measurements indicate that each profile serialization + staging process currently takes 300ns for a profile of 12 bytes. The actual QSPI data write is only 75ns for 3 32-bit words, so there is room for future improvement when the layout of each profile is known in advance.

TODO:

  • Document new modules

Reserved for future PR:
Define a "static" layout for the pounder profile to avoid the overhead of the serialization process. This would allow us to copy bytes into an array at known offsets and do it really quickly.

This fixes #172 and addresses part of the IO architecture in #147

src/main.rs Outdated
);

// IO_Update should be latched for 50ns after the QSPI profile write. Profile writes
// are always 16 bytes, with 2 cycles required per byte, coming out to a total of 32
Copy link
Member Author

Choose a reason for hiding this comment

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

This is now configurable, but the limit is 16 bytes. This is only limited by the QSPI FIFO size and the size used in the ad9959 serializer, so it could theoretically change in the future and invalidate these numbers

@ryan-summers
Copy link
Member Author

DS4_QuickPrint14

Above is a capture of SCK going to the DDS and the IO_Update signal. As can be seen, IO_Update is properly toggling on for 50nS after the DDS profile has been written over SPI

_ => panic!("Invalid"),
}
unsafe {
core::slice::from_raw_parts::<'a, u32>(
Copy link
Member

Choose a reason for hiding this comment

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

I think for this you'd have to consume self here (and not indirectly in write_profile), no?

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue here is that we're returning a slice and not an array. The slice has to be owned by something to exist properly. If we consumed self to generate a [u32; 4] array here and then loaned a slice ref to that array, the array itself would actually go out of scope too soon (in this function return). Hence, we have to keep self alive for the entire lifetime of the u32 slice.

I've played around with it and haven't found a nicer way to move ownership around, but if you have an idea, feel free to suggest it

Copy link
Member

Choose a reason for hiding this comment

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

Instead of the slice, maybe return (move) the array and a length.

src/main.rs Show resolved Hide resolved
ad9959/src/lib.rs Outdated Show resolved Hide resolved
ad9959/src/lib.rs Outdated Show resolved Hide resolved
ad9959/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@irwineffect irwineffect left a comment

Choose a reason for hiding this comment

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

Should we use integer arithmetic for all the frequency math to avoid any issues of floating point inaccuracy? We start to see inaccuracies for f32 when dealing with any integer values greater than 2^24 (~16.7*10^6). Or do we not care?

ad9959/src/lib.rs Outdated Show resolved Hide resolved
ad9959/src/lib.rs Outdated Show resolved Hide resolved
src/hrtimer.rs Outdated Show resolved Hide resolved
src/pounder/dds_output.rs Show resolved Hide resolved
@jordens
Copy link
Member

jordens commented Dec 5, 2020

Should we use integer arithmetic for all the frequency math to avoid any issues of floating point inaccuracy? We start to see inaccuracies for f32 when dealing with any integer values greater than 2^24 (~16.7*10^6). Or do we not care?

For precision and metrology applications we'll need to be able to set and get frequencies without rounding errors. Setting requires that the user can pass in (ultimately) the desired frequency as a fraction p/q of the reference frequency (p,q integer). p contains the PLL multiplier and the FTW, q is the max FTW. Getting requires giving the user that fraction.
For internal DSP it's much better to do the processing in machine units anyway. And those are integers (e.g. the FTW).
But also for many non-precision and non-metrology applications it's a bad idea to have the user supply and process fractions (and deal with the constraints on those!). For those applications f32 in SI units will be sufficient and most convenient.
Then again even for some "harmless" applications (e.g. superhet or certain fiber length stabilizations) we need to deal with the exact frequency differences/ratios between two DDS channels. Also for those cases intermediate conversions from and to f32 SI would be fatal.

src/main.rs Show resolved Hide resolved
@jordens
Copy link
Member

jordens commented Dec 7, 2020

bors r+

@jordens
Copy link
Member

jordens commented Dec 7, 2020

Bors isn't behaving.

@jordens jordens merged commit 85b8f12 into quartiq:master Dec 7, 2020
@ryan-summers ryan-summers deleted the feature/qspi-stream branch December 7, 2020 17:58
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.

ad9959 little things
3 participants