Skip to content

Cache coherence fixes (clean_d_cache, FIO, EDMAC)

Alex requested to merge branch/cache-fixes into branch/unified

While investigating this bug with @g3gg0, we concluded it's caused by FIO_WriteFile calls from regular (cacheable) memory. This function uses (SD/CF)DMA, so before calling it, we have to clean the data cache (thus making sure all the writes performed by the CPU actually reached the physical memory).

Update: implemented the wrapper and also added a test for it (which calls FIO_WriteFile with a cacheable buffer as argument). If you comment out the clean_d_cache() call from the FIO_WriteFile wrapper, this test will fail.

FIO_ReadFile appears more tricky to handle with cacheable buffers, so I've added a wrapper that simply warns on incorrect usage. The way we do this is by calling clean_d_cache. However, while trying to find a way to reproduce the issue, I came across an old caching bug, which caused some display artifacts when drawing the cropmarks in play mode (the display device also gets the data from main memory using some sort of DMA). Here I've noticed clean_d_cache did not seem to have any effect on these artifacts.

Found the bug, fixed it, and added a test into to make sure the fix actually works. Only tested on 5D3 1.2.3.

Note: if you undo the fix (8e1d43d0dfdb), you'll notice the differences appear only on source addresses with bit 0x400 set (the second half of the cache lines). Code for counting that is not included. In other words, before the fix, clean_d_cache() worked fine with 50% probability (depending on memory_address & 0x400).

@nikfreak - do you mind checking it on 70D and 100D?

For FIO_WriteFile, g3gg0 proposed a wrapper that cleans the data cache every time it's called with a cacheable buffer address (that way, it will just work, no matter what you'll throw at it). A little extra care is needed to avoid performance regressions in e.g. raw recording. This approach will probably make obsolete the current tricks (such as fio_malloc).

Update: added wrapper to FIO_WriteFile and a test (in that calls it with cacheable buffers (placed randomly on the stack). This test will fail if you comment out the clean_d_cache() call in the FIO_WriteFile wrappers.

FIO_ReadFile appears more difficult to handle, so I've only added a wrapper that warns for incorrect usage. That means, you'll get an error right away, rather than a bug report after a few years (as it was the case with the bug linked above).

Merge request reports