esp-rs / esp-hal

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

RMT API no wait #1749

Open fridolin-koch opened 1 month ago

fridolin-koch commented 1 month ago

Hi,

I'm currently porting https://github.com/Xinyuan-LilyGO/LilyGo-EPD47 to rust using esp-hal. I managed to implement a basic working poc.

While replicating the RMT behavior, I noticed that there are some hardware specific operations that are rather timing sensitive. The original code uses rmt_write_items with wait_tx_done set to false.

I was able to replicate this behavior, but I don't think the solution is very elegant.

As long as I can wait for the tx to complete, I'm able to reuse the TxChannel using the result of SingleShotTxTransaction::wait().

If I can't wait due to timing constraints, I have to re-create the channel the next time I want to use it. That feels rather inefficient and happens quite often (once for each row of the epd).

Hence I was wondering if it would make sense to add something like

pub fn nowait(self) -> C to SingleShotTxTransaction (or channel(self))

This would allow to reuse the channel without waiting for the tx.

I guess it would also make sense then to add something like is_busy() or is_done() to the channel (Not sure what would happen if I transmit more data while another tx is still in progress).

Disclaimer I'm neither a hardware nor a rust expert. This is basically my second rust project and was meant as a learning project. So there might be a chance I'm missing one or two things..

Best Regards, Frido

bjoernQ commented 1 month ago

Thanks for raising this issue. I'd say yes something like this would make sense for the case when the amount of data is less (or equal) than the FIFO size. In that case we could just fill the FIFO and the user can check the status before starting another write.

For that we should probably introduce a new type like SingleShotTxTransactionFittingFifo and a new function like transmit_fitting_fifo (I don't like that naming - just to illustrate things)

fridolin-koch commented 1 month ago

I was curious and took a crack at it:

I tested it and it seams to work on my ESP32S3

Here is my fork: https://github.com/esp-rs/esp-hal/compare/main...fridolin-koch:esp-hal:main

Some notes:

In case this is at least somewhat useful, I'm happy to send it as a PR :)

tschundler commented 1 month ago

Related, I'm also playing with the RMT driver. In my case, I want it to take an iterator, #1768 - this way I can translate data for the bits of LED colors without needing a large buffer.

I wonder if that would be a simpler solution for your use case of sending a large amount of data via RMT? I'm also looking into async support of the iterator, so you don't have to fully stop while sending from the iterator.

(my in-progress work is https://github.com/esp-rs/esp-hal/compare/main...tschundler:esp-hal:async-led-iterator)

The challenge is that if the iterator can't generate data fast enough to keep the RMT buffer filled, it will just loop through whatever is in the buffer, resending it. So if you want to use the iterator to send and then pause as you build the next line, you'd need to fill the RMT's ring buffer with transmitting all 0s or something.

tschundler commented 1 month ago

Ok, reading your ed047tc1.rs, what I'm proposing is probably not useful for how you are using RMT.

I think you can do what you want without needing anything new. But I can see where single-shot / no wait is handy.

With current RMT, You could use store the channel or the transaction both in your rmt struct. If there is a transaction, .wait() on it to get the channel at the start of the new pulse, where you busy loop now.

enum TXCh {
   #[default]
   None,
   Channel(TxChannel),
   Txn(SingleShotTxTransaction),
}

impl TXCh {
  fn take(&mut self) -> Result<TxChannel, error> {
    match mem::take(self) {
      None => panic!("very broken"),
      Channel(ch) => Ok(Ch)
      Txn(txn) => txn.wait()
    }
  }
}

If you do your design, updating rmt hal, maybe the busy wait should be in the hal rmt driver when starting a transaction, since wouldn't any user need to use that?

fridolin-koch commented 1 month ago

I wonder if that would be a simpler solution for your use case of sending a large amount of data via RMT? I'm also looking into async support of the iterator, so you don't have to fully stop while sending from the iterator.

I'm not sending a large amount of data. The rmt is used to control the gate clock (as far as I understand the datasheet/schematics). So I'm only sending 2 pulses which should result in 2 x 32bit written to the rmt ram (Which should be 48x32bit per channel, at least that's how I understand the code / technical reference).

Async was also my first approach, but that messed up the timing completely and resulted in garbage output on the screen, so I discarded that idea. I can't rule out that it was due to an implementation error on my side.

The challenge is that if the iterator can't generate data fast enough to keep the RMT buffer filled, it will just loop through whatever is in the buffer, resending it. So if you want to use the iterator to send and then pause as you build the next line, you'd need to fill the RMT's ring buffer with transmitting all 0s or something.

Probably because Wrap Tx Mode is enabled? (See pp. 1399 of the technical reference). Maybe try disabling it?

With current RMT, You could use store the channel or the transaction both in your rmt struct. If there is a transaction, .wait() on it to get the channel at the start of the new pulse, where you busy loop now.

That sounds like a good solution! Thanks!