Smoother PWM Behavior #76
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the first step towards getting better PWM smoothness, which is to separate out the PHY layer from the behavioral layer that sits right on top of it. By doing this, we can now share a little bit of PHY state between each RaspberryPWM instance. This currently disables the DMA-portion, and breaks the pattern PWM functionality. This is because each pin can and should have its own DMA channel, and needs its own DMA address pointer to work correctly. This is normally done in initPWM(), and I need to figure out the best solution for handling this piece going forward. It will probably involve recording all the DMA addresses when we init the phy, and letting each PWM get the channel they need from the PHY.
This change should restore the DMA behavior after the refactoring to extract the PHY out. When initializing the PHY for the first time. Even though we are getting information for more than one DMA channel on initialization now, the offset cost of not having to memmap /dev/mem for every channel helps here, if you need more than one DMA channel.
Mostly the goal here is to re-use the killClock() function in the RaspberryPHY class so that when the PWM Pattern starts, it properly signals to the basic PWM side of things that it needs to reinitialize the clock before taking over the PWM channel. Because the PWM Patterns don’t operate independently yet, like the PWM mode now does, don’t bother coalescing on stopPWM().
It doesn’t make sense to a cache a PHY for each base address. This isn’t a real scenario. Instead, let’s just cache the last PHY we create, and invalidate the cache in the case of a mismatch.
A couple problems cropped up here, where the period wasn’t actually adjusted properly. It assumed the period was in ns, but it is in slots. Each slot is 4 ns. Account for that. Also, don’t lose precision when calculating very slow frequencies in the realm of 15Hz.
Kaiede
commented
Sep 6, 2018
| // | ||
| // Double precision is used here to handle very low frequencies in the range of <15Hz. | ||
| // Otherwise we lose precision calculating data. | ||
| let range = UInt(max((Double(ns) / 4.0).rounded(.awayFromZero), 1)) |
There was a problem hiding this comment.
This part is updated from the original PR. It fixes the goofy "must have a 750ns range" behavior, and also fixes a goof on my part that I noticed where the range is assumed to be in nanoseconds, but the clock is running at 4 ns per tick.
I'm still testing this particular change, as it means my frequencies are about 1/4th what I thought they were. I'll follow up once I am satisfied this is working the way I think it should be working.
|
I merged these changes to PWM with version 1.3.7 on my fork. I had to fix a few types so it would work in 64 bits. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's in this pull request?
Changes how the PWM hardware is controlled in PWM.swift to make it possible to drive both channels at the same time and rapidly be able to change the duty cycle (pulsing, ramps, etc) on the PWM channels without excess flickering.
Why a New Request?
I originally pulled from my fork's master instead of the work branch. This one uses the work branch, so it doesn't accidentally include unrelated changes, like my aarch64 fixes.
Is there something you want to discuss?
This PR doesn't attempt to modify the pattern writing behaviors at all, and only attempts to clean things up so that when going into pattern writing, you get the existing behavior, while the simpler PWM control gets the new behavior, with a bit of work tracking which mode the PHY is currently in. Unfortunately, I don't know how successful this change is.
Pull Request Checklist