-
Notifications
You must be signed in to change notification settings - Fork 3k
i.MX RT1050: Reactivate data cache #10314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is a fix for Issue 10239. The change aligns the receive buffer lengths Signed-off-by: Mahesh Mahadevan <[email protected]>
This is me working by inspection - I don't have a board to work with. I'd appreciate confirmation that this (a) works and (b) shows some sort of performance improvement. |
@kjbracey-arm, thank you for your changes. |
Since commit 12c6b1b, the i.MX RT1050 has effectively had its data cache disabled, as the SDRAM was marked Shareable; for the Cortex-M7, shareable memory is not cached. This was done to make the Ethernet driver work without any cache maintenance code. This commit adds cache maintenance and memory barriers to the Ethernet driver, and removes the Shareable attribute from the SDRAM, so the data cache is used again. Cache code in the base fsl_enet.c driver has not been activated - the bulk of it is in higher-level Read and Write calls that we're not using, and there is one flawed invalidate in its initialisation. Instead imx_emac.cpp takes full cache responsibility. This commit also marks the SDRAM as read/write-allocate. As the Cortex-M7 has its "Dynamic read allocate mode" to automatically switch back to read-allocate in cases where write allocate is working poorly (eg large memset), this should result in a performance boost with no downside. Activating write-allocate is also an attempt to provoke any flaws in cache maintenance - the Ethernet transmit buffers for example will be more likely to have a little data in the cache that needs cleaning.
@kjbracey-arm Thank you for your changes, this works fine with the test provided by @cyborg71. I am running the full suite of mbed-os tests. |
@mmahadevan108 What are the results? Can you please review this PR |
Test run: FAILEDSummary: 1 of 11 test jobs failed Failed test jobs:
|
Known failure , not related, will restart tests a bit later once some 5.12.2 PRs progress @mmahadevan108 All tests passed? |
The below is not related to this change as I am seeing this on the master branch as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kjbracey-arm Thank you for the change
That's... not a very interesting test. Something about what it leaves in RAM confuses the system after reboot? Is it reliably just that test? What if you just shuffle things around a bit by adding some dummy data in ROM or RAM? |
CI test restarted |
Description
Since commit 12c6b1b, the i.MX RT1050 has effectively had its data cache disabled, as the SDRAM was marked Shareable; for the Cortex-M7, shareable memory is not cached.
This was done to make the Ethernet driver work without any cache maintenance code. This commit adds cache maintenance and memory barriers to the Ethernet driver, and removes the Shareable attribute from the SDRAM, so the data cache is used again.
Cache code in the base fsl_enet.c driver has not been activated - the bulk of it is in higher-level Read and Write calls that we're not using, and there is one flawed invalidate in its initialisation. Instead imx_emac.cpp takes full cache responsibility.
This commit also marks the SDRAM as read/write-allocate. As the Cortex-M7 has its "Dynamic read allocate mode" to automatically switch back to read-allocate in cases where write allocate is working poorly (eg large memset), this should result in a performance boost with no downside.
Activating write-allocate is also an attempt to provoke any flaws in cache maintenance - the Ethernet transmit buffers for example will be more likely to have a little data in the cache that needs cleaning.
Incorporates and depends on #10299.
Pull request type
Reviewers
@mmahadevan108, @cyborg71