DMA driven PWM implementation #193
Conversation
host/bcm283x/dma.go
Outdated
| // Memory mapped register | ||
| c.srcAddr = physToBus(srcAddr) | ||
| // BUG: EXPERIMENTING. | ||
| //t |= dmaSrcInc |
There was a problem hiding this comment.
To commit in master, please remove these.
host/bcm283x/dma.go
Outdated
| d.cs = dmaReset | ||
| // Clear bits if needed. | ||
| d.cs = dmaEnd | dmaInterrupt |
There was a problem hiding this comment.
clearing dmaInterrupt is probably (?) important.
There was a problem hiding this comment.
Not sure. I removed those lines because they did not make the channel isAvailable(). Simple dmaReset did work. Anyway, the code is not enabled yet. I leave dmaInterrupt and will test later whether it is harmful.
host/bcm283x/dma.go
Outdated
| @@ -763,6 +812,17 @@ func (d *driverDMA) Close() error { | |||
| return nil | |||
| } | |||
|
|
|||
| func ResetDMA(ch int) error { | |||
There was a problem hiding this comment.
The user shouldn't have to know about channels, so do not export this function.
There was a problem hiding this comment.
Made it private. Although it is not used anywhere, it is useful while debugging.
host/bcm283x/gpio.go
Outdated
| edge *sysfs.Pin // Set once, then never set back to nil. | ||
| usingEdge bool // Set when edge detection is enabled. | ||
| usingClock bool // Set when a GPCLK or PWM clock is used. | ||
| dmaCh *dmaChannel // Set when DMA is used for PWM. |
There was a problem hiding this comment.
One reason I'm unsure about making it GPIO specific is that we can affect multiple GPIO in one operation, for example reading multiple GPIO pins simultaneously.
There was a problem hiding this comment.
There are two options, 1 DMA for each channel or 1 DMA for multiple channels.
I think the former is simpler for PWM as long as we have enough DMA channels.
If we have a dedicated DMA channel, we need only two CBs for high and low durations and we can specify them independently.
If we share the channel, CBs will be complicated to accommodate different periods and duties.
For reading, we should share a channel with the highest sampling rate.
host/bcm283x/gpio.go
Outdated
| // | ||
| // PWM1 is exposed on pins 13, 19, 41 and 45. | ||
| // | ||
| // PWM0 and PWM1 share the same 25Mhz clock source. The period must be a | ||
| // divisor of 25Mhz. | ||
| // | ||
| // DMA driven PWM is aviable for all pins except PWM1 pins, its resolution is |
There was a problem hiding this comment.
Let's commit the working code without user visible changes first, then the user visible change afterwards.
There was a problem hiding this comment.
I reverted this file for this change.
Codecov Report
@@ Coverage Diff @@
## master #193 +/- ##
==========================================
- Coverage 86% 85.35% -0.66%
==========================================
Files 64 64
Lines 5251 5291 +40
==========================================
Hits 4516 4516
- Misses 647 687 +40
Partials 88 88
Continue to review full report at Codecov.
|
|
Thanks for the review. I think I have correctly updated the branch but I am still not familiar with github SxS diff tools. If I miss something, please let me know. |
host/bcm283x/dma.go
Outdated
| d.cbAddr = 0 | ||
| d.nextCB = 0 |
There was a problem hiding this comment.
I think it's better to clear this one too in case it was set.
host/bcm283x/dma.go
Outdated
| return nil, nil, err | ||
| } | ||
| u := buf.Uint32() | ||
| cb_bytes := uint32(32) |
There was a problem hiding this comment.
"_" in symbol names is not idiomatic in Go except rare circonstances.
No description provided.