micropython/micropython-esp32

Hardware SPI imposes a fixed-length buffer

mattytrentini opened this issue · 14 comments

It appears that there's a fixed-length buffer when using HW SPI (note: SW SPI works fine):

# Start with SW SPI
spi=SPI(baudrate=1000000, mosi=Pin(23, Pin.OUT), sck=Pin(18, Pin.OUT), miso=Pin(19, Pin.IN))
spi.write(b'\x00'*4092) # OK
spi.write(b'\x00'*4093) # OK

# Now for HW SPI
spi=SPI(1, baudrate=1000000, mosi=Pin(23, Pin.OUT), sck=Pin(18, Pin.OUT), miso=Pin(19, Pin.IN))
spi.write(b'\x00'*4092) # OK
spi.write(b'\x00'*4093) # Error!
E (262116879) spi_master: spi_device_queue_trans(620): txdata transfer > host maximum

The error is raised in the ESP-IDF in spi_master.c:636. The variable max_transfer_sz, which specifies the maximum length of the transmission, appears to be specified in spi_master.c:154 but seems dependent on DMA configuration.

I'm not sure how configurable the buffer lengths need to be (it may be helpful to specify different length buffers - shorter for low memory usage or longer for perhaps better throughput and lower power?) but, regardless, it seems that write should just work, dealing with stepping through multiple buffers internally as need be.

it seems that write should just work, dealing with stepping through multiple buffers internally as need be

Yes I agree, HW SPI should just work for an arbitrary length buffer.

I don't know if the ESP-IDF would consider doing it on their side (ie splitting up the write if it's too big), but if they don't then we would need an IDF function that can query the maximum length so the buffer can be split correctly. Does such a function already exist?

Beginning work on this.

@mattytrentini Can you be my tester? I don't have any devices that need that sort of SPI xfer size.

Sure @MrSurly, no worries, happy to help.

BTW note that you don't need any device to be connected for the error to occur - you can just send a >4092 byte buffer to an instance of a HW SPI. But I'll verify that data that was expected to be sent was actually sent...

And I've been meaning to investigate after @dpgeorge's comment. The buffer size is available in max_transfer_sz (for each of spihosts) though access to it is only via the spi_host_t structure. And the idea to address the issue inside the ESP-IDF has a lot of merit...

If it's something which makes sense to fix upstream (because it's relevant beyond MicroPython), and it's in the visible bits of ESP-IDF, then I think it makes sense to make it a PR there to at least get Espressif's feedback.

This is a good point. I think I'll raise an issue first; sometimes their attitude is "not a bug!" -- wouldn't want to waste the effort.

But I'll verify that data that was expected to be sent was actually sent...

Right, I can "dry-test" of course, but I want to make sure it works for your specific use-case.

And I've been meaning to investigate after @dpgeorge's comment. The buffer size is available in max_transfer_sz (for each of spihosts) though access to it is only via the spi_host_t structure.

Unfortunately: static spi_host_t *spihost[3];

And the idea to address the issue inside the ESP-IDF has a lot of merit...

It does.

@mattytrentini
https://github.com/MrSurly/micropython-esp32/tree/issue_225

I also fixed up the actual number of bits are to be sent, based on the SPI "bits" (per word) parameter, combined with the actual number of bytes in the data. Testing this is as simple as just setting the bits parameter to something like 4 or 12 (with some zero-byte buffering at the end), and seeing if it still works.

Submitted an issue to ESP-IDF
espressif/esp-idf#1329

Okay, so Espressif's answer was extremely terse, but what it meant is that you're supposed to set the max transfer bytes when you create the SPI object. Since there's about 0% chance of changing the µPy API to include this, and an equal chance of code changes by Espressif, I think we need to do chunking under the hood.

Edit: Now that I look more closely, I had assumed that @negativekelvin worked for Espressif, but I think I'm mistaken. In the end, my conclusion is the same.

what it meant is that you're supposed to set the max transfer bytes when you create the SPI object

Can't you set the max to "infinity", something like 0xffffffff?

Can't you set the max to "infinity", something like 0xffffffff?

It allocates a 12-byte structure[1] for every DMA descriptor it thinks it needs. Basically transfer_size_in_bytes / 4094 * 12. For 0xffffffff that would be about 12MiB. This leads to "what's a 'reasonable' size?" Hard to answer that one.

[1] Assuming it's doing 4-byte boundaries, and rounding the 9-byte structure up to 12.

For 0xffffffff that would be about 12MiB.

In that case I agree with you that the chunking should be done transparently via uPy.

This was fixed upstream in commit micropython/micropython@9123c8d