Add grabber module #270

Merged
sb10q merged 1 commits from esavkin/artiq-zynq:264-fix-grabber into master 2023-10-17 15:42:29 +08:00
Owner

Waits for testing

Waits for testing
mwojcik reviewed 2023-10-12 18:12:22 +08:00
@ -229,6 +230,7 @@ class GenericMaster(SoCCore):
pads=data_pads,
clk_freq=clk_freq)
self.csr_devices.append("gt_drtio")
self.config["CLOCK_FREQUENCY"] = int(description["rtio_frequency"])
Owner

clk_freq, actually can also introduce that to standalone

``clk_freq``, actually can also introduce that to standalone
mwojcik reviewed 2023-10-12 18:13:33 +08:00
@ -11,10 +11,13 @@ extern crate alloc;
#[cfg(feature = "target_kasli_soc")]
use core::cell::RefCell;
use embedded_hal::blocking::delay::DelayMs;
Owner

Also should be behind has_grabber, otherwise will warn about unused import

Also should be behind has_grabber, otherwise will warn about unused import
mwojcik reviewed 2023-10-12 18:17:52 +08:00
@ -60,0 +64,4 @@
async fn grabber_thread(mut timer: GlobalTimer) {
loop {
grabber::tick();
timer.delay_ms(200);
Owner

I believe this should be a countdown like in 812aea33b3/src/runtime/src/rtio_mgt.rs (L425), otherwise this task will never yield. Will need to double check. If timing is not an issue and grabber tick can be done anytime, it could be a yield like the io expander task above.

I believe this should be a countdown like in https://git.m-labs.hk/M-Labs/artiq-zynq/src/commit/812aea33b3cae41f70d35d5ca8a6eb4a507fdbed/src/runtime/src/rtio_mgt.rs#L425, otherwise this task will never yield. Will need to double check. If timing is not an issue and grabber tick can be done anytime, it could be a yield like the io expander task above.
Owner

Seems to work fine according to the customer, after replacing the delay with an async-friendly version.

Just replace timer.delay(200) with:

        let mut countdown = timer.countdown();
        delay(&mut countdown, Milliseconds(200)).await;

then remove DelayMs import, and add libasync::delay and libboard_zynq::time::Milliseconds. Remember to use cargo fmt too.

This will have to be integrated in release-7 too.

I'm thinking if to cut down on tasks, we could integrate the expander service together with the grabber tick. But that's beyond of the scope of getting grabber to work.

Seems to work fine according to the customer, after replacing the delay with an ``async``-friendly version. Just replace ``timer.delay(200)`` with: ``` let mut countdown = timer.countdown(); delay(&mut countdown, Milliseconds(200)).await; ``` then remove DelayMs import, and add ``libasync::delay`` and ``libboard_zynq::time::Milliseconds``. Remember to use cargo fmt too. This will have to be integrated in release-7 too. I'm thinking if to cut down on tasks, we could integrate the expander service together with the grabber tick. But that's beyond of the scope of getting grabber to work.
esavkin force-pushed 264-fix-grabber from 26a2bfacf5 to 92b702da1b 2023-10-16 10:30:09 +08:00 Compare
esavkin changed title from WIP: Add grabber module to Add grabber module 2023-10-16 10:30:27 +08:00
esavkin force-pushed 264-fix-grabber from 92b702da1b to 7ffa897ae5 2023-10-16 10:31:08 +08:00 Compare
mwojcik reviewed 2023-10-16 13:36:31 +08:00
@ -139,6 +140,7 @@ class GenericStandalone(SoCCore):
]
fix_serdes_timing_path(platform)
self.submodules.bootstrap = GTPBootstrapClock(self.platform, description["rtio_frequency"])
Owner

and use clk_freq here, since you just defined it :)

and use clk_freq here, since you just defined it :)
esavkin force-pushed 264-fix-grabber from 7ffa897ae5 to b768d5648c 2023-10-16 14:35:25 +08:00 Compare
sb10q merged commit b768d5648c into master 2023-10-17 15:42:29 +08:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/artiq-zynq#270
No description provided.