DMA support for SDcard

[IgorLektorovEpam]

It fixes possible troubles when irq disabled during SDcard writing. Tested with PYBV3. Need testing with other boards. Additional MICROPY_HW_HAS_SDCARD_DMA was included to "boards/PYBV3/mpconfigboard.h" for DMA/nonDMA swithing. Can be removed in future.

[dpgeorge]

Thanks for the patch, but there are some things that need improving:

1. Whether the SD card uses DMA or not should be private to sdcard.c. There should be functions called "sdcard_read_blocks" and "sdcard_write_blocks" and they should be configured to either use DMA or not.
2. DMA usage should be dynamically selected depending on whether irqs are endabled (use DMA) or disabled (don't use DMA). See i2c.c for an example of how this is done.
3. The 2 DMA streams/channels should probably be exclusively set aside for SD card use (ie not shared with any other peripherals like I2C or SPI). I'd need to check what streams can be used and what are already used.

[dhylands]

DMA usage should be dynamically selected depending on whether irqs are endabled (use DMA) or disabled (don't use DMA). See i2c.c for an example of how this is done.

Does that make sense for sdcard? Aren't some sdcard operations triggered from a timer? Which means interrupts would be disabled?

[dpgeorge]

Aren't some sdcard operations triggered from a timer? Which means interrupts would be disabled?

Exactly: if irqs are disabled then we can't use DMA (because it relies on an irq to signal that it's finished; probably this could be eliminated... but it's not easy).

So the read/write functions need to check if irqs are disabled, and if they are they must use polling (the existing implementation).

[dhylands]

Aren't some sdcard operations triggered from a timer? Which means interrupts would be disabled?

Exactly: if irqs are disabled then we can't use DMA (because it relies on an irq to signal that it's finished; probably this could be eliminated... but it's not easy).

So the read/write functions need to check if irqs are disabled, and if they are they must use polling (the existing implementation).

I guess I still disagree. I view sdcard operations as long operations (> 1 msec). There is no reason why a DMA operation can't be initiated while interrupts are disabled. Especially since it won't be finished for quite a while in the future.

Otherwise, you're still going to have major blocks of time where interrupts are disabled when they didn't have to be. I basically see no reason why sdcard operations should ever be done in a polling fashion, unless you're specifically trying to write code that runs with interrupts disabled, in which case I would expect you to call an API and say that you want sdcard operations done in polling mode.

Doing blocking file I/O inside an interrupt handler just doesn't make sense to me.

[IgorLektorovEpam]

dpgeorge, thanks for comments.
I'll improve code soon.

[dpgeorge]

There is no reason why a DMA operation can't be initiated while interrupts are disabled. Especially since it won't be finished for quite a while in the future.

SD card read operations must complete fully before returning because the caller is expecting the data. So you must wait for it to finish, and if irqs are disabled then just use polling.

SD card write operations could complete in the background, but that would require a temporary buffer to store the blocks to write, which is potentially a lot of RAM (and unknown amount of ram because you can write multiple blocks at once). But if you call sdcard_write quickly in succession then the second call will anyway need to wait for the first to finish flushing its buffers.

So, currently as the driver works you need to finish the read/write before you return. Thus you must wait, and if irqs are disabled just use polling. Otherwise use DMA and then sit in a busy wait loop waiting for the DMA to finish.

[dhylands]

There is no reason why a DMA operation can't be initiated while interrupts are disabled. Especially since it won't be finished for quite a while in the future.

SD card read operations must complete fully before returning because the caller is expecting the data. So you must wait for it to finish, and if irqs are disabled then just use polling.

you're right. I was jumping ahead too much.

SD card write operations could complete in the background, but that would require a temporary buffer to store the blocks to write, which is potentially a lot of RAM (and unknown amount of ram because you can write multiple blocks at once). But if you call sdcard_write quickly in succession then the second call will anyway need to wait for the first to finish flushing its buffers.

I think that the next logical thing to do after this is to make the following changes (not in this PR, but in another separate PR):

1 - Instead of waiting for the operation to complete, you switch things around and wait for the buffer to become available.
2 - Have the timer start the write, but not wait for the write to actually complete.

Then all waiting can happen with interrupts enabled. This will allow other interrupts (like other timers) to do work while the long sdcard operation is occurring. If we have the right kind of hook for the wait, then we can even allow other threads (assuming there is some type of cooperative threading going on) which don't need to wait on scard stuff to also do things while waiting.

[dhylands]

I'm taking a look at this (since the original author hasn't finished it).

I can't find anywhere in the code where sdcard operations are initiated from a timer (there are flash operations, but not sdcard operations).

So the question is, do we want to support sdcard operations from an IRQ? If so, then we'll need to have an IRQ initiated sdcard read/write fail if there is one already in progress (since the sdcard is essentially a half-duplex device).

This also means that we really only need 1 DMA channel since reads and writes can't happen at the same time.

[dpgeorge]

I can't find anywhere in the code where sdcard operations are initiated from a timer (there are flash operations, but not sdcard operations).

The flash write is initiated from a timer to flush the buffers after a small timeout. This is to turn many writes to a flash block (which holds many 512-byte blocks) into a single physical write.

So the question is, do we want to support sdcard operations from an IRQ?

For a first version, no.

I think just make it work like it's already almost implemented: initiate the DMA read/write, then sit there in a polling loop, waiting for the transfer to complete.

[dpgeorge]

DMA support for SD card is now implemented.