esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
680 stars 189 forks source link

Confusion about usage of I2S as DAC DMA unit #1356

Open Slamy opened 5 months ago

Slamy commented 5 months ago

Howdy!

I would like to use the DAC of the ESP32 in DMA mode. The dac.rs itself only supports direct writing to it but I guess that is fine for some use cases. According to the documentation in the reference manual, the DAC is driven by a special mode of the I2S0 DMA. I tried to wrap my head around it but I fail to understand how i2s.rs actually works.

The i2s peripheral from the PAC seems to be consumed by I2s::new_internal and is gone afterwards. I lack the understanding how I::* is even able to use it afterwards and where it is dropped or even dropped at all. My understanding of PhantomData might not be the best right now.

To be more concrete about my problem. I would like to use the I2S in DAC DMA mode using the LCD flag and with my current skills I would have to throw i2s.rs of the HAL crate away and directly use the PAC. As this might be not the best technical solution I'm asking for your guidance here in pursuit of more knowledge on how to handle this situation.

MabezDev commented 5 months ago

To be more concrete about my problem. I would like to use the I2S in DAC DMA mode using the LCD flag and with my current skills I would have to throw i2s.rs of the HAL crate away and directly use the PAC. As this might be not the best technical solution I'm asking for your guidance here in pursuit of more knowledge on how to handle this situation.

The correct way to add the functionality would be to modify the driver here to add DMA DAC mode, after all like you said we don't want to throw away what we currently have! :D.

I don't have a clear design for you to follow, but I can possibly give some hints as to what this API would need to do.

I2S has many modes, LCD mode, Camera mode, DAC mode, and of course, the default I2S output mode. We currently only support the latter, so we need some way to put the I2S instance in the correct mode for the specific application.

Once we have that figured out, we will need some way to extend the DAC driver to use our newly DMA capable I2S interface. I don't have a clear idea here, one tried and tested is how we manage it for SPI DMA. Our normal Spi instance has a method, Spi::with_dma(/* DMA stuff here*/). We could have a similar interface for DAC, something like with_dma which would take in a configured I2S instance.

On top of the design of the API, there are some other considerations you should take into account. Only I2S0 can be used as a DMA DAC driver, I2S1 doesn't work, so we need to check this in the driver.

I'd be happy to help you with adding this contribution if you're willing?

Slamy commented 5 months ago

I've read a little bit from the esp-idf code and also the considerations I could find inside the the reference manual. The result is this branch here. Like you said, the DAC driver needs some extension too. I don't know much about HAL design as everything needs to fit together and we would also like to use the type safety of rust to ensure that everything is correct. For the DAC the simplest way would be another function to just activate the DMA mode.

For I2S the only differences I can find so far are to the configure function. This one could either get a parameter or a different configure function could be added. What I would consider possible is the extension of pub enum Standard. Right now it has only Philips, but the DAC could be an entry in that enum. The standard is passed through to configure... hmmm... that could work. I think I'm adding another commit on top.

Slamy commented 5 months ago

Multiple things are going wrong right now and I'm not sure about everything here. 1) I2sTx has the function fn wait_tx_dma_done(&self) -> Result<(), Error> but it doesn't perform any checks on the DMA and instead goes for I2S_TX_IDLE. The documentation of the ESP32 is very awful here as it is unclear whether this bit might be used in DAC mode or if it is restricted to the real I2S mode. It might be required to go for the DMA channel and check the state there to actually be sure that everything was sent.

2) The one shot DMA mode seems to be not tested. There is no example for it and T::reset_tx(); inside start_tx_transfer kills the second DMA transfer. I'm not sure yet if this is related to DAC mode or a general issue.

3) This is not related about the hardware or the algorithms inside the HAL. This might be a Rust problem. The following line is not possible

static EVIL: Mutex<RefCell<Option<I2s<I2S0, I2s0DmaChannel, Blocking>>>> = Mutex::new(RefCell::new(None));

This is also getting pretty clear when investigating the examples. Nothing DMA related is using interrupts right now or maybe I've missed it. The compiler seems to be unhappy about the memory pointers here. I haven't yet mastered the Rustonomicon so I don't know how this can be fixed.

error[E0277]: `*mut u8` cannot be sent between threads safely
error[E0277]: `*mut DmaDescriptor` cannot be sent between threads safely

There are multiple approaches I assume. My current one is still avoiding embassy and going for synchronous. My goal was to have a one shot DMA with an interrupt to provide the next data and the last time I've checked this requires putting something inside a Mutex<RefCell<Option<>>>.

I hope I don't appear to angry here. I really like the idea behind Rust, but sometimes the experience is painful and I don't know how to progress. I have this problem especially with embedded Rust.

bjoernQ commented 5 months ago
  1. We don't rely on the DMA only here since from DMA's perspective the transfer is already done while I2S is still draining it's FIFO which would result in incomplete transfers

  2. I checked and can confirm that non-circular mode doesn't seem to work for ESP32. It works for other chips - e.g. ESP32-C3. Seems it's an ESP32 (and probably ESP32-S2) specific bug

  3. DMA interrupts are not that useful currently. We will most probably need a changed or new API. See https://github.com/esp-rs/esp-hal/pull/1300

It's possible to register an interrupt handler for a blocking channel but it's not really useful, yet. However, that wasn't really possible before and is something for a future enhancement. (And would need some new/additional API probably)

Is there a specific reason you cannot use circular transfers?

MabezDev commented 5 months ago

https://github.com/esp-rs/esp-hal/pull/1375 should solve your 2nd issue.

Slamy commented 5 months ago

I hadn't yet time to check whether the I2S FIFO is used at all in DAC mode. Maybe that would explain why the behavior is different here. But I guess I should at least explain why I'm not going for circular transfers here to avoid that I try to achieve something that is unexpected or weird. My goal is to build a scope clock in Rust without esp-idf and I want to use the 2 DACs as XY deflection. I'm not yet sure about the sample rate but it might be higher than usual. The signal delivered by the DACs is periodic but needs some pauses on the way. A Polygon is drawn and then the beam needs to be disabled. My goal is to use the IRQ to disable the beam, set the next buffer and continue. The buffers are the basic shapes drawn on the screen. I see the one shot mode better fitting with this application but I'm curious for your thoughts.

uxtechie commented 4 months ago

Does this mean that at the moment we cannot sample an analog audio signal with esp-hal-rs? Specify a sampling frequency and have the samples stored in a DMA buffer?

I think it's a fairly common requirement... In my application, I need that functionality :(

Other questions: Can an external source be used as a sampling clock? What is the maximum buffer size in circular mode?

I think Rust for embedded systems is fantastic.

Thanks everyone for the effort, I personally think it's a great initiative for the community.

bjoernQ commented 4 months ago

Yes, at the moment the I2S driver is only I2S master supporting Phillips standard - so currently doesn't support an external clock

I think the max buffer size is around 32k - probably you want less to reduce latency.

I think Rust for embedded systems is fantastic. Thanks everyone for the effort, I personally think it's a great initiative for the community.

❤️

uxtechie commented 4 months ago

Thanks @bjoernQ for your quick response.

Is there currently any way to sample an analog audio signal specifying an internal clock frequency, using internal ADC hardware?

Thanks in advance.

bjoernQ commented 4 months ago

We don't have that implemented, yet