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

Possible to remove CPU-intensive spinlock? #9

Closed
stonewareslord opened this issue Jul 18, 2021 · 8 comments
Closed

Possible to remove CPU-intensive spinlock? #9

stonewareslord opened this issue Jul 18, 2021 · 8 comments
Assignees

Comments

@stonewareslord
Copy link

stonewareslord commented Jul 18, 2021

The spinlock in sleep_busy_waiting_ms takes 100% CPU power from one core of the pi. I am trying to replace it with something more efficient, with no success.

I tried replacing it with thread::sleep, but all the LEDs turn white, and I cannot figure out why that might be. The only difference I know of for these two methods is that the spinlock has ~2microsecond overhead while thread::sleep has ~100microsecond overhead. Full test commit: stonewareslord@f69279b; summary of change:

diff --git a/examples/src/bin/moving-pixel.rs b/examples/src/bin/moving-pixel.rs
--- a/examples/src/bin/moving-pixel.rs
+++ b/examples/src/bin/moving-pixel.rs
@@ -31,6 +34,28 @@ fn main() {
         adapter.write_encoded_rgb(&data).unwrap();
 
         i = (i + 1) % num_leds;
-        sleep_busy_waiting_ms(1000 / 10); // 100ms / 10Hz
+        thread::sleep(Duration::from_millis(ms));
     }
 }

I could maybe get that we need to fill the SPI buf at a consistent rate, but adding randomness to the sleep time has no effect (spinlock still works, thread::sleep still doesn't). Full test commit: stonewareslord@b12346e; summary of change:

diff --git a/examples/src/bin/moving-pixel.rs b/examples/src/bin/moving-pixel.rs
index e39faed..4d4447a 100644
--- a/examples/src/bin/moving-pixel.rs
+++ b/examples/src/bin/moving-pixel.rs
@@ -34,7 +36,7 @@ fn main() {
         adapter.write_encoded_rgb(&data).unwrap();
 
         i = (i + 1) % num_leds;
-        sleep_busy_waiting_ms(1000 / 10); // 100ms / 10Hz
+        sleep_busy_waiting_ms(1000 / 10) + rng.gen_range(0, 10); // 100ms / 10Hz
     }
 }

Is there some kind of timing issue I am not aware of here? I would expect the loop time after write_encoded_rbg should not matter and the only side effects of both of these functions is the sleep time.

P.S. Thank you for the amazing library!

@phip1611
Copy link
Owner

Hi @stonewareslord
well, good question. At this point, after adapter.write_encoded_rgb(&data).unwrap();, it should be indeed irrelevant, what kind of sleep/wait-function is used. Maybe I have later time to look into this.

@phip1611 phip1611 self-assigned this Jul 19, 2021
@stonewareslord
Copy link
Author

Thank you. You can just clone my repo and run cargo run --bin moving-pixel to test.

Maybe this is just some weird issue with my pi? I tried combining the two -- using thread::sleep for half the time and spinlock for the other half, and the lights break if I sleep for 50ms or more, but not <= 40ms. I didn't commit this, but if you wanted to try that, here's the diff:

diff --git a/examples/src/bin/moving-pixel.rs b/examples/src/bin/moving-pixel.rs
index 4d4447a..7cb5a2c 100644
--- a/examples/src/bin/moving-pixel.rs
+++ b/examples/src/bin/moving-pixel.rs
@@ -38,19 +38,18 @@ fn main() {
         i = (i + 1) % num_leds;
         let ms = 1000 / 10 + rng.gen_range(0, 10); // 100ms / 10Hz
         let before = Instant::now(); // For printing time this takes
+        let target_time = Instant::now().add(Duration::from_millis(ms));
 
         // Using thread::sleep() - DOES NOT WORK - lights turn solid white
-        thread::sleep(Duration::from_millis(ms));
+        // Works when duration <= 40ms; breaks when duration >= 50ms
+        thread::sleep(Duration::from_millis(40));
 
         // Original code - WORKS FINE
-        /*
-        let target_time = Instant::now().add(Duration::from_millis(ms));
         loop {
             if Instant::now() >= target_time {
                 break;
             }
         }
-        */
 
         let after = Instant::now();
 

@phip1611
Copy link
Owner

I hope I can look into this next week - sorry for the delay

@stonewareslord
Copy link
Author

stonewareslord commented Aug 2, 2021 via email

@phip1611
Copy link
Owner

phip1611 commented Aug 19, 2021

I finally found some time to investigate this. I can reproduce it

With thread::sleep OS gives control to another thread temporarily. I guess in this time, the SPI devices does odd stuff. I try to find a way to investigate.. if it is a linux bug or a bug in the spidev crate or whatever..

@Type1J
Copy link

Type1J commented Sep 24, 2021

This isn't a Linux bug. It's the SPI clock varying with the CPU. Try this:
If you're on an RPi3 add to /boot/config.txt:

core_freq=250

If you're on an RPi4 add to /boot/config.txt:

core_freq=500
core_freq_min=500

Reboot, and you should be able to use std::thread::sleep() without any LEDs turning white.

@Type1J
Copy link

Type1J commented Sep 26, 2021

I think that this is actually GPU speed, but allowing it to vary breaks several serial interfaces when not using a clock line, including SPI. The Raspberry Pi docs say to use these paramters to be able to use the serial interfaces.

phip1611 added a commit that referenced this issue Jan 9, 2022
@phip1611
Copy link
Owner

phip1611 commented Jan 9, 2022

I didn't really fix something here but added a note to README and the code
012db26

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

3 participants