LCD_CAM example doesn't work in release

[JurajSadel]

While the lcd_i8080 example works as expected in debug (display blinking in red and blue), there is some issue in release profile - display blinking in black and white. Needs further investigation.

[Dominaezzz]

See #1651 (comment)

In short, the buffers being used end up in non-DMA capable memory, but only in release mode.

[bjoernQ]

That it's only happening in release mode doesn't make much sense to me but from a brief look something like this

        i8080.send(CMD_CSCON, 0, &[0xC3]).unwrap(); // Enable extension command 2 part I

Will locate the array in DROM. Probably making it &mut [0xC3] would help. Unfortunately, it won't help with an empty array

[MabezDev]

The FlashSafeDma wrapper would help here (but is probably missing some methods/traits for LCD_CAM), but maybe if every driver that uses DMA might need this we should build it into the DMA driver itself. I guess the hard part is sizing the temporary buffer, it might be doable once we have some configuration system.

[bjoernQ]

I thought about adding the "valid_ram_address" check in prepare_transfer_without_start but then I was wondering why using DROM doesn't trigger a DMA error and found this

Seems there are a lot of constraints when accessing external memory via DMA - not sure if we just shouldn't support it

[Dominaezzz]

My thoughts on this are similar to my proposal for SPI. The hal should have special buffer structs like DmaBuf, DmaExtRamBuf, etc. that validate the necessary hardware constraints in the constructor, then the user can then reuse these without paying for additional checks during each transfer. I feel like these better exposes the hardware capabilities (and sacrifices some ease of use).

On top of this can be a layer that does memcpy like FlashSafeDma does, and gives users a "it just works" experience (even if it's sorta slow).

but maybe if every driver that uses DMA might need this we should build it into the DMA driver itself.

I though the same thing but you then have the complication that every driver will have it's own way of how to correctly split up one big transfer into multiple chunks. i.e. You don't really want to restart the command phase of the transfer.

[Dominaezzz]

Short term though, should the example be patched to ensure the buffers are in DRAM? Or should it wait for the driver to be changed?

[bjoernQ]

Short term though, should the example be patched to ensure the buffers are in DRAM? Or should it wait for the driver to be changed?

I don't have a strong opinion here - leaving it as is doesn't make anything worse but on the other hand (and since the xtask always runs examples in release mode) it would probably be nice to have this working "out-of-the-box"

[MabezDev]

I think we should update the example, with a comment referring back to this issue so that when we fix the underlying driver we can remove it.

[playfulFence]

In the current esp-hal state (967c478), example lcd_i8080 works as expected - blinking colors from red to blue. Therefore, is this issue still valid?

[Dominaezzz]

Now that the DMA code checks that the buffer is in dram and the example is working, I can't think of a reason to keep this open.