uart: BW improvements #4620
uart: BW improvements #4620
Conversation
read_char straightly use hw buffer (+ ~10%bw) read by block (+ ~190%bw) (instead of generic Stream::readBytes) attributes for functions called by ISR remove overrun message remove some ISR flags which were not honoured
|
@mribble @jasoroony Since you are dealing with low latency or high bandwidth serial setup, |
|
My FTP application doesn't seem to have any problem with this change. I also used the new readBytes method when sending file-chunks, works well, but it didn't improve the performance. With WiFi, SD-Card and Serial all in the mix I didn't really expect it to. |
cores/esp8266/uart.c
Outdated
| @@ -268,9 +314,16 @@ uart_start_isr(uart_t* uart) | |||
| // triggers the IRS very often. A value of 127 would not leave much time | |||
| // for ISR to clear fifo before the next byte is dropped. So pick a value | |||
| // in the middle. | |||
| USC1(uart->uart_nr) = (100 << UCFFT) | (0x02 << UCTOT) | (1 <<UCTOE ); | |||
| // update: with direct peeking and loopback test @ 3Mbauds/8n1 (=2343Kibits/s): | |||
There was a problem hiding this comment.
I don't understand the code changes below, or this explanation. Considering that you seem to have investigated this in detail, could you please explain the new code, and the meaning of the registers? (future reference)
There was a problem hiding this comment.
These are the UITO's brothers above. I will reenable them but treat the interrupt differently.
There was a problem hiding this comment.
error interrupts are now managed, Serial.hasRxError() tells if receive error occured.
cores/esp8266/uart.c
Outdated
| @@ -252,7 +298,7 @@ uart_isr(void * arg) | |||
| ETS_UART_INTR_DISABLE(); | |||
| return; | |||
| } | |||
| if(USIS(uart->uart_nr) & ((1 << UIFF) | (1 << UITO))) | |||
| if(USIS(uart->uart_nr) & (1 << UIFF)) | |||
There was a problem hiding this comment.
I don't understand this code change. Could you please add comments to explain this new condition?
There was a problem hiding this comment.
I dont understand why this flag (UITO = RX FIFO timeout) triggers buffer copy, it has nothing to do. It is the role of UIFF alone (hw-rx-fifo full of 100 chars trigger).
What this interrupt could trigger is a flag in our future global sanity register stating that there are decoding error on the uart line.
There was a problem hiding this comment.
Doesn't UITO trigger when the RX FIFO is not empty, but also not "full" and some time has passed. That would be used in simpler libraries that only allowed access to the software buffer and needed the ISR to copy them over.
|
@d-a-v Nice work with this, the code changes make a lot of sense. My comments are mostly some minor refactoring and inline comments to explain what is going on for future reference. |
|
I've been inspired by all this, somewhat.. I had already written a C program to calculate the actual baud rates generated by the two host MCUs I'm testing with (along with the third ESP8266 they talk to)... Comparing the bauds that came out equal... One doesn't like baud rates higher than 1.5MHz, but the other goes up to 3MHz (not mentioning names, because the "better" one in this case has massive jumps at this high end which in my mind makes them more equal, except in peculiar circumstances.. and ones that are hard to work out). I am confident I'm running into problems where reading a single character at a time (my 2 main test sketches that check each character is +1 from the last) are taking too long to deal with @ 3MHz. FTP also doesn't work, even with the readBytes method that improves performance... But don't fret, I think it was sending the file (I.E. ESP8266 reads the WiFi and sends Serial data - so it's the host MCU that I think has the problem there). But.. A simulated DoS attack with the test sketches at this high speed does make the ESP8266 lose track of things (even with a 4KB software buffer). So, I would agree, anything that makes a tight loop reading a single character faster might be useful (regarding the inline comment above for reading a single character). Speed wise, 2.3MHz -> 3MHz Serial makes the FTP daemon go from 145KB/s to 180KB/s. Just FYI. |
|
My test isn't really high bandwidth, but it does bursts of a few hundred bytes at a time. I checked with this change and it continues to work well. |
|
remove WIP from title |
…ses local tests, not real CI) script-comment new recipe.hooks.core.prebuild.3 (along with already commented .1 and .2) moved CI package test to be first on the test list remove 'set -x', wish me luck
read_char straightly use hw buffer (+ ~10%bw)
read by block (+ ~190%bw) (instead of generic Stream::readBytes)
attributes for functions called by ISR
remove overrun message
remove some ISR flags which were not honored