[DMA 3/8] Remove ChannelCreator types, temporarily disable burst mode

[bugadani]

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

• I have updated existing examples or added new ones (if applicable).
• I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
• My changes were added to the CHANGELOG.md in the proper section.
• I have added necessary changes to user code to the Migration Guide.
• My changes are in accordance to the esp-rs API guidelines

Extra:

• I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

Description

This PR removes the ChannelCreator types, and instead provides the initialized DMA channels immediately. The PR further removes the channel-wide burst mode setting, and pushes the responsibility to the DMA buffer APIs. As a consequence, some peripherals currently don't support burst DMA transfers.

[bugadani]

Draft for a reason, I want to see where CI breaks.

[MabezDev]

I'm liking the direction of this a lot, I think if we choose to treat channels like peripherals and apply the same config method to drivers (whatever that may be from discussions in #2416) it should work quite nicely I think.

[bugadani]

I'm not happy that Channel/ChannelTx/ChannelRx would need to become Peripheral instances. The problem is that they currently have mutable state (as in, their configuration) and I'm not sure how that interacts with PeripheralRef. Most likely in a subtly surprising way because into_ref clones the inner peripheral. That's not a big deal as peripherals don't unusally configure the DMA channel, but once they do, releasing the channel would mean that the config changes become lost.

[bugadani]

My idea is to move burst mode config to Tx/RxBuffer (and consequently, Preparation if I need to, although burst length is still nebulous to me a bit). This allows us more flexibility (burst mode becomes a property of the transaction, essentially), and it allows us to remove in-memory configuration from the DMA channels.

[Dominaezzz]

Did you manage to figure out the bandwidth thing?
i.e. Are there any consequences for running all 10 DMA channels in burst mode at the full 40 MB/sec? (Or 5 channels at 80MB/sec)

If there are consequences, then (long term) users need some way to turn it off in some subset of channels and/or transactions.

EDIT: For future reference, I found this.

[bugadani]

Did you manage to figure out the bandwidth thing?

No, I mean I'd put this into the user's hands, so that they can enable burst transfer of buffers. There is very little clear documentation what burst mode actually means for internal SRAM, so I'd try not to be more clever than necessary.

i.e. Are there any consequences for running all 10 DMA channels in burst mode at the full 40 MB/sec? (Or 5 channels at 80MB/sec)

Only a single transfer can happen at any one time anyway, I think burst mode just prevents the arbiter from giving bus access to a different channel. I assume the DMA takes care of using aligned word access if it can, and I hope it doesn't need burst mode to be enabled for it.

[bjoernQ]

Cannot comment on the line:

There is the ChannelCreator struct left in gdma.rs

esp-hal\src\dma\gdma.rs
574:pub struct ChannelCreator<const N: u8> {}

[MabezDev]

LGTM, glad to see the creator types go!

[bjoernQ]

Thought I already approved - apparently not. Looking forward to the next PRs